From 32eb3afe16fe5eb224600ce89843f26a74942330 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sun, 4 May 2025 20:11:03 +0200 Subject: [PATCH 01/15] Player/handleIntent: a few comments --- app/src/main/java/org/schabi/newpipe/player/Player.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 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 fe84826d6..945eba795 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -359,7 +359,6 @@ public final class Player implements PlaybackListener, Listener { final PlayerType oldPlayerType = playerType; playerType = PlayerType.retrieveFromIntent(intent); initUIsForCurrentPlayerType(); - // We need to setup audioOnly before super(), see "sourceOf" isAudioOnly = audioPlayerSelected(); if (intent.hasExtra(PLAYBACK_QUALITY)) { @@ -371,7 +370,7 @@ public final class Player implements PlaybackListener, Listener { playQueue.append(newQueue.getStreams()); return; - // Resolve enqueue next intents + // Resolve enqueue next intents } else if (intent.getBooleanExtra(ENQUEUE_NEXT, false) && playQueue != null) { final int currentIndex = playQueue.getIndex(); playQueue.append(newQueue.getStreams()); @@ -379,16 +378,18 @@ public final class Player implements PlaybackListener, Listener { return; } + // initPlayback Parameters final PlaybackParameters savedParameters = retrievePlaybackParametersFromPrefs(this); final float playbackSpeed = savedParameters.speed; final float playbackPitch = savedParameters.pitch; final boolean playbackSkipSilence = getPrefs().getBoolean(getContext().getString( R.string.playback_skip_silence_key), getPlaybackSkipSilence()); - - final boolean samePlayQueue = playQueue != null && playQueue.equalStreamsAndIndex(newQueue); final int repeatMode = intent.getIntExtra(REPEAT_MODE, getRepeatMode()); final boolean playWhenReady = intent.getBooleanExtra(PLAY_WHEN_READY, true); + // branching parameters for below + final boolean samePlayQueue = playQueue != null && playQueue.equalStreamsAndIndex(newQueue); + /* * TODO As seen in #7427 this does not work: * There are 3 situations when playback shouldn't be started from scratch (zero timestamp): From 90e1ac56ef99692976acb20ad9df5043e7ede15a Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sun, 4 May 2025 20:46:22 +0200 Subject: [PATCH 02/15] NavigationHelper: inline getPlayerEnqueueIntent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Funnily enough, I’m pretty sure that whole comment will be not necessary, because we never check `resumePlayback` on handling the intent anyway. --- .../schabi/newpipe/util/NavigationHelper.java | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) 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 e1d296297..d4afaa6c2 100644 --- a/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java @@ -111,21 +111,6 @@ public final class NavigationHelper { .putExtra(Player.PLAY_WHEN_READY, playWhenReady); } - @NonNull - public static Intent getPlayerEnqueueIntent(@NonNull final Context context, - @NonNull final Class targetClazz, - @Nullable final PlayQueue playQueue) { - // when enqueueing `resumePlayback` is always `false` since: - // - if there is a video already playing, the value of `resumePlayback` just doesn't make - // any difference. - // - if there is nothing already playing, it is useful for the enqueue action to have a - // slightly different behaviour than the normal play action: the latter resumes playback, - // the former doesn't. (note that enqueue can be triggered when nothing is playing only - // by long pressing the video detail fragment, playlist or channel controls - return getPlayerIntent(context, targetClazz, playQueue, false) - .putExtra(Player.ENQUEUE, true); - } - @NonNull public static Intent getPlayerEnqueueNextIntent(@NonNull final Context context, @NonNull final Class targetClazz, @@ -191,7 +176,15 @@ public final class NavigationHelper { } Toast.makeText(context, R.string.enqueued, Toast.LENGTH_SHORT).show(); - final Intent intent = getPlayerEnqueueIntent(context, PlayerService.class, queue); + // when enqueueing `resumePlayback` is always `false` since: + // - if there is a video already playing, the value of `resumePlayback` just doesn't make + // any difference. + // - if there is nothing already playing, it is useful for the enqueue action to have a + // slightly different behaviour than the normal play action: the latter resumes playback, + // the former doesn't. (note that enqueue can be triggered when nothing is playing only + // by long pressing the video detail fragment, playlist or channel controls + final Intent intent = getPlayerIntent(context, PlayerService.class, queue, false) + .putExtra(Player.ENQUEUE, true); intent.putExtra(Player.PLAYER_TYPE, playerType.valueForIntent()); ContextCompat.startForegroundService(context, intent); From b592403a660966b5f80e1172f8f7947424970fee Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sun, 4 May 2025 20:56:27 +0200 Subject: [PATCH 03/15] NavigationHelper: push out resumePlayback one layer --- .../player/notification/NotificationUtil.java | 3 +- .../schabi/newpipe/util/NavigationHelper.java | 30 +++++++++++-------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/notification/NotificationUtil.java b/app/src/main/java/org/schabi/newpipe/player/notification/NotificationUtil.java index cfd91a0ae..c5e8af03f 100644 --- a/app/src/main/java/org/schabi/newpipe/player/notification/NotificationUtil.java +++ b/app/src/main/java/org/schabi/newpipe/player/notification/NotificationUtil.java @@ -254,7 +254,8 @@ public final class NotificationUtil { } else { // We are playing in fragment. Don't open another activity just show fragment. That's it final Intent intent = NavigationHelper.getPlayerIntent( - player.getContext(), MainActivity.class, null, true); + player.getContext(), MainActivity.class, null); + intent.putExtra(Player.RESUME_PLAYBACK, true); intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); intent.setAction(Intent.ACTION_MAIN); intent.addCategory(Intent.CATEGORY_LAUNCHER); 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 d4afaa6c2..72c3c82a4 100644 --- a/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java @@ -84,8 +84,7 @@ public final class NavigationHelper { @NonNull public static Intent getPlayerIntent(@NonNull final Context context, @NonNull final Class targetClazz, - @Nullable final PlayQueue playQueue, - final boolean resumePlayback) { + @Nullable final PlayQueue playQueue) { final Intent intent = new Intent(context, targetClazz); if (playQueue != null) { @@ -95,7 +94,6 @@ public final class NavigationHelper { } } intent.putExtra(Player.PLAYER_TYPE, PlayerType.MAIN.valueForIntent()); - intent.putExtra(Player.RESUME_PLAYBACK, resumePlayback); intent.putExtra(PlayerService.SHOULD_START_FOREGROUND_EXTRA, true); return intent; @@ -107,17 +105,19 @@ public final class NavigationHelper { @Nullable final PlayQueue playQueue, final boolean resumePlayback, final boolean playWhenReady) { - return getPlayerIntent(context, targetClazz, playQueue, resumePlayback) - .putExtra(Player.PLAY_WHEN_READY, playWhenReady); + return getPlayerIntent(context, targetClazz, playQueue) + .putExtra(Player.PLAY_WHEN_READY, playWhenReady) + .putExtra(Player.RESUME_PLAYBACK, resumePlayback); } @NonNull public static Intent getPlayerEnqueueNextIntent(@NonNull final Context context, @NonNull final Class targetClazz, @Nullable final PlayQueue playQueue) { - // see comment in `getPlayerEnqueueIntent` as to why `resumePlayback` is false - return getPlayerIntent(context, targetClazz, playQueue, false) - .putExtra(Player.ENQUEUE_NEXT, true); + return getPlayerIntent(context, targetClazz, playQueue) + .putExtra(Player.ENQUEUE_NEXT, true) + // see comment in `getPlayerEnqueueIntent` as to why `resumePlayback` is false + .putExtra(Player.RESUME_PLAYBACK, false); } /* PLAY */ @@ -151,8 +151,9 @@ public final class NavigationHelper { Toast.makeText(context, R.string.popup_playing_toast, Toast.LENGTH_SHORT).show(); - final Intent intent = getPlayerIntent(context, PlayerService.class, queue, resumePlayback); - intent.putExtra(Player.PLAYER_TYPE, PlayerType.POPUP.valueForIntent()); + final Intent intent = getPlayerIntent(context, PlayerService.class, queue); + intent.putExtra(Player.PLAYER_TYPE, PlayerType.POPUP.valueForIntent()) + .putExtra(Player.RESUME_PLAYBACK, resumePlayback); ContextCompat.startForegroundService(context, intent); } @@ -162,8 +163,9 @@ public final class NavigationHelper { Toast.makeText(context, R.string.background_player_playing_toast, Toast.LENGTH_SHORT) .show(); - final Intent intent = getPlayerIntent(context, PlayerService.class, queue, resumePlayback); + final Intent intent = getPlayerIntent(context, PlayerService.class, queue); intent.putExtra(Player.PLAYER_TYPE, PlayerType.AUDIO.valueForIntent()); + intent.putExtra(Player.RESUME_PLAYBACK, resumePlayback); ContextCompat.startForegroundService(context, intent); } @@ -176,6 +178,7 @@ public final class NavigationHelper { } Toast.makeText(context, R.string.enqueued, Toast.LENGTH_SHORT).show(); + // when enqueueing `resumePlayback` is always `false` since: // - if there is a video already playing, the value of `resumePlayback` just doesn't make // any difference. @@ -183,8 +186,9 @@ public final class NavigationHelper { // slightly different behaviour than the normal play action: the latter resumes playback, // the former doesn't. (note that enqueue can be triggered when nothing is playing only // by long pressing the video detail fragment, playlist or channel controls - final Intent intent = getPlayerIntent(context, PlayerService.class, queue, false) - .putExtra(Player.ENQUEUE, true); + final Intent intent = getPlayerIntent(context, PlayerService.class, queue) + .putExtra(Player.ENQUEUE, true) + .putExtra(Player.RESUME_PLAYBACK, false); intent.putExtra(Player.PLAYER_TYPE, playerType.valueForIntent()); ContextCompat.startForegroundService(context, intent); From e14ec3a4f9c98c20c210fd8100c10fd40e9d9bd8 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sun, 4 May 2025 21:01:22 +0200 Subject: [PATCH 04/15] NavigationHelper: inline trivial getPlayerIntent use --- .../newpipe/fragments/detail/VideoDetailFragment.java | 7 +++++-- .../org/schabi/newpipe/util/NavigationHelper.java | 11 ----------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java index 342333aa5..4d273a859 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java @@ -1166,8 +1166,11 @@ public final class VideoDetailFragment final PlayQueue queue = setupPlayQueueForIntent(false); tryAddVideoPlayerView(); - final Intent playerIntent = NavigationHelper.getPlayerIntent(requireContext(), - PlayerService.class, queue, true, autoPlayEnabled); + final Context context = requireContext(); + final Intent playerIntent = + NavigationHelper.getPlayerIntent(context, PlayerService.class, queue) + .putExtra(Player.PLAY_WHEN_READY, autoPlayEnabled) + .putExtra(Player.RESUME_PLAYBACK, true); ContextCompat.startForegroundService(activity, playerIntent); } 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 72c3c82a4..1dccf501f 100644 --- a/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java @@ -99,17 +99,6 @@ public final class NavigationHelper { return intent; } - @NonNull - public static Intent getPlayerIntent(@NonNull final Context context, - @NonNull final Class targetClazz, - @Nullable final PlayQueue playQueue, - final boolean resumePlayback, - final boolean playWhenReady) { - return getPlayerIntent(context, targetClazz, playQueue) - .putExtra(Player.PLAY_WHEN_READY, playWhenReady) - .putExtra(Player.RESUME_PLAYBACK, resumePlayback); - } - @NonNull public static Intent getPlayerEnqueueNextIntent(@NonNull final Context context, @NonNull final Class targetClazz, From fd24c08529e31c78052b5df28530760362aa669c Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sun, 4 May 2025 21:07:35 +0200 Subject: [PATCH 05/15] Player/handleIntent: de morgan samePlayQueue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Okay, so this is the … only? branch in this if-chain that will conditionally fire if `playQueue` *is* `null`, sometimes. This is why the unconditional `initPlayback` in `else` is not passed a `null` in many cases … because `RESUME_PLAYBACK` is `true` and `playQueue` is `null`. It’s gonna be hard to figure out which parts of that are intentional, I say. --- app/src/main/java/org/schabi/newpipe/player/Player.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 945eba795..1668d4ded 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -429,7 +429,8 @@ public final class Player implements PlaybackListener, Listener { } else if (intent.getBooleanExtra(RESUME_PLAYBACK, false) && DependentPreferenceHelper.getResumePlaybackEnabled(context) - && !samePlayQueue + // !samePlayQueue + && (playQueue == null || !playQueue.equalStreamsAndIndex(newQueue)) && !newQueue.isEmpty() && newQueue.getItem() != null && newQueue.getItem().getRecoveryPosition() == PlayQueueItem.RECOVERY_UNSET) { From ab7d1377e50d6a8306864ed5fe5d16936d936e9d Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sun, 4 May 2025 21:37:14 +0200 Subject: [PATCH 06/15] Player/handleIntent: always early return on ENQUEUE an ENQUEUE_NEXT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can do this, because: 1. if `playQueue` is not null, we return early 2. if `playQueue` is null and we need to enqueue: - the only “proper” case that could be triggered is the `RESUME_PLAYBACK` case, which is never `true` for the queuing intents, see the comment in `NavigationHelper.enqueueOnPlayer` - the generic `else` case is degenerate, because it would crash on `playQueue` being `null`. This makes some sense, because there is no way to trigger the enqueueing logic via the UI currently if there is no video playing yet, in which case `playQueue` is not `null`. So we need to transform this whole if desaster into a big switch. --- .../java/org/schabi/newpipe/player/Player.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 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 1668d4ded..87f0025cf 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -366,15 +366,20 @@ public final class Player implements PlaybackListener, Listener { } // Resolve enqueue intents - if (intent.getBooleanExtra(ENQUEUE, false) && playQueue != null) { - playQueue.append(newQueue.getStreams()); + if (intent.getBooleanExtra(ENQUEUE, false)) { + if (playQueue != null) { + playQueue.append(newQueue.getStreams()); + } return; + } // Resolve enqueue next intents - } else if (intent.getBooleanExtra(ENQUEUE_NEXT, false) && playQueue != null) { - final int currentIndex = playQueue.getIndex(); - playQueue.append(newQueue.getStreams()); - playQueue.move(playQueue.size() - 1, currentIndex + 1); + if (intent.getBooleanExtra(ENQUEUE_NEXT, false)) { + if (playQueue != null) { + final int currentIndex = playQueue.getIndex(); + playQueue.append(newQueue.getStreams()); + playQueue.move(playQueue.size() - 1, currentIndex + 1); + } return; } From 5750ef6aa8b0d3dbee9c589699cd1e5e1d001027 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sun, 4 May 2025 23:46:29 +0200 Subject: [PATCH 07/15] Player/handleIntent: start converting intent data to enum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The goal here is to convert all player intents to use a single enum with extra data for each case. The queue ones are pretty easy, they don’t carry any extra data. We fall through for everything else for now. --- .../fragments/detail/VideoDetailFragment.java | 4 ++- .../org/schabi/newpipe/player/Player.java | 35 ++++++++++--------- .../schabi/newpipe/player/PlayerIntentType.kt | 15 ++++++++ .../player/notification/NotificationUtil.java | 4 ++- .../schabi/newpipe/util/NavigationHelper.java | 19 ++++++---- 5 files changed, 52 insertions(+), 25 deletions(-) create mode 100644 app/src/main/java/org/schabi/newpipe/player/PlayerIntentType.kt diff --git a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java index 4d273a859..c43007da4 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java @@ -93,6 +93,7 @@ import org.schabi.newpipe.local.dialog.PlaylistDialog; import org.schabi.newpipe.local.history.HistoryRecordManager; import org.schabi.newpipe.local.playlist.LocalPlaylistFragment; 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.event.OnKeyDownListener; @@ -1168,7 +1169,8 @@ public final class VideoDetailFragment final Context context = requireContext(); final Intent playerIntent = - NavigationHelper.getPlayerIntent(context, PlayerService.class, queue) + NavigationHelper.getPlayerIntent(context, PlayerService.class, queue, + PlayerIntentType.AllOthers) .putExtra(Player.PLAY_WHEN_READY, autoPlayEnabled) .putExtra(Player.RESUME_PLAYBACK, true); ContextCompat.startForegroundService(activity, playerIntent); 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 87f0025cf..e3143fddb 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -157,11 +157,10 @@ public final class Player implements PlaybackListener, Listener { public static final String REPEAT_MODE = "repeat_mode"; public static final String PLAYBACK_QUALITY = "playback_quality"; public static final String PLAY_QUEUE_KEY = "play_queue_key"; - public static final String ENQUEUE = "enqueue"; - public static final String ENQUEUE_NEXT = "enqueue_next"; public static final String RESUME_PLAYBACK = "resume_playback"; 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"; /*////////////////////////////////////////////////////////////////////////// // Time constants @@ -365,22 +364,26 @@ public final class Player implements PlaybackListener, Listener { videoResolver.setPlaybackQuality(intent.getStringExtra(PLAYBACK_QUALITY)); } - // Resolve enqueue intents - if (intent.getBooleanExtra(ENQUEUE, false)) { - if (playQueue != null) { - playQueue.append(newQueue.getStreams()); - } - return; - } + final PlayerIntentType playerIntentType = intent.getParcelableExtra(PLAYER_INTENT_TYPE); - // Resolve enqueue next intents - if (intent.getBooleanExtra(ENQUEUE_NEXT, false)) { - if (playQueue != null) { - final int currentIndex = playQueue.getIndex(); - playQueue.append(newQueue.getStreams()); - playQueue.move(playQueue.size() - 1, currentIndex + 1); + switch (playerIntentType) { + case Enqueue -> { + if (playQueue != null) { + playQueue.append(newQueue.getStreams()); + } + return; + } + case EnqueueNext -> { + if (playQueue != null) { + final int currentIndex = playQueue.getIndex(); + playQueue.append(newQueue.getStreams()); + playQueue.move(playQueue.size() - 1, currentIndex + 1); + } + return; + } + case AllOthers -> { + // fallthrough; TODO: put other intent data in separate cases } - return; } // initPlayback Parameters diff --git a/app/src/main/java/org/schabi/newpipe/player/PlayerIntentType.kt b/app/src/main/java/org/schabi/newpipe/player/PlayerIntentType.kt new file mode 100644 index 000000000..9d5e4531a --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/player/PlayerIntentType.kt @@ -0,0 +1,15 @@ +package org.schabi.newpipe.player + +import android.os.Parcelable +import kotlinx.parcelize.Parcelize + +// We model this as an enum class plus one struct for each enum value +// so we can consume it from Java properly. After converting to Kotlin, +// we could switch to a sealed enum class & a proper Kotlin `when` match. + +@Parcelize +enum class PlayerIntentType : Parcelable { + Enqueue, + EnqueueNext, + AllOthers +} diff --git a/app/src/main/java/org/schabi/newpipe/player/notification/NotificationUtil.java b/app/src/main/java/org/schabi/newpipe/player/notification/NotificationUtil.java index c5e8af03f..79ae81de2 100644 --- a/app/src/main/java/org/schabi/newpipe/player/notification/NotificationUtil.java +++ b/app/src/main/java/org/schabi/newpipe/player/notification/NotificationUtil.java @@ -23,6 +23,7 @@ import androidx.core.content.ContextCompat; import org.schabi.newpipe.MainActivity; import org.schabi.newpipe.R; import org.schabi.newpipe.player.Player; +import org.schabi.newpipe.player.PlayerIntentType; import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi; import org.schabi.newpipe.util.NavigationHelper; @@ -254,7 +255,8 @@ public final class NotificationUtil { } else { // We are playing in fragment. Don't open another activity just show fragment. That's it final Intent intent = NavigationHelper.getPlayerIntent( - player.getContext(), MainActivity.class, null); + player.getContext(), MainActivity.class, null, + PlayerIntentType.AllOthers); intent.putExtra(Player.RESUME_PLAYBACK, true); intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); intent.setAction(Intent.ACTION_MAIN); 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 1dccf501f..a63fcb0c6 100644 --- a/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java @@ -9,6 +9,7 @@ import android.content.Context; import android.content.Intent; import android.net.Uri; import android.os.Build; +import android.os.Parcelable; import android.util.Log; import android.widget.Toast; @@ -57,6 +58,7 @@ import org.schabi.newpipe.local.subscription.SubscriptionFragment; import org.schabi.newpipe.local.subscription.SubscriptionsImportFragment; import org.schabi.newpipe.player.PlayQueueActivity; 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.helper.PlayerHelper; @@ -84,7 +86,8 @@ public final class NavigationHelper { @NonNull public static Intent getPlayerIntent(@NonNull final Context context, @NonNull final Class targetClazz, - @Nullable final PlayQueue playQueue) { + @Nullable final PlayQueue playQueue, + @NonNull final PlayerIntentType playerIntentType) { final Intent intent = new Intent(context, targetClazz); if (playQueue != null) { @@ -95,6 +98,7 @@ public final class NavigationHelper { } intent.putExtra(Player.PLAYER_TYPE, PlayerType.MAIN.valueForIntent()); intent.putExtra(PlayerService.SHOULD_START_FOREGROUND_EXTRA, true); + intent.putExtra(Player.PLAYER_INTENT_TYPE, (Parcelable) playerIntentType); return intent; } @@ -103,8 +107,7 @@ public final class NavigationHelper { public static Intent getPlayerEnqueueNextIntent(@NonNull final Context context, @NonNull final Class targetClazz, @Nullable final PlayQueue playQueue) { - return getPlayerIntent(context, targetClazz, playQueue) - .putExtra(Player.ENQUEUE_NEXT, true) + return getPlayerIntent(context, targetClazz, playQueue, PlayerIntentType.EnqueueNext) // see comment in `getPlayerEnqueueIntent` as to why `resumePlayback` is false .putExtra(Player.RESUME_PLAYBACK, false); } @@ -140,7 +143,8 @@ public final class NavigationHelper { Toast.makeText(context, R.string.popup_playing_toast, Toast.LENGTH_SHORT).show(); - final Intent intent = getPlayerIntent(context, PlayerService.class, queue); + final Intent intent = getPlayerIntent(context, PlayerService.class, queue, + PlayerIntentType.AllOthers); intent.putExtra(Player.PLAYER_TYPE, PlayerType.POPUP.valueForIntent()) .putExtra(Player.RESUME_PLAYBACK, resumePlayback); ContextCompat.startForegroundService(context, intent); @@ -152,7 +156,8 @@ public final class NavigationHelper { Toast.makeText(context, R.string.background_player_playing_toast, Toast.LENGTH_SHORT) .show(); - final Intent intent = getPlayerIntent(context, PlayerService.class, queue); + final Intent intent = getPlayerIntent(context, PlayerService.class, queue, + PlayerIntentType.AllOthers); intent.putExtra(Player.PLAYER_TYPE, PlayerType.AUDIO.valueForIntent()); intent.putExtra(Player.RESUME_PLAYBACK, resumePlayback); ContextCompat.startForegroundService(context, intent); @@ -175,8 +180,8 @@ public final class NavigationHelper { // slightly different behaviour than the normal play action: the latter resumes playback, // the former doesn't. (note that enqueue can be triggered when nothing is playing only // by long pressing the video detail fragment, playlist or channel controls - final Intent intent = getPlayerIntent(context, PlayerService.class, queue) - .putExtra(Player.ENQUEUE, true) + final Intent intent = getPlayerIntent(context, PlayerService.class, queue, + PlayerIntentType.Enqueue) .putExtra(Player.RESUME_PLAYBACK, false); intent.putExtra(Player.PLAYER_TYPE, playerType.valueForIntent()); From 8fb3e90fe17f000bdeb93efefffb2eae271b2a1a Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Mon, 5 May 2025 15:33:20 +0200 Subject: [PATCH 08/15] Player: remove unused REPEAT_MODE intent key --- .../main/java/org/schabi/newpipe/player/Player.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 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 e3143fddb..88d292392 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -154,7 +154,6 @@ public final class Player implements PlaybackListener, Listener { // Intent //////////////////////////////////////////////////////////////////////////*/ - public static final String REPEAT_MODE = "repeat_mode"; public static final String PLAYBACK_QUALITY = "playback_quality"; public static final String PLAY_QUEUE_KEY = "play_queue_key"; public static final String RESUME_PLAYBACK = "resume_playback"; @@ -392,7 +391,6 @@ public final class Player implements PlaybackListener, Listener { final float playbackPitch = savedParameters.pitch; final boolean playbackSkipSilence = getPrefs().getBoolean(getContext().getString( R.string.playback_skip_silence_key), getPlaybackSkipSilence()); - final int repeatMode = intent.getIntExtra(REPEAT_MODE, getRepeatMode()); final boolean playWhenReady = intent.getBooleanExtra(PLAY_WHEN_READY, true); // branching parameters for below @@ -454,7 +452,7 @@ public final class Player implements PlaybackListener, Listener { newQueue.setRecovery(newQueue.getIndex(), state.getProgressMillis()); } - initPlayback(newQueue, repeatMode, playbackSpeed, playbackPitch, + initPlayback(newQueue, playbackSpeed, playbackPitch, playbackSkipSilence, playWhenReady); }, error -> { @@ -462,19 +460,19 @@ public final class Player implements PlaybackListener, Listener { Log.w(TAG, "Failed to start playback", error); } // In case any error we can start playback without history - initPlayback(newQueue, repeatMode, playbackSpeed, playbackPitch, + initPlayback(newQueue, playbackSpeed, playbackPitch, playbackSkipSilence, playWhenReady); }, () -> { // Completed but not found in history - initPlayback(newQueue, repeatMode, playbackSpeed, playbackPitch, + initPlayback(newQueue, playbackSpeed, playbackPitch, playbackSkipSilence, playWhenReady); } )); } else { // Good to go... // In a case of equal PlayQueues we can re-init old one but only when it is disposed - initPlayback(samePlayQueue ? playQueue : newQueue, repeatMode, playbackSpeed, + initPlayback(samePlayQueue ? playQueue : newQueue, playbackSpeed, playbackPitch, playbackSkipSilence, playWhenReady); } @@ -521,14 +519,12 @@ public final class Player implements PlaybackListener, Listener { } private void initPlayback(@NonNull final PlayQueue queue, - @RepeatMode final int repeatMode, final float playbackSpeed, final float playbackPitch, final boolean playbackSkipSilence, final boolean playOnReady) { destroyPlayer(); initPlayer(playOnReady); - setRepeatMode(repeatMode); setPlaybackParameters(playbackSpeed, playbackPitch, playbackSkipSilence); playQueue = queue; From d534946550ba0fcc90eb6fa7716eec314d376a44 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Mon, 5 May 2025 15:40:16 +0200 Subject: [PATCH 09/15] Player: inline repeat mode cycling --- .../org/schabi/newpipe/player/Player.java | 21 +++++++++++++------ .../newpipe/player/helper/PlayerHelper.java | 18 ---------------- 2 files changed, 15 insertions(+), 24 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 88d292392..f92c72415 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -24,12 +24,12 @@ import static com.google.android.exoplayer2.Player.DISCONTINUITY_REASON_SEEK_ADJ import static com.google.android.exoplayer2.Player.DISCONTINUITY_REASON_SKIP; import static com.google.android.exoplayer2.Player.DiscontinuityReason; import static com.google.android.exoplayer2.Player.Listener; +import static com.google.android.exoplayer2.Player.REPEAT_MODE_ALL; import static com.google.android.exoplayer2.Player.REPEAT_MODE_OFF; import static com.google.android.exoplayer2.Player.REPEAT_MODE_ONE; import static com.google.android.exoplayer2.Player.RepeatMode; import static org.schabi.newpipe.extractor.ServiceList.YouTube; import static org.schabi.newpipe.extractor.utils.Utils.isNullOrEmpty; -import static org.schabi.newpipe.player.helper.PlayerHelper.nextRepeatMode; import static org.schabi.newpipe.player.helper.PlayerHelper.retrievePlaybackParametersFromPrefs; import static org.schabi.newpipe.player.helper.PlayerHelper.retrieveSeekDurationFromPreferences; import static org.schabi.newpipe.player.helper.PlayerHelper.savePlaybackParametersToPrefs; @@ -1181,16 +1181,25 @@ public final class Player implements PlaybackListener, Listener { return exoPlayerIsNull() ? REPEAT_MODE_OFF : simpleExoPlayer.getRepeatMode(); } - public void setRepeatMode(@RepeatMode final int repeatMode) { + public void cycleNextRepeatMode() { if (!exoPlayerIsNull()) { + @RepeatMode final int repeatMode; + switch (simpleExoPlayer.getRepeatMode()) { + case REPEAT_MODE_OFF: + repeatMode = REPEAT_MODE_ONE; + break; + case REPEAT_MODE_ONE: + repeatMode = REPEAT_MODE_ALL; + break; + case REPEAT_MODE_ALL: + default: + repeatMode = REPEAT_MODE_OFF; + break; + } simpleExoPlayer.setRepeatMode(repeatMode); } } - public void cycleNextRepeatMode() { - setRepeatMode(nextRepeatMode(getRepeatMode())); - } - @Override public void onRepeatModeChanged(@RepeatMode final int repeatMode) { if (DEBUG) { diff --git a/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHelper.java b/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHelper.java index 266d65f36..3c69ff78b 100644 --- a/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHelper.java +++ b/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHelper.java @@ -1,8 +1,5 @@ package org.schabi.newpipe.player.helper; -import static com.google.android.exoplayer2.Player.REPEAT_MODE_ALL; -import static com.google.android.exoplayer2.Player.REPEAT_MODE_OFF; -import static com.google.android.exoplayer2.Player.REPEAT_MODE_ONE; import static org.schabi.newpipe.player.helper.PlayerHelper.AutoplayType.AUTOPLAY_TYPE_ALWAYS; import static org.schabi.newpipe.player.helper.PlayerHelper.AutoplayType.AUTOPLAY_TYPE_NEVER; import static org.schabi.newpipe.player.helper.PlayerHelper.AutoplayType.AUTOPLAY_TYPE_WIFI; @@ -25,7 +22,6 @@ import androidx.core.content.ContextCompat; import androidx.preference.PreferenceManager; import com.google.android.exoplayer2.PlaybackParameters; -import com.google.android.exoplayer2.Player.RepeatMode; import com.google.android.exoplayer2.SeekParameters; import com.google.android.exoplayer2.source.ProgressiveMediaSource; import com.google.android.exoplayer2.trackselection.AdaptiveTrackSelection; @@ -410,23 +406,9 @@ public final class PlayerHelper { return singlePlayQueue; } - // endregion // region Utils used by player - @RepeatMode - public static int nextRepeatMode(@RepeatMode final int repeatMode) { - switch (repeatMode) { - case REPEAT_MODE_OFF: - return REPEAT_MODE_ONE; - case REPEAT_MODE_ONE: - return REPEAT_MODE_ALL; - case REPEAT_MODE_ALL: - default: - return REPEAT_MODE_OFF; - } - } - @ResizeMode public static int retrieveResizeModeFromPrefs(final Player player) { return player.getPrefs().getInt(player.getContext().getString(R.string.last_resize_mode), From 25a4a9a2535203b25af0d217ea1a2f3f4831e1eb Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Mon, 5 May 2025 15:48:26 +0200 Subject: [PATCH 10/15] Player/handleIntent: move prefs parameters into initPlayback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit They are just read from the player preferences and don’t influence the branching, no need to read them in the intent parsing logic. --- .../org/schabi/newpipe/player/Player.java | 26 ++++++------------- 1 file changed, 8 insertions(+), 18 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 f92c72415..6fd06c31e 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -385,12 +385,6 @@ public final class Player implements PlaybackListener, Listener { } } - // initPlayback Parameters - final PlaybackParameters savedParameters = retrievePlaybackParametersFromPrefs(this); - final float playbackSpeed = savedParameters.speed; - final float playbackPitch = savedParameters.pitch; - final boolean playbackSkipSilence = getPrefs().getBoolean(getContext().getString( - R.string.playback_skip_silence_key), getPlaybackSkipSilence()); final boolean playWhenReady = intent.getBooleanExtra(PLAY_WHEN_READY, true); // branching parameters for below @@ -452,28 +446,24 @@ public final class Player implements PlaybackListener, Listener { newQueue.setRecovery(newQueue.getIndex(), state.getProgressMillis()); } - initPlayback(newQueue, playbackSpeed, playbackPitch, - playbackSkipSilence, playWhenReady); + initPlayback(newQueue, playWhenReady); }, error -> { if (DEBUG) { Log.w(TAG, "Failed to start playback", error); } // In case any error we can start playback without history - initPlayback(newQueue, playbackSpeed, playbackPitch, - playbackSkipSilence, playWhenReady); + initPlayback(newQueue, playWhenReady); }, () -> { // Completed but not found in history - initPlayback(newQueue, playbackSpeed, playbackPitch, - playbackSkipSilence, playWhenReady); + initPlayback(newQueue, playWhenReady); } )); } else { // Good to go... // In a case of equal PlayQueues we can re-init old one but only when it is disposed - initPlayback(samePlayQueue ? playQueue : newQueue, playbackSpeed, - playbackPitch, playbackSkipSilence, playWhenReady); + initPlayback(samePlayQueue ? playQueue : newQueue, playWhenReady); } if (oldPlayerType != playerType && playQueue != null) { @@ -519,13 +509,13 @@ public final class Player implements PlaybackListener, Listener { } private void initPlayback(@NonNull final PlayQueue queue, - final float playbackSpeed, - final float playbackPitch, - final boolean playbackSkipSilence, final boolean playOnReady) { destroyPlayer(); initPlayer(playOnReady); - setPlaybackParameters(playbackSpeed, playbackPitch, playbackSkipSilence); + final boolean playbackSkipSilence = getPrefs().getBoolean(getContext().getString( + R.string.playback_skip_silence_key), getPlaybackSkipSilence()); + final PlaybackParameters savedParameters = retrievePlaybackParametersFromPrefs(this); + setPlaybackParameters(savedParameters.speed, savedParameters.pitch, playbackSkipSilence); playQueue = queue; playQueue.init(); From 3803d49489863b4b8c2301986f667a2fc294682a Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Mon, 5 May 2025 16:17:27 +0200 Subject: [PATCH 11/15] 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); } } From 150649aea9f2cf55b7b006d5538aaecd2ec57f42 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Mon, 5 May 2025 17:59:34 +0200 Subject: [PATCH 12/15] =?UTF-8?q?Player/handleIntent:=20Don=E2=80=99t=20de?= =?UTF-8?q?lete=20queue=20when=20clicking=20on=20timestamp?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes https://github.com/TeamNewPipe/NewPipe/issues/11013 We finally are at the point where we can have good logic around clicking on timestamps. This is pretty straightforward: 1) if we are already playing the stream (usual case), we skip to the correct second directly 2) If we don’t have a queue yet, create a trivial one with the stream 3) If we have a queue, we insert the video as next item and start playing it. The skipping logic in 1) is similar to the one further down in the old optimization block, but will always correctly fire for timestamps now. I copied it because it’s not quite the same code, and moving into a separate method at this stage would complicate the code too much. --- .../org/schabi/newpipe/player/Player.java | 48 ++++++++++++++++--- .../newpipe/player/playqueue/PlayQueue.java | 19 +++++++- .../player/playqueue/PlayQueueItem.java | 18 ++++++- .../player/playqueue/SinglePlayQueue.java | 4 +- 4 files changed, 78 insertions(+), 11 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 3021aae61..ee4081068 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -403,9 +403,8 @@ public final class Player implements PlaybackListener, Listener { if (newQueue == null) { return; } - final int currentIndex = playQueue.getIndex(); - playQueue.append(newQueue.getStreams()); - playQueue.move(playQueue.size() - 1, currentIndex + 1); + final PlayQueueItem newItem = newQueue.getStreams().get(0); + newQueue.enqueueNext(newItem, false); } return; } @@ -417,9 +416,43 @@ public final class Player implements PlaybackListener, Listener { 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); + final @Nullable PlayQueue oldPlayQueue = playQueue; + info.setStartPosition(dat.getSeconds()); + final PlayQueueItem playQueueItem = new PlayQueueItem(info); + + // If the stream is already playing, + // we can just seek to the appropriate timestamp + if (oldPlayQueue != null + && playQueueItem.isSameItem(oldPlayQueue.getItem())) { + // Player can have state = IDLE when playback is stopped or failed + // and we should retry in this case + if (simpleExoPlayer.getPlaybackState() + == com.google.android.exoplayer2.Player.STATE_IDLE) { + simpleExoPlayer.prepare(); + } + simpleExoPlayer.seekTo(oldPlayQueue.getIndex(), + dat.getSeconds() * 1000L); + simpleExoPlayer.setPlayWhenReady(playWhenReady); + + } else { + final PlayQueue newPlayQueue; + + // If there is no queue yet, just add our item + if (oldPlayQueue == null) { + newPlayQueue = new SinglePlayQueue(playQueueItem); + + // else we add the timestamped stream behind the current video + // and start playing it. + } else { + oldPlayQueue.enqueueNext(playQueueItem, true); + oldPlayQueue.offsetIndex(1); + newPlayQueue = oldPlayQueue; + } + initPlayback(newPlayQueue, playWhenReady); + } + + handleIntentPost(oldPlayerType); + }, 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 @@ -456,7 +489,7 @@ public final class Player implements PlaybackListener, Listener { if (!exoPlayerIsNull() && newQueue.size() == 1 && newQueue.getItem() != null && playQueue != null && playQueue.size() == 1 && playQueue.getItem() != null - && newQueue.getItem().getUrl().equals(playQueue.getItem().getUrl()) + && newQueue.getItem().isSameItem(playQueue.getItem()) && newQueue.getItem().getRecoveryPosition() != PlayQueueItem.RECOVERY_UNSET) { // Player can have state = IDLE when playback is stopped or failed // and we should retry in this case @@ -522,6 +555,7 @@ public final class Player implements PlaybackListener, Listener { 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 diff --git a/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueue.java b/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueue.java index cfa2ab316..97196805d 100644 --- a/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueue.java +++ b/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueue.java @@ -291,6 +291,22 @@ public abstract class PlayQueue implements Serializable { broadcast(new AppendEvent(itemList.size())); } + /** + * Add the given item after the current stream. + * + * @param item item to add. + * @param skipIfSame if set, skip adding if the next stream is the same stream. + */ + public void enqueueNext(@NonNull final PlayQueueItem item, final boolean skipIfSame) { + final int currentIndex = getIndex(); + // if the next item is the same item as the one we want to enqueue, skip if flag is true + if (skipIfSame && item.isSameItem(getItem(currentIndex + 1))) { + return; + } + append(List.of(item)); + move(size() - 1, currentIndex + 1); + } + /** * Removes the item at the given index from the play queue. *

@@ -529,8 +545,7 @@ public abstract class PlayQueue implements Serializable { final PlayQueueItem stream = streams.get(i); final PlayQueueItem otherStream = other.streams.get(i); // Check is based on serviceId and URL - if (stream.getServiceId() != otherStream.getServiceId() - || !stream.getUrl().equals(otherStream.getUrl())) { + if (!stream.isSameItem(otherStream)) { return false; } } diff --git a/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueueItem.java b/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueueItem.java index 759c51267..d1d897c39 100644 --- a/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueueItem.java +++ b/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueueItem.java @@ -38,7 +38,7 @@ public class PlayQueueItem implements Serializable { private long recoveryPosition; private Throwable error; - PlayQueueItem(@NonNull final StreamInfo info) { + public PlayQueueItem(@NonNull final StreamInfo info) { this(info.getName(), info.getUrl(), info.getServiceId(), info.getDuration(), info.getThumbnails(), info.getUploaderName(), info.getUploaderUrl(), info.getStreamType()); @@ -71,6 +71,22 @@ public class PlayQueueItem implements Serializable { this.recoveryPosition = RECOVERY_UNSET; } + /** Whether these two items should be treated as the same stream + * for the sake of keeping the same player running when e.g. jumping between timestamps. + * + * @param other the {@link PlayQueueItem} to compare against. + * @return whether the two items are the same so the stream can be re-used. + */ + public boolean isSameItem(@Nullable final PlayQueueItem other) { + if (other == null) { + return false; + } + // We assume that the same service & URL uniquely determines + // that we can keep the same stream running. + return serviceId == other.serviceId + && url.equals(other.url); + } + @NonNull public String getTitle() { return title; diff --git a/app/src/main/java/org/schabi/newpipe/player/playqueue/SinglePlayQueue.java b/app/src/main/java/org/schabi/newpipe/player/playqueue/SinglePlayQueue.java index 0eb0f235a..f13d7924d 100644 --- a/app/src/main/java/org/schabi/newpipe/player/playqueue/SinglePlayQueue.java +++ b/app/src/main/java/org/schabi/newpipe/player/playqueue/SinglePlayQueue.java @@ -16,7 +16,9 @@ public final class SinglePlayQueue extends PlayQueue { public SinglePlayQueue(final StreamInfo info) { super(0, List.of(new PlayQueueItem(info))); } - + public SinglePlayQueue(final PlayQueueItem item) { + super(0, List.of(item)); + } public SinglePlayQueue(final StreamInfo info, final long startPosition) { super(0, List.of(new PlayQueueItem(info))); getItem().setRecoveryPosition(startPosition); From 01f9a3de33b71cb44a3fadf68681c4972001d7f6 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Tue, 6 May 2025 09:58:41 +0200 Subject: [PATCH 13/15] Fix Checkstyle & remove unused fields --- .../newpipe/util/text/InternalUrlsHandler.java | 16 +++------------- .../schabi/newpipe/util/text/TextLinkifier.java | 4 ++-- .../util/text/UrlLongPressClickableSpan.java | 6 ------ 3 files changed, 5 insertions(+), 21 deletions(-) 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 8bf94f3eb..3288b4347 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 @@ -5,11 +5,6 @@ import android.content.Intent; import androidx.core.content.ContextCompat; import androidx.annotation.NonNull; - -import org.schabi.newpipe.MainActivity; -import org.schabi.newpipe.error.ErrorInfo; -import org.schabi.newpipe.error.ErrorUtil; -import org.schabi.newpipe.error.UserAction; import org.schabi.newpipe.extractor.NewPipe; import org.schabi.newpipe.extractor.StreamingService; import org.schabi.newpipe.extractor.exceptions.ExtractionException; @@ -21,15 +16,11 @@ import org.schabi.newpipe.util.NavigationHelper; import java.util.regex.Matcher; import java.util.regex.Pattern; -import io.reactivex.rxjava3.disposables.CompositeDisposable; - public final class InternalUrlsHandler { - private static final String TAG = InternalUrlsHandler.class.getSimpleName(); - private static final boolean DEBUG = MainActivity.DEBUG; - private static final Pattern AMPERSAND_TIMESTAMP_PATTERN = Pattern.compile("(.*)&t=(\\d+)"); - private static final Pattern HASHTAG_TIMESTAMP_PATTERN = - Pattern.compile("(.*)#timestamp=(\\d+)"); + + private InternalUrlsHandler() { + } /** * Handle a YouTube timestamp description URL in NewPipe. @@ -39,7 +30,6 @@ public final class InternalUrlsHandler { * player will be opened when the user will click on the timestamp in the video description, * 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 diff --git a/app/src/main/java/org/schabi/newpipe/util/text/TextLinkifier.java b/app/src/main/java/org/schabi/newpipe/util/text/TextLinkifier.java index 1419ac85a..4221da398 100644 --- a/app/src/main/java/org/schabi/newpipe/util/text/TextLinkifier.java +++ b/app/src/main/java/org/schabi/newpipe/util/text/TextLinkifier.java @@ -192,7 +192,7 @@ public final class TextLinkifier { *

* Instead of using an {@link android.content.Intent#ACTION_VIEW} intent in the description of * a content, this method will parse the {@link CharSequence} and replace all current web links - * with {@link ShareUtils#openUrlInBrowser(Context, String, boolean)}. + * with {@link ShareUtils#openUrlInBrowser(Context, String)}. *

* *

@@ -240,7 +240,7 @@ public final class TextLinkifier { for (final URLSpan span : urls) { final String url = span.getURL(); final LongPressClickableSpan longPressClickableSpan = - new UrlLongPressClickableSpan(context, disposables, url); + new UrlLongPressClickableSpan(context, url); textBlockLinked.setSpan(longPressClickableSpan, textBlockLinked.getSpanStart(span), 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 fd533bb5c..ec3cefc62 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 @@ -7,22 +7,16 @@ import androidx.annotation.NonNull; import org.schabi.newpipe.util.external_communication.ShareUtils; -import io.reactivex.rxjava3.disposables.CompositeDisposable; - final class UrlLongPressClickableSpan extends LongPressClickableSpan { @NonNull private final Context context; @NonNull - private final CompositeDisposable disposables; - @NonNull private final String url; UrlLongPressClickableSpan(@NonNull final Context context, - @NonNull final CompositeDisposable disposables, @NonNull final String url) { this.context = context; - this.disposables = disposables; this.url = url; } From d77771a60c85869b17a6b5c25ca3f0ebb5a58574 Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Fri, 5 Sep 2025 19:02:04 +0200 Subject: [PATCH 14/15] Player/handleIntent: fix enqueue if player not running MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In 063dcd41e57c46721f1691cd57d21fbde75a35ea I falsely claimed that the fallthrough case is always degenerate, but it kinda somehow still worked because if you long-click on e.g. the popup button, it would call enqueue, but if nothing was running yet it would fallthrough to the very last case and start the player with the video. So let’s return to that and add a TODO for further refactoring in the future. --- .../main/java/org/schabi/newpipe/player/Player.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 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 ee4081068..1664dc0e3 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -394,8 +394,12 @@ public final class Player implements PlaybackListener, Listener { return; } playQueue.append(newQueue.getStreams()); + return; } - return; + + // TODO: This falls through to the old logic, there was no playQueue + // yet so we should start the player and add the new video + break; } case EnqueueNext -> { if (playQueue != null) { @@ -405,8 +409,12 @@ public final class Player implements PlaybackListener, Listener { } final PlayQueueItem newItem = newQueue.getStreams().get(0); newQueue.enqueueNext(newItem, false); + return; } - return; + + // TODO: This falls through to the old logic, there was no playQueue + // yet so we should start the player and add the new video + break; } case TimestampChange -> { final TimestampChangeData dat = intent.getParcelableExtra(PLAYER_INTENT_DATA); From eb277fe14b87b35118cea41dc74fb8c2e2ba7b1c Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sat, 6 Sep 2025 15:31:14 +0200 Subject: [PATCH 15/15] Player/handleIntent: call handleIntentPost unconditionally We always need to handleIntentPost otherwise the VideoDetailFragment is not setup correctly. --- app/src/main/java/org/schabi/newpipe/player/Player.java | 6 +----- .../main/java/org/schabi/newpipe/player/PlayerService.java | 2 ++ 2 files changed, 3 insertions(+), 5 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 1664dc0e3..f98e4295d 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -375,7 +375,6 @@ public final class Player implements PlaybackListener, Listener { } } - final PlayerType oldPlayerType = playerType; playerType = newPlayerType; initUIsForCurrentPlayerType(); isAudioOnly = audioPlayerSelected(); @@ -459,8 +458,6 @@ public final class Player implements PlaybackListener, Listener { initPlayback(newPlayQueue, playWhenReady); } - handleIntentPost(oldPlayerType); - }, 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 @@ -560,11 +557,10 @@ public final class Player implements PlaybackListener, Listener { initPlayback(samePlayQueue ? playQueue : newQueue, playWhenReady); } - handleIntentPost(oldPlayerType); } - private void handleIntentPost(final PlayerType oldPlayerType) { + public 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) diff --git a/app/src/main/java/org/schabi/newpipe/player/PlayerService.java b/app/src/main/java/org/schabi/newpipe/player/PlayerService.java index adc050e4b..3b6224b47 100644 --- a/app/src/main/java/org/schabi/newpipe/player/PlayerService.java +++ b/app/src/main/java/org/schabi/newpipe/player/PlayerService.java @@ -169,7 +169,9 @@ public final class PlayerService extends MediaBrowserServiceCompat { } if (player != null) { + final PlayerType oldPlayerType = player.getPlayerType(); player.handleIntent(intent); + player.handleIntentPost(oldPlayerType); player.UIs().get(MediaSessionPlayerUi.class) .ifPresent(ui -> ui.handleMediaButtonIntent(intent)); }