From 2eab93d5c530380c30a4b95ad19bdc3243b1b398 Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 17 Jan 2023 17:30:02 +0000 Subject: [PATCH] Make availableCommands known when bundling PlayerInfo When bundling PlayerInfo, we remove data when the controller is not allowed to access this data via getters. We also remove data for performance reasons. In the toBundle() method, it's currently hard to make the connection between allowed commands and filtering, because the values are checked at a different place. This can be made more readable by forwarding the applicable Commands directly. The only functional fix is to filter the Timeline when sending the first PlayerInfo after a connecting a controller if the command to get the Timeline is not available. This also allows us to remove a path to filter MediaItems from Timelines as it isn't used. PiperOrigin-RevId: 502607391 (cherry picked from commit c90ca7ba5fb9e83956e9494a584ae6b0620e3b14) --- .../java/androidx/media3/common/Timeline.java | 45 +++---------------- .../androidx/media3/common/TimelineTest.java | 6 +-- .../media3/session/ConnectionState.java | 13 ++---- .../androidx/media3/session/MediaSession.java | 4 +- .../media3/session/MediaSessionImpl.java | 21 +++------ .../media3/session/MediaSessionStub.java | 28 +++++------- .../androidx/media3/session/MediaUtils.java | 5 ++- .../androidx/media3/session/PlayerInfo.java | 26 +++++------ 8 files changed, 45 insertions(+), 103 deletions(-) diff --git a/libraries/common/src/main/java/androidx/media3/common/Timeline.java b/libraries/common/src/main/java/androidx/media3/common/Timeline.java index 8e37968a0c..1d7706f907 100644 --- a/libraries/common/src/main/java/androidx/media3/common/Timeline.java +++ b/libraries/common/src/main/java/androidx/media3/common/Timeline.java @@ -430,18 +430,17 @@ public abstract class Timeline implements Bundleable { private static final String FIELD_POSITION_IN_FIRST_PERIOD_US = Util.intToStringMaxRadix(13); /** - * Returns a {@link Bundle} representing the information stored in this object. + * {@inheritDoc} * *

It omits the {@link #uid} and {@link #manifest} fields. The {@link #uid} of an instance * restored by {@link #CREATOR} will be a fake {@link Object} and the {@link #manifest} of the * instance will be {@code null}. - * - * @param excludeMediaItem Whether to exclude {@link #mediaItem} of window. */ @UnstableApi - public Bundle toBundle(boolean excludeMediaItem) { + @Override + public Bundle toBundle() { Bundle bundle = new Bundle(); - if (!excludeMediaItem) { + if (!MediaItem.EMPTY.equals(mediaItem)) { bundle.putBundle(FIELD_MEDIA_ITEM, mediaItem.toBundle()); } if (presentationStartTimeMs != C.TIME_UNSET) { @@ -485,20 +484,6 @@ public abstract class Timeline implements Bundleable { return bundle; } - /** - * {@inheritDoc} - * - *

It omits the {@link #uid} and {@link #manifest} fields. The {@link #uid} of an instance - * restored by {@link #CREATOR} will be a fake {@link Object} and the {@link #manifest} of the - * instance will be {@code null}. - */ - // TODO(b/166765820): See if missing fields would be okay and add them to the Bundle otherwise. - @UnstableApi - @Override - public Bundle toBundle() { - return toBundle(/* excludeMediaItem= */ false); - } - /** * Object that can restore {@link Period} from a {@link Bundle}. * @@ -1396,18 +1381,15 @@ public abstract class Timeline implements Bundleable { *

The {@link #getWindow(int, Window)} windows} and {@link #getPeriod(int, Period) periods} of * an instance restored by {@link #CREATOR} may have missing fields as described in {@link * Window#toBundle()} and {@link Period#toBundle()}. - * - * @param excludeMediaItems Whether to exclude all {@link Window#mediaItem media items} of windows - * in the timeline. */ @UnstableApi - public final Bundle toBundle(boolean excludeMediaItems) { + @Override + public final Bundle toBundle() { List windowBundles = new ArrayList<>(); int windowCount = getWindowCount(); Window window = new Window(); for (int i = 0; i < windowCount; i++) { - windowBundles.add( - getWindow(i, window, /* defaultPositionProjectionUs= */ 0).toBundle(excludeMediaItems)); + windowBundles.add(getWindow(i, window, /* defaultPositionProjectionUs= */ 0).toBundle()); } List periodBundles = new ArrayList<>(); @@ -1434,19 +1416,6 @@ public abstract class Timeline implements Bundleable { return bundle; } - /** - * {@inheritDoc} - * - *

The {@link #getWindow(int, Window)} windows} and {@link #getPeriod(int, Period) periods} of - * an instance restored by {@link #CREATOR} may have missing fields as described in {@link - * Window#toBundle()} and {@link Period#toBundle()}. - */ - @UnstableApi - @Override - public final Bundle toBundle() { - return toBundle(/* excludeMediaItems= */ false); - } - /** * Object that can restore a {@link Timeline} from a {@link Bundle}. * diff --git a/libraries/common/src/test/java/androidx/media3/common/TimelineTest.java b/libraries/common/src/test/java/androidx/media3/common/TimelineTest.java index 111652b38b..716e16ec3c 100644 --- a/libraries/common/src/test/java/androidx/media3/common/TimelineTest.java +++ b/libraries/common/src/test/java/androidx/media3/common/TimelineTest.java @@ -350,10 +350,8 @@ public class TimelineTest { Bundle windowBundle = window.toBundle(); - // Check that default values are skipped when bundling. MediaItem key is not added to the bundle - // only when excludeMediaItem is true. - assertThat(windowBundle.keySet()).hasSize(1); - assertThat(window.toBundle(/* excludeMediaItem= */ true).keySet()).isEmpty(); + // Check that default values are skipped when bundling. + assertThat(windowBundle.keySet()).isEmpty(); Timeline.Window restoredWindow = Timeline.Window.CREATOR.fromBundle(windowBundle); diff --git a/libraries/session/src/main/java/androidx/media3/session/ConnectionState.java b/libraries/session/src/main/java/androidx/media3/session/ConnectionState.java index 113848eda0..c681ab4420 100644 --- a/libraries/session/src/main/java/androidx/media3/session/ConnectionState.java +++ b/libraries/session/src/main/java/androidx/media3/session/ConnectionState.java @@ -94,19 +94,12 @@ import androidx.media3.common.util.Util; bundle.putBundle(FIELD_PLAYER_COMMANDS_FROM_SESSION, playerCommandsFromSession.toBundle()); bundle.putBundle(FIELD_PLAYER_COMMANDS_FROM_PLAYER, playerCommandsFromPlayer.toBundle()); bundle.putBundle(FIELD_TOKEN_EXTRAS, tokenExtras); + Player.Commands intersectedCommands = + MediaUtils.intersect(playerCommandsFromSession, playerCommandsFromPlayer); bundle.putBundle( FIELD_PLAYER_INFO, playerInfo.toBundle( - /* excludeMediaItems= */ !playerCommandsFromPlayer.contains(Player.COMMAND_GET_TIMELINE) - || !playerCommandsFromSession.contains(Player.COMMAND_GET_TIMELINE), - /* excludeMediaItemsMetadata= */ !playerCommandsFromPlayer.contains( - Player.COMMAND_GET_MEDIA_ITEMS_METADATA) - || !playerCommandsFromSession.contains(Player.COMMAND_GET_MEDIA_ITEMS_METADATA), - /* excludeCues= */ !playerCommandsFromPlayer.contains(Player.COMMAND_GET_TEXT) - || !playerCommandsFromSession.contains(Player.COMMAND_GET_TEXT), - /* excludeTimeline= */ false, - /* excludeTracks= */ !playerCommandsFromPlayer.contains(Player.COMMAND_GET_TRACKS) - || !playerCommandsFromSession.contains(Player.COMMAND_GET_TRACKS))); + intersectedCommands, /* excludeTimeline= */ false, /* excludeTracks= */ false)); bundle.putInt(FIELD_SESSION_INTERFACE_VERSION, sessionInterfaceVersion); return bundle; } diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaSession.java b/libraries/session/src/main/java/androidx/media3/session/MediaSession.java index 5fead90f84..d6bcc84d3c 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSession.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSession.java @@ -1155,9 +1155,7 @@ public class MediaSession { default void onPlayerInfoChanged( int seq, PlayerInfo playerInfo, - boolean excludeMediaItems, - boolean excludeMediaItemsMetadata, - boolean excludeCues, + Player.Commands availableCommands, boolean excludeTimeline, boolean excludeTracks, int controllerInterfaceVersion) 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 7ad0de53e3..11b6f37b15 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java @@ -15,9 +15,6 @@ */ package androidx.media3.session; -import static androidx.media3.common.Player.COMMAND_GET_MEDIA_ITEMS_METADATA; -import static androidx.media3.common.Player.COMMAND_GET_TEXT; -import static androidx.media3.common.Player.COMMAND_GET_TIMELINE; import static androidx.media3.common.Player.COMMAND_GET_TRACKS; import static androidx.media3.common.util.Assertions.checkNotNull; import static androidx.media3.common.util.Assertions.checkStateNotNull; @@ -416,21 +413,17 @@ import org.checkerframework.checker.initialization.qual.Initialized; // 0 is OK for legacy controllers, because they didn't have sequence numbers. seq = 0; } + Player.Commands intersectedCommands = + MediaUtils.intersect( + controllersManager.getAvailablePlayerCommands(controller), + getPlayerWrapper().getAvailableCommands()); checkStateNotNull(controller.getControllerCb()) .onPlayerInfoChanged( seq, playerInfo, - /* excludeMediaItems= */ !controllersManager.isPlayerCommandAvailable( - controller, COMMAND_GET_TIMELINE), - /* excludeMediaItemsMetadata= */ !controllersManager.isPlayerCommandAvailable( - controller, COMMAND_GET_MEDIA_ITEMS_METADATA), - /* excludeCues= */ !controllersManager.isPlayerCommandAvailable( - controller, COMMAND_GET_TEXT), - excludeTimeline - || !controllersManager.isPlayerCommandAvailable( - controller, COMMAND_GET_TIMELINE), - excludeTracks - || !controllersManager.isPlayerCommandAvailable(controller, COMMAND_GET_TRACKS), + intersectedCommands, + excludeTimeline, + excludeTracks, controller.getInterfaceVersion()); } catch (DeadObjectException e) { onDeadObjectException(controller); 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 aa51cb519c..1cbb8106d5 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java @@ -1606,35 +1606,29 @@ import java.util.concurrent.ExecutionException; public void onPlayerInfoChanged( int sequenceNumber, PlayerInfo playerInfo, - boolean excludeMediaItems, - boolean excludeMediaItemsMetadata, - boolean excludeCues, + Player.Commands availableCommands, boolean excludeTimeline, boolean excludeTracks, int controllerInterfaceVersion) throws RemoteException { Assertions.checkState(controllerInterfaceVersion != 0); + // The bundling exclusions merge the performance overrides with the available commands. + boolean bundlingExclusionsTimeline = + excludeTimeline || !availableCommands.contains(Player.COMMAND_GET_TIMELINE); + boolean bundlingExclusionsTracks = + excludeTracks || !availableCommands.contains(Player.COMMAND_GET_TRACKS); if (controllerInterfaceVersion >= 2) { iController.onPlayerInfoChangedWithExclusions( sequenceNumber, - playerInfo.toBundle( - excludeMediaItems, - excludeMediaItemsMetadata, - excludeCues, - excludeTimeline, - excludeTracks), - new PlayerInfo.BundlingExclusions(excludeTimeline, excludeTracks).toBundle()); + playerInfo.toBundle(availableCommands, excludeTimeline, excludeTracks), + new PlayerInfo.BundlingExclusions(bundlingExclusionsTimeline, bundlingExclusionsTracks) + .toBundle()); } else { //noinspection deprecation iController.onPlayerInfoChanged( sequenceNumber, - playerInfo.toBundle( - excludeMediaItems, - excludeMediaItemsMetadata, - excludeCues, - excludeTimeline, - /* excludeTracks= */ true), - excludeTimeline); + playerInfo.toBundle(availableCommands, excludeTimeline, /* excludeTracks= */ true), + bundlingExclusionsTimeline); } } diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java b/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java index 716f08a29d..0d61696904 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java @@ -1292,7 +1292,10 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; * Returns the intersection of {@link Player.Command commands} from the given two {@link * Commands}. */ - public static Commands intersect(Commands commands1, Commands commands2) { + public static Commands intersect(@Nullable Commands commands1, @Nullable Commands commands2) { + if (commands1 == null || commands2 == null) { + return Commands.EMPTY; + } Commands.Builder intersectCommandsBuilder = new Commands.Builder(); for (int i = 0; i < commands1.size(); i++) { if (commands2.contains(commands1.get(i))) { diff --git a/libraries/session/src/main/java/androidx/media3/session/PlayerInfo.java b/libraries/session/src/main/java/androidx/media3/session/PlayerInfo.java index dfb94e8a3d..f4bd254eed 100644 --- a/libraries/session/src/main/java/androidx/media3/session/PlayerInfo.java +++ b/libraries/session/src/main/java/androidx/media3/session/PlayerInfo.java @@ -799,11 +799,7 @@ import com.google.errorprone.annotations.CanIgnoreReturnValue; // Next field key = 31 public Bundle toBundle( - boolean excludeMediaItems, - boolean excludeMediaItemsMetadata, - boolean excludeCues, - boolean excludeTimeline, - boolean excludeTracks) { + Player.Commands availableCommands, boolean excludeTimeline, boolean excludeTracks) { Bundle bundle = new Bundle(); if (playerError != null) { bundle.putBundle(FIELD_PLAYBACK_ERROR, playerError.toBundle()); @@ -816,16 +812,16 @@ import com.google.errorprone.annotations.CanIgnoreReturnValue; bundle.putBundle(FIELD_PLAYBACK_PARAMETERS, playbackParameters.toBundle()); bundle.putInt(FIELD_REPEAT_MODE, repeatMode); bundle.putBoolean(FIELD_SHUFFLE_MODE_ENABLED, shuffleModeEnabled); - if (!excludeTimeline) { - bundle.putBundle(FIELD_TIMELINE, timeline.toBundle(excludeMediaItems)); + if (!excludeTimeline && availableCommands.contains(Player.COMMAND_GET_TIMELINE)) { + bundle.putBundle(FIELD_TIMELINE, timeline.toBundle()); } bundle.putBundle(FIELD_VIDEO_SIZE, videoSize.toBundle()); - if (!excludeMediaItemsMetadata) { + if (availableCommands.contains(Player.COMMAND_GET_MEDIA_ITEMS_METADATA)) { bundle.putBundle(FIELD_PLAYLIST_METADATA, playlistMetadata.toBundle()); } bundle.putFloat(FIELD_VOLUME, volume); bundle.putBundle(FIELD_AUDIO_ATTRIBUTES, audioAttributes.toBundle()); - if (!excludeCues) { + if (availableCommands.contains(Player.COMMAND_GET_TEXT)) { bundle.putBundle(FIELD_CUE_GROUP, cueGroup.toBundle()); } bundle.putBundle(FIELD_DEVICE_INFO, deviceInfo.toBundle()); @@ -836,13 +832,13 @@ import com.google.errorprone.annotations.CanIgnoreReturnValue; bundle.putInt(FIELD_PLAYBACK_STATE, playbackState); bundle.putBoolean(FIELD_IS_PLAYING, isPlaying); bundle.putBoolean(FIELD_IS_LOADING, isLoading); - bundle.putBundle( - FIELD_MEDIA_METADATA, - excludeMediaItems ? MediaMetadata.EMPTY.toBundle() : mediaMetadata.toBundle()); + if (availableCommands.contains(Player.COMMAND_GET_TIMELINE)) { + bundle.putBundle(FIELD_MEDIA_METADATA, mediaMetadata.toBundle()); + } bundle.putLong(FIELD_SEEK_BACK_INCREMENT_MS, seekBackIncrementMs); bundle.putLong(FIELD_SEEK_FORWARD_INCREMENT_MS, seekForwardIncrementMs); bundle.putLong(FIELD_MAX_SEEK_TO_PREVIOUS_POSITION_MS, maxSeekToPreviousPositionMs); - if (!excludeTracks) { + if (!excludeTracks && availableCommands.contains(Player.COMMAND_GET_TRACKS)) { bundle.putBundle(FIELD_CURRENT_TRACKS, currentTracks.toBundle()); } bundle.putBundle(FIELD_TRACK_SELECTION_PARAMETERS, trackSelectionParameters.toBundle()); @@ -853,9 +849,7 @@ import com.google.errorprone.annotations.CanIgnoreReturnValue; @Override public Bundle toBundle() { return toBundle( - /* excludeMediaItems= */ false, - /* excludeMediaItemsMetadata= */ false, - /* excludeCues= */ false, + /* availableCommands= */ new Player.Commands.Builder().addAllCommands().build(), /* excludeTimeline= */ false, /* excludeTracks= */ false); }