Android SurfaceViewRenderer: Create EGL context on render thread

webrtc/api/android/java/src/org/webrtc/SurfaceViewRenderer.java

Index: webrtc/api/android/java/src/org/webrtc/SurfaceViewRenderer.java
diff --git a/webrtc/api/android/java/src/org/webrtc/SurfaceViewRenderer.java b/webrtc/api/android/java/src/org/webrtc/SurfaceViewRenderer.java
index fd02fd583af83d8d7fcc1c5d81b910d92662c6a8..daeada85d74fe6a1d969f040b88c5c3738f2c56f 100644
--- a/webrtc/api/android/java/src/org/webrtc/SurfaceViewRenderer.java
+++ b/webrtc/api/android/java/src/org/webrtc/SurfaceViewRenderer.java
@@ -145,8 +145,9 @@ public class SurfaceViewRenderer extends SurfaceView
    * |drawer|. It is allowed to call init() to reinitialize the renderer after a previous
    * init()/release() cycle.
    */
-  public void init(EglBase.Context sharedContext, RendererCommon.RendererEvents rendererEvents,
-      int[] configAttributes, RendererCommon.GlDrawer drawer) {
+  public void init(
+      final EglBase.Context sharedContext, RendererCommon.RendererEvents rendererEvents,
+      final int[] configAttributes, RendererCommon.GlDrawer drawer) {
     synchronized (handlerLock) {
       if (renderThreadHandler != null) {
         throw new IllegalStateException(getResourceName() + "Already initialized");
@@ -156,8 +157,16 @@ public class SurfaceViewRenderer extends SurfaceView
       this.drawer = drawer;
       renderThread = new HandlerThread(TAG);
       renderThread.start();
-      eglBase = EglBase.create(sharedContext, configAttributes);
       renderThreadHandler = new Handler(renderThread.getLooper());
+      // 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.

[perkj_webrtc, 2016/09/14 07:02:05]

nit: link comment to bug. 

[magjed_webrtc, 2016/09/14 07:50:00]

Done.

+      ThreadUtils.invokeAtFrontUninterruptibly(renderThreadHandler, new Runnable() {

[sakal, 2016/09/14 07:10:05]

Invoking is safer but do we need to do that? I think all access to the eglBase
is done on the rendering thread so it should be safe to post?

[magjed_webrtc, 2016/09/14 07:50:00]

I'm worried about release() being called immediately after init(). In release()
we postAtFrontOfQueue(), so there will be a race between that and the post from
init(). If release() is executed before init(), we will currently fail with null
pointer exception on EglBase. I could add a check for that, but init() will
still be called after release() which means that we leak the EglBase.

I would like to land this fix as simple as possible, and the current fix is as
simple as it can be I think.

I can change this class to always post normally, i.e. not at front of queue, in
a follow-up CL, to preserve the fifo order. Then I can remove the invoke.

+        @Override
+        public void run() {
+          eglBase = EglBase.create(sharedContext, configAttributes);

[perkj_webrtc, 2016/09/14 07:02:05]

ok- so previously we created it on this thread and released it on the render
thread. 
This lgtm

[magjed_webrtc, 2016/09/14 07:50:00]

Yes, which should be fine according to the documentation, but doesn't work in
practice on all devices.

+        }
+      });
     }
     tryCreateEglSurface();
   }