Android SurfaceViewRenderer: Create EGL context on render thread

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

 /*
  *  Copyright 2015 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 127 matching lines @@
       EglBase.Context sharedContext, RendererCommon.RendererEvents rendererEvents) {
     init(sharedContext, rendererEvents, EglBase.CONFIG_PLAIN, new GlRectDrawer());
   }
 
   /**
    * 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
    * |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");
       }
       Logging.d(TAG, getResourceName() + "Initializing.");
       this.rendererEvents = rendererEvents;
       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();
   }
 
   /**
    * Create and make an EGLSurface current if both init() and surfaceCreated() have been called.
    */
   public void tryCreateEglSurface() {
     // |renderThreadHandler| is only created after |eglBase| is created in init(), so the
     // following code will only execute if eglBase != null.
@@ skipping 391 matching lines @@
       if (framesReceived > 0 && framesRendered > 0) {
         final long timeSinceFirstFrameNs = System.nanoTime() - firstFrameTimeNs;
         Logging.d(TAG, getResourceName() + "Duration: " + (int) (timeSinceFirstFrameNs / 1e6) +
             " ms. FPS: " + framesRendered * 1e9 / timeSinceFirstFrameNs);
         Logging.d(TAG, getResourceName() + "Average render time: "
             + (int) (renderTimeNs / (1000 * framesRendered)) + " us.");
       }
     }
   }
 }