From 7a7e1eb23b4bbf7d0c6c15b2f484434e65cd2c68 Mon Sep 17 00:00:00 2001 From: christosts Date: Fri, 12 Aug 2022 16:53:36 +0000 Subject: [PATCH] MediaNotificationManager: handle playback ended When the player finishes playback and reaches the STATE_ENDED, the notification remains visible with a pause button and the service is kept in the foreground. This is a bug. With this change, when the player reaches the STATE_ENDED, the service is stopped from the foreground and a notification is shown with a play button. If the play icon is tapped, the player will restart playback of the last played item. Playing the last played item again is the existing behavior when play/pause commands are received from the legacy MediaSession (e.g. BT headset buttons). #minor-release Issue: google/ExoPlayer#112 PiperOrigin-RevId: 467231509 --- RELEASENOTES.md | 6 +++ .../DefaultMediaNotificationProvider.java | 16 +++++--- .../session/MediaNotificationManager.java | 25 ++++++------ .../session/MediaSessionLegacyStub.java | 27 +++++++------ .../session/MediaSessionKeyEventTest.java | 38 ++++++++++++++++++- 5 files changed, 79 insertions(+), 33 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index eb06385bbc..7e22569382 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -43,6 +43,12 @@ ([#99](https://github.com/androidx/media/issues/99)). * `SimpleBitmapLoader` can load bitmap from `file://` URIs ([#108](https://github.com/androidx/media/issues/108)). + * Fix bug where the `MediaSessionService` was kept started in the + foreground and a notification with a pause button was still shown when + the player completed playback. Now, once playback is completed, the + service is stopped from the foreground and a notification with a play + button is shown to restart playback of the last media item + ([#112](https://github.com/androidx/media/issues/112)). * RTSP: * Add H263 fragmented packet handling ([#119](https://github.com/androidx/media/pull/119)). diff --git a/libraries/session/src/main/java/androidx/media3/session/DefaultMediaNotificationProvider.java b/libraries/session/src/main/java/androidx/media3/session/DefaultMediaNotificationProvider.java index 372a576683..e6a90fde8b 100644 --- a/libraries/session/src/main/java/androidx/media3/session/DefaultMediaNotificationProvider.java +++ b/libraries/session/src/main/java/androidx/media3/session/DefaultMediaNotificationProvider.java @@ -23,6 +23,7 @@ import static androidx.media3.common.Player.COMMAND_SEEK_TO_NEXT_MEDIA_ITEM; import static androidx.media3.common.Player.COMMAND_SEEK_TO_PREVIOUS; import static androidx.media3.common.Player.COMMAND_SEEK_TO_PREVIOUS_MEDIA_ITEM; import static androidx.media3.common.Player.COMMAND_STOP; +import static androidx.media3.common.Player.STATE_ENDED; import static androidx.media3.common.util.Assertions.checkState; import static androidx.media3.common.util.Assertions.checkStateNotNull; @@ -269,7 +270,11 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi int[] compactViewIndices = addNotificationActions( mediaSession, - getMediaButtons(player.getAvailableCommands(), customLayout, player.getPlayWhenReady()), + getMediaButtons( + player.getAvailableCommands(), + customLayout, + /* showPauseButton= */ player.getPlayWhenReady() + && player.getPlaybackState() != STATE_ENDED), builder, actionFactory); mediaStyle.setShowActionsInCompactView(compactViewIndices); @@ -371,11 +376,12 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi * @param playerCommands The available player commands. * @param customLayout The {@linkplain MediaSession#setCustomLayout(List) custom layout of * commands}. - * @param playWhenReady The current {@code playWhenReady} state. + * @param showPauseButton Whether the notification should show a pause button (e.g., because the + * player is currently playing content), otherwise show a play button to start playback. * @return The ordered list of command buttons to be placed on the notification. */ protected List getMediaButtons( - Player.Commands playerCommands, List customLayout, boolean playWhenReady) { + Player.Commands playerCommands, List customLayout, boolean showPauseButton) { // Skip to previous action. List commandButtons = new ArrayList<>(); if (playerCommands.containsAny(COMMAND_SEEK_TO_PREVIOUS, COMMAND_SEEK_TO_PREVIOUS_MEDIA_ITEM)) { @@ -397,12 +403,12 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi new CommandButton.Builder() .setPlayerCommand(COMMAND_PLAY_PAUSE) .setIconResId( - playWhenReady + showPauseButton ? R.drawable.media3_notification_pause : R.drawable.media3_notification_play) .setExtras(commandButtonExtras) .setDisplayName( - playWhenReady + showPauseButton ? context.getString(R.string.media3_controls_pause_description) : context.getString(R.string.media3_controls_play_description)) .build()); diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaNotificationManager.java b/libraries/session/src/main/java/androidx/media3/session/MediaNotificationManager.java index d6385d725b..4ed2e8f52c 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaNotificationManager.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaNotificationManager.java @@ -162,7 +162,8 @@ import java.util.concurrent.TimeoutException; } public void updateNotification(MediaSession session) { - if (!mediaSessionService.isSessionAdded(session) || !canStartPlayback(session.getPlayer())) { + if (!mediaSessionService.isSessionAdded(session) + || !shouldShowNotification(session.getPlayer())) { maybeStopForegroundService(/* removeNotifications= */ true); return; } @@ -199,7 +200,7 @@ import java.util.concurrent.TimeoutException; this.mediaNotification = mediaNotification; Player player = session.getPlayer(); - if (player.getPlayWhenReady() && canStartPlayback(player)) { + if (shouldRunInForeground(player)) { ContextCompat.startForegroundService(mediaSessionService, startSelfIntent); if (Util.SDK_INT >= 29) { Api29.startForeground(mediaSessionService, mediaNotification); @@ -223,8 +224,7 @@ import java.util.concurrent.TimeoutException; private void maybeStopForegroundService(boolean removeNotifications) { List sessions = mediaSessionService.getSessions(); for (int i = 0; i < sessions.size(); i++) { - Player player = sessions.get(i).getPlayer(); - if (player.getPlayWhenReady() && canStartPlayback(player)) { + if (shouldRunInForeground(sessions.get(i).getPlayer())) { return; } } @@ -242,12 +242,14 @@ import java.util.concurrent.TimeoutException; } } - /** - * Returns whether {@code player} can start playback and therefore we should present a - * notification for this player. - */ - private static boolean canStartPlayback(Player player) { - return player.getPlaybackState() != Player.STATE_IDLE && !player.getCurrentTimeline().isEmpty(); + private static boolean shouldShowNotification(Player player) { + return !player.getCurrentTimeline().isEmpty() && player.getPlaybackState() != Player.STATE_IDLE; + } + + private static boolean shouldRunInForeground(Player player) { + return player.getPlayWhenReady() + && (player.getPlaybackState() == Player.STATE_READY + || player.getPlaybackState() == Player.STATE_BUFFERING); } private static final class MediaControllerListener @@ -266,8 +268,7 @@ import java.util.concurrent.TimeoutException; } public void onConnected() { - if (canStartPlayback(session.getPlayer())) { - // We need to present a notification. + if (shouldShowNotification(session.getPlayer())) { mediaSessionService.onUpdateNotification(session); } } 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 fd6ef392ba..afc3af1739 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionLegacyStub.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionLegacyStub.java @@ -241,17 +241,14 @@ import org.checkerframework.checker.initialization.qual.Initialized; private void handleMediaPlayPauseOnHandler(RemoteUserInfo remoteUserInfo) { mediaPlayPauseKeyHandler.clearPendingMediaPlayPauseKey(); - if (sessionImpl.getPlayerWrapper().getPlayWhenReady()) { - dispatchSessionTaskWithPlayerCommand( - COMMAND_PLAY_PAUSE, - (controller) -> sessionImpl.getPlayerWrapper().pause(), - remoteUserInfo); - } else { - dispatchSessionTaskWithPlayerCommand( - COMMAND_PLAY_PAUSE, - (controller) -> { - PlayerWrapper playerWrapper = sessionImpl.getPlayerWrapper(); - @Player.State int playbackState = playerWrapper.getPlaybackState(); + dispatchSessionTaskWithPlayerCommand( + COMMAND_PLAY_PAUSE, + (controller) -> { + PlayerWrapper playerWrapper = sessionImpl.getPlayerWrapper(); + @Player.State int playbackState = playerWrapper.getPlaybackState(); + if (!playerWrapper.getPlayWhenReady() + || playbackState == STATE_ENDED + || playbackState == STATE_IDLE) { if (playbackState == STATE_IDLE) { playerWrapper.prepare(); } else if (playbackState == STATE_ENDED) { @@ -259,9 +256,11 @@ import org.checkerframework.checker.initialization.qual.Initialized; playerWrapper.getCurrentMediaItemIndex(), /* positionMs= */ C.TIME_UNSET); } playerWrapper.play(); - }, - remoteUserInfo); - } + } else { + playerWrapper.pause(); + } + }, + remoteUserInfo); } @Override diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionKeyEventTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionKeyEventTest.java index 6c87986e4a..1ef9e15947 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionKeyEventTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionKeyEventTest.java @@ -26,6 +26,7 @@ import android.content.Context; import android.media.AudioManager; import android.media.MediaPlayer; import android.view.KeyEvent; +import androidx.media3.common.Player; import androidx.media3.common.util.Util; import androidx.media3.session.MediaSession.ControllerInfo; import androidx.media3.test.session.common.HandlerThreadTestRule; @@ -166,17 +167,50 @@ public class MediaSessionKeyEventTest { } @Test - public void playPauseKeyEvent_play() throws Exception { + public void playPauseKeyEvent_paused_play() throws Exception { + handler.postAndSync( + () -> { + player.playbackState = Player.STATE_READY; + }); + dispatchMediaKeyEvent(KeyEvent.KEYCODE_MEDIA_PLAY_PAUSE, false); player.awaitMethodCalled(MockPlayer.METHOD_PLAY, TIMEOUT_MS); } @Test - public void playPauseKeyEvent_pause() throws Exception { + public void playPauseKeyEvent_fromIdle_prepareAndPlay() throws Exception { + handler.postAndSync( + () -> { + player.playbackState = Player.STATE_IDLE; + }); + + dispatchMediaKeyEvent(KeyEvent.KEYCODE_MEDIA_PLAY_PAUSE, false); + + player.awaitMethodCalled(MockPlayer.METHOD_PREPARE, TIMEOUT_MS); + player.awaitMethodCalled(MockPlayer.METHOD_PLAY, TIMEOUT_MS); + } + + @Test + public void playPauseKeyEvent_playWhenReadyAndEnded_seekAndPlay() throws Exception { handler.postAndSync( () -> { player.playWhenReady = true; + player.playbackState = Player.STATE_ENDED; + }); + + dispatchMediaKeyEvent(KeyEvent.KEYCODE_MEDIA_PLAY_PAUSE, false); + + player.awaitMethodCalled(MockPlayer.METHOD_SEEK_TO_WITH_MEDIA_ITEM_INDEX, TIMEOUT_MS); + player.awaitMethodCalled(MockPlayer.METHOD_PLAY, TIMEOUT_MS); + } + + @Test + public void playPauseKeyEvent_playing_pause() throws Exception { + handler.postAndSync( + () -> { + player.playWhenReady = true; + player.playbackState = Player.STATE_READY; }); dispatchMediaKeyEvent(KeyEvent.KEYCODE_MEDIA_PLAY_PAUSE, false);