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 853b3c38d4..9049607fc6 100644 --- a/libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoFrameProcessor.java +++ b/libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoFrameProcessor.java @@ -99,18 +99,19 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { /** A factory for {@link DefaultVideoFrameProcessor} instances. */ public static final class Factory implements VideoFrameProcessor.Factory { + private static final String THREAD_NAME = "Effect:GlThread"; /** A builder for {@link DefaultVideoFrameProcessor.Factory} instances. */ public static final class Builder { private boolean enableColorTransfers; - private GlObjectsProvider glObjectsProvider; - @Nullable private TextureOutputListener textureOutputListener; + private @MonotonicNonNull GlObjectsProvider glObjectsProvider; + @Nullable private ExecutorService executorService; + private @MonotonicNonNull TextureOutputListener textureOutputListener; private int textureOutputCapacity; /** Creates an instance. */ public Builder() { enableColorTransfers = true; - glObjectsProvider = new DefaultGlObjectsProvider(); } /** @@ -135,6 +136,24 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { return this; } + /** + * Sets the {@link Util#newSingleThreadScheduledExecutor} to execute GL commands from. + * + *

If set and non-null, the {@link ExecutorService} must be {@link + * ExecutorService#shutdown} by the caller. + * + *

The default value is a new {@link Util#newSingleThreadScheduledExecutor}, owned and + * {@link ExecutorService#shutdown} by the created {@link DefaultVideoFrameProcessor}. + * + * @param executorService The {@link ExecutorService}. + */ + @CanIgnoreReturnValue + @VisibleForTesting(otherwise = PACKAGE_PRIVATE) + public Builder setExecutorService(@Nullable ExecutorService executorService) { + this.executorService = executorService; + return this; + } + /** * Sets texture output settings. * @@ -168,22 +187,29 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { /** Builds an {@link DefaultVideoFrameProcessor.Factory} instance. */ public DefaultVideoFrameProcessor.Factory build() { return new DefaultVideoFrameProcessor.Factory( - enableColorTransfers, glObjectsProvider, textureOutputListener, textureOutputCapacity); + enableColorTransfers, + glObjectsProvider == null ? new DefaultGlObjectsProvider() : glObjectsProvider, + executorService, + textureOutputListener, + textureOutputCapacity); } } private final boolean enableColorTransfers; private final GlObjectsProvider glObjectsProvider; + @Nullable private final ExecutorService executorService; @Nullable private final TextureOutputListener textureOutputListener; private final int textureOutputCapacity; private Factory( boolean enableColorTransfers, GlObjectsProvider glObjectsProvider, + @Nullable ExecutorService executorService, @Nullable TextureOutputListener textureOutputListener, int textureOutputCapacity) { this.enableColorTransfers = enableColorTransfers; this.glObjectsProvider = glObjectsProvider; + this.executorService = executorService; this.textureOutputListener = textureOutputListener; this.textureOutputCapacity = textureOutputCapacity; } @@ -254,10 +280,15 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { checkArgument(outputColorInfo.colorTransfer == C.COLOR_TRANSFER_GAMMA_2_2); } - ExecutorService singleThreadExecutorService = Util.newSingleThreadExecutor(THREAD_NAME); + boolean shouldShutdownExecutorService = executorService == null; + ExecutorService instanceExecutorService = + executorService == null ? Util.newSingleThreadExecutor(THREAD_NAME) : executorService; + VideoFrameProcessingTaskExecutor videoFrameProcessingTaskExecutor = + new VideoFrameProcessingTaskExecutor( + instanceExecutorService, shouldShutdownExecutorService, listener); Future defaultVideoFrameProcessorFuture = - singleThreadExecutorService.submit( + instanceExecutorService.submit( () -> createOpenGlObjectsAndFrameProcessor( context, @@ -266,7 +297,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { outputColorInfo, enableColorTransfers, renderFramesAutomatically, - singleThreadExecutorService, + videoFrameProcessingTaskExecutor, listenerExecutor, listener, glObjectsProvider, @@ -285,8 +316,6 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { } private static final String TAG = "DefaultFrameProcessor"; - private static final String THREAD_NAME = "Effect:GlThread"; - private static final long RELEASE_WAIT_TIME_MS = 500; private final Context context; private final GlObjectsProvider glObjectsProvider; @@ -524,8 +553,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { @Override public void release() { try { - videoFrameProcessingTaskExecutor.release( - /* releaseTask= */ this::releaseGlObjects, RELEASE_WAIT_TIME_MS); + videoFrameProcessingTaskExecutor.release(/* releaseTask= */ this::releaseGlObjects); } catch (InterruptedException unexpected) { Thread.currentThread().interrupt(); throw new IllegalStateException(unexpected); @@ -562,8 +590,8 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { * *

All {@link Effect} instances must be {@link GlEffect} instances. * - *

This method must be executed using the {@code singleThreadExecutorService}, as later OpenGL - * commands will be called on that thread. + *

This method must be called on the {@link Factory.Builder#setExecutorService}, as later + * OpenGL commands will be called on that thread. */ private static DefaultVideoFrameProcessor createOpenGlObjectsAndFrameProcessor( Context context, @@ -572,15 +600,13 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { ColorInfo outputColorInfo, boolean enableColorTransfers, boolean renderFramesAutomatically, - ExecutorService singleThreadExecutorService, + VideoFrameProcessingTaskExecutor videoFrameProcessingTaskExecutor, Executor videoFrameProcessorListenerExecutor, Listener listener, GlObjectsProvider glObjectsProvider, @Nullable TextureOutputListener textureOutputListener, int textureOutputCapacity) throws GlUtil.GlException, VideoFrameProcessingException { - checkState(Thread.currentThread().getName().equals(THREAD_NAME)); - EGLDisplay eglDisplay = GlUtil.getDefaultEglDisplay(); int[] configAttributes = ColorInfo.isTransferHdr(outputColorInfo) @@ -604,8 +630,6 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { throw new VideoFrameProcessingException("BT.2020 PQ OpenGL output isn't supported."); } } - VideoFrameProcessingTaskExecutor videoFrameProcessingTaskExecutor = - new VideoFrameProcessingTaskExecutor(singleThreadExecutorService, listener); ColorInfo linearColorInfo = outputColorInfo .buildUpon() @@ -780,7 +804,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { /** * Releases the {@link GlShaderProgram} instances and destroys the OpenGL context. * - *

This method must be called on the {@linkplain #THREAD_NAME background thread}. + *

This method must be called on the {@link Factory.Builder#setExecutorService}. */ private void releaseGlObjects() { try { 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 26bfd82375..a23402277e 100644 --- a/libraries/effect/src/main/java/androidx/media3/effect/VideoFrameProcessingTaskExecutor.java +++ b/libraries/effect/src/main/java/androidx/media3/effect/VideoFrameProcessingTaskExecutor.java @@ -57,6 +57,9 @@ import java.util.concurrent.RejectedExecutionException; void run() throws VideoFrameProcessingException, GlUtil.GlException; } + private static final long RELEASE_WAIT_TIME_MS = 500; + + private final boolean shouldShutdownExecutorService; private final ExecutorService singleThreadExecutorService; private final VideoFrameProcessor.Listener listener; private final Object lock; @@ -69,8 +72,11 @@ import java.util.concurrent.RejectedExecutionException; /** Creates a new instance. */ public VideoFrameProcessingTaskExecutor( - ExecutorService singleThreadExecutorService, VideoFrameProcessor.Listener listener) { + ExecutorService singleThreadExecutorService, + boolean shouldShutdownExecutorService, + VideoFrameProcessor.Listener listener) { this.singleThreadExecutorService = singleThreadExecutorService; + this.shouldShutdownExecutorService = shouldShutdownExecutorService; this.listener = listener; lock = new Object(); highPriorityTasks = new ArrayDeque<>(); @@ -162,24 +168,28 @@ import java.util.concurrent.RejectedExecutionException; } /** - * Cancels remaining tasks, runs the given release task, and shuts down the background thread. + * Cancels remaining tasks, runs the given release task + * + *

If {@code shouldShutdownExecutorService} is {@code true}, shuts down the {@linkplain + * ExecutorService background thread}. * * @param releaseTask A {@link Task} to execute before shutting down the background thread. - * @param releaseWaitTimeMs How long to wait for the release task to terminate, in milliseconds. * @throws InterruptedException If interrupted while releasing resources. */ - public void release(Task releaseTask, long releaseWaitTimeMs) throws InterruptedException { + public void release(Task releaseTask) throws InterruptedException { synchronized (lock) { shouldCancelTasks = true; highPriorityTasks.clear(); } Future unused = wrapTaskAndSubmitToExecutorService(releaseTask, /* isFlushOrReleaseTask= */ true); - singleThreadExecutorService.shutdown(); - if (!singleThreadExecutorService.awaitTermination(releaseWaitTimeMs, MILLISECONDS)) { - listener.onError( - new VideoFrameProcessingException( - "Release timed out. OpenGL resources may not be cleaned up properly.")); + if (shouldShutdownExecutorService) { + singleThreadExecutorService.shutdown(); + if (!singleThreadExecutorService.awaitTermination(RELEASE_WAIT_TIME_MS, MILLISECONDS)) { + listener.onError( + new VideoFrameProcessingException( + "Release timed out. OpenGL resources may not be cleaned up properly.")); + } } } diff --git a/libraries/effect/src/test/java/androidx/media3/effect/ChainingGlShaderProgramListenerTest.java b/libraries/effect/src/test/java/androidx/media3/effect/ChainingGlShaderProgramListenerTest.java index 6bdd05a148..a7bd7688a7 100644 --- a/libraries/effect/src/test/java/androidx/media3/effect/ChainingGlShaderProgramListenerTest.java +++ b/libraries/effect/src/test/java/androidx/media3/effect/ChainingGlShaderProgramListenerTest.java @@ -37,7 +37,9 @@ public final class ChainingGlShaderProgramListenerTest { mock(VideoFrameProcessor.Listener.class); private final VideoFrameProcessingTaskExecutor videoFrameProcessingTaskExecutor = new VideoFrameProcessingTaskExecutor( - Util.newSingleThreadExecutor("Test"), mockFrameProcessorListener); + Util.newSingleThreadExecutor("Test"), + /* shouldShutdownExecutorService= */ true, + mockFrameProcessorListener); private final GlObjectsProvider mockGlObjectsProvider = mock(GlObjectsProvider.class); private final GlShaderProgram mockProducingGlShaderProgram = mock(GlShaderProgram.class); private final GlShaderProgram mockConsumingGlShaderProgram = mock(GlShaderProgram.class); @@ -50,7 +52,7 @@ public final class ChainingGlShaderProgramListenerTest { @After public void release() throws InterruptedException { - videoFrameProcessingTaskExecutor.release(/* releaseTask= */ () -> {}, EXECUTOR_WAIT_TIME_MS); + videoFrameProcessingTaskExecutor.release(/* releaseTask= */ () -> {}); } @Test diff --git a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/VideoCompositorPixelTest.java b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/VideoCompositorPixelTest.java index 7633a4e484..e94ea2aa37 100644 --- a/libraries/transformer/src/androidTest/java/androidx/media3/transformer/VideoCompositorPixelTest.java +++ b/libraries/transformer/src/androidTest/java/androidx/media3/transformer/VideoCompositorPixelTest.java @@ -22,6 +22,7 @@ import static androidx.media3.test.utils.BitmapPixelTestUtil.maybeSaveTestBitmap import static androidx.media3.test.utils.BitmapPixelTestUtil.readBitmap; import static androidx.test.core.app.ApplicationProvider.getApplicationContext; import static com.google.common.truth.Truth.assertThat; +import static java.util.concurrent.TimeUnit.MILLISECONDS; import android.graphics.Bitmap; import android.opengl.EGLContext; @@ -33,6 +34,7 @@ import androidx.media3.common.GlTextureInfo; import androidx.media3.common.VideoFrameProcessingException; import androidx.media3.common.VideoFrameProcessor; import androidx.media3.common.util.GlUtil; +import androidx.media3.common.util.Util; import androidx.media3.effect.DefaultGlObjectsProvider; import androidx.media3.effect.DefaultVideoFrameProcessor; import androidx.media3.effect.RgbFilter; @@ -40,16 +42,19 @@ import androidx.media3.effect.ScaleAndRotateTransformation; import androidx.media3.effect.VideoCompositor; import androidx.media3.test.utils.BitmapPixelTestUtil; import androidx.media3.test.utils.VideoFrameProcessorTestRunner; -import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.google.common.collect.ImmutableList; import java.io.IOException; +import java.util.concurrent.ExecutorService; import java.util.concurrent.atomic.AtomicReference; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; +import org.checkerframework.checker.nullness.qual.Nullable; import org.junit.After; import org.junit.Test; import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; /** Pixel test for {@link VideoCompositor} compositing 2 input frames into 1 output frame. */ -@RunWith(AndroidJUnit4.class) +@RunWith(Parameterized.class) public final class VideoCompositorPixelTest { private @MonotonicNonNull VideoFrameProcessorTestRunner inputVfpTestRunner1; private @MonotonicNonNull VideoFrameProcessorTestRunner inputVfpTestRunner2; @@ -66,22 +71,42 @@ public final class VideoCompositorPixelTest { new ScaleAndRotateTransformation.Builder().setRotationDegrees(180).build(); private static final Effect GRAYSCALE = RgbFilter.createGrayscaleFilter(); - // TODO: b/262694346 - Create and share a VideoFrameProcessingTaskExecutor for all - // DefaultVideoFrameProcessor and VideoCompositor instances. + @Parameterized.Parameters(name = "useSharedExecutor={0}") + public static ImmutableList useSharedExecutor() { + return ImmutableList.of(true, false); + } + + @Parameterized.Parameter public boolean useSharedExecutor; + + public @Nullable ExecutorService executorService; @After - public void release() { + public void tearDown() { if (inputVfpTestRunner1 != null) { inputVfpTestRunner1.release(); } if (inputVfpTestRunner2 != null) { inputVfpTestRunner2.release(); } + + if (executorService != null) { + try { + executorService.shutdown(); + if (!executorService.awaitTermination(/* timeout= */ 5000, MILLISECONDS)) { + throw new IllegalStateException("Missed shutdown timeout."); + } + } catch (InterruptedException unexpected) { + Thread.currentThread().interrupt(); + throw new IllegalStateException(unexpected); + } + } } @Test public void compositeTwoFrames_matchesExpected() throws Exception { - String testId = "compositeTwoFrames_matchesExpected"; + String testId = + "compositeTwoFrames_matchesExpected[useSharedExecutor=" + useSharedExecutor + "]"; + executorService = useSharedExecutor ? Util.newSingleThreadExecutor("Effect:GlThread") : null; // Arrange VideoCompositor and VideoFrameProcessor instances. EGLContext sharedEglContext = AndroidTestUtil.createOpenGlObjects(); @@ -96,7 +121,11 @@ public final class VideoCompositorPixelTest { releaseOutputTextureCallback, syncObject) -> { try { - GlUtil.awaitSyncObject(syncObject); + if (useSharedExecutor) { + GlUtil.deleteSyncObject(syncObject); + } else { + GlUtil.awaitSyncObject(syncObject); + } compositedOutputBitmap.set( BitmapPixelTestUtil.createArgb8888BitmapFromCurrentGlFramebuffer( outputTexture.getWidth(), outputTexture.getHeight())); @@ -110,14 +139,22 @@ public final class VideoCompositorPixelTest { TextureBitmapReader inputTextureBitmapReader1 = new TextureBitmapReader(); VideoFrameProcessorTestRunner inputVfpTestRunner1 = getFrameProcessorTestRunnerBuilder( - testId, inputTextureBitmapReader1, videoCompositor, sharedGlObjectsProvider) + testId, + inputTextureBitmapReader1, + videoCompositor, + executorService, + sharedGlObjectsProvider) .setEffects(GRAYSCALE) .build(); this.inputVfpTestRunner1 = inputVfpTestRunner1; TextureBitmapReader inputTextureBitmapReader2 = new TextureBitmapReader(); VideoFrameProcessorTestRunner inputVfpTestRunner2 = getFrameProcessorTestRunnerBuilder( - testId, inputTextureBitmapReader2, videoCompositor, sharedGlObjectsProvider) + testId, + inputTextureBitmapReader2, + videoCompositor, + executorService, + sharedGlObjectsProvider) .setEffects(ROTATE_180) .build(); this.inputVfpTestRunner2 = inputVfpTestRunner2; @@ -172,6 +209,7 @@ public final class VideoCompositorPixelTest { String testId, TextureBitmapReader textureBitmapReader, VideoCompositor videoCompositor, + @Nullable ExecutorService executorService, GlObjectsProvider glObjectsProvider) { int inputId = videoCompositor.registerInputSource(); VideoFrameProcessor.Factory defaultVideoFrameProcessorFactory = @@ -189,6 +227,7 @@ public final class VideoCompositorPixelTest { inputId, outputTexture, presentationTimeUs, releaseOutputTextureCallback); }, /* textureOutputCapacity= */ 1) + .setExecutorService(executorService) .build(); return new VideoFrameProcessorTestRunner.Builder() .setTestId(testId)