From b2ba5da09b05d7910db820f92375569e29a7679a Mon Sep 17 00:00:00 2001 From: dancho Date: Mon, 2 Dec 2024 06:06:06 -0800 Subject: [PATCH] Implement cancellation of FrameExtractor.getFrame Previously cancelling one FrameExtractor.getFrame ListenableFuture caused the following requests to fail. Implement ability to cancel Futures, and correctly issue next player seek commands after all past requests complete. Removes uses of SettableFuture, and replaces them with CallbackToFutureAdapter. Adds a dependency on androidx.concurrent:concurrent-futures PiperOrigin-RevId: 701941403 --- libraries/transformer/build.gradle | 1 + .../transformer/FrameExtractorTest.java | 31 ++++ .../ExperimentalFrameExtractor.java | 161 ++++++++++-------- 3 files changed, 120 insertions(+), 73 deletions(-) diff --git a/libraries/transformer/build.gradle b/libraries/transformer/build.gradle index 9eb3f7277e..1104fbb8d1 100644 --- a/libraries/transformer/build.gradle +++ b/libraries/transformer/build.gradle @@ -48,6 +48,7 @@ android { dependencies { implementation 'androidx.annotation:annotation:' + androidxAnnotationVersion + implementation 'androidx.concurrent:concurrent-futures:1.2.0' implementation project(modulePrefix + 'lib-datasource') implementation project(modulePrefix + 'lib-container') api project(modulePrefix + 'lib-exoplayer') diff --git a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/FrameExtractorTest.java b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/FrameExtractorTest.java index cfd21a40a3..9ed8c6b4cf 100644 --- a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/FrameExtractorTest.java +++ b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/FrameExtractorTest.java @@ -45,6 +45,7 @@ import com.google.common.util.concurrent.ListenableFuture; import java.util.ArrayList; import java.util.List; import java.util.Locale; +import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicReference; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @@ -409,4 +410,34 @@ public class FrameExtractorTest { .renderedOutputBufferCount) .isEqualTo(1); } + + @Test + public void extractFrame_randomAccessWithCancellation_returnsCorrectFrames() throws Exception { + frameExtractor = + new ExperimentalFrameExtractor( + context, + new ExperimentalFrameExtractor.Configuration.Builder().build(), + MediaItem.fromUri(FILE_PATH), + /* effects= */ ImmutableList.of()); + + ListenableFuture frame5 = frameExtractor.getFrame(/* positionMs= */ 5_000); + ListenableFuture frame3 = frameExtractor.getFrame(/* positionMs= */ 3_000); + ListenableFuture frame7 = frameExtractor.getFrame(/* positionMs= */ 7_000); + ListenableFuture frame2 = frameExtractor.getFrame(/* positionMs= */ 2_000); + ListenableFuture frame8 = frameExtractor.getFrame(/* positionMs= */ 8_000); + frame5.cancel(/* mayInterruptIfRunning= */ false); + frame7.cancel(/* mayInterruptIfRunning= */ false); + + assertThat(frame3.get(TIMEOUT_SECONDS, SECONDS).presentationTimeMs).isEqualTo(3_032); + assertThat(frame2.get(TIMEOUT_SECONDS, SECONDS).presentationTimeMs).isEqualTo(2_032); + assertThat(frame8.get(TIMEOUT_SECONDS, SECONDS).presentationTimeMs).isEqualTo(8_031); + assertThrows(CancellationException.class, () -> frame5.get(TIMEOUT_SECONDS, SECONDS)); + assertThrows(CancellationException.class, () -> frame7.get(TIMEOUT_SECONDS, SECONDS)); + assertThat( + frameExtractor + .getDecoderCounters() + .get(TIMEOUT_SECONDS, SECONDS) + .renderedOutputBufferCount) + .isEqualTo(4); + } } diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/ExperimentalFrameExtractor.java b/libraries/transformer/src/main/java/androidx/media3/transformer/ExperimentalFrameExtractor.java index adc7b6856d..a711367401 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/ExperimentalFrameExtractor.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/ExperimentalFrameExtractor.java @@ -44,6 +44,7 @@ import androidx.annotation.CallSuper; import androidx.annotation.Nullable; import androidx.annotation.RequiresApi; import androidx.annotation.VisibleForTesting; +import androidx.concurrent.futures.CallbackToFutureAdapter; import androidx.media3.common.Effect; import androidx.media3.common.Format; import androidx.media3.common.GlObjectsProvider; @@ -220,18 +221,19 @@ public final class ExperimentalFrameExtractor implements AnalyticsListener { private final AtomicBoolean extractedFrameNeedsRendering; /** - * A {@link SettableFuture} representing the frame currently being extracted. Accessed on both the - * {@linkplain ExoPlayer#getApplicationLooper() ExoPlayer application thread}, and the video - * effects GL thread. + * A {@link CallbackToFutureAdapter.Completer} corresponding to the frame currently being + * extracted. Accessed on both the {@linkplain ExoPlayer#getApplicationLooper() ExoPlayer + * application thread}, and the video effects GL thread. */ - private final AtomicReference<@NullableType SettableFuture> - frameBeingExtractedFutureAtomicReference; + private final AtomicReference> + frameBeingExtractedCompleterAtomicReference; /** - * The last {@link SettableFuture} returned by {@link #getFrame(long)}. Accessed on the frame - * extractor application thread. + * A {@link ListenableFuture} that completes when all previous {@link #getFrame(long)} requests + * complete. Upon completion, the result corresponds to the last request to {@link + * #getFrame(long)}. */ - private SettableFuture lastRequestedFrameFuture; + private ListenableFuture lastRequestedFrameFuture; /** * The last {@link Frame} that was extracted successfully. Accessed on the {@linkplain @@ -270,23 +272,28 @@ public final class ExperimentalFrameExtractor implements AnalyticsListener { .build(); playerApplicationThreadHandler = new Handler(player.getApplicationLooper()); extractedFrameNeedsRendering = new AtomicBoolean(); - lastRequestedFrameFuture = SettableFuture.create(); // TODO: b/350498258 - Extracting the first frame is a workaround for ExoPlayer.setVideoEffects // returning incorrect timestamps if we seek the player before rendering starts from zero. - frameBeingExtractedFutureAtomicReference = new AtomicReference<>(lastRequestedFrameFuture); - // TODO: b/350498258 - Refactor this and remove declaring this reference as initialized - // to satisfy the nullness checker. - @SuppressWarnings("nullness:assignment") - @Initialized - ExperimentalFrameExtractor thisRef = this; - playerApplicationThreadHandler.post( - () -> { - player.addAnalyticsListener(thisRef); - player.setVideoEffects(buildVideoEffects(effects)); - player.setMediaItem(mediaItem); - player.setPlayWhenReady(false); - player.prepare(); - }); + frameBeingExtractedCompleterAtomicReference = new AtomicReference<>(null); + lastRequestedFrameFuture = + CallbackToFutureAdapter.getFuture( + completer -> { + frameBeingExtractedCompleterAtomicReference.set(completer); + // TODO: b/350498258 - Refactor this and remove declaring this reference as + // initialized to satisfy the nullness checker. + @SuppressWarnings("nullness:assignment") + @Initialized + ExperimentalFrameExtractor thisRef = this; + playerApplicationThreadHandler.post( + () -> { + player.addAnalyticsListener(thisRef); + player.setVideoEffects(thisRef.buildVideoEffects(effects)); + player.setMediaItem(mediaItem); + player.setPlayWhenReady(false); + player.prepare(); + }); + return "ExperimentalFrameExtractor constructor"; + }); } /** @@ -296,47 +303,55 @@ public final class ExperimentalFrameExtractor implements AnalyticsListener { * @return A {@link ListenableFuture} of the result. */ public ListenableFuture getFrame(long positionMs) { - SettableFuture frameSettableFuture = SettableFuture.create(); - // Process frameSettableFuture after lastRequestedFrameFuture completes. - // If lastRequestedFrameFuture is done, the callbacks are invoked immediately. - Futures.addCallback( - lastRequestedFrameFuture, - new FutureCallback() { - @Override - public void onSuccess(Frame result) { - playerApplicationThreadHandler.post( - () -> { - lastExtractedFrame = result; - @Nullable PlaybackException playerError; - if (player.isReleased()) { - playerError = - new PlaybackException( - "The player is already released", - null, - ERROR_CODE_FAILED_RUNTIME_CHECK); - } else { - playerError = player.getPlayerError(); - } - if (playerError != null) { - frameSettableFuture.setException(playerError); - } else { - checkState( - frameBeingExtractedFutureAtomicReference.compareAndSet( - null, frameSettableFuture)); - extractedFrameNeedsRendering.set(false); - player.seekTo(positionMs); - } - }); - } + ListenableFuture previousRequestedFrame = lastRequestedFrameFuture; + ListenableFuture frameListenableFuture = + CallbackToFutureAdapter.getFuture( + completer -> { + Futures.addCallback( + previousRequestedFrame, + new FutureCallback() { + @Override + public void onSuccess(Frame result) { + lastExtractedFrame = result; + processNext(positionMs, completer); + } - @Override - public void onFailure(Throwable t) { - frameSettableFuture.setException(t); - } - }, - directExecutor()); - lastRequestedFrameFuture = frameSettableFuture; - return lastRequestedFrameFuture; + @Override + public void onFailure(Throwable t) { + processNext(positionMs, completer); + } + }, + playerApplicationThreadHandler::post); + return "ExperimentalFrameExtractor.getFrame"; + }); + lastRequestedFrameFuture = + Futures.whenAllComplete(lastRequestedFrameFuture, frameListenableFuture) + .call(() -> Futures.getDone(frameListenableFuture), directExecutor()); + return frameListenableFuture; + } + + private void processNext(long positionMs, CallbackToFutureAdapter.Completer completer) { + // Cancellation listener is invoked instantaneously if the returned future is already cancelled. + AtomicBoolean cancelled = new AtomicBoolean(false); + completer.addCancellationListener(() -> cancelled.set(true), directExecutor()); + if (cancelled.get()) { + return; + } + @Nullable PlaybackException playerError; + if (player.isReleased()) { + playerError = + new PlaybackException( + "The player is already released", null, ERROR_CODE_FAILED_RUNTIME_CHECK); + } else { + playerError = player.getPlayerError(); + } + if (playerError != null) { + completer.setException(playerError); + } else { + checkState(frameBeingExtractedCompleterAtomicReference.compareAndSet(null, completer)); + extractedFrameNeedsRendering.set(false); + player.seekTo(positionMs); + } } /** @@ -365,10 +380,10 @@ public final class ExperimentalFrameExtractor implements AnalyticsListener { // Fail the next frame to be extracted. Errors will propagate to later pending requests via // Future callbacks. @Nullable - SettableFuture frameBeingExtractedFuture = - frameBeingExtractedFutureAtomicReference.getAndSet(null); - if (frameBeingExtractedFuture != null) { - frameBeingExtractedFuture.setException(error); + CallbackToFutureAdapter.Completer frameBeingExtractedCompleter = + frameBeingExtractedCompleterAtomicReference.getAndSet(null); + if (frameBeingExtractedCompleter != null) { + frameBeingExtractedCompleter.setException(error); } } @@ -381,9 +396,9 @@ public final class ExperimentalFrameExtractor implements AnalyticsListener { // If the seek resolves to the current position, the renderer position will not be reset // and extractedFrameNeedsRendering remains false. No frames are rendered. Repeat the // previously returned frame. - SettableFuture frameBeingExtractedFuture = - checkNotNull(frameBeingExtractedFutureAtomicReference.getAndSet(null)); - frameBeingExtractedFuture.set(checkNotNull(lastExtractedFrame)); + CallbackToFutureAdapter.Completer frameBeingExtractedCompleter = + checkNotNull(frameBeingExtractedCompleterAtomicReference.getAndSet(null)); + frameBeingExtractedCompleter.set(checkNotNull(lastExtractedFrame)); } } @@ -532,9 +547,9 @@ public final class ExperimentalFrameExtractor implements AnalyticsListener { } bitmap.copyPixelsFromBuffer(byteBuffer); - SettableFuture frameBeingExtractedFuture = - checkNotNull(frameBeingExtractedFutureAtomicReference.getAndSet(null)); - frameBeingExtractedFuture.set(new Frame(usToMs(presentationTimeUs), bitmap)); + CallbackToFutureAdapter.Completer frameBeingExtractedCompleter = + checkNotNull(frameBeingExtractedCompleterAtomicReference.getAndSet(null)); + frameBeingExtractedCompleter.set(new Frame(usToMs(presentationTimeUs), bitmap)); // Drop frame: do not call outputListener.onOutputFrameAvailable(). // Block effects pipeline: do not call inputListener.onReadyToAcceptInputFrame(). // The effects pipeline will unblock and receive new frames when flushed after a seek.