From 073ee8a1d087fbf09141981589d3e9fae09677fc Mon Sep 17 00:00:00 2001 From: christosts Date: Wed, 6 Dec 2023 09:49:44 -0800 Subject: [PATCH] Move instantiation of CompositingVideoSinkProvider This change moves the instantiation of the CompositingVideoSinkProvider out of MediaCodecVideoRenderer so that the composition preview player can re-use the CompositingVideoSinkProvider instance for driving the rendering of images. The main point of the change is the ownership of the VideoFrameReleaseControl, which decides when a frame should be rendered and so far was owned by the MediaCodecVideoRenderer. With this change, in the context of composition preview, the VideoFrameReleaseControl is no longer owned by MediaCodecVideoRenderer, but provided to it. This way, the CompositingVideoSinkProvider instance, hence the VideoFrameReleaseControl can be re-used to funnel images into the video pipeline and render the pipeline from elsewhere (and not MediaCodecVideoRenderer). PiperOrigin-RevId: 588459007 --- RELEASENOTES.md | 7 ++ .../video/CompositingVideoSinkProvider.java | 81 +++++++++++++++---- .../video/MediaCodecVideoRenderer.java | 41 +++++----- .../video/VideoFrameReleaseControl.java | 6 +- .../exoplayer/video/VideoSinkProvider.java | 6 ++ .../CompositingVideoSinkProviderTest.java | 9 --- 6 files changed, 103 insertions(+), 47 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 3b661f0c06..f61c591b73 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -56,6 +56,13 @@ Google TV, and Lenovo M10 FHD Plus that causes 60fps AVC streams to be marked as unsupported ([#693](https://github.com/androidx/media/issues/693)). + * Change the `MediaCodecVideoRenderer` constructor that takes a + `VideoFrameProcessor.Factory` argument and replace it with a constructor + that takes a `VideoSinkProvider` argument. Apps that want to inject a + custom `VideoFrameProcessor.Factory` can instantiate a + `CompositingVideoSinkProvider` that uses the custom + `VideoFrameProcessor.Factory` and pass the video sink provider to + `MediaCodecVideoRenderer`. * Text: * Fix serialization of bitmap cues to resolve `Tried to marshall a Parcel that contained Binder objects` error when using diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/CompositingVideoSinkProvider.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/CompositingVideoSinkProvider.java index 87ffb8b6ae..917563f754 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/CompositingVideoSinkProvider.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/CompositingVideoSinkProvider.java @@ -46,6 +46,7 @@ import androidx.media3.common.util.TimestampIterator; import androidx.media3.common.util.UnstableApi; import androidx.media3.common.util.Util; import androidx.media3.exoplayer.ExoPlaybackException; +import androidx.media3.exoplayer.video.VideoFrameReleaseControl.FrameTimingEvaluator; import com.google.common.base.Supplier; import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableList; @@ -61,7 +62,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; /** Handles composition of video sinks. */ @UnstableApi -/* package */ final class CompositingVideoSinkProvider +public final class CompositingVideoSinkProvider implements VideoSinkProvider, VideoGraph.Listener, VideoFrameRenderControl.FrameRenderer { /** A builder for {@link CompositingVideoSinkProvider} instances. */ @@ -112,6 +113,21 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; /** * Sets the {@link VideoFrameReleaseControl} that will be used. * + *

By default, a {@link VideoFrameReleaseControl} will be used with a {@link + * FrameTimingEvaluator} implementation which: + * + *

+ * * @param videoFrameReleaseControl The {@link VideoFrameReleaseControl}. * @return This builder, for convenience. */ @@ -123,10 +139,6 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; /** * Builds the {@link CompositingVideoSinkProvider}. * - *

A {@link VideoFrameReleaseControl} must be set with {@link - * #setVideoFrameReleaseControl(VideoFrameReleaseControl)} otherwise this method throws {@link - * IllegalStateException}. - * *

This method must be called at most once and will throw an {@link IllegalStateException} if * it has already been called. */ @@ -140,6 +152,11 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; previewingVideoGraphFactory = new ReflectivePreviewingSingleInputVideoGraphFactory(videoFrameProcessorFactory); } + if (videoFrameReleaseControl == null) { + videoFrameReleaseControl = + new VideoFrameReleaseControl( + context, new CompositionFrameTimingEvaluator(), /* allowedJoiningTimeMs= */ 0); + } CompositingVideoSinkProvider compositingVideoSinkProvider = new CompositingVideoSinkProvider(this); built = true; @@ -147,6 +164,41 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; } } + /** The time threshold, in microseconds, after which a frame is considered late. */ + public static final long FRAME_LATE_THRESHOLD_US = -30_000; + + /** + * The maximum elapsed time threshold, in microseconds, since last releasing a frame after which a + * frame can be force released. + */ + public static final long FRAME_RELEASE_THRESHOLD_US = 100_000; + + /** A {@link FrameTimingEvaluator} for composition frames. */ + private static final class CompositionFrameTimingEvaluator implements FrameTimingEvaluator { + + @Override + public boolean shouldForceReleaseFrame(long earlyUs, long elapsedSinceLastReleaseUs) { + return earlyUs < FRAME_LATE_THRESHOLD_US + && elapsedSinceLastReleaseUs > FRAME_RELEASE_THRESHOLD_US; + } + + @Override + public boolean shouldDropFrame(long earlyUs, long elapsedRealtimeUs, boolean isLastFrame) { + return earlyUs < FRAME_LATE_THRESHOLD_US && !isLastFrame; + } + + @Override + public boolean shouldIgnoreFrame( + long earlyUs, + long positionUs, + long elapsedRealtimeUs, + boolean isLastFrame, + boolean treatDroppedBuffersAsSkipped) { + // TODO b/293873191 - Handle very late buffers and drop to key frame. + return false; + } + } + private static final Executor NO_OP_EXECUTOR = runnable -> {}; private final Context context; @@ -232,13 +284,10 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; if (released) { return; } - if (handler != null) { handler.removeCallbacksAndMessages(/* token= */ null); } - if (videoSinkImpl != null) { - videoSinkImpl.release(); - } + if (videoGraph != null) { videoGraph.release(); } @@ -298,6 +347,11 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; this.videoFrameMetadataListener = videoFrameMetadataListener; } + @Override + public VideoFrameReleaseControl getVideoFrameReleaseControl() { + return videoFrameReleaseControl; + } + @Override public void setClock(Clock clock) { checkState(!isInitialized()); @@ -356,7 +410,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @Override public void renderFrame( - long renderTimeNs, long bufferPresentationTimeUs, long streamOffsetUs, boolean isFirstFrame) { + long renderTimeNs, long presentationTimeUs, long streamOffsetUs, boolean isFirstFrame) { if (isFirstFrame && listenerExecutor != NO_OP_EXECUTOR) { VideoSinkImpl videoSink = checkStateNotNull(videoSinkImpl); VideoSink.Listener currentListener = this.listener; @@ -367,7 +421,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; // onVideoSizeChanged is announced after the first frame is available for rendering. Format format = outputFormat == null ? new Format.Builder().build() : outputFormat; videoFrameMetadataListener.onVideoFrameAboutToBeRendered( - /* presentationTimeUs= */ bufferPresentationTimeUs - streamOffsetUs, + /* presentationTimeUs= */ presentationTimeUs - streamOffsetUs, clock.nanoTime(), format, /* mediaFormat= */ null); @@ -619,11 +673,6 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; // Other methods - /** Releases the video sink. */ - public void release() { - videoFrameProcessor.release(); - } - /** Sets the {@linkplain Effect video effects}. */ public void setVideoEffects(List videoEffects) { setPendingVideoEffects(videoEffects); diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/MediaCodecVideoRenderer.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/MediaCodecVideoRenderer.java index 5a555c2275..9ecb26171b 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/MediaCodecVideoRenderer.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/MediaCodecVideoRenderer.java @@ -51,7 +51,6 @@ import androidx.media3.common.Effect; import androidx.media3.common.Format; import androidx.media3.common.MimeTypes; import androidx.media3.common.PlaybackException; -import androidx.media3.common.VideoFrameProcessor; import androidx.media3.common.VideoSize; import androidx.media3.common.util.Clock; import androidx.media3.common.util.Log; @@ -343,7 +342,7 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer eventListener, maxDroppedFramesToNotify, assumedMinimumCodecOperatingRate, - /* videoFrameProcessorFactory= */ null); + /* videoSinkProvider= */ null); } /** @@ -366,8 +365,12 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer * @param assumedMinimumCodecOperatingRate A codec operating rate that all codecs instantiated by * this renderer are assumed to meet implicitly (i.e. without the operating rate being set * explicitly using {@link MediaFormat#KEY_OPERATING_RATE}). - * @param videoFrameProcessorFactory The {@link VideoFrameProcessor.Factory} applied on video - * output. {@code null} means a default implementation will be applied. + * @param videoSinkProvider The {@link VideoSinkProvider} that will used be used for applying + * video effects also providing the {@linkplain + * VideoSinkProvider#getVideoFrameReleaseControl() VideoFrameReleaseControl} for releasing + * video frames. If {@code null}, the {@link CompositingVideoSinkProvider} with its default + * configuration will be used, and the renderer will drive releasing of video frames by + * itself. */ public MediaCodecVideoRenderer( Context context, @@ -379,7 +382,7 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer @Nullable VideoRendererEventListener eventListener, int maxDroppedFramesToNotify, float assumedMinimumCodecOperatingRate, - @Nullable VideoFrameProcessor.Factory videoFrameProcessorFactory) { + @Nullable VideoSinkProvider videoSinkProvider) { super( C.TRACK_TYPE_VIDEO, codecAdapterFactory, @@ -388,20 +391,20 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer assumedMinimumCodecOperatingRate); this.maxDroppedFramesToNotify = maxDroppedFramesToNotify; this.context = context.getApplicationContext(); - @SuppressWarnings("nullness:assignment") - VideoFrameReleaseControl.@Initialized FrameTimingEvaluator thisRef = this; - videoFrameReleaseControl = - new VideoFrameReleaseControl( - this.context, /* frameTimingEvaluator= */ thisRef, allowedJoiningTimeMs); - videoFrameReleaseInfo = new VideoFrameReleaseControl.FrameReleaseInfo(); eventDispatcher = new EventDispatcher(eventHandler, eventListener); - CompositingVideoSinkProvider.Builder compositingVideoSinkProvider = - new CompositingVideoSinkProvider.Builder(context) - .setVideoFrameReleaseControl(videoFrameReleaseControl); - if (videoFrameProcessorFactory != null) { - compositingVideoSinkProvider.setVideoFrameProcessorFactory(videoFrameProcessorFactory); + if (videoSinkProvider == null) { + @SuppressWarnings("nullness:assignment") + VideoFrameReleaseControl.@Initialized FrameTimingEvaluator thisRef = this; + videoSinkProvider = + new CompositingVideoSinkProvider.Builder(this.context) + .setVideoFrameReleaseControl( + new VideoFrameReleaseControl( + this.context, /* frameTimingEvaluator= */ thisRef, allowedJoiningTimeMs)) + .build(); } - videoSinkProvider = compositingVideoSinkProvider.build(); + this.videoSinkProvider = videoSinkProvider; + this.videoFrameReleaseControl = this.videoSinkProvider.getVideoFrameReleaseControl(); + videoFrameReleaseInfo = new VideoFrameReleaseControl.FrameReleaseInfo(); deviceNeedsNoPostProcessWorkaround = deviceNeedsNoPostProcessWorkaround(); scalingMode = C.VIDEO_SCALING_MODE_DEFAULT; decodedVideoSize = VideoSize.UNKNOWN; @@ -1107,10 +1110,6 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer hasEffects = true; } - protected final VideoSinkProvider getVideoSinkProvider() { - return videoSinkProvider; - } - @Override protected void onCodecInitialized( String name, diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoFrameReleaseControl.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoFrameReleaseControl.java index f0112f1c52..dc9dcd933c 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoFrameReleaseControl.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoFrameReleaseControl.java @@ -36,7 +36,8 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; /** Controls the releasing of video frames. */ -/* package */ final class VideoFrameReleaseControl { +@UnstableApi +public final class VideoFrameReleaseControl { /** * The frame release action returned by {@link #getFrameReleaseAction(long, long, long, long, @@ -181,6 +182,9 @@ import java.lang.annotation.Target; * Creates an instance. * * @param applicationContext The application context. + * @param frameTimingEvaluator The {@link FrameTimingEvaluator} that will assist in {@linkplain + * #getFrameReleaseAction(long, long, long, long, boolean, FrameReleaseInfo)} frame release + * actions}. * @param allowedJoiningTimeMs The maximum duration in milliseconds for which the renderer can * attempt to seamlessly join an ongoing playback. */ diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoSinkProvider.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoSinkProvider.java index e0808df4d2..e90eae9db2 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoSinkProvider.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoSinkProvider.java @@ -80,6 +80,12 @@ public interface VideoSinkProvider { /** Sets a {@link VideoFrameMetadataListener} which is used in the returned {@link VideoSink}. */ void setVideoFrameMetadataListener(VideoFrameMetadataListener videoFrameMetadataListener); + /** + * Returns the {@link VideoFrameReleaseControl} that will be used for releasing of video frames + * during rendering. + */ + VideoFrameReleaseControl getVideoFrameReleaseControl(); + /** * Sets the {@link Clock} that the provider should use internally. * diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/CompositingVideoSinkProviderTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/CompositingVideoSinkProviderTest.java index ead017c07b..d2ba7e4616 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/CompositingVideoSinkProviderTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/CompositingVideoSinkProviderTest.java @@ -42,15 +42,6 @@ import org.mockito.Mockito; @RunWith(AndroidJUnit4.class) public final class CompositingVideoSinkProviderTest { - @Test - public void builder_withoutVideoFrameReleaseControl_throws() { - assertThrows( - IllegalStateException.class, - () -> - new CompositingVideoSinkProvider.Builder(ApplicationProvider.getApplicationContext()) - .build()); - } - @Test public void builder_calledMultipleTimes_throws() { CompositingVideoSinkProvider.Builder builder =