From fd0a8cadc6a70fc06fd3d07a905da6e8f51b4fda Mon Sep 17 00:00:00 2001
From: andrewlewis <andrewlewis@google.com>
Date: Fri, 24 Feb 2023 13:03:35 +0000
Subject: [PATCH] Continue releasing programs on failure

Currently if releasing a shader program throws we don't release the GL
context, which could leak resources, and any errors are silently dropped
as we suppress notifications during releasing.

Improve resource cleanup and debuggability of errors from custom effects
by continuing to release shaders on failure (for runtime and
`VideoFrameProcessingException`s) and always clean up the GL context.

Note: this doesn't help with the case where releasing a custom shader
blocks for a long time, causing releasing the frame processor to
time out.
PiperOrigin-RevId: 512042501
---
 .../effect/DefaultVideoFrameProcessor.java    | 25 ++++++++++++++-----
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/library/effect/src/main/java/com/google/android/exoplayer2/effect/DefaultVideoFrameProcessor.java b/library/effect/src/main/java/com/google/android/exoplayer2/effect/DefaultVideoFrameProcessor.java
index 2e63868729..1cc06268fb 100644
--- a/library/effect/src/main/java/com/google/android/exoplayer2/effect/DefaultVideoFrameProcessor.java
+++ b/library/effect/src/main/java/com/google/android/exoplayer2/effect/DefaultVideoFrameProcessor.java
@@ -37,6 +37,7 @@ import com.google.android.exoplayer2.util.DebugViewProvider;
 import com.google.android.exoplayer2.util.Effect;
 import com.google.android.exoplayer2.util.FrameInfo;
 import com.google.android.exoplayer2.util.GlUtil;
+import com.google.android.exoplayer2.util.Log;
 import com.google.android.exoplayer2.util.SurfaceInfo;
 import com.google.android.exoplayer2.util.Util;
 import com.google.android.exoplayer2.util.VideoFrameProcessingException;
@@ -59,7 +60,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
 public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
 
   /** A factory for {@link DefaultVideoFrameProcessor} instances. */
-  public static class Factory implements VideoFrameProcessor.Factory {
+  public static final class Factory implements VideoFrameProcessor.Factory {
     /**
      * {@inheritDoc}
      *
@@ -348,6 +349,8 @@ 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 = 100;
 
@@ -546,11 +549,21 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor {
    * <p>This method must be called on the {@linkplain #THREAD_NAME background thread}.
    */
   @WorkerThread
-  private void releaseShaderProgramsAndDestroyGlContext()
-      throws GlUtil.GlException, VideoFrameProcessingException {
-    for (int i = 0; i < allShaderPrograms.size(); i++) {
-      allShaderPrograms.get(i).release();
+  private void releaseShaderProgramsAndDestroyGlContext() {
+    try {
+      for (int i = 0; i < allShaderPrograms.size(); i++) {
+        try {
+          allShaderPrograms.get(i).release();
+        } catch (Exception e) {
+          Log.e(TAG, "Error releasing shader program", e);
+        }
+      }
+    } finally {
+      try {
+        GlUtil.destroyEglContext(eglDisplay, eglContext);
+      } catch (GlUtil.GlException e) {
+        Log.e(TAG, "Error releasing GL context", e);
+      }
     }
-    GlUtil.destroyEglContext(eglDisplay, eglContext);
   }
 }