From 346b9257ba01cdaacc2e2b2fe4d0b9354e45660d Mon Sep 17 00:00:00 2001 From: huangdarwin Date: Thu, 26 Oct 2023 11:04:11 -0700 Subject: [PATCH] Flush: VideoFrameProcessor texture output PiperOrigin-RevId: 576928149 --- .../effect/DefaultVideoFrameProcessor.java | 10 +++++++ .../effect/FinalShaderProgramWrapper.java | 29 +++++++++++++------ ...ocessorMultipleTextureOutputPixelTest.java | 26 +++++++++++++++++ 3 files changed, 56 insertions(+), 9 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 67ab19bac2..df4ffcf622 100644 --- a/libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoFrameProcessor.java +++ b/libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoFrameProcessor.java @@ -547,6 +547,16 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { inputSwitcher.signalEndOfInputStream(); } + /** + * {@inheritDoc} + * + *

The downstream frame consumer must be flushed before this instance is flushed, and stop + * accepting input until this DefaultVideoFrameProcessor instance finishes flushing. + * + *

After this method is called, any object consuming {@linkplain + * Factory.Builder#setTextureOutput texture output} must not access any output textures that were + * {@link GlTextureProducer.Listener#onTextureRendered rendered} before calling this method. + */ @Override public void flush() { if (!inputSwitcher.hasActiveInput()) { 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 8e7b92dfdc..a9870c0086 100644 --- a/libraries/effect/src/main/java/androidx/media3/effect/FinalShaderProgramWrapper.java +++ b/libraries/effect/src/main/java/androidx/media3/effect/FinalShaderProgramWrapper.java @@ -155,11 +155,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @Override public void setInputListener(InputListener inputListener) { this.inputListener = inputListener; - int inputCapacity = - textureOutputListener == null - ? SURFACE_INPUT_CAPACITY - : outputTexturePool.freeTextureCount(); - for (int i = 0; i < inputCapacity; i++) { + for (int i = 0; i < getInputCapacity(); i++) { inputListener.onReadyToAcceptInputFrame(); } } @@ -254,20 +250,35 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @Override public void flush() { + // The downstream consumer must already have been flushed, so the textureOutputListener + // implementation does not access its previously output textures, per its contract. However, the + // downstream consumer may not have called releaseOutputTexture on all these textures. Release + // all output textures that aren't already released. + if (textureOutputListener != null) { + outputTexturePool.freeAllTextures(); + outputTextureTimestamps.clear(); + syncObjects.clear(); + } + // Drops all frames that aren't rendered yet. availableFrames.clear(); if (defaultShaderProgram != null) { defaultShaderProgram.flush(); } + + // Signal flush upstream. inputListener.onFlush(); - if (textureOutputListener == null) { - // TODO: b/293572152 - Add texture output flush() support, propagating the flush() signal to - // downstream components so that they can release TexturePool resources and FinalWrapper can - // call onReadyToAcceptInputFrame(). + for (int i = 0; i < getInputCapacity(); i++) { inputListener.onReadyToAcceptInputFrame(); } } + private int getInputCapacity() { + return textureOutputListener == null + ? SURFACE_INPUT_CAPACITY + : outputTexturePool.freeTextureCount(); + } + @Override public synchronized void release() throws VideoFrameProcessingException { if (defaultShaderProgram != null) { diff --git a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/DefaultVideoFrameProcessorMultipleTextureOutputPixelTest.java b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/DefaultVideoFrameProcessorMultipleTextureOutputPixelTest.java index 65cb33dcee..242b820d99 100644 --- a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/DefaultVideoFrameProcessorMultipleTextureOutputPixelTest.java +++ b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/mh/DefaultVideoFrameProcessorMultipleTextureOutputPixelTest.java @@ -119,6 +119,32 @@ public class DefaultVideoFrameProcessorMultipleTextureOutputPixelTest { .isAtMost(MAXIMUM_AVERAGE_PIXEL_ABSOLUTE_DIFFERENCE_DIFFERENT_DEVICE); } + // This tests a condition that is difficult to synchronize, and is subject to a race + // condition. It may flake/fail if any queued frames are processed in the VideoFrameProcessor + // thread, before flush begins and cancels these pending frames. However, this is better than not + // testing this behavior at all, and in practice has succeeded every time on a 1000-time run. + // TODO: b/302695659 - Make this test more deterministic. + @Test + public void textureOutput_queueFiveBitmapsAndFlush_outputsOnlyAfterFlush() throws Exception { + String testId = "textureOutput_queueFiveBitmapsAndFlush_outputsOnlyAfterFlush"; + videoFrameProcessorTestRunner = getFrameProcessorTestRunnerBuilder(testId).build(); + ImmutableList inputTimestamps1 = ImmutableList.of(1_000_000L, 2_000_000L, 3_000_000L); + ImmutableList inputTimestamps2 = ImmutableList.of(4_000_000L, 5_000_000L, 6_000_000L); + + queueBitmaps(videoFrameProcessorTestRunner, ORIGINAL_PNG_ASSET_PATH, inputTimestamps1); + videoFrameProcessorTestRunner.flush(); + queueBitmaps(videoFrameProcessorTestRunner, MEDIA3_TEST_PNG_ASSET_PATH, inputTimestamps2); + videoFrameProcessorTestRunner.endFrameProcessing(); + + TextureBitmapReader textureBitmapReader = checkNotNull(this.textureBitmapReader); + Set actualOutputTimestamps = textureBitmapReader.getOutputTimestamps(); + assertThat(actualOutputTimestamps).containsAtLeastElementsIn(inputTimestamps2).inOrder(); + // This assertion is subject to flaking, per test comments. If it flakes, consider increasing + // the number of elements in inputTimestamps2. + assertThat(actualOutputTimestamps.size()) + .isLessThan(inputTimestamps1.size() + inputTimestamps2.size()); + } + private void queueBitmaps( VideoFrameProcessorTestRunner videoFrameProcessorTestRunner, String bitmapAssetPath,