Enable SurfaceViewRenderer for AppRTCDemo

Enable SurfaceViewRenderer for AppRTCDemo

BUG=webrtc:4742, webrtc:4910, webrtc:4909
R=glaznev@webrtc.org, perkj@webrtc.org

Committed: https://crrev.com/7076729c57c27aa813760d2038be02c36f4d7649
Cr-Commit-Position: refs/heads/master@{#10054}

[magjed_webrtc, 2015-09-21 08:45:36]

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

[magjed_webrtc, 2015-09-21 08:45:48]

Here you go.

[perkj_webrtc, 2015-09-21 15:03:53]

https://codereview.webrtc.org/1356603004/diff/20001/webrtc/examples/androidap...
File webrtc/examples/androidapp/res/layout/activity_call.xml (right):

https://codereview.webrtc.org/1356603004/diff/20001/webrtc/examples/androidap...
webrtc/examples/androidapp/res/layout/activity_call.xml:14:
android:id="@+id/remote_video_view"
indentation

https://codereview.webrtc.org/1356603004/diff/20001/webrtc/examples/androidap...
File webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java
(right):

https://codereview.webrtc.org/1356603004/diff/20001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java:281:
localRender.release();
renderer.release needs to be done before the factory is destroyed since the
renderer might hold on to a video frame. When the factory is destroyed the
signaling thread can be destroyed.

https://codereview.webrtc.org/1356603004/diff/20001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java:488:
Log.w(TAG, "Room is connected, but EGL context is not ready yet.");
Is this still true?

[magjed_webrtc, 2015-09-22 06:59:54]

Patchset #1 (id:1) has been deleted

[magjed_webrtc, 2015-09-22 07:15:46]

perkj - Please take another look.

https://codereview.webrtc.org/1356603004/diff/20001/webrtc/examples/androidap...
File webrtc/examples/androidapp/res/layout/activity_call.xml (right):

https://codereview.webrtc.org/1356603004/diff/20001/webrtc/examples/androidap...
webrtc/examples/androidapp/res/layout/activity_call.xml:14:
android:id="@+id/remote_video_view"
On 2015/09/21 15:03:53, perkj1 wrote:
> indentation

Done.

https://codereview.webrtc.org/1356603004/diff/20001/webrtc/examples/androidap...
File webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java
(right):

https://codereview.webrtc.org/1356603004/diff/20001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java:281:
localRender.release();
On 2015/09/21 15:03:53, perkj1 wrote:
> renderer.release needs to be done before the factory is destroyed since the
> renderer might hold on to a video frame. When the factory is destroyed the
> signaling thread can be destroyed.

Done. I moved the renderer.release() inside disconnect() before the
peerconnection factory is destroyed.

https://codereview.webrtc.org/1356603004/diff/20001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java:488:
Log.w(TAG, "Room is connected, but EGL context is not ready yet.");
On 2015/09/21 15:03:53, perkj1 wrote:
> Is this still true? 

No. I inlined createPeerConnectionFactory() and removed this.

[perkj_webrtc, 2015-09-22 12:24:43]

lgtm, but you might want to ask if it is ok to inline the creation of pcfactory.

Might want to add that we have tested this on multiple devices.

https://codereview.webrtc.org/1356603004/diff/20001/webrtc/examples/androidap...
File webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java
(right):

https://codereview.webrtc.org/1356603004/diff/20001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java:397: private
void createPeerConnectionFactory() {
This is ok by me to remove this. But I am not sure if glaznew want to keep the
async behaviour to speed up the creation of the activity.

[magjed_webrtc, 2015-09-22 13:18:17]

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

[magjed_webrtc, 2015-09-22 13:22:25]

Alex - Please take a look. We would like to enable the new renderer for
AppRTCDemo.

https://codereview.webrtc.org/1356603004/diff/40001/webrtc/examples/androidap...
File webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java (left):

https://codereview.webrtc.org/1356603004/diff/40001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java:277:
super.onDestroy();
Note: I moved super.onDestroy() to the bottom of this function.

https://codereview.webrtc.org/1356603004/diff/40001/webrtc/examples/androidap...
File webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java
(right):

https://codereview.webrtc.org/1356603004/diff/40001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java:178:
rootEglBase = new EglBase();
If you want, I can preserve the async EGL creation with something like this:
new Thread(new Runnable() {
  @Override
  public void run() {
    rootEglBase = new EglBase();
    localRender.init(rootEglBase.getContext(), null);
    remoteRender.init(rootEglBase.getContext(), null);
    localRender.setZOrderMediaOverlay(true);
    createPeerConnectionFactory();
  }
}).start();

[AlexG, 2015-09-23 23:49:28]

lgtm

https://codereview.webrtc.org/1356603004/diff/40001/webrtc/examples/androidap...
File webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java
(right):

https://codereview.webrtc.org/1356603004/diff/40001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java:285:
rootEglBase = null;
nit: "=null" is not necessary

https://codereview.webrtc.org/1356603004/diff/40001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java:402: if
(localRender != null) {
Probably worth to close renderers after peer connection client closing.

[magjed_webrtc, 2015-09-24 14:01:47]

https://codereview.webrtc.org/1356603004/diff/40001/webrtc/examples/androidap...
File webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java
(right):

https://codereview.webrtc.org/1356603004/diff/40001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java:285:
rootEglBase = null;
On 2015/09/23 23:49:28, AlexG wrote:
> nit: "=null" is not necessary

Done.

https://codereview.webrtc.org/1356603004/diff/40001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java:402: if
(localRender != null) {
On 2015/09/23 23:49:28, AlexG wrote:
> Probably worth to close renderers after peer connection client closing.

Done.

[magjed_webrtc, 2015-09-24 14:02:22]

Committed patchset #3 (id:60001) manually as
7076729c57c27aa813760d2038be02c36f4d7649 (presubmit successful).

[commit-bot: I haz the power, 2015-09-24 14:02:28]

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