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

Author: Stypox

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 Player(@NonNull final PlayerService service) {
         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 void destroy() {
 
         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,48 +755,63 @@ private void unregisterBroadcastReceiver() {
     //////////////////////////////////////////////////////////////////////////*/
     //region Thumbnail loading
 
-    private void loadCurrentThumbnail(final String url) {
-        if (DEBUG) {
-            Log.d(TAG, "Thumbnail - loadCurrentThumbnail() called with url = ["
-                    + (url == null ? "null" : url) + "]");
-        }
-
-        // 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;
-        if (isNullOrEmpty(url)) {
-            return;
-        }
-
-        // scale down the notification thumbnail for performance
-        PicassoHelper.loadScaledDownThumbnail(context, url).into(new Target() {
+    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: url = [" + url
-                            + "], " + "bitmap = [" + bitmap + " -> " + bitmap.getWidth() + "x"
-                            + bitmap.getHeight() + "], from = [" + from + "]");
+                    Log.d(TAG, "Thumbnail - onBitmapLoaded() called with: bitmap = [" + bitmap
+                            + " -> " + bitmap.getWidth() + "x" + bitmap.getHeight() + "], from = ["
+                            + from + "]");
                 }
-
                 currentThumbnail = bitmap;
-                // there is a new thumbnail, so changed the end screen thumbnail, too.
+                // 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: url = [" + url + "]", e);
+                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: url = [" + url + "]");
+                    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;
+        if (isNullOrEmpty(url)) {
+            return;
+        }
+
+        // scale down the notification thumbnail for performance
+        PicassoHelper.loadScaledDownThumbnail(context, url)
+                .tag(PICASSO_PLAYER_THUMBNAIL_TAG)
+                .into(currentThumbnailTarget);
+    }
+
+    private void cancelLoadingCurrentThumbnail() {
+        // cancel the Picasso job associated with the player thumbnail, if any
+        PicassoHelper.cancelTag(PICASSO_PLAYER_THUMBNAIL_TAG);
     }
     //endregion
 

app/src/main/java/org/schabi/newpipe/util/PicassoHelper.java

@@ -30,7 +30,6 @@
 
 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";
 
@@ -127,7 +126,6 @@ public static RequestCreator loadSeekbarThumbnailPreview(final String url) {
     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) {