Skip to content

Commit

Permalink
Don't advertise commands that are not available
Browse files Browse the repository at this point in the history 
When calling `MediaController.getCommandButtonForMediaItem(MediaItem)`
command buttons with custom commands that are not available
shouldn't be advertised to the controller when connected to
a Media3 session.

In contrast, when connected to a legacy session, available commands
are not enforced when advertising commands. Similarly, when sending
a custom commands that is referenced by a command button for media
items, sending is permitted without the command being available.

This is required because available commands match to custom actions
in `PlaybackStateCompat` of the legacy session. Adding commands for
media items to custom action of the `PlaybackStateCompat` would
interfere with other use cases.

Issue: #1474
#cherrypick
PiperOrigin-RevId: 683717723
  • Loading branch information
@marcbaechinger @copybara-github
marcbaechinger authored and copybara-github committed Oct 8, 2024
1 parent 546d7da commit c4ff07e
Show file tree
Hide file tree
Showing 9 changed files with 185 additions and 74 deletions.
37 changes: 33 additions & 4 deletions libraries/session/src/main/java/androidx/media3/session/MediaBrowserImplLegacy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Objects;
import org.checkerframework.checker.initialization.qual.UnderInitialization;

/** Implementation of MediaBrowser with the {@link MediaBrowserCompat} for legacy support. */
Expand Down Expand Up @@ -93,8 +94,21 @@ public SessionCommands getAvailableSessionCommands() {
}

