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 42adeb336a..6ee33ee6dc 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/BufferingVideoSink.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/BufferingVideoSink.java @@ -33,13 +33,16 @@ import java.util.concurrent.Executor; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; /** - * A {@link VideoSink} that delays the operations performed on it until it {@linkplain + * A {@link VideoSink} that delays most operations performed on it until it {@linkplain * #setVideoSink(VideoSink) receives} a sink. + * + *

Some operations are not delayed. Their behavior in case there is no underlying {@link + * VideoSink} is documented in the corresponding method. */ /* package */ final class BufferingVideoSink implements VideoSink { private final Context context; - private final List pendingOperations; + private final List pendingOperations; @Nullable private VideoSink videoSink; private boolean isInitialized; @@ -53,24 +56,20 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; /** * Sets the {@link VideoSink} to execute the pending and future operations on. * - * @param videoSink The {@link VideoSink} to execute the operations on. - * @throws VideoSinkException If an error occurred executing the pending operations on the sink. + * @param videoSink The {@link VideoSink} to execute the operations on, or {@code null} to remove + * the underlying {@link VideoSink}. */ - public void setVideoSink(VideoSink videoSink) throws VideoSinkException { + public void setVideoSink(@Nullable VideoSink videoSink) { this.videoSink = videoSink; + if (videoSink == null) { + return; + } for (int i = 0; i < pendingOperations.size(); i++) { pendingOperations.get(i).execute(videoSink); } pendingOperations.clear(); } - /** - * Removes the underlying {@link VideoSink} if it is {@linkplain #setVideoSink(VideoSink) set}. - */ - public void removeVideoSink() { - this.videoSink = null; - } - /** Returns the underlying {@link VideoSink} or {@code null} if there is none. */ @Nullable public VideoSink getVideoSink() { @@ -107,17 +106,19 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; executeOrDelay(videoSink -> videoSink.setListener(listener, executor)); } + /** + * {@inheritDoc} + * + *

This operation won't be added to the pending operations if the {@linkplain + * #setVideoSink(VideoSink) underlying sink} is {@code null}. + * + *

{@code true} is always returned if the {@linkplain #setVideoSink(VideoSink) underlying sink} + * is {@code null}. + */ @Override public boolean initialize(Format sourceFormat) throws VideoSinkException { - executeOrDelayThrowing( - videoSink -> { - if (videoSink.isInitialized()) { - return; - } - videoSink.initialize(sourceFormat); - }); - isInitialized = true; - return true; + isInitialized = videoSink == null || videoSink.initialize(sourceFormat); + return isInitialized; } @Override @@ -130,8 +131,16 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; executeOrDelay(videoSink -> videoSink.flush(resetPosition)); } + /** + * {@inheritDoc} + * + *

{@code true} is always returned if the {@linkplain #setVideoSink(VideoSink) underlying sink} + * is {@code null}. + */ @Override public boolean isReady(boolean rendererOtherwiseReady) { + // Return true if the VideoSink is null to indicate that the renderer can be started. Indeed, + // for prewarming, a VideoSink is set on the BufferingVideoSink when the renderer is started. return videoSink == null || videoSink.isReady(rendererOtherwiseReady); } @@ -145,6 +154,12 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; executeOrDelay(VideoSink::signalEndOfInput); } + /** + * {@inheritDoc} + * + *

{@code false} is always returned if the {@linkplain #setVideoSink(VideoSink) underlying + * sink} is {@code null}. + */ @Override public boolean isEnded() { return videoSink != null && videoSink.isEnded(); @@ -212,6 +227,12 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; executeOrDelay(videoSink -> videoSink.onInputStreamChanged(inputType, format, videoEffects)); } + /** + * {@inheritDoc} + * + *

{@code false} is always returned if the {@linkplain #setVideoSink(VideoSink) underlying + * sink} is {@code null}. + */ @Override public boolean handleInputFrame( long framePresentationTimeUs, boolean isLastFrame, VideoFrameHandler videoFrameHandler) { @@ -219,11 +240,23 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; && videoSink.handleInputFrame(framePresentationTimeUs, isLastFrame, videoFrameHandler); } + /** + * {@inheritDoc} + * + *

{@code false} is always returned if the {@linkplain #setVideoSink(VideoSink) underlying + * sink} is {@code null}. + */ @Override public boolean handleInputBitmap(Bitmap inputBitmap, TimestampIterator timestampIterator) { return videoSink != null && videoSink.handleInputBitmap(inputBitmap, timestampIterator); } + /** + * {@inheritDoc} + * + *

This operation won't be added to the pending operations if the {@linkplain + * #setVideoSink(VideoSink) underlying sink} is {@code null}. + */ @Override public void render(long positionUs, long elapsedRealtimeUs) throws VideoSinkException { if (videoSink != null) { @@ -257,15 +290,6 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; } } - private void executeOrDelayThrowing(ThrowingVideoSinkOperation operation) - throws VideoSinkException { - if (videoSink != null) { - operation.execute(videoSink); - } else { - pendingOperations.add(operation); - } - } - private PlaceholderSurface getPlaceholderSurface() { if (placeholderSurface == null) { placeholderSurface = PlaceholderSurface.newInstance(context, /* secure= */ false); @@ -273,14 +297,8 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; return placeholderSurface; } - private interface ThrowingVideoSinkOperation { + private interface VideoSinkOperation { - void execute(VideoSink videoSink) throws VideoSinkException; - } - - private interface VideoSinkOperation extends ThrowingVideoSinkOperation { - - @Override void execute(VideoSink videoSink); } } diff --git a/libraries/transformer/src/test/java/androidx/media3/transformer/BufferingVideoSinkTest.java b/libraries/transformer/src/test/java/androidx/media3/transformer/BufferingVideoSinkTest.java index fe1625490f..1c8a87a168 100644 --- a/libraries/transformer/src/test/java/androidx/media3/transformer/BufferingVideoSinkTest.java +++ b/libraries/transformer/src/test/java/androidx/media3/transformer/BufferingVideoSinkTest.java @@ -15,15 +15,11 @@ */ package androidx.media3.transformer; -import static org.junit.Assert.assertThrows; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import android.content.Context; -import androidx.media3.common.Format; -import androidx.media3.common.MimeTypes; import androidx.media3.exoplayer.video.VideoSink; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; @@ -39,7 +35,7 @@ public class BufferingVideoSinkTest { private final Context context = ApplicationProvider.getApplicationContext(); @Test - public void executeOperation_withVideoSinkSet_callsVideoSink() throws Exception { + public void executeOperation_withVideoSinkSet_callsVideoSink() { BufferingVideoSink bufferingVideoSink = new BufferingVideoSink(context); VideoSink videoSinkMock = mock(VideoSink.class); @@ -53,7 +49,7 @@ public class BufferingVideoSinkTest { } @Test - public void setVideoSink_executesPendingOperations() throws Exception { + public void setVideoSink_executesPendingOperations() { BufferingVideoSink bufferingVideoSink = new BufferingVideoSink(context); VideoSink videoSinkMock = mock(VideoSink.class); @@ -67,27 +63,12 @@ public class BufferingVideoSinkTest { } @Test - public void setVideoSink_withFailingPendingOperation_throws() throws Exception { - BufferingVideoSink bufferingVideoSink = new BufferingVideoSink(context); - VideoSink videoSinkMock = mock(VideoSink.class); - Format format = new Format.Builder().setSampleMimeType(MimeTypes.VIDEO_H264).build(); - Mockito.doThrow(new VideoSink.VideoSinkException(new RuntimeException(), format)) - .when(videoSinkMock) - .initialize(any()); - - bufferingVideoSink.initialize(format); - - assertThrows( - VideoSink.VideoSinkException.class, () -> bufferingVideoSink.setVideoSink(videoSinkMock)); - } - - @Test - public void removeVideoSink_thenExecuteOperations_doesNotCallVideoSink() throws Exception { + public void setNullVideoSink_thenExecuteOperations_doesNotCallVideoSink() { BufferingVideoSink bufferingVideoSink = new BufferingVideoSink(context); VideoSink videoSinkMock = mock(VideoSink.class); bufferingVideoSink.setVideoSink(videoSinkMock); - bufferingVideoSink.removeVideoSink(); + bufferingVideoSink.setVideoSink(null); bufferingVideoSink.onRendererEnabled(/* mayRenderStartOfStream= */ true); bufferingVideoSink.onRendererStarted(); @@ -96,7 +77,7 @@ public class BufferingVideoSinkTest { } @Test - public void clearPendingOperations_clearsPendingOperations() throws Exception { + public void clearPendingOperations_clearsPendingOperations() { BufferingVideoSink bufferingVideoSink = new BufferingVideoSink(context); VideoSink videoSinkMock = mock(VideoSink.class);