From 3803d49489863b4b8c2301986f667a2fc294682a Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Mon, 5 May 2025 16:17:27 +0200 Subject: [PATCH] Player/handleIntent: separate out the timestamp request into enum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../org/schabi/newpipe/player/Player.java | 89 +++++++++++++++++-- .../schabi/newpipe/player/PlayerIntentType.kt | 11 +++ .../schabi/newpipe/util/NavigationHelper.java | 13 +++ .../util/text/InternalUrlsHandler.java | 87 ++++-------------- .../text/TimestampLongPressClickableSpan.java | 2 +- .../util/text/UrlLongPressClickableSpan.java | 3 +- 6 files changed, 121 insertions(+), 84 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/Player.java b/app/src/main/java/org/schabi/newpipe/player/Player.java index 6fd06c31e..3021aae61 100644 --- 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.MediaSourceManager; 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.PlayerUiList; 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 java.util.stream.IntStream; 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,18 +352,31 @@ public final class Player implements PlaybackListener, Listener { @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(); @@ -363,29 +384,61 @@ public final class Player implements PlaybackListener, Listener { 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 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 final class Player implements PlaybackListener, Listener { 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 final class Player implements PlaybackListener, Listener { 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 final class Player implements PlaybackListener, Listener { 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 index 9d5e4531a..d9d83c69c 100644 --- 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 index a63fcb0c6..bc2f21c27 100644 --- 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.Player; 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 final class NavigationHelper { 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 Intent getPlayerEnqueueNextIntent(@NonNull final Context context, @NonNull final Class 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 index 2e4aa320f..8bf94f3eb 100644 --- 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.StreamingService; 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. - *

- * 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. *

@@ -71,31 +44,9 @@ public final class InternalUrlsHandler { * @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. - *

- * 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 @@ public final class InternalUrlsHandler { } 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 @@ public final class InternalUrlsHandler { * @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 final class InternalUrlsHandler { return false; } - final Single 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 index f5864794a..35a9fd996 100644 --- 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 index 61c1a546d..fd533bb5c 100644 --- a/app/src/main/java/org/schabi/newpipe/util/text/UrlLongPressClickableSpan.java +++ b/app/src/main/java/org/schabi/newpipe/util/text/UrlLongPressClickableSpan.java @@ -28,8 +28,7 @@ final class UrlLongPressClickableSpan extends LongPressClickableSpan { @Override public void onClick(@NonNull final View view) { - if (!InternalUrlsHandler.handleUrlDescriptionTimestamp( - disposables, context, url)) { + if (!InternalUrlsHandler.handleUrlDescriptionTimestamp(context, url)) { ShareUtils.openUrlInApp(context, url); } }