From 3db6f9b4dff2e220c0b50358881201f34b2a0036 Mon Sep 17 00:00:00 2001 From: Magnus Jedvert Date: Thu, 31 Mar 2016 13:17:11 +0200 Subject: [PATCH] Android EGL: Synchronize calls to eglSwapBuffers and eglMakeCurrent BUG=webrtc:5702 R=glaznev@webrtc.org, perkj@webrtc.org Review URL: https://codereview.webrtc.org/1848483002 . Cr-Commit-Position: refs/heads/master@{#12178} --- .../api/java/android/org/webrtc/EglBase.java | 6 +++++ .../java/android/org/webrtc/EglBase10.java | 18 ++++++++----- .../java/android/org/webrtc/EglBase14.java | 26 ++++++++++++------- .../org/webrtc/SurfaceTextureHelper.java | 9 ++++--- 4 files changed, 41 insertions(+), 18 deletions(-) diff --git a/webrtc/api/java/android/org/webrtc/EglBase.java b/webrtc/api/java/android/org/webrtc/EglBase.java index 2f3fcffee6..05dd8060d4 100644 --- a/webrtc/api/java/android/org/webrtc/EglBase.java +++ b/webrtc/api/java/android/org/webrtc/EglBase.java @@ -25,6 +25,12 @@ public abstract class EglBase { public static class Context { } + // According to the documentation, EGL can be used from multiple threads at the same time if each + // thread has its own EGLContext, but in practice it deadlocks on some devices when doing this. + // Therefore, synchronize on this global lock before calling dangerous EGL functions that might + // deadlock. See https://bugs.chromium.org/p/webrtc/issues/detail?id=5702 for more info. + public static final Object lock = new Object(); + // These constants are taken from EGL14.EGL_OPENGL_ES2_BIT and EGL14.EGL_CONTEXT_CLIENT_VERSION. // https://android.googlesource.com/platform/frameworks/base/+/master/opengl/java/android/opengl/EGL14.java // This is similar to how GlSurfaceView does: diff --git a/webrtc/api/java/android/org/webrtc/EglBase10.java b/webrtc/api/java/android/org/webrtc/EglBase10.java index fd3644f77a..fe2b73ee1c 100644 --- a/webrtc/api/java/android/org/webrtc/EglBase10.java +++ b/webrtc/api/java/android/org/webrtc/EglBase10.java @@ -220,17 +220,21 @@ final class EglBase10 extends EglBase { if (eglSurface == EGL10.EGL_NO_SURFACE) { throw new RuntimeException("No EGLSurface - can't make current"); } - if (!egl.eglMakeCurrent(eglDisplay, eglSurface, eglSurface, eglContext)) { - throw new RuntimeException("eglMakeCurrent failed"); + synchronized (EglBase.lock) { + if (!egl.eglMakeCurrent(eglDisplay, eglSurface, eglSurface, eglContext)) { + throw new RuntimeException("eglMakeCurrent failed"); + } } } // Detach the current EGL context, so that it can be made current on another thread. @Override public void detachCurrent() { - if (!egl.eglMakeCurrent( - eglDisplay, EGL10.EGL_NO_SURFACE, EGL10.EGL_NO_SURFACE, EGL10.EGL_NO_CONTEXT)) { - throw new RuntimeException("eglMakeCurrent failed"); + synchronized (EglBase.lock) { + if (!egl.eglMakeCurrent( + eglDisplay, EGL10.EGL_NO_SURFACE, EGL10.EGL_NO_SURFACE, EGL10.EGL_NO_CONTEXT)) { + throw new RuntimeException("eglDetachCurrent failed"); + } } } @@ -240,7 +244,9 @@ final class EglBase10 extends EglBase { if (eglSurface == EGL10.EGL_NO_SURFACE) { throw new RuntimeException("No EGLSurface - can't swap buffers"); } - egl.eglSwapBuffers(eglDisplay, eglSurface); + synchronized (EglBase.lock) { + egl.eglSwapBuffers(eglDisplay, eglSurface); + } } // Return an EGLDisplay, or die trying. diff --git a/webrtc/api/java/android/org/webrtc/EglBase14.java b/webrtc/api/java/android/org/webrtc/EglBase14.java index 5b377e0bad..6ac61d062c 100644 --- a/webrtc/api/java/android/org/webrtc/EglBase14.java +++ b/webrtc/api/java/android/org/webrtc/EglBase14.java @@ -168,17 +168,21 @@ public final class EglBase14 extends EglBase { if (eglSurface == EGL14.EGL_NO_SURFACE) { throw new RuntimeException("No EGLSurface - can't make current"); } - if (!EGL14.eglMakeCurrent(eglDisplay, eglSurface, eglSurface, eglContext)) { - throw new RuntimeException("eglMakeCurrent failed"); + synchronized (EglBase.lock) { + if (!EGL14.eglMakeCurrent(eglDisplay, eglSurface, eglSurface, eglContext)) { + throw new RuntimeException("eglMakeCurrent failed"); + } } } // Detach the current EGL context, so that it can be made current on another thread. @Override public void detachCurrent() { - if (!EGL14.eglMakeCurrent( - eglDisplay, EGL14.EGL_NO_SURFACE, EGL14.EGL_NO_SURFACE, EGL14.EGL_NO_CONTEXT)) { - throw new RuntimeException("eglMakeCurrent failed"); + synchronized (EglBase.lock) { + if (!EGL14.eglMakeCurrent( + eglDisplay, EGL14.EGL_NO_SURFACE, EGL14.EGL_NO_SURFACE, EGL14.EGL_NO_CONTEXT)) { + throw new RuntimeException("eglDetachCurrent failed"); + } } } @@ -188,7 +192,9 @@ public final class EglBase14 extends EglBase { if (eglSurface == EGL14.EGL_NO_SURFACE) { throw new RuntimeException("No EGLSurface - can't swap buffers"); } - EGL14.eglSwapBuffers(eglDisplay, eglSurface); + synchronized (EglBase.lock) { + EGL14.eglSwapBuffers(eglDisplay, eglSurface); + } } public void swapBuffers(long timeStampNs) { @@ -196,9 +202,11 @@ public final class EglBase14 extends EglBase { if (eglSurface == EGL14.EGL_NO_SURFACE) { throw new RuntimeException("No EGLSurface - can't swap buffers"); } - // See https://android.googlesource.com/platform/frameworks/native/+/tools_r22.2/opengl/specs/EGL_ANDROID_presentation_time.txt - EGLExt.eglPresentationTimeANDROID(eglDisplay, eglSurface, timeStampNs); - EGL14.eglSwapBuffers(eglDisplay, eglSurface); + synchronized (EglBase.lock) { + // See https://android.googlesource.com/platform/frameworks/native/+/tools_r22.2/opengl/specs/EGL_ANDROID_presentation_time.txt + EGLExt.eglPresentationTimeANDROID(eglDisplay, eglSurface, timeStampNs); + EGL14.eglSwapBuffers(eglDisplay, eglSurface); + } } // Return an EGLDisplay, or die trying. diff --git a/webrtc/api/java/android/org/webrtc/SurfaceTextureHelper.java b/webrtc/api/java/android/org/webrtc/SurfaceTextureHelper.java index ecff32380b..4d75d6891d 100644 --- a/webrtc/api/java/android/org/webrtc/SurfaceTextureHelper.java +++ b/webrtc/api/java/android/org/webrtc/SurfaceTextureHelper.java @@ -451,8 +451,12 @@ class SurfaceTextureHelper { isTextureInUse = true; hasPendingTexture = false; - eglBase.makeCurrent(); - surfaceTexture.updateTexImage(); + // SurfaceTexture.updateTexImage apparently can compete and deadlock with eglSwapBuffers, + // as observed on Nexus 5. Therefore, synchronize it with the EGL functions. + // See https://bugs.chromium.org/p/webrtc/issues/detail?id=5702 for more info. + synchronized (EglBase.lock) { + surfaceTexture.updateTexImage(); + } final float[] transformMatrix = new float[16]; surfaceTexture.getTransformMatrix(transformMatrix); @@ -473,7 +477,6 @@ class SurfaceTextureHelper { if (yuvConverter != null) yuvConverter.release(); } - eglBase.makeCurrent(); GLES20.glDeleteTextures(1, new int[] {oesTextureId}, 0); surfaceTexture.release(); eglBase.release();