Test: Add TextureOutputListener for texture output tests

Before this CL, SurfaceTexture.onFrameAvailable was used to tell whether a frame
was available in the VideoFrameProcessor's output texture. This was incorrect, as
it would rely on having the texture be written to before the
SurfaceTexture.onFrameAvailableListener is invoked, leading to null-pointer-
exceptions on timeouts.

Instead of using DefaultVideoFrameProcessor different interfaces to set that we
want to output to a texture, and get that output texture, use one interface that
sets a listener, and renders to a texture iff that listener is set. As this
listener is executed on the GL thread, this also allows us to no longer need to
expand visibility for the GL task executor and tasks.

PiperOrigin-RevId: 522362101

library/effect/src/main/java/com/google/android/exoplayer2/effect/DefaultVideoFrameProcessor.java

@@ -15,6 +15,7 @@
  */
 package com.google.android.exoplayer2.effect;
 
+import static androidx.annotation.VisibleForTesting.PACKAGE_PRIVATE;
 import static com.google.android.exoplayer2.util.Assertions.checkArgument;
 import static com.google.android.exoplayer2.util.Assertions.checkNotNull;
 import static com.google.android.exoplayer2.util.Assertions.checkState;
@@ -61,12 +62,21 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
  */
 public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
 
+  /** Listener interface for texture output. */
+  @VisibleForTesting(otherwise = PACKAGE_PRIVATE)
+  public interface TextureOutputListener {
+    /** Called when a texture has been rendered to. */
+    void onTextureRendered(GlTextureInfo outputTexture, long presentationTimeUs)
+        throws GlUtil.GlException;
+  }
+
   /** A factory for {@link DefaultVideoFrameProcessor} instances. */
   public static final class Factory implements VideoFrameProcessor.Factory {
 
     /** A builder for {@link DefaultVideoFrameProcessor.Factory} instances. */
     public static final class Builder {
       private boolean enableColorTransfers;
+      @Nullable private TextureOutputListener textureOutputListener;
 
       /** Creates an instance. */
       public Builder() {
@@ -79,25 +89,39 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
        * <p>If the input or output is HDR, this must be {@code true}.
        */
       @CanIgnoreReturnValue
-      public DefaultVideoFrameProcessor.Factory.Builder setEnableColorTransfers(
-          boolean enableColorTransfers) {
+      public Builder setEnableColorTransfers(boolean enableColorTransfers) {
         this.enableColorTransfers = enableColorTransfers;
         return this;
       }
 
+      /**
+       * Sets the {@link TextureOutputListener}.
+       *
+       * <p>If set, the {@link VideoFrameProcessor} will output to an OpenGL texture.
+       */
+      @VisibleForTesting
+      @CanIgnoreReturnValue
+      public Builder setOnTextureRenderedListener(TextureOutputListener textureOutputListener) {
+        this.textureOutputListener = textureOutputListener;
+        return this;
+      }
+
       /** Builds an {@link DefaultVideoFrameProcessor.Factory} instance. */
       public DefaultVideoFrameProcessor.Factory build() {
-        return new DefaultVideoFrameProcessor.Factory(enableColorTransfers);
+        return new DefaultVideoFrameProcessor.Factory(enableColorTransfers, textureOutputListener);
       }
     }
 
     /** Whether to transfer colors to an intermediate color space when applying effects. */
-    public final boolean enableColorTransfers;
+    private final boolean enableColorTransfers;
+
+    @Nullable private final TextureOutputListener textureOutputListener;
 
     private GlObjectsProvider glObjectsProvider = GlObjectsProvider.DEFAULT;
-    private boolean outputToTexture;
 
-    private Factory(boolean enableColorTransfers) {
+    private Factory(
+        boolean enableColorTransfers, @Nullable TextureOutputListener textureOutputListener) {
+      this.textureOutputListener = textureOutputListener;
       this.enableColorTransfers = enableColorTransfers;
     }
 
@@ -107,7 +131,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
      */
     @Deprecated
     public Factory() {
-      this(/* enableColorTransfers= */ true);
+      this(/* enableColorTransfers= */ true, /* textureOutputListener= */ null);
     }
 
     // TODO(276913828): Move this setter to the DefaultVideoFrameProcessor.Factory.Builder.
@@ -117,26 +141,11 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
      * <p>The default value is {@link GlObjectsProvider#DEFAULT}.
      */
     @Override
-    public DefaultVideoFrameProcessor.Factory setGlObjectsProvider(
-        GlObjectsProvider glObjectsProvider) {
+    public Factory setGlObjectsProvider(GlObjectsProvider glObjectsProvider) {
       this.glObjectsProvider = glObjectsProvider;
       return this;
     }
 
-    // TODO(276913828): Move this setter to the DefaultVideoFrameProcessor.Factory.Builder.
-    /**
-     * Sets whether to output to a texture for testing.
-     *
-     * <p>Must be called before {@link VideoFrameProcessor.Factory#create}.
-     *
-     * <p>The default value is {@code false}.
-     */
-    @VisibleForTesting
-    public Factory setOutputToTexture(boolean outputToTexture) {
-      this.outputToTexture = outputToTexture;
-      return this;
-    }
-
     /**
      * {@inheritDoc}
      *
@@ -221,7 +230,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
                       listenerExecutor,
                       listener,
                       glObjectsProvider,
-                      outputToTexture));
+                      textureOutputListener));
 
       try {
         return defaultVideoFrameProcessorFuture.get();
@@ -357,20 +366,6 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
     finalShaderProgramWrapper.setOutputSurfaceInfo(outputSurfaceInfo);
   }
 
-  /**
-   * Gets the output {@link GlTextureInfo}.
-   *
-   * <p>Should only be called if {@code outputToTexture} is true, and after a frame is available, as
-   * reported by the output {@linkplain #setOutputSurfaceInfo surface}'s {@link
-   * SurfaceTexture#setOnFrameAvailableListener}. Returns {@code null} if an output texture is not
-   * yet available.
-   */
-  @VisibleForTesting
-  @Nullable
-  public GlTextureInfo getOutputTextureInfo() {
-    return finalShaderProgramWrapper.getOutputTextureInfo();
-  }
-
   @Override
   public void releaseOutputFrame(long releaseTimeNs) {
     checkState(
@@ -466,7 +461,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
       Executor executor,
       Listener listener,
       GlObjectsProvider glObjectsProvider,
-      boolean outputToTexture)
+      @Nullable TextureOutputListener textureOutputListener)
       throws GlUtil.GlException, VideoFrameProcessingException {
     checkState(Thread.currentThread().getName().equals(THREAD_NAME));
 
@@ -511,7 +506,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
             executor,
             listener,
             glObjectsProvider,
-            outputToTexture);
+            textureOutputListener);
     setGlObjectProviderOnShaderPrograms(shaderPrograms, glObjectsProvider);
     VideoFrameProcessingTaskExecutor videoFrameProcessingTaskExecutor =
         new VideoFrameProcessingTaskExecutor(singleThreadExecutorService, listener);
@@ -552,7 +547,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
       Executor executor,
       Listener listener,
       GlObjectsProvider glObjectsProvider,
-      boolean outputToTexture)
+      @Nullable TextureOutputListener textureOutputListener)
       throws VideoFrameProcessingException {
     ImmutableList.Builder<GlShaderProgram> shaderProgramListBuilder = new ImmutableList.Builder<>();
     ImmutableList.Builder<GlMatrixTransformation> matrixTransformationListBuilder =
@@ -638,7 +633,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
             executor,
             listener,
             glObjectsProvider,
-            outputToTexture));
+            textureOutputListener));
     return shaderProgramListBuilder.build();
   }
 

