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