diff --git a/RELEASENOTES.md b/RELEASENOTES.md index fbc1e5e6f2..631cf4cdde 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -48,6 +48,10 @@ * `void increaseDeviceVolume(int)` * `void decreaseDeviceVolume(int)` * `void setDeviceMuted(boolean, int)` + * Fix issue that `TrackSelectionOverride` instances sent from a + `MediaController` are ignored if they reference a group with + `Format.metadata` + ([#296](https://github.com/androidx/media/issues/296)). * Audio: * Fix bug where some playbacks fail when tunneling is enabled and `AudioProcessors` are active, e.g. for gapless trimming 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 734a8306b8..a932ac6924 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 final class Tracks implements Bundleable { 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 f37eb13947..3fac801364 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java @@ -352,7 +352,7 @@ import org.checkerframework.checker.initialization.qual.Initialized; 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 c9ec362d0f..ee33cffe39 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java @@ -72,7 +72,10 @@ import androidx.media3.common.MediaMetadata; 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; @@ -84,6 +87,7 @@ import androidx.media3.session.MediaSession.ControllerCb; 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; @@ -115,6 +119,9 @@ import java.util.concurrent.ExecutionException; 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); @@ -122,6 +129,7 @@ import java.util.concurrent.ExecutionException; 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() { @@ -495,6 +503,7 @@ import java.util.concurrent.ExecutionException; // session/controller. PlayerWrapper playerWrapper = sessionImpl.getPlayerWrapper(); PlayerInfo playerInfo = playerWrapper.createPlayerInfoForBundling(); + playerInfo = generateAndCacheUniqueTrackGroupIds(playerInfo); ConnectionState state = new ConnectionState( MediaLibraryInfo.VERSION_INT, @@ -1493,7 +1502,11 @@ import java.util.concurrent.ExecutionException; sequenceNumber, COMMAND_SET_TRACK_SELECTION_PARAMETERS, sendSessionResultSuccess( - player -> player.setTrackSelectionParameters(trackSelectionParameters))); + player -> { + TrackSelectionParameters updatedParameters = + updateOverridesUsingUniqueTrackGroupIds(trackSelectionParameters); + player.setTrackSelectionParameters(updatedParameters); + })); } ////////////////////////////////////////////////////////////////////////////////////////////// @@ -1680,6 +1693,65 @@ import java.util.concurrent.ExecutionException; 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 b8d27ac58e..1eb583db5b 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 class MediaControllerListenerTest { 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 0bf910b686..4a4c01db4f 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.IllegalSeekPositionException; 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.Rating; 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 class MediaControllerTest { 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 class MediaControllerTest { 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 c4def47aad..172f6a8dfa 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 static java.util.concurrent.TimeUnit.MILLISECONDS; 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 class MediaSessionAndControllerTest { 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)); + } }