From 6879698d7e53d582e325c2bf851d021c7a830ab6 Mon Sep 17 00:00:00 2001 From: tofunmi Date: Fri, 12 Jan 2024 11:18:10 -0800 Subject: [PATCH] Move setting bitmapFactory options from interface to implementation Moves setting bitmapFactory options from BitmapLoader to DatasourceBitmapLoader BitmapLoader is a general interface for bitmap loading that could use loading implementations other that BitmapFactory, with the rise of Glide being a loader of choice. It's best to correct this interface so that it remains generic We can't deprecate easily because the other loadBitmap method in that case has a default implementation that relies on the first one, so the change is still breaking. BitmapLoader is public api in common, but it's @UnstableAPI and hasn't been around for very long (https://github.com/androidx/media/commit/be3867039105a9398262d3b9b032e00d26a2db00 added it), so it seems this is the best way forward. PiperOrigin-RevId: 597897098 --- .../media3/common/util/BitmapLoader.java | 8 +------ .../DataSourceBitmapLoaderTest.java | 8 +++---- .../datasource/DataSourceBitmapLoader.java | 23 ++++++++++++++++--- .../media3/session/CacheBitmapLoader.java | 5 ++-- .../media3/session/SimpleBitmapLoader.java | 17 ++++++-------- .../media3/transformer/ImageAssetLoader.java | 13 ++++++----- 6 files changed, 41 insertions(+), 33 deletions(-) diff --git a/libraries/common/src/main/java/androidx/media3/common/util/BitmapLoader.java b/libraries/common/src/main/java/androidx/media3/common/util/BitmapLoader.java index 118d2cd575..44b7cadb28 100644 --- a/libraries/common/src/main/java/androidx/media3/common/util/BitmapLoader.java +++ b/libraries/common/src/main/java/androidx/media3/common/util/BitmapLoader.java @@ -16,7 +16,6 @@ package androidx.media3.common.util; import android.graphics.Bitmap; -import android.graphics.BitmapFactory; import android.net.Uri; import androidx.annotation.Nullable; import androidx.media3.common.MediaMetadata; @@ -29,12 +28,7 @@ public interface BitmapLoader { ListenableFuture decodeBitmap(byte[] data); /** Loads an image from {@code uri}. */ - default ListenableFuture loadBitmap(Uri uri) { - return loadBitmap(uri, /* options= */ null); - } - - /** Loads an image from {@code uri} with the given {@link BitmapFactory.Options}. */ - ListenableFuture loadBitmap(Uri uri, @Nullable BitmapFactory.Options options); + ListenableFuture loadBitmap(Uri uri); /** * Loads an image from {@link MediaMetadata}. Returns null if {@code metadata} doesn't contain diff --git a/libraries/datasource/src/androidTest/java/androidx/media3/datasource/DataSourceBitmapLoaderTest.java b/libraries/datasource/src/androidTest/java/androidx/media3/datasource/DataSourceBitmapLoaderTest.java index 5c81878f1e..e3c007897b 100644 --- a/libraries/datasource/src/androidTest/java/androidx/media3/datasource/DataSourceBitmapLoaderTest.java +++ b/libraries/datasource/src/androidTest/java/androidx/media3/datasource/DataSourceBitmapLoaderTest.java @@ -205,13 +205,13 @@ public class DataSourceBitmapLoaderTest { File file = tempFolder.newFile(); Files.write(Paths.get(file.getAbsolutePath()), imageData); Uri uri = Uri.fromFile(file); - DataSourceBitmapLoader bitmapLoader = - new DataSourceBitmapLoader(MoreExecutors.newDirectExecutorService(), dataSourceFactory); - BitmapFactory.Options options = new BitmapFactory.Options(); options.inMutable = true; + DataSourceBitmapLoader bitmapLoader = + new DataSourceBitmapLoader( + MoreExecutors.newDirectExecutorService(), dataSourceFactory, options); - Bitmap bitmap = bitmapLoader.loadBitmap(uri, options).get(); + Bitmap bitmap = bitmapLoader.loadBitmap(uri).get(); assertThat(bitmap.isMutable()).isTrue(); } diff --git a/libraries/datasource/src/main/java/androidx/media3/datasource/DataSourceBitmapLoader.java b/libraries/datasource/src/main/java/androidx/media3/datasource/DataSourceBitmapLoader.java index 6c0b547874..7d4570e6bd 100644 --- a/libraries/datasource/src/main/java/androidx/media3/datasource/DataSourceBitmapLoader.java +++ b/libraries/datasource/src/main/java/androidx/media3/datasource/DataSourceBitmapLoader.java @@ -39,7 +39,7 @@ import java.util.concurrent.Executors; /** * A {@link BitmapLoader} implementation that uses a {@link DataSource} to support fetching images - * from URIs. + * from URIs and {@link BitmapFactory} to load them into {@link Bitmap}. * *

Loading tasks are delegated to a {@link ListeningExecutorService} defined during construction. * If no executor service is passed, all tasks are delegated to a single-thread executor service @@ -54,6 +54,7 @@ public final class DataSourceBitmapLoader implements BitmapLoader { private final ListeningExecutorService listeningExecutorService; private final DataSource.Factory dataSourceFactory; + @Nullable private final BitmapFactory.Options options; /** * Creates an instance that uses a {@link DefaultHttpDataSource} for image loading and delegates @@ -72,17 +73,33 @@ public final class DataSourceBitmapLoader implements BitmapLoader { */ public DataSourceBitmapLoader( ListeningExecutorService listeningExecutorService, DataSource.Factory dataSourceFactory) { + this(listeningExecutorService, dataSourceFactory, /* options= */ null); + } + + /** + * Creates an instance that delegates loading tasks to the {@link ListeningExecutorService}. + * + * @param listeningExecutorService The {@link ListeningExecutorService}. + * @param dataSourceFactory The {@link DataSource.Factory} that creates the {@link DataSource} + * used to load the image. + * @param options The {@link BitmapFactory.Options} the image should be loaded with. + */ + public DataSourceBitmapLoader( + ListeningExecutorService listeningExecutorService, + DataSource.Factory dataSourceFactory, + @Nullable BitmapFactory.Options options) { this.listeningExecutorService = listeningExecutorService; this.dataSourceFactory = dataSourceFactory; + this.options = options; } @Override public ListenableFuture decodeBitmap(byte[] data) { - return listeningExecutorService.submit(() -> decode(data, /* options= */ null)); + return listeningExecutorService.submit(() -> decode(data, options)); } @Override - public ListenableFuture loadBitmap(Uri uri, @Nullable BitmapFactory.Options options) { + public ListenableFuture loadBitmap(Uri uri) { return listeningExecutorService.submit( () -> load(dataSourceFactory.createDataSource(), uri, options)); } diff --git a/libraries/session/src/main/java/androidx/media3/session/CacheBitmapLoader.java b/libraries/session/src/main/java/androidx/media3/session/CacheBitmapLoader.java index 5426d670a2..4fc3f5717f 100644 --- a/libraries/session/src/main/java/androidx/media3/session/CacheBitmapLoader.java +++ b/libraries/session/src/main/java/androidx/media3/session/CacheBitmapLoader.java @@ -18,7 +18,6 @@ package androidx.media3.session; import static androidx.media3.common.util.Assertions.checkStateNotNull; import android.graphics.Bitmap; -import android.graphics.BitmapFactory; import android.net.Uri; import androidx.annotation.Nullable; import androidx.media3.common.util.BitmapLoader; @@ -60,11 +59,11 @@ public final class CacheBitmapLoader implements BitmapLoader { } @Override - public ListenableFuture loadBitmap(Uri uri, @Nullable BitmapFactory.Options options) { + public ListenableFuture loadBitmap(Uri uri) { if (lastBitmapLoadRequest != null && lastBitmapLoadRequest.matches(uri)) { return lastBitmapLoadRequest.getFuture(); } - ListenableFuture future = bitmapLoader.loadBitmap(uri, options); + ListenableFuture future = bitmapLoader.loadBitmap(uri); lastBitmapLoadRequest = new BitmapLoadRequest(uri, future); return future; } diff --git a/libraries/session/src/main/java/androidx/media3/session/SimpleBitmapLoader.java b/libraries/session/src/main/java/androidx/media3/session/SimpleBitmapLoader.java index a31eceb9d2..6649aa95e3 100644 --- a/libraries/session/src/main/java/androidx/media3/session/SimpleBitmapLoader.java +++ b/libraries/session/src/main/java/androidx/media3/session/SimpleBitmapLoader.java @@ -68,24 +68,21 @@ public final class SimpleBitmapLoader implements BitmapLoader { @Override public ListenableFuture decodeBitmap(byte[] data) { - return executorService.submit(() -> decode(data, /* options= */ null)); + return executorService.submit(() -> decode(data)); } @Override - public ListenableFuture loadBitmap(Uri uri, @Nullable BitmapFactory.Options options) { - return executorService.submit(() -> load(uri, options)); + public ListenableFuture loadBitmap(Uri uri) { + return executorService.submit(() -> load(uri)); } - // BitmapFactory's options parameter is null-ok. - @SuppressWarnings("nullness:argument.type.incompatible") - private static Bitmap decode(byte[] data, @Nullable BitmapFactory.Options options) { - @Nullable - Bitmap bitmap = BitmapFactory.decodeByteArray(data, /* offset= */ 0, data.length, options); + private static Bitmap decode(byte[] data) { + @Nullable Bitmap bitmap = BitmapFactory.decodeByteArray(data, /* offset= */ 0, data.length); checkArgument(bitmap != null, "Could not decode image data"); return bitmap; } - private static Bitmap load(Uri uri, @Nullable BitmapFactory.Options options) throws IOException { + private static Bitmap load(Uri uri) throws IOException { if ("file".equals(uri.getScheme())) { @Nullable String path = uri.getPath(); if (path == null) { @@ -108,7 +105,7 @@ public final class SimpleBitmapLoader implements BitmapLoader { throw new IOException("Invalid response status code: " + responseCode); } try (InputStream inputStream = httpConnection.getInputStream()) { - return decode(ByteStreams.toByteArray(inputStream), options); + return decode(ByteStreams.toByteArray(inputStream)); } } } diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/ImageAssetLoader.java b/libraries/transformer/src/main/java/androidx/media3/transformer/ImageAssetLoader.java index 8214a6a9d3..b621123ad5 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/ImageAssetLoader.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/ImageAssetLoader.java @@ -112,17 +112,18 @@ public final class ImageAssetLoader implements AssetLoader { progressState = PROGRESS_STATE_AVAILABLE; listener.onDurationUs(editedMediaItem.durationUs); listener.onTrackCount(1); - BitmapLoader bitmapLoader = - new DataSourceBitmapLoader( - MoreExecutors.listeningDecorator(scheduledExecutorService), dataSourceFactory); - MediaItem.LocalConfiguration localConfiguration = - checkNotNull(editedMediaItem.mediaItem.localConfiguration); @Nullable BitmapFactory.Options options = null; if (Util.SDK_INT >= 26) { options = new BitmapFactory.Options(); options.inPreferredColorSpace = ColorSpace.get(ColorSpace.Named.SRGB); } - ListenableFuture future = bitmapLoader.loadBitmap(localConfiguration.uri, options); + BitmapLoader bitmapLoader = + new DataSourceBitmapLoader( + MoreExecutors.listeningDecorator(scheduledExecutorService), dataSourceFactory, options); + MediaItem.LocalConfiguration localConfiguration = + checkNotNull(editedMediaItem.mediaItem.localConfiguration); + + ListenableFuture future = bitmapLoader.loadBitmap(localConfiguration.uri); Futures.addCallback( future, new FutureCallback() {