From 8a709a7d76fe02378dabf7ef7066b05cdf83bb64 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 9 Jan 2025 04:45:21 -0800 Subject: [PATCH] Remove setPendingVideoEffects from VideoSink. VideoSink#onInputStreamChanged(int, Format, List) should now be used to set video effects on a new input stream. PiperOrigin-RevId: 713627389 --- .../exoplayer/video/DefaultVideoSink.java | 20 ++-- .../video/MediaCodecVideoRenderer.java | 16 +-- .../video/PlaybackVideoGraphWrapper.java | 32 ++++-- .../media3/exoplayer/video/VideoSink.java | 20 ++-- .../video/PlaybackVideoGraphWrapperTest.java | 103 +++++++++++++++++- .../transformer/BufferingVideoSink.java | 10 +- .../transformer/SequenceRenderersFactory.java | 18 ++- 7 files changed, 154 insertions(+), 65 deletions(-) diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/DefaultVideoSink.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/DefaultVideoSink.java index 48f513512c..653ee143c2 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/DefaultVideoSink.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/DefaultVideoSink.java @@ -15,6 +15,7 @@ */ package androidx.media3.exoplayer.video; +import static androidx.media3.common.util.Assertions.checkState; import static androidx.media3.common.util.Assertions.checkStateNotNull; import android.graphics.Bitmap; @@ -170,16 +171,6 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; throw new UnsupportedOperationException(); } - /** - * {@inheritDoc} - * - *

