Android EGL: Synchronize calls to eglSwapBuffers and eglMakeCurrent

Android EGL: Synchronize calls to eglSwapBuffers and eglMakeCurrent

BUG=webrtc:5702
R=glaznev@webrtc.org, perkj@webrtc.org

Committed: https://crrev.com/3db6f9b4dff2e220c0b50358881201f34b2a0036
Cr-Commit-Position: refs/heads/master@{#12178}

[magjed_webrtc, 2016-03-30 14:55:23]

Description was changed from

==========
Android EGL: Synchronize calls to eglSwapBuffers and eglMakeCurrent

BUG=webrtc:5702
==========

to

==========
Android EGL: Synchronize calls to eglSwapBuffers and eglMakeCurrent

BUG=webrtc:5702
==========

[magjed_webrtc, 2016-03-30 14:55:23]

magjed@webrtc.org changed reviewers:
+ glaznev@webrtc.org, perkj@webrtc.org

[magjed_webrtc, 2016-03-30 14:56:05]

Please take a look.

[perkj_webrtc, 2016-03-30 15:21:50]

lgtm with the suggested comments added.

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
File webrtc/api/java/android/org/webrtc/EglBase.java (right):

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
webrtc/api/java/android/org/webrtc/EglBase.java:31: // deadlock.
please add link to bug.

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
File webrtc/api/java/android/org/webrtc/SurfaceTextureHelper.java (right):

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
webrtc/api/java/android/org/webrtc/SurfaceTextureHelper.java:454: synchronized
(EglBase.lock) {
please add a comment and a reference to the bug. This otherwise looks very
hacky.

[AlexG, 2016-03-30 16:54:23]

lgtm

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
File webrtc/api/java/android/org/webrtc/EglBase10.java (right):

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
webrtc/api/java/android/org/webrtc/EglBase10.java:236: throw new
RuntimeException("eglMakeCurrent failed");
Use a little different name in exception to distinguish from makeCurrent
exception

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
File webrtc/api/java/android/org/webrtc/EglBase14.java (right):

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
webrtc/api/java/android/org/webrtc/EglBase14.java:184: throw new
RuntimeException("eglMakeCurrent failed");
"eglMakeCurrent" -> "eglDetachCurrent" or some other name to differentiate this
exception with makeCurrent call

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
File webrtc/api/java/android/org/webrtc/SurfaceTextureHelper.java (right):

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
webrtc/api/java/android/org/webrtc/SurfaceTextureHelper.java:453:
eglBase.makeCurrent();
Is it an overhead to call eglMakeCurrent here? It is already called in
SurfaceTextureHelper ctor, so EGL context should be current on this thread.

[magjed_webrtc, 2016-03-31 08:50:08]

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
File webrtc/api/java/android/org/webrtc/EglBase.java (right):

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
webrtc/api/java/android/org/webrtc/EglBase.java:31: // deadlock.
On 2016/03/30 15:21:50, perkj_webrtc wrote:
> please add link to bug.

Done.

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
File webrtc/api/java/android/org/webrtc/EglBase10.java (right):

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
webrtc/api/java/android/org/webrtc/EglBase10.java:236: throw new
RuntimeException("eglMakeCurrent failed");
On 2016/03/30 16:54:22, AlexG wrote:
> Use a little different name in exception to distinguish from makeCurrent
> exception

Done.

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
File webrtc/api/java/android/org/webrtc/EglBase14.java (right):

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
webrtc/api/java/android/org/webrtc/EglBase14.java:184: throw new
RuntimeException("eglMakeCurrent failed");
On 2016/03/30 16:54:22, AlexG wrote:
> "eglMakeCurrent" -> "eglDetachCurrent" or some other name to differentiate
this
> exception with makeCurrent call

Done.

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
File webrtc/api/java/android/org/webrtc/SurfaceTextureHelper.java (right):

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
webrtc/api/java/android/org/webrtc/SurfaceTextureHelper.java:453:
eglBase.makeCurrent();
On 2016/03/30 16:54:22, AlexG wrote:
> Is it an overhead to call eglMakeCurrent here? It is already called in
> SurfaceTextureHelper ctor, so EGL context should be current on this thread.

I don't know how much overhead it is when the context is already current, but it
does indeed look unnecessary, so I'm removing it.

https://codereview.webrtc.org/1848483002/diff/1/webrtc/api/java/android/org/w...
webrtc/api/java/android/org/webrtc/SurfaceTextureHelper.java:454: synchronized
(EglBase.lock) {
On 2016/03/30 15:21:50, perkj_webrtc wrote:
> please add a comment and a reference to the bug. This otherwise looks very
> hacky.

Done.

[magjed_webrtc, 2016-03-31 08:50:14]

The CQ bit was checked by magjed@webrtc.org

[magjed_webrtc, 2016-03-31 08:50:14]

The patchset sent to the CQ was uploaded after l-g-t-m from glaznev@webrtc.org,
perkj@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1848483002/#ps20001 (title:
"Address comments")

[commit-bot: I haz the power, 2016-03-31 08:50:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1848483002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1848483002/20001

[commit-bot: I haz the power, 2016-03-31 09:24:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-31 09:24:53]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[magjed_webrtc, 2016-03-31 11:17:01]

On 2016/03/31 09:24:53, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

All bots look fine without android_arm64_rel that timed out. I'm landing this CL
manually.

[magjed_webrtc, 2016-03-31 11:17:25]

Description was changed from

==========
Android EGL: Synchronize calls to eglSwapBuffers and eglMakeCurrent

BUG=webrtc:5702
==========

to

==========
Android EGL: Synchronize calls to eglSwapBuffers and eglMakeCurrent

BUG=webrtc:5702
R=glaznev@webrtc.org, perkj@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/3db6f9b4dff2e220c0b503588...
==========

[magjed_webrtc, 2016-03-31 11:17:26]

Committed patchset #2 (id:20001) manually as
3db6f9b4dff2e220c0b50358881201f34b2a0036 (presubmit successful).

[commit-bot: I haz the power, 2016-03-31 11:17:27]

Description was changed from

==========
Android EGL: Synchronize calls to eglSwapBuffers and eglMakeCurrent

BUG=webrtc:5702
R=glaznev@webrtc.org, perkj@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/3db6f9b4dff2e220c0b503588...
==========

to

==========
Android EGL: Synchronize calls to eglSwapBuffers and eglMakeCurrent

BUG=webrtc:5702
R=glaznev@webrtc.org, perkj@webrtc.org

Committed: https://crrev.com/3db6f9b4dff2e220c0b50358881201f34b2a0036
Cr-Commit-Position: refs/heads/master@{#12178}
==========

[commit-bot: I haz the power, 2016-03-31 11:17:28]

Patchset 2 (id:??) landed as
https://crrev.com/3db6f9b4dff2e220c0b50358881201f34b2a0036
Cr-Commit-Position: refs/heads/master@{#12178}