Add COMMAND_RELEASE to set whether Player instances can be released

This wasn't added so far because releasing is always allowed from a
MediaController (as it just releases the connection, not the session
player). But Player instances can be created for other purposes and
the receiver of a Player instance should not always be allowed to
call release if it doesn't own the player resource.

PiperOrigin-RevId: 519121122
This commit is contained in:
tonihei 2023-03-24 12:45:43 +00:00 committed by Tianyi Feng
parent 710b462b5f
commit fc54a7a487
13 changed files with 70 additions and 24 deletions

View File

@ -729,6 +729,7 @@ package androidx.media3.common {
field public static final int COMMAND_INVALID = -1; // 0xffffffff
field public static final int COMMAND_PLAY_PAUSE = 1; // 0x1
field public static final int COMMAND_PREPARE = 2; // 0x2
field public static final int COMMAND_RELEASE = 32; // 0x20
field public static final int COMMAND_SEEK_BACK = 11; // 0xb
field public static final int COMMAND_SEEK_FORWARD = 12; // 0xc
field public static final int COMMAND_SEEK_IN_CURRENT_MEDIA_ITEM = 5; // 0x5
@ -809,7 +810,7 @@ package androidx.media3.common {
field public static final int TIMELINE_CHANGE_REASON_SOURCE_UPDATE = 1; // 0x1
}
@IntDef({androidx.media3.common.Player.COMMAND_INVALID, androidx.media3.common.Player.COMMAND_PLAY_PAUSE, androidx.media3.common.Player.COMMAND_PREPARE, androidx.media3.common.Player.COMMAND_STOP, androidx.media3.common.Player.COMMAND_SEEK_TO_DEFAULT_POSITION, androidx.media3.common.Player.COMMAND_SEEK_IN_CURRENT_MEDIA_ITEM, androidx.media3.common.Player.COMMAND_SEEK_TO_PREVIOUS_MEDIA_ITEM, androidx.media3.common.Player.COMMAND_SEEK_TO_PREVIOUS, androidx.media3.common.Player.COMMAND_SEEK_TO_NEXT_MEDIA_ITEM, androidx.media3.common.Player.COMMAND_SEEK_TO_NEXT, androidx.media3.common.Player.COMMAND_SEEK_TO_MEDIA_ITEM, androidx.media3.common.Player.COMMAND_SEEK_BACK, androidx.media3.common.Player.COMMAND_SEEK_FORWARD, androidx.media3.common.Player.COMMAND_SET_SPEED_AND_PITCH, androidx.media3.common.Player.COMMAND_SET_SHUFFLE_MODE, androidx.media3.common.Player.COMMAND_SET_REPEAT_MODE, androidx.media3.common.Player.COMMAND_GET_CURRENT_MEDIA_ITEM, androidx.media3.common.Player.COMMAND_GET_TIMELINE, androidx.media3.common.Player.COMMAND_GET_MEDIA_ITEMS_METADATA, androidx.media3.common.Player.COMMAND_SET_MEDIA_ITEMS_METADATA, androidx.media3.common.Player.COMMAND_SET_MEDIA_ITEM, androidx.media3.common.Player.COMMAND_CHANGE_MEDIA_ITEMS, androidx.media3.common.Player.COMMAND_GET_AUDIO_ATTRIBUTES, androidx.media3.common.Player.COMMAND_GET_VOLUME, androidx.media3.common.Player.COMMAND_GET_DEVICE_VOLUME, androidx.media3.common.Player.COMMAND_SET_VOLUME, androidx.media3.common.Player.COMMAND_SET_DEVICE_VOLUME, androidx.media3.common.Player.COMMAND_ADJUST_DEVICE_VOLUME, androidx.media3.common.Player.COMMAND_SET_VIDEO_SURFACE, androidx.media3.common.Player.COMMAND_GET_TEXT, androidx.media3.common.Player.COMMAND_SET_TRACK_SELECTION_PARAMETERS, androidx.media3.common.Player.COMMAND_GET_TRACKS}) @java.lang.annotation.Documented @java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.SOURCE) @java.lang.annotation.Target({java.lang.annotation.ElementType.FIELD, java.lang.annotation.ElementType.METHOD, java.lang.annotation.ElementType.PARAMETER, java.lang.annotation.ElementType.LOCAL_VARIABLE, java.lang.annotation.ElementType.TYPE_USE}) public static @interface Player.Command {
@IntDef({androidx.media3.common.Player.COMMAND_INVALID, androidx.media3.common.Player.COMMAND_PLAY_PAUSE, androidx.media3.common.Player.COMMAND_PREPARE, androidx.media3.common.Player.COMMAND_STOP, androidx.media3.common.Player.COMMAND_SEEK_TO_DEFAULT_POSITION, androidx.media3.common.Player.COMMAND_SEEK_IN_CURRENT_MEDIA_ITEM, androidx.media3.common.Player.COMMAND_SEEK_TO_PREVIOUS_MEDIA_ITEM, androidx.media3.common.Player.COMMAND_SEEK_TO_PREVIOUS, androidx.media3.common.Player.COMMAND_SEEK_TO_NEXT_MEDIA_ITEM, androidx.media3.common.Player.COMMAND_SEEK_TO_NEXT, androidx.media3.common.Player.COMMAND_SEEK_TO_MEDIA_ITEM, androidx.media3.common.Player.COMMAND_SEEK_BACK, androidx.media3.common.Player.COMMAND_SEEK_FORWARD, androidx.media3.common.Player.COMMAND_SET_SPEED_AND_PITCH, androidx.media3.common.Player.COMMAND_SET_SHUFFLE_MODE, androidx.media3.common.Player.COMMAND_SET_REPEAT_MODE, androidx.media3.common.Player.COMMAND_GET_CURRENT_MEDIA_ITEM, androidx.media3.common.Player.COMMAND_GET_TIMELINE, androidx.media3.common.Player.COMMAND_GET_MEDIA_ITEMS_METADATA, androidx.media3.common.Player.COMMAND_SET_MEDIA_ITEMS_METADATA, androidx.media3.common.Player.COMMAND_SET_MEDIA_ITEM, androidx.media3.common.Player.COMMAND_CHANGE_MEDIA_ITEMS, androidx.media3.common.Player.COMMAND_GET_AUDIO_ATTRIBUTES, androidx.media3.common.Player.COMMAND_GET_VOLUME, androidx.media3.common.Player.COMMAND_GET_DEVICE_VOLUME, androidx.media3.common.Player.COMMAND_SET_VOLUME, androidx.media3.common.Player.COMMAND_SET_DEVICE_VOLUME, androidx.media3.common.Player.COMMAND_ADJUST_DEVICE_VOLUME, androidx.media3.common.Player.COMMAND_SET_VIDEO_SURFACE, androidx.media3.common.Player.COMMAND_GET_TEXT, androidx.media3.common.Player.COMMAND_SET_TRACK_SELECTION_PARAMETERS, androidx.media3.common.Player.COMMAND_GET_TRACKS, androidx.media3.common.Player.COMMAND_RELEASE}) @java.lang.annotation.Documented @java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.SOURCE) @java.lang.annotation.Target({java.lang.annotation.ElementType.FIELD, java.lang.annotation.ElementType.METHOD, java.lang.annotation.ElementType.PARAMETER, java.lang.annotation.ElementType.LOCAL_VARIABLE, java.lang.annotation.ElementType.TYPE_USE}) public static @interface Player.Command {
}
public static final class Player.Commands {

View File

@ -108,7 +108,8 @@ public final class CastPlayer extends BasePlayer {
COMMAND_SET_MEDIA_ITEMS_METADATA,
COMMAND_SET_MEDIA_ITEM,
COMMAND_CHANGE_MEDIA_ITEMS,
COMMAND_GET_TRACKS)
COMMAND_GET_TRACKS,
COMMAND_RELEASE)
.build();
public static final float MIN_SPEED_SUPPORTED = 0.5f;

