From 9c075b692e9e76959e47e9ad3cb018d3071f9154 Mon Sep 17 00:00:00 2001
From: dancho <dancho@google.com>
Date: Tue, 23 Jul 2024 04:56:46 -0700
Subject: [PATCH] Ensure EGLSurface is released on GL thread

Add VideoFrameProcessingTaskExecutor.invoke() method that blocks until
Task has executed on GL thread.
Use that for FinalShaderProgramWrapper.setOutputSurfaceInfo

PiperOrigin-RevId: 655119768
---
 .../effect/FinalShaderProgramWrapper.java     | 14 +++++-
 .../VideoFrameProcessingTaskExecutor.java     | 48 ++++++++++++++++++-
 2 files changed, 59 insertions(+), 3 deletions(-)

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 32bfe03016..867d42abab 100644
--- a/libraries/effect/src/main/java/androidx/media3/effect/FinalShaderProgramWrapper.java
+++ b/libraries/effect/src/main/java/androidx/media3/effect/FinalShaderProgramWrapper.java
@@ -327,7 +327,19 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
   }
 
   /** See {@link DefaultVideoFrameProcessor#setOutputSurfaceInfo} */
-  public synchronized void setOutputSurfaceInfo(@Nullable SurfaceInfo outputSurfaceInfo) {
+  public void setOutputSurfaceInfo(@Nullable SurfaceInfo outputSurfaceInfo) {
+    try {
+      videoFrameProcessingTaskExecutor.invoke(
+          () -> setOutputSurfaceInfoInternal(outputSurfaceInfo));
+    } catch (InterruptedException e) {
+      Thread.currentThread().interrupt();
+      videoFrameProcessorListenerExecutor.execute(
+          () -> videoFrameProcessorListener.onError(VideoFrameProcessingException.from(e)));
+    }
+  }
+
+  /** Must be called on the GL thread. */
+  private synchronized void setOutputSurfaceInfoInternal(@Nullable SurfaceInfo outputSurfaceInfo) {
     if (textureOutputListener != null) {
       return;
     }
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 db675ad772..0445113edc 100644
--- a/libraries/effect/src/main/java/androidx/media3/effect/VideoFrameProcessingTaskExecutor.java
+++ b/libraries/effect/src/main/java/androidx/media3/effect/VideoFrameProcessingTaskExecutor.java
@@ -24,9 +24,11 @@ import androidx.media3.common.util.GlUtil;
 import java.util.ArrayDeque;
 import java.util.Queue;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
 import java.util.concurrent.RejectedExecutionException;
+import java.util.concurrent.TimeoutException;
 
 /**
  * Wrapper around a single thread {@link ExecutorService} for executing {@link Task} instances.
@@ -61,10 +63,11 @@ import java.util.concurrent.RejectedExecutionException;
     void onError(VideoFrameProcessingException exception);
   }
 
-  private static final long RELEASE_WAIT_TIME_MS = 500;
+  private static final long EXECUTOR_SERVICE_TIMEOUT_MS = 500;
 
   private final boolean shouldShutdownExecutorService;
   private final ExecutorService singleThreadExecutorService;
+  private final Future<Thread> threadFuture; // Used to identify the GL thread.
   private final ErrorListener errorListener;
   private final Object lock;
 
@@ -80,6 +83,7 @@ import java.util.concurrent.RejectedExecutionException;
       boolean shouldShutdownExecutorService,
       ErrorListener errorListener) {
     this.singleThreadExecutorService = singleThreadExecutorService;
+    threadFuture = singleThreadExecutorService.submit(Thread::currentThread);
     this.shouldShutdownExecutorService = shouldShutdownExecutorService;
     this.errorListener = errorListener;
     lock = new Object();
@@ -106,6 +110,45 @@ import java.util.concurrent.RejectedExecutionException;
     }
   }
 
+  /** Blocks the caller until the given {@link Task} has completed. */
+  public void invoke(Task task) throws InterruptedException {
+    // If running on the executor service thread, run synchronously.
+    // Calling future.get() on the single executor thread would deadlock.
+    Thread videoFrameProcessingThread;
+    try {
+      videoFrameProcessingThread = threadFuture.get(EXECUTOR_SERVICE_TIMEOUT_MS, MILLISECONDS);
+    } catch (InterruptedException e) {
+      throw e;
+    } catch (Exception e) {
+      handleException(e);
+      return;
+    }
+    if (Thread.currentThread() == videoFrameProcessingThread) {
+      try {
+        task.run();
+      } catch (Exception e) {
+        handleException(e);
+      }
+      return;
+    }
+
+    // Not running on the executor service thread. Block until task.run() returns.
+    try {
+      Future<?> taskFuture =
+          singleThreadExecutorService.submit(
+              () -> {
+                try {
+                  task.run();
+                } catch (Exception e) {
+                  handleException(e);
+                }
+              });
+      taskFuture.get(EXECUTOR_SERVICE_TIMEOUT_MS, MILLISECONDS);
+    } catch (RuntimeException | ExecutionException | TimeoutException e) {
+      handleException(e);
+    }
+  }
+
   /**
    * Submits the given {@link Task} to be executed after the currently running task and all
    * previously submitted high-priority tasks have completed.
@@ -173,7 +216,8 @@ import java.util.concurrent.RejectedExecutionException;
         wrapTaskAndSubmitToExecutorService(releaseTask, /* isFlushOrReleaseTask= */ true);
     if (shouldShutdownExecutorService) {
       singleThreadExecutorService.shutdown();
-      if (!singleThreadExecutorService.awaitTermination(RELEASE_WAIT_TIME_MS, MILLISECONDS)) {
+      if (!singleThreadExecutorService.awaitTermination(
+          EXECUTOR_SERVICE_TIMEOUT_MS, MILLISECONDS)) {
         errorListener.onError(
             new VideoFrameProcessingException(
                 "Release timed out. OpenGL resources may not be cleaned up properly."));