From 9506445148677721deb33944665cf49c53cdbd4f Mon Sep 17 00:00:00 2001 From: kimvde Date: Wed, 22 May 2024 01:36:14 -0700 Subject: [PATCH] Remove VideoFrameReleaseControl setter from SinkProvider Move the parameter to the constructor instead. PiperOrigin-RevId: 636077477 --- .../video/CompositingVideoSinkProvider.java | 144 ++++++++---------- .../video/MediaCodecVideoRenderer.java | 9 +- .../video/VideoFrameReleaseControl.java | 6 +- .../media3/exoplayer/video/VideoSink.java | 2 - .../exoplayer/video/VideoSinkProvider.java | 17 +-- .../CompositingVideoSinkProviderTest.java | 35 ++--- .../media3/transformer/CompositionPlayer.java | 8 +- 7 files changed, 88 insertions(+), 133 deletions(-) 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 5e468015d4..5a837d37d5 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 @@ -71,8 +71,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; /** Handles composition of video sinks. */ @UnstableApi @RestrictTo({Scope.LIBRARY_GROUP}) -public final class CompositingVideoSinkProvider - implements VideoSinkProvider, VideoGraph.Listener, VideoFrameRenderControl.FrameRenderer { +public final class CompositingVideoSinkProvider implements VideoSinkProvider, VideoGraph.Listener { /** Listener for {@link CompositingVideoSinkProvider} events. */ public interface Listener { @@ -118,14 +117,16 @@ public final class CompositingVideoSinkProvider /** A builder for {@link CompositingVideoSinkProvider} instances. */ public static final class Builder { private final Context context; + private final VideoFrameReleaseControl videoFrameReleaseControl; private VideoFrameProcessor.@MonotonicNonNull Factory videoFrameProcessorFactory; private PreviewingVideoGraph.@MonotonicNonNull Factory previewingVideoGraphFactory; private boolean built; - /** Creates a builder with the supplied {@linkplain Context application context}. */ - public Builder(Context context) { + /** Creates a builder. */ + public Builder(Context context, VideoFrameReleaseControl videoFrameReleaseControl) { this.context = context.getApplicationContext(); + this.videoFrameReleaseControl = videoFrameReleaseControl; } /** @@ -198,12 +199,12 @@ public final class CompositingVideoSinkProvider private final Context context; private final VideoSinkImpl videoSinkImpl; + private final VideoFrameReleaseControl videoFrameReleaseControl; + private final VideoFrameRenderControl videoFrameRenderControl; private final PreviewingVideoGraph.Factory previewingVideoGraphFactory; private final CopyOnWriteArraySet listeners; private @MonotonicNonNull Clock clock; - private @MonotonicNonNull VideoFrameReleaseControl videoFrameReleaseControl; - private @MonotonicNonNull VideoFrameRenderControl videoFrameRenderControl; private @MonotonicNonNull Format outputFormat; private @MonotonicNonNull VideoFrameMetadataListener videoFrameMetadataListener; private @MonotonicNonNull HandlerWrapper handler; @@ -211,15 +212,16 @@ public final class CompositingVideoSinkProvider @Nullable private Pair currentSurfaceAndSize; private int pendingFlushCount; private @State int state; - private float playbackSpeed; private CompositingVideoSinkProvider(Builder builder) { context = builder.context; videoSinkImpl = new VideoSinkImpl(context); + videoFrameReleaseControl = builder.videoFrameReleaseControl; + videoFrameRenderControl = + new VideoFrameRenderControl(new FrameRendererImpl(), videoFrameReleaseControl); previewingVideoGraphFactory = checkStateNotNull(builder.previewingVideoGraphFactory); listeners = new CopyOnWriteArraySet<>(); state = STATE_CREATED; - playbackSpeed = 1f; addListener(videoSinkImpl); } @@ -244,16 +246,6 @@ public final class CompositingVideoSinkProvider // VideoSinkProvider methods @Override - public void setVideoFrameReleaseControl(VideoFrameReleaseControl videoFrameReleaseControl) { - checkState(!isInitialized()); - this.videoFrameReleaseControl = videoFrameReleaseControl; - videoFrameRenderControl = - new VideoFrameRenderControl(/* frameRenderer= */ this, videoFrameReleaseControl); - videoFrameRenderControl.setPlaybackSpeed(playbackSpeed); - } - - @Override - @Nullable public VideoFrameReleaseControl getVideoFrameReleaseControl() { return videoFrameReleaseControl; } @@ -306,7 +298,7 @@ public final class CompositingVideoSinkProvider @Override public void onOutputSizeChanged(int width, int height) { // We forward output size changes to render control even if we are still flushing. - checkStateNotNull(videoFrameRenderControl).onOutputSizeChanged(width, height); + videoFrameRenderControl.onOutputSizeChanged(width, height); } @Override @@ -315,8 +307,7 @@ public final class CompositingVideoSinkProvider // Ignore available frames while the sink provider is flushing return; } - checkStateNotNull(videoFrameRenderControl) - .onOutputFrameAvailableForRendering(presentationTimeUs); + videoFrameRenderControl.onOutputFrameAvailableForRendering(presentationTimeUs); } @Override @@ -331,50 +322,6 @@ public final class CompositingVideoSinkProvider } } - // FrameRenderer methods - - @Override - public void onVideoSizeChanged(VideoSize videoSize) { - outputFormat = - new Format.Builder() - .setWidth(videoSize.width) - .setHeight(videoSize.height) - .setSampleMimeType(MimeTypes.VIDEO_RAW) - .build(); - for (CompositingVideoSinkProvider.Listener listener : listeners) { - listener.onVideoSizeChanged(/* compositingVideoSinkProvider= */ this, videoSize); - } - } - - @Override - public void renderFrame( - long renderTimeNs, long presentationTimeUs, long streamOffsetUs, boolean isFirstFrame) { - if (isFirstFrame && currentSurfaceAndSize != null) { - for (CompositingVideoSinkProvider.Listener listener : listeners) { - listener.onFirstFrameRendered(/* compositingVideoSinkProvider= */ this); - } - } - if (videoFrameMetadataListener != null) { - // TODO b/292111083 - outputFormat is initialized after the first frame is rendered because - // onVideoSizeChanged is announced after the first frame is available for rendering. - Format format = outputFormat == null ? new Format.Builder().build() : outputFormat; - videoFrameMetadataListener.onVideoFrameAboutToBeRendered( - /* presentationTimeUs= */ presentationTimeUs - streamOffsetUs, - checkStateNotNull(clock).nanoTime(), - format, - /* mediaFormat= */ null); - } - checkStateNotNull(videoGraph).renderOutputFrame(renderTimeNs); - } - - @Override - public void dropFrame() { - for (CompositingVideoSinkProvider.Listener listener : listeners) { - listener.onFrameDropped(/* compositingVideoSinkProvider= */ this); - } - checkStateNotNull(videoGraph).renderOutputFrame(VideoFrameProcessor.DROP_OUTPUT_FRAME); - } - // Other public methods /** @@ -386,7 +333,7 @@ public final class CompositingVideoSinkProvider */ public void render(long positionUs, long elapsedRealtimeUs) throws ExoPlaybackException { if (pendingFlushCount == 0) { - checkStateNotNull(videoFrameRenderControl).render(positionUs, elapsedRealtimeUs); + videoFrameRenderControl.render(positionUs, elapsedRealtimeUs); } } @@ -405,7 +352,6 @@ public final class CompositingVideoSinkProvider private VideoFrameProcessor initialize(Format sourceFormat, Clock clock) throws VideoSink.VideoSinkException { checkState(state == STATE_CREATED); - checkState(videoFrameRenderControl != null && videoFrameReleaseControl != null); this.clock = clock; handler = clock.createHandler(checkStateNotNull(Looper.myLooper()), /* callback= */ null); @@ -451,17 +397,16 @@ public final class CompositingVideoSinkProvider // Update the surface on the video graph and the video frame release control together. SurfaceInfo surfaceInfo = surface != null ? new SurfaceInfo(surface, width, height) : null; videoGraph.setOutputSurfaceInfo(surfaceInfo); - checkNotNull(videoFrameReleaseControl).setOutputSurface(surface); + videoFrameReleaseControl.setOutputSurface(surface); } } private boolean isReady() { - return pendingFlushCount == 0 && checkStateNotNull(videoFrameRenderControl).isReady(); + return pendingFlushCount == 0 && videoFrameRenderControl.isReady(); } private boolean hasReleasedFrame(long presentationTimeUs) { - return pendingFlushCount == 0 - && checkStateNotNull(videoFrameRenderControl).hasReleasedFrame(presentationTimeUs); + return pendingFlushCount == 0 && videoFrameRenderControl.hasReleasedFrame(presentationTimeUs); } private void flush() { @@ -471,7 +416,7 @@ public final class CompositingVideoSinkProvider pendingFlushCount++; // Flush the render control now to ensure it has no data, eg calling isReady() must return false // and render() should not render any frames. - checkStateNotNull(videoFrameRenderControl).flush(); + videoFrameRenderControl.flush(); // Finish flushing after handling pending video graph callbacks to ensure video size changes // reach the video render control. checkStateNotNull(handler).post(this::flushInternal); @@ -486,7 +431,7 @@ public final class CompositingVideoSinkProvider throw new IllegalStateException(String.valueOf(pendingFlushCount)); } // Flush the render control again. - checkStateNotNull(videoFrameRenderControl).flush(); + videoFrameRenderControl.flush(); } private void setVideoFrameMetadataListener( @@ -495,15 +440,11 @@ public final class CompositingVideoSinkProvider } private void setPlaybackSpeed(float speed) { - this.playbackSpeed = speed; - if (videoFrameRenderControl != null) { - videoFrameRenderControl.setPlaybackSpeed(speed); - } + videoFrameRenderControl.setPlaybackSpeed(speed); } private void onStreamOffsetChange(long bufferPresentationTimeUs, long streamOffsetUs) { - checkStateNotNull(videoFrameRenderControl) - .onStreamOffsetChange(bufferPresentationTimeUs, streamOffsetUs); + videoFrameRenderControl.onStreamOffsetChange(bufferPresentationTimeUs, streamOffsetUs); } private static ColorInfo getAdjustedInputColorInfo(@Nullable ColorInfo inputColorInfo) { @@ -872,6 +813,51 @@ public final class CompositingVideoSinkProvider } } + private final class FrameRendererImpl implements VideoFrameRenderControl.FrameRenderer { + + @Override + public void onVideoSizeChanged(VideoSize videoSize) { + outputFormat = + new Format.Builder() + .setWidth(videoSize.width) + .setHeight(videoSize.height) + .setSampleMimeType(MimeTypes.VIDEO_RAW) + .build(); + for (CompositingVideoSinkProvider.Listener listener : listeners) { + listener.onVideoSizeChanged(CompositingVideoSinkProvider.this, videoSize); + } + } + + @Override + public void renderFrame( + long renderTimeNs, long presentationTimeUs, long streamOffsetUs, boolean isFirstFrame) { + if (isFirstFrame && currentSurfaceAndSize != null) { + for (CompositingVideoSinkProvider.Listener listener : listeners) { + listener.onFirstFrameRendered(CompositingVideoSinkProvider.this); + } + } + if (videoFrameMetadataListener != null) { + // TODO b/292111083 - outputFormat is initialized after the first frame is rendered because + // onVideoSizeChanged is announced after the first frame is available for rendering. + Format format = outputFormat == null ? new Format.Builder().build() : outputFormat; + videoFrameMetadataListener.onVideoFrameAboutToBeRendered( + /* presentationTimeUs= */ presentationTimeUs - streamOffsetUs, + checkStateNotNull(clock).nanoTime(), + format, + /* mediaFormat= */ null); + } + checkStateNotNull(videoGraph).renderOutputFrame(renderTimeNs); + } + + @Override + public void dropFrame() { + for (CompositingVideoSinkProvider.Listener listener : listeners) { + listener.onFrameDropped(CompositingVideoSinkProvider.this); + } + checkStateNotNull(videoGraph).renderOutputFrame(VideoFrameProcessor.DROP_OUTPUT_FRAME); + } + } + /** * Delays reflection for loading a {@linkplain PreviewingVideoGraph.Factory * PreviewingSingleInputVideoGraph} instance. 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 739a49223b..efa4f65fe9 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 @@ -393,14 +393,13 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer eventDispatcher = new EventDispatcher(eventHandler, eventListener); ownsVideoSink = videoSinkProvider == null; if (videoSinkProvider == null) { - videoSinkProvider = new CompositingVideoSinkProvider.Builder(this.context).build(); - } - if (videoSinkProvider.getVideoFrameReleaseControl() == null) { @SuppressWarnings("nullness:assignment") VideoFrameReleaseControl.@Initialized FrameTimingEvaluator thisRef = this; - videoSinkProvider.setVideoFrameReleaseControl( + VideoFrameReleaseControl videoFrameReleaseControl = new VideoFrameReleaseControl( - this.context, /* frameTimingEvaluator= */ thisRef, allowedJoiningTimeMs)); + this.context, /* frameTimingEvaluator= */ thisRef, allowedJoiningTimeMs); + videoSinkProvider = + new CompositingVideoSinkProvider.Builder(this.context, videoFrameReleaseControl).build(); } videoSink = videoSinkProvider.getSink(); videoFrameReleaseControl = checkStateNotNull(videoSinkProvider.getVideoFrameReleaseControl()); 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 1b29ff95b6..a9b08865e8 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 @@ -184,7 +184,7 @@ public final class VideoFrameReleaseControl { * * @param applicationContext The application context. * @param frameTimingEvaluator The {@link FrameTimingEvaluator} that will assist in {@linkplain - * #getFrameReleaseAction(long, long, long, long, boolean, FrameReleaseInfo)} frame release + * #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. @@ -248,7 +248,7 @@ public final class VideoFrameReleaseControl { } /** - * Called when a frame have been released. + * Called when a frame has been released. * * @return Whether this is the first released frame. */ @@ -358,7 +358,7 @@ public final class VideoFrameReleaseControl { return FRAME_RELEASE_TRY_AGAIN_LATER; } - // Calculate release time and and adjust earlyUs to screen vsync. + // Calculate release time and adjust earlyUs to screen vsync. long systemTimeNs = clock.nanoTime(); frameReleaseInfo.releaseTimeNs = frameReleaseHelper.adjustReleaseTime(systemTimeNs + (frameReleaseInfo.earlyUs * 1_000)); diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoSink.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoSink.java index 1460339d61..66bcda4b03 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoSink.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoSink.java @@ -222,8 +222,6 @@ public interface VideoSink { /** * Incrementally renders processed video frames. * - *

Must be called after the sink is {@linkplain #initialize(Format, Clock) initialized}. - * * @param positionUs The current playback position, in microseconds. * @param elapsedRealtimeUs {@link android.os.SystemClock#elapsedRealtime()} in microseconds, * taken approximately at the time the playback position was {@code positionUs}. 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 8d466a891e..95f0004f4d 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 @@ -17,33 +17,18 @@ package androidx.media3.exoplayer.video; import android.view.Surface; -import androidx.media3.common.Format; -import androidx.media3.common.util.Clock; import androidx.media3.common.util.Size; import androidx.media3.common.util.UnstableApi; -import org.checkerframework.checker.nullness.qual.Nullable; /** A provider of {@link VideoSink VideoSinks}. */ @UnstableApi public interface VideoSinkProvider { - /** - * Sets the {@link VideoFrameReleaseControl} that will be used for releasing of video frames - * during rendering. - * - *

Must be called before the first {@linkplain #getSink() sink} is {@linkplain - * VideoSink#initialize(Format, Clock) initialized}. - */ - void setVideoFrameReleaseControl(VideoFrameReleaseControl videoFrameReleaseControl); - /** * Returns the {@link VideoFrameReleaseControl} that will be used for releasing of video frames * during rendering. - * - *

If this value is {@code null}, it must be {@linkplain #setVideoFrameReleaseControl set} to a - * non-null value before rendering begins. */ - @Nullable VideoFrameReleaseControl getVideoFrameReleaseControl(); + VideoFrameReleaseControl getVideoFrameReleaseControl(); /** Returns a {@link VideoSink} to forward video frames for processing. */ VideoSink getSink(); 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 b1d625a98f..e9a71b53c7 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 @@ -40,32 +40,17 @@ import org.mockito.Mockito; /** Unit test for {@link CompositingVideoSinkProvider}. */ @RunWith(AndroidJUnit4.class) public final class CompositingVideoSinkProviderTest { - @Test public void builder_calledMultipleTimes_throws() { + Context context = ApplicationProvider.getApplicationContext(); CompositingVideoSinkProvider.Builder builder = - new CompositingVideoSinkProvider.Builder(ApplicationProvider.getApplicationContext()); + new CompositingVideoSinkProvider.Builder(context, createVideoFrameReleaseControl()); builder.build(); assertThrows(IllegalStateException.class, builder::build); } - @Test - public void initializeSink_withoutReleaseControl_throws() { - CompositingVideoSinkProvider provider = - new CompositingVideoSinkProvider.Builder(ApplicationProvider.getApplicationContext()) - .setPreviewingVideoGraphFactory(new TestPreviewingVideoGraphFactory()) - .build(); - VideoSink sink = provider.getSink(); - - assertThrows( - IllegalStateException.class, - () -> - sink.initialize( - new Format.Builder().setWidth(640).setHeight(480).build(), Clock.DEFAULT)); - } - @Test public void initializeSink_calledTwice_throws() throws VideoSink.VideoSinkException { CompositingVideoSinkProvider provider = createCompositingVideoSinkProvider(); @@ -95,6 +80,13 @@ public final class CompositingVideoSinkProviderTest { } private static CompositingVideoSinkProvider createCompositingVideoSinkProvider() { + Context context = ApplicationProvider.getApplicationContext(); + return new CompositingVideoSinkProvider.Builder(context, createVideoFrameReleaseControl()) + .setPreviewingVideoGraphFactory(new TestPreviewingVideoGraphFactory()) + .build(); + } + + private static VideoFrameReleaseControl createVideoFrameReleaseControl() { Context context = ApplicationProvider.getApplicationContext(); VideoFrameReleaseControl.FrameTimingEvaluator frameTimingEvaluator = new VideoFrameReleaseControl.FrameTimingEvaluator() { @@ -119,13 +111,8 @@ public final class CompositingVideoSinkProviderTest { return false; } }; - CompositingVideoSinkProvider compositingVideoSinkProvider = - new CompositingVideoSinkProvider.Builder(context) - .setPreviewingVideoGraphFactory(new TestPreviewingVideoGraphFactory()) - .build(); - compositingVideoSinkProvider.setVideoFrameReleaseControl( - new VideoFrameReleaseControl(context, frameTimingEvaluator, /* allowedJoiningTimeMs= */ 0)); - return compositingVideoSinkProvider; + return new VideoFrameReleaseControl( + context, frameTimingEvaluator, /* allowedJoiningTimeMs= */ 0); } private static class TestPreviewingVideoGraphFactory implements PreviewingVideoGraph.Factory { diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/CompositionPlayer.java b/libraries/transformer/src/main/java/androidx/media3/transformer/CompositionPlayer.java index c3f7c40a49..45e2f3771d 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/CompositionPlayer.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/CompositionPlayer.java @@ -595,13 +595,13 @@ public final class CompositionPlayer extends SimpleBasePlayer new DefaultAudioMixer.Factory(), composition.effects.audioProcessors, checkNotNull(finalAudioSink)); + VideoFrameReleaseControl videoFrameReleaseControl = + new VideoFrameReleaseControl( + context, new CompositionFrameTimingEvaluator(), /* allowedJoiningTimeMs= */ 0); CompositingVideoSinkProvider compositingVideoSinkProvider = - new CompositingVideoSinkProvider.Builder(context) + new CompositingVideoSinkProvider.Builder(context, videoFrameReleaseControl) .setPreviewingVideoGraphFactory(checkNotNull(previewingVideoGraphFactory)) .build(); - compositingVideoSinkProvider.setVideoFrameReleaseControl( - new VideoFrameReleaseControl( - context, new CompositionFrameTimingEvaluator(), /* allowedJoiningTimeMs= */ 0)); compositingVideoSinkProvider.addListener(this); for (int i = 0; i < composition.sequences.size(); i++) { EditedMediaItemSequence editedMediaItemSequence = composition.sequences.get(i);