From cf9ff4de45026c0c81f931e151a7f2118fd3981a Mon Sep 17 00:00:00 2001 From: tianyifeng Date: Thu, 29 Feb 2024 04:01:01 -0800 Subject: [PATCH] Avoid position jumping back when controller replaces the current item When the controller replaces the current item, the masking position will be changed to the default position of the new item for a short while, before the correct position comes from the session. This will interrupt the current position fetched from the controller when the playback doesn't interrupted by the item replacing. Issue: androidx/media#951 PiperOrigin-RevId: 611417539 (cherry picked from commit 1bdc58de0be73c14e1b5fdffab64d69da470ffb3) --- RELEASENOTES.md | 6 +- .../session/MediaControllerImplBase.java | 91 +++++++++++++++++-- .../androidx/media3/session/MediaUtils.java | 3 +- .../MediaControllerStateMaskingTest.java | 13 +++ 4 files changed, 102 insertions(+), 11 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index cc696c8f9f9..49d102f5269 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -27,7 +27,11 @@ ([#966](https://github.com/androidx/media/issues/966)). * Effect: * Improved PQ to SDR tone-mapping by converting color spaces. -* UI: +* Session: + * Fix issue where the current position jumps back when the controller + replaces the current item + ([#951](https://github.com/androidx/media/issues/951)). +* UI: * Fallback to include audio track language name if `Locale` cannot identify a display name ([#988](https://github.com/androidx/media/issues/988)). 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 e4c5cfcae85..9e590dbe610 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java @@ -970,7 +970,9 @@ private void addMediaItemsInternal(int index, List mediaItems) { } // 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); + PlayerInfo newPlayerInfo = + maskPlayerInfoForAddedItems( + playerInfo, index, mediaItems, getCurrentPosition(), getContentPosition()); @Nullable @Player.MediaItemTransitionReason Integer mediaItemTransitionReason = @@ -983,8 +985,23 @@ private void addMediaItemsInternal(int index, List mediaItems) { /* mediaItemTransitionReason= */ mediaItemTransitionReason); } - private static PlayerInfo maskPlaybackInfoForAddedItems( - PlayerInfo playerInfo, int index, List mediaItems) { + /** + * Returns a masking {@link PlayerInfo} for the added {@linkplain MediaItem media items} with the + * provided information. + * + * @param playerInfo The {@link PlayerInfo} that the new masking {@link PlayerInfo} is based on. + * @param index The index at which the {@linkplain MediaItem media items} are added. + * @param mediaItems The {@linkplain MediaItem media items} added. + * @param currentPositionMs The current position in milliseconds. + * @param currentContentPositionMs The current content position in milliseconds. + * @return A masking {@link PlayerInfo}. + */ + private static PlayerInfo maskPlayerInfoForAddedItems( + PlayerInfo playerInfo, + int index, + List mediaItems, + long currentPositionMs, + long currentContentPositionMs) { Timeline oldTimeline = playerInfo.timeline; List newWindows = new ArrayList<>(); List newPeriods = new ArrayList<>(); @@ -1017,6 +1034,8 @@ private static PlayerInfo maskPlaybackInfoForAddedItems( newTimeline, newMediaItemIndex, newPeriodIndex, + currentPositionMs, + currentContentPositionMs, Player.DISCONTINUITY_REASON_INTERNAL); } @@ -1066,7 +1085,14 @@ private void removeMediaItemsInternal(int fromIndex, int toIndex) { } boolean wasCurrentItemRemoved = getCurrentMediaItemIndex() >= fromIndex && getCurrentMediaItemIndex() < toIndex; - PlayerInfo newPlayerInfo = maskPlayerInfoForRemovedItems(playerInfo, fromIndex, toIndex); + PlayerInfo newPlayerInfo = + maskPlayerInfoForRemovedItems( + playerInfo, + fromIndex, + toIndex, + /* isReplacingItems= */ false, + getCurrentPosition(), + getContentPosition()); boolean didMediaItemTransitionHappen = playerInfo.sessionPositionInfo.positionInfo.mediaItemIndex >= fromIndex && playerInfo.sessionPositionInfo.positionInfo.mediaItemIndex < toIndex; @@ -1082,8 +1108,30 @@ private void removeMediaItemsInternal(int fromIndex, int toIndex) { : null); } + /** + * Returns a masking {@link PlayerInfo} for the removed {@linkplain MediaItem media items} with + * the provided information. + * + * @param playerInfo The {@link PlayerInfo} that the new masking {@link PlayerInfo} is based on. + * @param fromIndex The index at which to start removing media items (inclusive). + * @param toIndex The index of the first item to be kept (exclusive). + * @param isReplacingItems A boolean indicating whether the media items are removed due to + * replacing. + * @param currentPositionMs The current position in milliseconds. This value will be used in the + * new masking {@link PlayerInfo} if the removal of the media items doesn't affect the current + * playback position. + * @param currentContentPositionMs The current content position in milliseconds. This value will + * be used in the new masking {@link PlayerInfo} if the removal of the media items doesn't + * affect the current playback position. + * @return A masking {@link PlayerInfo}. + */ private static PlayerInfo maskPlayerInfoForRemovedItems( - PlayerInfo playerInfo, int fromIndex, int toIndex) { + PlayerInfo playerInfo, + int fromIndex, + int toIndex, + boolean isReplacingItems, + long currentPositionMs, + long currentContentPositionMs) { Timeline oldTimeline = playerInfo.timeline; List newWindows = new ArrayList<>(); List newPeriods = new ArrayList<>(); @@ -1141,6 +1189,16 @@ private static PlayerInfo maskPlayerInfoForRemovedItems( newPositionInfo, SessionPositionInfo.DEFAULT, Player.DISCONTINUITY_REASON_REMOVE); + } else if (isReplacingItems) { + newPlayerInfo = + maskTimelineAndPositionInfo( + playerInfo, + newTimeline, + newMediaItemIndex, + newPeriodIndex, + currentPositionMs, + currentContentPositionMs, + Player.DISCONTINUITY_REASON_REMOVE); } else { Window newWindow = newTimeline.getWindow(newMediaItemIndex, new Window()); long defaultPositionMs = newWindow.getDefaultPositionMs(); @@ -1182,6 +1240,8 @@ private static PlayerInfo maskPlayerInfoForRemovedItems( newTimeline, newMediaItemIndex, newPeriodIndex, + currentPositionMs, + currentContentPositionMs, Player.DISCONTINUITY_REASON_REMOVE); } @@ -1290,8 +1350,17 @@ private void replaceMediaItemsInternal(int fromIndex, int toIndex, List= fromIndex && playerInfo.sessionPositionInfo.positionInfo.mediaItemIndex < toIndex; @@ -2111,6 +2180,8 @@ private void moveMediaItemsInternal(int fromIndex, int toIndex, int newIndex) { newTimeline, newWindowIndex, newPeriodIndex, + getCurrentPosition(), + getContentPosition(), Player.DISCONTINUITY_REASON_INTERNAL); updatePlayerInfo( @@ -2972,6 +3043,8 @@ private static PlayerInfo maskTimelineAndPositionInfo( Timeline timeline, int newMediaItemIndex, int newPeriodIndex, + long newPositionMs, + long newContentPositionMs, int discontinuityReason) { PositionInfo newPositionInfo = new PositionInfo( @@ -2980,8 +3053,8 @@ private static PlayerInfo maskTimelineAndPositionInfo( timeline.getWindow(newMediaItemIndex, new Window()).mediaItem, /* periodUid= */ null, newPeriodIndex, - playerInfo.sessionPositionInfo.positionInfo.positionMs, - playerInfo.sessionPositionInfo.positionInfo.contentPositionMs, + newPositionMs, + newContentPositionMs, playerInfo.sessionPositionInfo.positionInfo.adGroupIndex, playerInfo.sessionPositionInfo.positionInfo.adIndexInAdGroup); return maskTimelineAndPositionInfo( 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 5a3a224ef2c..bae4127da85 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java @@ -244,7 +244,8 @@ public static long getUpdatedCurrentPositionMs( long lastSetPlayWhenReadyCalledTimeMs, long timeDiffMs) { boolean receivedUpdatedPositionInfo = - lastSetPlayWhenReadyCalledTimeMs < playerInfo.sessionPositionInfo.eventTimeMs; + playerInfo.sessionPositionInfo.equals(SessionPositionInfo.DEFAULT) + || (lastSetPlayWhenReadyCalledTimeMs < playerInfo.sessionPositionInfo.eventTimeMs); if (!playerInfo.isPlaying) { if (receivedUpdatedPositionInfo || currentPositionMs == C.TIME_UNSET) { return playerInfo.sessionPositionInfo.positionInfo.positionMs; 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 9accbf2e3b4..972f08330a2 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 @@ -2348,6 +2348,7 @@ public void removeMediaItems_currentItemRemoved() throws Exception { createMediaItems(firstMediaId, secondMediaId, thirdMediaId))) .setCurrentMediaItemIndex(initialMediaItemIndex) .setCurrentPeriodIndex(initialMediaItemIndex) + .setCurrentPosition(2000L) .build(); remoteSession.setPlayer(playerConfig); @@ -2409,6 +2410,7 @@ public void onEvents(Player player, Player.Events events) { /* testLastPeriodIndex= */ testCurrentMediaItemIndex); assertThat(newMediaItemRef.get().mediaId).isEqualTo(testCurrentMediaId); assertThat(newPositionInfoRef.get().mediaItemIndex).isEqualTo(testCurrentMediaItemIndex); + assertThat(newPositionInfoRef.get().positionMs).isEqualTo(0L); assertThat(getEventsAsList(onEventsRef.get())) .containsExactly( Player.EVENT_TIMELINE_CHANGED, @@ -3439,12 +3441,14 @@ public void replaceMediaItems_replacingCurrentItem_correctMasking() throws Excep new RemoteMediaSession.MockPlayerConfigBuilder() .setTimeline(MediaTestUtils.createTimeline(3)) .setCurrentMediaItemIndex(1) + .setCurrentPosition(2000L) .build(); remoteSession.setPlayer(playerConfig); MediaController controller = controllerTestRule.createController(remoteSession.getToken()); CountDownLatch latch = new CountDownLatch(2); AtomicReference newTimelineRef = new AtomicReference<>(); AtomicReference onEventsRef = new AtomicReference<>(); + AtomicReference newPositionInfoRef = new AtomicReference<>(); Player.Listener listener = new Player.Listener() { @Override @@ -3458,6 +3462,14 @@ public void onEvents(Player player, Player.Events events) { onEventsRef.set(events); latch.countDown(); } + + @Override + public void onPositionDiscontinuity( + PositionInfo oldPosition, + PositionInfo newPosition, + @Player.DiscontinuityReason int reason) { + newPositionInfoRef.set(newPosition); + } }; threadTestRule.getHandler().postAndSync(() -> controller.addListener(listener)); AtomicInteger currentMediaItemIndexRef = new AtomicInteger(); @@ -3478,6 +3490,7 @@ public void onEvents(Player player, Player.Events events) { Player.EVENT_TIMELINE_CHANGED, Player.EVENT_POSITION_DISCONTINUITY, Player.EVENT_MEDIA_ITEM_TRANSITION); + assertThat(newPositionInfoRef.get().positionMs).isEqualTo(2000L); } @Test