From 41a2f9a6b3243d38430402cdeab90d2866b9822d Mon Sep 17 00:00:00 2001 From: olly Date: Fri, 3 Dec 2021 17:52:35 +0000 Subject: [PATCH] Simplify threading for media3 session callbacks This change removes the requirement that callback implementations need to be able to handle two specific callbacks being called on two different threads. PiperOrigin-RevId: 413958545 --- .../media3/session/MediaLibraryService.java | 10 +++-- .../MediaLibraryServiceLegacyStub.java | 32 ++++++++------- .../androidx/media3/session/MediaSession.java | 6 ++- .../MediaSessionServiceLegacyStub.java | 39 ++++++++++++++----- 4 files changed, 57 insertions(+), 30 deletions(-) diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaLibraryService.java b/libraries/session/src/main/java/androidx/media3/session/MediaLibraryService.java index 1708aa7488..e3b7a715d7 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaLibraryService.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaLibraryService.java @@ -148,10 +148,12 @@ public abstract class MediaLibraryService extends MediaSessionService { * MediaItem#mediaId}. The media id is required for the browser to get the children under the * root. * - *

Interoperability: This method may be called on the main thread regardless of the - * application thread if legacy {@link android.support.v4.media.MediaBrowserCompat} requested - * a {@link androidx.media.MediaBrowserServiceCompat.BrowserRoot}. In this case, you must - * return a completed future. + *

Interoperability: If this callback is called because a legacy {@link + * android.support.v4.media.MediaBrowserCompat} has requested a {@link + * androidx.media.MediaBrowserServiceCompat.BrowserRoot}, then the main thread may be blocked + * until the returned future is done. If your service may be queried by a legacy {@link + * android.support.v4.media.MediaBrowserCompat}, you should ensure that the future completes + * quickly to avoid blocking the main thread for a long period of time. * * @param session The session for this event. * @param browser The browser information. diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaLibraryServiceLegacyStub.java b/libraries/session/src/main/java/androidx/media3/session/MediaLibraryServiceLegacyStub.java index 8f874b1459..2687c75cca 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaLibraryServiceLegacyStub.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaLibraryServiceLegacyStub.java @@ -35,6 +35,7 @@ import androidx.core.util.ObjectsCompat; import androidx.media.MediaBrowserServiceCompat; import androidx.media.MediaSessionManager.RemoteUserInfo; import androidx.media3.common.MediaItem; +import androidx.media3.common.util.ConditionVariable; import androidx.media3.common.util.Log; import androidx.media3.common.util.Util; import androidx.media3.session.MediaLibraryService.LibraryParams; @@ -49,6 +50,7 @@ import java.util.List; import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; +import java.util.concurrent.atomic.AtomicReference; /** * Implementation of {@link MediaBrowserServiceCompat} for interoperability between {@link @@ -89,22 +91,24 @@ import java.util.concurrent.Future; @Nullable LibraryParams params = MediaUtils.convertToLibraryParams(librarySessionImpl.getContext(), rootHints); - // Call onGetLibraryRoot() directly instead of posting to the application thread not to block - // the main thread as MediaBrowserServiceCompat requires to return browser root here. - // onGetLibraryRoot() has documentation that it may be called on the main thread. - ListenableFuture> future = - checkNotNull( - librarySessionImpl - .getCallback() - .onGetLibraryRoot(librarySessionImpl.getInstance(), controller, params), - "onGetLibraryRoot must return non-null future"); - if (!future.isDone()) { - throw new RuntimeException( - "onGetLibraryRoot must return a completed future " + "for legacy MediaBrowserCompat"); - } + AtomicReference>> futureReference = + new AtomicReference<>(); + ConditionVariable haveFuture = new ConditionVariable(); + postOrRun( + librarySessionImpl.getApplicationHandler(), + () -> { + futureReference.set( + checkNotNull( + librarySessionImpl + .getCallback() + .onGetLibraryRoot(librarySessionImpl.getInstance(), controller, params), + "onGetLibraryRoot must return non-null future")); + haveFuture.open(); + }); @Nullable LibraryResult result = null; try { - result = checkNotNull(future.get(), "LibraryResult must not be null"); + haveFuture.block(); + result = checkNotNull(futureReference.get().get(), "LibraryResult must not be null"); } catch (CancellationException | ExecutionException | InterruptedException e) { Log.e(TAG, "Couldn't get a result from onGetLibraryRoot", e); } 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 fdf08ace0c..b145d60c78 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSession.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSession.java @@ -792,8 +792,10 @@ public class MediaSession { * #sendCustomCommand}, {@link #setCustomLayout}) will be ignored. Use {@link #onPostConnect} * for custom initialization of the controller instead. * - *

Interoperability: this callback may be called on the main thread, regardless of the - * application thread. + *

Interoperability: If a legacy controller is connecting to the session then this callback + * may block the main thread, even if it's called on a different application thread. If it's + * possible that legacy controllers will connect to the session, you should ensure that the + * callback returns quickly to avoid blocking the main thread for a long period of time. * * @param session The session for this event. * @param controller The controller information. diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaSessionServiceLegacyStub.java b/libraries/session/src/main/java/androidx/media3/session/MediaSessionServiceLegacyStub.java index eaea4a5d93..0e77087224 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionServiceLegacyStub.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionServiceLegacyStub.java @@ -15,6 +15,9 @@ */ package androidx.media3.session; +import static androidx.media3.common.util.Assertions.checkNotNull; +import static androidx.media3.common.util.Util.postOrRun; + import android.os.Bundle; import android.support.v4.media.MediaBrowserCompat.MediaItem; import android.support.v4.media.session.MediaSessionCompat; @@ -22,8 +25,11 @@ import androidx.annotation.Nullable; import androidx.media.MediaBrowserServiceCompat; import androidx.media.MediaSessionManager; import androidx.media.MediaSessionManager.RemoteUserInfo; +import androidx.media3.common.util.ConditionVariable; +import androidx.media3.common.util.Log; import androidx.media3.session.MediaSession.ControllerInfo; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; /** * Implementation of {@link MediaBrowserServiceCompat} for interoperability between {@link @@ -31,6 +37,8 @@ import java.util.List; */ /* package */ class MediaSessionServiceLegacyStub extends MediaBrowserServiceCompat { + private static final String TAG = "MSSLegacyStub"; + private final MediaSessionManager manager; private final MediaSession.MediaSessionImpl sessionImpl; private final ConnectedControllersManager connectedControllersManager; @@ -55,19 +63,30 @@ import java.util.List; String clientPackageName, int clientUid, @Nullable Bundle rootHints) { RemoteUserInfo info = getCurrentBrowserInfo(); MediaSession.ControllerInfo controller = createControllerInfo(info); - // Call onConnect() directly instead of posting to the application thread not to block the main - // thread as MediaBrowserServiceCompat requires to return browser root here. - // onConnect() has documentation that it may be called on the main thread. - MediaSession.ConnectionResult connectionResult = - sessionImpl.getCallback().onConnect(sessionImpl.getInstance(), controller); - if (!connectionResult.isAccepted) { + + AtomicReference resultReference = new AtomicReference<>(); + ConditionVariable haveResult = new ConditionVariable(); + postOrRun( + sessionImpl.getApplicationHandler(), + () -> { + resultReference.set( + checkNotNull( + sessionImpl.getCallback().onConnect(sessionImpl.getInstance(), controller), + "onConnect must return non-null future")); + haveResult.open(); + }); + try { + haveResult.block(); + } catch (InterruptedException e) { + Log.e(TAG, "Couldn't get a result from onConnect", e); + return null; + } + MediaSession.ConnectionResult result = resultReference.get(); + if (!result.isAccepted) { return null; } connectedControllersManager.addController( - info, - controller, - connectionResult.availableSessionCommands, - connectionResult.availablePlayerCommands); + info, controller, result.availableSessionCommands, result.availablePlayerCommands); // No library root, but keep browser compat connected to allow getting session. return MediaUtils.defaultBrowserRoot; }