From 8f8e48731eeedea8636d4d31be4a0c0b9a574bec Mon Sep 17 00:00:00 2001 From: claincly Date: Wed, 7 Aug 2024 05:47:03 -0700 Subject: [PATCH] Add a method to disallow VFP destroying shared eglContext Previously in MultiInputVideoGraph, each VFP would destroy the shared eglContext, such that the same eglContext object is destroyed multiple times. Adding a flag to disallow this. The alternative being we could add a flag on the VFP constructor, but I think that is too subscriptive (meaning if we later might want to add another boolean to control another GL behaviour, multiple booleans would make the class less reason-able), and would incur a lot of code changes at places. PiperOrigin-RevId: 660354367 --- RELEASENOTES.md | 1 + .../media3/common/GlObjectsProvider.java | 7 +++++ .../effect/DefaultGlObjectsProvider.java | 18 ++++++++++- .../media3/effect/DefaultVideoCompositor.java | 4 +-- .../effect/DefaultVideoFrameProcessor.java | 13 ++++---- .../effect/MultipleInputVideoGraph.java | 30 +++++++++++++++---- 6 files changed, 56 insertions(+), 17 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index bdbc9193bc..4327760db3 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -77,6 +77,7 @@ such that output is closer to expected. * Speed up `DefaultVideoFrameProcessor.queueInputBitmap()`. As a result, exporting images to videos with `Transformer` is faster. + * Add a `release()` method to `GlObjectsProvider`. * Muxers: * IMA extension: * Fix bug where clearing the playlist may cause an diff --git a/libraries/common/src/main/java/androidx/media3/common/GlObjectsProvider.java b/libraries/common/src/main/java/androidx/media3/common/GlObjectsProvider.java index 72a4d049fe..fbf350465b 100644 --- a/libraries/common/src/main/java/androidx/media3/common/GlObjectsProvider.java +++ b/libraries/common/src/main/java/androidx/media3/common/GlObjectsProvider.java @@ -81,4 +81,11 @@ public interface GlObjectsProvider { * @throws GlException If an error occurs during creation. */ GlTextureInfo createBuffersForTexture(int texId, int width, int height) throws GlException; + + /** + * Releases the created objects. + * + * @param eglDisplay The {@link EGLDisplay} to release the objects for. + */ + void release(EGLDisplay eglDisplay) throws GlException; } diff --git a/libraries/effect/src/main/java/androidx/media3/effect/DefaultGlObjectsProvider.java b/libraries/effect/src/main/java/androidx/media3/effect/DefaultGlObjectsProvider.java index 274adfa9b8..6450c96562 100644 --- a/libraries/effect/src/main/java/androidx/media3/effect/DefaultGlObjectsProvider.java +++ b/libraries/effect/src/main/java/androidx/media3/effect/DefaultGlObjectsProvider.java @@ -15,6 +15,8 @@ */ package androidx.media3.effect; +import static androidx.media3.common.util.GlUtil.destroyEglContext; + import android.opengl.EGL14; import android.opengl.EGLContext; import android.opengl.EGLDisplay; @@ -25,6 +27,8 @@ import androidx.media3.common.GlObjectsProvider; import androidx.media3.common.GlTextureInfo; import androidx.media3.common.util.GlUtil; import androidx.media3.common.util.UnstableApi; +import java.util.ArrayList; +import java.util.List; // TODO(b/261820382): Add tests for sharing context. /** @@ -38,6 +42,7 @@ import androidx.media3.common.util.UnstableApi; public final class DefaultGlObjectsProvider implements GlObjectsProvider { private final EGLContext sharedEglContext; + private final List createdEglContexts; /** Creates an instance with no shared EGL context. */ public DefaultGlObjectsProvider() { @@ -51,12 +56,16 @@ public final class DefaultGlObjectsProvider implements GlObjectsProvider { */ public DefaultGlObjectsProvider(@Nullable EGLContext sharedEglContext) { this.sharedEglContext = sharedEglContext != null ? sharedEglContext : EGL14.EGL_NO_CONTEXT; + createdEglContexts = new ArrayList<>(); } @Override public EGLContext createEglContext( EGLDisplay eglDisplay, int openGlVersion, int[] configAttributes) throws GlUtil.GlException { - return GlUtil.createEglContext(sharedEglContext, eglDisplay, openGlVersion, configAttributes); + EGLContext eglContext = + GlUtil.createEglContext(sharedEglContext, eglDisplay, openGlVersion, configAttributes); + createdEglContexts.add(eglContext); + return eglContext; } @Override @@ -81,4 +90,11 @@ public final class DefaultGlObjectsProvider implements GlObjectsProvider { int fboId = GlUtil.createFboForTexture(texId); return new GlTextureInfo(texId, fboId, /* rboId= */ C.INDEX_UNSET, width, height); } + + @Override + public void release(EGLDisplay eglDisplay) throws GlUtil.GlException { + for (int i = 0; i < createdEglContexts.size(); i++) { + destroyEglContext(eglDisplay, createdEglContexts.get(i)); + } + } } diff --git a/libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoCompositor.java b/libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoCompositor.java index b4065193ef..e72917eff4 100644 --- a/libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoCompositor.java +++ b/libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoCompositor.java @@ -420,9 +420,9 @@ public final class DefaultVideoCompositor implements VideoCompositor { Log.e(TAG, "Error releasing GL resources", e); } finally { try { - GlUtil.destroyEglContext(eglDisplay, eglContext); + glObjectsProvider.release(checkNotNull(eglDisplay)); } catch (GlUtil.GlException e) { - Log.e(TAG, "Error releasing GL context", e); + Log.e(TAG, "Error releasing GL objects", e); } } } 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 89e3d3765e..77211b0c6f 100644 --- a/libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoFrameProcessor.java +++ b/libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoFrameProcessor.java @@ -19,6 +19,7 @@ import static androidx.media3.common.util.Assertions.checkArgument; import static androidx.media3.common.util.Assertions.checkNotNull; import static androidx.media3.common.util.Assertions.checkState; import static androidx.media3.common.util.Assertions.checkStateNotNull; +import static androidx.media3.common.util.GlUtil.getDefaultEglDisplay; import static androidx.media3.common.util.Util.SDK_INT; import static androidx.media3.effect.DebugTraceUtil.COMPONENT_VFP; import static androidx.media3.effect.DebugTraceUtil.EVENT_RECEIVE_END_OF_ALL_INPUT; @@ -445,7 +446,6 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { private final Context context; private final GlObjectsProvider glObjectsProvider; private final EGLDisplay eglDisplay; - private final EGLContext eglContext; private final InputSwitcher inputSwitcher; private final VideoFrameProcessingTaskExecutor videoFrameProcessingTaskExecutor; private final VideoFrameProcessor.Listener listener; @@ -483,10 +483,9 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { Context context, GlObjectsProvider glObjectsProvider, EGLDisplay eglDisplay, - EGLContext eglContext, InputSwitcher inputSwitcher, VideoFrameProcessingTaskExecutor videoFrameProcessingTaskExecutor, - VideoFrameProcessor.Listener listener, + Listener listener, Executor listenerExecutor, FinalShaderProgramWrapper finalShaderProgramWrapper, boolean renderFramesAutomatically, @@ -494,7 +493,6 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { this.context = context; this.glObjectsProvider = glObjectsProvider; this.eglDisplay = eglDisplay; - this.eglContext = eglContext; this.inputSwitcher = inputSwitcher; this.videoFrameProcessingTaskExecutor = videoFrameProcessingTaskExecutor; this.listener = listener; @@ -820,7 +818,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { boolean experimentalAdjustSurfaceTextureTransformationMatrix, boolean experimentalRepeatInputBitmapWithoutResampling) throws GlUtil.GlException, VideoFrameProcessingException { - EGLDisplay eglDisplay = GlUtil.getDefaultEglDisplay(); + EGLDisplay eglDisplay = getDefaultEglDisplay(); int[] configAttributes = ColorInfo.isTransferHdr(outputColorInfo) ? GlUtil.EGL_CONFIG_ATTRIBUTES_RGBA_1010102 @@ -873,7 +871,6 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { context, glObjectsProvider, eglDisplay, - eglContextAndPlaceholderSurface.first, inputSwitcher, videoFrameProcessingTaskExecutor, listener, @@ -1106,9 +1103,9 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { } } finally { try { - GlUtil.destroyEglContext(eglDisplay, eglContext); + glObjectsProvider.release(eglDisplay); } catch (GlUtil.GlException e) { - Log.e(TAG, "Error releasing GL context", e); + Log.e(TAG, "Error releasing GL objects", e); } } } diff --git a/libraries/effect/src/main/java/androidx/media3/effect/MultipleInputVideoGraph.java b/libraries/effect/src/main/java/androidx/media3/effect/MultipleInputVideoGraph.java index e0d8bed8d0..4a6fa05dec 100644 --- a/libraries/effect/src/main/java/androidx/media3/effect/MultipleInputVideoGraph.java +++ b/libraries/effect/src/main/java/androidx/media3/effect/MultipleInputVideoGraph.java @@ -21,6 +21,8 @@ import static androidx.media3.common.util.Assertions.checkArgument; import static androidx.media3.common.util.Assertions.checkNotNull; import static androidx.media3.common.util.Assertions.checkState; import static androidx.media3.common.util.Assertions.checkStateNotNull; +import static androidx.media3.common.util.GlUtil.destroyEglContext; +import static androidx.media3.common.util.GlUtil.getDefaultEglDisplay; import static androidx.media3.common.util.Util.contains; import static androidx.media3.common.util.Util.newSingleThreadScheduledExecutor; import static androidx.media3.effect.DebugTraceUtil.COMPONENT_COMPOSITOR; @@ -47,6 +49,8 @@ import androidx.media3.common.VideoFrameProcessingException; import androidx.media3.common.VideoFrameProcessor; import androidx.media3.common.VideoGraph; import androidx.media3.common.util.GlUtil; +import androidx.media3.common.util.GlUtil.GlException; +import androidx.media3.common.util.Log; import androidx.media3.common.util.UnstableApi; import com.google.common.util.concurrent.MoreExecutors; import java.util.ArrayDeque; @@ -61,6 +65,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @UnstableApi public abstract class MultipleInputVideoGraph implements VideoGraph { + private static final String TAG = "MultiInputVG"; private static final String SHARED_EXECUTOR_NAME = "Effect:MultipleInputVideoGraph:Thread"; private static final long RELEASE_WAIT_TIME_MS = 1_000; @@ -70,7 +75,7 @@ public abstract class MultipleInputVideoGraph implements VideoGraph { private final Context context; private final ColorInfo outputColorInfo; - private final GlObjectsProvider glObjectsProvider; + private final SingleContextGlObjectsProvider glObjectsProvider; private final DebugViewProvider debugViewProvider; private final VideoGraph.Listener listener; private final Executor listenerExecutor; @@ -307,6 +312,15 @@ public abstract class MultipleInputVideoGraph implements VideoGraph { compositionVideoFrameProcessor = null; } + try { + // The eglContext is not released by any of the frame processors. + if (glObjectsProvider.singleEglContext != null) { + destroyEglContext(getDefaultEglDisplay(), glObjectsProvider.singleEglContext); + } + } catch (GlUtil.GlException e) { + Log.e(TAG, "Error releasing GL context", e); + } + sharedExecutorService.shutdown(); try { sharedExecutorService.awaitTermination(RELEASE_WAIT_TIME_MS, MILLISECONDS); @@ -472,8 +486,7 @@ public abstract class MultipleInputVideoGraph implements VideoGraph { @Override public EGLContext createEglContext( - EGLDisplay eglDisplay, int openGlVersion, int[] configAttributes) - throws GlUtil.GlException { + EGLDisplay eglDisplay, int openGlVersion, int[] configAttributes) throws GlException { if (singleEglContext == null) { singleEglContext = glObjectsProvider.createEglContext(eglDisplay, openGlVersion, configAttributes); @@ -487,21 +500,26 @@ public abstract class MultipleInputVideoGraph implements VideoGraph { Object surface, @C.ColorTransfer int colorTransfer, boolean isEncoderInputSurface) - throws GlUtil.GlException { + throws GlException { return glObjectsProvider.createEglSurface( eglDisplay, surface, colorTransfer, isEncoderInputSurface); } @Override public EGLSurface createFocusedPlaceholderEglSurface( - EGLContext eglContext, EGLDisplay eglDisplay) throws GlUtil.GlException { + EGLContext eglContext, EGLDisplay eglDisplay) throws GlException { return glObjectsProvider.createFocusedPlaceholderEglSurface(eglContext, eglDisplay); } @Override public GlTextureInfo createBuffersForTexture(int texId, int width, int height) - throws GlUtil.GlException { + throws GlException { return glObjectsProvider.createBuffersForTexture(texId, width, height); } + + @Override + public void release(EGLDisplay eglDisplay) { + // The eglContext is released in the VideoGraph after all VideoFrameProcessors are released. + } } }