The {@link #release()} method must only be called if this command is {@linkplain + * #isCommandAvailable(int) available}. + */ + int COMMAND_RELEASE = 32; /** Represents an invalid {@link Command}. */ int COMMAND_INVALID = -1; @@ -2526,8 +2536,10 @@ public interface Player { /** * Releases the player. This method must be called when the player is no longer required. The * player must not be used after calling this method. + * + *
This method must only be called if {@link #COMMAND_RELEASE} is {@linkplain + * #getAvailableCommands() available}. */ - // TODO(b/261158047): Document that COMMAND_RELEASE must be available once it exists. void release(); /** diff --git a/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java b/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java index 842f63912b..9f9713bfba 100644 --- a/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java +++ b/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java @@ -2338,7 +2338,7 @@ public abstract class SimpleBasePlayer extends BasePlayer { verifyApplicationThreadAndInitState(); // Use a local copy to ensure the lambda below uses the current state value. State state = this.state; - if (released) { // TODO(b/261158047): Replace by !shouldHandleCommand(Player.COMMAND_RELEASE) + if (!shouldHandleCommand(Player.COMMAND_RELEASE)) { return; } updateStateForPendingOperation( @@ -2837,10 +2837,11 @@ public abstract class SimpleBasePlayer extends BasePlayer { /** * Handles calls to {@link Player#release}. * + *
Will only be called if {@link Player#COMMAND_RELEASE} is available. + * * @return A {@link ListenableFuture} indicating the completion of all immediate {@link State} * changes caused by this call. */ - // TODO(b/261158047): Add that this method will only be called if COMMAND_RELEASE is available. @ForOverride protected ListenableFuture handleRelease() { throw new IllegalStateException("Missing implementation to handle COMMAND_RELEASE"); diff --git a/libraries/common/src/test/java/androidx/media3/common/SimpleBasePlayerTest.java b/libraries/common/src/test/java/androidx/media3/common/SimpleBasePlayerTest.java index a78d2a9c58..add4bca7bf 100644 --- a/libraries/common/src/test/java/androidx/media3/common/SimpleBasePlayerTest.java +++ b/libraries/common/src/test/java/androidx/media3/common/SimpleBasePlayerTest.java @@ -54,7 +54,6 @@ import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; -import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.shadows.ShadowLooper; @@ -2473,14 +2472,12 @@ public class SimpleBasePlayerTest { verifyNoMoreInteractions(listener); } - @Ignore("b/261158047: Ignore test while Player.COMMAND_RELEASE doesn't exist.") @Test public void release_withoutAvailableCommand_isNotForwarded() { State state = new State.Builder() - // TODO(b/261158047): Uncomment once test is no longer ignored. - // .setAvailableCommands( - // new Commands.Builder().addAllCommands().remove(Player.COMMAND_RELEASE).build()) + .setAvailableCommands( + new Commands.Builder().addAllCommands().remove(Player.COMMAND_RELEASE).build()) .build(); AtomicBoolean callForwarded = new AtomicBoolean(); SimpleBasePlayer player = diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java index df6b4b12d0..86bb683261 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java @@ -311,7 +311,8 @@ import java.util.concurrent.TimeoutException; COMMAND_SET_DEVICE_VOLUME, COMMAND_ADJUST_DEVICE_VOLUME, COMMAND_SET_VIDEO_SURFACE, - COMMAND_GET_TEXT) + COMMAND_GET_TEXT, + COMMAND_RELEASE) .addIf( COMMAND_SET_TRACK_SELECTION_PARAMETERS, trackSelector.isSetParametersSupported()) .build(); diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java index e6568f8795..0403daf231 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java @@ -27,6 +27,7 @@ import static androidx.media3.common.Player.COMMAND_GET_TRACKS; import static androidx.media3.common.Player.COMMAND_GET_VOLUME; import static androidx.media3.common.Player.COMMAND_PLAY_PAUSE; import static androidx.media3.common.Player.COMMAND_PREPARE; +import static androidx.media3.common.Player.COMMAND_RELEASE; import static androidx.media3.common.Player.COMMAND_SEEK_BACK; import static androidx.media3.common.Player.COMMAND_SEEK_FORWARD; import static androidx.media3.common.Player.COMMAND_SEEK_IN_CURRENT_MEDIA_ITEM; @@ -9151,6 +9152,7 @@ public final class ExoPlayerTest { assertThat(player.isCommandAvailable(COMMAND_GET_TEXT)).isTrue(); assertThat(player.isCommandAvailable(COMMAND_SET_TRACK_SELECTION_PARAMETERS)).isTrue(); assertThat(player.isCommandAvailable(COMMAND_GET_TRACKS)).isTrue(); + assertThat(player.isCommandAvailable(COMMAND_RELEASE)).isTrue(); } @Test @@ -12502,7 +12504,8 @@ public final class ExoPlayerTest { COMMAND_SET_VIDEO_SURFACE, COMMAND_GET_TEXT, COMMAND_SET_TRACK_SELECTION_PARAMETERS, - COMMAND_GET_TRACKS); + COMMAND_GET_TRACKS, + COMMAND_RELEASE); if (!isTimelineEmpty) { builder.add(COMMAND_SEEK_TO_PREVIOUS); } diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java index 8f224cb1ee..25f45b6ba2 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java @@ -22,7 +22,6 @@ import static androidx.media3.common.util.Assertions.checkState; import static androidx.media3.common.util.Assertions.checkStateNotNull; import static androidx.media3.common.util.Util.usToMs; import static androidx.media3.session.MediaUtils.calculateBufferedPercentage; -import static androidx.media3.session.MediaUtils.intersect; import static androidx.media3.session.MediaUtils.mergePlayerInfo; import static java.lang.Math.max; import static java.lang.Math.min; @@ -146,7 +145,8 @@ import org.checkerframework.checker.nullness.qual.NonNull; sessionCommands = SessionCommands.EMPTY; playerCommandsFromSession = Commands.EMPTY; playerCommandsFromPlayer = Commands.EMPTY; - intersectedPlayerCommands = Commands.EMPTY; + intersectedPlayerCommands = + createIntersectedCommands(playerCommandsFromSession, playerCommandsFromPlayer); listeners = new ListenerSet<>( applicationLooper, @@ -250,6 +250,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; @Override public void release() { + // No check for COMMAND_RELEASE needed as MediaControllers can always be released. @Nullable IMediaSession iSession = this.iSession; if (released) { return; @@ -2141,7 +2142,8 @@ import org.checkerframework.checker.nullness.qual.NonNull; sessionCommands = result.sessionCommands; playerCommandsFromSession = result.playerCommandsFromSession; playerCommandsFromPlayer = result.playerCommandsFromPlayer; - intersectedPlayerCommands = intersect(playerCommandsFromSession, playerCommandsFromPlayer); + intersectedPlayerCommands = + createIntersectedCommands(playerCommandsFromSession, playerCommandsFromPlayer); playerInfo = result.playerInfo; try { // Implementation for the local binder is no-op, @@ -2420,7 +2422,8 @@ import org.checkerframework.checker.nullness.qual.NonNull; if (playerCommandsChanged) { playerCommandsFromSession = playerCommands; Commands prevIntersectedPlayerCommands = intersectedPlayerCommands; - intersectedPlayerCommands = intersect(playerCommandsFromSession, playerCommandsFromPlayer); + intersectedPlayerCommands = + createIntersectedCommands(playerCommandsFromSession, playerCommandsFromPlayer); intersectedPlayerCommandsChanged = !Util.areEqual(intersectedPlayerCommands, prevIntersectedPlayerCommands); } @@ -2449,7 +2452,8 @@ import org.checkerframework.checker.nullness.qual.NonNull; } playerCommandsFromPlayer = commandsFromPlayer; Commands prevIntersectedPlayerCommands = intersectedPlayerCommands; - intersectedPlayerCommands = intersect(playerCommandsFromSession, playerCommandsFromPlayer); + intersectedPlayerCommands = + createIntersectedCommands(playerCommandsFromSession, playerCommandsFromPlayer); boolean intersectedPlayerCommandsChanged = !Util.areEqual(intersectedPlayerCommands, prevIntersectedPlayerCommands); if (intersectedPlayerCommandsChanged) { @@ -2829,6 +2833,19 @@ import org.checkerframework.checker.nullness.qual.NonNull; return newMediaItemIndex; } + private static Commands createIntersectedCommands( + Commands commandFromSession, Commands commandsFromPlayer) { + Commands.Builder intersectCommandsBuilder = new Commands.Builder(); + // Release is always available as it just releases the connection, not the underlying player. + intersectCommandsBuilder.add(Player.COMMAND_RELEASE); + for (int i = 0; i < commandFromSession.size(); i++) { + if (commandsFromPlayer.contains(commandFromSession.get(i))) { + intersectCommandsBuilder.add(commandFromSession.get(i)); + } + } + return intersectCommandsBuilder.build(); + } + // This will be called on the main thread. private class SessionServiceConnection implements ServiceConnection { 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 13cf696db0..3f174dc28f 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionLegacyStub.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionLegacyStub.java @@ -174,6 +174,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; } public void release() { + // No check for COMMAND_RELEASE needed as MediaControllers can always be released. sessionCompat.release(); } 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 95f3e3bbf5..f9c814033c 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java @@ -26,6 +26,7 @@ import static androidx.media3.common.Player.COMMAND_GET_MEDIA_ITEMS_METADATA; import static androidx.media3.common.Player.COMMAND_GET_TIMELINE; import static androidx.media3.common.Player.COMMAND_PLAY_PAUSE; import static androidx.media3.common.Player.COMMAND_PREPARE; +import static androidx.media3.common.Player.COMMAND_RELEASE; import static androidx.media3.common.Player.COMMAND_SEEK_BACK; import static androidx.media3.common.Player.COMMAND_SEEK_FORWARD; import static androidx.media3.common.Player.COMMAND_SEEK_IN_CURRENT_MEDIA_ITEM; @@ -1143,7 +1144,8 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; COMMAND_GET_TIMELINE, COMMAND_GET_MEDIA_ITEMS_METADATA, COMMAND_GET_CURRENT_MEDIA_ITEM, - COMMAND_GET_AUDIO_ATTRIBUTES); + COMMAND_GET_AUDIO_ATTRIBUTES, + COMMAND_RELEASE); if ((sessionFlags & FLAG_HANDLES_QUEUE_COMMANDS) != 0) { playerCommandsBuilder.add(COMMAND_CHANGE_MEDIA_ITEMS); if (hasAction(actions, PlaybackStateCompat.ACTION_SKIP_TO_QUEUE_ITEM)) { diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java index 0beb95bef4..9b77490203 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java @@ -2207,9 +2207,13 @@ public class MediaControllerListenerTest { Commands commandsWithSetRepeat = createPlayerCommandsWith(Player.COMMAND_SET_REPEAT_MODE); remoteSession.getMockPlayer().notifyAvailableCommandsChanged(commandsWithSetRepeat); + Commands expectedCommands = + new Commands.Builder() + .addAll(Player.COMMAND_SET_REPEAT_MODE, Player.COMMAND_RELEASE) + .build(); assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue(); - assertThat(availableCommandsFromParamRef.get()).isEqualTo(commandsWithSetRepeat); - assertThat(availableCommandsFromGetterRef.get()).isEqualTo(commandsWithSetRepeat); + assertThat(availableCommandsFromParamRef.get()).isEqualTo(expectedCommands); + assertThat(availableCommandsFromGetterRef.get()).isEqualTo(expectedCommands); assertThat(getEventsAsList(eventsRef.get())) .containsExactly(Player.EVENT_AVAILABLE_COMMANDS_CHANGED); } @@ -2342,10 +2346,14 @@ public class MediaControllerListenerTest { Commands commandsWithSetRepeat = createPlayerCommandsWith(Player.COMMAND_SET_REPEAT_MODE); remoteSession.setAvailableCommands(SessionCommands.EMPTY, commandsWithSetRepeat); + Commands expectedCommands = + new Commands.Builder() + .addAll(Player.COMMAND_SET_REPEAT_MODE, Player.COMMAND_RELEASE) + .build(); assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue(); - assertThat(availableCommandsFromParamRef.get()).isEqualTo(commandsWithSetRepeat); - assertThat(availableCommandsFromGetterRef.get()).isEqualTo(commandsWithSetRepeat); - assertThat(availableCommandsFromOnEventsRef.get()).isEqualTo(commandsWithSetRepeat); + assertThat(availableCommandsFromParamRef.get()).isEqualTo(expectedCommands); + assertThat(availableCommandsFromGetterRef.get()).isEqualTo(expectedCommands); + assertThat(availableCommandsFromOnEventsRef.get()).isEqualTo(expectedCommands); assertThat(getEventsAsList(eventsRef.get())) .containsExactly(Player.EVENT_AVAILABLE_COMMANDS_CHANGED); } diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaUtilsTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaUtilsTest.java index b2235e4750..e949c5669c 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaUtilsTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaUtilsTest.java @@ -498,7 +498,8 @@ public final class MediaUtilsTest { Player.COMMAND_GET_CURRENT_MEDIA_ITEM, Player.COMMAND_GET_DEVICE_VOLUME, Player.COMMAND_GET_MEDIA_ITEMS_METADATA, - Player.COMMAND_GET_AUDIO_ATTRIBUTES); + Player.COMMAND_GET_AUDIO_ATTRIBUTES, + Player.COMMAND_RELEASE); } @Test