Various improvements to BufferingVideoSink

PiperOrigin-RevId: 717807436

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.
+ *
+ * <p>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<ThrowingVideoSinkOperation> pendingOperations;
+  private final List<VideoSinkOperation> 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.
+   * @param videoSink The {@link VideoSink} to execute the operations on, or {@code null} to remove
-   * @throws VideoSinkException If an error occurred executing the pending operations on the sink.
+   *     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}
+   *
+   * <p>This operation won't be added to the pending operations if the {@linkplain
+   * #setVideoSink(VideoSink) underlying sink} is {@code null}.
+   *
+   * <p>{@code true} is always returned if the {@linkplain #setVideoSink(VideoSink) underlying sink}
+   * is {@code null}.
+   */
   @Override
   public boolean initialize(Format sourceFormat) throws VideoSinkException {
-    executeOrDelayThrowing(
+    isInitialized = videoSink == null || videoSink.initialize(sourceFormat);
-        videoSink -> {
+    return isInitialized;
-          if (videoSink.isInitialized()) {
-            return;
-          }
-          videoSink.initialize(sourceFormat);
-        });
-    isInitialized = true;
-    return true;
   }
 
   @Override
@@ -130,8 +131,16 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
     executeOrDelay(videoSink -> videoSink.flush(resetPosition));
   }
 
+  /**
+   * {@inheritDoc}
+   *
+   * <p>{@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}
+   *
+   * <p>{@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}
+   *
+   * <p>{@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}
+   *
+   * <p>{@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}
+   *
+   * <p>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);
   }
 }

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 {
+  public void setNullVideoSink_thenExecuteOperations_doesNotCallVideoSink() {
-    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 {
     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);