Revert "Adopt EglThread in EglRenderer" This reverts commit f4d0a493b49eef7e55c66e4d03d329e3e655b182. Reason for revert: Potentially causing crash in eglSurfaceCreationRunnable (b/286664896) Original change's description: > Adopt EglThread in EglRenderer > > This allows EglRenderer to share render thread EGLContext with other renderers. > go/meet-android-eglcontext-reduction > > Bug: b/225229697 > Change-Id: I8aa41e61ada195fbbe3338c05815b26f3238dd78 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/306281 > Reviewed-by: Xavier Lepaul <xalep@webrtc.org> > Commit-Queue: Linus Nilsson <lnilsson@webrtc.org> > Cr-Commit-Position: refs/heads/main@{#40141} Bug: b/225229697 Change-Id: I4c57ea88047bde6e9782f0ce76bdaacd1bad4af3 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/308580 Commit-Queue: Linus Nilsson <lnilsson@webrtc.org> Reviewed-by: Xavier Lepaul <xalep@webrtc.org> Cr-Commit-Position: refs/heads/main@{#40266}
diff --git a/sdk/android/api/org/webrtc/EglRenderer.java b/sdk/android/api/org/webrtc/EglRenderer.java index a8dee89..5ab0868 100644 --- a/sdk/android/api/org/webrtc/EglRenderer.java +++ b/sdk/android/api/org/webrtc/EglRenderer.java
@@ -14,8 +14,11 @@ import android.graphics.Matrix; import android.graphics.SurfaceTexture; import android.opengl.GLES20; +import android.os.Handler; +import android.os.HandlerThread; +import android.os.Looper; +import android.os.Message; import android.view.Surface; -import androidx.annotation.GuardedBy; import androidx.annotation.Nullable; import java.nio.ByteBuffer; import java.text.DecimalFormat; @@ -83,11 +86,35 @@ } } + /** + * Handler that triggers a callback when an uncaught exception happens when handling a message. + */ + private static class HandlerWithExceptionCallback extends Handler { + private final Runnable exceptionCallback; + + public HandlerWithExceptionCallback(Looper looper, Runnable exceptionCallback) { + super(looper); + this.exceptionCallback = exceptionCallback; + } + + @Override + public void dispatchMessage(Message msg) { + try { + super.dispatchMessage(msg); + } catch (Exception e) { + Logging.e(TAG, "Exception on EglRenderer thread", e); + exceptionCallback.run(); + throw e; + } + } + } + protected final String name; - // `eglThread` is used for rendering, and is synchronized on `threadLock`. - private final Object threadLock = new Object(); - @GuardedBy("threadLock") @Nullable private EglThread eglThread; + // `renderThreadHandler` is a handler for communicating with `renderThread`, and is synchronized + // on `handlerLock`. + private final Object handlerLock = new Object(); + @Nullable private Handler renderThreadHandler; private final ArrayList<FrameListenerAndParams> frameListeners = new ArrayList<>(); @@ -145,10 +172,10 @@ @Override public void run() { logStatistics(); - synchronized (threadLock) { - if (eglThread != null) { - eglThread.getHandler().removeCallbacks(logStatisticsRunnable); - eglThread.getHandler().postDelayed( + synchronized (handlerLock) { + if (renderThreadHandler != null) { + renderThreadHandler.removeCallbacks(logStatisticsRunnable); + renderThreadHandler.postDelayed( logStatisticsRunnable, TimeUnit.SECONDS.toMillis(LOG_INTERVAL_SEC)); } } @@ -158,8 +185,8 @@ private final EglSurfaceCreation eglSurfaceCreationRunnable = new EglSurfaceCreation(); /** - * Standard constructor. The name will be included when logging. In order to render something, you - * must first call init() and createEglSurface. + * Standard constructor. The name will be used for the render thread name and included when + * logging. In order to render something, you must first call init() and createEglSurface. */ public EglRenderer(String name) { this(name, new VideoFrameDrawer()); @@ -170,29 +197,6 @@ this.frameDrawer = videoFrameDrawer; } - public void init( - EglThread eglThread, RendererCommon.GlDrawer drawer, boolean usePresentationTimeStamp) { - synchronized (threadLock) { - if (this.eglThread != null) { - throw new IllegalStateException(name + "Already initialized"); - } - - logD("Initializing EglRenderer"); - this.eglThread = eglThread; - this.drawer = drawer; - this.usePresentationTimeStamp = usePresentationTimeStamp; - - eglBase = eglThread.createEglBaseWithSharedConnection(); - eglThread.getHandler().post(eglSurfaceCreationRunnable); - - final long currentTimeNs = System.nanoTime(); - resetStatistics(currentTimeNs); - - eglThread.getHandler().postDelayed( - logStatisticsRunnable, TimeUnit.SECONDS.toMillis(LOG_INTERVAL_SEC)); - } - } - /** * Initialize this class, sharing resources with `sharedContext`. The custom `drawer` will be used * for drawing frames on the EGLSurface. This class is responsible for calling release() on @@ -203,9 +207,46 @@ */ public void init(@Nullable final EglBase.Context sharedContext, final int[] configAttributes, RendererCommon.GlDrawer drawer, boolean usePresentationTimeStamp) { - EglThread thread = - EglThread.create(/* releaseMonitor= */ null, sharedContext, configAttributes); - init(thread, drawer, usePresentationTimeStamp); + synchronized (handlerLock) { + if (renderThreadHandler != null) { + throw new IllegalStateException(name + "Already initialized"); + } + logD("Initializing EglRenderer"); + this.drawer = drawer; + this.usePresentationTimeStamp = usePresentationTimeStamp; + + final HandlerThread renderThread = new HandlerThread(name + "EglRenderer"); + renderThread.start(); + renderThreadHandler = + new HandlerWithExceptionCallback(renderThread.getLooper(), new Runnable() { + @Override + public void run() { + synchronized (handlerLock) { + renderThreadHandler = null; + } + } + }); + // Create EGL context on the newly created render thread. It should be possibly to create the + // context on this thread and make it current on the render thread, but this causes failure on + // some Marvel based JB devices. https://bugs.chromium.org/p/webrtc/issues/detail?id=6350. + ThreadUtils.invokeAtFrontUninterruptibly(renderThreadHandler, () -> { + // If sharedContext is null, then texture frames are disabled. This is typically for old + // devices that might not be fully spec compliant, so force EGL 1.0 since EGL 1.4 has + // caused trouble on some weird devices. + if (sharedContext == null) { + logD("EglBase10.create context"); + eglBase = EglBase.createEgl10(configAttributes); + } else { + logD("EglBase.create shared context"); + eglBase = EglBase.create(sharedContext, configAttributes); + } + }); + renderThreadHandler.post(eglSurfaceCreationRunnable); + final long currentTimeNs = System.nanoTime(); + resetStatistics(currentTimeNs); + renderThreadHandler.postDelayed( + logStatisticsRunnable, TimeUnit.SECONDS.toMillis(LOG_INTERVAL_SEC)); + } } /** @@ -240,15 +281,14 @@ public void release() { logD("Releasing."); final CountDownLatch eglCleanupBarrier = new CountDownLatch(1); - synchronized (threadLock) { - if (eglThread == null) { + synchronized (handlerLock) { + if (renderThreadHandler == null) { logD("Already released"); return; } - eglThread.getHandler().removeCallbacks(logStatisticsRunnable); - + renderThreadHandler.removeCallbacks(logStatisticsRunnable); // Release EGL and GL resources on render thread. - eglThread.getHandler().postAtFrontOfQueue(() -> { + renderThreadHandler.postAtFrontOfQueue(() -> { // Detach current shader program. synchronized (EglBase.lock) { GLES20.glUseProgram(/* program= */ 0); @@ -259,19 +299,23 @@ } frameDrawer.release(); bitmapTextureFramebuffer.release(); - if (eglBase != null) { + logD("eglBase detach and release."); + eglBase.detachCurrent(); eglBase.release(); eglBase = null; } - frameListeners.clear(); eglCleanupBarrier.countDown(); }); - + final Looper renderLooper = renderThreadHandler.getLooper(); + // TODO(magjed): Replace this post() with renderLooper.quitSafely() when API support >= 18. + renderThreadHandler.post(() -> { + logD("Quitting render thread."); + renderLooper.quit(); + }); // Don't accept any more frames or messages to the render thread. - eglThread.release(); - eglThread = null; + renderThreadHandler = null; } // Make sure the EGL/GL cleanup posted above is executed. ThreadUtils.awaitUninterruptibly(eglCleanupBarrier); @@ -299,9 +343,9 @@ } public void printStackTrace() { - synchronized (threadLock) { + synchronized (handlerLock) { final Thread renderThread = - (eglThread == null) ? null : eglThread.getHandler().getLooper().getThread(); + (renderThreadHandler == null) ? null : renderThreadHandler.getLooper().getThread(); if (renderThread != null) { final StackTraceElement[] renderStackTrace = renderThread.getStackTrace(); if (renderStackTrace.length > 0) { @@ -431,11 +475,11 @@ */ public void removeFrameListener(final FrameListener listener) { final CountDownLatch latch = new CountDownLatch(1); - synchronized (threadLock) { - if (eglThread == null) { + synchronized (handlerLock) { + if (renderThreadHandler == null) { return; } - if (Thread.currentThread() == eglThread.getHandler().getLooper().getThread()) { + if (Thread.currentThread() == renderThreadHandler.getLooper().getThread()) { throw new RuntimeException("removeFrameListener must not be called on the render thread."); } postToRenderThread(() -> { @@ -463,8 +507,8 @@ ++framesReceived; } final boolean dropOldFrame; - synchronized (threadLock) { - if (eglThread == null) { + synchronized (handlerLock) { + if (renderThreadHandler == null) { logD("Dropping frame - Not initialized or already released."); return; } @@ -475,7 +519,7 @@ } pendingFrame = frame; pendingFrame.retain(); - eglThread.getHandler().post(this::renderFrameOnRenderThread); + renderThreadHandler.post(this ::renderFrameOnRenderThread); } } if (dropOldFrame) { @@ -492,11 +536,12 @@ // Ensure that the render thread is no longer touching the Surface before returning from this // function. eglSurfaceCreationRunnable.setSurface(null /* surface */); - synchronized (threadLock) { - if (eglThread != null) { - eglThread.getHandler().removeCallbacks(eglSurfaceCreationRunnable); - eglThread.getHandler().postAtFrontOfQueue(() -> { + synchronized (handlerLock) { + if (renderThreadHandler != null) { + renderThreadHandler.removeCallbacks(eglSurfaceCreationRunnable); + renderThreadHandler.postAtFrontOfQueue(() -> { if (eglBase != null) { + eglBase.detachCurrent(); eglBase.releaseSurface(); } completionCallback.run(); @@ -511,9 +556,9 @@ * Private helper function to post tasks safely. */ private void postToRenderThread(Runnable runnable) { - synchronized (threadLock) { - if (eglThread != null) { - eglThread.getHandler().post(runnable); + synchronized (handlerLock) { + if (renderThreadHandler != null) { + renderThreadHandler.post(runnable); } } } @@ -521,7 +566,6 @@ private void clearSurfaceOnRenderThread(float r, float g, float b, float a) { if (eglBase != null && eglBase.hasSurface()) { logD("clearSurface"); - eglBase.makeCurrent(); GLES20.glClearColor(r, g, b, a); GLES20.glClear(GLES20.GL_COLOR_BUFFER_BIT); eglBase.swapBuffers(); @@ -539,11 +583,11 @@ * Post a task to clear the surface to a specific color. */ public void clearImage(final float r, final float g, final float b, final float a) { - synchronized (threadLock) { - if (eglThread == null) { + synchronized (handlerLock) { + if (renderThreadHandler == null) { return; } - eglThread.getHandler().postAtFrontOfQueue(() -> clearSurfaceOnRenderThread(r, g, b, a)); + renderThreadHandler.postAtFrontOfQueue(() -> clearSurfaceOnRenderThread(r, g, b, a)); } } @@ -565,8 +609,6 @@ frame.release(); return; } - eglBase.makeCurrent(); - // Check if fps reduction is active. final boolean shouldRenderFrame; synchronized (fpsReductionLock) {