@Override
public ImmutableMap<String, CommandButton> getCommandButtonsForMediaItemsMap() {
return commandButtonsForMediaItems;
public ImmutableList<CommandButton> getCommandButtonsForMediaItem(MediaItem mediaItem) {
// Do not filter by available commands. When connected to a legacy session, the available
// session commands are read from the custom actions in PlaybackStateCompat (see
// LegacyConversion.convertToSessionCommands). Filtering by these commands would force a
// legacy session to put all commands for media items into the playback state as custom commands
// which would interfere with the custom commands set for media controls.
ImmutableList<String> supportedActions = mediaItem.mediaMetadata.supportedCommands;
ImmutableList.Builder<CommandButton> commandButtonsForMediaItem = new ImmutableList.Builder<>();
for (int i = 0; i < supportedActions.size(); i++) {
CommandButton commandButton = commandButtonsForMediaItems.get(supportedActions.get(i));
if (commandButton != null && commandButton.sessionCommand != null) {
commandButtonsForMediaItem.add(commandButton);
}
}
return commandButtonsForMediaItem.build();
}

@Override
Expand Down Expand Up @@ -307,7 +321,9 @@ public void onError(String query, @Nullable Bundle extrasSent) {
@Override
public ListenableFuture<SessionResult> sendCustomCommand(SessionCommand command, Bundle args) {
MediaBrowserCompat browserCompat = getBrowserCompat();
if (browserCompat != null && instance.isSessionCommandAvailable(command)) {
if (browserCompat != null
&& (instance.isSessionCommandAvailable(command)
|| isContainedInCommandButtonsForMediaItems(command))) {
SettableFuture<SessionResult> settable = SettableFuture.create();
browserCompat.sendCustomAction(
command.customAction,
Expand All @@ -330,7 +346,20 @@ public void onError(String action, @Nullable Bundle extras, @Nullable Bundle dat
});
return settable;
}
return super.sendCustomCommand(command, args);
return Futures.immediateFuture(new SessionResult(SessionResult.RESULT_ERROR_PERMISSION_DENIED));
}

// Using this method as a proxy whether an browser is allowed to send a custom action can be
// justified because a MediaBrowserCompat can declare the custom browse actions in onGetRoot()
// specifically for each browser that connects. This is different to Media3 where the command
// buttons for media items are declared on the session level, and are constraint by the available
// session commands granted individually to a controller/browser in onConnect.
private boolean isContainedInCommandButtonsForMediaItems(SessionCommand command) {
if (command.commandCode != SessionCommand.COMMAND_CODE_CUSTOM) {
return false;
}
CommandButton commandButton = commandButtonsForMediaItems.get(command.customAction);
return commandButton != null && Objects.equals(commandButton.sessionCommand, command);
}

private MediaBrowserCompat getBrowserCompat(LibraryParams extras) {
Expand Down
14 changes: 2 additions & 12 deletions libraries/session/src/main/java/androidx/media3/session/MediaController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
import androidx.media3.datasource.DataSourceBitmapLoader;
import androidx.media3.session.legacy.MediaBrowserCompat;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
Expand Down Expand Up @@ -674,16 +673,7 @@ public final boolean isConnected() {
*/
@UnstableApi
public final ImmutableList<CommandButton> getCommandButtonsForMediaItem(MediaItem mediaItem) {
ImmutableMap<String, CommandButton> buttonMap = impl.getCommandButtonsForMediaItemsMap();
ImmutableList<String> supportedActions = mediaItem.mediaMetadata.supportedCommands;
ImmutableList.Builder<CommandButton> commandButtonsForMediaItem = new ImmutableList.Builder<>();
for (int i = 0; i < supportedActions.size(); i++) {
CommandButton commandButton = buttonMap.get(supportedActions.get(i));
if (commandButton != null) {
commandButtonsForMediaItem.add(commandButton);
}
}
return commandButtonsForMediaItem.build();
return impl.getCommandButtonsForMediaItem(mediaItem);
}

@Override
Expand Down Expand Up @@ -2174,7 +2164,7 @@ private void verifyApplicationThread() {

ImmutableList<CommandButton> getCustomLayout();

ImmutableMap<String, CommandButton> getCommandButtonsForMediaItemsMap();
ImmutableList<CommandButton> getCommandButtonsForMediaItem(MediaItem mediaItem);

Bundle getSessionExtras();

Expand Down
15 changes: 13 additions & 2 deletions libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -745,8 +745,19 @@ public ImmutableList<CommandButton> getCustomLayout() {
}

@Override
public ImmutableMap<String, CommandButton> getCommandButtonsForMediaItemsMap() {
return commandButtonsForMediaItemsMap;
public ImmutableList<CommandButton> getCommandButtonsForMediaItem(MediaItem mediaItem) {
ImmutableList<String> supportedActions = mediaItem.mediaMetadata.supportedCommands;
SessionCommands availableSessionCommands = getAvailableSessionCommands();
ImmutableList.Builder<CommandButton> commandButtonsForMediaItem = new ImmutableList.Builder<>();
for (int i = 0; i < supportedActions.size(); i++) {
CommandButton commandButton = commandButtonsForMediaItemsMap.get(supportedActions.get(i));
if (commandButton != null
&& commandButton.sessionCommand != null
&& availableSessionCommands.contains(commandButton.sessionCommand)) {
commandButtonsForMediaItem.add(commandButton);
}
}
return commandButtonsForMediaItem.build();
}

@Override
Expand Down
14 changes: 8 additions & 6 deletions libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@
import androidx.media3.session.legacy.RatingCompat;
import androidx.media3.session.legacy.VolumeProviderCompat;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.SettableFuture;
Expand All @@ -102,6 +101,7 @@
private final ListenerSet<Listener> listeners;
private final ControllerCompatCallback controllerCompatCallback;
private final BitmapLoader bitmapLoader;
private final ImmutableList<CommandButton> commandButtonsForMediaItems;

@Nullable private MediaControllerCompat controllerCompat;
@Nullable private MediaBrowserCompat browserCompat;
Expand Down Expand Up @@ -137,6 +137,8 @@ public MediaControllerImplLegacy(
this.bitmapLoader = bitmapLoader;
currentPositionMs = C.TIME_UNSET;
lastSetPlayWhenReadyCalledTimeMs = C.TIME_UNSET;
// Always empty. Only supported for a MediaBrowser connected to a MediaBrowserServiceCompat.
commandButtonsForMediaItems = ImmutableList.of();
}

/* package */ MediaController getInstance() {
Expand Down Expand Up @@ -410,6 +412,11 @@ public void seekForward() {
controllerCompat.getTransportControls().fastForward();
}

@Override
public ImmutableList<CommandButton> getCommandButtonsForMediaItem(MediaItem mediaItem) {
return commandButtonsForMediaItems;
}

@Override
@Nullable
public PendingIntent getSessionActivity() {
Expand All @@ -421,11 +428,6 @@ public ImmutableList<CommandButton> getCustomLayout() {
return controllerInfo.customLayout;
}

@Override
public ImmutableMap<String, CommandButton> getCommandButtonsForMediaItemsMap() {
return ImmutableMap.of();
}

@Override
public Bundle getSessionExtras() {
return controllerInfo.sessionExtras;
Expand Down
2 changes: 2 additions & 0 deletions ...ssion_common/src/main/java/androidx/media3/test/session/common/MediaSessionConstants.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public class MediaSessionConstants {
public static final String TEST_GET_CUSTOM_LAYOUT = "testGetCustomLayout";
public static final String TEST_GET_COMMAND_BUTTONS_FOR_MEDIA_ITEMS =
"testGetCommandButtonsForMediaItems";
public static final String TEST_GET_COMMAND_BUTTONS_FOR_MEDIA_ITEMS_COMMANDS_NOT_AVAILABLE =
"testGetCommandButtonsForMediaItemsCommandsNotAvailable";
public static final String TEST_WITH_CUSTOM_COMMANDS = "testWithCustomCommands";
public static final String TEST_CONTROLLER_LISTENER_SESSION_REJECTS = "connection_sessionRejects";
public static final String TEST_IS_SESSION_COMMAND_AVAILABLE = "testIsSessionCommandAvailable";
Expand Down
49 changes: 47 additions & 2 deletions ...t/java/androidx/media3/session/MediaBrowserListenerWithMediaBrowserServiceCompatTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,20 @@ public void getItem_supportedCommandActions_convertedCorrectly() throws Exceptio
@Test
public void sendCustomCommandWithMediaItem_mediaItemIdConvertedCorrectly() throws Exception {
remoteService.setProxyForTest(TEST_MEDIA_ITEMS_WITH_BROWSE_ACTIONS);
MediaBrowser mediaBrowser = createBrowser(/* listener= */ null);
MediaBrowser mediaBrowser =
createBrowser(
/* connectionHints= */ Bundle.EMPTY,
/* maxCommandsForMediaItems= */ 2,
/* listener= */ null);
MediaItem mediaItem = new MediaItem.Builder().setMediaId("mediaIdFromCommand").build();
// When connected to a legacy browser service, the library root needs to be requested
// before media item commands are available.
LibraryResult<MediaItem> libraryRootResult =
threadTestRule
.getHandler()
.postAndSync(() -> mediaBrowser.getLibraryRoot(new LibraryParams.Builder().build()))
.get(TIMEOUT_MS, MILLISECONDS);
assertThat(libraryRootResult.resultCode).isEqualTo(RESULT_SUCCESS);

SessionResult sessionResult =
threadTestRule
Expand All @@ -232,12 +244,45 @@ public void sendCustomCommandWithMediaItem_mediaItemIdConvertedCorrectly() throw
new SessionCommand(MediaBrowserConstants.COMMAND_RADIO, Bundle.EMPTY),
mediaItem,
/* args= */ Bundle.EMPTY))
.get();
.get(TIMEOUT_MS, MILLISECONDS);

assertThat(sessionResult.extras.getString(MediaConstants.EXTRA_KEY_MEDIA_ID))
.isEqualTo("mediaIdFromCommand");
}

@Test
public void sendCustomCommandWithMediaItem_commandButtonNotAvailable_permissionDenied()
throws Exception {
remoteService.setProxyForTest(TEST_MEDIA_ITEMS_WITH_BROWSE_ACTIONS);
MediaBrowser mediaBrowser =
createBrowser(
/* connectionHints= */ Bundle.EMPTY,
/* maxCommandsForMediaItems= */ 0,
/* listener= */ null);
MediaItem mediaItem = new MediaItem.Builder().setMediaId("mediaIdFromCommand").build();
// When connected to a legacy browser service, the library root needs to be requested
// before media item commands are available.
LibraryResult<MediaItem> libraryRootResult =
threadTestRule
.getHandler()
.postAndSync(() -> mediaBrowser.getLibraryRoot(new LibraryParams.Builder().build()))
.get(TIMEOUT_MS, MILLISECONDS);
assertThat(libraryRootResult.resultCode).isEqualTo(RESULT_SUCCESS);

SessionResult sessionResult =
threadTestRule
.getHandler()
.postAndSync(
() ->
mediaBrowser.sendCustomCommand(
new SessionCommand(MediaBrowserConstants.COMMAND_RADIO, Bundle.EMPTY),
mediaItem,
/* args= */ Bundle.EMPTY))
.get(TIMEOUT_MS, MILLISECONDS);

assertThat(sessionResult.resultCode).isEqualTo(SessionResult.RESULT_ERROR_PERMISSION_DENIED);
}

@Test
public void onChildrenChanged_subscribeAndUnsubscribe() throws Exception {
String testParentId = "testOnChildrenChanged";
Expand Down
Loading

0 comments on commit c4ff07e

Please sign in to comment.