From e95b15100b1afd10fa320b9310de4e588b6f34ae Mon Sep 17 00:00:00 2001 From: jaewan Date: Sat, 15 Jan 2022 16:21:16 +0000 Subject: [PATCH] Use MediaController to control sessions This is a small refactoring toward merging MediaNotificationHandler and PlayerNotificationManager In detail, this CL includes following changes: - Use MediaController to dispatch commands to sessions in MediaSessionService, rather than media key events. - Use MediaController to monitor changes in MediaSession's underlying Player, rather than ForegroundServiceEventCallback. Removed the callback interface as well. PiperOrigin-RevId: 422049265 --- .../session/MediaNotificationHandler.java | 133 ++++++++++++------ .../androidx/media3/session/MediaSession.java | 25 ---- .../media3/session/MediaSessionImpl.java | 17 --- .../media3/session/MediaSessionService.java | 21 +-- .../session/PlayerNotificationManager.java | 3 +- .../session/MediaSessionServiceTest.java | 31 ++-- 6 files changed, 116 insertions(+), 114 deletions(-) diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaNotificationHandler.java b/libraries/session/src/main/java/androidx/media3/session/MediaNotificationHandler.java index f16f8d35e4..6eebfec37c 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaNotificationHandler.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaNotificationHandler.java @@ -11,7 +11,7 @@ * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and - * limitations under the License. + * limitations under the License */ package androidx.media3.session; @@ -30,30 +30,38 @@ import android.content.Context; import android.content.Intent; import android.graphics.Bitmap; import android.graphics.BitmapFactory; +import android.os.Handler; +import android.os.Looper; import android.support.v4.media.session.PlaybackStateCompat; import android.view.KeyEvent; -import androidx.annotation.GuardedBy; import androidx.annotation.Nullable; import androidx.core.app.NotificationCompat; import androidx.core.content.ContextCompat; import androidx.media.app.NotificationCompat.MediaStyle; import androidx.media3.common.MediaMetadata; +import androidx.media3.common.Player; import androidx.media3.common.util.Util; +import com.google.common.util.concurrent.ListenableFuture; +import java.util.HashMap; import java.util.List; -import java.util.WeakHashMap; +import java.util.Map; +import java.util.concurrent.CancellationException; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executor; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; /** * Class to provide default media notification for {@link MediaSessionService}, and set the service * as foreground/background according to the player state. */ -/* package */ class MediaNotificationHandler - implements MediaSession.ForegroundServiceEventCallback { +/* package */ class MediaNotificationHandler { private static final int NOTIFICATION_ID = 1001; private static final String NOTIFICATION_CHANNEL_ID = "default_channel_id"; - private final Object lock; private final MediaSessionService service; + private final Executor mainExecutor; private final NotificationManager notificationManager; private final String notificationChannelName; @@ -63,13 +71,14 @@ import java.util.WeakHashMap; private final NotificationCompat.Action skipToPrevAction; private final NotificationCompat.Action skipToNextAction; - @GuardedBy("lock") - private final WeakHashMap playerInfoMap; + private final Map> controllerMap; public MediaNotificationHandler(MediaSessionService service) { - lock = new Object(); this.service = service; + Handler mainHandler = new Handler(Looper.getMainLooper()); + mainExecutor = (runnable) -> Util.postOrRun(mainHandler, runnable); startSelfIntent = new Intent(service, service.getClass()); + controllerMap = new HashMap<>(); notificationManager = (NotificationManager) service.getSystemService(Context.NOTIFICATION_SERVICE); @@ -100,8 +109,45 @@ import java.util.WeakHashMap; android.R.drawable.ic_media_next, R.string.media3_controls_seek_to_next_description, ACTION_SKIP_TO_NEXT); + } - playerInfoMap = new WeakHashMap<>(); + public void addSession(MediaSession session) { + if (controllerMap.containsKey(session)) { + return; + } + MediaControllerListener listener = new MediaControllerListener(session); + ListenableFuture controllerFuture = + new MediaController.Builder(service, session.getToken()) + .setListener(listener) + .setApplicationLooper(Looper.getMainLooper()) + .buildAsync(); + controllerFuture.addListener( + () -> { + MediaController controller; + try { + controller = controllerFuture.get(/* time= */ 0, TimeUnit.MILLISECONDS); + } catch (CancellationException + | ExecutionException + | InterruptedException + | TimeoutException e) { + // MediaSession or MediaController is released too early. Stop monitoring session. + service.removeSession(session); + return; + } + if (controller != null) { + listener.onConnected(); + controller.addListener(listener); + } + }, + mainExecutor); + controllerMap.put(session, controllerFuture); + } + + public void removeSession(MediaSession session) { + ListenableFuture controllerFuture = controllerMap.remove(session); + if (controllerFuture != null) { + MediaController.releaseFuture(controllerFuture); + } } private void updateNotificationIfNeeded(MediaSession session) { @@ -124,8 +170,8 @@ import java.util.WeakHashMap; notification.extras.putParcelable(Notification.EXTRA_MEDIA_SESSION, fwkToken); } - PlayerInfo playerInfo = getPlayerInfoOfSession(session); - if (playerInfo.playWhenReady) { + Player player = session.getPlayer(); + if (player.getPlayWhenReady()) { ContextCompat.startForegroundService(service, startSelfIntent); service.startForeground(id, notification); } else { @@ -134,30 +180,11 @@ import java.util.WeakHashMap; } } - @Override - public void onPlayerInfoChanged( - MediaSession session, PlayerInfo oldPlayerInfo, PlayerInfo newPlayerInfo) { - synchronized (lock) { - playerInfoMap.put(session, newPlayerInfo); - } - if (Util.areEqual(oldPlayerInfo.mediaMetadata, newPlayerInfo.mediaMetadata) - && oldPlayerInfo.playWhenReady == newPlayerInfo.playWhenReady) { - return; - } - updateNotificationIfNeeded(session); - } - - @Override - public void onSessionReleased(MediaSession session) { - service.removeSession(session); - stopForegroundServiceIfNeeded(); - } - private void stopForegroundServiceIfNeeded() { List sessions = service.getSessions(); for (int i = 0; i < sessions.size(); i++) { - PlayerInfo playerInfo = getPlayerInfoOfSession(sessions.get(i)); - if (playerInfo.playWhenReady) { + Player player = sessions.get(i).getPlayer(); + if (player.getPlayWhenReady()) { return; } } @@ -169,7 +196,7 @@ import java.util.WeakHashMap; /** Creates a default media style notification for {@link MediaSessionService}. */ public MediaSessionService.MediaNotification onUpdateNotification(MediaSession session) { - PlayerInfo playerInfo = getPlayerInfoOfSession(session); + Player player = session.getPlayer(); ensureNotificationChannel(); @@ -178,7 +205,7 @@ import java.util.WeakHashMap; // TODO(b/193193926): Filter actions depending on the player's available commands. builder.addAction(skipToPrevAction); - if (playerInfo.playWhenReady) { + if (player.getPlayWhenReady()) { builder.addAction(pauseAction); } else { builder.addAction(playAction); @@ -186,7 +213,7 @@ import java.util.WeakHashMap; builder.addAction(skipToNextAction); // Set metadata info in the notification. - MediaMetadata metadata = playerInfo.mediaMetadata; + MediaMetadata metadata = player.getMediaMetadata(); builder.setContentTitle(metadata.title).setContentText(metadata.artist); if (metadata.artworkData != null) { Bitmap artworkBitmap = @@ -236,14 +263,6 @@ import java.util.WeakHashMap; } } - private PlayerInfo getPlayerInfoOfSession(MediaSession session) { - @Nullable PlayerInfo playerInfo; - synchronized (lock) { - playerInfo = playerInfoMap.get(session); - } - return playerInfo == null ? PlayerInfo.DEFAULT : playerInfo; - } - private static NotificationCompat.Action createNotificationAction( MediaSessionService service, int iconResId, @@ -271,4 +290,30 @@ import java.util.WeakHashMap; Util.SDK_INT >= 23 ? PendingIntent.FLAG_IMMUTABLE : 0); } } + + private final class MediaControllerListener implements MediaController.Listener, Player.Listener { + private final MediaSession session; + + public MediaControllerListener(MediaSession session) { + this.session = session; + } + + public void onConnected() { + updateNotificationIfNeeded(session); + } + + @Override + public void onEvents(Player player, Player.Events events) { + if (events.containsAny( + Player.EVENT_PLAY_WHEN_READY_CHANGED, Player.EVENT_MEDIA_METADATA_CHANGED)) { + updateNotificationIfNeeded(session); + } + } + + @Override + public void onDisconnected(MediaController controller) { + service.removeSession(session); + stopForegroundServiceIfNeeded(); + } + } } 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 2ff846577e..b314ee3781 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSession.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSession.java @@ -740,23 +740,6 @@ public class MediaSession { impl.setSessionPositionUpdateDelayMsOnHandler(updateDelayMs); } - /** - * Sets a {@link ForegroundServiceEventCallback} to the session. Should be called on the - * application looper of the underlying player. - */ - /* package */ void setForegroundServiceEventCallback( - ForegroundServiceEventCallback foregroundServiceEventCallback) { - impl.setForegroundServiceEventCallback(foregroundServiceEventCallback); - } - - /** - * Clears the {@link ForegroundServiceEventCallback} from the session. Should be called on the - * application looper of the underlying player. - */ - /* package */ void clearForegroundServiceEventCallback() { - impl.clearForegroundServiceEventCallback(); - } - private Uri getUri() { return impl.getUri(); } @@ -1066,14 +1049,6 @@ public class MediaSession { } } - /* package */ interface ForegroundServiceEventCallback { - - void onPlayerInfoChanged( - MediaSession session, PlayerInfo oldPlayerInfo, PlayerInfo newPlayerInfo); - - void onSessionReleased(MediaSession session); - } - /* package */ interface ControllerCb { default void onSessionResult(int seq, SessionResult result) throws RemoteException {} 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 50be173374..96126216f0 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java @@ -138,8 +138,6 @@ import org.checkerframework.checker.initialization.qual.Initialized; // Should be only accessed on the application looper private long sessionPositionUpdateDelayMs; - @Nullable private MediaSession.ForegroundServiceEventCallback foregroundServiceEventCallback; - public MediaSessionImpl( MediaSession instance, Context context, @@ -298,9 +296,6 @@ import org.checkerframework.checker.initialization.qual.Initialized; if (playerListener != null) { playerWrapper.removeListener(playerListener); } - if (foregroundServiceEventCallback != null) { - foregroundServiceEventCallback.onSessionReleased(instance); - } }); } catch (Exception e) { // Catch all exceptions to ensure the rest of this method to be executed as exceptions may be @@ -375,9 +370,6 @@ import org.checkerframework.checker.initialization.qual.Initialized; } private void dispatchOnPlayerInfoChanged(boolean excludeTimeline) { - if (foregroundServiceEventCallback != null) { - foregroundServiceEventCallback.onPlayerInfoChanged(instance, lastPlayerInfo, playerInfo); - } lastPlayerInfo = playerInfo; List controllers = @@ -550,15 +542,6 @@ import org.checkerframework.checker.initialization.qual.Initialized; this::notifyPeriodicSessionPositionInfoChangesOnHandler, updateDelayMs); } - protected void setForegroundServiceEventCallback( - MediaSession.ForegroundServiceEventCallback foregroundServiceEventCallback) { - this.foregroundServiceEventCallback = foregroundServiceEventCallback; - } - - protected void clearForegroundServiceEventCallback() { - foregroundServiceEventCallback = null; - } - @Nullable protected MediaSessionServiceLegacyStub getLegacyBrowserService() { synchronized (lock) { diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaSessionService.java b/libraries/session/src/main/java/androidx/media3/session/MediaSessionService.java index 8d7ec3eee3..148d616a64 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionService.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionService.java @@ -30,6 +30,7 @@ import android.os.Binder; import android.os.Bundle; import android.os.Handler; import android.os.IBinder; +import android.os.Looper; import android.os.RemoteException; import android.view.KeyEvent; import androidx.annotation.CallSuper; @@ -130,6 +131,7 @@ public abstract class MediaSessionService extends Service { private static final String TAG = "MSSImpl"; private final Object lock; + private final Handler mainHandler; @GuardedBy("lock") private final Map sessions; @@ -145,6 +147,7 @@ public abstract class MediaSessionService extends Service { /** Creates a service. */ public MediaSessionService() { lock = new Object(); + mainHandler = new Handler(Looper.getMainLooper()); sessions = new ArrayMap<>(); } @@ -225,18 +228,7 @@ public abstract class MediaSessionService extends Service { synchronized (lock) { handler = checkStateNotNull(notificationHandler); } - postOrRun( - session.getImpl().getApplicationHandler(), - () -> { - handler.onPlayerInfoChanged( - session, - /* oldPlayerInfo= */ PlayerInfo.DEFAULT, - /* newPlayerInfo= */ session - .getImpl() - .getPlayerWrapper() - .createPlayerInfoForBundling()); - session.setForegroundServiceEventCallback(handler); - }); + postOrRun(mainHandler, () -> handler.addSession(session)); } } @@ -250,11 +242,12 @@ public abstract class MediaSessionService extends Service { */ public final void removeSession(MediaSession session) { checkNotNull(session, "session must not be null"); + MediaNotificationHandler handler; synchronized (lock) { sessions.remove(session.getId()); + handler = checkStateNotNull(notificationHandler); } - postOrRun( - session.getImpl().getApplicationHandler(), session::clearForegroundServiceEventCallback); + postOrRun(mainHandler, () -> handler.removeSession(session)); } /** diff --git a/libraries/session/src/main/java/androidx/media3/session/PlayerNotificationManager.java b/libraries/session/src/main/java/androidx/media3/session/PlayerNotificationManager.java index 4222ab2c94..170fd5aaa7 100644 --- a/libraries/session/src/main/java/androidx/media3/session/PlayerNotificationManager.java +++ b/libraries/session/src/main/java/androidx/media3/session/PlayerNotificationManager.java @@ -1104,8 +1104,9 @@ public class PlayerNotificationManager { } else if (controller.getPlaybackState() == controller.STATE_ENDED) { controller.seekToDefaultPosition(controller.getCurrentWindowIndex()); } + controller.setPlayWhenReady(true); } else { - controller.pause(); + controller.setPlayWhenReady(false); } break; case COMMAND_SEEK_TO_PREVIOUS: 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 968a3fbae7..1cccd7da4e 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 @@ -85,6 +85,8 @@ public class MediaSessionServiceTest { */ @Test public void onGetSessionIsCalled() throws Exception { + Bundle testHints = new Bundle(); + testHints.putString("test_key", "test_value"); List controllerInfoList = new ArrayList<>(); CountDownLatch latch = new CountDownLatch(1); TestServiceRegistry.getInstance() @@ -92,19 +94,18 @@ public class MediaSessionServiceTest { new TestServiceRegistry.OnGetSessionHandler() { @Override public MediaSession onGetSession(ControllerInfo controllerInfo) { - controllerInfoList.add(controllerInfo); - latch.countDown(); + if (SUPPORT_APP_PACKAGE_NAME.equals(controllerInfo.getPackageName()) + && TestUtils.equals(testHints, controllerInfo.getConnectionHints())) { + controllerInfoList.add(controllerInfo); + latch.countDown(); + } return null; } }); - - Bundle testHints = new Bundle(); - testHints.putString("test_key", "test_value"); controllerTestRule.createRemoteController(token, /* waitForConnection= */ false, testHints); // onGetSession() should be called. assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue(); - assertThat(controllerInfoList.get(0).getPackageName()).isEqualTo(SUPPORT_APP_PACKAGE_NAME); assertThat(TestUtils.equals(controllerInfoList.get(0).getConnectionHints(), testHints)) .isTrue(); } @@ -117,6 +118,8 @@ public class MediaSessionServiceTest { */ @Test public void onGetSession_returnsSession() throws Exception { + Bundle testHints = new Bundle(); + testHints.putString("test_key", "test_value"); List controllerInfoList = new ArrayList<>(); CountDownLatch latch = new CountDownLatch(1); @@ -130,8 +133,11 @@ public class MediaSessionServiceTest { @Override public MediaSession.ConnectionResult onConnect( MediaSession session, ControllerInfo controller) { - controllerInfoList.add(controller); - latch.countDown(); + if (SUPPORT_APP_PACKAGE_NAME.equals(controller.getPackageName()) + && TestUtils.equals(testHints, controller.getConnectionHints())) { + controllerInfoList.add(controller); + latch.countDown(); + } return MediaSession.ConnectionResult.accept( SessionCommands.EMPTY, Player.Commands.EMPTY); } @@ -147,14 +153,11 @@ public class MediaSessionServiceTest { } }); - Bundle testHints = new Bundle(); - testHints.putString("test_key", "test_value"); RemoteMediaController controller = controllerTestRule.createRemoteController(token, true, testHints); // MediaSession.SessionCallback#onConnect() should be called. assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue(); - assertThat(controllerInfoList.get(0).getPackageName()).isEqualTo(SUPPORT_APP_PACKAGE_NAME); assertThat(TestUtils.equals(controllerInfoList.get(0).getConnectionHints(), testHints)) .isTrue(); @@ -288,6 +291,7 @@ public class MediaSessionServiceTest { service.removeSession(session); sessions = service.getSessions(); + assertThat(sessions.contains(session)).isFalse(); } @@ -302,8 +306,9 @@ public class MediaSessionServiceTest { assertThat(sessions.contains(session)).isTrue(); assertThat(sessions.size()).isEqualTo(2); threadTestRule.getHandler().postAndSync(session::release); - sessions = service.getSessions(); - assertThat(sessions.contains(session)).isFalse(); + // Wait until release of session is propagated. + MainLooperTestRule.runOnMainSync(() -> {}); + assertThat(service.getSessions()).doesNotContain(session); } private MediaSession createMediaSession(String id) {