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 41e1f5a97a..68fcbf3576 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java @@ -47,6 +47,7 @@ import android.view.SurfaceHolder; import android.view.SurfaceView; import android.view.TextureView; import androidx.annotation.Nullable; +import androidx.collection.ArraySet; import androidx.media3.common.AdPlaybackState; import androidx.media3.common.AudioAttributes; import androidx.media3.common.BundleListRetriever; @@ -110,6 +111,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; private final SurfaceCallback surfaceCallback; private final ListenerSet listeners; private final FlushCommandQueueHandler flushCommandQueueHandler; + private final ArraySet pendingMaskingSequencedFutureNumbers; @Nullable private SessionToken connectedToken; @Nullable private SessionServiceConnection serviceConnection; @@ -127,6 +129,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; @Nullable private IMediaSession iSession; private long lastReturnedCurrentPositionMs; private long lastSetPlayWhenReadyCalledTimeMs; + @Nullable private Timeline pendingPlayerInfoUpdateTimeline; public MediaControllerImplBase( Context context, @@ -154,6 +157,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; this.context = context; sequencedFutureManager = new SequencedFutureManager(); controllerStub = new MediaControllerStub(this); + pendingMaskingSequencedFutureNumbers = new ArraySet<>(); this.token = token; this.connectionHints = connectionHints; deathRecipient = @@ -303,7 +307,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; if (command != Player.COMMAND_SET_VIDEO_SURFACE) { flushCommandQueueHandler.sendFlushCommandQueueMessage(); } - return dispatchRemoteSessionTask(iSession, task); + return dispatchRemoteSessionTask(iSession, task, /* addToPendingMaskingOperations= */ true); } private ListenableFuture dispatchRemoteSessionTaskWithSessionCommand( @@ -326,17 +330,22 @@ import org.checkerframework.checker.nullness.qual.NonNull; sessionCommand != null ? getSessionInterfaceWithSessionCommandIfAble(sessionCommand) : getSessionInterfaceWithSessionCommandIfAble(commandCode), - task); + task, + /* addToPendingMaskingOperations= */ false); } private ListenableFuture dispatchRemoteSessionTask( - IMediaSession iSession, RemoteSessionTask task) { + IMediaSession iSession, RemoteSessionTask task, boolean addToPendingMaskingOperations) { if (iSession != null) { SequencedFutureManager.SequencedFuture result = sequencedFutureManager.createSequencedFuture( new SessionResult(SessionResult.RESULT_INFO_SKIPPED)); try { - task.run(iSession, result.getSequenceNumber()); + int sequenceNumber = result.getSequenceNumber(); + task.run(iSession, sequenceNumber); + if (addToPendingMaskingOperations) { + pendingMaskingSequencedFutureNumbers.add(sequenceNumber); + } } catch (RemoteException e) { Log.w(TAG, "Cannot connect to the service or the session is gone", e); result.set(new SessionResult(SessionResult.RESULT_ERROR_SESSION_DISCONNECTED)); @@ -2223,7 +2232,12 @@ import org.checkerframework.checker.nullness.qual.NonNull; } void setFutureResult(int seq, T futureResult) { + // Don't set the future result on the application looper so that the result can be obtained by a + // blocking future.get() on the application looper. But post a message to remove the pending + // masking operation on the application looper to ensure it's executed in order with other + // updates sent to the application looper. sequencedFutureManager.setFutureResult(seq, futureResult); + getInstance().runOnApplicationLooper(() -> pendingMaskingSequencedFutureNumbers.remove(seq)); } void onConnected(ConnectionState result) { @@ -2313,11 +2327,23 @@ import org.checkerframework.checker.nullness.qual.NonNull; if (!isConnected()) { return; } + if (!pendingMaskingSequencedFutureNumbers.isEmpty()) { + // We are still waiting for all pending masking operations to be handled. + if (!isTimelineExcluded) { + pendingPlayerInfoUpdateTimeline = newPlayerInfo.timeline; + } + return; + } PlayerInfo oldPlayerInfo = playerInfo; playerInfo = newPlayerInfo; if (isTimelineExcluded) { - playerInfo = playerInfo.copyWithTimeline(oldPlayerInfo.timeline); + playerInfo = + playerInfo.copyWithTimeline( + pendingPlayerInfoUpdateTimeline != null + ? pendingPlayerInfoUpdateTimeline + : oldPlayerInfo.timeline); } + pendingPlayerInfoUpdateTimeline = null; PlaybackException oldPlayerError = oldPlayerInfo.playerError; PlaybackException playerError = playerInfo.playerError; boolean errorsMatch = @@ -2568,7 +2594,8 @@ import org.checkerframework.checker.nullness.qual.NonNull; } private void updateSessionPositionInfoIfNeeded(SessionPositionInfo sessionPositionInfo) { - if (playerInfo.sessionPositionInfo.eventTimeMs < sessionPositionInfo.eventTimeMs) { + if (pendingMaskingSequencedFutureNumbers.isEmpty() + && playerInfo.sessionPositionInfo.eventTimeMs < sessionPositionInfo.eventTimeMs) { playerInfo = playerInfo.copyWithSessionPositionInfo(sessionPositionInfo); } } 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 e7ffef7880..7e7f18dc17 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 @@ -2878,6 +2878,85 @@ public class MediaControllerStateMaskingTest { /* testPreviousMediaItemIndex= */ C.INDEX_UNSET); } + @Test + public void incompatibleUpdatesDuringMasking_areOnlyReportedOnceAllPendingUpdatesAreResolved() + throws Exception { + // Test setup: + // 1. Report a discontinuity from item 0 to item 1 in the session. + // 2. Before (1) can be handled by the controller, remove item 1. + // Expectation: + // - Session: State is updated to ENDED as the current item is removed. + // - Controller: Discontinuity is only reported after the state is fully resolved + // = The discontinuity is only reported once we also report the state change to ENDED. + Timeline timeline = MediaTestUtils.createTimeline(/* windowCount= */ 2); + remoteSession.getMockPlayer().setTimeline(timeline); + remoteSession + .getMockPlayer() + .notifyTimelineChanged(Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED); + remoteSession.getMockPlayer().setCurrentMediaItemIndex(0); + MediaController controller = controllerTestRule.createController(remoteSession.getToken()); + CountDownLatch positionDiscontinuityReported = new CountDownLatch(1); + AtomicBoolean reportedStateChangeToEndedAtSameTimeAsDiscontinuity = new AtomicBoolean(); + Player.Listener listener = + new Player.Listener() { + @Override + public void onEvents(Player player, Player.Events events) { + if (events.contains(Player.EVENT_POSITION_DISCONTINUITY)) { + if (events.contains(Player.EVENT_PLAYBACK_STATE_CHANGED) + && player.getPlaybackState() == Player.STATE_ENDED) { + reportedStateChangeToEndedAtSameTimeAsDiscontinuity.set(true); + } + positionDiscontinuityReported.countDown(); + } + } + }; + threadTestRule.getHandler().postAndSync(() -> controller.addListener(listener)); + + // Step 1: Report a discontinuity from item 0 to item 1 in the session. + PositionInfo oldPositionInfo = + new PositionInfo( + /* windowUid= */ timeline.getWindow(/* windowIndex= */ 0, new Window()).uid, + /* mediaItemIndex= */ 0, + MediaItem.EMPTY, + /* periodUid= */ timeline.getPeriod( + /* periodIndex= */ 0, new Period(), /* setIds= */ true) + .uid, + /* periodIndex= */ 0, + /* positionMs= */ 10_000, + /* contentPositionMs= */ 10_000, + /* adGroupIndex= */ C.INDEX_UNSET, + /* adIndexInAdGroup= */ C.INDEX_UNSET); + PositionInfo newPositionInfo = + new PositionInfo( + /* windowUid= */ timeline.getWindow(/* windowIndex= */ 1, new Window()).uid, + /* mediaItemIndex= */ 1, + MediaItem.EMPTY, + /* periodUid= */ timeline.getPeriod( + /* periodIndex= */ 1, new Period(), /* setIds= */ true) + .uid, + /* periodIndex= */ 1, + /* positionMs= */ 0, + /* contentPositionMs= */ 0, + /* adGroupIndex= */ C.INDEX_UNSET, + /* adIndexInAdGroup= */ C.INDEX_UNSET); + remoteSession.getMockPlayer().setCurrentMediaItemIndex(1); + remoteSession + .getMockPlayer() + .notifyPositionDiscontinuity( + oldPositionInfo, newPositionInfo, Player.DISCONTINUITY_REASON_AUTO_TRANSITION); + // Step 2: Before step 1 can be handled by the controller, remove item 1. + threadTestRule.getHandler().postAndSync(() -> controller.removeMediaItem(/* index= */ 1)); + remoteSession.getMockPlayer().setCurrentMediaItemIndex(0); + remoteSession.getMockPlayer().setTimeline(MediaTestUtils.createTimeline(/* windowCount= */ 1)); + remoteSession.getMockPlayer().notifyPlaybackStateChanged(Player.STATE_ENDED); + remoteSession + .getMockPlayer() + .notifyTimelineChanged(Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED); + + assertThat(positionDiscontinuityReported.await(TIMEOUT_MS, MILLISECONDS)).isTrue(); + assertThat(reportedStateChangeToEndedAtSameTimeAsDiscontinuity.get()).isTrue(); + } + private void assertMoveMediaItems( int initialMediaItemCount, int initialMediaItemIndex, diff --git a/libraries/test_session_current/src/main/java/androidx/media3/session/MediaSessionProviderService.java b/libraries/test_session_current/src/main/java/androidx/media3/session/MediaSessionProviderService.java index 2f91a3a286..fa312abd31 100644 --- a/libraries/test_session_current/src/main/java/androidx/media3/session/MediaSessionProviderService.java +++ b/libraries/test_session_current/src/main/java/androidx/media3/session/MediaSessionProviderService.java @@ -785,6 +785,13 @@ public class MediaSessionProviderService extends Service { MediaSession session = sessionMap.get(sessionId); MockPlayer player = (MockPlayer) session.getPlayer(); player.timeline = Timeline.CREATOR.fromBundle(timelineBundle); + List mediaItems = new ArrayList<>(); + for (int i = 0; i < player.timeline.getWindowCount(); i++) { + mediaItems.add( + player.timeline.getWindow(/* windowIndex= */ i, new Timeline.Window()).mediaItem); + } + player.mediaItems.clear(); + player.mediaItems.addAll(mediaItems); }); } @@ -800,6 +807,8 @@ public class MediaSessionProviderService extends Service { mediaItems.add( MediaTestUtils.createMediaItem(TestUtils.getMediaIdInFakeTimeline(windowIndex))); } + player.mediaItems.clear(); + player.mediaItems.addAll(mediaItems); player.timeline = new PlaylistTimeline(mediaItems); }); }