From 7fbef35daab5b7122237cca636aa4ffc2a3368fe Mon Sep 17 00:00:00 2001 From: Stypox Date: Sun, 28 Aug 2022 17:24:51 +0200 Subject: [PATCH 1/4] Unify onThumbnailLoaded calls to ensure UIs always updated --- .../java/org/schabi/newpipe/player/Player.java | 18 +++++++++++++----- 1 file changed, 13 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 319c163e8..040070b5b 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -765,17 +765,15 @@ public final class Player implements PlaybackListener, Listener { + " -> " + bitmap.getWidth() + "x" + bitmap.getHeight() + "], from = [" + from + "]"); } - currentThumbnail = bitmap; // there is a new thumbnail, so e.g. the end screen thumbnail needs to change, too. - UIs.call(playerUi -> playerUi.onThumbnailLoaded(bitmap)); + onThumbnailLoaded(bitmap); } @Override public void onBitmapFailed(final Exception e, final Drawable errorDrawable) { Log.e(TAG, "Thumbnail - onBitmapFailed() called", e); - currentThumbnail = null; // there is a new thumbnail, so e.g. the end screen thumbnail needs to change, too. - UIs.call(playerUi -> playerUi.onThumbnailLoaded(null)); + onThumbnailLoaded(null); } @Override @@ -798,7 +796,7 @@ public final class Player implements PlaybackListener, Listener { // Unset currentThumbnail, since it is now outdated. This ensures it is not used in media // session metadata while the new thumbnail is being loaded by Picasso. - currentThumbnail = null; + onThumbnailLoaded(null); if (isNullOrEmpty(url)) { return; } @@ -813,6 +811,16 @@ public final class Player implements PlaybackListener, Listener { // cancel the Picasso job associated with the player thumbnail, if any PicassoHelper.cancelTag(PICASSO_PLAYER_THUMBNAIL_TAG); } + + private void onThumbnailLoaded(@Nullable final Bitmap bitmap) { + // Avoid useless thumbnail updates, if the thumbnail has not actually changed. Based on the + // thumbnail loading code, this if would be skipped only when both bitmaps are `null`, since + // onThumbnailLoaded won't be called twice with the same nonnull bitmap by Picasso's target. + if (currentThumbnail != bitmap) { + currentThumbnail = bitmap; + UIs.call(playerUi -> playerUi.onThumbnailLoaded(bitmap)); + } + } //endregion From 4a7af6f9ac50c0a5a3ed3b306d08d4b8aa622aea Mon Sep 17 00:00:00 2001 From: Stypox Date: Sun, 28 Aug 2022 18:32:27 +0200 Subject: [PATCH 2/4] Remove thumbnail before sync, if outdated Also refactor onPlaybackSynchronize and add comments --- .../org/schabi/newpipe/player/Player.java | 62 ++++++++++--------- 1 file changed, 32 insertions(+), 30 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 040070b5b..99d36f66e 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -1509,48 +1509,50 @@ public final class Player implements PlaybackListener, Listener { Log.d(TAG, "Playback - onPlaybackSynchronize(was blocked: " + wasBlocked + ") called with item=[" + item.getTitle() + "], url=[" + item.getUrl() + "]"); } - if (exoPlayerIsNull() || playQueue == null) { - return; + if (exoPlayerIsNull() || playQueue == null || currentItem == item) { + return; // nothing to synchronize } - final boolean hasPlayQueueItemChanged = currentItem != item; + final int playQueueIndex = playQueue.indexOf(item); + final int playlistIndex = simpleExoPlayer.getCurrentMediaItemIndex(); + final int playlistSize = simpleExoPlayer.getCurrentTimeline().getWindowCount(); + final boolean removeThumbnailBeforeSync = currentItem == null + || currentItem.getServiceId() != item.getServiceId() + || !currentItem.getUrl().equals(item.getUrl()); - final int currentPlayQueueIndex = playQueue.indexOf(item); - final int currentPlaylistIndex = simpleExoPlayer.getCurrentMediaItemIndex(); - final int currentPlaylistSize = simpleExoPlayer.getCurrentTimeline().getWindowCount(); - - // If nothing to synchronize - if (!hasPlayQueueItemChanged) { - return; - } currentItem = item; - // Check if on wrong window - if (currentPlayQueueIndex != playQueue.getIndex()) { - Log.e(TAG, "Playback - Play Queue may be desynchronized: item " - + "index=[" + currentPlayQueueIndex + "], " - + "queue index=[" + playQueue.getIndex() + "]"); + if (playQueueIndex != playQueue.getIndex()) { + // wrong window (this should be impossible, as this method is called with + // `item=playQueue.getItem()`, so the index of that item must be equal to `getIndex()`) + Log.e(TAG, "Playback - Play Queue may be not in sync: item index=[" + + playQueueIndex + "], " + "queue index=[" + playQueue.getIndex() + "]"); - // Check if bad seek position - } else if ((currentPlaylistSize > 0 && currentPlayQueueIndex >= currentPlaylistSize) - || currentPlayQueueIndex < 0) { - Log.e(TAG, "Playback - Trying to seek to invalid " - + "index=[" + currentPlayQueueIndex + "] with " - + "playlist length=[" + currentPlaylistSize + "]"); + } else if ((playlistSize > 0 && playQueueIndex >= playlistSize) || playQueueIndex < 0) { + // the queue and the player's timeline are not in sync, since the play queue index + // points outside of the timeline + Log.e(TAG, "Playback - Trying to seek to invalid index=[" + playQueueIndex + + "] with playlist length=[" + playlistSize + "]"); - } else if (wasBlocked || currentPlaylistIndex != currentPlayQueueIndex || !isPlaying()) { + } else if (wasBlocked || playlistIndex != playQueueIndex || !isPlaying()) { + // either the player needs to be unblocked, or the play queue index has just been + // changed and needs to be synchronized, or the player is not playing if (DEBUG) { - Log.d(TAG, "Playback - Rewinding to correct " - + "index=[" + currentPlayQueueIndex + "], " - + "from=[" + currentPlaylistIndex + "], " - + "size=[" + currentPlaylistSize + "]."); + Log.d(TAG, "Playback - Rewinding to correct index=[" + playQueueIndex + "], " + + "from=[" + playlistIndex + "], size=[" + playlistSize + "]."); } + if (removeThumbnailBeforeSync) { + // unset the current (now outdated) thumbnail to ensure it is not used during sync + onThumbnailLoaded(null); + } + + // sync the player index with the queue index, and seek to the correct position if (item.getRecoveryPosition() != PlayQueueItem.RECOVERY_UNSET) { - simpleExoPlayer.seekTo(currentPlayQueueIndex, item.getRecoveryPosition()); - playQueue.unsetRecovery(currentPlayQueueIndex); + simpleExoPlayer.seekTo(playQueueIndex, item.getRecoveryPosition()); + playQueue.unsetRecovery(playQueueIndex); } else { - simpleExoPlayer.seekToDefaultPosition(currentPlayQueueIndex); + simpleExoPlayer.seekToDefaultPosition(playQueueIndex); } } } From f9109ebc81e5c7d9a01b63c4b68d2dc46b030f1c Mon Sep 17 00:00:00 2001 From: Stypox Date: Sun, 28 Aug 2022 18:35:21 +0200 Subject: [PATCH 3/4] Use player.getThumbnail() instead of field in VideoPlayerUi --- .../newpipe/player/ui/VideoPlayerUi.java | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/ui/VideoPlayerUi.java b/app/src/main/java/org/schabi/newpipe/player/ui/VideoPlayerUi.java index a972d2f71..1709755f2 100644 --- a/app/src/main/java/org/schabi/newpipe/player/ui/VideoPlayerUi.java +++ b/app/src/main/java/org/schabi/newpipe/player/ui/VideoPlayerUi.java @@ -109,7 +109,6 @@ public abstract class VideoPlayerUi extends PlayerUi private final Handler controlsVisibilityHandler = new Handler(Looper.getMainLooper()); @Nullable private SurfaceHolderCallback surfaceHolderCallback; boolean surfaceIsSetup = false; - @Nullable private Bitmap thumbnail = null; /*////////////////////////////////////////////////////////////////////////// @@ -385,9 +384,7 @@ public abstract class VideoPlayerUi extends PlayerUi @Override public void destroy() { super.destroy(); - if (binding != null) { - binding.endScreen.setImageBitmap(null); - } + binding.endScreen.setImageDrawable(null); deinitPlayerSeekOverlay(); deinitListeners(); } @@ -422,12 +419,10 @@ public abstract class VideoPlayerUi extends PlayerUi public void onBroadcastReceived(final Intent intent) { super.onBroadcastReceived(intent); if (Intent.ACTION_CONFIGURATION_CHANGED.equals(intent.getAction())) { - // When the orientation changed, the screen height might be smaller. - // If the end screen thumbnail is not re-scaled, - // it can be larger than the current screen height - // and thus enlarging the whole player. - // This causes the seekbar to be ouf the visible area. - updateEndScreenThumbnail(); + // When the orientation changes, the screen height might be smaller. If the end screen + // thumbnail is not re-scaled, it can be larger than the current screen height and thus + // enlarging the whole player. This causes the seekbar to be out of the visible area. + updateEndScreenThumbnail(player.getThumbnail()); } } //endregion @@ -449,11 +444,10 @@ public abstract class VideoPlayerUi extends PlayerUi @Override public void onThumbnailLoaded(@Nullable final Bitmap bitmap) { super.onThumbnailLoaded(bitmap); - thumbnail = bitmap; - updateEndScreenThumbnail(); + updateEndScreenThumbnail(bitmap); } - private void updateEndScreenThumbnail() { + private void updateEndScreenThumbnail(@Nullable final Bitmap thumbnail) { if (thumbnail == null) { // remove end screen thumbnail binding.endScreen.setImageDrawable(null); From ed87465565eaa431afc95fabc82e6383e77125d9 Mon Sep 17 00:00:00 2001 From: Stypox Date: Sun, 28 Aug 2022 18:35:58 +0200 Subject: [PATCH 4/4] Only update notification large icon when it changes --- .../notification/NotificationPlayerUi.java | 2 +- .../player/notification/NotificationUtil.java | 21 ++++++++++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/notification/NotificationPlayerUi.java b/app/src/main/java/org/schabi/newpipe/player/notification/NotificationPlayerUi.java index ed678a18c..4b1fc417f 100644 --- a/app/src/main/java/org/schabi/newpipe/player/notification/NotificationPlayerUi.java +++ b/app/src/main/java/org/schabi/newpipe/player/notification/NotificationPlayerUi.java @@ -43,7 +43,7 @@ public final class NotificationPlayerUi extends PlayerUi { @Override public void onThumbnailLoaded(@Nullable final Bitmap bitmap) { super.onThumbnailLoaded(bitmap); - notificationUtil.createNotificationIfNeededAndUpdate(false); + notificationUtil.updateThumbnail(); } @Override 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 2c3199a28..3488ec61e 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 @@ -24,6 +24,8 @@ import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi; import org.schabi.newpipe.util.NavigationHelper; import java.util.List; +import java.util.Objects; +import java.util.Optional; import static android.app.PendingIntent.FLAG_UPDATE_CURRENT; import static androidx.media.app.NotificationCompat.MediaStyle; @@ -40,8 +42,6 @@ import static org.schabi.newpipe.player.notification.NotificationConstants.ACTIO /** * This is a utility class for player notifications. - * - * @author cool-student */ public final class NotificationUtil { private static final String TAG = NotificationUtil.class.getSimpleName(); @@ -79,6 +79,19 @@ public final class NotificationUtil { notificationManager.notify(NOTIFICATION_ID, notificationBuilder.build()); } + public synchronized void updateThumbnail() { + if (notificationBuilder != null) { + if (DEBUG) { + Log.d(TAG, "updateThumbnail() called with thumbnail = [" + Integer.toHexString( + Optional.ofNullable(player.getThumbnail()).map(Objects::hashCode).orElse(0)) + + "], title = [" + player.getVideoTitle() + "]"); + } + + setLargeIcon(notificationBuilder); + notificationManager.notify(NOTIFICATION_ID, notificationBuilder.build()); + } + } + private synchronized NotificationCompat.Builder createNotification() { if (DEBUG) { Log.d(TAG, "createNotification()"); @@ -123,6 +136,9 @@ public final class NotificationUtil { .setDeleteIntent(PendingIntent.getBroadcast(player.getContext(), NOTIFICATION_ID, new Intent(ACTION_CLOSE), FLAG_UPDATE_CURRENT)); + // set the initial value for the video thumbnail, updatable with updateNotificationThumbnail + setLargeIcon(builder); + return builder; } @@ -142,7 +158,6 @@ public final class NotificationUtil { notificationBuilder.setTicker(player.getVideoTitle()); updateActions(notificationBuilder); - setLargeIcon(notificationBuilder); }