From 2a74cb7ef21350f32897462f46f17078171f3ff3 Mon Sep 17 00:00:00 2001 From: christosts Date: Thu, 23 Nov 2023 03:51:24 -0800 Subject: [PATCH] Move VideoFrameProcessor reflective loading code Move the reflective loading of VideoFrameProcessor from MediaCodecVideoRenderer to the CompositingVideoSinkProvider. This is so that all reflective code lives in one place. The CompositingVideoSinkProvider already has reflective code to load the default PreviewingVideoGraph.Factory. PiperOrigin-RevId: 584852547 --- .../PreviewingSingleInputVideoGraph.java | 12 ++ .../video/CompositingVideoSinkProvider.java | 161 +++++++++++++++--- .../video/MediaCodecVideoRenderer.java | 72 +------- .../CompositingVideoSinkProviderTest.java | 43 +++-- 4 files changed, 194 insertions(+), 94 deletions(-) diff --git a/libraries/effect/src/main/java/androidx/media3/effect/PreviewingSingleInputVideoGraph.java b/libraries/effect/src/main/java/androidx/media3/effect/PreviewingSingleInputVideoGraph.java index fcaa96e894..10cff7faed 100644 --- a/libraries/effect/src/main/java/androidx/media3/effect/PreviewingSingleInputVideoGraph.java +++ b/libraries/effect/src/main/java/androidx/media3/effect/PreviewingSingleInputVideoGraph.java @@ -40,6 +40,18 @@ public final class PreviewingSingleInputVideoGraph extends SingleInputVideoGraph private final VideoFrameProcessor.Factory videoFrameProcessorFactory; + /** + * Creates a new factory that uses the {@link DefaultVideoFrameProcessor.Factory} with its + * default values. + */ + public Factory() { + this(new DefaultVideoFrameProcessor.Factory.Builder().build()); + } + + /** + * Creates an instance that uses the supplied {@code videoFrameProcessorFactory} to create + * {@link VideoFrameProcessor} instances. + */ public Factory(VideoFrameProcessor.Factory videoFrameProcessorFactory) { this.videoFrameProcessorFactory = videoFrameProcessorFactory; } 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 31e83cd478..87ffb8b6ae 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 @@ -26,7 +26,6 @@ import android.util.Pair; import android.view.Surface; import androidx.annotation.FloatRange; import androidx.annotation.Nullable; -import androidx.annotation.VisibleForTesting; import androidx.media3.common.C; import androidx.media3.common.ColorInfo; import androidx.media3.common.DebugViewProvider; @@ -47,6 +46,8 @@ import androidx.media3.common.util.TimestampIterator; import androidx.media3.common.util.UnstableApi; import androidx.media3.common.util.Util; import androidx.media3.exoplayer.ExoPlaybackException; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableList; import java.lang.reflect.Constructor; import java.lang.reflect.Method; @@ -63,6 +64,89 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; /* package */ final class CompositingVideoSinkProvider implements VideoSinkProvider, VideoGraph.Listener, VideoFrameRenderControl.FrameRenderer { + /** A builder for {@link CompositingVideoSinkProvider} instances. */ + public static final class Builder { + private final Context context; + + private VideoFrameProcessor.@MonotonicNonNull Factory videoFrameProcessorFactory; + private PreviewingVideoGraph.@MonotonicNonNull Factory previewingVideoGraphFactory; + private @MonotonicNonNull VideoFrameReleaseControl videoFrameReleaseControl; + private boolean built; + + /** Creates a builder with the supplied {@linkplain Context application context}. */ + public Builder(Context context) { + this.context = context; + } + + /** + * Sets the {@link VideoFrameProcessor.Factory} that will be used for creating {@link + * VideoFrameProcessor} instances. + * + *

By default, the {@code DefaultVideoFrameProcessor.Factory} with its default values will be + * used. + * + * @param videoFrameProcessorFactory The {@link VideoFrameProcessor.Factory}. + * @return This builder, for convenience. + */ + public Builder setVideoFrameProcessorFactory( + VideoFrameProcessor.Factory videoFrameProcessorFactory) { + this.videoFrameProcessorFactory = videoFrameProcessorFactory; + return this; + } + + /** + * Sets the {@link PreviewingVideoGraph.Factory} that will be used for creating {@link + * PreviewingVideoGraph} instances. + * + *

By default, the {@code PreviewingSingleInputVideoGraph.Factory} will be used. + * + * @param previewingVideoGraphFactory The {@link PreviewingVideoGraph.Factory}. + * @return This builder, for convenience. + */ + public Builder setPreviewingVideoGraphFactory( + PreviewingVideoGraph.Factory previewingVideoGraphFactory) { + this.previewingVideoGraphFactory = previewingVideoGraphFactory; + return this; + } + + /** + * Sets the {@link VideoFrameReleaseControl} that will be used. + * + * @param videoFrameReleaseControl The {@link VideoFrameReleaseControl}. + * @return This builder, for convenience. + */ + public Builder setVideoFrameReleaseControl(VideoFrameReleaseControl videoFrameReleaseControl) { + this.videoFrameReleaseControl = videoFrameReleaseControl; + return this; + } + + /** + * 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. + */ + public CompositingVideoSinkProvider build() { + checkState(!built); + + if (previewingVideoGraphFactory == null) { + if (videoFrameProcessorFactory == null) { + videoFrameProcessorFactory = new ReflectiveDefaultVideoFrameProcessorFactory(); + } + previewingVideoGraphFactory = + new ReflectivePreviewingSingleInputVideoGraphFactory(videoFrameProcessorFactory); + } + CompositingVideoSinkProvider compositingVideoSinkProvider = + new CompositingVideoSinkProvider(this); + built = true; + return compositingVideoSinkProvider; + } + } + private static final Executor NO_OP_EXECUTOR = runnable -> {}; private final Context context; @@ -83,25 +167,10 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; private int pendingFlushCount; private boolean released; - /** Creates a new instance. */ - public CompositingVideoSinkProvider( - Context context, - VideoFrameProcessor.Factory videoFrameProcessorFactory, - VideoFrameReleaseControl videoFrameReleaseControl) { - this( - context, - new ReflectivePreviewingSingleInputVideoGraphFactory(videoFrameProcessorFactory), - videoFrameReleaseControl); - } - - @VisibleForTesting - /* package */ CompositingVideoSinkProvider( - Context context, - PreviewingVideoGraph.Factory previewingVideoGraphFactory, - VideoFrameReleaseControl videoFrameReleaseControl) { - this.context = context; - this.previewingVideoGraphFactory = previewingVideoGraphFactory; - this.videoFrameReleaseControl = videoFrameReleaseControl; + private CompositingVideoSinkProvider(Builder builder) { + this.context = builder.context; + this.previewingVideoGraphFactory = checkStateNotNull(builder.previewingVideoGraphFactory); + videoFrameReleaseControl = checkStateNotNull(builder.videoFrameReleaseControl); @SuppressWarnings("nullness:assignment") VideoFrameRenderControl.@Initialized FrameRenderer thisRef = this; videoFrameRenderControl = @@ -682,4 +751,56 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; } } } + + /** + * Delays reflection for loading a {@linkplain VideoFrameProcessor.Factory + * DefaultVideoFrameProcessor.Factory} instance. + */ + private static final class ReflectiveDefaultVideoFrameProcessorFactory + implements VideoFrameProcessor.Factory { + private static final Supplier + VIDEO_FRAME_PROCESSOR_FACTORY_SUPPLIER = + Suppliers.memoize( + () -> { + try { + // TODO: b/284964524- Add LINT and proguard checks for media3.effect reflection. + Class defaultVideoFrameProcessorFactoryBuilderClass = + Class.forName( + "androidx.media3.effect.DefaultVideoFrameProcessor$Factory$Builder"); + Object builder = + defaultVideoFrameProcessorFactoryBuilderClass + .getConstructor() + .newInstance(); + return (VideoFrameProcessor.Factory) + checkNotNull( + defaultVideoFrameProcessorFactoryBuilderClass + .getMethod("build") + .invoke(builder)); + } catch (Exception e) { + throw new IllegalStateException(e); + } + }); + + @Override + public VideoFrameProcessor create( + Context context, + DebugViewProvider debugViewProvider, + ColorInfo inputColorInfo, + ColorInfo outputColorInfo, + boolean renderFramesAutomatically, + Executor listenerExecutor, + VideoFrameProcessor.Listener listener) + throws VideoFrameProcessingException { + return VIDEO_FRAME_PROCESSOR_FACTORY_SUPPLIER + .get() + .create( + context, + debugViewProvider, + inputColorInfo, + outputColorInfo, + renderFramesAutomatically, + listenerExecutor, + listener); + } + } } 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 836be4fe47..aebf6cd98c 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 @@ -46,14 +46,11 @@ import androidx.annotation.DoNotInline; import androidx.annotation.Nullable; import androidx.annotation.RequiresApi; import androidx.media3.common.C; -import androidx.media3.common.ColorInfo; -import androidx.media3.common.DebugViewProvider; import androidx.media3.common.DrmInitData; import androidx.media3.common.Effect; import androidx.media3.common.Format; import androidx.media3.common.MimeTypes; import androidx.media3.common.PlaybackException; -import androidx.media3.common.VideoFrameProcessingException; import androidx.media3.common.VideoFrameProcessor; import androidx.media3.common.VideoSize; import androidx.media3.common.util.Clock; @@ -80,13 +77,10 @@ import androidx.media3.exoplayer.mediacodec.MediaCodecSelector; import androidx.media3.exoplayer.mediacodec.MediaCodecUtil; import androidx.media3.exoplayer.mediacodec.MediaCodecUtil.DecoderQueryException; import androidx.media3.exoplayer.video.VideoRendererEventListener.EventDispatcher; -import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableList; import com.google.common.util.concurrent.MoreExecutors; import java.nio.ByteBuffer; import java.util.List; -import java.util.concurrent.Executor; import org.checkerframework.checker.initialization.qual.Initialized; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; import org.checkerframework.checker.nullness.qual.RequiresNonNull; @@ -349,7 +343,7 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer eventListener, maxDroppedFramesToNotify, assumedMinimumCodecOperatingRate, - new ReflectiveDefaultVideoFrameProcessorFactory()); + /* videoFrameProcessorFactory= */ null); } /** @@ -385,7 +379,7 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer @Nullable VideoRendererEventListener eventListener, int maxDroppedFramesToNotify, float assumedMinimumCodecOperatingRate, - VideoFrameProcessor.Factory videoFrameProcessorFactory) { + @Nullable VideoFrameProcessor.Factory videoFrameProcessorFactory) { super( C.TRACK_TYPE_VIDEO, codecAdapterFactory, @@ -401,9 +395,13 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer this.context, /* frameTimingEvaluator= */ thisRef, allowedJoiningTimeMs); videoFrameReleaseInfo = new VideoFrameReleaseControl.FrameReleaseInfo(); eventDispatcher = new EventDispatcher(eventHandler, eventListener); - videoSinkProvider = - new CompositingVideoSinkProvider( - context, videoFrameProcessorFactory, videoFrameReleaseControl); + CompositingVideoSinkProvider.Builder compositingVideoSinkProvider = + new CompositingVideoSinkProvider.Builder(context) + .setVideoFrameReleaseControl(videoFrameReleaseControl); + if (videoFrameProcessorFactory != null) { + compositingVideoSinkProvider.setVideoFrameProcessorFactory(videoFrameProcessorFactory); + } + videoSinkProvider = compositingVideoSinkProvider.build(); deviceNeedsNoPostProcessWorkaround = deviceNeedsNoPostProcessWorkaround(); scalingMode = C.VIDEO_SCALING_MODE_DEFAULT; decodedVideoSize = VideoSize.UNKNOWN; @@ -1891,58 +1889,6 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer return new MediaCodecVideoDecoderException(cause, codecInfo, displaySurface); } - /** - * Delays reflection for loading a {@linkplain VideoFrameProcessor.Factory - * DefaultVideoFrameProcessor} instance. - */ - private static final class ReflectiveDefaultVideoFrameProcessorFactory - implements VideoFrameProcessor.Factory { - private static final Supplier - VIDEO_FRAME_PROCESSOR_FACTORY_SUPPLIER = - Suppliers.memoize( - () -> { - try { - // TODO: b/284964524- Add LINT and proguard checks for media3.effect reflection. - Class defaultVideoFrameProcessorFactoryBuilderClass = - Class.forName( - "androidx.media3.effect.DefaultVideoFrameProcessor$Factory$Builder"); - Object builder = - defaultVideoFrameProcessorFactoryBuilderClass - .getConstructor() - .newInstance(); - return (VideoFrameProcessor.Factory) - checkNotNull( - defaultVideoFrameProcessorFactoryBuilderClass - .getMethod("build") - .invoke(builder)); - } catch (Exception e) { - throw new IllegalStateException(e); - } - }); - - @Override - public VideoFrameProcessor create( - Context context, - DebugViewProvider debugViewProvider, - ColorInfo inputColorInfo, - ColorInfo outputColorInfo, - boolean renderFramesAutomatically, - Executor listenerExecutor, - VideoFrameProcessor.Listener listener) - throws VideoFrameProcessingException { - return VIDEO_FRAME_PROCESSOR_FACTORY_SUPPLIER - .get() - .create( - context, - debugViewProvider, - inputColorInfo, - outputColorInfo, - renderFramesAutomatically, - listenerExecutor, - listener); - } - } - /** * Returns a maximum video size to use when configuring a codec for {@code format} in a way that * will allow possible adaptation to other compatible formats that are expected to have the same 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 f6d413486f..ead017c07b 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 @@ -18,6 +18,7 @@ package androidx.media3.exoplayer.video; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import android.content.Context; @@ -28,7 +29,6 @@ import androidx.media3.common.Format; import androidx.media3.common.PreviewingVideoGraph; import androidx.media3.common.VideoFrameProcessor; import androidx.media3.common.VideoGraph; -import androidx.media3.exoplayer.ExoPlaybackException; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.common.collect.ImmutableList; @@ -42,6 +42,30 @@ 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 = + new CompositingVideoSinkProvider.Builder(ApplicationProvider.getApplicationContext()) + .setVideoFrameReleaseControl( + new VideoFrameReleaseControl( + ApplicationProvider.getApplicationContext(), + mock(VideoFrameReleaseControl.FrameTimingEvaluator.class), + /* allowedJoiningTimeMs= */ 0)); + + builder.build(); + + assertThrows(IllegalStateException.class, builder::build); + } + @Test public void initialize() throws VideoSink.VideoSinkException { CompositingVideoSinkProvider provider = createCompositingVideoSinkProvider(); @@ -133,6 +157,7 @@ public final class CompositingVideoSinkProviderTest { } private static CompositingVideoSinkProvider createCompositingVideoSinkProvider() { + Context context = ApplicationProvider.getApplicationContext(); VideoFrameReleaseControl.FrameTimingEvaluator frameTimingEvaluator = new VideoFrameReleaseControl.FrameTimingEvaluator() { @Override @@ -152,20 +177,16 @@ public final class CompositingVideoSinkProviderTest { long positionUs, long elapsedRealtimeUs, boolean isLastFrame, - boolean treatDroppedBuffersAsSkipped) - throws ExoPlaybackException { + boolean treatDroppedBuffersAsSkipped) { return false; } }; VideoFrameReleaseControl releaseControl = - new VideoFrameReleaseControl( - ApplicationProvider.getApplicationContext(), - frameTimingEvaluator, - /* allowedJoiningTimeMs= */ 0); - return new CompositingVideoSinkProvider( - ApplicationProvider.getApplicationContext(), - new TestPreviewingVideoGraphFactory(), - releaseControl); + new VideoFrameReleaseControl(context, frameTimingEvaluator, /* allowedJoiningTimeMs= */ 0); + return new CompositingVideoSinkProvider.Builder(context) + .setPreviewingVideoGraphFactory(new TestPreviewingVideoGraphFactory()) + .setVideoFrameReleaseControl(releaseControl) + .build(); } private static class TestPreviewingVideoGraphFactory implements PreviewingVideoGraph.Factory {