Player/handleIntent: separate out the timestamp request into enum

Profpatsch · Profpatsch · commit 3803d4948986 · 2025-09-06T15:06:53.000+02:00
Instead of implicitely reconstructing whether the intent was
intended (lol) to be a timestamp change, we create a new kind of
intent that *only* sets the data we need to switch to a new timestamp.

This means that the logic of what to do (opening a popup player) gets
moved from `InternalUrlsHandler.playOnPopup` to the
`Player.handleIntent` method, we only pass that we want to jump to a
new timestamp. Thus, the stream is now loaded *after* sending the
intent instead of before sending.

This is somewhat messy right now and still does not fix the issue of
queue deletion, but from now on the queue logic should get more
straightforward to implement.

In the end, everything should be a giant switch. Thus we don’t
fall-through anymore, but run the post-setup code manually by calling
`handeIntentPost` and then returning.
diff --git a/app/src/main/java/org/schabi/newpipe/player/Player.java b/app/src/main/java/org/schabi/newpipe/player/Player.java
@@ -109,6 +109,7 @@
 import org.schabi.newpipe.player.playback.PlaybackListener;
 import org.schabi.newpipe.player.playqueue.PlayQueue;
 import org.schabi.newpipe.player.playqueue.PlayQueueItem;
+import org.schabi.newpipe.player.playqueue.SinglePlayQueue;
 import org.schabi.newpipe.player.resolver.AudioPlaybackResolver;
 import org.schabi.newpipe.player.resolver.VideoPlaybackResolver;
 import org.schabi.newpipe.player.resolver.VideoPlaybackResolver.SourceType;
@@ -118,8 +119,10 @@
 import org.schabi.newpipe.player.ui.PopupPlayerUi;
 import org.schabi.newpipe.player.ui.VideoPlayerUi;
 import org.schabi.newpipe.util.DependentPreferenceHelper;
+import org.schabi.newpipe.util.ExtractorHelper;
 import org.schabi.newpipe.util.ListHelper;
 import org.schabi.newpipe.util.NavigationHelper;
+import org.schabi.newpipe.util.PermissionHelper;
 import org.schabi.newpipe.util.SerializedCache;
 import org.schabi.newpipe.util.StreamTypeUtil;
 import org.schabi.newpipe.util.image.PicassoHelper;
@@ -130,9 +133,11 @@
 
 import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers;
 import io.reactivex.rxjava3.core.Observable;
+import io.reactivex.rxjava3.core.Single;
 import io.reactivex.rxjava3.disposables.CompositeDisposable;
 import io.reactivex.rxjava3.disposables.Disposable;
 import io.reactivex.rxjava3.disposables.SerialDisposable;