View File

@ -1372,6 +1372,7 @@ public class CastPlayerTest {
assertThat(castPlayer.isCommandAvailable(COMMAND_ADJUST_DEVICE_VOLUME)).isFalse();
assertThat(castPlayer.isCommandAvailable(COMMAND_SET_VIDEO_SURFACE)).isFalse();
assertThat(castPlayer.isCommandAvailable(COMMAND_GET_TEXT)).isFalse();
assertThat(castPlayer.isCommandAvailable(Player.COMMAND_RELEASE)).isTrue();
}
@Test

View File

@ -385,6 +385,7 @@ public interface Player {
COMMAND_GET_TEXT,
COMMAND_SET_TRACK_SELECTION_PARAMETERS,
COMMAND_GET_TRACKS,
COMMAND_RELEASE
};
private final FlagSet.Builder flagsBuilder;
@ -1441,6 +1442,7 @@ public interface Player {
* <li>{@link #COMMAND_GET_TEXT}
* <li>{@link #COMMAND_SET_TRACK_SELECTION_PARAMETERS}
* <li>{@link #COMMAND_GET_TRACKS}
* <li>{@link #COMMAND_RELEASE}
* </ul>
*/
// @Target list includes both 'default' targets and TYPE_USE, to ensure backwards compatibility
@ -1481,6 +1483,7 @@ public interface Player {
COMMAND_GET_TEXT,
COMMAND_SET_TRACK_SELECTION_PARAMETERS,
COMMAND_GET_TRACKS,
COMMAND_RELEASE,
})
@interface Command {}
/**
@ -1837,6 +1840,13 @@ public interface Player {
* #isCommandAvailable(int) available}.
*/
int COMMAND_GET_TRACKS = 30;
/**
* Command to release the player.
*
* <p>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.
*
* <p>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();
/**

View File

@ -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}.
*
* <p>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");

View File

@ -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 =

View File

@ -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();

View File

@ -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);
}

View File

@ -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 {

View File

@ -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();
}

View File

@ -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)) {

View File

@ -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);
}

View File

@ -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