diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaSession.java b/libraries/session/src/main/java/androidx/media3/session/MediaSession.java index a2eece61663..ab84e4855ed 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSession.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSession.java @@ -1935,6 +1935,14 @@ default ListenableFuture onPlaybackResumption( * automatically as required. Any additional initial setup like setting playback speed, repeat * mode or shuffle mode can be done from within this callback. * + *

If the returned list is empty or an exception is returned, and the request to resume + * playback came from {@link MediaButtonReceiver}, an {@link IllegalStateException} will be + * thrown. This is because the {@link MediaButtonReceiver} has already requested a foreground + * service start, and it's not possible to avoid a {@code + * ForegroundServiceDidNotStartInTimeException} anymore. To avoid a crash as result, override + * {@link MediaButtonReceiver#shouldStartForegroundService(Context, Intent)} to return {@code + * false} if there is nothing to resume. This will avoid requesting a foreground service start. + * *

The method will only be called if the {@link Player} has {@link * Player#COMMAND_GET_CURRENT_MEDIA_ITEM} and either {@link Player#COMMAND_SET_MEDIA_ITEM} or * {@link Player#COMMAND_CHANGE_MEDIA_ITEMS} available. 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 90f17afdcca..086e798bad8 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java @@ -1113,20 +1113,23 @@ protected MediaSessionServiceLegacyStub getLegacyBrowserService() { }); } - /* package */ boolean onPlayRequested() { + /* package */ ListenableFuture onPlayRequested() { if (Looper.myLooper() != Looper.getMainLooper()) { SettableFuture playRequested = SettableFuture.create(); - mainHandler.post(() -> playRequested.set(onPlayRequested())); - try { - return playRequested.get(); - } catch (InterruptedException | ExecutionException e) { - throw new IllegalStateException(e); - } + mainHandler.post( + () -> { + try { + playRequested.set(onPlayRequested().get()); + } catch (ExecutionException | InterruptedException e) { + playRequested.setException(new IllegalStateException(e)); + } + }); + return playRequested; } if (this.mediaSessionListener != null) { - return this.mediaSessionListener.onPlayRequested(instance); + return Futures.immediateFuture(this.mediaSessionListener.onPlayRequested(instance)); } - return true; + return Futures.immediateFuture(true); } /** @@ -1137,84 +1140,137 @@ protected MediaSessionServiceLegacyStub getLegacyBrowserService() { * * @param controller The controller requesting to play. */ - /* package */ void handleMediaControllerPlayRequest( - ControllerInfo controller, boolean callOnPlayerInteractionFinished) { - if (!onPlayRequested()) { - // Request denied, e.g. due to missing foreground service abilities. - return; - } - 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 future = - checkNotNull( - callback.onPlaybackResumption( - instance, controllerForRequest, /* isForPlayback= */ true), - "Callback.onPlaybackResumption must return a non-null future"); - Futures.addCallback( - future, - new FutureCallback() { - @Override - public void onSuccess(MediaItemsWithStartPosition mediaItemsWithStartPosition) { - callWithControllerForCurrentRequestSet( - controllerForRequest, - () -> { - MediaUtils.setMediaItemsWithStartIndexAndPosition( - playerWrapper, mediaItemsWithStartPosition); - Util.handlePlayButtonAction(playerWrapper); - if (callOnPlayerInteractionFinished) { - onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand); - } - }) - .run(); + /* package */ ListenableFuture handleMediaControllerPlayRequest( + ControllerInfo controller, + boolean callOnPlayerInteractionFinished, + boolean mustStartForegroundService) { + SettableFuture sessionFuture = SettableFuture.create(); + ListenableFuture playRequestedFuture = onPlayRequested(); + playRequestedFuture.addListener( + () -> { + boolean playRequested; + try { + playRequested = playRequestedFuture.get(); + } catch (ExecutionException | InterruptedException e) { + sessionFuture.setException(new IllegalStateException(e)); + return; + } + if (!playRequested) { + // Request denied, e.g. due to missing foreground service abilities. + sessionFuture.set(new SessionResult(SessionResult.RESULT_ERROR_UNKNOWN)); + return; + } + 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"); } - - @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); - } + Util.handlePlayButtonAction(playerWrapper); + sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS)); + if (callOnPlayerInteractionFinished) { + onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand); } - }, - this::postOrRunOnApplicationHandler); - } + } else { + @Nullable + ListenableFuture future = + checkNotNull( + callback.onPlaybackResumption( + instance, controllerForRequest, /* isForPlayback= */ true), + "Callback.onPlaybackResumption must return a non-null future"); + Futures.addCallback( + future, + new FutureCallback() { + @Override + public void onSuccess(MediaItemsWithStartPosition mediaItemsWithStartPosition) { + callWithControllerForCurrentRequestSet( + controllerForRequest, + () -> { + if (mediaItemsWithStartPosition.mediaItems.isEmpty()) { + if (mustStartForegroundService) { + applicationHandler.postAtFrontOfQueue( + () -> { + throw new IllegalArgumentException( + "Callback.onPlaybackResumption must return non-empty" + + " MediaItemsWithStartPosition if started from a" + + " media button receiver. If there is nothing to" + + " resume playback with, override" + + " MediaButtonReceiver.shouldStartForegroundService()" + + " and return false."); + }); + return; + } + Log.w( + TAG, + "onPlaybackResumption() is trying to resume with empty" + + " playlist, this will make the resumption notification" + + " appear broken."); + } + MediaUtils.setMediaItemsWithStartIndexAndPosition( + playerWrapper, mediaItemsWithStartPosition); + Util.handlePlayButtonAction(playerWrapper); + sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS)); + if (callOnPlayerInteractionFinished) { + onPlayerInteractionFinishedOnHandler( + controllerForRequest, playCommand); + } + }) + .run(); + } + + @Override + public void onFailure(Throwable t) { + RuntimeException e; + if (t instanceof UnsupportedOperationException) { + e = + new 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 { + e = + new IllegalStateException( + "Failure calling MediaSession.Callback.onPlaybackResumption(): " + + t.getMessage(), + t); + } + if (mustStartForegroundService) { + // MediaButtonReceiver already called startForegroundService(). If we do not + // crash ourselves, ForegroundServiceDidNotStartInTimeException will do it + // for us. Let's at least get a useful stack trace out there. + applicationHandler.postAtFrontOfQueue( + () -> { + throw e; + }); + return; + } + Log.e(TAG, Objects.requireNonNull(Log.getThrowableString(e))); + // Play as requested even if playback resumption fails. + Util.handlePlayButtonAction(playerWrapper); + sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS)); + if (callOnPlayerInteractionFinished) { + onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand); + } + } + }, + this::postOrRunOnApplicationHandler); + } + }, + this::postOrRunOnApplicationHandler); + return sessionFuture; } /* package */ void triggerPlayerInfoUpdate() { @@ -1460,13 +1516,13 @@ private void handleAvailablePlayerCommandsChanged(Player.Commands availableComma // Double tap detection. int keyCode = keyEvent.getKeyCode(); boolean isTvApp = context.getPackageManager().hasSystemFeature(PackageManager.FEATURE_LEANBACK); + boolean isEventSourceMediaButtonReceiver = + callerInfo.getControllerVersion() != ControllerInfo.LEGACY_CONTROLLER_VERSION; boolean doubleTapCompleted = false; switch (keyCode) { case KEYCODE_MEDIA_PLAY_PAUSE: case KEYCODE_HEADSETHOOK: - if (isTvApp - || callerInfo.getControllerVersion() != ControllerInfo.LEGACY_CONTROLLER_VERSION - || keyEvent.getRepeatCount() != 0) { + if (isTvApp || isEventSourceMediaButtonReceiver || keyEvent.getRepeatCount() != 0) { // Double tap detection is only for mobile apps that receive a media button event from // external sources (for instance Bluetooth) and excluding long press (repeatCount > 0). mediaPlayPauseKeyHandler.flush(); @@ -1506,11 +1562,18 @@ private void handleAvailablePlayerCommandsChanged(Player.Commands availableComma intent.getBooleanExtra( MediaNotification.NOTIFICATION_DISMISSED_EVENT_KEY, /* defaultValue= */ false); return keyEvent.getRepeatCount() > 0 - || applyMediaButtonKeyEvent(keyEvent, doubleTapCompleted, isDismissNotificationEvent); + || applyMediaButtonKeyEvent( + keyEvent, + doubleTapCompleted, + isDismissNotificationEvent, + isEventSourceMediaButtonReceiver); } private boolean applyMediaButtonKeyEvent( - KeyEvent keyEvent, boolean doubleTapCompleted, boolean isDismissNotificationEvent) { + KeyEvent keyEvent, + boolean doubleTapCompleted, + boolean isDismissNotificationEvent, + boolean mustStartForegroundService) { ControllerInfo controllerInfo = checkNotNull(instance.getMediaNotificationControllerInfo()); Runnable command; int keyCode = keyEvent.getKeyCode(); @@ -1524,10 +1587,15 @@ private boolean applyMediaButtonKeyEvent( command = getPlayerWrapper().getPlayWhenReady() ? () -> sessionStub.pauseForControllerInfo(controllerInfo, UNKNOWN_SEQUENCE_NUMBER) - : () -> sessionStub.playForControllerInfo(controllerInfo, UNKNOWN_SEQUENCE_NUMBER); + : () -> + sessionStub.playForControllerInfo( + controllerInfo, UNKNOWN_SEQUENCE_NUMBER, mustStartForegroundService); break; case KEYCODE_MEDIA_PLAY: - command = () -> sessionStub.playForControllerInfo(controllerInfo, UNKNOWN_SEQUENCE_NUMBER); + command = + () -> + sessionStub.playForControllerInfo( + controllerInfo, UNKNOWN_SEQUENCE_NUMBER, mustStartForegroundService); break; case KEYCODE_MEDIA_PAUSE: command = () -> sessionStub.pauseForControllerInfo(controllerInfo, UNKNOWN_SEQUENCE_NUMBER); @@ -2142,7 +2210,8 @@ public void setPendingPlayPauseTask(ControllerInfo controllerInfo, KeyEvent keyE applyMediaButtonKeyEvent( keyEvent, /* doubleTapCompleted= */ false, - /* isDismissNotificationEvent= */ false); + /* isDismissNotificationEvent= */ false, + /* mustStartForegroundService= */ false); } else { sessionLegacyStub.handleMediaPlayPauseOnHandler( checkNotNull(controllerInfo.getRemoteUserInfo())); 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 95dfbc4f20c..9f9c996d903 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionLegacyStub.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionLegacyStub.java @@ -607,9 +607,29 @@ public void onPrepareFromUri(@Nullable Uri mediaUri, @Nullable Bundle extras) { public void onPlay() { dispatchSessionTaskWithPlayerCommand( COMMAND_PLAY_PAUSE, - controller -> - sessionImpl.handleMediaControllerPlayRequest( - controller, /* callOnPlayerInteractionFinished= */ true), + controller -> { + ListenableFuture resultFuture = + sessionImpl.handleMediaControllerPlayRequest( + controller, + /* callOnPlayerInteractionFinished= */ true, + /* mustStartForegroundService= */ false); + Futures.addCallback( + resultFuture, + new FutureCallback() { + @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); } diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java b/libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java index a0c574f31f0..50f041d4a16 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java @@ -761,24 +761,22 @@ public void play(@Nullable IMediaController caller, int sequenceNumber) { @Nullable ControllerInfo controller = connectedControllersManager.getController(caller.asBinder()); if (controller != null) { - playForControllerInfo(controller, sequenceNumber); + playForControllerInfo(controller, sequenceNumber, /* mustStartForegroundService= */ false); } } - public void playForControllerInfo(ControllerInfo controller, int sequenceNumber) { + public void playForControllerInfo( + ControllerInfo controller, int sequenceNumber, boolean mustStartForegroundService) { queueSessionTaskWithPlayerCommandForControllerInfo( 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, + /* mustStartForegroundService= */ mustStartForegroundService))); } @Override diff --git a/libraries/session/src/test/java/androidx/media3/session/MediaSessionServiceTest.java b/libraries/session/src/test/java/androidx/media3/session/MediaSessionServiceTest.java index 470da4a5878..48feda02024 100644 --- a/libraries/session/src/test/java/androidx/media3/session/MediaSessionServiceTest.java +++ b/libraries/session/src/test/java/androidx/media3/session/MediaSessionServiceTest.java @@ -18,6 +18,7 @@ import static androidx.media3.test.utils.robolectric.RobolectricUtil.runMainLooperUntil; import static com.google.common.truth.Truth.assertThat; import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static org.junit.Assert.assertThrows; import android.app.NotificationManager; import android.content.Context; @@ -794,6 +795,20 @@ public void onEvents(Player player, Player.Events events) { serviceController.destroy(); } + @Test + public void onStartCommand_playbackResumption_emptyResultWillThrow() { + Intent playIntent = new Intent(Intent.ACTION_MEDIA_BUTTON); + playIntent.putExtra( + Intent.EXTRA_KEY_EVENT, new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_MEDIA_PLAY)); + ServiceController serviceController = + Robolectric.buildService(TestServiceWithPlaybackResumption.class, playIntent); + TestServiceWithPlaybackResumption service = serviceController.create().get(); + service.setMediaItems(ImmutableList.of()); + assertThrows(IllegalArgumentException.class, + () -> serviceController.startCommand(/* flags= */ 0, /* startId= */ 0)); + serviceController.destroy(); + } + @Test public void onStartCommand_customCommands_deliveredByMediaNotificationController() throws InterruptedException { diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionServiceTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionServiceTest.java index 96fbee105ac..f95496ac6be 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionServiceTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionServiceTest.java @@ -28,14 +28,17 @@ import android.content.Context; import android.os.Bundle; import android.os.Handler; +import android.os.HandlerThread; import android.os.Looper; import android.service.notification.StatusBarNotification; import android.support.v4.media.session.MediaControllerCompat; import android.support.v4.media.session.MediaSessionCompat; import android.support.v4.media.session.PlaybackStateCompat; import androidx.annotation.Nullable; +import androidx.core.content.ContextCompat; import androidx.media3.common.ForwardingPlayer; import androidx.media3.common.MediaItem; +import androidx.media3.common.PlaybackParameters; import androidx.media3.common.Player; import androidx.media3.common.util.ConditionVariable; import androidx.media3.exoplayer.ExoPlayer; @@ -51,6 +54,8 @@ import androidx.test.filters.MediumTest; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; +import com.google.common.util.concurrent.FutureCallback; +import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.SettableFuture; import java.util.ArrayList; @@ -248,6 +253,70 @@ public ListenableFuture> onAddMediaItems( service.blockUntilAllControllersUnbind(TIMEOUT_MS); } + @Test + public void onPlayRequested_doesNotCauseDeadlock_ifPlaybackThreadIsNotMain() throws Exception { + HandlerThread ht = new HandlerThread("MSSTest:PlaybackThread"); + ht.start(); + TestServiceRegistry testServiceRegistry = TestServiceRegistry.getInstance(); + ConditionVariable playerToldToSpeedUp = new ConditionVariable(); + AtomicReference mediaController = new AtomicReference<>(); + AtomicReference mediaSession = new AtomicReference<>(); + testServiceRegistry.setOnGetSessionHandler( + controllerInfo -> { + MockMediaSessionService service = + (MockMediaSessionService) testServiceRegistry.getServiceInstance(); + Player player = new ExoPlayer.Builder(service).setLooper(ht.getLooper()).build(); + player.addListener( + new Player.Listener() { + @Override + public void onPlaybackParametersChanged(PlaybackParameters playbackParameters) { + if (playbackParameters.speed == 2f) { + playerToldToSpeedUp.open(); + } + } + }); + mediaSession.set(new MediaSession.Builder(service, player).build()); + return mediaSession.get(); + }); + TestHandler handler = new TestHandler(Looper.getMainLooper()); + TestHandler bgHandler = new TestHandler(ht.getLooper()); + handler.post( + () -> { + ListenableFuture controllerFuture = + new MediaController.Builder(context, token).buildAsync(); + Futures.addCallback( + controllerFuture, + new FutureCallback() { + @Override + public void onSuccess(MediaController controller) { + controller.addMediaItem(new MediaItem.Builder().setMediaId("media_id").build()); + controller.play(); + bgHandler.post( + () -> + handler.post( + () -> { + controller.setPlaybackSpeed(2f); + mediaController.set(controller); + })); + } + + @Override + public void onFailure(Throwable t) { + throw new RuntimeException(t); + } + }, + ContextCompat.getMainExecutor(context)); + }); + playerToldToSpeedUp.block(TIMEOUT_MS); + if (!playerToldToSpeedUp.isOpen()) { + ht.interrupt(); // avoid deadlocking the test forever. + } + assertThat(playerToldToSpeedUp.isOpen()).isTrue(); + handler.postAndSync(() -> mediaController.get().release()); + mediaSession.get().release(); + ht.quitSafely(); + } + @Test public void onCreate_withCustomLayout_correctSessionStateFromOnConnect() throws Exception { SessionCommand command1 = new SessionCommand("command1", Bundle.EMPTY);