Skip to content

Commit

Permalink
Ensure TrackSelectionParameters overrides match existing groups
Browse files Browse the repository at this point in the history 
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.

#minor-release

Issue: #296
PiperOrigin-RevId: 523986626
  • Loading branch information
@tonihei @rohitjoins
tonihei authored and rohitjoins committed Apr 13, 2023
1 parent 596a7c7 commit 1c557e2
Show file tree
Hide file tree
Showing 7 changed files with 272 additions and 7 deletions.
4 changes: 4 additions & 0 deletions RELEASENOTES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions libraries/common/src/main/java/androidx/media3/common/Tracks.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ public void broadcastCustomCommand(SessionCommand command, Bundle args) {

private void dispatchOnPlayerInfoChanged(
PlayerInfo playerInfo, boolean excludeTimeline, boolean excludeTracks) {

playerInfo = sessionStub.generateAndCacheUniqueTrackGroupIds(playerInfo);
List<ControllerInfo> controllers =
sessionStub.getConnectedControllersManager().getConnectedControllers();
for (int i = 0; i < controllers.size(); i++) {
Expand Down
74 changes: 73 additions & 1 deletion libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,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;
Expand All @@ -84,6 +87,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;
Expand Down Expand Up @@ -115,13 +119,17 @@
private final ConnectedControllersManager<IBinder> connectedControllersManager;
private final Set<ControllerInfo> pendingControllers;

private ImmutableBiMap<TrackGroup, String> trackGroupIdMap;
private int nextUniqueTrackGroupIdPrefix;

public MediaSessionStub(MediaSessionImpl sessionImpl) {
// Initialize members with params.
this.sessionImpl = new WeakReference<>(sessionImpl);
sessionManager = MediaSessionManager.getSessionManager(sessionImpl.getContext());
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<IBinder> getConnectedControllersManager() {
Expand Down Expand Up @@ -495,6 +503,7 @@ public void connect(
// session/controller.
PlayerWrapper playerWrapper = sessionImpl.getPlayerWrapper();
PlayerInfo playerInfo = playerWrapper.createPlayerInfoForBundling();
playerInfo = generateAndCacheUniqueTrackGroupIds(playerInfo);
ConnectionState state =
new ConnectionState(
MediaLibraryInfo.VERSION_INT,
Expand Down Expand Up @@ -1493,7 +1502,11 @@ public void setTrackSelectionParameters(
sequenceNumber,
COMMAND_SET_TRACK_SELECTION_PARAMETERS,
sendSessionResultSuccess(
player -> player.setTrackSelectionParameters(trackSelectionParameters)));
player -> {
TrackSelectionParameters updatedParameters =
updateOverridesUsingUniqueTrackGroupIds(trackSelectionParameters);
player.setTrackSelectionParameters(updatedParameters);
}));
}

//////////////////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -1680,6 +1693,65 @@ public void unsubscribe(@Nullable IMediaController caller, int sequenceNumber, S
librarySessionImpl.onUnsubscribeOnHandler(controller, parentId)));
}

/* package */ PlayerInfo generateAndCacheUniqueTrackGroupIds(PlayerInfo playerInfo) {
ImmutableList<Tracks.Group> trackGroups = playerInfo.currentTracks.getGroups();
ImmutableList.Builder<Tracks.Group> updatedTrackGroups = ImmutableList.builder();
ImmutableBiMap.Builder<TrackGroup, String> 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, K extends MediaSessionImpl> {
T run(K sessionImpl, ControllerInfo controller, int sequenceNumber);
Expand Down
9 changes: 5 additions & 4 deletions ...ion_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
116 changes: 115 additions & 1 deletion ...est_session_current/src/androidTest/java/androidx/media3/session/MediaControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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()
Expand Down
Loading

0 comments on commit 1c557e2

Please sign in to comment.