+import io.reactivex.rxjava3.schedulers.Schedulers;
 
 public final class Player implements PlaybackListener, Listener {
     public static final boolean DEBUG = MainActivity.DEBUG;
@@ -160,6 +165,7 @@ public final class Player implements PlaybackListener, Listener {
     public static final String PLAY_WHEN_READY = "play_when_ready";
     public static final String PLAYER_TYPE = "player_type";
     public static final String PLAYER_INTENT_TYPE = "player_intent_type";
+    public static final String PLAYER_INTENT_DATA = "player_intent_data";
 
     /*//////////////////////////////////////////////////////////////////////////
     // Time constants
@@ -244,6 +250,8 @@ public final class Player implements PlaybackListener, Listener {
     private final SerialDisposable progressUpdateDisposable = new SerialDisposable();
     @NonNull
     private final CompositeDisposable databaseUpdateDisposable = new CompositeDisposable();
+    @NonNull
+    private final CompositeDisposable streamItemDisposable = new CompositeDisposable();
 
     // This is the only listener we need for thumbnail loading, since there is always at most only
     // one thumbnail being loaded at a time. This field is also here to maintain a strong reference,
@@ -344,48 +352,93 @@ public int getOverrideResolutionIndex(final List<VideoStream> sortedVideos,
 
     @SuppressWarnings("MethodLength")
     public void handleIntent(@NonNull final Intent intent) {
-        // fail fast if no play queue was provided
-        final String queueCache = intent.getStringExtra(PLAY_QUEUE_KEY);
-        if (queueCache == null) {
+
+        final PlayerIntentType playerIntentType = intent.getParcelableExtra(PLAYER_INTENT_TYPE);
+        if (playerIntentType == null) {
             return;
         }
-        final PlayQueue newQueue = SerializedCache.getInstance().take(queueCache, PlayQueue.class);
-        if (newQueue == null) {
-            return;
+        final PlayerType newPlayerType;
+        // TODO: this should be in the second switch below, but I’m not sure whether I
+        // can move the initUIs stuff without breaking the setup for edge cases somehow.
+        switch (playerIntentType) {
+            case TimestampChange -> {
+                // TODO: this breaks out of the pattern of asking for the permission before
+                // sending the PlayerIntent, but I’m not sure yet how to combine the permissions
+                // with the new enum approach. Maybe it’s better that the player asks anyway?
+                if (!PermissionHelper.isPopupEnabledElseAsk(context)) {
+                    return;
+                }
+                newPlayerType = PlayerType.POPUP;
+            }
+            default -> {
+                newPlayerType = PlayerType.retrieveFromIntent(intent);
+            }
         }
 
         final PlayerType oldPlayerType = playerType;
-        playerType = PlayerType.retrieveFromIntent(intent);
+        playerType = newPlayerType;
         initUIsForCurrentPlayerType();
         isAudioOnly = audioPlayerSelected();
 
         if (intent.hasExtra(PLAYBACK_QUALITY)) {
             videoResolver.setPlaybackQuality(intent.getStringExtra(PLAYBACK_QUALITY));
         }
 
-        final PlayerIntentType playerIntentType = intent.getParcelableExtra(PLAYER_INTENT_TYPE);
+        final boolean playWhenReady = intent.getBooleanExtra(PLAY_WHEN_READY, true);
 
         switch (playerIntentType) {
             case Enqueue -> {
                 if (playQueue != null) {
+                    final PlayQueue newQueue = getPlayQueueFromCache(intent);
+                    if (newQueue == null) {
+                        return;
+                    }
                     playQueue.append(newQueue.getStreams());
                 }
                 return;
             }
             case EnqueueNext -> {
                 if (playQueue != null) {
+                    final PlayQueue newQueue = getPlayQueueFromCache(intent);
+                    if (newQueue == null) {
+                        return;
+                    }
                     final int currentIndex = playQueue.getIndex();
                     playQueue.append(newQueue.getStreams());
                     playQueue.move(playQueue.size() - 1, currentIndex + 1);
                 }
                 return;
             }
+            case TimestampChange -> {
+                final TimestampChangeData dat = intent.getParcelableExtra(PLAYER_INTENT_DATA);
+                assert dat != null;
+                final Single<StreamInfo> single =
+                        ExtractorHelper.getStreamInfo(dat.getServiceId(), dat.getUrl(), false);
+                streamItemDisposable.add(single.subscribeOn(Schedulers.io())
+                        .observeOn(AndroidSchedulers.mainThread())
+                        .subscribe(info -> {
+                            final PlayQueue newPlayQueue = new SinglePlayQueue(info,
+                                    dat.getSeconds() * 1000L);
+                            NavigationHelper.playOnPopupPlayer(context, playQueue, false);
+                        }, throwable -> {
+                            // This will only show a snackbar if the passed context has a root view:
+                            // otherwise it will resort to showing a notification, so we are safe
+                            // here.
+                            ErrorUtil.createNotification(context,
+                                    new ErrorInfo(throwable, UserAction.PLAY_ON_POPUP, dat.getUrl(),
+                                            null, dat.getUrl()));
+                        }));
+                return;
+            }
             case AllOthers -> {
                 // fallthrough; TODO: put other intent data in separate cases
             }
         }
 
-        final boolean playWhenReady = intent.getBooleanExtra(PLAY_WHEN_READY, true);
+        final PlayQueue newQueue = getPlayQueueFromCache(intent);
+        if (newQueue == null) {
+            return;
+        }
 
         // branching parameters for below
         final boolean samePlayQueue = playQueue != null && playQueue.equalStreamsAndIndex(newQueue);
@@ -466,6 +519,10 @@ public void handleIntent(@NonNull final Intent intent) {
             initPlayback(samePlayQueue ? playQueue : newQueue, playWhenReady);
         }
 
+        handleIntentPost(oldPlayerType);
+    }
+
+    private void handleIntentPost(final PlayerType oldPlayerType) {
         if (oldPlayerType != playerType && playQueue != null) {
             // If playerType changes from one to another we should reload the player
             // (to disable/enable video stream or to set quality)
@@ -476,6 +533,19 @@ public void handleIntent(@NonNull final Intent intent) {
         NavigationHelper.sendPlayerStartedEvent(context);
     }
 
+    @Nullable
+    private static PlayQueue getPlayQueueFromCache(@NonNull final Intent intent) {
+        final String queueCache = intent.getStringExtra(PLAY_QUEUE_KEY);
+        if (queueCache == null) {
+            return null;
+        }
+        final PlayQueue newQueue = SerializedCache.getInstance().take(queueCache, PlayQueue.class);
+        if (newQueue == null) {
+            return null;
+        }
+        return newQueue;
+    }
+
     private void initUIsForCurrentPlayerType() {
         if ((UIs.get(MainPlayerUi.class).isPresent() && playerType == PlayerType.MAIN)
                 || (UIs.get(PopupPlayerUi.class).isPresent() && playerType == PlayerType.POPUP)) {
@@ -605,6 +675,7 @@ public void destroy() {
 
         databaseUpdateDisposable.clear();
         progressUpdateDisposable.set(null);
+        streamItemDisposable.clear();
         cancelLoadingCurrentThumbnail();
 
         UIs.destroyAll(Object.class); // destroy every UI: obviously every UI extends Object
diff --git a/app/src/main/java/org/schabi/newpipe/player/PlayerIntentType.kt b/app/src/main/java/org/schabi/newpipe/player/PlayerIntentType.kt
@@ -11,5 +11,16 @@ import kotlinx.parcelize.Parcelize
 enum class PlayerIntentType : Parcelable {
     Enqueue,
     EnqueueNext,
+    TimestampChange,
     AllOthers
 }
+
+/**
+ * A timestamp on the given was clicked and we should switch the playing stream to it.
+ */
+@Parcelize
+data class TimestampChangeData(
+    val serviceId: Int,
+    val url: String,
+    val seconds: Int
+) : Parcelable
diff --git a/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java b/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java
@@ -61,6 +61,7 @@
 import org.schabi.newpipe.player.PlayerIntentType;
 import org.schabi.newpipe.player.PlayerService;
 import org.schabi.newpipe.player.PlayerType;
+import org.schabi.newpipe.player.TimestampChangeData;
 import org.schabi.newpipe.player.helper.PlayerHelper;
 import org.schabi.newpipe.player.helper.PlayerHolder;
 import org.schabi.newpipe.player.playqueue.PlayQueue;
@@ -103,6 +104,18 @@ public static <T> Intent getPlayerIntent(@NonNull final Context context,
         return intent;
     }
 
+    @NonNull
+    public static Intent getPlayerTimestampIntent(@NonNull final Context context,
+                                                   @NonNull final TimestampChangeData
+                                                           timestampChangeData) {
+        final Intent intent = new Intent(context, PlayerService.class);
+
+        intent.putExtra(Player.PLAYER_INTENT_TYPE, (Parcelable) PlayerIntentType.TimestampChange);
+        intent.putExtra(Player.PLAYER_INTENT_DATA, timestampChangeData);
+
+        return intent;
+    }
+
     @NonNull
     public static <T> Intent getPlayerEnqueueNextIntent(@NonNull final Context context,
                                                         @NonNull final Class<T> targetClazz,
diff --git a/app/src/main/java/org/schabi/newpipe/util/text/InternalUrlsHandler.java b/app/src/main/java/org/schabi/newpipe/util/text/InternalUrlsHandler.java
@@ -1,6 +1,8 @@
 package org.schabi.newpipe.util.text;
 
 import android.content.Context;
+import android.content.Intent;
+import androidx.core.content.ContextCompat;
 
 import androidx.annotation.NonNull;
 
@@ -13,19 +15,13 @@
 import org.schabi.newpipe.extractor.exceptions.ExtractionException;
 import org.schabi.newpipe.extractor.exceptions.ParsingException;
 import org.schabi.newpipe.extractor.linkhandler.LinkHandlerFactory;
-import org.schabi.newpipe.extractor.stream.StreamInfo;
-import org.schabi.newpipe.player.playqueue.PlayQueue;
-import org.schabi.newpipe.player.playqueue.SinglePlayQueue;
-import org.schabi.newpipe.util.ExtractorHelper;
+import org.schabi.newpipe.player.TimestampChangeData;
 import org.schabi.newpipe.util.NavigationHelper;
 
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers;
-import io.reactivex.rxjava3.core.Single;
 import io.reactivex.rxjava3.disposables.CompositeDisposable;
-import io.reactivex.rxjava3.schedulers.Schedulers;
 
 public final class InternalUrlsHandler {
     private static final String TAG = InternalUrlsHandler.class.getSimpleName();
@@ -35,29 +31,6 @@ public final class InternalUrlsHandler {
     private static final Pattern HASHTAG_TIMESTAMP_PATTERN =
             Pattern.compile("(.*)#timestamp=(\\d+)");
 
-    private InternalUrlsHandler() {
-    }
-
-    /**
-     * Handle a YouTube timestamp comment URL in NewPipe.
-     * <p>
-     * This method will check if the provided url is a YouTube comment description URL ({@code
-     * https://www.youtube.com/watch?v=}video_id{@code #timestamp=}time_in_seconds). If yes, the
-     * popup player will be opened when the user will click on the timestamp in the comment,
-     * at the time and for the video indicated in the timestamp.
-     *
-     * @param disposables a field of the Activity/Fragment class that calls this method
-     * @param context     the context to use
-     * @param url         the URL to check if it can be handled
-     * @return true if the URL can be handled by NewPipe, false if it cannot
-     */
-    public static boolean handleUrlCommentsTimestamp(@NonNull final CompositeDisposable
-                                                             disposables,
-                                                     final Context context,
-                                                     @NonNull final String url) {
-        return handleUrl(context, url, HASHTAG_TIMESTAMP_PATTERN, disposables);
-    }
-
     /**
      * Handle a YouTube timestamp description URL in NewPipe.
      * <p>
@@ -71,31 +44,9 @@ public static boolean handleUrlCommentsTimestamp(@NonNull final CompositeDisposa
      * @param url         the URL to check if it can be handled
      * @return true if the URL can be handled by NewPipe, false if it cannot
      */
-    public static boolean handleUrlDescriptionTimestamp(@NonNull final CompositeDisposable
-                                                                disposables,
-                                                        final Context context,
+    public static boolean handleUrlDescriptionTimestamp(final Context context,
                                                         @NonNull final String url) {
-        return handleUrl(context, url, AMPERSAND_TIMESTAMP_PATTERN, disposables);
-    }
-
-    /**
-     * Handle an URL in NewPipe.
-     * <p>
-     * This method will check if the provided url can be handled in NewPipe or not. If this is a
-     * service URL with a timestamp, the popup player will be opened and true will be returned;
-     * else, false will be returned.
-     *
-     * @param context     the context to use
-     * @param url         the URL to check if it can be handled
-     * @param pattern     the pattern to use
-     * @param disposables a field of the Activity/Fragment class that calls this method
-     * @return true if the URL can be handled by NewPipe, false if it cannot
-     */
-    private static boolean handleUrl(final Context context,
-                                     @NonNull final String url,
-                                     @NonNull final Pattern pattern,
-                                     @NonNull final CompositeDisposable disposables) {
-        final Matcher matcher = pattern.matcher(url);
+        final Matcher matcher = AMPERSAND_TIMESTAMP_PATTERN.matcher(url);
         if (!matcher.matches()) {
             return false;
         }
@@ -120,7 +71,7 @@ private static boolean handleUrl(final Context context,
         }
 
         if (linkType == StreamingService.LinkType.STREAM && seconds != -1) {
-            return playOnPopup(context, matchedUrl, service, seconds, disposables);
+            return playOnPopup(context, matchedUrl, service, seconds);
         } else {
             NavigationHelper.openRouterActivity(context, matchedUrl);
             return true;
@@ -134,15 +85,12 @@ private static boolean handleUrl(final Context context,
      * @param url         the URL of the content
      * @param service     the service of the content
      * @param seconds     the position in seconds at which the floating player will start
-     * @param disposables disposables created by the method are added here and their lifecycle
-     *                    should be handled by the calling class
      * @return true if the playback of the content has successfully started or false if not
      */
     public static boolean playOnPopup(final Context context,
                                       final String url,
                                       @NonNull final StreamingService service,
-                                      final int seconds,
-                                      @NonNull final CompositeDisposable disposables) {
+                                      final int seconds) {
         final LinkHandlerFactory factory = service.getStreamLHFactory();
         final String cleanUrl;
 
@@ -152,19 +100,14 @@ public static boolean playOnPopup(final Context context,
             return false;
         }
 
-        final Single<StreamInfo> single =
-                ExtractorHelper.getStreamInfo(service.getServiceId(), cleanUrl, false);
-        disposables.add(single.subscribeOn(Schedulers.io())
-                .observeOn(AndroidSchedulers.mainThread())
-                .subscribe(info -> {
-                    final PlayQueue playQueue = new SinglePlayQueue(info, seconds * 1000L);
-                    NavigationHelper.playOnPopupPlayer(context, playQueue, false);
-                }, throwable -> {
-                    // This will only show a snackbar if the passed context has a root view:
-                    // otherwise it will resort to showing a notification, so we are safe here.
-                    ErrorUtil.showSnackbar(context,
-                            new ErrorInfo(throwable, UserAction.PLAY_ON_POPUP, url, null, url));
-                }));
+        final Intent intent = NavigationHelper.getPlayerTimestampIntent(context,
+                new TimestampChangeData(
+                        service.getServiceId(),
+                        cleanUrl,
+                        seconds
+                ));
+        ContextCompat.startForegroundService(context, intent);
+
         return true;
     }
 }
diff --git a/app/src/main/java/org/schabi/newpipe/util/text/TimestampLongPressClickableSpan.java b/app/src/main/java/org/schabi/newpipe/util/text/TimestampLongPressClickableSpan.java
@@ -46,7 +46,7 @@ final class TimestampLongPressClickableSpan extends LongPressClickableSpan {
     @Override
     public void onClick(@NonNull final View view) {
         playOnPopup(context, relatedStreamUrl, relatedInfoService,
-                timestampMatchDTO.seconds(), disposables);
+                timestampMatchDTO.seconds());
     }
 
     @Override
diff --git a/app/src/main/java/org/schabi/newpipe/util/text/UrlLongPressClickableSpan.java b/app/src/main/java/org/schabi/newpipe/util/text/UrlLongPressClickableSpan.java

Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,7 @@ final class TimestampLongPressClickableSpan extends LongPressClickableSpan {`
`46`	`46`	`@Override`
`47`	`47`	`public void onClick(@NonNull final View view) {`
`48`	`48`	`playOnPopup(context, relatedStreamUrl, relatedInfoService,`
`49`		`- timestampMatchDTO.seconds(), disposables);`
	`49`	`+ timestampMatchDTO.seconds());`
`50`	`50`	`}`
`51`	`51`
`52`	`52`	`@Override`