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 893968b3b0..731dca5630 100644 --- a/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java +++ b/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java @@ -1937,6 +1937,7 @@ public abstract class SimpleBasePlayer extends BasePlayer { private final Timeline.Period period; private @MonotonicNonNull State state; + private boolean released; /** * Creates the base class. @@ -1999,7 +2000,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 (!state.availableCommands.contains(Player.COMMAND_PLAY_PAUSE)) { + if (!shouldHandleCommand(Player.COMMAND_PLAY_PAUSE)) { return; } updateStateForPendingOperation( @@ -2053,7 +2054,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 (!state.availableCommands.contains(Player.COMMAND_PREPARE)) { + if (!shouldHandleCommand(Player.COMMAND_PREPARE)) { return; } updateStateForPendingOperation( @@ -2091,7 +2092,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 (!state.availableCommands.contains(Player.COMMAND_SET_REPEAT_MODE)) { + if (!shouldHandleCommand(Player.COMMAND_SET_REPEAT_MODE)) { return; } updateStateForPendingOperation( @@ -2111,7 +2112,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 (!state.availableCommands.contains(Player.COMMAND_SET_SHUFFLE_MODE)) { + if (!shouldHandleCommand(Player.COMMAND_SET_SHUFFLE_MODE)) { return; } updateStateForPendingOperation( @@ -2167,7 +2168,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 (!state.availableCommands.contains(Player.COMMAND_SET_SPEED_AND_PITCH)) { + if (!shouldHandleCommand(Player.COMMAND_SET_SPEED_AND_PITCH)) { return; } updateStateForPendingOperation( @@ -2187,7 +2188,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 (!state.availableCommands.contains(Player.COMMAND_STOP)) { + if (!shouldHandleCommand(Player.COMMAND_STOP)) { return; } updateStateForPendingOperation( @@ -2212,8 +2213,25 @@ public abstract class SimpleBasePlayer extends BasePlayer { @Override public final void release() { - // TODO: implement. - throw new IllegalStateException(); + 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) + return; + } + updateStateForPendingOperation( + /* pendingOperation= */ handleRelease(), /* placeholderStateSupplier= */ () -> state); + released = true; + listeners.release(); + // Enforce some final state values in case getters are called after release. + this.state = + this.state + .buildUpon() + .setPlaybackState(Player.STATE_IDLE) + .setTotalBufferedDurationMs(PositionSupplier.ZERO) + .setContentBufferedPositionMs(state.contentPositionMsSupplier) + .setAdBufferedPositionMs(state.adPositionMsSupplier) + .build(); } @Override @@ -2233,7 +2251,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 (!state.availableCommands.contains(Player.COMMAND_SET_TRACK_SELECTION_PARAMETERS)) { + if (!shouldHandleCommand(Player.COMMAND_SET_TRACK_SELECTION_PARAMETERS)) { return; } updateStateForPendingOperation( @@ -2259,7 +2277,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 (!state.availableCommands.contains(Player.COMMAND_SET_MEDIA_ITEMS_METADATA)) { + if (!shouldHandleCommand(Player.COMMAND_SET_MEDIA_ITEMS_METADATA)) { return; } updateStateForPendingOperation( @@ -2360,7 +2378,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 (!state.availableCommands.contains(Player.COMMAND_SET_VOLUME)) { + if (!shouldHandleCommand(Player.COMMAND_SET_VOLUME)) { return; } updateStateForPendingOperation( @@ -2379,7 +2397,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 (!state.availableCommands.contains(Player.COMMAND_SET_VIDEO_SURFACE)) { + if (!shouldHandleCommand(Player.COMMAND_SET_VIDEO_SURFACE)) { return; } if (surface == null) { @@ -2397,7 +2415,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 (!state.availableCommands.contains(Player.COMMAND_SET_VIDEO_SURFACE)) { + if (!shouldHandleCommand(Player.COMMAND_SET_VIDEO_SURFACE)) { return; } if (surfaceHolder == null) { @@ -2415,7 +2433,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 (!state.availableCommands.contains(Player.COMMAND_SET_VIDEO_SURFACE)) { + if (!shouldHandleCommand(Player.COMMAND_SET_VIDEO_SURFACE)) { return; } if (surfaceView == null) { @@ -2436,7 +2454,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 (!state.availableCommands.contains(Player.COMMAND_SET_VIDEO_SURFACE)) { + if (!shouldHandleCommand(Player.COMMAND_SET_VIDEO_SURFACE)) { return; } if (textureView == null) { @@ -2484,7 +2502,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 (!state.availableCommands.contains(Player.COMMAND_SET_VIDEO_SURFACE)) { + if (!shouldHandleCommand(Player.COMMAND_SET_VIDEO_SURFACE)) { return; } updateStateForPendingOperation( @@ -2533,7 +2551,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 (!state.availableCommands.contains(Player.COMMAND_SET_DEVICE_VOLUME)) { + if (!shouldHandleCommand(Player.COMMAND_SET_DEVICE_VOLUME)) { return; } updateStateForPendingOperation( @@ -2546,7 +2564,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 (!state.availableCommands.contains(Player.COMMAND_ADJUST_DEVICE_VOLUME)) { + if (!shouldHandleCommand(Player.COMMAND_ADJUST_DEVICE_VOLUME)) { return; } updateStateForPendingOperation( @@ -2560,7 +2578,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 (!state.availableCommands.contains(Player.COMMAND_ADJUST_DEVICE_VOLUME)) { + if (!shouldHandleCommand(Player.COMMAND_ADJUST_DEVICE_VOLUME)) { return; } updateStateForPendingOperation( @@ -2574,7 +2592,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 (!state.availableCommands.contains(Player.COMMAND_ADJUST_DEVICE_VOLUME)) { + if (!shouldHandleCommand(Player.COMMAND_ADJUST_DEVICE_VOLUME)) { return; } updateStateForPendingOperation( @@ -2593,7 +2611,7 @@ public abstract class SimpleBasePlayer extends BasePlayer { */ protected final void invalidateState() { verifyApplicationThreadAndInitState(); - if (!pendingOperations.isEmpty()) { + if (!pendingOperations.isEmpty() || released) { return; } updateStateAndInformListeners(getState()); @@ -2672,6 +2690,18 @@ public abstract class SimpleBasePlayer extends BasePlayer { throw new IllegalStateException(); } + /** + * Handles calls to {@link Player#release}. + * + * @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(); + } + /** * Handles calls to {@link Player#setRepeatMode}. * @@ -2844,6 +2874,11 @@ public abstract class SimpleBasePlayer extends BasePlayer { throw new IllegalStateException(); } + @RequiresNonNull("state") + private boolean shouldHandleCommand(@Player.Command int commandCode) { + return !released && state.availableCommands.contains(commandCode); + } + @SuppressWarnings("deprecation") // Calling deprecated listener methods. @RequiresNonNull("state") private void updateStateAndInformListeners(State newState) { @@ -3088,7 +3123,7 @@ public abstract class SimpleBasePlayer extends BasePlayer { () -> { castNonNull(state); // Already checked by method @RequiresNonNull pre-condition. pendingOperations.remove(pendingOperation); - if (pendingOperations.isEmpty()) { + if (pendingOperations.isEmpty() && !released) { updateStateAndInformListeners(getState()); } }, 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 0ef67a9e7d..5c53e5e27b 100644 --- a/libraries/common/src/test/java/androidx/media3/common/SimpleBasePlayerTest.java +++ b/libraries/common/src/test/java/androidx/media3/common/SimpleBasePlayerTest.java @@ -22,6 +22,7 @@ import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyFloat; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -50,6 +51,7 @@ import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.shadows.ShadowLooper; @@ -2162,6 +2164,155 @@ public class SimpleBasePlayerTest { assertThat(callForwarded.get()).isFalse(); } + @Test + public void release_immediateHandling_updatesStateInformsListenersAndReturnsIdle() { + State state = + new State.Builder() + .setAvailableCommands(new Commands.Builder().addAllCommands().build()) + .setPlaybackState(Player.STATE_READY) + .setPlaylist( + ImmutableList.of( + new SimpleBasePlayer.MediaItemData.Builder(/* uid= */ new Object()).build())) + .build(); + State updatedState = state.buildUpon().setRepeatMode(Player.REPEAT_MODE_ALL).build(); + SimpleBasePlayer player = + new SimpleBasePlayer(Looper.myLooper()) { + private State playerState = state; + + @Override + protected State getState() { + return playerState; + } + + @Override + protected ListenableFuture handleRelease() { + playerState = updatedState; + return Futures.immediateVoidFuture(); + } + }; + Listener listener = mock(Listener.class); + player.addListener(listener); + + player.release(); + + assertThat(player.getPlaybackState()).isEqualTo(Player.STATE_IDLE); + assertThat(player.getRepeatMode()).isEqualTo(Player.REPEAT_MODE_ALL); + verify(listener).onRepeatModeChanged(Player.REPEAT_MODE_ALL); + verify(listener).onEvents(eq(player), any()); + verifyNoMoreInteractions(listener); + } + + @Test + public void release_asyncHandling_returnsIdleAndIgnoredAsyncStateUpdate() { + State state = + new State.Builder() + .setAvailableCommands(new Commands.Builder().addAllCommands().build()) + .setPlaybackState(Player.STATE_READY) + .setPlaylist( + ImmutableList.of( + new SimpleBasePlayer.MediaItemData.Builder(/* uid= */ new Object()).build())) + .build(); + // Additionally set the repeat mode to see a difference between the placeholder and new state. + State updatedState = state.buildUpon().setRepeatMode(Player.REPEAT_MODE_ALL).build(); + SettableFuture future = SettableFuture.create(); + SimpleBasePlayer player = + new SimpleBasePlayer(Looper.myLooper()) { + @Override + protected State getState() { + return future.isDone() ? updatedState : state; + } + + @Override + protected ListenableFuture handleRelease() { + return future; + } + }; + Listener listener = mock(Listener.class); + player.addListener(listener); + + player.release(); + + // Verify initial change to IDLE without listener call. + assertThat(player.getPlaybackState()).isEqualTo(Player.STATE_IDLE); + verifyNoMoreInteractions(listener); + + future.set(null); + + // Verify no further update happened. + assertThat(player.getRepeatMode()).isEqualTo(Player.REPEAT_MODE_OFF); + 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()) + .build(); + AtomicBoolean callForwarded = new AtomicBoolean(); + SimpleBasePlayer player = + new SimpleBasePlayer(Looper.myLooper()) { + @Override + protected State getState() { + return state; + } + + @Override + protected ListenableFuture handleRelease() { + callForwarded.set(true); + return Futures.immediateVoidFuture(); + } + }; + + player.release(); + + assertThat(callForwarded.get()).isFalse(); + } + + @Test + public void release_withSubsequentPlayerAction_ignoresSubsequentAction() { + AtomicBoolean releaseCalled = new AtomicBoolean(); + AtomicBoolean getStateCalledAfterRelease = new AtomicBoolean(); + AtomicBoolean handlePlayWhenReadyCalledAfterRelease = new AtomicBoolean(); + SimpleBasePlayer player = + new SimpleBasePlayer(Looper.myLooper()) { + @Override + protected State getState() { + if (releaseCalled.get()) { + getStateCalledAfterRelease.set(true); + } + return new State.Builder() + .setAvailableCommands(new Commands.Builder().addAllCommands().build()) + .build(); + } + + @Override + protected ListenableFuture handleSetPlayWhenReady(boolean playWhenReady) { + if (releaseCalled.get()) { + handlePlayWhenReadyCalledAfterRelease.set(true); + } + return Futures.immediateVoidFuture(); + } + + @Override + protected ListenableFuture handleRelease() { + return Futures.immediateVoidFuture(); + } + }; + + player.release(); + releaseCalled.set(true); + // Try triggering a regular player action and to invalidate the state manually. + player.setPlayWhenReady(true); + player.invalidateState(); + + assertThat(getStateCalledAfterRelease.get()).isFalse(); + assertThat(handlePlayWhenReadyCalledAfterRelease.get()).isFalse(); + } + @Test public void setRepeatMode_immediateHandling_updatesStateAndInformsListeners() { State state =