From 510efaae976c1fe15a85c3f81c4a49573760097f Mon Sep 17 00:00:00 2001 From: Stypox Date: Fri, 22 Jul 2022 14:39:25 +0200 Subject: [PATCH] Keep strong reference to Picasso thumbnail loading target Before the Target would sometimes be garbage collected before being called with the loaded thumbnail, since Picasso holds weak references to targets --- .../org/schabi/newpipe/player/Player.java | 79 ++++++++++++------- .../schabi/newpipe/util/PicassoHelper.java | 2 - 2 files changed, 51 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 22d46bcbe..45b9b0fde 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -174,6 +174,7 @@ public final class Player implements PlaybackListener, Listener { //////////////////////////////////////////////////////////////////////////*/ public static final int RENDERER_UNAVAILABLE = -1; + private static final String PICASSO_PLAYER_THUMBNAIL_TAG = "PICASSO_PLAYER_THUMBNAIL_TAG"; /*////////////////////////////////////////////////////////////////////////// // Playback @@ -232,6 +233,11 @@ public final class Player implements PlaybackListener, Listener { @NonNull private final SerialDisposable progressUpdateDisposable = new SerialDisposable(); @NonNull private final CompositeDisposable databaseUpdateDisposable = 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, + // which would otherwise be garbage collected since Picasso holds weak references to targets. + @NonNull private final Target currentThumbnailTarget; + /*////////////////////////////////////////////////////////////////////////// // Utils //////////////////////////////////////////////////////////////////////////*/ @@ -263,6 +269,8 @@ public final class Player implements PlaybackListener, Listener { videoResolver = new VideoPlaybackResolver(context, dataSource, getQualityResolver()); audioResolver = new AudioPlaybackResolver(context, dataSource); + currentThumbnailTarget = getCurrentThumbnailTarget(); + // The UIs added here should always be present. They will be initialized when the player // reaches the initialization step. Make sure the media session ui is before the // notification ui in the UIs list, since the notification depends on the media session in @@ -573,7 +581,7 @@ public final class Player implements PlaybackListener, Listener { databaseUpdateDisposable.clear(); progressUpdateDisposable.set(null); - PicassoHelper.cancelTag(PicassoHelper.PLAYER_THUMBNAIL_TAG); // cancel thumbnail loading + cancelLoadingCurrentThumbnail(); UIs.destroyAll(Object.class); // destroy every UI: obviously every UI extends Object } @@ -747,12 +755,47 @@ public final class Player implements PlaybackListener, Listener { //////////////////////////////////////////////////////////////////////////*/ //region Thumbnail loading + private Target getCurrentThumbnailTarget() { + // a Picasso target is just a listener for thumbnail loading events + return new Target() { + @Override + public void onBitmapLoaded(final Bitmap bitmap, final Picasso.LoadedFrom from) { + if (DEBUG) { + Log.d(TAG, "Thumbnail - onBitmapLoaded() called with: bitmap = [" + bitmap + + " -> " + 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)); + } + + @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)); + } + + @Override + public void onPrepareLoad(final Drawable placeHolderDrawable) { + if (DEBUG) { + Log.d(TAG, "Thumbnail - onPrepareLoad() called"); + } + } + }; + } + private void loadCurrentThumbnail(final String url) { if (DEBUG) { Log.d(TAG, "Thumbnail - loadCurrentThumbnail() called with url = [" + (url == null ? "null" : url) + "]"); } + // first cancel any previous loading + cancelLoadingCurrentThumbnail(); + // 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; @@ -761,34 +804,14 @@ public final class Player implements PlaybackListener, Listener { } // scale down the notification thumbnail for performance - PicassoHelper.loadScaledDownThumbnail(context, url).into(new Target() { - @Override - public void onBitmapLoaded(final Bitmap bitmap, final Picasso.LoadedFrom from) { - if (DEBUG) { - Log.d(TAG, "Thumbnail - onBitmapLoaded() called with: url = [" + url - + "], " + "bitmap = [" + bitmap + " -> " + bitmap.getWidth() + "x" - + bitmap.getHeight() + "], from = [" + from + "]"); - } + PicassoHelper.loadScaledDownThumbnail(context, url) + .tag(PICASSO_PLAYER_THUMBNAIL_TAG) + .into(currentThumbnailTarget); + } - currentThumbnail = bitmap; - // there is a new thumbnail, so changed the end screen thumbnail, too. - UIs.call(playerUi -> playerUi.onThumbnailLoaded(bitmap)); - } - - @Override - public void onBitmapFailed(final Exception e, final Drawable errorDrawable) { - Log.e(TAG, "Thumbnail - onBitmapFailed() called: url = [" + url + "]", e); - currentThumbnail = null; - UIs.call(playerUi -> playerUi.onThumbnailLoaded(null)); - } - - @Override - public void onPrepareLoad(final Drawable placeHolderDrawable) { - if (DEBUG) { - Log.d(TAG, "Thumbnail - onPrepareLoad() called: url = [" + url + "]"); - } - } - }); + private void cancelLoadingCurrentThumbnail() { + // cancel the Picasso job associated with the player thumbnail, if any + PicassoHelper.cancelTag(PICASSO_PLAYER_THUMBNAIL_TAG); } //endregion diff --git a/app/src/main/java/org/schabi/newpipe/util/PicassoHelper.java b/app/src/main/java/org/schabi/newpipe/util/PicassoHelper.java index 5739b930b..2e781631e 100644 --- a/app/src/main/java/org/schabi/newpipe/util/PicassoHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/PicassoHelper.java @@ -27,7 +27,6 @@ import okhttp3.OkHttpClient; public final class PicassoHelper { private static final String TAG = PicassoHelper.class.getSimpleName(); - public static final String PLAYER_THUMBNAIL_TAG = "PICASSO_PLAYER_THUMBNAIL_TAG"; private static final String PLAYER_THUMBNAIL_TRANSFORMATION_KEY = "PICASSO_PLAYER_THUMBNAIL_TRANSFORMATION_KEY"; @@ -128,7 +127,6 @@ public final class PicassoHelper { public static RequestCreator loadScaledDownThumbnail(final Context context, final String url) { // scale down the notification thumbnail for performance return PicassoHelper.loadThumbnail(url) - .tag(PLAYER_THUMBNAIL_TAG) .transform(new Transformation() { @Override public Bitmap transform(final Bitmap source) {