diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaController.java b/libraries/session/src/main/java/androidx/media3/session/MediaController.java index 235ca6553f..7f09bd14ca 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaController.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaController.java @@ -98,8 +98,10 @@ import org.checkerframework.checker.initialization.qual.UnderInitialization; * {@link MediaSession} in it. {@link MediaSessionService} will provide a session to connect. * *

When you're done, use {@link #releaseFuture(Future)} or {@link #release()} to clean up - * resources. This also helps session service to be destroyed when there's no controller associated - * with it. + * resources. This also helps the session service to be destroyed when there's no controller + * associated with it. Releasing the controller will still deliver all pending commands sent to the + * session and only unbind from the session service once these commands have been handled, or after + * a timeout of {@link #RELEASE_UNBIND_TIMEOUT_MS}. * *

Threading Model

* @@ -161,6 +163,12 @@ import org.checkerframework.checker.initialization.qual.UnderInitialization; */ public class MediaController implements Player { + /** + * The timeout for handling pending commands after calling {@link #release()}. If the timeout is + * reached, the controller is unbound from the session service even if commands are still pending. + */ + @UnstableApi public static final long RELEASE_UNBIND_TIMEOUT_MS = 30_000; + private static final String TAG = "MediaController"; private static final String WRONG_THREAD_ERROR_MESSAGE = diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java index df7d2e5b00..aadc8a42d6 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java @@ -162,6 +162,8 @@ import org.checkerframework.checker.nullness.qual.NonNull; public static final String TAG = "MCImplBase"; + private static final long RELEASE_TIMEOUT_MS = 30_000; + private final MediaController instance; protected final SequencedFutureManager sequencedFutureManager; protected final MediaControllerStub controllerStub; @@ -315,14 +317,9 @@ import org.checkerframework.checker.nullness.qual.NonNull; return; } released = true; - if (serviceConnection != null) { - context.unbindService(serviceConnection); - serviceConnection = null; - } connectedToken = null; flushCommandQueueHandler.release(); this.iSession = null; - controllerStub.destroy(); if (iSession != null) { int seq = sequencedFutureManager.obtainNextSequenceNumber(); try { @@ -332,8 +329,16 @@ import org.checkerframework.checker.nullness.qual.NonNull; // No-op. } } - sequencedFutureManager.release(); listeners.release(); + sequencedFutureManager.lazyRelease( + RELEASE_TIMEOUT_MS, + () -> { + if (serviceConnection != null) { + context.unbindService(serviceConnection); + serviceConnection = null; + } + controllerStub.destroy(); + }); } @Override diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaControllerStub.java b/libraries/session/src/main/java/androidx/media3/session/MediaControllerStub.java index 1e27fd8e65..9e9fa0fc62 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaControllerStub.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaControllerStub.java @@ -29,6 +29,7 @@ import androidx.media3.common.util.Log; import androidx.media3.session.MediaLibraryService.LibraryParams; import java.lang.ref.WeakReference; import java.util.List; +import org.checkerframework.checker.nullness.qual.NonNull; /* package */ class MediaControllerStub extends IMediaController.Stub { @@ -41,7 +42,7 @@ import java.util.List; } @Override - public void onSessionResult(int seq, Bundle sessionResultBundle) { + public void onSessionResult(int sequenceNum, Bundle sessionResultBundle) { SessionResult result; try { result = SessionResult.CREATOR.fromBundle(sessionResultBundle); @@ -52,11 +53,11 @@ import java.util.List; // Don't post setting future result so the result can be obtained on the application looper. // For an example, {@code MediaController.setRating(rating).get()} wouldn't return if the // result is posted. - dispatchControllerTask(controller -> controller.setFutureResult(seq, result)); + setControllerFutureResult(sequenceNum, result); } @Override - public void onLibraryResult(int seq, Bundle libraryResultBundle) { + public void onLibraryResult(int sequenceNum, Bundle libraryResultBundle) { LibraryResult result; try { result = LibraryResult.UNKNOWN_TYPE_CREATOR.fromBundle(libraryResultBundle); @@ -67,8 +68,7 @@ import java.util.List; // Don't post setting future result so the result can be obtained on the application looper. // For an example, {@code MediaBrowser.getLibraryRoot(params).get()} wouldn't return if the // result is posted. - dispatchControllerTask( - (ControllerTask) browser -> browser.setFutureResult(seq, result)); + setControllerFutureResult(sequenceNum, result); } @Override @@ -246,16 +246,15 @@ import java.util.List; controller.clear(); } - private void dispatchControllerTask(ControllerTask task) { + private void setControllerFutureResult( + int sequenceNum, T futureResult) { long token = Binder.clearCallingIdentity(); try { @Nullable MediaControllerImplBase controller = this.controller.get(); - if (controller == null || controller.isReleased()) { + if (controller == null) { return; } - @SuppressWarnings("unchecked") - T castedController = (T) controller; - task.run(castedController); + controller.setFutureResult(sequenceNum, futureResult); } finally { Binder.restoreCallingIdentity(token); } diff --git a/libraries/session/src/main/java/androidx/media3/session/SequencedFutureManager.java b/libraries/session/src/main/java/androidx/media3/session/SequencedFutureManager.java index b468bee847..21fe8a0fa6 100644 --- a/libraries/session/src/main/java/androidx/media3/session/SequencedFutureManager.java +++ b/libraries/session/src/main/java/androidx/media3/session/SequencedFutureManager.java @@ -15,10 +15,14 @@ */ package androidx.media3.session; +import static androidx.media3.common.util.Assertions.checkNotNull; + +import android.os.Handler; import androidx.annotation.GuardedBy; import androidx.annotation.Nullable; import androidx.collection.ArrayMap; import androidx.media3.common.util.Log; +import androidx.media3.common.util.Util; import com.google.common.util.concurrent.AbstractFuture; import com.google.common.util.concurrent.ListenableFuture; import java.util.ArrayList; @@ -39,6 +43,17 @@ import org.checkerframework.checker.nullness.qual.NonNull; @GuardedBy("lock") private final ArrayMap> seqToFutureMap; + @GuardedBy("lock") + @Nullable + private Runnable pendingLazyReleaseCallback; + + @GuardedBy("lock") + @Nullable + private Handler releaseCallbackHandler; + + @GuardedBy("lock") + private boolean isReleased; + public SequencedFutureManager() { lock = new Object(); seqToFutureMap = new ArrayMap<>(); @@ -66,7 +81,11 @@ import org.checkerframework.checker.nullness.qual.NonNull; synchronized (lock) { int seq = obtainNextSequenceNumber(); SequencedFuture result = SequencedFuture.create(seq, resultWhenClosed); - seqToFutureMap.put(seq, result); + if (isReleased) { + result.setWithTheValueOfResultWhenClosed(); + } else { + seqToFutureMap.put(seq, result); + } return result; } } @@ -94,20 +113,42 @@ import org.checkerframework.checker.nullness.qual.NonNull; + result.getClass()); } } + if (pendingLazyReleaseCallback != null && seqToFutureMap.isEmpty()) { + release(); + } } } public void release() { List> pendingResults; synchronized (lock) { + isReleased = true; pendingResults = new ArrayList<>(seqToFutureMap.values()); seqToFutureMap.clear(); + if (pendingLazyReleaseCallback != null) { + checkNotNull(releaseCallbackHandler).post(pendingLazyReleaseCallback); + pendingLazyReleaseCallback = null; + releaseCallbackHandler = null; + } } for (SequencedFuture result : pendingResults) { result.setWithTheValueOfResultWhenClosed(); } } + public void lazyRelease(long timeoutMs, Runnable releaseCallback) { + synchronized (lock) { + Handler releaseCallbackHandler = Util.createHandlerForCurrentLooper(); + this.releaseCallbackHandler = releaseCallbackHandler; + pendingLazyReleaseCallback = releaseCallback; + if (seqToFutureMap.isEmpty()) { + release(); + } else { + releaseCallbackHandler.postDelayed(this::release, timeoutMs); + } + } + } + public static final class SequencedFuture extends AbstractFuture { private final int sequenceNumber; diff --git a/libraries/test_session_current/build.gradle b/libraries/test_session_current/build.gradle index 4d7ad668a4..429b89881a 100644 --- a/libraries/test_session_current/build.gradle +++ b/libraries/test_session_current/build.gradle @@ -38,6 +38,7 @@ dependencies { implementation project(modulePrefix + 'lib-session') implementation project(modulePrefix + 'test-session-common') implementation 'androidx.media:media:' + androidxMediaVersion + androidTestImplementation project(modulePrefix + 'lib-exoplayer') androidTestImplementation 'androidx.test.ext:junit:' + androidxTestJUnitVersion androidTestImplementation 'androidx.test.ext:truth:' + androidxTestTruthVersion androidTestImplementation 'androidx.test:core:' + androidxTestCoreVersion 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 fb6c95f615..6f83813127 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 @@ -25,8 +25,12 @@ import static org.junit.Assert.assertThrows; import android.content.ComponentName; import android.content.Context; import android.os.Bundle; +import android.os.Handler; import android.os.Looper; +import androidx.media3.common.MediaItem; import androidx.media3.common.Player; +import androidx.media3.common.util.ConditionVariable; +import androidx.media3.exoplayer.ExoPlayer; import androidx.media3.session.MediaSession.ControllerInfo; import androidx.media3.test.session.common.HandlerThreadTestRule; import androidx.media3.test.session.common.MainLooperTestRule; @@ -34,11 +38,14 @@ import androidx.media3.test.session.common.TestUtils; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.MediumTest; +import com.google.common.collect.ImmutableList; import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.SettableFuture; import java.util.ArrayList; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; +import java.util.concurrent.atomic.AtomicBoolean; import org.junit.After; import org.junit.Before; import org.junit.ClassRule; @@ -79,6 +86,65 @@ public class MediaSessionServiceTest { TestServiceRegistry.getInstance().cleanUp(); } + @Test + public void controllerRelease_keepsControllerBoundUntilCommandsHandled() throws Exception { + TestServiceRegistry testServiceRegistry = TestServiceRegistry.getInstance(); + ConditionVariable mediaItemsAdded = new ConditionVariable(); + AtomicBoolean controllerBoundWhenMediaItemsAdded = new AtomicBoolean(false); + testServiceRegistry.setOnGetSessionHandler( + controllerInfo -> { + // Save bound state at the point where media items are added and listeners are informed. + MockMediaSessionService service = + (MockMediaSessionService) testServiceRegistry.getServiceInstance(); + Player player = new ExoPlayer.Builder(service).build(); + player.addListener( + new Player.Listener() { + @Override + public void onEvents(Player player, Player.Events events) { + if (events.contains(Player.EVENT_TIMELINE_CHANGED) + && !player.getCurrentTimeline().isEmpty()) { + controllerBoundWhenMediaItemsAdded.set(service.hasBoundController()); + mediaItemsAdded.open(); + } + } + }); + // Add short delay for resolving media items. + return new MediaSession.Builder(service, player) + .setCallback( + new MediaSession.Callback() { + @Override + public ListenableFuture> onAddMediaItems( + MediaSession mediaSession, + ControllerInfo controller, + List mediaItems) { + SettableFuture> future = SettableFuture.create(); + MediaItem playableItem = + mediaItems.get(0).buildUpon().setUri("https://test.test").build(); + new Handler(Looper.myLooper()) + .postDelayed( + () -> future.set(ImmutableList.of(playableItem)), + /* delayMillis= */ 500); + return future; + } + }) + .build(); + }); + RemoteMediaController controller = + controllerTestRule.createRemoteController( + token, /* waitForConnection= */ true, /* connectionHints= */ null); + MockMediaSessionService service = + (MockMediaSessionService) testServiceRegistry.getServiceInstance(); + + // Add items and release controller immediately. + controller.addMediaItem(new MediaItem.Builder().setMediaId("media_id").build()); + controller.release(); + + // Assert controller is still bound when command is fully handled and unbound after that. + mediaItemsAdded.block(TIMEOUT_MS); + assertThat(controllerBoundWhenMediaItemsAdded.get()).isEqualTo(true); + service.blockUntilAllControllersUnbind(TIMEOUT_MS); + } + /** * Tests whether {@link MediaSessionService#onGetSession(ControllerInfo)} is called when * controller tries to connect, with the proper arguments. diff --git a/libraries/test_session_current/src/main/java/androidx/media3/session/MockMediaSessionService.java b/libraries/test_session_current/src/main/java/androidx/media3/session/MockMediaSessionService.java index 2ccf5acfc1..d3bc2aec82 100644 --- a/libraries/test_session_current/src/main/java/androidx/media3/session/MockMediaSessionService.java +++ b/libraries/test_session_current/src/main/java/androidx/media3/session/MockMediaSessionService.java @@ -17,11 +17,17 @@ package androidx.media3.session; import static androidx.media3.test.session.common.CommonConstants.SUPPORT_APP_PACKAGE_NAME; +import android.content.Intent; import android.os.HandlerThread; +import android.os.IBinder; import android.text.TextUtils; +import androidx.annotation.Nullable; +import androidx.media3.common.util.ConditionVariable; import androidx.media3.common.util.UnstableApi; import androidx.media3.common.util.Util; import androidx.media3.session.MediaSession.ControllerInfo; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicInteger; /** A mock MediaSessionService */ @UnstableApi @@ -29,8 +35,37 @@ public class MockMediaSessionService extends MediaSessionService { /** ID of the session that this service will create. */ public static final String ID = "TestSession"; + private final AtomicInteger boundControllerCount; + public MediaSession session; + private HandlerThread handlerThread; + private ConditionVariable allControllersUnbound; + + public MockMediaSessionService() { + boundControllerCount = new AtomicInteger(/* initialValue= */ 0); + allControllersUnbound = new ConditionVariable(); + allControllersUnbound.open(); + } + + /** Returns whether at least one controller is bound to this service. */ + public boolean hasBoundController() { + return !allControllersUnbound.isOpen(); + } + + /** + * Blocks until all bound controllers unbind. + * + * @param timeoutMs The block timeout in milliseconds. + * @throws TimeoutException If the block timed out. + * @throws InterruptedException If the block was interrupted. + */ + public void blockUntilAllControllersUnbind(long timeoutMs) + throws TimeoutException, InterruptedException { + if (!allControllersUnbound.block(timeoutMs)) { + throw new TimeoutException(); + } + } @Override public void onCreate() { @@ -40,6 +75,21 @@ public class MockMediaSessionService extends MediaSessionService { handlerThread.start(); } + @Override + public IBinder onBind(@Nullable Intent intent) { + boundControllerCount.incrementAndGet(); + allControllersUnbound.close(); + return super.onBind(intent); + } + + @Override + public boolean onUnbind(Intent intent) { + if (boundControllerCount.decrementAndGet() == 0) { + allControllersUnbound.open(); + } + return super.onUnbind(intent); + } + @Override public void onDestroy() { super.onDestroy();