From 3b2b8d528dd49a38386bfb9b7ce4dfd2e26589b2 Mon Sep 17 00:00:00 2001 From: tonihei Date: Thu, 13 Apr 2023 14:42:24 +0100 Subject: [PATCH] Ensure TrackSelectionParameters overrides match existing groups The overrides specified by a MediaController may not use the exact same TrackGroup instances as known to the Player because the groups have been bundled to and from the controller. This bundling may alter the instance slightly depending on the version used on each side of the communication and the fields set (e.g. Format.metadata is not supported for bundling). This issue can be solved by creating unique track group ids for each group on the session side before bundling. On the way back, the groups in the track selection parameters can be mapped backed to their original instances based on this id. Issue: androidx/media#296 PiperOrigin-RevId: 523986626 (cherry picked from commit 1c557e2fd18c77243474af2f8c99fa6ea061b38a) --- .../java/androidx/media3/common/Tracks.java | 12 ++ .../media3/session/MediaSessionImpl.java | 2 +- .../media3/session/MediaSessionStub.java | 74 ++++++++++- .../session/MediaControllerListenerTest.java | 9 +- .../media3/session/MediaControllerTest.java | 116 +++++++++++++++++- .../MediaSessionAndControllerTest.java | 62 ++++++++++ 6 files changed, 268 insertions(+), 7 deletions(-) diff --git a/libraries/common/src/main/java/androidx/media3/common/Tracks.java b/libraries/common/src/main/java/androidx/media3/common/Tracks.java index 734a8306b8b..a932ac69246 100644 --- a/libraries/common/src/main/java/androidx/media3/common/Tracks.java +++ b/libraries/common/src/main/java/androidx/media3/common/Tracks.java @@ -191,6 +191,18 @@ public boolean isTrackSelected(int trackIndex) { return mediaTrackGroup.type; } + /** + * Copies the {@code Group} with a new {@link TrackGroup#id}. + * + * @param groupId The new {@link TrackGroup#id} + * @return The copied {@code Group}. + */ + @UnstableApi + public Group copyWithId(String groupId) { + return new Group( + mediaTrackGroup.copyWithId(groupId), adaptiveSupported, trackSupport, trackSelected); + } + @Override public boolean equals(@Nullable Object other) { if (this == other) { diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java b/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java index 13738587b41..95082214a49 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java @@ -351,7 +351,7 @@ public void broadcastCustomCommand(SessionCommand command, Bundle args) { private void dispatchOnPlayerInfoChanged( PlayerInfo playerInfo, boolean excludeTimeline, boolean excludeTracks) { - + playerInfo = sessionStub.generateAndCacheUniqueTrackGroupIds(playerInfo); List controllers = sessionStub.getConnectedControllersManager().getConnectedControllers(); for (int i = 0; i < controllers.size(); i++) { diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java b/libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java index 3c0b27a775d..2480dd9951d 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java @@ -70,7 +70,10 @@ import androidx.media3.common.PlaybackParameters; import androidx.media3.common.Player; import androidx.media3.common.Rating; +import androidx.media3.common.TrackGroup; +import androidx.media3.common.TrackSelectionOverride; import androidx.media3.common.TrackSelectionParameters; +import androidx.media3.common.Tracks; import androidx.media3.common.util.Assertions; import androidx.media3.common.util.BundleableUtil; import androidx.media3.common.util.Consumer; @@ -82,6 +85,7 @@ import androidx.media3.session.MediaSession.ControllerInfo; import androidx.media3.session.MediaSession.MediaItemsWithStartPosition; import androidx.media3.session.SessionCommand.CommandCode; +import com.google.common.collect.ImmutableBiMap; import com.google.common.collect.ImmutableList; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; @@ -113,6 +117,9 @@ private final ConnectedControllersManager connectedControllersManager; private final Set pendingControllers; + private ImmutableBiMap trackGroupIdMap; + private int nextUniqueTrackGroupIdPrefix; + public MediaSessionStub(MediaSessionImpl sessionImpl) { // Initialize members with params. this.sessionImpl = new WeakReference<>(sessionImpl); @@ -120,6 +127,7 @@ public MediaSessionStub(MediaSessionImpl sessionImpl) { connectedControllersManager = new ConnectedControllersManager<>(sessionImpl); // ConcurrentHashMap has a bug in APIs 21-22 that can result in lost updates. pendingControllers = Collections.synchronizedSet(new HashSet<>()); + trackGroupIdMap = ImmutableBiMap.of(); } public ConnectedControllersManager getConnectedControllersManager() { @@ -493,6 +501,7 @@ public void connect( // session/controller. PlayerWrapper playerWrapper = sessionImpl.getPlayerWrapper(); PlayerInfo playerInfo = playerWrapper.createPlayerInfoForBundling(); + playerInfo = generateAndCacheUniqueTrackGroupIds(playerInfo); ConnectionState state = new ConnectionState( MediaLibraryInfo.VERSION_INT, @@ -1435,7 +1444,11 @@ public void setTrackSelectionParameters( sequenceNumber, COMMAND_SET_TRACK_SELECTION_PARAMETERS, sendSessionResultSuccess( - player -> player.setTrackSelectionParameters(trackSelectionParameters))); + player -> { + TrackSelectionParameters updatedParameters = + updateOverridesUsingUniqueTrackGroupIds(trackSelectionParameters); + player.setTrackSelectionParameters(updatedParameters); + })); } ////////////////////////////////////////////////////////////////////////////////////////////// @@ -1622,6 +1635,65 @@ public void unsubscribe(@Nullable IMediaController caller, int sequenceNumber, S librarySessionImpl.onUnsubscribeOnHandler(controller, parentId))); } + /* package */ PlayerInfo generateAndCacheUniqueTrackGroupIds(PlayerInfo playerInfo) { + ImmutableList trackGroups = playerInfo.currentTracks.getGroups(); + ImmutableList.Builder updatedTrackGroups = ImmutableList.builder(); + ImmutableBiMap.Builder updatedTrackGroupIdMap = ImmutableBiMap.builder(); + for (int i = 0; i < trackGroups.size(); i++) { + Tracks.Group trackGroup = trackGroups.get(i); + TrackGroup mediaTrackGroup = trackGroup.getMediaTrackGroup(); + @Nullable String uniqueId = trackGroupIdMap.get(mediaTrackGroup); + if (uniqueId == null) { + uniqueId = generateUniqueTrackGroupId(mediaTrackGroup); + } + updatedTrackGroupIdMap.put(mediaTrackGroup, uniqueId); + updatedTrackGroups.add(trackGroup.copyWithId(uniqueId)); + } + trackGroupIdMap = updatedTrackGroupIdMap.buildOrThrow(); + playerInfo = playerInfo.copyWithCurrentTracks(new Tracks(updatedTrackGroups.build())); + if (playerInfo.trackSelectionParameters.overrides.isEmpty()) { + return playerInfo; + } + TrackSelectionParameters.Builder updatedTrackSelectionParameters = + playerInfo.trackSelectionParameters.buildUpon().clearOverrides(); + for (TrackSelectionOverride override : playerInfo.trackSelectionParameters.overrides.values()) { + TrackGroup trackGroup = override.mediaTrackGroup; + @Nullable String uniqueId = trackGroupIdMap.get(trackGroup); + if (uniqueId != null) { + updatedTrackSelectionParameters.addOverride( + new TrackSelectionOverride(trackGroup.copyWithId(uniqueId), override.trackIndices)); + } else { + updatedTrackSelectionParameters.addOverride(override); + } + } + return playerInfo.copyWithTrackSelectionParameters(updatedTrackSelectionParameters.build()); + } + + private TrackSelectionParameters updateOverridesUsingUniqueTrackGroupIds( + TrackSelectionParameters trackSelectionParameters) { + if (trackSelectionParameters.overrides.isEmpty()) { + return trackSelectionParameters; + } + TrackSelectionParameters.Builder updateTrackSelectionParameters = + trackSelectionParameters.buildUpon().clearOverrides(); + for (TrackSelectionOverride override : trackSelectionParameters.overrides.values()) { + TrackGroup trackGroup = override.mediaTrackGroup; + @Nullable TrackGroup originalTrackGroup = trackGroupIdMap.inverse().get(trackGroup.id); + if (originalTrackGroup != null + && override.mediaTrackGroup.length == originalTrackGroup.length) { + updateTrackSelectionParameters.addOverride( + new TrackSelectionOverride(originalTrackGroup, override.trackIndices)); + } else { + updateTrackSelectionParameters.addOverride(override); + } + } + return updateTrackSelectionParameters.build(); + } + + private String generateUniqueTrackGroupId(TrackGroup trackGroup) { + return Util.intToStringMaxRadix(nextUniqueTrackGroupIdPrefix++) + "-" + trackGroup.id; + } + /** Common interface for code snippets to handle all incoming commands from the controller. */ private interface SessionTask { T run(K sessionImpl, ControllerInfo controller, int sequenceNumber); diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java index 0a195d6ae14..a81ef25d518 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java @@ -1095,15 +1095,16 @@ public void onEvents(Player player, Player.Events events) { assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue(); assertThat(initialCurrentTracksRef.get()).isEqualTo(Tracks.EMPTY); - assertThat(changedCurrentTracksFromParamRef.get()).isEqualTo(currentTracks); - assertThat(changedCurrentTracksFromGetterRef.get()).isEqualTo(currentTracks); + assertThat(changedCurrentTracksFromParamRef.get().getGroups()).hasSize(2); + assertThat(changedCurrentTracksFromGetterRef.get()) + .isEqualTo(changedCurrentTracksFromParamRef.get()); assertThat(capturedEvents).hasSize(2); assertThat(getEventsAsList(capturedEvents.get(0))).containsExactly(Player.EVENT_TRACKS_CHANGED); assertThat(getEventsAsList(capturedEvents.get(1))) .containsExactly(Player.EVENT_IS_LOADING_CHANGED); assertThat(changedCurrentTracksFromOnEvents).hasSize(2); - assertThat(changedCurrentTracksFromOnEvents.get(0)).isEqualTo(currentTracks); - assertThat(changedCurrentTracksFromOnEvents.get(1)).isEqualTo(currentTracks); + assertThat(changedCurrentTracksFromOnEvents.get(0).getGroups()).hasSize(2); + assertThat(changedCurrentTracksFromOnEvents.get(1).getGroups()).hasSize(2); // Assert that an equal instance is not re-sent over the binder. assertThat(changedCurrentTracksFromOnEvents.get(0)) .isSameInstanceAs(changedCurrentTracksFromOnEvents.get(1)); diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerTest.java index 0bf910b6864..4a4c01db4fc 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerTest.java @@ -42,6 +42,7 @@ import androidx.media3.common.MediaItem; import androidx.media3.common.MediaLibraryInfo; import androidx.media3.common.MediaMetadata; +import androidx.media3.common.Metadata; import androidx.media3.common.PlaybackException; import androidx.media3.common.PlaybackParameters; import androidx.media3.common.Player; @@ -50,6 +51,7 @@ import androidx.media3.common.StarRating; import androidx.media3.common.Timeline; import androidx.media3.common.TrackGroup; +import androidx.media3.common.TrackSelectionOverride; import androidx.media3.common.TrackSelectionParameters; import androidx.media3.common.Tracks; import androidx.media3.common.VideoSize; @@ -427,7 +429,7 @@ public void gettersAfterConnected() throws Exception { assertThat(seekForwardIncrementRef.get()).isEqualTo(seekForwardIncrementMs); assertThat(maxSeekToPreviousPositionMsRef.get()).isEqualTo(maxSeekToPreviousPositionMs); assertThat(trackSelectionParametersRef.get()).isEqualTo(trackSelectionParameters); - assertThat(currentTracksRef.get()).isEqualTo(currentTracks); + assertThat(currentTracksRef.get().getGroups()).hasSize(2); assertTimelineMediaItemsEquals(timelineRef.get(), timeline); assertThat(currentMediaItemIndexRef.get()).isEqualTo(currentMediaItemIndex); assertThat(currentMediaItemRef.get()).isEqualTo(currentMediaItem); @@ -1211,6 +1213,118 @@ public void getMediaMetadata() throws Exception { assertThat(mediaMetadata).isEqualTo(testMediaMetadata); } + @Test + public void getCurrentTracks_hasEqualTrackGroupsForEqualGroupsInPlayer() throws Exception { + // Include metadata in Format to ensure the track group can't be fully bundled. + Tracks initialPlayerTracks = + new Tracks( + ImmutableList.of( + new Tracks.Group( + new TrackGroup( + new Format.Builder().setMetadata(new Metadata()).setId("1").build()), + /* adaptiveSupported= */ false, + /* trackSupport= */ new int[1], + /* trackSelected= */ new boolean[1]), + new Tracks.Group( + new TrackGroup( + new Format.Builder().setMetadata(new Metadata()).setId("2").build()), + /* adaptiveSupported= */ false, + /* trackSupport= */ new int[1], + /* trackSelected= */ new boolean[1]))); + Tracks updatedPlayerTracks = + new Tracks( + ImmutableList.of( + new Tracks.Group( + new TrackGroup( + new Format.Builder().setMetadata(new Metadata()).setId("2").build()), + /* adaptiveSupported= */ true, + /* trackSupport= */ new int[] {C.FORMAT_HANDLED}, + /* trackSelected= */ new boolean[] {true}), + new Tracks.Group( + new TrackGroup( + new Format.Builder().setMetadata(new Metadata()).setId("3").build()), + /* adaptiveSupported= */ false, + /* trackSupport= */ new int[1], + /* trackSelected= */ new boolean[1]))); + Bundle playerConfig = + new RemoteMediaSession.MockPlayerConfigBuilder() + .setCurrentTracks(initialPlayerTracks) + .build(); + remoteSession.setPlayer(playerConfig); + MediaController controller = controllerTestRule.createController(remoteSession.getToken()); + CountDownLatch trackChangedEvent = new CountDownLatch(1); + threadTestRule + .getHandler() + .postAndSync( + () -> + controller.addListener( + new Player.Listener() { + @Override + public void onTracksChanged(Tracks tracks) { + trackChangedEvent.countDown(); + } + })); + + Tracks initialControllerTracks = + threadTestRule.getHandler().postAndSync(controller::getCurrentTracks); + // Do something unrelated first to ensure tracks are correctly kept even after multiple updates. + remoteSession.getMockPlayer().notifyPlaybackStateChanged(Player.STATE_READY); + remoteSession.getMockPlayer().notifyTracksChanged(updatedPlayerTracks); + trackChangedEvent.await(); + Tracks updatedControllerTracks = + threadTestRule.getHandler().postAndSync(controller::getCurrentTracks); + + assertThat(initialControllerTracks.getGroups()).hasSize(2); + assertThat(updatedControllerTracks.getGroups()).hasSize(2); + assertThat(initialControllerTracks.getGroups().get(1).getMediaTrackGroup()) + .isEqualTo(updatedControllerTracks.getGroups().get(0).getMediaTrackGroup()); + } + + @Test + public void getCurrentTracksAndTrackOverrides_haveEqualTrackGroupsForEqualGroupsInPlayer() + throws Exception { + // Include metadata in Format to ensure the track group can't be fully bundled. + TrackGroup playerTrackGroupForOverride = + new TrackGroup(new Format.Builder().setMetadata(new Metadata()).setId("2").build()); + Tracks playerTracks = + new Tracks( + ImmutableList.of( + new Tracks.Group( + new TrackGroup( + new Format.Builder().setMetadata(new Metadata()).setId("1").build()), + /* adaptiveSupported= */ false, + /* trackSupport= */ new int[1], + /* trackSelected= */ new boolean[1]), + new Tracks.Group( + playerTrackGroupForOverride, + /* adaptiveSupported= */ false, + /* trackSupport= */ new int[1], + /* trackSelected= */ new boolean[1]))); + TrackSelectionParameters trackSelectionParameters = + TrackSelectionParameters.DEFAULT_WITHOUT_CONTEXT + .buildUpon() + .addOverride( + new TrackSelectionOverride(playerTrackGroupForOverride, /* trackIndex= */ 0)) + .build(); + Bundle playerConfig = + new RemoteMediaSession.MockPlayerConfigBuilder() + .setCurrentTracks(playerTracks) + .setTrackSelectionParameters(trackSelectionParameters) + .build(); + remoteSession.setPlayer(playerConfig); + MediaController controller = controllerTestRule.createController(remoteSession.getToken()); + + Tracks controllerTracks = threadTestRule.getHandler().postAndSync(controller::getCurrentTracks); + TrackSelectionParameters controllerTrackSelectionParameters = + threadTestRule.getHandler().postAndSync(controller::getTrackSelectionParameters); + + TrackGroup controllerTrackGroup = controllerTracks.getGroups().get(1).getMediaTrackGroup(); + assertThat(controllerTrackSelectionParameters.overrides) + .containsExactly( + controllerTrackGroup, + new TrackSelectionOverride(controllerTrackGroup, /* trackIndex= */ 0)); + } + @Test public void setMediaItems_setLessMediaItemsThanCurrentMediaItemIndex_masksCurrentMediaItemIndexAndStateCorrectly() diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionAndControllerTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionAndControllerTest.java index c4def47aad8..172f6a8dfa0 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionAndControllerTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionAndControllerTest.java @@ -24,13 +24,21 @@ import android.content.Context; import android.os.Handler; import android.os.Looper; +import androidx.media3.common.C; +import androidx.media3.common.Format; +import androidx.media3.common.Metadata; +import androidx.media3.common.MimeTypes; import androidx.media3.common.Player; +import androidx.media3.common.TrackGroup; +import androidx.media3.common.TrackSelectionOverride; +import androidx.media3.common.Tracks; import androidx.media3.common.util.Util; import androidx.media3.test.session.common.HandlerThreadTestRule; import androidx.media3.test.session.common.MainLooperTestRule; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.LargeTest; +import com.google.common.collect.ImmutableList; import com.google.common.util.concurrent.ListenableFuture; import java.util.concurrent.CancellationException; import java.util.concurrent.CountDownLatch; @@ -248,4 +256,58 @@ public void commandBeforeControllerRelease_handledBySession() throws Exception { player.awaitMethodCalled(MockPlayer.METHOD_PREPARE, TIMEOUT_MS); player.awaitMethodCalled(MockPlayer.METHOD_PLAY, TIMEOUT_MS); } + + @Test + public void setTrackSelectionParameters_withOverrides_matchesExpectedTrackGroupInPlayer() + throws Exception { + MockPlayer player = + new MockPlayer.Builder().setApplicationLooper(Looper.getMainLooper()).build(); + // Intentionally add metadata to the format as this can't be bundled. + Tracks.Group trackGroupInPlayer = + new Tracks.Group( + new TrackGroup( + new Format.Builder() + .setId("0") + .setSampleMimeType(MimeTypes.VIDEO_H264) + .setMetadata(new Metadata()) + .build(), + new Format.Builder() + .setId("1") + .setSampleMimeType(MimeTypes.VIDEO_H264) + .setMetadata(new Metadata()) + .build()), + /* adaptiveSupported= */ false, + /* trackSupport= */ new int[] {C.FORMAT_HANDLED, C.FORMAT_HANDLED}, + /* trackSelected= */ new boolean[] {true, false}); + player.currentTracks = new Tracks(ImmutableList.of(trackGroupInPlayer)); + MediaSession session = + sessionTestRule.ensureReleaseAfterTest( + new MediaSession.Builder(context, player).setId(TAG).build()); + MediaController controller = controllerTestRule.createController(session.getToken()); + + threadTestRule + .getHandler() + .postAndSync( + () -> + controller.setTrackSelectionParameters( + controller + .getTrackSelectionParameters() + .buildUpon() + .setOverrideForType( + new TrackSelectionOverride( + controller + .getCurrentTracks() + .getGroups() + .get(0) + .getMediaTrackGroup(), + /* trackIndex= */ 1)) + .build())); + player.awaitMethodCalled(MockPlayer.METHOD_SET_TRACK_SELECTION_PARAMETERS, TIMEOUT_MS); + + assertThat(player.trackSelectionParameters.overrides) + .containsExactly( + trackGroupInPlayer.getMediaTrackGroup(), + new TrackSelectionOverride( + trackGroupInPlayer.getMediaTrackGroup(), /* trackIndex= */ 1)); + } }