From 0d30edae75e3b506513a1124d7b4e5382f366414 Mon Sep 17 00:00:00 2001 From: huangdarwin Date: Thu, 6 Apr 2023 17:55:42 +0100 Subject: [PATCH] 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 --- .../effect/DefaultVideoFrameProcessor.java | 79 +++++++++---------- .../effect/FinalShaderProgramWrapper.java | 18 ++--- .../effect/VideoFrameProcessingTask.java | 6 +- .../VideoFrameProcessingTaskExecutor.java | 5 +- .../utils/VideoFrameProcessorTestRunner.java | 49 ++++-------- ...oFrameProcessorTextureOutputPixelTest.java | 35 ++------ 6 files changed, 70 insertions(+), 122 deletions(-) diff --git a/libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoFrameProcessor.java b/libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoFrameProcessor.java index 2a6523c2f7..86a269c538 100644 --- a/libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoFrameProcessor.java +++ b/libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoFrameProcessor.java @@ -15,6 +15,7 @@ */ package androidx.media3.effect; +import static androidx.annotation.VisibleForTesting.PACKAGE_PRIVATE; import static androidx.media3.common.util.Assertions.checkArgument; import static androidx.media3.common.util.Assertions.checkNotNull; import static androidx.media3.common.util.Assertions.checkState; @@ -63,12 +64,21 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @UnstableApi 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() { @@ -81,25 +91,39 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { *

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}. + * + *

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; } @@ -109,7 +133,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. @@ -119,26 +143,11 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { *

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. - * - *

Must be called before {@link VideoFrameProcessor.Factory#create}. - * - *

The default value is {@code false}. - */ - @VisibleForTesting - public Factory setOutputToTexture(boolean outputToTexture) { - this.outputToTexture = outputToTexture; - return this; - } - /** * {@inheritDoc} * @@ -223,7 +232,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { listenerExecutor, listener, glObjectsProvider, - outputToTexture)); + textureOutputListener)); try { return defaultVideoFrameProcessorFuture.get(); @@ -359,20 +368,6 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { finalShaderProgramWrapper.setOutputSurfaceInfo(outputSurfaceInfo); } - /** - * Gets the output {@link GlTextureInfo}. - * - *

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( @@ -468,7 +463,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)); @@ -513,7 +508,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { executor, listener, glObjectsProvider, - outputToTexture); + textureOutputListener); setGlObjectProviderOnShaderPrograms(shaderPrograms, glObjectsProvider); VideoFrameProcessingTaskExecutor videoFrameProcessingTaskExecutor = new VideoFrameProcessingTaskExecutor(singleThreadExecutorService, listener); @@ -554,7 +549,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { Executor executor, Listener listener, GlObjectsProvider glObjectsProvider, - boolean outputToTexture) + @Nullable TextureOutputListener textureOutputListener) throws VideoFrameProcessingException { ImmutableList.Builder shaderProgramListBuilder = new ImmutableList.Builder<>(); ImmutableList.Builder matrixTransformationListBuilder = @@ -640,7 +635,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { executor, listener, glObjectsProvider, - outputToTexture)); + textureOutputListener)); return shaderProgramListBuilder.build(); } diff --git a/libraries/effect/src/main/java/androidx/media3/effect/FinalShaderProgramWrapper.java b/libraries/effect/src/main/java/androidx/media3/effect/FinalShaderProgramWrapper.java index 516ab60719..e654e1c55c 100644 --- a/libraries/effect/src/main/java/androidx/media3/effect/FinalShaderProgramWrapper.java +++ b/libraries/effect/src/main/java/androidx/media3/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 androidx.media3.common.C; import androidx.media3.common.ColorInfo; import androidx.media3.common.DebugViewProvider; @@ -84,7 +83,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; private final float[] textureTransformMatrix; private final Queue streamOffsetUsQueue; private final Queue> 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()); diff --git a/libraries/effect/src/main/java/androidx/media3/effect/VideoFrameProcessingTask.java b/libraries/effect/src/main/java/androidx/media3/effect/VideoFrameProcessingTask.java index 77feee2cae..d62e78195c 100644 --- a/libraries/effect/src/main/java/androidx/media3/effect/VideoFrameProcessingTask.java +++ b/libraries/effect/src/main/java/androidx/media3/effect/VideoFrameProcessingTask.java @@ -15,9 +15,6 @@ */ package androidx.media3.effect; -import static androidx.annotation.VisibleForTesting.PACKAGE_PRIVATE; - -import androidx.annotation.VisibleForTesting; import androidx.media3.common.VideoFrameProcessingException; import androidx.media3.common.util.GlUtil; import androidx.media3.common.util.UnstableApi; @@ -27,8 +24,7 @@ import androidx.media3.common.util.UnstableApi; * VideoFrameProcessingException}. */ @UnstableApi -@VisibleForTesting(otherwise = PACKAGE_PRIVATE) -public interface VideoFrameProcessingTask { +/* package */ interface VideoFrameProcessingTask { /** Runs the task. */ void run() throws VideoFrameProcessingException, GlUtil.GlException; } diff --git a/libraries/effect/src/main/java/androidx/media3/effect/VideoFrameProcessingTaskExecutor.java b/libraries/effect/src/main/java/androidx/media3/effect/VideoFrameProcessingTaskExecutor.java index 37f2968885..274aabea2e 100644 --- a/libraries/effect/src/main/java/androidx/media3/effect/VideoFrameProcessingTaskExecutor.java +++ b/libraries/effect/src/main/java/androidx/media3/effect/VideoFrameProcessingTaskExecutor.java @@ -15,12 +15,10 @@ */ package androidx.media3.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 androidx.media3.common.VideoFrameProcessingException; import androidx.media3.common.VideoFrameProcessor; import androidx.media3.common.util.UnstableApi; @@ -48,8 +46,7 @@ import java.util.concurrent.RejectedExecutionException; * equal priority are executed in FIFO order. */ @UnstableApi -@VisibleForTesting(otherwise = PACKAGE_PRIVATE) -public final class VideoFrameProcessingTaskExecutor { +/* package */ final class VideoFrameProcessingTaskExecutor { private final ExecutorService singleThreadExecutorService; private final VideoFrameProcessor.Listener listener; diff --git a/libraries/test_utils/src/main/java/androidx/media3/test/utils/VideoFrameProcessorTestRunner.java b/libraries/test_utils/src/main/java/androidx/media3/test/utils/VideoFrameProcessorTestRunner.java index f1f7125a80..0b1b8e8d77 100644 --- a/libraries/test_utils/src/main/java/androidx/media3/test/utils/VideoFrameProcessorTestRunner.java +++ b/libraries/test_utils/src/main/java/androidx/media3/test/utils/VideoFrameProcessorTestRunner.java @@ -58,7 +58,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 effects; @@ -99,13 +99,13 @@ public final class VideoFrameProcessorTestRunner { } /** - * Sets the {@link BitmapReader.Factory}. + * Sets the {@link BitmapReader}. * - *

The default value is {@link SurfaceBitmapReader.Factory}. + *

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; } @@ -218,7 +218,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, @@ -250,7 +250,7 @@ public final class VideoFrameProcessorTestRunner { private VideoFrameProcessorTestRunner( String testId, VideoFrameProcessor.Factory videoFrameProcessorFactory, - BitmapReader.Factory bitmapReaderFactory, + BitmapReader bitmapReader, @Nullable String videoAssetPath, String outputFileLabel, ImmutableList effects, @@ -261,6 +261,7 @@ public final class VideoFrameProcessorTestRunner { OnOutputFrameAvailableListener onOutputFrameAvailableListener) throws VideoFrameProcessingException { this.testId = testId; + this.bitmapReader = bitmapReader; this.videoAssetPath = videoAssetPath; this.outputFileLabel = outputFileLabel; this.pixelWidthHeightRatio = pixelWidthHeightRatio; @@ -279,11 +280,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 @@ -360,12 +359,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(); @@ -378,26 +374,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; - - @SuppressLint("WrongConstant") - private SurfaceBitmapReader(int width, int height) { - imageReader = - ImageReader.newInstance(width, height, PixelFormat.RGBA_8888, /* maxImages= */ 1); - } + private @MonotonicNonNull ImageReader imageReader; @Override - public Surface getSurface() { + @SuppressLint("WrongConstant") + public Surface getSurface(int width, int height) { + imageReader = + ImageReader.newInstance(width, height, PixelFormat.RGBA_8888, /* maxImages= */ 1); return imageReader.getSurface(); } diff --git a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/DefaultVideoFrameProcessorTextureOutputPixelTest.java b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/DefaultVideoFrameProcessorTextureOutputPixelTest.java index 83b49a1479..ace0bdfe47 100644 --- a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/DefaultVideoFrameProcessorTextureOutputPixelTest.java +++ b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/DefaultVideoFrameProcessorTextureOutputPixelTest.java @@ -26,7 +26,6 @@ import android.graphics.Bitmap; import android.graphics.SurfaceTexture; import android.view.Surface; import androidx.media3.common.GlTextureInfo; -import androidx.media3.common.VideoFrameProcessor; import androidx.media3.common.util.GlUtil; import androidx.media3.effect.BitmapOverlay; import androidx.media3.effect.DefaultVideoFrameProcessor; @@ -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 =