From 33af245465ac9a8441a58a6fd6bf75c34e2efa26 Mon Sep 17 00:00:00 2001 From: tonihei Date: Mon, 22 May 2023 13:53:52 +0100 Subject: [PATCH] Improve handling of adding items to empty playlist in MediaController This is a follow-up to https://github.com/androidx/media/commit/99dac0be0f522586ca41911ca266cc5761f9dc1a where we made the same change in ExoPlayerImpl and SimpleBasePlayer, but for consistency it makes sense to also update the masking code in MediaControllerImplBase to assume the same logic. Note: MediaControllerImplLegacy already handles this case via setMediaItems and doesn't need to be updated further. #minor-release PiperOrigin-RevId: 534038759 --- .../session/MediaControllerImplBase.java | 10 ++ .../MediaControllerStateMaskingTest.java | 102 ++++++++++++++++-- 2 files changed, 106 insertions(+), 6 deletions(-) diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java index cfe1a1175f..8b037dfde9 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java @@ -918,6 +918,16 @@ import org.checkerframework.checker.nullness.qual.NonNull; if (mediaItems.isEmpty()) { return; } + if (playerInfo.timeline.isEmpty()) { + // Handle initial items in a playlist as a set operation to ensure state changes and initial + // position are updated correctly. + setMediaItemsInternal( + mediaItems, + /* startIndex= */ C.INDEX_UNSET, + /* startPositionMs= */ C.TIME_UNSET, + /* resetToDefaultPosition= */ false); + return; + } // Add media items to the end of the timeline if the index exceeds the window count. index = min(index, playerInfo.timeline.getWindowCount()); PlayerInfo newPlayerInfo = maskPlaybackInfoForAddedItems(playerInfo, index, mediaItems); diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerStateMaskingTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerStateMaskingTest.java index 086930523f..15d3dd5a65 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerStateMaskingTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerStateMaskingTest.java @@ -1833,16 +1833,21 @@ public class MediaControllerStateMaskingTest { } @Test - public void addMediaItems_toEmptyTimeline() throws Exception { + public void addMediaItems_withIdleStateAndEmptyTimeline() throws Exception { int testMediaItemCount = 2; - int testCurrentMediaItemIndex = 0; - int testNextMediaItemIndex = 1; - int testPreviousMediaItemIndex = C.INDEX_UNSET; - int testCurrentPeriodIndex = 0; + int testCurrentMediaItemIndex = 1; + int testNextMediaItemIndex = C.INDEX_UNSET; + int testPreviousMediaItemIndex = 0; + int testCurrentPeriodIndex = 1; List testMediaItems = createMediaItems(testMediaItemCount); MediaItem testMediaItem = testMediaItems.get(testCurrentPeriodIndex); - Bundle playerConfig = new RemoteMediaSession.MockPlayerConfigBuilder().build(); + Bundle playerConfig = + new RemoteMediaSession.MockPlayerConfigBuilder() + .setPlaybackState(Player.STATE_IDLE) + .setCurrentMediaItemIndex(1) + .setCurrentPeriodIndex(1) + .build(); remoteSession.setPlayer(playerConfig); MediaController controller = controllerTestRule.createController(remoteSession.getToken()); @@ -1876,6 +1881,7 @@ public class MediaControllerStateMaskingTest { AtomicInteger nextMediaItemIndexRef = new AtomicInteger(); AtomicInteger previousMediaItemIndexRef = new AtomicInteger(); AtomicInteger currentPeriodIndexRef = new AtomicInteger(); + AtomicInteger playbackStateRef = new AtomicInteger(); threadTestRule .getHandler() .postAndSync( @@ -1885,6 +1891,7 @@ public class MediaControllerStateMaskingTest { nextMediaItemIndexRef.set(controller.getNextMediaItemIndex()); previousMediaItemIndexRef.set(controller.getPreviousMediaItemIndex()); currentPeriodIndexRef.set(controller.getCurrentPeriodIndex()); + playbackStateRef.set(controller.getPlaybackState()); }); assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue(); @@ -1901,6 +1908,89 @@ public class MediaControllerStateMaskingTest { assertThat(previousMediaItemIndexRef.get()).isEqualTo(testPreviousMediaItemIndex); assertThat(currentPeriodIndexRef.get()).isEqualTo(testCurrentPeriodIndex); assertThat(newMediaItemRef.get()).isEqualTo(testMediaItem); + assertThat(playbackStateRef.get()).isEqualTo(Player.STATE_IDLE); + } + + @Test + public void addMediaItems_withEndedStateAndEmptyTimeline() throws Exception { + int testMediaItemCount = 2; + int testCurrentMediaItemIndex = 1; + int testNextMediaItemIndex = C.INDEX_UNSET; + int testPreviousMediaItemIndex = 0; + int testCurrentPeriodIndex = 1; + List testMediaItems = createMediaItems(testMediaItemCount); + MediaItem testMediaItem = testMediaItems.get(testCurrentPeriodIndex); + + Bundle playerConfig = + new RemoteMediaSession.MockPlayerConfigBuilder() + .setPlaybackState(Player.STATE_ENDED) + .setCurrentMediaItemIndex(1) + .setCurrentPeriodIndex(1) + .build(); + remoteSession.setPlayer(playerConfig); + + MediaController controller = controllerTestRule.createController(remoteSession.getToken()); + CountDownLatch latch = new CountDownLatch(3); + AtomicReference newTimelineRef = new AtomicReference<>(); + AtomicReference onEventsRef = new AtomicReference<>(); + AtomicReference newMediaItemRef = new AtomicReference<>(); + Player.Listener listener = + new Player.Listener() { + @Override + public void onTimelineChanged(Timeline timeline, int reason) { + newTimelineRef.set(timeline); + latch.countDown(); + } + + @Override + public void onMediaItemTransition(@Nullable MediaItem mediaItem, int reason) { + newMediaItemRef.set(mediaItem); + latch.countDown(); + } + + @Override + public void onEvents(Player player, Player.Events events) { + onEventsRef.set(events); + latch.countDown(); + } + }; + threadTestRule.getHandler().postAndSync(() -> controller.addListener(listener)); + + AtomicInteger currentMediaItemIndexRef = new AtomicInteger(); + AtomicInteger nextMediaItemIndexRef = new AtomicInteger(); + AtomicInteger previousMediaItemIndexRef = new AtomicInteger(); + AtomicInteger currentPeriodIndexRef = new AtomicInteger(); + AtomicInteger playbackStateRef = new AtomicInteger(); + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.addMediaItems(testMediaItems); + currentMediaItemIndexRef.set(controller.getCurrentMediaItemIndex()); + nextMediaItemIndexRef.set(controller.getNextMediaItemIndex()); + previousMediaItemIndexRef.set(controller.getPreviousMediaItemIndex()); + currentPeriodIndexRef.set(controller.getCurrentPeriodIndex()); + playbackStateRef.set(controller.getPlaybackState()); + }); + + assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue(); + assertTimeline( + newTimelineRef.get(), + testMediaItemCount, + testCurrentMediaItemIndex, + /* testFirstPeriodIndex= */ testCurrentPeriodIndex, + /* testLastPeriodIndex= */ testCurrentPeriodIndex); + assertThat(getEventsAsList(onEventsRef.get())) + .containsExactly( + Player.EVENT_TIMELINE_CHANGED, + Player.EVENT_MEDIA_ITEM_TRANSITION, + Player.EVENT_PLAYBACK_STATE_CHANGED); + assertThat(currentMediaItemIndexRef.get()).isEqualTo(testCurrentMediaItemIndex); + assertThat(nextMediaItemIndexRef.get()).isEqualTo(testNextMediaItemIndex); + assertThat(previousMediaItemIndexRef.get()).isEqualTo(testPreviousMediaItemIndex); + assertThat(currentPeriodIndexRef.get()).isEqualTo(testCurrentPeriodIndex); + assertThat(newMediaItemRef.get()).isEqualTo(testMediaItem); + assertThat(playbackStateRef.get()).isEqualTo(Player.STATE_BUFFERING); } @Test