This method will always throw an {@link UnsupportedOperationException}. - */ - @Override - public void setPendingVideoEffects(List videoEffects) { - throw new UnsupportedOperationException(); - } - @Override public void setStreamTimestampInfo( long streamStartPositionUs, long bufferTimestampAdjustmentUs, long lastResetPositionUs) { @@ -212,8 +203,15 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; videoFrameReleaseControl.allowReleaseFirstFrameBeforeStarted(); } + /** + * {@inheritDoc} + * + *

{@code videoEffects} is required to be empty + */ @Override - public void onInputStreamChanged(@InputType int inputType, Format format) { + public void onInputStreamChanged( + @InputType int inputType, Format format, List videoEffects) { + checkState(videoEffects.isEmpty()); if (format.width != inputFormat.width || format.height != inputFormat.height) { videoFrameRenderControl.onVideoSizeChanged(format.width, format.height); } 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 df56c3c8c7..78627af195 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 @@ -1397,8 +1397,8 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer decodedVideoSize = new VideoSize(width, height, pixelWidthHeightRatio); if (videoSink != null && pendingVideoSinkInputStreamChange) { - onReadyToChangeVideoSinkInputStream(); - videoSink.onInputStreamChanged( + changeVideoSinkInputStream( + videoSink, /* inputType= */ VideoSink.INPUT_TYPE_SURFACE, format .buildUpon() @@ -1413,13 +1413,15 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer } /** - * Called when ready to {@linkplain VideoSink#onInputStreamChanged(int, Format) change} the input - * stream when {@linkplain #setVideoEffects video effects} are enabled. + * Called when ready to {@linkplain VideoSink#onInputStreamChanged(int, Format, List) + * change} the input stream. * - *

The default implementation is a no-op. + *

The default implementation applies this renderer's video effects. */ - protected void onReadyToChangeVideoSinkInputStream() { - // do nothing. + protected void changeVideoSinkInputStream( + VideoSink videoSink, @VideoSink.InputType int inputType, Format format) { + List videoEffectsToApply = videoEffects != null ? videoEffects : ImmutableList.of(); + videoSink.onInputStreamChanged(inputType, format, videoEffectsToApply); } @Override diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/PlaybackVideoGraphWrapper.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/PlaybackVideoGraphWrapper.java index 8140f3a045..6e2bd279e0 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/PlaybackVideoGraphWrapper.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/PlaybackVideoGraphWrapper.java @@ -382,13 +382,15 @@ public final class PlaybackVideoGraphWrapper implements VideoSinkProvider, Video // We forward output size changes to the sink even if we are still flushing. videoGraphOutputFormat = videoGraphOutputFormat.buildUpon().setWidth(width).setHeight(height).build(); - defaultVideoSink.onInputStreamChanged(INPUT_TYPE_SURFACE, videoGraphOutputFormat); + defaultVideoSink.onInputStreamChanged( + INPUT_TYPE_SURFACE, videoGraphOutputFormat, /* videoEffects= */ ImmutableList.of()); } @Override public void onOutputFrameRateChanged(float frameRate) { videoGraphOutputFormat = videoGraphOutputFormat.buildUpon().setFrameRate(frameRate).build(); - defaultVideoSink.onInputStreamChanged(INPUT_TYPE_SURFACE, videoGraphOutputFormat); + defaultVideoSink.onInputStreamChanged( + INPUT_TYPE_SURFACE, videoGraphOutputFormat, /* videoEffects= */ ImmutableList.of()); } @Override @@ -685,7 +687,8 @@ public final class PlaybackVideoGraphWrapper implements VideoSinkProvider, Video } @Override - public void onInputStreamChanged(@InputType int inputType, Format format) { + public void onInputStreamChanged( + @InputType int inputType, Format format, List videoEffects) { checkState(isInitialized()); switch (inputType) { case INPUT_TYPE_SURFACE: @@ -694,6 +697,7 @@ public final class PlaybackVideoGraphWrapper implements VideoSinkProvider, Video default: throw new UnsupportedOperationException("Unsupported input type " + inputType); } + setPendingVideoEffects(videoEffects); this.inputType = inputType; this.inputFormat = format; finalBufferPresentationTimeUs = C.TIME_UNSET; @@ -729,15 +733,6 @@ public final class PlaybackVideoGraphWrapper implements VideoSinkProvider, Video } } - @Override - public void setPendingVideoEffects(List videoEffects) { - this.videoEffects = - new ImmutableList.Builder() - .addAll(videoEffects) - .addAll(compositionEffects) - .build(); - } - @Override public void setStreamTimestampInfo( long streamStartPositionUs, long bufferTimestampAdjustmentUs, long lastResetPositionUs) { @@ -887,6 +882,19 @@ public final class PlaybackVideoGraphWrapper implements VideoSinkProvider, Video // Private methods + /** + * Sets the pending video effects. + * + *

Effects are pending until a new input stream is registered. + */ + private void setPendingVideoEffects(List newVideoEffects) { + this.videoEffects = + new ImmutableList.Builder() + .addAll(newVideoEffects) + .addAll(compositionEffects) + .build(); + } + private void registerInputStream(Format inputFormat) { Format adjustedInputFormat = inputFormat 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 ddf2408727..6224890b36 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 @@ -187,8 +187,9 @@ public interface VideoSink { * *

This method returns {@code true} if the end of the last input stream has been {@linkplain * #signalEndOfCurrentInputStream() signaled} and all the input frames have been rendered. Note - * that a new input stream can be {@linkplain #onInputStreamChanged(int, Format) signaled} even - * when this method returns true (in which case the sink will not be ended anymore). + * that a new input stream can be {@linkplain #onInputStreamChanged(int, Format, List) + * signaled} even when this method returns true (in which case the sink will not be ended + * anymore). */ boolean isEnded(); @@ -208,12 +209,6 @@ public interface VideoSink { /** Sets {@linkplain Effect video effects} to apply immediately. */ void setVideoEffects(List videoEffects); - /** - * Sets {@linkplain Effect video effects} to apply after the next stream {@linkplain - * VideoSink#onInputStreamChanged(int, Format) change}. - */ - void setPendingVideoEffects(List videoEffects); - /** * Sets information about the timestamps of the current input stream. * @@ -250,20 +245,21 @@ public interface VideoSink { void enableMayRenderStartOfStream(); /** - * Informs the video sink that a new input stream will be queued. + * Informs the video sink that a new input stream will be queued with the given effects. * *

Must be called after the sink is {@linkplain #initialize(Format) initialized}. * * @param inputType The {@link InputType} of the stream. * @param format The {@link Format} of the stream. + * @param videoEffects The {@link List} to apply to the new stream. */ - void onInputStreamChanged(@InputType int inputType, Format format); + void onInputStreamChanged(@InputType int inputType, Format format, List videoEffects); /** * Handles a video input frame. * *

Must be called after the corresponding stream is {@linkplain #onInputStreamChanged(int, - * Format) signaled}. + * Format, List) signaled}. * * @param framePresentationTimeUs The frame's presentation time, in microseconds. * @param isLastFrame Whether this is the last frame of the video stream. This flag is set on a @@ -280,7 +276,7 @@ public interface VideoSink { * Handles an input {@link Bitmap}. * *

Must be called after the corresponding stream is {@linkplain #onInputStreamChanged(int, - * Format) signaled}. + * Format, List) signaled}. * * @param inputBitmap The {@link Bitmap} to queue to the video sink. * @param timestampIterator The times within the current stream that the bitmap should be shown diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/PlaybackVideoGraphWrapperTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/PlaybackVideoGraphWrapperTest.java index 0c776c30c8..be22af47ab 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/PlaybackVideoGraphWrapperTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/PlaybackVideoGraphWrapperTest.java @@ -15,20 +15,28 @@ */ 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.when; import android.content.Context; +import android.graphics.Bitmap; +import android.view.Surface; +import androidx.annotation.Nullable; import androidx.media3.common.ColorInfo; import androidx.media3.common.DebugViewProvider; import androidx.media3.common.Effect; import androidx.media3.common.Format; +import androidx.media3.common.OnInputFrameProcessedListener; import androidx.media3.common.PreviewingVideoGraph; +import androidx.media3.common.SurfaceInfo; import androidx.media3.common.VideoFrameProcessor; import androidx.media3.common.VideoGraph; +import androidx.media3.common.util.TimestampIterator; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.google.common.collect.ImmutableList; import java.util.List; import java.util.concurrent.Executor; import org.junit.Test; @@ -51,17 +59,40 @@ public final class PlaybackVideoGraphWrapperTest { @Test public void initializeSink_calledTwice_throws() throws VideoSink.VideoSinkException { - PlaybackVideoGraphWrapper provider = createPlaybackVideoGraphWrapper(); - VideoSink sink = provider.getSink(); + PlaybackVideoGraphWrapper playbackVideoGraphWrapper = + createPlaybackVideoGraphWrapper(new FakeVideoFrameProcessor()); + VideoSink sink = playbackVideoGraphWrapper.getSink(); sink.initialize(new Format.Builder().build()); assertThrows(IllegalStateException.class, () -> sink.initialize(new Format.Builder().build())); } - private static PlaybackVideoGraphWrapper createPlaybackVideoGraphWrapper() { + @Test + public void onInputStreamChanged_setsVideoSinkVideoEffects() throws VideoSink.VideoSinkException { + ImmutableList firstEffects = ImmutableList.of(Mockito.mock(Effect.class)); + ImmutableList secondEffects = + ImmutableList.of(Mockito.mock(Effect.class), Mockito.mock(Effect.class)); + FakeVideoFrameProcessor videoFrameProcessor = new FakeVideoFrameProcessor(); + PlaybackVideoGraphWrapper playbackVideoGraphWrapper = + createPlaybackVideoGraphWrapper(videoFrameProcessor); + Format format = new Format.Builder().build(); + VideoSink sink = playbackVideoGraphWrapper.getSink(); + + sink.initialize(format); + + sink.onInputStreamChanged(VideoSink.INPUT_TYPE_SURFACE, format, firstEffects); + assertThat(videoFrameProcessor.registeredEffects).isEqualTo(firstEffects); + sink.onInputStreamChanged(VideoSink.INPUT_TYPE_SURFACE, format, secondEffects); + assertThat(videoFrameProcessor.registeredEffects).isEqualTo(secondEffects); + sink.onInputStreamChanged(VideoSink.INPUT_TYPE_SURFACE, format, ImmutableList.of()); + assertThat(videoFrameProcessor.registeredEffects).isEmpty(); + } + + private static PlaybackVideoGraphWrapper createPlaybackVideoGraphWrapper( + VideoFrameProcessor videoFrameProcessor) { Context context = ApplicationProvider.getApplicationContext(); return new PlaybackVideoGraphWrapper.Builder(context, createVideoFrameReleaseControl()) - .setPreviewingVideoGraphFactory(new TestPreviewingVideoGraphFactory()) + .setPreviewingVideoGraphFactory(new TestPreviewingVideoGraphFactory(videoFrameProcessor)) .build(); } @@ -94,12 +125,73 @@ public final class PlaybackVideoGraphWrapperTest { context, frameTimingEvaluator, /* allowedJoiningTimeMs= */ 0); } + private static class FakeVideoFrameProcessor implements VideoFrameProcessor { + + List registeredEffects = ImmutableList.of(); + + @Override + public boolean queueInputBitmap(Bitmap inputBitmap, TimestampIterator timestampIterator) { + return false; + } + + @Override + public boolean queueInputTexture(int textureId, long presentationTimeUs) { + return false; + } + + @Override + public void setOnInputFrameProcessedListener(OnInputFrameProcessedListener listener) {} + + @Override + public void setOnInputSurfaceReadyListener(Runnable listener) {} + + @Override + public Surface getInputSurface() { + return null; + } + + @Override + public void registerInputStream( + @InputType int inputType, Format format, List effects, long offsetToAddUs) { + registeredEffects = effects; + } + + @Override + public boolean registerInputFrame() { + return true; + } + + @Override + public int getPendingInputFrameCount() { + return 0; + } + + @Override + public void setOutputSurfaceInfo(@Nullable SurfaceInfo outputSurfaceInfo) {} + + @Override + public void renderOutputFrame(long renderTimeNs) {} + + @Override + public void signalEndOfInput() {} + + @Override + public void flush() {} + + @Override + public void release() {} + } + private static class TestPreviewingVideoGraphFactory implements PreviewingVideoGraph.Factory { // Using a mock but we don't assert mock interactions. If needed to assert interactions, we // should a fake instead. private final PreviewingVideoGraph previewingVideoGraph = Mockito.mock(PreviewingVideoGraph.class); - private final VideoFrameProcessor videoFrameProcessor = Mockito.mock(VideoFrameProcessor.class); + private final VideoFrameProcessor videoFrameProcessor; + + public TestPreviewingVideoGraphFactory(VideoFrameProcessor videoFrameProcessor) { + this.videoFrameProcessor = videoFrameProcessor; + } @Override public PreviewingVideoGraph create( @@ -111,7 +203,6 @@ public final class PlaybackVideoGraphWrapperTest { List compositionEffects, long initialTimestampOffsetUs) { when(previewingVideoGraph.getProcessor(anyInt())).thenReturn(videoFrameProcessor); - when(videoFrameProcessor.registerInputFrame()).thenReturn(true); return previewingVideoGraph; } } diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/BufferingVideoSink.java b/libraries/transformer/src/main/java/androidx/media3/transformer/BufferingVideoSink.java index 8698501a39..3e2a979811 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/BufferingVideoSink.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/BufferingVideoSink.java @@ -176,11 +176,6 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; executeOrDelay(videoSink -> videoSink.setVideoEffects(videoEffects)); } - @Override - public void setPendingVideoEffects(List videoEffects) { - executeOrDelay(videoSink -> videoSink.setPendingVideoEffects(videoEffects)); - } - @Override public void setStreamTimestampInfo( long streamStartPositionUs, long bufferTimestampAdjustmentUs, long lastResetPositionUs) { @@ -211,8 +206,9 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; } @Override - public void onInputStreamChanged(@InputType int inputType, Format format) { - executeOrDelay(videoSink -> videoSink.onInputStreamChanged(inputType, format)); + public void onInputStreamChanged( + @InputType int inputType, Format format, List videoEffects) { + executeOrDelay(videoSink -> videoSink.onInputStreamChanged(inputType, format, videoEffects)); } @Override diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/SequenceRenderersFactory.java b/libraries/transformer/src/main/java/androidx/media3/transformer/SequenceRenderersFactory.java index e9ce974c50..cd254cfd59 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/SequenceRenderersFactory.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/SequenceRenderersFactory.java @@ -287,7 +287,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; private final VideoSink videoSink; private final boolean requestToneMapping; - @Nullable private ImmutableList pendingEffect; + private ImmutableList pendingEffects; @Nullable private EditedMediaItem currentEditedMediaItem; private long offsetToCompositionTimeUs; @@ -312,6 +312,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; this.sequence = sequence; this.videoSink = videoSink; this.requestToneMapping = requestToneMapping; + this.pendingEffects = ImmutableList.of(); experimentalEnableProcessedStreamChangedAtStart(); } @@ -329,7 +330,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; // previous one. currentEditedMediaItem = getRepeatedEditedMediaItem(sequence, mediaItemIndex); offsetToCompositionTimeUs = getOffsetToCompositionTimeUs(sequence, mediaItemIndex, offsetUs); - pendingEffect = sequence.editedMediaItems.get(mediaItemIndex).effects.videoEffects; + pendingEffects = sequence.editedMediaItems.get(mediaItemIndex).effects.videoEffects; super.onStreamChanged(formats, startPositionUs, offsetUs, mediaPeriodId); } @@ -370,12 +371,9 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; } @Override - protected void onReadyToChangeVideoSinkInputStream() { - @Nullable ImmutableList pendingEffect = this.pendingEffect; - if (pendingEffect != null) { - videoSink.setPendingVideoEffects(pendingEffect); - this.pendingEffect = null; - } + protected void changeVideoSinkInputStream( + VideoSink videoSink, @VideoSink.InputType int inputType, Format format) { + videoSink.onInputStreamChanged(inputType, format, pendingEffects); } } @@ -519,7 +517,6 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; long positionUs, long elapsedRealtimeUs, Bitmap outputImage, long timeUs) { if (inputStreamPending) { checkState(streamStartPositionUs != C.TIME_UNSET); - videoSink.setPendingVideoEffects(videoEffects); videoSink.setStreamTimestampInfo( streamStartPositionUs, /* bufferTimestampAdjustmentUs= */ offsetToCompositionTimeUs, @@ -532,7 +529,8 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; .setHeight(outputImage.getHeight()) .setColorInfo(ColorInfo.SRGB_BT709_FULL) .setFrameRate(/* frameRate= */ DEFAULT_FRAME_RATE) - .build()); + .build(), + videoEffects); inputStreamPending = false; } if (!videoSink.handleInputBitmap(outputImage, checkStateNotNull(timestampIterator))) {