From d848d3358a67ce2439db7cf170eec7b8c3ecffbf Mon Sep 17 00:00:00 2001 From: tianyifeng Date: Mon, 19 Dec 2022 17:43:50 +0000 Subject: [PATCH] Add BitmapLoader injection in MediaController Also clean up the strict mode violations of using `BitmapFactory.convertToByteArray` on the main thread. PiperOrigin-RevId: 496422355 --- .../androidx/media3/session/MediaBrowser.java | 42 +++++++++-- .../session/MediaBrowserImplLegacy.java | 5 +- .../media3/session/MediaController.java | 34 +++++++-- .../session/MediaControllerImplLegacy.java | 67 ++++++++++++++--- .../androidx/media3/session/MediaUtils.java | 18 ----- ...CompatCallbackWithMediaControllerTest.java | 74 ++++++++++++++++++- 6 files changed, 200 insertions(+), 40 deletions(-) diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaBrowser.java b/libraries/session/src/main/java/androidx/media3/session/MediaBrowser.java index 2ee3be9c96..ffe84bb11e 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaBrowser.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaBrowser.java @@ -32,6 +32,7 @@ import androidx.annotation.Nullable; import androidx.media3.common.MediaItem; import androidx.media3.common.Player; import androidx.media3.common.util.Consumer; +import androidx.media3.common.util.UnstableApi; import androidx.media3.common.util.Util; import androidx.media3.session.MediaLibraryService.LibraryParams; import com.google.common.collect.ImmutableList; @@ -57,6 +58,7 @@ public final class MediaBrowser extends MediaController { private Bundle connectionHints; private Listener listener; private Looper applicationLooper; + private @MonotonicNonNull BitmapLoader bitmapLoader; /** * Creates a builder for {@link MediaBrowser}. @@ -121,6 +123,21 @@ public final class MediaBrowser extends MediaController { return this; } + /** + * Sets a {@link BitmapLoader} for the {@link MediaBrowser} to decode bitmaps from compressed + * binary data. If not set, a {@link CacheBitmapLoader} that wraps a {@link SimpleBitmapLoader} + * will be used. + * + * @param bitmapLoader The bitmap loader. + * @return The builder to allow chaining. + */ + @UnstableApi + @CanIgnoreReturnValue + public Builder setBitmapLoader(BitmapLoader bitmapLoader) { + this.bitmapLoader = checkNotNull(bitmapLoader); + return this; + } + /** * Builds a {@link MediaBrowser} asynchronously. * @@ -149,8 +166,12 @@ public final class MediaBrowser extends MediaController { */ public ListenableFuture buildAsync() { MediaControllerHolder holder = new MediaControllerHolder<>(applicationLooper); + if (token.isLegacySession() && bitmapLoader == null) { + bitmapLoader = new CacheBitmapLoader(new SimpleBitmapLoader()); + } MediaBrowser browser = - new MediaBrowser(context, token, connectionHints, listener, applicationLooper, holder); + new MediaBrowser( + context, token, connectionHints, listener, applicationLooper, holder, bitmapLoader); postOrRun(new Handler(applicationLooper), () -> holder.setController(browser)); return holder; } @@ -215,8 +236,16 @@ public final class MediaBrowser extends MediaController { Bundle connectionHints, Listener listener, Looper applicationLooper, - ConnectionCallback connectionCallback) { - super(context, token, connectionHints, listener, applicationLooper, connectionCallback); + ConnectionCallback connectionCallback, + @Nullable BitmapLoader bitmapLoader) { + super( + context, + token, + connectionHints, + listener, + applicationLooper, + connectionCallback, + bitmapLoader); } @Override @@ -226,10 +255,13 @@ public final class MediaBrowser extends MediaController { Context context, SessionToken token, Bundle connectionHints, - Looper applicationLooper) { + Looper applicationLooper, + @Nullable BitmapLoader bitmapLoader) { MediaBrowserImpl impl; if (token.isLegacySession()) { - impl = new MediaBrowserImplLegacy(context, this, token, applicationLooper); + impl = + new MediaBrowserImplLegacy( + context, this, token, applicationLooper, checkNotNull(bitmapLoader)); } else { impl = new MediaBrowserImplBase(context, this, token, connectionHints, applicationLooper); } diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaBrowserImplLegacy.java b/libraries/session/src/main/java/androidx/media3/session/MediaBrowserImplLegacy.java index fc924af3d9..8b3fb24eef 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaBrowserImplLegacy.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaBrowserImplLegacy.java @@ -57,8 +57,9 @@ import org.checkerframework.checker.initialization.qual.UnderInitialization; Context context, @UnderInitialization MediaBrowser instance, SessionToken token, - Looper applicationLooper) { - super(context, instance, token, applicationLooper); + Looper applicationLooper, + BitmapLoader bitmapLoader) { + super(context, instance, token, applicationLooper, bitmapLoader); this.instance = instance; } diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaController.java b/libraries/session/src/main/java/androidx/media3/session/MediaController.java index 496e6ea946..e9855421d6 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaController.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaController.java @@ -67,6 +67,7 @@ import java.util.concurrent.Executor; import java.util.concurrent.Future; import org.checkerframework.checker.initialization.qual.NotOnlyInitialized; import org.checkerframework.checker.initialization.qual.UnderInitialization; +import org.checkerframework.checker.nullness.qual.MonotonicNonNull; /** * A controller that interacts with a {@link MediaSession}, a {@link MediaSessionService} hosting a @@ -183,6 +184,7 @@ public class MediaController implements Player { private Bundle connectionHints; private Listener listener; private Looper applicationLooper; + private @MonotonicNonNull BitmapLoader bitmapLoader; /** * Creates a builder for {@link MediaController}. @@ -261,6 +263,21 @@ public class MediaController implements Player { return this; } + /** + * Sets a {@link BitmapLoader} for the {@link MediaController} to decode bitmaps from compressed + * binary data. If not set, a {@link CacheBitmapLoader} that wraps a {@link SimpleBitmapLoader} + * will be used. + * + * @param bitmapLoader The bitmap loader. + * @return The builder to allow chaining. + */ + @UnstableApi + @CanIgnoreReturnValue + public Builder setBitmapLoader(BitmapLoader bitmapLoader) { + this.bitmapLoader = checkNotNull(bitmapLoader); + return this; + } + /** * Builds a {@link MediaController} asynchronously. * @@ -290,8 +307,12 @@ public class MediaController implements Player { public ListenableFuture buildAsync() { MediaControllerHolder holder = new MediaControllerHolder<>(applicationLooper); + if (token.isLegacySession() && bitmapLoader == null) { + bitmapLoader = new CacheBitmapLoader(new SimpleBitmapLoader()); + } MediaController controller = - new MediaController(context, token, connectionHints, listener, applicationLooper, holder); + new MediaController( + context, token, connectionHints, listener, applicationLooper, holder, bitmapLoader); postOrRun(new Handler(applicationLooper), () -> holder.setController(controller)); return holder; } @@ -404,7 +425,8 @@ public class MediaController implements Player { Bundle connectionHints, Listener listener, Looper applicationLooper, - ConnectionCallback connectionCallback) { + ConnectionCallback connectionCallback, + @Nullable BitmapLoader bitmapLoader) { checkNotNull(context, "context must not be null"); checkNotNull(token, "token must not be null"); @@ -417,7 +439,7 @@ public class MediaController implements Player { applicationHandler = new Handler(applicationLooper); this.connectionCallback = connectionCallback; - impl = createImpl(context, token, connectionHints, applicationLooper); + impl = createImpl(context, token, connectionHints, applicationLooper, bitmapLoader); impl.connect(); } @@ -427,9 +449,11 @@ public class MediaController implements Player { Context context, SessionToken token, Bundle connectionHints, - Looper applicationLooper) { + Looper applicationLooper, + @Nullable BitmapLoader bitmapLoader) { if (token.isLegacySession()) { - return new MediaControllerImplLegacy(context, this, token, applicationLooper); + return new MediaControllerImplLegacy( + context, this, token, applicationLooper, checkNotNull(bitmapLoader)); } else { return new MediaControllerImplBase(context, this, token, connectionHints, applicationLooper); } diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java index ff8e3c7c9a..489997beb8 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java @@ -25,6 +25,7 @@ import static java.lang.Math.min; import android.app.PendingIntent; import android.content.Context; +import android.graphics.Bitmap; import android.media.AudioManager; import android.os.Bundle; import android.os.Handler; @@ -76,7 +77,10 @@ import com.google.common.util.concurrent.SettableFuture; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.concurrent.CancellationException; +import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; +import java.util.concurrent.atomic.AtomicInteger; import org.checkerframework.checker.initialization.qual.UnderInitialization; import org.checkerframework.checker.nullness.compatqual.NullableType; @@ -93,6 +97,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; private final SessionToken token; private final ListenerSet listeners; private final ControllerCompatCallback controllerCompatCallback; + private final BitmapLoader bitmapLoader; @Nullable private MediaControllerCompat controllerCompat; @Nullable private MediaBrowserCompat browserCompat; @@ -106,7 +111,8 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; Context context, @UnderInitialization MediaController instance, SessionToken token, - Looper applicationLooper) { + Looper applicationLooper, + BitmapLoader bitmapLoader) { // Initialize default values. legacyPlayerInfo = new LegacyPlayerInfo(); pendingLegacyPlayerInfo = new LegacyPlayerInfo(); @@ -122,6 +128,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; this.instance = instance; controllerCompatCallback = new ControllerCompatCallback(applicationLooper); this.token = token; + this.bitmapLoader = bitmapLoader; } /* package */ MediaController getInstance() { @@ -716,11 +723,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; /* mediaItemTransitionReason= */ null); if (isPrepared()) { - for (int i = 0; i < mediaItems.size(); i++) { - MediaItem mediaItem = mediaItems.get(i); - controllerCompat.addQueueItem( - MediaUtils.convertToMediaDescriptionCompat(mediaItem), index + i); - } + addQueueItems(mediaItems, index); } } @@ -1340,15 +1343,61 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; } // Add all other items to the playlist if supported. if (getAvailableCommands().contains(Player.COMMAND_CHANGE_MEDIA_ITEMS)) { + List adjustedMediaItems = new ArrayList<>(); for (int i = 0; i < queueTimeline.getWindowCount(); i++) { if (i == currentIndex || queueTimeline.getQueueId(i) != QueueItem.UNKNOWN_ID) { // Skip the current item (added above) and all items already known to the session. continue; } - MediaItem mediaItem = queueTimeline.getWindow(/* windowIndex= */ i, window).mediaItem; - controllerCompat.addQueueItem( - MediaUtils.convertToMediaDescriptionCompat(mediaItem), /* index= */ i); + adjustedMediaItems.add(queueTimeline.getWindow(/* windowIndex= */ i, window).mediaItem); } + addQueueItems(adjustedMediaItems, /* startIndex= */ 0); + } + } + + private void addQueueItems(List mediaItems, int startIndex) { + List<@NullableType ListenableFuture> bitmapFutures = new ArrayList<>(); + final AtomicInteger resultCount = new AtomicInteger(0); + Runnable handleBitmapFuturesTask = + () -> { + int completedBitmapFutureCount = resultCount.incrementAndGet(); + if (completedBitmapFutureCount == mediaItems.size()) { + handleBitmapFuturesAllCompletedAndAddQueueItems( + bitmapFutures, mediaItems, /* startIndex= */ startIndex); + } + }; + + for (int i = 0; i < mediaItems.size(); i++) { + MediaItem mediaItem = mediaItems.get(i); + MediaMetadata metadata = mediaItem.mediaMetadata; + if (metadata.artworkData == null) { + bitmapFutures.add(null); + handleBitmapFuturesTask.run(); + } else { + ListenableFuture bitmapFuture = bitmapLoader.decodeBitmap(metadata.artworkData); + bitmapFutures.add(bitmapFuture); + bitmapFuture.addListener(handleBitmapFuturesTask, getInstance().applicationHandler::post); + } + } + } + + private void handleBitmapFuturesAllCompletedAndAddQueueItems( + List<@NullableType ListenableFuture> bitmapFutures, + List mediaItems, + int startIndex) { + for (int i = 0; i < bitmapFutures.size(); i++) { + @Nullable ListenableFuture future = bitmapFutures.get(i); + @Nullable Bitmap bitmap = null; + if (future != null) { + try { + bitmap = Futures.getDone(future); + } catch (CancellationException | ExecutionException e) { + Log.d(TAG, "Failed to get bitmap"); + } + } + controllerCompat.addQueueItem( + MediaUtils.convertToMediaDescriptionCompat(mediaItems.get(i), bitmap), + /* index= */ startIndex + i); } } diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java b/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java index 919d552178..11735dd6ac 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java @@ -46,7 +46,6 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; import android.annotation.SuppressLint; import android.content.Context; import android.graphics.Bitmap; -import android.graphics.BitmapFactory; import android.media.AudioManager; import android.net.Uri; import android.os.Bundle; @@ -311,23 +310,6 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; return result; } - /** - * Converts a {@link MediaItem} to a {@link MediaDescriptionCompat}. - * - * @deprecated Use {@link #convertToMediaDescriptionCompat(MediaItem, Bitmap)} instead. - */ - @Deprecated - public static MediaDescriptionCompat convertToMediaDescriptionCompat(MediaItem item) { - MediaMetadata metadata = item.mediaMetadata; - @Nullable Bitmap artworkBitmap = null; - if (metadata.artworkData != null) { - artworkBitmap = - BitmapFactory.decodeByteArray(metadata.artworkData, 0, metadata.artworkData.length); - } - - return convertToMediaDescriptionCompat(item, artworkBitmap); - } - /** Converts a {@link MediaItem} to a {@link MediaDescriptionCompat} */ public static MediaDescriptionCompat convertToMediaDescriptionCompat( MediaItem item, @Nullable Bitmap artworkBitmap) { diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionCompatCallbackWithMediaControllerTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionCompatCallbackWithMediaControllerTest.java index 82e4008c4a..b16847b8a8 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionCompatCallbackWithMediaControllerTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionCompatCallbackWithMediaControllerTest.java @@ -37,6 +37,7 @@ import android.support.v4.media.session.PlaybackStateCompat.RepeatMode; import android.support.v4.media.session.PlaybackStateCompat.ShuffleMode; import androidx.media.AudioManagerCompat; import androidx.media.VolumeProviderCompat; +import androidx.media3.common.C; import androidx.media3.common.MediaItem; import androidx.media3.common.PlaybackParameters; import androidx.media3.common.Player; @@ -52,6 +53,7 @@ import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.SdkSuppress; import androidx.test.filters.SmallTest; +import com.google.common.collect.ImmutableList; import java.util.ArrayList; import java.util.List; import java.util.concurrent.CountDownLatch; @@ -327,7 +329,7 @@ public class MediaSessionCompatCallbackWithMediaControllerTest { @Test public void addMediaItems() throws Exception { int size = 2; - List testList = MediaTestUtils.createMediaItems(size); + List testList = MediaTestUtils.createMediaItemsWithArtworkData(size); List testQueue = MediaTestUtils.convertToQueueItemsWithoutBitmap(testList); session.setQueue(testQueue); @@ -345,6 +347,7 @@ public class MediaSessionCompatCallbackWithMediaControllerTest { assertThat(sessionCallback.queueIndices.get(i)).isEqualTo(testIndex + i); assertThat(sessionCallback.queueDescriptionListForAdd.get(i).getMediaId()) .isEqualTo(testList.get(i).mediaId); + assertThat(sessionCallback.queueDescriptionListForAdd.get(i).getIconBitmap()).isNotNull(); } } @@ -391,6 +394,75 @@ public class MediaSessionCompatCallbackWithMediaControllerTest { assertThat(sessionCallback.onSkipToNextCalled).isTrue(); } + @Test + public void setMediaItems_nonEmptyList_startFromFirstMediaItem() throws Exception { + int size = 3; + List testList = MediaTestUtils.createMediaItemsWithArtworkData(size); + + session.setFlags(FLAG_HANDLES_QUEUE_COMMANDS); + setPlaybackState(PlaybackStateCompat.STATE_PLAYING); + RemoteMediaController controller = createControllerAndWaitConnection(); + sessionCallback.reset(size); + + controller.setMediaItems(testList); + + assertThat(sessionCallback.await(TIMEOUT_MS)).isTrue(); + assertThat(sessionCallback.onPlayFromMediaIdCalled).isTrue(); + assertThat(sessionCallback.mediaId).isEqualTo(testList.get(0).mediaId); + for (int i = 0; i < size - 1; i++) { + assertThat(sessionCallback.queueIndices.get(i)).isEqualTo(i); + assertThat(sessionCallback.queueDescriptionListForAdd.get(i).getMediaId()) + .isEqualTo(testList.get(i + 1).mediaId); + assertThat(sessionCallback.queueDescriptionListForAdd.get(i).getIconBitmap()).isNotNull(); + } + } + + @Test + public void setMediaItems_nonEmptyList_startFromNonFirstMediaItem() throws Exception { + int size = 5; + List testList = MediaTestUtils.createMediaItemsWithArtworkData(size); + + session.setFlags(FLAG_HANDLES_QUEUE_COMMANDS); + setPlaybackState(PlaybackStateCompat.STATE_PLAYING); + RemoteMediaController controller = createControllerAndWaitConnection(); + sessionCallback.reset(size); + int testStartIndex = 2; + + controller.setMediaItems(testList, testStartIndex, /* startPositionMs= */ C.TIME_UNSET); + + assertThat(sessionCallback.await(TIMEOUT_MS)).isTrue(); + assertThat(sessionCallback.onPlayFromMediaIdCalled).isTrue(); + assertThat(sessionCallback.mediaId).isEqualTo(testList.get(testStartIndex).mediaId); + for (int i = 0; i < size - 1; i++) { + assertThat(sessionCallback.queueIndices.get(i)).isEqualTo(i); + int adjustedIndex = (i < testStartIndex) ? i : i + 1; + assertThat(sessionCallback.queueDescriptionListForAdd.get(i).getMediaId()) + .isEqualTo(testList.get(adjustedIndex).mediaId); + assertThat(sessionCallback.queueDescriptionListForAdd.get(i).getIconBitmap()).isNotNull(); + } + } + + @Test + public void setMediaItems_emptyList() throws Exception { + int size = 3; + List testList = MediaTestUtils.createMediaItems(size); + List testQueue = MediaTestUtils.convertToQueueItemsWithoutBitmap(testList); + + session.setQueue(testQueue); + session.setFlags(FLAG_HANDLES_QUEUE_COMMANDS); + setPlaybackState(PlaybackStateCompat.STATE_PLAYING); + RemoteMediaController controller = createControllerAndWaitConnection(); + sessionCallback.reset(size); + + controller.setMediaItems(ImmutableList.of()); + + assertThat(sessionCallback.await(TIMEOUT_MS)).isTrue(); + for (int i = 0; i < size; i++) { + assertThat(sessionCallback.queueDescriptionListForRemove.get(i).getMediaId()) + .isEqualTo(testList.get(i).mediaId); + } + } + @Test public void setShuffleMode() throws Exception { session.setShuffleMode(PlaybackStateCompat.SHUFFLE_MODE_NONE);