Refactor GlObjectsProvider release logic

When possible, make it the caller's responsibility to release the
GlObjectsProvider. It's good practice that the class creating an object
also releases it. It avoids releasing the same object (here, the
GlObjectsProvider) multiple times, which was causing
DefaultVideoCompositorPixelTest to fail.

PiperOrigin-RevId: 725983417

RELEASENOTES.md

@@ -10,6 +10,9 @@
     *   Add `MediaProjectionAssetLoader`, which provides media from a
         `MediaProjection` for screen recording.
     *   Add `#getInputFormat()` to `Codec` interface.
+    *   Shift the responsibility to release the `GlObjectsProvider` onto the
+        caller in `DefaultVideoFrameProcessor` and `DefaultVideoCompositor` when
+        possible.
 *   Track Selection:
 *   Extractors:
 *   DataSource:

libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoCompositor.java

@@ -105,8 +105,9 @@ public final class DefaultVideoCompositor implements VideoCompositor {
   /**
    * Creates an instance.
    *
-   * <p>The {@link ExecutorService} must be {@linkplain ExecutorService#shutdown shut down} by the
+   * <p>It's the caller's responsibility to {@linkplain GlObjectsProvider#release(EGLDisplay)
-   * caller.
+   * release} the {@link GlObjectsProvider} on the {@link ExecutorService}'s thread, and to
+   * {@linkplain ExecutorService#shutdown shut down} the {@link ExecutorService}.
    */
   public DefaultVideoCompositor(
       Context context,
@@ -409,12 +410,6 @@ public final class DefaultVideoCompositor implements VideoCompositor {
       GlUtil.destroyEglSurface(eglDisplay, placeholderEglSurface);
     } catch (GlUtil.GlException e) {
       Log.e(TAG, "Error releasing GL resources", e);
-    } finally {
-      try {
-        glObjectsProvider.release(checkNotNull(eglDisplay));
-      } catch (GlUtil.GlException e) {
-        Log.e(TAG, "Error releasing GL objects", e);
-      }
     }
   }
 

libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoFrameProcessor.java

@@ -235,6 +235,10 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
        * Sets the {@link GlObjectsProvider}.
        *
        * <p>The default value is a {@link DefaultGlObjectsProvider}.
+       *
+       * <p>If both the {@link GlObjectsProvider} and the {@link ExecutorService} are set, it's the
+       * caller's responsibility to release the {@link GlObjectsProvider} on the {@link
+       * ExecutorService}'s thread.
        */
       @CanIgnoreReturnValue
       public Builder setGlObjectsProvider(GlObjectsProvider glObjectsProvider) {
@@ -408,16 +412,16 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
         throws VideoFrameProcessingException {
       // TODO(b/261188041) Add tests to verify the Listener is invoked on the given Executor.
 
-      boolean shouldShutdownExecutorService = executorService == null;
       ExecutorService instanceExecutorService =
           executorService == null ? Util.newSingleThreadExecutor(THREAD_NAME) : executorService;
-
+      boolean shouldShutdownExecutorService = executorService == null;
       VideoFrameProcessingTaskExecutor videoFrameProcessingTaskExecutor =
           new VideoFrameProcessingTaskExecutor(
               instanceExecutorService, shouldShutdownExecutorService, listener::onError);
 
-      GlObjectsProvider glObjectsProvider =
+      boolean shouldReleaseGlObjectsProvider = glObjectsProvider == null || executorService == null;
-          this.glObjectsProvider == null ? new DefaultGlObjectsProvider() : this.glObjectsProvider;
+      GlObjectsProvider instanceGlObjectsProvider =
+          glObjectsProvider == null ? new DefaultGlObjectsProvider() : glObjectsProvider;
 
       Future<DefaultVideoFrameProcessor> defaultVideoFrameProcessorFuture =
           instanceExecutorService.submit(
@@ -431,7 +435,8 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
                       videoFrameProcessingTaskExecutor,
                       listenerExecutor,
                       listener,
-                      glObjectsProvider,
+                      instanceGlObjectsProvider,
+                      shouldReleaseGlObjectsProvider,
                       textureOutputListener,
                       textureOutputCapacity,
                       repeatLastRegisteredFrame,
@@ -453,6 +458,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
 
   private final Context context;
   private final GlObjectsProvider glObjectsProvider;
+  private final boolean shouldReleaseGlObjectsProvider;
   private final EGLDisplay eglDisplay;
   private final InputSwitcher inputSwitcher;
   private final VideoFrameProcessingTaskExecutor videoFrameProcessingTaskExecutor;
@@ -493,6 +499,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
   private DefaultVideoFrameProcessor(
       Context context,
       GlObjectsProvider glObjectsProvider,
+      boolean shouldReleaseGlObjectsProvider,
       EGLDisplay eglDisplay,
       InputSwitcher inputSwitcher,
       VideoFrameProcessingTaskExecutor videoFrameProcessingTaskExecutor,
@@ -504,6 +511,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
       DebugViewProvider debugViewProvider) {
     this.context = context;
     this.glObjectsProvider = glObjectsProvider;
+    this.shouldReleaseGlObjectsProvider = shouldReleaseGlObjectsProvider;
     this.eglDisplay = eglDisplay;
     this.inputSwitcher = inputSwitcher;
     this.videoFrameProcessingTaskExecutor = videoFrameProcessingTaskExecutor;
@@ -833,6 +841,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
       Executor videoFrameProcessorListenerExecutor,
       Listener listener,
       GlObjectsProvider glObjectsProvider,
+      boolean shouldReleaseGlObjectsProvider,
       @Nullable GlTextureProducer.Listener textureOutputListener,
       int textureOutputCapacity,
       boolean repeatLastRegisteredFrame,
@@ -890,6 +899,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
     return new DefaultVideoFrameProcessor(
         context,
         glObjectsProvider,
+        shouldReleaseGlObjectsProvider,
         eglDisplay,
         inputSwitcher,
         videoFrameProcessingTaskExecutor,
@@ -1139,10 +1149,12 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
         Log.e(TAG, "Error releasing shader program", e);
       }
     } finally {
-      try {
+      if (shouldReleaseGlObjectsProvider) {
-        glObjectsProvider.release(eglDisplay);
+        try {
-      } catch (GlUtil.GlException e) {
+          glObjectsProvider.release(eglDisplay);
-        Log.e(TAG, "Error releasing GL objects", e);
+        } catch (GlUtil.GlException e) {
+          Log.e(TAG, "Error releasing GL objects", e);
+        }
       }
     }
   }

libraries/effect/src/main/java/androidx/media3/effect/MultipleInputVideoGraph.java

@@ -76,7 +76,7 @@ public abstract class MultipleInputVideoGraph implements VideoGraph {
   private final Context context;
 
   private final ColorInfo outputColorInfo;
-  private final SingleContextGlObjectsProvider glObjectsProvider;
+  private final GlObjectsProvider glObjectsProvider;
   private final DebugViewProvider debugViewProvider;
   private final VideoGraph.Listener listener;
   private final Executor listenerExecutor;
@@ -310,12 +310,9 @@ public abstract class MultipleInputVideoGraph implements VideoGraph {
         sharedExecutorService.submit(
             () -> {
               try {
-                // The eglContext is not released by any of the frame processors.
+                glObjectsProvider.release(getDefaultEglDisplay());
-                if (glObjectsProvider.singleEglContext != null) {
-                  destroyEglContext(getDefaultEglDisplay(), glObjectsProvider.singleEglContext);
-                }
               } catch (Exception e) {
-                Log.e(TAG, "Error releasing GL context", e);
+                Log.e(TAG, "Error releasing GlObjectsProvider", e);
               }
             });
 
@@ -520,8 +517,10 @@ public abstract class MultipleInputVideoGraph implements VideoGraph {
     }
 
     @Override
-    public void release(EGLDisplay eglDisplay) {
+    public void release(EGLDisplay eglDisplay) throws GlException {
-      // The eglContext is released in the VideoGraph after all VideoFrameProcessors are released.
+      if (singleEglContext != null) {
+        destroyEglContext(eglDisplay, singleEglContext);
+      }
     }
   }
 }

libraries/transformer/src/androidTest/java/androidx/media3/transformer/DefaultVideoCompositorPixelTest.java

@@ -48,6 +48,7 @@ import androidx.media3.common.OverlaySettings;
 import androidx.media3.common.VideoCompositorSettings;
 import androidx.media3.common.VideoFrameProcessingException;
 import androidx.media3.common.util.GlUtil;
+import androidx.media3.common.util.Log;
 import androidx.media3.common.util.Size;
 import androidx.media3.common.util.Util;
 import androidx.media3.effect.AlphaScale;
@@ -74,6 +75,7 @@ import java.util.List;
 import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
 import java.util.concurrent.atomic.AtomicReference;
 import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
 import org.junit.After;
@@ -94,6 +96,7 @@ public final class DefaultVideoCompositorPixelTest {
 
   @Rule public final TestName testName = new TestName();
 
+  private static final String TAG = "DefaultVideoCompositorPixelTest";
   private static final String ORIGINAL_PNG_ASSET_PATH = "media/png/media3test_srgb.png";
   private static final String TEST_DIRECTORY = "test-generated-goldens/CompositorTestTimestamps/";
   private static final ImmutableList<ImmutableList<Effect>> TWO_INPUT_COMPOSITOR_EFFECT_LISTS =
@@ -623,6 +626,7 @@ public final class DefaultVideoCompositorPixelTest {
     private final List<VideoFrameProcessorTestRunner> inputVideoFrameProcessorTestRunners;
     private final VideoCompositor videoCompositor;
     private final ExecutorService sharedExecutorService;
+    private final GlObjectsProvider glObjectsProvider;
     private final AtomicReference<VideoFrameProcessingException> compositionException;
     private final CountDownLatch compositorEnded;
     private final String testId;
@@ -660,7 +664,7 @@ public final class DefaultVideoCompositorPixelTest {
       this.testId = testId;
       timeoutMs = inputEffectLists.size() * VIDEO_FRAME_PROCESSING_WAIT_MS;
       sharedExecutorService = Util.newSingleThreadExecutor("Effect:Shared:GlThread");
-      GlObjectsProvider glObjectsProvider = new DefaultGlObjectsProvider();
+      glObjectsProvider = new DefaultGlObjectsProvider();
 
       compositionException = new AtomicReference<>();
       outputTimestampsToBitmaps = new LinkedHashMap<>();
@@ -800,7 +804,15 @@ public final class DefaultVideoCompositorPixelTest {
         inputVideoFrameProcessorTestRunners.get(i).release();
       }
       videoCompositor.release();
-
+      Future<?> unused =
+          sharedExecutorService.submit(
+              () -> {
+                try {
+                  glObjectsProvider.release(GlUtil.getDefaultEglDisplay());
+                } catch (Exception e) {
+                  Log.e(TAG, "Error releasing GlObjectsProvider", e);
+                }
+              });
       try {
         sharedExecutorService.shutdown();
         if (!sharedExecutorService.awaitTermination(timeoutMs, MILLISECONDS)) {