library/effect/src/main/java/com/google/android/exoplayer2/effect/FinalShaderProgramWrapper.java

@@ -32,7 +32,6 @@ import android.view.SurfaceHolder;
 import android.view.SurfaceView;
 import androidx.annotation.GuardedBy;
 import androidx.annotation.Nullable;
-import androidx.annotation.VisibleForTesting;
 import com.google.android.exoplayer2.C;
 import com.google.android.exoplayer2.util.DebugViewProvider;
 import com.google.android.exoplayer2.util.GlObjectsProvider;
@@ -84,7 +83,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
   private final float[] textureTransformMatrix;
   private final Queue<Long> streamOffsetUsQueue;
   private final Queue<Pair<GlTextureInfo, Long>> availableFrames;
-  private final boolean outputToTexture;
+  @Nullable private final DefaultVideoFrameProcessor.TextureOutputListener textureOutputListener;
 
   private int inputWidth;
   private int inputHeight;
@@ -123,7 +122,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
       Executor videoFrameProcessorListenerExecutor,
       VideoFrameProcessor.Listener videoFrameProcessorListener,
       GlObjectsProvider glObjectsProvider,
-      boolean outputToTexture) {
+      @Nullable DefaultVideoFrameProcessor.TextureOutputListener textureOutputListener) {
     this.context = context;
     this.matrixTransformations = matrixTransformations;
     this.rgbMatrices = rgbMatrices;
@@ -139,7 +138,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
     this.videoFrameProcessorListenerExecutor = videoFrameProcessorListenerExecutor;
     this.videoFrameProcessorListener = videoFrameProcessorListener;
     this.glObjectsProvider = glObjectsProvider;
-    this.outputToTexture = outputToTexture;
+    this.textureOutputListener = textureOutputListener;
 
     textureTransformMatrix = GlUtil.create4x4IdentityMatrix();
     streamOffsetUsQueue = new ConcurrentLinkedQueue<>();
@@ -310,7 +309,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
       GlTextureInfo inputTexture, long presentationTimeUs, long releaseTimeNs) {
     try {
       maybeRenderFrameToOutputSurface(inputTexture, presentationTimeUs, releaseTimeNs);
-      if (outputToTexture && defaultShaderProgram != null) {
+      if (textureOutputListener != null && defaultShaderProgram != null) {
         renderFrameToOutputTexture(inputTexture, presentationTimeUs);
       }
 
@@ -364,12 +363,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
         outputTexture.fboId, outputTexture.width, outputTexture.height);
     GlUtil.clearOutputFrame();
     checkNotNull(defaultShaderProgram).drawFrame(inputTexture.texId, presentationTimeUs);
-  }
-
-  @VisibleForTesting
-  @Nullable
-  /* package */ GlTextureInfo getOutputTextureInfo() {
-    return outputTexture;
+    checkNotNull(textureOutputListener).onTextureRendered(outputTexture, presentationTimeUs);
   }
 
   /**
@@ -444,7 +438,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
     if (defaultShaderProgram == null) {
       DefaultShaderProgram defaultShaderProgram =
           createDefaultShaderProgramForOutputSurface(outputSurfaceInfo);
-      if (outputToTexture) {
+      if (textureOutputListener != null) {
         configureOutputTexture(
             checkNotNull(outputSizeBeforeSurfaceTransformation).getWidth(),
             checkNotNull(outputSizeBeforeSurfaceTransformation).getHeight());

library/effect/src/main/java/com/google/android/exoplayer2/effect/VideoFrameProcessingTask.java

@@ -15,9 +15,6 @@
  */
 package com.google.android.exoplayer2.effect;
 
-import static androidx.annotation.VisibleForTesting.PACKAGE_PRIVATE;
-
-import androidx.annotation.VisibleForTesting;
 import com.google.android.exoplayer2.util.GlUtil;
 import com.google.android.exoplayer2.util.VideoFrameProcessingException;
 
@@ -25,8 +22,7 @@ import com.google.android.exoplayer2.util.VideoFrameProcessingException;
  * Interface for tasks that may throw a {@link GlUtil.GlException} or {@link
  * VideoFrameProcessingException}.
  */
-@VisibleForTesting(otherwise = PACKAGE_PRIVATE)
-public interface VideoFrameProcessingTask {
+/* package */ interface VideoFrameProcessingTask {
   /** Runs the task. */
   void run() throws VideoFrameProcessingException, GlUtil.GlException;
 }

library/effect/src/main/java/com/google/android/exoplayer2/effect/VideoFrameProcessingTaskExecutor.java

@@ -15,12 +15,10 @@
  */
 package com.google.android.exoplayer2.effect;
 
-import static androidx.annotation.VisibleForTesting.PACKAGE_PRIVATE;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 
 import androidx.annotation.GuardedBy;
 import androidx.annotation.Nullable;
-import androidx.annotation.VisibleForTesting;
 import com.google.android.exoplayer2.util.VideoFrameProcessingException;
 import com.google.android.exoplayer2.util.VideoFrameProcessor;
 import java.util.ArrayDeque;
@@ -46,8 +44,7 @@ import java.util.concurrent.RejectedExecutionException;
  * executed before {@linkplain #submit(VideoFrameProcessingTask) default priority tasks}. Tasks with
  * equal priority are executed in FIFO order.
  */
-@VisibleForTesting(otherwise = PACKAGE_PRIVATE)
-public final class VideoFrameProcessingTaskExecutor {
+/* package */ final class VideoFrameProcessingTaskExecutor {
 
   private final ExecutorService singleThreadExecutorService;
   private final VideoFrameProcessor.Listener listener;

library/transformer/src/androidTest/java/com/google/android/exoplayer2/transformer/mh/DefaultVideoFrameProcessorTextureOutputPixelTest.java

@@ -33,7 +33,6 @@ import com.google.android.exoplayer2.testutil.BitmapPixelTestUtil;
 import com.google.android.exoplayer2.testutil.VideoFrameProcessorTestRunner;
 import com.google.android.exoplayer2.util.GlTextureInfo;
 import com.google.android.exoplayer2.util.GlUtil;
-import com.google.android.exoplayer2.util.VideoFrameProcessor;
 import com.google.common.collect.ImmutableList;
 import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
 import org.junit.After;
@@ -105,13 +104,16 @@ public final class DefaultVideoFrameProcessorTextureOutputPixelTest {
 
   private VideoFrameProcessorTestRunner.Builder getDefaultFrameProcessorTestRunnerBuilder(
       String testId) {
+    TextureBitmapReader textureBitmapReader = new TextureBitmapReader();
     DefaultVideoFrameProcessor.Factory defaultVideoFrameProcessorFactory =
-        new DefaultVideoFrameProcessor.Factory().setOutputToTexture(true);
+        new DefaultVideoFrameProcessor.Factory.Builder()
+            .setOnTextureRenderedListener(textureBitmapReader::readBitmapFromTexture)
+            .build();
     return new VideoFrameProcessorTestRunner.Builder()
         .setTestId(testId)
         .setVideoFrameProcessorFactory(defaultVideoFrameProcessorFactory)
         .setVideoAssetPath(INPUT_SDR_MP4_ASSET_STRING)
-        .setBitmapReaderFactory(new TextureBitmapReader.Factory());
+        .setBitmapReader(textureBitmapReader);
   }
 
   /**
@@ -122,24 +124,11 @@ public final class DefaultVideoFrameProcessorTextureOutputPixelTest {
   private static final class TextureBitmapReader
       implements VideoFrameProcessorTestRunner.BitmapReader {
     // TODO(b/239172735): This outputs an incorrect black output image on emulators.
-    public static final class Factory
-        implements VideoFrameProcessorTestRunner.BitmapReader.Factory {
-      @Override
-      public TextureBitmapReader create(
-          VideoFrameProcessor videoFrameProcessor, int width, int height) {
-        return new TextureBitmapReader((DefaultVideoFrameProcessor) videoFrameProcessor);
-      }
-    }
 
-    private final DefaultVideoFrameProcessor defaultVideoFrameProcessor;
     private @MonotonicNonNull Bitmap outputBitmap;
 
-    private TextureBitmapReader(DefaultVideoFrameProcessor defaultVideoFrameProcessor) {
-      this.defaultVideoFrameProcessor = defaultVideoFrameProcessor;
-    }
-
     @Override
-    public Surface getSurface() {
+    public Surface getSurface(int width, int height) {
       int texId;
       try {
         texId = GlUtil.createExternalTexture();
@@ -147,7 +136,6 @@ public final class DefaultVideoFrameProcessorTextureOutputPixelTest {
         throw new RuntimeException(e);
       }
       SurfaceTexture surfaceTexture = new SurfaceTexture(texId);
-      surfaceTexture.setOnFrameAvailableListener(this::onSurfaceTextureFrameAvailable);
       return new Surface(surfaceTexture);
     }
 
@@ -156,15 +144,8 @@ public final class DefaultVideoFrameProcessorTextureOutputPixelTest {
       return checkStateNotNull(outputBitmap);
     }
 
-    private void onSurfaceTextureFrameAvailable(SurfaceTexture surfaceTexture) {
-      defaultVideoFrameProcessor
-          .getTaskExecutor()
-          .submitWithHighPriority(this::getBitmapFromTexture);
-    }
-
-    private void getBitmapFromTexture() throws GlUtil.GlException {
-      GlTextureInfo outputTexture = checkNotNull(defaultVideoFrameProcessor.getOutputTextureInfo());
-
+    public void readBitmapFromTexture(GlTextureInfo outputTexture, long presentationTimeUs)
+        throws GlUtil.GlException {
       GlUtil.focusFramebufferUsingCurrentContext(
           outputTexture.fboId, outputTexture.width, outputTexture.height);
       outputBitmap =

testutils/src/main/java/com/google/android/exoplayer2/testutil/VideoFrameProcessorTestRunner.java

@@ -56,7 +56,7 @@ public final class VideoFrameProcessorTestRunner {
 
     private @MonotonicNonNull String testId;
     private VideoFrameProcessor.@MonotonicNonNull Factory videoFrameProcessorFactory;
-    private BitmapReader.@MonotonicNonNull Factory bitmapReaderFactory;
+    private @MonotonicNonNull BitmapReader bitmapReader;
     private @MonotonicNonNull String videoAssetPath;
     private @MonotonicNonNull String outputFileLabel;
     private @MonotonicNonNull ImmutableList<Effect> effects;
@@ -97,13 +97,13 @@ public final class VideoFrameProcessorTestRunner {
     }
 
     /**
-     * Sets the {@link BitmapReader.Factory}.
+     * Sets the {@link BitmapReader}.
      *
-     * <p>The default value is {@link SurfaceBitmapReader.Factory}.
+     * <p>The default value is a {@link SurfaceBitmapReader} instance.
      */
     @CanIgnoreReturnValue
-    public Builder setBitmapReaderFactory(BitmapReader.Factory bitmapReaderFactory) {
-      this.bitmapReaderFactory = bitmapReaderFactory;
+    public Builder setBitmapReader(BitmapReader bitmapReader) {
+      this.bitmapReader = bitmapReader;
       return this;
     }
 
@@ -216,7 +216,7 @@ public final class VideoFrameProcessorTestRunner {
       return new VideoFrameProcessorTestRunner(
           testId,
           videoFrameProcessorFactory,
-          bitmapReaderFactory == null ? new SurfaceBitmapReader.Factory() : bitmapReaderFactory,
+          bitmapReader == null ? new SurfaceBitmapReader() : bitmapReader,
           videoAssetPath,
           outputFileLabel == null ? "" : outputFileLabel,
           effects == null ? ImmutableList.of() : effects,
@@ -248,7 +248,7 @@ public final class VideoFrameProcessorTestRunner {
   private VideoFrameProcessorTestRunner(
       String testId,
       VideoFrameProcessor.Factory videoFrameProcessorFactory,
-      BitmapReader.Factory bitmapReaderFactory,
+      BitmapReader bitmapReader,
       @Nullable String videoAssetPath,
       String outputFileLabel,
       ImmutableList<Effect> effects,
@@ -259,6 +259,7 @@ public final class VideoFrameProcessorTestRunner {
       OnOutputFrameAvailableListener onOutputFrameAvailableListener)
       throws VideoFrameProcessingException {
     this.testId = testId;
+    this.bitmapReader = bitmapReader;
     this.videoAssetPath = videoAssetPath;
     this.outputFileLabel = outputFileLabel;
     this.pixelWidthHeightRatio = pixelWidthHeightRatio;
@@ -277,11 +278,9 @@ public final class VideoFrameProcessorTestRunner {
             new VideoFrameProcessor.Listener() {
               @Override
               public void onOutputSizeChanged(int width, int height) {
-                bitmapReader =
-                    bitmapReaderFactory.create(checkNotNull(videoFrameProcessor), width, height);
-                Surface outputSurface = bitmapReader.getSurface();
-                videoFrameProcessor.setOutputSurfaceInfo(
-                    new SurfaceInfo(outputSurface, width, height));
+                Surface outputSurface = bitmapReader.getSurface(width, height);
+                checkNotNull(videoFrameProcessor)
+                    .setOutputSurfaceInfo(new SurfaceInfo(outputSurface, width, height));
               }
 
               @Override
@@ -358,12 +357,9 @@ public final class VideoFrameProcessorTestRunner {
 
   /** Reads a {@link Bitmap} from {@link VideoFrameProcessor} output. */
   public interface BitmapReader {
-    interface Factory {
-      BitmapReader create(VideoFrameProcessor videoFrameProcessor, int width, int height);
-    }
 
     /** Returns the {@link VideoFrameProcessor} output {@link Surface}. */
-    Surface getSurface();
+    Surface getSurface(int width, int height);
 
     /** Returns the output {@link Bitmap}. */
     Bitmap getBitmap();
@@ -376,26 +372,15 @@ public final class VideoFrameProcessorTestRunner {
    */
   public static final class SurfaceBitmapReader
       implements VideoFrameProcessorTestRunner.BitmapReader {
-    public static final class Factory
-        implements VideoFrameProcessorTestRunner.BitmapReader.Factory {
-      @Override
-      public SurfaceBitmapReader create(
-          VideoFrameProcessor videoFrameProcessor, int width, int height) {
-        return new SurfaceBitmapReader(width, height);
-      }
-    }
 
     // ImageReader only supports SDR input.
-    private final ImageReader imageReader;
+    private @MonotonicNonNull ImageReader imageReader;
 
+    @Override
     @SuppressLint("WrongConstant")
-    private SurfaceBitmapReader(int width, int height) {
+    public Surface getSurface(int width, int height) {
       imageReader =
           ImageReader.newInstance(width, height, PixelFormat.RGBA_8888, /* maxImages= */ 1);
-    }
-
-    @Override
-    public Surface getSurface() {
       return imageReader.getSurface();
     }