Skip to content

Commit 1c557e2

Browse files
toniheirohitjoins
tonihei
authored and
rohitjoins
committed
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. #minor-release Issue: #296 PiperOrigin-RevId: 523986626
1 parent 596a7c7 commit 1c557e2

File tree

7 files changed

+272
-7
lines changed

7 files changed

+272
-7
lines changed

‎RELEASENOTES.md

+4
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@
4848
* `void increaseDeviceVolume(int)`
4949
* `void decreaseDeviceVolume(int)`
5050
* `void setDeviceMuted(boolean, int)`
51+
* Fix issue that `TrackSelectionOverride` instances sent from a
52+
`MediaController` are ignored if they reference a group with
53+
`Format.metadata`
54+
([#296](https://github.com/androidx/media/issues/296)).
5155
* Audio:
5256
* Fix bug where some playbacks fail when tunneling is enabled and
5357
`AudioProcessors` are active, e.g. for gapless trimming

‎libraries/common/src/main/java/androidx/media3/common/Tracks.java

+12
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,18 @@ public boolean isTrackSelected(int trackIndex) {
191191
return mediaTrackGroup.type;
192192
}
193193

194+
/**
195+
* Copies the {@code Group} with a new {@link TrackGroup#id}.
196+
*
197+
* @param groupId The new {@link TrackGroup#id}
198+
* @return The copied {@code Group}.
199+
*/
200+
@UnstableApi
201+
public Group copyWithId(String groupId) {
202+
return new Group(
203+
mediaTrackGroup.copyWithId(groupId), adaptiveSupported, trackSupport, trackSelected);
204+
}
205+
194206
@Override
195207
public boolean equals(@Nullable Object other) {
196208
if (this == other) {

‎libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ public void broadcastCustomCommand(SessionCommand command, Bundle args) {
352352

353353
private void dispatchOnPlayerInfoChanged(
354354
PlayerInfo playerInfo, boolean excludeTimeline, boolean excludeTracks) {
355-
355+
playerInfo = sessionStub.generateAndCacheUniqueTrackGroupIds(playerInfo);
356356
List<ControllerInfo> controllers =
357357
sessionStub.getConnectedControllersManager().getConnectedControllers();
358358
for (int i = 0; i < controllers.size(); i++) {

‎libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java

+73-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,10 @@
7272
import androidx.media3.common.PlaybackParameters;
7373
import androidx.media3.common.Player;
7474
import androidx.media3.common.Rating;
75+
import androidx.media3.common.TrackGroup;
76+
import androidx.media3.common.TrackSelectionOverride;
7577
import androidx.media3.common.TrackSelectionParameters;
78+
import androidx.media3.common.Tracks;
7679
import androidx.media3.common.util.Assertions;
7780
import androidx.media3.common.util.BundleableUtil;
7881
import androidx.media3.common.util.Consumer;
@@ -84,6 +87,7 @@
8487
import androidx.media3.session.MediaSession.ControllerInfo;
8588
import androidx.media3.session.MediaSession.MediaItemsWithStartPosition;
8689
import androidx.media3.session.SessionCommand.CommandCode;
90+
import com.google.common.collect.ImmutableBiMap;
8791
import com.google.common.collect.ImmutableList;
8892
import com.google.common.util.concurrent.Futures;
8993
import com.google.common.util.concurrent.ListenableFuture;
@@ -115,13 +119,17 @@
115119
private final ConnectedControllersManager<IBinder> connectedControllersManager;
116120
private final Set<ControllerInfo> pendingControllers;
117121

122+
private ImmutableBiMap<TrackGroup, String> trackGroupIdMap;
123+
private int nextUniqueTrackGroupIdPrefix;
124+
118125
public MediaSessionStub(MediaSessionImpl sessionImpl) {
119126
// Initialize members with params.
120127
this.sessionImpl = new WeakReference<>(sessionImpl);
121128
sessionManager = MediaSessionManager.getSessionManager(sessionImpl.getContext());
122129
connectedControllersManager = new ConnectedControllersManager<>(sessionImpl);
123130
// ConcurrentHashMap has a bug in APIs 21-22 that can result in lost updates.
124131
pendingControllers = Collections.synchronizedSet(new HashSet<>());
132+
trackGroupIdMap = ImmutableBiMap.of();
125133
}
126134

127135
public ConnectedControllersManager<IBinder> getConnectedControllersManager() {
@@ -495,6 +503,7 @@ public void connect(
495503
// session/controller.
496504
PlayerWrapper playerWrapper = sessionImpl.getPlayerWrapper();
497505
PlayerInfo playerInfo = playerWrapper.createPlayerInfoForBundling();
506+
playerInfo = generateAndCacheUniqueTrackGroupIds(playerInfo);
498507
ConnectionState state =
499508
new ConnectionState(
500509
MediaLibraryInfo.VERSION_INT,
@@ -1493,7 +1502,11 @@ public void setTrackSelectionParameters(
14931502
sequenceNumber,
14941503
COMMAND_SET_TRACK_SELECTION_PARAMETERS,
14951504
sendSessionResultSuccess(
1496-
player -> player.setTrackSelectionParameters(trackSelectionParameters)));
1505+
player -> {
1506+
TrackSelectionParameters updatedParameters =
1507+
updateOverridesUsingUniqueTrackGroupIds(trackSelectionParameters);
1508+
player.setTrackSelectionParameters(updatedParameters);
1509+
}));
14971510
}
14981511

14991512
//////////////////////////////////////////////////////////////////////////////////////////////
@@ -1680,6 +1693,65 @@ public void unsubscribe(@Nullable IMediaController caller, int sequenceNumber, S
16801693
librarySessionImpl.onUnsubscribeOnHandler(controller, parentId)));
16811694
}
16821695

1696+
/* package */ PlayerInfo generateAndCacheUniqueTrackGroupIds(PlayerInfo playerInfo) {
1697+
ImmutableList<Tracks.Group> trackGroups = playerInfo.currentTracks.getGroups();
1698+
ImmutableList.Builder<Tracks.Group> updatedTrackGroups = ImmutableList.builder();
1699+
ImmutableBiMap.Builder<TrackGroup, String> updatedTrackGroupIdMap = ImmutableBiMap.builder();
1700+
for (int i = 0; i < trackGroups.size(); i++) {
1701+
Tracks.Group trackGroup = trackGroups.get(i);
1702+
TrackGroup mediaTrackGroup = trackGroup.getMediaTrackGroup();
1703+
@Nullable String uniqueId = trackGroupIdMap.get(mediaTrackGroup);
1704+
if (uniqueId == null) {
1705+
uniqueId = generateUniqueTrackGroupId(mediaTrackGroup);
1706+
}
1707+
updatedTrackGroupIdMap.put(mediaTrackGroup, uniqueId);
1708+
updatedTrackGroups.add(trackGroup.copyWithId(uniqueId));
1709+
}
1710+
trackGroupIdMap = updatedTrackGroupIdMap.buildOrThrow();
1711+
playerInfo = playerInfo.copyWithCurrentTracks(new Tracks(updatedTrackGroups.build()));
1712+
if (playerInfo.trackSelectionParameters.overrides.isEmpty()) {
1713+
return playerInfo;
1714+
}
1715+
TrackSelectionParameters.Builder updatedTrackSelectionParameters =
1716+
playerInfo.trackSelectionParameters.buildUpon().clearOverrides();
1717+
for (TrackSelectionOverride override : playerInfo.trackSelectionParameters.overrides.values()) {
1718+
TrackGroup trackGroup = override.mediaTrackGroup;
1719+
@Nullable String uniqueId = trackGroupIdMap.get(trackGroup);
1720+
if (uniqueId != null) {
1721+
updatedTrackSelectionParameters.addOverride(
1722+
new TrackSelectionOverride(trackGroup.copyWithId(uniqueId), override.trackIndices));
1723+
} else {
1724+
updatedTrackSelectionParameters.addOverride(override);
1725+
}
1726+
}
1727+
return playerInfo.copyWithTrackSelectionParameters(updatedTrackSelectionParameters.build());
1728+
}
1729+
1730+
private TrackSelectionParameters updateOverridesUsingUniqueTrackGroupIds(
1731+
TrackSelectionParameters trackSelectionParameters) {
1732+
if (trackSelectionParameters.overrides.isEmpty()) {
1733+
return trackSelectionParameters;
1734+
}
1735+
TrackSelectionParameters.Builder updateTrackSelectionParameters =
1736+
trackSelectionParameters.buildUpon().clearOverrides();
1737+
for (TrackSelectionOverride override : trackSelectionParameters.overrides.values()) {
1738+
TrackGroup trackGroup = override.mediaTrackGroup;
1739+
@Nullable TrackGroup originalTrackGroup = trackGroupIdMap.inverse().get(trackGroup.id);
1740+
if (originalTrackGroup != null
1741+
&& override.mediaTrackGroup.length == originalTrackGroup.length) {
1742+
updateTrackSelectionParameters.addOverride(
1743+
new TrackSelectionOverride(originalTrackGroup, override.trackIndices));
1744+
} else {
1745+
updateTrackSelectionParameters.addOverride(override);
1746+
}
1747+
}
1748+
return updateTrackSelectionParameters.build();
1749+
}
1750+
1751+
private String generateUniqueTrackGroupId(TrackGroup trackGroup) {
1752+
return Util.intToStringMaxRadix(nextUniqueTrackGroupIdPrefix++) + "-" + trackGroup.id;
1753+
}
1754+
16831755
/** Common interface for code snippets to handle all incoming commands from the controller. */
16841756
private interface SessionTask<T, K extends MediaSessionImpl> {
16851757
T run(K sessionImpl, ControllerInfo controller, int sequenceNumber);

‎libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java

+5-4
Original file line numberDiff line numberDiff line change
@@ -1095,15 +1095,16 @@ public void onEvents(Player player, Player.Events events) {
10951095

10961096
assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
10971097
assertThat(initialCurrentTracksRef.get()).isEqualTo(Tracks.EMPTY);
1098-
assertThat(changedCurrentTracksFromParamRef.get()).isEqualTo(currentTracks);
1099-
assertThat(changedCurrentTracksFromGetterRef.get()).isEqualTo(currentTracks);
1098+
assertThat(changedCurrentTracksFromParamRef.get().getGroups()).hasSize(2);
1099+
assertThat(changedCurrentTracksFromGetterRef.get())
1100+
.isEqualTo(changedCurrentTracksFromParamRef.get());
11001101
assertThat(capturedEvents).hasSize(2);
11011102
assertThat(getEventsAsList(capturedEvents.get(0))).containsExactly(Player.EVENT_TRACKS_CHANGED);
11021103
assertThat(getEventsAsList(capturedEvents.get(1)))
11031104
.containsExactly(Player.EVENT_IS_LOADING_CHANGED);
11041105
assertThat(changedCurrentTracksFromOnEvents).hasSize(2);
1105-
assertThat(changedCurrentTracksFromOnEvents.get(0)).isEqualTo(currentTracks);
1106-
assertThat(changedCurrentTracksFromOnEvents.get(1)).isEqualTo(currentTracks);
1106+
assertThat(changedCurrentTracksFromOnEvents.get(0).getGroups()).hasSize(2);
1107+
assertThat(changedCurrentTracksFromOnEvents.get(1).getGroups()).hasSize(2);
11071108
// Assert that an equal instance is not re-sent over the binder.
11081109
assertThat(changedCurrentTracksFromOnEvents.get(0))
11091110
.isSameInstanceAs(changedCurrentTracksFromOnEvents.get(1));

‎libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerTest.java

+115-1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import androidx.media3.common.MediaItem;
4343
import androidx.media3.common.MediaLibraryInfo;
4444
import androidx.media3.common.MediaMetadata;
45+
import androidx.media3.common.Metadata;
4546
import androidx.media3.common.PlaybackException;
4647
import androidx.media3.common.PlaybackParameters;
4748
import androidx.media3.common.Player;
@@ -50,6 +51,7 @@
5051
import androidx.media3.common.StarRating;
5152
import androidx.media3.common.Timeline;
5253
import androidx.media3.common.TrackGroup;
54+
import androidx.media3.common.TrackSelectionOverride;
5355
import androidx.media3.common.TrackSelectionParameters;
5456
import androidx.media3.common.Tracks;
5557
import androidx.media3.common.VideoSize;
@@ -427,7 +429,7 @@ public void gettersAfterConnected() throws Exception {
427429
assertThat(seekForwardIncrementRef.get()).isEqualTo(seekForwardIncrementMs);
428430
assertThat(maxSeekToPreviousPositionMsRef.get()).isEqualTo(maxSeekToPreviousPositionMs);
429431
assertThat(trackSelectionParametersRef.get()).isEqualTo(trackSelectionParameters);
430-
assertThat(currentTracksRef.get()).isEqualTo(currentTracks);
432+
assertThat(currentTracksRef.get().getGroups()).hasSize(2);
431433
assertTimelineMediaItemsEquals(timelineRef.get(), timeline);
432434
assertThat(currentMediaItemIndexRef.get()).isEqualTo(currentMediaItemIndex);
433435
assertThat(currentMediaItemRef.get()).isEqualTo(currentMediaItem);
@@ -1211,6 +1213,118 @@ public void getMediaMetadata() throws Exception {
12111213
assertThat(mediaMetadata).isEqualTo(testMediaMetadata);
12121214
}
12131215

1216+
@Test
1217+
public void getCurrentTracks_hasEqualTrackGroupsForEqualGroupsInPlayer() throws Exception {
1218+
// Include metadata in Format to ensure the track group can't be fully bundled.
1219+
Tracks initialPlayerTracks =
1220+
new Tracks(
1221+
ImmutableList.of(
1222+
new Tracks.Group(
1223+
new TrackGroup(
1224+
new Format.Builder().setMetadata(new Metadata()).setId("1").build()),
1225+
/* adaptiveSupported= */ false,
1226+
/* trackSupport= */ new int[1],
1227+
/* trackSelected= */ new boolean[1]),
1228+
new Tracks.Group(
1229+
new TrackGroup(
1230+
new Format.Builder().setMetadata(new Metadata()).setId("2").build()),
1231+
/* adaptiveSupported= */ false,
1232+
/* trackSupport= */ new int[1],
1233+
/* trackSelected= */ new boolean[1])));
1234+
Tracks updatedPlayerTracks =
1235+
new Tracks(
1236+
ImmutableList.of(
1237+
new Tracks.Group(
1238+
new TrackGroup(
1239+
new Format.Builder().setMetadata(new Metadata()).setId("2").build()),
1240+
/* adaptiveSupported= */ true,
1241+
/* trackSupport= */ new int[] {C.FORMAT_HANDLED},
1242+
/* trackSelected= */ new boolean[] {true}),
1243+
new Tracks.Group(
1244+
new TrackGroup(
1245+
new Format.Builder().setMetadata(new Metadata()).setId("3").build()),
1246+
/* adaptiveSupported= */ false,
1247+
/* trackSupport= */ new int[1],
1248+
/* trackSelected= */ new boolean[1])));
1249+
Bundle playerConfig =
1250+
new RemoteMediaSession.MockPlayerConfigBuilder()
1251+
.setCurrentTracks(initialPlayerTracks)
1252+
.build();
1253+
remoteSession.setPlayer(playerConfig);
1254+
MediaController controller = controllerTestRule.createController(remoteSession.getToken());
1255+
CountDownLatch trackChangedEvent = new CountDownLatch(1);
1256+
threadTestRule
1257+
.getHandler()
1258+
.postAndSync(
1259+
() ->
1260+
controller.addListener(
1261+
new Player.Listener() {
1262+
@Override
1263+
public void onTracksChanged(Tracks tracks) {
1264+
trackChangedEvent.countDown();
1265+
}
1266+
}));
1267+
1268+
Tracks initialControllerTracks =
1269+
threadTestRule.getHandler().postAndSync(controller::getCurrentTracks);
1270+
// Do something unrelated first to ensure tracks are correctly kept even after multiple updates.
1271+
remoteSession.getMockPlayer().notifyPlaybackStateChanged(Player.STATE_READY);
1272+
remoteSession.getMockPlayer().notifyTracksChanged(updatedPlayerTracks);
1273+
trackChangedEvent.await();
1274+
Tracks updatedControllerTracks =
1275+
threadTestRule.getHandler().postAndSync(controller::getCurrentTracks);
1276+
1277+
assertThat(initialControllerTracks.getGroups()).hasSize(2);
1278+
assertThat(updatedControllerTracks.getGroups()).hasSize(2);
1279+
assertThat(initialControllerTracks.getGroups().get(1).getMediaTrackGroup())
1280+
.isEqualTo(updatedControllerTracks.getGroups().get(0).getMediaTrackGroup());
1281+
}
1282+
1283+
@Test
1284+
public void getCurrentTracksAndTrackOverrides_haveEqualTrackGroupsForEqualGroupsInPlayer()
1285+
throws Exception {
1286+
// Include metadata in Format to ensure the track group can't be fully bundled.
1287+
TrackGroup playerTrackGroupForOverride =
1288+
new TrackGroup(new Format.Builder().setMetadata(new Metadata()).setId("2").build());
1289+
Tracks playerTracks =
1290+
new Tracks(
1291+
ImmutableList.of(
1292+
new Tracks.Group(
1293+
new TrackGroup(
1294+
new Format.Builder().setMetadata(new Metadata()).setId("1").build()),
1295+
/* adaptiveSupported= */ false,
1296+
/* trackSupport= */ new int[1],
1297+
/* trackSelected= */ new boolean[1]),
1298+
new Tracks.Group(
1299+
playerTrackGroupForOverride,
1300+
/* adaptiveSupported= */ false,
1301+
/* trackSupport= */ new int[1],
1302+
/* trackSelected= */ new boolean[1])));
1303+
TrackSelectionParameters trackSelectionParameters =
1304+
TrackSelectionParameters.DEFAULT_WITHOUT_CONTEXT
1305+
.buildUpon()
1306+
.addOverride(
1307+
new TrackSelectionOverride(playerTrackGroupForOverride, /* trackIndex= */ 0))
1308+
.build();
1309+
Bundle playerConfig =
1310+
new RemoteMediaSession.MockPlayerConfigBuilder()
1311+
.setCurrentTracks(playerTracks)
1312+
.setTrackSelectionParameters(trackSelectionParameters)
1313+
.build();
1314+
remoteSession.setPlayer(playerConfig);
1315+
MediaController controller = controllerTestRule.createController(remoteSession.getToken());
1316+
1317+
Tracks controllerTracks = threadTestRule.getHandler().postAndSync(controller::getCurrentTracks);
1318+
TrackSelectionParameters controllerTrackSelectionParameters =
1319+
threadTestRule.getHandler().postAndSync(controller::getTrackSelectionParameters);
1320+
1321+
TrackGroup controllerTrackGroup = controllerTracks.getGroups().get(1).getMediaTrackGroup();
1322+
assertThat(controllerTrackSelectionParameters.overrides)
1323+
.containsExactly(
1324+
controllerTrackGroup,
1325+
new TrackSelectionOverride(controllerTrackGroup, /* trackIndex= */ 0));
1326+
}
1327+
12141328
@Test
12151329
public void
12161330
setMediaItems_setLessMediaItemsThanCurrentMediaItemIndex_masksCurrentMediaItemIndexAndStateCorrectly()

0 commit comments

Comments
 (0)