Skip to content

Fix out-of-order command execution in MediaController for Android 12(L) #3279

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nift4 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix out-of-order command execution in MediaController for Android 12(L) #3279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8596ecb
Add failing test
nift4 Jun 17, 2026
0e26b88
Fix out-of-order command execution in MediaController for Android 12(L)
nift4 Jun 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
170 changes: 79 additions & 91 deletions libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@
import com.google.common.base.Supplier;
import com.google.common.base.Suppliers;
import com.google.common.collect.ImmutableList;
import com.google.common.util.concurrent.FutureCallback;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import java.lang.ref.WeakReference;
Expand Down Expand Up @@ -1290,103 +1289,92 @@ protected MediaSessionServiceLegacyStub getLegacyBrowserService() {
*
* @param controller The controller requesting to play.
*/
/* package */ void handleMediaControllerPlayRequest(
/* package */ ListenableFuture<SessionResult> handleMediaControllerPlayRequest(
ControllerInfo controller, boolean callOnPlayerInteractionFinished) {
ListenableFuture<Boolean> playRequestFuture = onPlayRequested();
Futures.addCallback(
playRequestFuture,
new FutureCallback<Boolean>() {
@Override
public void onSuccess(Boolean result) {
if (result) {
handleMediaControllerPlayRequestInternal(controller, callOnPlayerInteractionFinished);
}
return Futures.transformAsync(
onPlayRequested(),
playRequested -> {
if (!playRequested) {
// Request denied, e.g. due to missing foreground service abilities.
return Futures.immediateFuture(new SessionResult(SessionResult.RESULT_ERROR_UNKNOWN));
}

@Override
public void onFailure(Throwable t) {
Log.e(TAG, "Failed calling onPlayRequested", t);
boolean hasCurrentMediaItem =
playerWrapper.isCommandAvailable(Player.COMMAND_GET_CURRENT_MEDIA_ITEM)
&& playerWrapper.getCurrentMediaItem() != null;
boolean canAddMediaItems =
playerWrapper.isCommandAvailable(COMMAND_SET_MEDIA_ITEM)
|| playerWrapper.isCommandAvailable(COMMAND_CHANGE_MEDIA_ITEMS);
ControllerInfo controllerForRequest = resolveControllerInfoForCallback(controller);
Player.Commands playCommand =
new Player.Commands.Builder().add(Player.COMMAND_PLAY_PAUSE).build();
if (hasCurrentMediaItem || !canAddMediaItems) {
// No playback resumption needed or possible.
if (!hasCurrentMediaItem) {
Log.w(
TAG,
"Play requested without current MediaItem, but playback resumption prevented by"
+ " missing available commands");
}
Util.handlePlayButtonAction(playerWrapper);
if (callOnPlayerInteractionFinished) {
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
}
return Futures.immediateFuture(new SessionResult(SessionResult.RESULT_SUCCESS));
} else {
ListenableFuture<SessionResult> future =
Futures.transform(
checkNotNull(
callback.onPlaybackResumption(
instance, controllerForRequest, /* isForPlayback= */ true),
"Callback.onPlaybackResumption must return a non-null future"),
mediaItemsWithStartPosition -> {
callWithControllerForCurrentRequestSet(
controllerForRequest,
() -> {
MediaUtils.setMediaItemsWithStartIndexAndPosition(
playerWrapper, mediaItemsWithStartPosition);
Util.handlePlayButtonAction(playerWrapper);
if (callOnPlayerInteractionFinished) {
onPlayerInteractionFinishedOnHandler(
controllerForRequest, playCommand);
}
})
.run();
return new SessionResult(SessionResult.RESULT_SUCCESS);
},
this::postOrRunOnApplicationHandler);
return Futures.catching(
future,
Throwable.class,
t -> {
if (t instanceof UnsupportedOperationException) {
Log.w(
TAG,
"UnsupportedOperationException: Make sure to implement"
+ " MediaSession.Callback.onPlaybackResumption() if you add a media"
+ " button receiver to your manifest or if you implement the recent"
+ " media item contract with your MediaLibraryService.",
t);
} else {
Log.e(
TAG,
"Failure calling MediaSession.Callback.onPlaybackResumption(): "
+ t.getMessage(),
t);
}
// Play as requested even if playback resumption fails.
Util.handlePlayButtonAction(playerWrapper);
if (callOnPlayerInteractionFinished) {
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
}
return new SessionResult(SessionResult.RESULT_SUCCESS);
},
this::postOrRunOnApplicationHandler);
}
},
this::postOrRunOnApplicationHandler);
}

private void handleMediaControllerPlayRequestInternal(
ControllerInfo controller, boolean callOnPlayerInteractionFinished) {
boolean hasCurrentMediaItem =
playerWrapper.isCommandAvailable(Player.COMMAND_GET_CURRENT_MEDIA_ITEM)
&& playerWrapper.getCurrentMediaItem() != null;
boolean canAddMediaItems =
playerWrapper.isCommandAvailable(COMMAND_SET_MEDIA_ITEM)
|| playerWrapper.isCommandAvailable(COMMAND_CHANGE_MEDIA_ITEMS);
ControllerInfo controllerForRequest = resolveControllerInfoForCallback(controller);
Player.Commands playCommand =
new Player.Commands.Builder().add(Player.COMMAND_PLAY_PAUSE).build();
if (hasCurrentMediaItem || !canAddMediaItems) {
// No playback resumption needed or possible.
if (!hasCurrentMediaItem) {
Log.w(
TAG,
"Play requested without current MediaItem, but playback resumption prevented by"
+ " missing available commands");
}
Util.handlePlayButtonAction(playerWrapper);
if (callOnPlayerInteractionFinished) {
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
}
} else {
@Nullable
ListenableFuture<MediaItemsWithStartPosition> future =
checkNotNull(
callback.onPlaybackResumption(
instance, controllerForRequest, /* isForPlayback= */ true),
"Callback.onPlaybackResumption must return a non-null future");
Futures.addCallback(
future,
new FutureCallback<MediaItemsWithStartPosition>() {
@Override
public void onSuccess(MediaItemsWithStartPosition mediaItemsWithStartPosition) {
callWithControllerForCurrentRequestSet(
controllerForRequest,
() -> {
MediaUtils.setMediaItemsWithStartIndexAndPosition(
playerWrapper, mediaItemsWithStartPosition);
Util.handlePlayButtonAction(playerWrapper);
if (callOnPlayerInteractionFinished) {
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
}
})
.run();
}

@Override
public void onFailure(Throwable t) {
if (t instanceof UnsupportedOperationException) {
Log.w(
TAG,
"UnsupportedOperationException: Make sure to implement"
+ " MediaSession.Callback.onPlaybackResumption() if you add a"
+ " media button receiver to your manifest or if you implement the recent"
+ " media item contract with your MediaLibraryService.",
t);
} else {
Log.e(
TAG,
"Failure calling MediaSession.Callback.onPlaybackResumption(): "
+ t.getMessage(),
t);
}
// Play as requested even if playback resumption fails.
Util.handlePlayButtonAction(playerWrapper);
if (callOnPlayerInteractionFinished) {
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
}
}
},
this::postOrRunOnApplicationHandler);
}
}

/* package */ void triggerPlayerInfoUpdate() {
onPlayerInfoChangedHandler.sendPlayerInfoChangedMessage(
/* excludeTimeline= */ true, /* excludeTracks= */ true);
Expand Down
24 changes: 21 additions & 3 deletions libraries/session/src/main/java/androidx/media3/session/MediaSessionLegacyStub.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -836,9 +836,27 @@ public ConnectedControllersManager<RemoteUserInfo> getConnectedControllersManage
private void dispatchSessionTaskWithPlayRequest() {
dispatchSessionTaskWithPlayerCommand(
COMMAND_PLAY_PAUSE,
controller ->
sessionImpl.handleMediaControllerPlayRequest(
controller, /* callOnPlayerInteractionFinished= */ true),
controller -> {
ListenableFuture<SessionResult> resultFuture =
sessionImpl.handleMediaControllerPlayRequest(
controller, /* callOnPlayerInteractionFinished= */ true);
Futures.addCallback(
resultFuture,
new FutureCallback<SessionResult>() {
@Override
public void onSuccess(SessionResult result) {
if (result.resultCode != RESULT_SUCCESS) {
Log.w(TAG, "onPlay() failed: " + result + " (from: " + controller + ")");
}
}

@Override
public void onFailure(Throwable t) {
Log.e(TAG, "Unexpected exception in onPlay() of " + controller, t);
}
},
MoreExecutors.directExecutor());
},
sessionCompat.getCurrentControllerInfo(),
/* callOnPlayerInteractionFinished= */ false);
}
Expand Down
13 changes: 4 additions & 9 deletions libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -826,15 +826,10 @@ public void playForControllerInfo(ControllerInfo controller, int sequenceNumber)
controller,
sequenceNumber,
COMMAND_PLAY_PAUSE,
sendSessionResultSuccess(
player -> {
@Nullable MediaSessionImpl impl = sessionImpl.get();
if (impl == null || impl.isReleased()) {
return;
}
impl.handleMediaControllerPlayRequest(
controller, /* callOnPlayerInteractionFinished= */ false);
}));
sendSessionResultWhenReady(
(session, theController, sequenceId) ->
session.handleMediaControllerPlayRequest(
theController, /* callOnPlayerInteractionFinished= */ false)));
}

@Override
Expand Down
16 changes: 16 additions & 0 deletions ...est/java/androidx/media3/session/MediaSessionServiceWithNonMainApplicationThreadTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ public void play_playCalledOnAppThread() throws Exception {
Looper appLooper = appThread.getLooper();

SettableFuture<Looper> playFuture = SettableFuture.create();
SettableFuture<Void> setVolumeWasCalled = SettableFuture.create();
testServiceRegistry.setOnGetSessionHandler(
controllerInfo -> {
ExoPlayer exoPlayer =
Expand All @@ -107,6 +108,17 @@ public void play_playCalledOnAppThread() throws Exception {
public void play() {
playFuture.set(Looper.myLooper());
}

@Override
public void setVolume(float volume) {
// Must be already done as setVolume() is called after play() on the controller.
try {
assertThat(playFuture.get(0, MILLISECONDS)).isNotNull();
} catch (Exception e) {
throw new RuntimeException(e);
}
setVolumeWasCalled.set(null);
}
};
return new MediaSession.Builder(context, player)
.setCallback(
Expand All @@ -133,9 +145,13 @@ public ListenableFuture<MediaItemsWithStartPosition> onPlaybackResumption(
RemoteMediaController controller = controllerTestRule.createRemoteController(token);

controller.play();
// Send a command after play to assert they are processed in the correct order even if play
// takes a moment. See setVolume above.
controller.setVolume(0.5f);

// Verify that playback starts on the application looper.
assertThat(playFuture.get(TIMEOUT_MS, MILLISECONDS)).isEqualTo(appLooper);
assertThat(setVolumeWasCalled.get(TIMEOUT_MS, MILLISECONDS)).isEqualTo(null);
appThread.quitSafely();
}

Expand Down