From f53e1bc6f63caba7774c35aeb663b9178941faf5 Mon Sep 17 00:00:00 2001 From: bachinger Date: Mon, 16 Oct 2023 08:11:31 -0700 Subject: [PATCH] Only set the queue when COMMAND_GET_TIMELINE is available Android Auto shows a queue button when the queue is not empty. Apps were able to remove this queue button with the legacy API by not setting the queue of the session. After this change, removing `COMMAND_GET_TIMELINE` from the commands of the media notification controller or the session player sets the queue in the platform session to null. #minor-release Issue: androidx/media#339 PiperOrigin-RevId: 573813558 --- RELEASENOTES.md | 6 ++ .../media3/session/MediaSessionImpl.java | 23 ++-- .../session/MediaSessionLegacyStub.java | 100 ++++++++---------- .../common/IRemoteMediaControllerCompat.aidl | 1 + ...aSessionWithMediaControllerCompatTest.java | 61 +++++++++++ .../MediaControllerCompatProviderService.java | 9 ++ .../session/RemoteMediaControllerCompat.java | 6 ++ 7 files changed, 147 insertions(+), 59 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index d6c3925676..b249fdf517 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -24,6 +24,12 @@ * Muxers: * IMA extension: * Session: + * Do not set the queue of the framework session when + `COMMAND_GET_TIMELINE` is not available for the media notification + controller. With Android Auto as the client controller reading from the + framework session, this has the effect that the `queue` button in the UI + of Android Auto is not displayed + (([#339](https://github.com/androidx/media/issues/339)). * UI: * Downloads: * OkHttp Extension: 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 4b43243596..29efaaa2ab 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java @@ -476,7 +476,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; ControllerInfo controller, ImmutableList customLayout) { if (isMediaNotificationController(controller)) { playerWrapper.setCustomLayout(customLayout); - sessionLegacyStub.updateLegacySessionPlaybackStateCompat(); + sessionLegacyStub.updateLegacySessionPlaybackState(playerWrapper); } return dispatchRemoteControllerTask( controller, (controller1, seq) -> controller1.setCustomLayout(seq, customLayout)); @@ -519,8 +519,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; ControllerInfo controller, SessionCommands sessionCommands, Player.Commands playerCommands) { if (sessionStub.getConnectedControllersManager().isConnected(controller)) { if (isMediaNotificationController(controller)) { - playerWrapper.setAvailableCommands(sessionCommands, playerCommands); - sessionLegacyStub.updateLegacySessionPlaybackStateCompat(); + setAvailableFrameworkControllerCommands(sessionCommands, playerCommands); ControllerInfo systemUiControllerInfo = getSystemUiControllerInfo(); if (systemUiControllerInfo != null) { // Set the available commands of the proxy controller to the ConnectedControllerRecord of @@ -618,13 +617,12 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; "Callback.onConnect must return non-null future"); if (isMediaNotificationController(controller)) { isMediaNotificationControllerConnected = true; - playerWrapper.setAvailableCommands( - connectionResult.availableSessionCommands, connectionResult.availablePlayerCommands); playerWrapper.setCustomLayout( connectionResult.customLayout != null ? connectionResult.customLayout : instance.getCustomLayout()); - sessionLegacyStub.updateLegacySessionPlaybackStateCompat(); + setAvailableFrameworkControllerCommands( + connectionResult.availableSessionCommands, connectionResult.availablePlayerCommands); } return connectionResult; } @@ -896,6 +894,19 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; executor); } + private void setAvailableFrameworkControllerCommands( + SessionCommands sessionCommands, Player.Commands playerCommands) { + boolean commandGetTimelineChanged = + playerWrapper.getAvailablePlayerCommands().contains(Player.COMMAND_GET_TIMELINE) + != playerCommands.contains(Player.COMMAND_GET_TIMELINE); + playerWrapper.setAvailableCommands(sessionCommands, playerCommands); + if (commandGetTimelineChanged) { + sessionLegacyStub.updateLegacySessionPlaybackStateAndQueue(playerWrapper); + } else { + sessionLegacyStub.updateLegacySessionPlaybackState(playerWrapper); + } + } + private void dispatchRemoteControllerTaskToLegacyStub(RemoteControllerTask task) { try { task.run(sessionLegacyStub.getControllerLegacyCbForBroadcast(), /* seq= */ 0); diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaSessionLegacyStub.java b/libraries/session/src/main/java/androidx/media3/session/MediaSessionLegacyStub.java index 26875d9e1a..2e8b6ef4af 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionLegacyStub.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionLegacyStub.java @@ -124,7 +124,7 @@ import org.checkerframework.checker.initialization.qual.Initialized; private final MediaSessionImpl sessionImpl; private final MediaSessionManager sessionManager; - private final ControllerCb controllerLegacyCbForBroadcast; + private final ControllerLegacyCbForBroadcast controllerLegacyCbForBroadcast; private final ConnectionTimeoutHandler connectionTimeoutHandler; private final MediaPlayPauseKeyHandler mediaPlayPauseKeyHandler; private final MediaSessionCompat sessionCompat; @@ -832,13 +832,22 @@ import org.checkerframework.checker.initialization.qual.Initialized; connectionTimeoutMs = timeoutMs; } - public void updateLegacySessionPlaybackStateCompat() { + public void updateLegacySessionPlaybackState(PlayerWrapper playerWrapper) { postOrRun( sessionImpl.getApplicationHandler(), - () -> - sessionImpl - .getSessionCompat() - .setPlaybackState(sessionImpl.getPlayerWrapper().createPlaybackStateCompat())); + () -> sessionCompat.setPlaybackState(playerWrapper.createPlaybackStateCompat())); + } + + public void updateLegacySessionPlaybackStateAndQueue(PlayerWrapper playerWrapper) { + postOrRun( + sessionImpl.getApplicationHandler(), + () -> { + sessionCompat.setPlaybackState(playerWrapper.createPlaybackStateCompat()); + controllerLegacyCbForBroadcast.updateQueue( + playerWrapper.getAvailableCommands().contains(Player.COMMAND_GET_TIMELINE) + ? playerWrapper.getCurrentTimeline() + : Timeline.EMPTY); + }); } private void handleMediaRequest(MediaItem mediaItem, boolean play) { @@ -967,9 +976,14 @@ import org.checkerframework.checker.initialization.qual.Initialized; } @SuppressWarnings("nullness:argument") // MediaSessionCompat didn't annotate @Nullable. - private static void setQueueTitle( - MediaSessionCompat sessionCompat, @Nullable CharSequence title) { - sessionCompat.setQueueTitle(title); + private void setQueueTitle(MediaSessionCompat sessionCompat, @Nullable CharSequence title) { + sessionCompat.setQueueTitle(isQueueEnabled() ? title : null); + } + + private boolean isQueueEnabled() { + PlayerWrapper playerWrapper = sessionImpl.getPlayerWrapper(); + return playerWrapper.getAvailablePlayerCommands().contains(Player.COMMAND_GET_TIMELINE) + && playerWrapper.getAvailableCommands().contains(Player.COMMAND_GET_TIMELINE); } private static MediaItem createMediaItemForMediaRequest( @@ -1037,7 +1051,7 @@ import org.checkerframework.checker.initialization.qual.Initialized; public void onAvailableCommandsChangedFromPlayer(int seq, Player.Commands availableCommands) { PlayerWrapper playerWrapper = sessionImpl.getPlayerWrapper(); maybeUpdateFlags(playerWrapper); - sessionImpl.getSessionCompat().setPlaybackState(playerWrapper.createPlaybackStateCompat()); + updateLegacySessionPlaybackState(playerWrapper); } @Override @@ -1093,28 +1107,28 @@ import org.checkerframework.checker.initialization.qual.Initialized; // If PlaybackStateCompat isn't updated by above if-statement, forcefully update // PlaybackStateCompat to tell the latest position and its event // time. This would also update playback speed, buffering state, player state, and error. - sessionCompat.setPlaybackState(newPlayerWrapper.createPlaybackStateCompat()); + updateLegacySessionPlaybackState(newPlayerWrapper); } } @Override public void onPlayerError(int seq, @Nullable PlaybackException playerError) { - updateLegacySessionPlaybackStateCompat(); + updateLegacySessionPlaybackState(sessionImpl.getPlayerWrapper()); } @Override public void setCustomLayout(int seq, List layout) { - updateLegacySessionPlaybackStateCompat(); + updateLegacySessionPlaybackState(sessionImpl.getPlayerWrapper()); } @Override public void onSessionExtrasChanged(int seq, Bundle sessionExtras) { - sessionImpl.getSessionCompat().setExtras(sessionExtras); + sessionCompat.setExtras(sessionExtras); } @Override public void sendCustomCommand(int seq, SessionCommand command, Bundle args) { - sessionImpl.getSessionCompat().sendSessionEvent(command.customAction, args); + sessionCompat.sendSessionEvent(command.customAction, args); } @Override @@ -1122,17 +1136,13 @@ import org.checkerframework.checker.initialization.qual.Initialized; int seq, boolean playWhenReady, @Player.PlaybackSuppressionReason int reason) throws RemoteException { // Note: This method does not use any of the given arguments. - sessionImpl - .getSessionCompat() - .setPlaybackState(sessionImpl.getPlayerWrapper().createPlaybackStateCompat()); + updateLegacySessionPlaybackState(sessionImpl.getPlayerWrapper()); } @Override public void onPlaybackSuppressionReasonChanged( int seq, @Player.PlaybackSuppressionReason int reason) throws RemoteException { - sessionImpl - .getSessionCompat() - .setPlaybackState(sessionImpl.getPlayerWrapper().createPlaybackStateCompat()); + updateLegacySessionPlaybackState(sessionImpl.getPlayerWrapper()); } @Override @@ -1140,16 +1150,12 @@ import org.checkerframework.checker.initialization.qual.Initialized; int seq, @Player.State int state, @Nullable PlaybackException playerError) throws RemoteException { // Note: This method does not use any of the given arguments. - sessionImpl - .getSessionCompat() - .setPlaybackState(sessionImpl.getPlayerWrapper().createPlaybackStateCompat()); + updateLegacySessionPlaybackState(sessionImpl.getPlayerWrapper()); } @Override public void onIsPlayingChanged(int seq, boolean isPlaying) throws RemoteException { - sessionImpl - .getSessionCompat() - .setPlaybackState(sessionImpl.getPlayerWrapper().createPlaybackStateCompat()); + updateLegacySessionPlaybackState(sessionImpl.getPlayerWrapper()); } @Override @@ -1160,18 +1166,14 @@ import org.checkerframework.checker.initialization.qual.Initialized; @DiscontinuityReason int reason) throws RemoteException { // Note: This method does not use any of the given arguments. - sessionImpl - .getSessionCompat() - .setPlaybackState(sessionImpl.getPlayerWrapper().createPlaybackStateCompat()); + updateLegacySessionPlaybackState(sessionImpl.getPlayerWrapper()); } @Override public void onPlaybackParametersChanged(int seq, PlaybackParameters playbackParameters) throws RemoteException { // Note: This method does not use any of the given arguments. - sessionImpl - .getSessionCompat() - .setPlaybackState(sessionImpl.getPlayerWrapper().createPlaybackStateCompat()); + updateLegacySessionPlaybackState(sessionImpl.getPlayerWrapper()); } @Override @@ -1186,9 +1188,7 @@ import org.checkerframework.checker.initialization.qual.Initialized; sessionCompat.setRatingType( MediaUtils.getRatingCompatStyle(mediaItem.mediaMetadata.userRating)); } - sessionImpl - .getSessionCompat() - .setPlaybackState(sessionImpl.getPlayerWrapper().createPlaybackStateCompat()); + updateLegacySessionPlaybackState(sessionImpl.getPlayerWrapper()); } @Override @@ -1200,17 +1200,16 @@ import org.checkerframework.checker.initialization.qual.Initialized; public void onTimelineChanged( int seq, Timeline timeline, @Player.TimelineChangeReason int reason) throws RemoteException { - if (timeline.isEmpty()) { - setQueue(sessionCompat, null); - return; - } - updateQueue(timeline); // Duration might be unknown at onMediaItemTransition and become available afterward. updateMetadataIfChanged(); } private void updateQueue(Timeline timeline) { + if (!isQueueEnabled() || timeline.isEmpty()) { + setQueue(sessionCompat, /* queue= */ null); + return; + } List mediaItemList = MediaUtils.convertToMediaItemList(timeline); List<@NullableType ListenableFuture> bitmapFutures = new ArrayList<>(); final AtomicInteger resultCount = new AtomicInteger(0); @@ -1266,11 +1265,11 @@ import org.checkerframework.checker.initialization.qual.Initialized; TAG, "Sending " + truncatedList.size() + " items out of " + timeline.getWindowCount()); } - sessionCompat.setQueue(truncatedList); + setQueue(sessionCompat, truncatedList); } else { // Framework MediaSession#setQueue() uses ParceledListSlice, // which means we can safely send long lists. - sessionCompat.setQueue(queueItemList); + setQueue(sessionCompat, queueItemList); } } @@ -1288,16 +1287,13 @@ import org.checkerframework.checker.initialization.qual.Initialized; @Override public void onShuffleModeEnabledChanged(int seq, boolean shuffleModeEnabled) throws RemoteException { - sessionImpl - .getSessionCompat() - .setShuffleMode(MediaUtils.convertToPlaybackStateCompatShuffleMode(shuffleModeEnabled)); + sessionCompat.setShuffleMode( + MediaUtils.convertToPlaybackStateCompatShuffleMode(shuffleModeEnabled)); } @Override public void onRepeatModeChanged(int seq, @RepeatMode int repeatMode) throws RemoteException { - sessionImpl - .getSessionCompat() - .setRepeatMode(MediaUtils.convertToPlaybackStateCompatRepeatMode(repeatMode)); + sessionCompat.setRepeatMode(MediaUtils.convertToPlaybackStateCompatRepeatMode(repeatMode)); } @Override @@ -1337,9 +1333,7 @@ import org.checkerframework.checker.initialization.qual.Initialized; boolean unusedCanAccessCurrentMediaItem, boolean unusedCanAccessTimeline) throws RemoteException { - sessionImpl - .getSessionCompat() - .setPlaybackState(sessionImpl.getPlayerWrapper().createPlaybackStateCompat()); + updateLegacySessionPlaybackState(sessionImpl.getPlayerWrapper()); } private void updateMetadataIfChanged() { @@ -1512,7 +1506,7 @@ import org.checkerframework.checker.initialization.qual.Initialized; if (keyEvent == null) { return; } - getSessionCompat().getController().dispatchMediaButtonEvent(keyEvent); + sessionCompat.getController().dispatchMediaButtonEvent(keyEvent); } } diff --git a/libraries/test_session_common/src/main/aidl/androidx/media3/test/session/common/IRemoteMediaControllerCompat.aidl b/libraries/test_session_common/src/main/aidl/androidx/media3/test/session/common/IRemoteMediaControllerCompat.aidl index 051f887b63..beb34a396a 100644 --- a/libraries/test_session_common/src/main/aidl/androidx/media3/test/session/common/IRemoteMediaControllerCompat.aidl +++ b/libraries/test_session_common/src/main/aidl/androidx/media3/test/session/common/IRemoteMediaControllerCompat.aidl @@ -26,6 +26,7 @@ interface IRemoteMediaControllerCompat { void addQueueItem(String controllerId, in Bundle description); void addQueueItemWithIndex(String controllerId, in Bundle description, int index); void removeQueueItem(String controllerId, in Bundle description); + int getQueueSize(String controllerId); void setVolumeTo(String controllerId, int value, int flags); void adjustVolume(String controllerId, int direction, int flags); void sendCommand(String controllerId, String command, in Bundle params, in ResultReceiver cb); diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionWithMediaControllerCompatTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionWithMediaControllerCompatTest.java index d4035e0801..c880e5beb0 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionWithMediaControllerCompatTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionWithMediaControllerCompatTest.java @@ -15,17 +15,23 @@ */ package androidx.media3.session; +import static androidx.media3.session.MediaSession.ConnectionResult.DEFAULT_PLAYER_COMMANDS; +import static androidx.media3.session.RemoteMediaControllerCompat.QUEUE_IS_NULL; import static androidx.media3.test.session.common.TestUtils.TIMEOUT_MS; import static com.google.common.truth.Truth.assertThat; import static java.util.concurrent.TimeUnit.MILLISECONDS; import android.content.Context; +import android.os.Bundle; import android.support.v4.media.session.MediaControllerCompat; +import androidx.media3.common.MediaItem; +import androidx.media3.common.Player; 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 java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicInteger; import org.junit.Before; @@ -89,4 +95,59 @@ public class MediaSessionWithMediaControllerCompatTest { assertThat(connectedLatch.await(TIMEOUT_MS, MILLISECONDS)).isTrue(); assertThat(controllerVersionRef.get()).isLessThan(1_000_000); } + + @Test + public void notificationController_commandGetTimelineNotAvailable_queueIsNull() throws Exception { + Player.Commands playerCommands = + DEFAULT_PLAYER_COMMANDS.buildUpon().remove(Player.COMMAND_GET_TIMELINE).build(); + CountDownLatch connectedLatch = new CountDownLatch(1); + MediaSession.Callback callback = + new MediaSession.Callback() { + @Override + public MediaSession.ConnectionResult onConnect( + MediaSession session, MediaSession.ControllerInfo controller) { + if (session.isMediaNotificationController(controller)) { + return new MediaSession.ConnectionResult.AcceptedResultBuilder(session) + .setAvailablePlayerCommands(playerCommands) + .build(); + } + connectedLatch.countDown(); + return MediaSession.Callback.super.onConnect(session, controller); + } + }; + player.timeline = + new PlaylistTimeline( + ImmutableList.of( + new MediaItem.Builder().setMediaId("id1").setUri("https://example.com/1").build(), + new MediaItem.Builder().setMediaId("id2").setUri("https://example.com/2").build())); + MediaSession session = + sessionTestRule.ensureReleaseAfterTest( + new MediaSession.Builder(context, player).setId(TAG).setCallback(callback).build()); + Bundle connectionHints = new Bundle(); + connectionHints.putBoolean(MediaNotificationManager.KEY_MEDIA_NOTIFICATION_MANAGER, true); + new MediaController.Builder(context.getApplicationContext(), session.getToken()) + .setConnectionHints(connectionHints) + .buildAsync() + .get(); + + RemoteMediaControllerCompat controllerCompat = + remoteControllerTestRule.createRemoteControllerCompat( + session.getSessionCompat().getSessionToken()); + controllerCompat.transportControls.play(); + + assertThat(connectedLatch.await(TIMEOUT_MS, MILLISECONDS)).isTrue(); + assertThat(controllerCompat.getQueueSize()).isEqualTo(QUEUE_IS_NULL); + + session.setAvailableCommands( + session.getMediaNotificationControllerInfo(), + SessionCommands.EMPTY, + Player.Commands.EMPTY.buildUpon().add(Player.COMMAND_GET_TIMELINE).build()); + RemoteMediaControllerCompat controllerCompat2 = + remoteControllerTestRule.createRemoteControllerCompat( + session.getSessionCompat().getSessionToken()); + controllerCompat2.transportControls.pause(); + + assertThat(controllerCompat.getQueueSize()).isEqualTo(2); + assertThat(controllerCompat2.getQueueSize()).isEqualTo(2); + } } diff --git a/libraries/test_session_current/src/main/java/androidx/media3/session/MediaControllerCompatProviderService.java b/libraries/test_session_current/src/main/java/androidx/media3/session/MediaControllerCompatProviderService.java index cee8339186..b9e2385ffd 100644 --- a/libraries/test_session_current/src/main/java/androidx/media3/session/MediaControllerCompatProviderService.java +++ b/libraries/test_session_current/src/main/java/androidx/media3/session/MediaControllerCompatProviderService.java @@ -15,6 +15,7 @@ */ package androidx.media3.session; +import static androidx.media3.session.RemoteMediaControllerCompat.QUEUE_IS_NULL; import static androidx.media3.test.session.common.CommonConstants.ACTION_MEDIA_CONTROLLER_COMPAT; import static androidx.media3.test.session.common.CommonConstants.KEY_ARGUMENTS; import static androidx.media3.test.session.common.TestUtils.SERVICE_CONNECTION_TIMEOUT_MS; @@ -37,6 +38,7 @@ import androidx.media3.common.util.Log; import androidx.media3.test.session.common.IRemoteMediaControllerCompat; import androidx.media3.test.session.common.TestHandler; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executor; @@ -127,6 +129,13 @@ public class MediaControllerCompatProviderService extends Service { controller.removeQueueItem(desc); } + @Override + public int getQueueSize(String controllerId) throws RemoteException { + MediaControllerCompat controller = mediaControllerCompatMap.get(controllerId); + List queue = controller.getQueue(); + return queue != null ? controller.getQueue().size() : QUEUE_IS_NULL; + } + @Override public void setVolumeTo(String controllerId, int value, int flags) throws RemoteException { MediaControllerCompat controller = mediaControllerCompatMap.get(controllerId); diff --git a/libraries/test_session_current/src/main/java/androidx/media3/session/RemoteMediaControllerCompat.java b/libraries/test_session_current/src/main/java/androidx/media3/session/RemoteMediaControllerCompat.java index e44a35dd49..b4aa03c92b 100644 --- a/libraries/test_session_current/src/main/java/androidx/media3/session/RemoteMediaControllerCompat.java +++ b/libraries/test_session_current/src/main/java/androidx/media3/session/RemoteMediaControllerCompat.java @@ -49,6 +49,8 @@ import java.util.concurrent.CountDownLatch; *

Users can run {@link MediaControllerCompat} methods remotely with this object. */ public class RemoteMediaControllerCompat { + + public static final int QUEUE_IS_NULL = -1; static final String TAG = "RMediaControllerCompat"; final String controllerId; @@ -106,6 +108,10 @@ public class RemoteMediaControllerCompat { binder.removeQueueItem(controllerId, createBundleWithParcelable(description)); } + public int getQueueSize() throws RemoteException { + return binder.getQueueSize(controllerId); + } + public void setVolumeTo(int value, int flags) throws RemoteException { binder.setVolumeTo(controllerId, value, flags); }