From 1797359950fbf1acceddd7f40058cbd003383544 Mon Sep 17 00:00:00 2001 From: dancho Date: Tue, 23 Jul 2024 06:18:09 -0700 Subject: [PATCH] Destroy EGLSurface immediately by focusing a placeholder surface eglDestroySurface only destroys the surface when it's made not current. Pass the placeholder surface in FinalShaderProgramWrapper and use it when destroying eglSurface. PiperOrigin-RevId: 655139661 --- .../effect/DefaultVideoFrameProcessor.java | 26 ++++++++++----- .../effect/FinalShaderProgramWrapper.java | 33 ++++++++++++++----- 2 files changed, 42 insertions(+), 17 deletions(-) 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 c726deb701..9c539b7c5f 100644 --- a/libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoFrameProcessor.java +++ b/libraries/effect/src/main/java/androidx/media3/effect/DefaultVideoFrameProcessor.java @@ -32,8 +32,10 @@ import android.graphics.Bitmap; import android.graphics.SurfaceTexture; import android.opengl.EGLContext; import android.opengl.EGLDisplay; +import android.opengl.EGLSurface; import android.opengl.GLES20; import android.opengl.GLES30; +import android.util.Pair; import android.view.Surface; import androidx.annotation.GuardedBy; import androidx.annotation.IntDef; @@ -813,7 +815,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { ColorInfo.isTransferHdr(outputColorInfo) ? GlUtil.EGL_CONFIG_ATTRIBUTES_RGBA_1010102 : GlUtil.EGL_CONFIG_ATTRIBUTES_RGBA_8888; - EGLContext eglContext = + Pair eglContextAndPlaceholderSurface = createFocusedEglContextWithFallback(glObjectsProvider, eglDisplay, configAttributes); ColorInfo linearColorInfo = @@ -845,7 +847,8 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { new FinalShaderProgramWrapper( context, eglDisplay, - eglContext, + eglContextAndPlaceholderSurface.first, + eglContextAndPlaceholderSurface.second, debugViewProvider, outputColorInfo, videoFrameProcessingTaskExecutor, @@ -860,7 +863,7 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { context, glObjectsProvider, eglDisplay, - eglContext, + eglContextAndPlaceholderSurface.first, inputSwitcher, videoFrameProcessingTaskExecutor, listener, @@ -1100,8 +1103,12 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { } } - /** Creates an OpenGL ES 3.0 context if possible, and an OpenGL ES 2.0 context otherwise. */ - private static EGLContext createFocusedEglContextWithFallback( + /** + * Creates an OpenGL ES 3.0 context if possible, and an OpenGL ES 2.0 context otherwise. + * + *

See {@link #createFocusedEglContext}. + */ + private static Pair createFocusedEglContextWithFallback( GlObjectsProvider glObjectsProvider, EGLDisplay eglDisplay, int[] configAttributes) throws GlUtil.GlException { if (SDK_INT < 29) { @@ -1121,8 +1128,10 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { /** * Creates an {@link EGLContext} and focus it using a {@linkplain * GlObjectsProvider#createFocusedPlaceholderEglSurface placeholder EGL Surface}. + * + * @return The {@link EGLContext} and a placeholder {@link EGLSurface} as a {@link Pair}. */ - private static EGLContext createFocusedEglContext( + private static Pair createFocusedEglContext( GlObjectsProvider glObjectsProvider, EGLDisplay eglDisplay, int openGlVersion, @@ -1133,8 +1142,9 @@ public final class DefaultVideoFrameProcessor implements VideoFrameProcessor { // Some OpenGL ES 3.0 contexts returned from createEglContext may throw EGL_BAD_MATCH when being // used to createFocusedPlaceHolderEglSurface, despite GL documentation suggesting the contexts, // if successfully created, are valid. Check early whether the context is really valid. - glObjectsProvider.createFocusedPlaceholderEglSurface(eglContext, eglDisplay); - return eglContext; + EGLSurface eglSurface = + glObjectsProvider.createFocusedPlaceholderEglSurface(eglContext, eglDisplay); + return Pair.create(eglContext, eglSurface); } private static final class InputStreamInfo { 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 867d42abab..5ab0882b0b 100644 --- a/libraries/effect/src/main/java/androidx/media3/effect/FinalShaderProgramWrapper.java +++ b/libraries/effect/src/main/java/androidx/media3/effect/FinalShaderProgramWrapper.java @@ -83,6 +83,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; private final List rgbMatrices; private final EGLDisplay eglDisplay; private final EGLContext eglContext; + private final EGLSurface placeholderSurface; private final DebugViewProvider debugViewProvider; private final ColorInfo outputColorInfo; private final VideoFrameProcessingTaskExecutor videoFrameProcessingTaskExecutor; @@ -129,6 +130,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; Context context, EGLDisplay eglDisplay, EGLContext eglContext, + EGLSurface placeholderSurface, DebugViewProvider debugViewProvider, ColorInfo outputColorInfo, VideoFrameProcessingTaskExecutor videoFrameProcessingTaskExecutor, @@ -143,6 +145,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; this.rgbMatrices = new ArrayList<>(); this.eglDisplay = eglDisplay; this.eglContext = eglContext; + this.placeholderSurface = placeholderSurface; this.debugViewProvider = debugViewProvider; this.outputColorInfo = outputColorInfo; this.videoFrameProcessingTaskExecutor = videoFrameProcessingTaskExecutor; @@ -354,16 +357,10 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; // outputEglSurface is a graphics buffer producer for a BufferQueue, and // this.outputSurfaceInfo.surface is the associated consumer. The consumer owns the // BufferQueue https://source.android.com/docs/core/graphics/arch-bq-gralloc#BufferQueue. - // If the consumer and the BufferQueue are released while the producer is still alive, EGL - // gets stuck trying to dequeue a new buffer from the released BufferQueue. This probably + // If the BufferQueue is released while the producer is still alive, EGL gets stuck trying + // to dequeue a new buffer from the released BufferQueue. This probably // happens when the previously queued back buffer is ready for display. - try { - GlUtil.destroyEglSurface(eglDisplay, outputEglSurface); - } catch (GlUtil.GlException e) { - videoFrameProcessorListenerExecutor.execute( - () -> videoFrameProcessorListener.onError(VideoFrameProcessingException.from(e))); - } - this.outputEglSurface = null; + destroyOutputEglSurface(); } outputSurfaceInfoChanged = this.outputSurfaceInfo == null @@ -374,6 +371,24 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; this.outputSurfaceInfo = outputSurfaceInfo; } + private synchronized void destroyOutputEglSurface() { + if (outputEglSurface == null) { + return; + } + try { + // outputEglSurface will be destroyed only if it's not current. + // See EGL docs. Make the placeholder surface current before destroying. + GlUtil.focusEglSurface( + eglDisplay, eglContext, placeholderSurface, /* width= */ 1, /* height= */ 1); + GlUtil.destroyEglSurface(eglDisplay, outputEglSurface); + } catch (GlUtil.GlException e) { + videoFrameProcessorListenerExecutor.execute( + () -> videoFrameProcessorListener.onError(VideoFrameProcessingException.from(e))); + } finally { + this.outputEglSurface = null; + } + } + private synchronized void renderFrame( GlObjectsProvider glObjectsProvider, GlTextureInfo inputTexture,