From 6eaff5ca6a0628c526351394670fad73528c1679 Mon Sep 17 00:00:00 2001 From: Stypox Date: Mon, 7 Jun 2021 11:24:13 +0200 Subject: [PATCH] Apply review: move thumbnail loading out of Player --- app/src/main/java/org/schabi/newpipe/App.java | 2 +- .../fragments/detail/VideoDetailFragment.java | 1 + .../org/schabi/newpipe/player/Player.java | 108 +++++------------- .../schabi/newpipe/util/PicassoHelper.java | 53 ++++++++- 4 files changed, 85 insertions(+), 79 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/App.java b/app/src/main/java/org/schabi/newpipe/App.java index c27dc7de2..9f4b6d550 100644 --- a/app/src/main/java/org/schabi/newpipe/App.java +++ b/app/src/main/java/org/schabi/newpipe/App.java @@ -104,7 +104,7 @@ public class App extends MultiDexApplication { PicassoHelper.init(this); PicassoHelper.setShouldLoadImages( prefs.getBoolean(getString(R.string.download_thumbnail_key), true)); - PicassoHelper.setIndicatorsEnabled(BuildConfig.DEBUG + PicassoHelper.setIndicatorsEnabled(MainActivity.DEBUG && prefs.getBoolean(getString(R.string.show_image_indicators_key), false)); configureRxJavaErrorHandler(); 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 e9d1a12e2..7ee70fcc4 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 @@ -690,6 +690,7 @@ public final class VideoDetailFragment .into(binding.detailThumbnailImageView, new Callback() { @Override public void onSuccess() { + // nothing to do, the image was loaded correctly into the thumbnail } @Override 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 403132116..83c567cb6 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -85,7 +85,6 @@ import com.google.android.exoplayer2.video.VideoListener; import com.google.android.material.floatingactionbutton.FloatingActionButton; import com.squareup.picasso.Picasso; import com.squareup.picasso.Target; -import com.squareup.picasso.Transformation; import org.schabi.newpipe.DownloaderImpl; import org.schabi.newpipe.MainActivity; @@ -161,6 +160,7 @@ import static com.google.android.exoplayer2.Player.REPEAT_MODE_ONE; import static com.google.android.exoplayer2.Player.RepeatMode; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.schabi.newpipe.extractor.ServiceList.YouTube; +import static org.schabi.newpipe.extractor.utils.Utils.isNullOrEmpty; import static org.schabi.newpipe.ktx.ViewUtils.animate; import static org.schabi.newpipe.ktx.ViewUtils.animateRotation; import static org.schabi.newpipe.player.MainPlayer.ACTION_CLOSE; @@ -250,9 +250,6 @@ public final class Player implements private static final int RENDERER_UNAVAILABLE = -1; - private static final String PICASSO_PLAYER_TAG = "PICASSO_PLAYER_TAG"; - private static final String PICASSO_TRANSFORMATION_KEY = "PICASSO_TRANSFORMATION_KEY"; - /*////////////////////////////////////////////////////////////////////////// // Playback //////////////////////////////////////////////////////////////////////////*/ @@ -823,7 +820,7 @@ public final class Player implements databaseUpdateDisposable.clear(); progressUpdateDisposable.set(null); - PicassoHelper.cancelTag(PICASSO_PLAYER_TAG); + PicassoHelper.cancelTag(PicassoHelper.PLAYER_THUMBNAIL_TAG); // cancel thumbnail loading if (binding != null) { binding.endScreen.setImageBitmap(null); @@ -1221,85 +1218,42 @@ public final class Player implements Log.d(TAG, "Thumbnail - initThumbnail() called with url = [" + (url == null ? "null" : url) + "]"); } - if (url == null || url.isEmpty()) { + if (isNullOrEmpty(url)) { return; } // scale down the notification thumbnail for performance - PicassoHelper.loadThumbnail(url) - .tag(PICASSO_PLAYER_TAG) - .transform(new Transformation() { - @Override - public Bitmap transform(final Bitmap source) { - final float notificationThumbnailWidth = Math.min( - context.getResources() - .getDimension(R.dimen.player_notification_thumbnail_width), - source.getWidth()); + PicassoHelper.loadScaledDownThumbnail(context, url).into(new Target() { + @Override + public void onBitmapLoaded(final Bitmap bitmap, final Picasso.LoadedFrom from) { + if (DEBUG) { + Log.d(TAG, "Thumbnail - onLoadingComplete() called with: url = [" + url + + "], " + "loadedImage = [" + bitmap + " -> " + bitmap.getWidth() + "x" + + bitmap.getHeight() + "], from = [" + from + "]"); + } - final Bitmap result = Bitmap.createScaledBitmap( - source, - (int) notificationThumbnailWidth, - (int) (source.getHeight() - / (source.getWidth() / notificationThumbnailWidth)), - true); + currentThumbnail = bitmap; + NotificationUtil.getInstance() + .createNotificationIfNeededAndUpdate(Player.this, false); + // there is a new thumbnail, so changed the end screen thumbnail, too. + updateEndScreenThumbnail(); + } - if (result == source) { - // create a new mutable bitmap to prevent strange crashes on some - // devices (see #4638) - final Bitmap copied = Bitmap.createScaledBitmap( - source, - (int) notificationThumbnailWidth - 1, - (int) (source.getHeight() / (source.getWidth() - / (notificationThumbnailWidth - 1))), - true); - source.recycle(); - return copied; - } else { - source.recycle(); - return result; - } - } + @Override + public void onBitmapFailed(final Exception e, final Drawable errorDrawable) { + Log.e(TAG, "Thumbnail - onBitmapFailed() called with: url = [" + url + "]", e); + currentThumbnail = null; + NotificationUtil.getInstance() + .createNotificationIfNeededAndUpdate(Player.this, false); + } - @Override - public String key() { - return PICASSO_TRANSFORMATION_KEY; - } - }) - .into(new Target() { - @Override - public void onBitmapLoaded(final Bitmap bitmap, final Picasso.LoadedFrom from) { - - if (DEBUG) { - Log.d(TAG, "Thumbnail - onLoadingComplete() called with: " - + "url = [" + url + "], " + "loadedImage = [" + bitmap + " -> " - + bitmap.getWidth() + "x" + bitmap.getHeight() - + "], from = [" + from + "]"); - } - - currentThumbnail = bitmap; - NotificationUtil.getInstance() - .createNotificationIfNeededAndUpdate(Player.this, false); - // there is a new thumbnail, so changed the end screen thumbnail, too. - updateEndScreenThumbnail(); - } - - @Override - public void onBitmapFailed(final Exception e, final Drawable errorDrawable) { - Log.e(TAG, "Thumbnail - onBitmapFailed() called with: url = [" - + url + "]", e); - currentThumbnail = null; - NotificationUtil.getInstance() - .createNotificationIfNeededAndUpdate(Player.this, false); - } - - @Override - public void onPrepareLoad(final Drawable placeHolderDrawable) { - if (DEBUG) { - Log.d(TAG, "Thumbnail - onLoadingStarted() called with: url = [" - + url + "]"); - } - } - }); + @Override + public void onPrepareLoad(final Drawable placeHolderDrawable) { + if (DEBUG) { + Log.d(TAG, "Thumbnail - onLoadingStarted() called with: url = [" + url + "]"); + } + } + }); } /** 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 3a9ce090b..173b45776 100644 --- a/app/src/main/java/org/schabi/newpipe/util/PicassoHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/PicassoHelper.java @@ -9,6 +9,7 @@ import com.squareup.picasso.LruCache; import com.squareup.picasso.OkHttp3Downloader; import com.squareup.picasso.Picasso; import com.squareup.picasso.RequestCreator; +import com.squareup.picasso.Transformation; import org.schabi.newpipe.R; @@ -21,6 +22,9 @@ import okhttp3.OkHttpClient; import static org.schabi.newpipe.extractor.utils.Utils.isBlank; public final class PicassoHelper { + public static final String PLAYER_THUMBNAIL_TAG = "PICASSO_PLAYER_THUMBNAIL_TAG"; + private static final String PLAYER_THUMBNAIL_TRANSFORMATION_KEY + = "PICASSO_PLAYER_THUMBNAIL_TRANSFORMATION_KEY"; private PicassoHelper() { } @@ -63,7 +67,10 @@ public final class PicassoHelper { public static void clearCache(final Context context) throws IOException { picassoInstance.shutdown(); picassoCache.clear(); // clear memory cache - picassoDownloaderClient.cache().delete(); // clear disk cache + final okhttp3.Cache diskCache = picassoDownloaderClient.cache(); + if (diskCache != null) { + diskCache.delete(); // clear disk cache + } init(context); } @@ -105,6 +112,50 @@ 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) { + final float notificationThumbnailWidth = Math.min( + context.getResources() + .getDimension(R.dimen.player_notification_thumbnail_width), + source.getWidth()); + + final Bitmap result = Bitmap.createScaledBitmap( + source, + (int) notificationThumbnailWidth, + (int) (source.getHeight() + / (source.getWidth() / notificationThumbnailWidth)), + true); + + if (result == source) { + // create a new mutable bitmap to prevent strange crashes on some + // devices (see #4638) + final Bitmap copied = Bitmap.createScaledBitmap( + source, + (int) notificationThumbnailWidth - 1, + (int) (source.getHeight() / (source.getWidth() + / (notificationThumbnailWidth - 1))), + true); + source.recycle(); + return copied; + } else { + source.recycle(); + return result; + } + } + + @Override + public String key() { + return PLAYER_THUMBNAIL_TRANSFORMATION_KEY; + } + }); + } + + private static RequestCreator loadImageDefault(final String url, final int placeholderResId) { return picassoInstance .load((!shouldLoadImages || isBlank(url)) ? null : url)