Support building WebRTC without audio and video for Android

Support building WebRTC without audio and video for Android

This CL makes the WebRTC Java Wrapper more modular and allows the android
users to build WebRTC without audio and video(DataChannel only).

The BUILD file in sdk/android/ is modified to support modular WebRTC.

The peerconnection_jni.cc is split into peerconnection_jni.cc, video_jni.cc,
video_renderer_jni.cc and ownedfactoryandthreads.h/cc.

Add new modular build targets to JNI layer: audio_jni, video_jni,
null_audio_jni, null_video_jni. The users can link with different 
targets to for different WebRTC functionalities.

This is split from CL: https://codereview.webrtc.org/2854123003/

TBR=magjed@webrtc.org
BUG=webrtc:7613
Review-Url: https://codereview.webrtc.org/2939203002
Cr-Commit-Position: refs/heads/master@{#18647}
Committed: https://chromium.googlesource.com/external/webrtc/+/af6293517fa80247f227fa80846d74ea5eef18c2

[Zhi Huang, 2017-06-15 22:13:45]

Patchset #2 (id:20001) has been deleted

[Zhi Huang, 2017-06-15 22:25:51]

Patchset #2 (id:40001) has been deleted

[Zhi Huang, 2017-06-15 22:26:48]

The CQ bit was checked by zhihuang@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-15 22:26:59]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[Zhi Huang, 2017-06-15 22:28:19]

Description was changed from

==========
Support building WebRTC without audio and video for Android

This CL makes the WebRTC Java Wrapper more modular and allows the android
users to build WebRTC without audio and video(DataChannel only).

The BUILD file in sdk/android/ is modified to support modular WebRTC.

The peerconnection_jni.cc is split into peerconnection_jni.cc, video_jni.cc,
video_renderer_jni.cc and ownedfactoryandthreads.h/cc.

Add new modular build targets to JNI layer: audio_jni, video_jni,
null_audio_jni, null_video_jni. The users can link with different target to
get different WebRTC functionalities.

BUG=webrtc:7613
==========

to

==========
Support building WebRTC without audio and video for Android

This CL makes the WebRTC Java Wrapper more modular and allows the android
users to build WebRTC without audio and video(DataChannel only).

The BUILD file in sdk/android/ is modified to support modular WebRTC.

The peerconnection_jni.cc is split into peerconnection_jni.cc, video_jni.cc,
video_renderer_jni.cc and ownedfactoryandthreads.h/cc.

Add new modular build targets to JNI layer: audio_jni, video_jni,
null_audio_jni, null_video_jni. The users can link with different targets to for
different WebRTC functionalities.

BUG=webrtc:7613
==========

[Zhi Huang, 2017-06-15 22:28:33]

Description was changed from

==========
Support building WebRTC without audio and video for Android

This CL makes the WebRTC Java Wrapper more modular and allows the android
users to build WebRTC without audio and video(DataChannel only).

The BUILD file in sdk/android/ is modified to support modular WebRTC.

The peerconnection_jni.cc is split into peerconnection_jni.cc, video_jni.cc,
video_renderer_jni.cc and ownedfactoryandthreads.h/cc.

Add new modular build targets to JNI layer: audio_jni, video_jni,
null_audio_jni, null_video_jni. The users can link with different targets to for
different WebRTC functionalities.

BUG=webrtc:7613
==========

to

==========
Support building WebRTC without audio and video for Android

This CL makes the WebRTC Java Wrapper more modular and allows the android
users to build WebRTC without audio and video(DataChannel only).

The BUILD file in sdk/android/ is modified to support modular WebRTC.

The peerconnection_jni.cc is split into peerconnection_jni.cc, video_jni.cc,
video_renderer_jni.cc and ownedfactoryandthreads.h/cc.

Add new modular build targets to JNI layer: audio_jni, video_jni,
null_audio_jni, null_video_jni. The users can link with different 
targets to for different WebRTC functionalities.

BUG=webrtc:7613
==========

[commit-bot: I haz the power, 2017-06-15 22:49:15]

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

[commit-bot: I haz the power, 2017-06-15 22:49:16]

Dry run: This issue passed the CQ dry run.

[Zhi Huang, 2017-06-16 07:17:59]

Description was changed from

==========
Support building WebRTC without audio and video for Android

This CL makes the WebRTC Java Wrapper more modular and allows the android
users to build WebRTC without audio and video(DataChannel only).

The BUILD file in sdk/android/ is modified to support modular WebRTC.

The peerconnection_jni.cc is split into peerconnection_jni.cc, video_jni.cc,
video_renderer_jni.cc and ownedfactoryandthreads.h/cc.

Add new modular build targets to JNI layer: audio_jni, video_jni,
null_audio_jni, null_video_jni. The users can link with different 
targets to for different WebRTC functionalities.

BUG=webrtc:7613
==========

to

==========
Support building WebRTC without audio and video for Android

This CL makes the WebRTC Java Wrapper more modular and allows the android
users to build WebRTC without audio and video(DataChannel only).

The BUILD file in sdk/android/ is modified to support modular WebRTC.

The peerconnection_jni.cc is split into peerconnection_jni.cc, video_jni.cc,
video_renderer_jni.cc and ownedfactoryandthreads.h/cc.

Add new modular build targets to JNI layer: audio_jni, video_jni,
null_audio_jni, null_video_jni. The users can link with different 
targets to for different WebRTC functionalities.

This is split from CL: https://codereview.webrtc.org/2854123003/

BUG=webrtc:7613
==========

[Zhi Huang, 2017-06-16 17:45:39]

The CQ bit was checked by zhihuang@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-16 17:45:49]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-06-16 18:09:14]

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

[commit-bot: I haz the power, 2017-06-16 18:09:14]

Dry run: This issue passed the CQ dry run.

[Taylor Brandstetter, 2017-06-16 22:19:07]

deadbeef@webrtc.org changed reviewers:
+ deadbeef@webrtc.org

[Taylor Brandstetter, 2017-06-16 22:19:09]

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/BUILD.gn
File webrtc/sdk/android/BUILD.gn (right):

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/BUILD...
webrtc/sdk/android/BUILD.gn:25: rtc_static_library("base_jni") {
May want to make these rtc_source_sets

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/BUILD...
webrtc/sdk/android/BUILD.gn:215: rtc_static_library("metrics_jni") {
Do we need the two "metrics_jni" targets? It seems like we could just use the
existing one, and change its dependencies from
"libjingle_peerconnection/libjingle_peerconnection_jni" to
"peerconnection/peerconnection_jni".

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/BUILD...
webrtc/sdk/android/BUILD.gn:273: # }
May want to mention that this is still under development, and that some things
(such as "null_audio_jni" with "video_jni") won't work.

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/src/j...
File webrtc/sdk/android/src/jni/null_audio_jni.cc (right):

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/src/j...
webrtc/sdk/android/src/jni/null_audio_jni.cc:24: // support.
It's odd that this has an implementation of a CreatePeerConnectionFactory
method, which is normally implemented in a build target at a different level
(webrtc/pc/create_pc_factory). This is also related to the "video/null_audio"
combination not working. So it would be nice to have a comment summarizing the
current state and describing what would need to be done to fix it.

But if it's not too much extra work, I think it would make sense to have:

- video_jni: Real implementation of CreateVideo[En|De]coderFactory
- null_video_jni: "Null" implementation of CreateVideo[En|De]coderFactory
- audio_jni: Real implementation of CreateAudio[En|De]coderFactory, similar to
video_jni
- null_audio_jni: "Null" implementation of CreateAudio[En|De]coderFactory
- media_jni: Implementation of nativeCreatePeerConnectionFactory that creates
media engine and call factory, and calls CreateModularPeerConnectionFactory.
- null_media_jni: Implementation of nativeCreatePeerConnectionFactory that
passes null pointers into CreateModularPeerConnectionFactory.

Then that limitation would be fixed, and we'd have a slightly cleaner version of
this CL. Though maybe there are some issues I haven't considered that would make
this difficult.

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/src/j...
File webrtc/sdk/android/src/jni/ownedfactoryandthreads.h (right):

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/src/j...
webrtc/sdk/android/src/jni/ownedfactoryandthreads.h:23: #include
"webrtc/base/thread_checker.h"
Some of these headers and "using" statements aren't needed for this file.

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/src/j...
File webrtc/sdk/android/src/jni/peerconnection_jni.cc (right):

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/src/j...
webrtc/sdk/android/src/jni/peerconnection_jni.cc:123:
rtc::scoped_refptr<SurfaceTextureHelper> surface_texture_helper);
It's odd to have these declarations in the .cc file. Should at least have a
comment saying "the implementations are in either video_jni.cc or
null_video_jni.cc". But it may make sense to put them in a "video_jni.h".

[Zhi Huang, 2017-06-17 01:06:45]

Patchset #5 (id:120001) has been deleted

[Zhi Huang, 2017-06-17 01:07:08]

Patchset #5 (id:140001) has been deleted

[Zhi Huang, 2017-06-17 01:27:10]

Patchset #5 (id:160001) has been deleted

[Zhi Huang, 2017-06-17 01:41:49]

Patchset #5 (id:180001) has been deleted

[Zhi Huang, 2017-06-17 01:46:22]

Patchset #5 (id:200001) has been deleted

[Zhi Huang, 2017-06-17 01:46:44]

PTAL

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/BUILD.gn
File webrtc/sdk/android/BUILD.gn (right):

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/BUILD...
webrtc/sdk/android/BUILD.gn:25: rtc_static_library("base_jni") {
On 2017/06/16 22:19:08, Taylor Brandstetter wrote:
> May want to make these rtc_source_sets

Done.

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/BUILD...
webrtc/sdk/android/BUILD.gn:215: rtc_static_library("metrics_jni") {
On 2017/06/16 22:19:08, Taylor Brandstetter wrote:
> Do we need the two "metrics_jni" targets? It seems like we could just use the
> existing one, and change its dependencies from
> "libjingle_peerconnection/libjingle_peerconnection_jni" to
> "peerconnection/peerconnection_jni".

Done.

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/BUILD...
webrtc/sdk/android/BUILD.gn:273: # }
On 2017/06/16 22:19:08, Taylor Brandstetter wrote:
> May want to mention that this is still under development, and that some things
> (such as "null_audio_jni" with "video_jni") won't work.

I'll try to remove the limitation.

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/src/j...
File webrtc/sdk/android/src/jni/null_audio_jni.cc (right):

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/src/j...
webrtc/sdk/android/src/jni/null_audio_jni.cc:24: // support.
On 2017/06/16 22:19:08, Taylor Brandstetter wrote:
> It's odd that this has an implementation of a CreatePeerConnectionFactory
> method, which is normally implemented in a build target at a different level
> (webrtc/pc/create_pc_factory). This is also related to the "video/null_audio"
> combination not working. So it would be nice to have a comment summarizing the
> current state and describing what would need to be done to fix it.
> 
> But if it's not too much extra work, I think it would make sense to have:
> 
> - video_jni: Real implementation of CreateVideo[En|De]coderFactory
> - null_video_jni: "Null" implementation of CreateVideo[En|De]coderFactory
> - audio_jni: Real implementation of CreateAudio[En|De]coderFactory, similar to
> video_jni
> - null_audio_jni: "Null" implementation of CreateAudio[En|De]coderFactory
> - media_jni: Implementation of nativeCreatePeerConnectionFactory that creates
> media engine and call factory, and calls CreateModularPeerConnectionFactory.
> - null_media_jni: Implementation of nativeCreatePeerConnectionFactory that
> passes null pointers into CreateModularPeerConnectionFactory.
> 
> Then that limitation would be fixed, and we'd have a slightly cleaner version
of
> this CL. Though maybe there are some issues I haven't considered that would
make
> this difficult.

Instead of moving entire nativeCreatePeerConnectionFactory to media_jni.cc, I
made a helper function to create native peerconnectionfactory for two reasons:

1.It is simpler and less code duplication.

2. nativeCreatePeerConnectionFactory relies on some method and static global
variables in peerconnection_jni.cc. It requires some extra work to move
everything.

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/src/j...
File webrtc/sdk/android/src/jni/ownedfactoryandthreads.h (right):

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/src/j...
webrtc/sdk/android/src/jni/ownedfactoryandthreads.h:23: #include
"webrtc/base/thread_checker.h"
On 2017/06/16 22:19:08, Taylor Brandstetter wrote:
> Some of these headers and "using" statements aren't needed for this file.

Done.

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/src/j...
File webrtc/sdk/android/src/jni/peerconnection_jni.cc (right):

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/src/j...
webrtc/sdk/android/src/jni/peerconnection_jni.cc:123:
rtc::scoped_refptr<SurfaceTextureHelper> surface_texture_helper);
On 2017/06/16 22:19:09, Taylor Brandstetter wrote:
> It's odd to have these declarations in the .cc file. Should at least have a
> comment saying "the implementations are in either video_jni.cc or
> null_video_jni.cc". But it may make sense to put them in a "video_jni.h".

Done.

[Taylor Brandstetter, 2017-06-17 04:41:40]

lgtm. I think this is about as clean as we can get with this approach; good work

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/src/j...
File webrtc/sdk/android/src/jni/null_audio_jni.cc (right):

https://codereview.webrtc.org/2939203002/diff/100001/webrtc/sdk/android/src/j...
webrtc/sdk/android/src/jni/null_audio_jni.cc:24: // support.
On 2017/06/17 01:46:43, Zhi Huang wrote:
> On 2017/06/16 22:19:08, Taylor Brandstetter wrote:
> > It's odd that this has an implementation of a CreatePeerConnectionFactory
> > method, which is normally implemented in a build target at a different level
> > (webrtc/pc/create_pc_factory). This is also related to the
"video/null_audio"
> > combination not working. So it would be nice to have a comment summarizing
the
> > current state and describing what would need to be done to fix it.
> > 
> > But if it's not too much extra work, I think it would make sense to have:
> > 
> > - video_jni: Real implementation of CreateVideo[En|De]coderFactory
> > - null_video_jni: "Null" implementation of CreateVideo[En|De]coderFactory
> > - audio_jni: Real implementation of CreateAudio[En|De]coderFactory, similar
to
> > video_jni
> > - null_audio_jni: "Null" implementation of CreateAudio[En|De]coderFactory
> > - media_jni: Implementation of nativeCreatePeerConnectionFactory that
creates
> > media engine and call factory, and calls CreateModularPeerConnectionFactory.
> > - null_media_jni: Implementation of nativeCreatePeerConnectionFactory that
> > passes null pointers into CreateModularPeerConnectionFactory.
> > 
> > Then that limitation would be fixed, and we'd have a slightly cleaner
version
> of
> > this CL. Though maybe there are some issues I haven't considered that would
> make
> > this difficult.
> 
> Instead of moving entire nativeCreatePeerConnectionFactory to media_jni.cc, I
> made a helper function to create native peerconnectionfactory for two reasons:
> 
> 1.It is simpler and less code duplication.
> 
> 2. nativeCreatePeerConnectionFactory relies on some method and static global
> variables in peerconnection_jni.cc. It requires some extra work to move
> everything. 

That makes sense; it's still the same general idea, just an extra level of
indirection.

[Zhi Huang, 2017-06-17 05:08:11]

The CQ bit was checked by zhihuang@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-17 05:08:17]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[Zhi Huang, 2017-06-17 05:09:39]

The CQ bit was unchecked by zhihuang@webrtc.org

[Zhi Huang, 2017-06-17 05:11:02]

Description was changed from

==========
Support building WebRTC without audio and video for Android

This CL makes the WebRTC Java Wrapper more modular and allows the android
users to build WebRTC without audio and video(DataChannel only).

The BUILD file in sdk/android/ is modified to support modular WebRTC.

The peerconnection_jni.cc is split into peerconnection_jni.cc, video_jni.cc,
video_renderer_jni.cc and ownedfactoryandthreads.h/cc.

Add new modular build targets to JNI layer: audio_jni, video_jni,
null_audio_jni, null_video_jni. The users can link with different 
targets to for different WebRTC functionalities.

This is split from CL: https://codereview.webrtc.org/2854123003/

BUG=webrtc:7613
==========

to

==========
Support building WebRTC without audio and video for Android

This CL makes the WebRTC Java Wrapper more modular and allows the android
users to build WebRTC without audio and video(DataChannel only).

The BUILD file in sdk/android/ is modified to support modular WebRTC.

The peerconnection_jni.cc is split into peerconnection_jni.cc, video_jni.cc,
video_renderer_jni.cc and ownedfactoryandthreads.h/cc.

Add new modular build targets to JNI layer: audio_jni, video_jni,
null_audio_jni, null_video_jni. The users can link with different 
targets to for different WebRTC functionalities.

This is split from CL: https://codereview.webrtc.org/2854123003/

TBR=magjed@webrtc.org
BUG=webrtc:7613
==========

[Zhi Huang, 2017-06-18 05:10:31]

The CQ bit was checked by zhihuang@webrtc.org

[commit-bot: I haz the power, 2017-06-18 05:10:41]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-06-18 05:31:24]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1497762631348320,
"parent_rev": "d053fe4ab36a77db866fb813fec825a116590704", "commit_rev":
"af6293517fa80247f227fa80846d74ea5eef18c2"}

[commit-bot: I haz the power, 2017-06-18 05:31:28]

Description was changed from

==========
Support building WebRTC without audio and video for Android

This CL makes the WebRTC Java Wrapper more modular and allows the android
users to build WebRTC without audio and video(DataChannel only).

The BUILD file in sdk/android/ is modified to support modular WebRTC.

The peerconnection_jni.cc is split into peerconnection_jni.cc, video_jni.cc,
video_renderer_jni.cc and ownedfactoryandthreads.h/cc.

Add new modular build targets to JNI layer: audio_jni, video_jni,
null_audio_jni, null_video_jni. The users can link with different 
targets to for different WebRTC functionalities.

This is split from CL: https://codereview.webrtc.org/2854123003/

TBR=magjed@webrtc.org
BUG=webrtc:7613
==========

to

==========
Support building WebRTC without audio and video for Android

This CL makes the WebRTC Java Wrapper more modular and allows the android
users to build WebRTC without audio and video(DataChannel only).

The BUILD file in sdk/android/ is modified to support modular WebRTC.

The peerconnection_jni.cc is split into peerconnection_jni.cc, video_jni.cc,
video_renderer_jni.cc and ownedfactoryandthreads.h/cc.

Add new modular build targets to JNI layer: audio_jni, video_jni,
null_audio_jni, null_video_jni. The users can link with different 
targets to for different WebRTC functionalities.

This is split from CL: https://codereview.webrtc.org/2854123003/

TBR=magjed@webrtc.org
BUG=webrtc:7613

Review-Url: https://codereview.webrtc.org/2939203002
Cr-Commit-Position: refs/heads/master@{#18647}
Committed:
https://chromium.googlesource.com/external/webrtc/+/af6293517fa80247f227fa808...
==========

[commit-bot: I haz the power, 2017-06-18 05:31:29]

Committed patchset #5 (id:220001) as
https://chromium.googlesource.com/external/webrtc/+/af6293517fa80247f227fa808...

[magjed_webrtc, 2017-06-19 07:56:04]

Description was changed from

==========
Support building WebRTC without audio and video for Android

This CL makes the WebRTC Java Wrapper more modular and allows the android
users to build WebRTC without audio and video(DataChannel only).

The BUILD file in sdk/android/ is modified to support modular WebRTC.

The peerconnection_jni.cc is split into peerconnection_jni.cc, video_jni.cc,
video_renderer_jni.cc and ownedfactoryandthreads.h/cc.

Add new modular build targets to JNI layer: audio_jni, video_jni,
null_audio_jni, null_video_jni. The users can link with different 
targets to for different WebRTC functionalities.

This is split from CL: https://codereview.webrtc.org/2854123003/

TBR=magjed@webrtc.org
BUG=webrtc:7613

Review-Url: https://codereview.webrtc.org/2939203002
Cr-Commit-Position: refs/heads/master@{#18647}
Committed:
https://chromium.googlesource.com/external/webrtc/+/af6293517fa80247f227fa808...
==========

to

==========
Support building WebRTC without audio and video for Android

This CL makes the WebRTC Java Wrapper more modular and allows the android
users to build WebRTC without audio and video(DataChannel only).

The BUILD file in sdk/android/ is modified to support modular WebRTC.

The peerconnection_jni.cc is split into peerconnection_jni.cc, video_jni.cc,
video_renderer_jni.cc and ownedfactoryandthreads.h/cc.

Add new modular build targets to JNI layer: audio_jni, video_jni,
null_audio_jni, null_video_jni. The users can link with different 
targets to for different WebRTC functionalities.

This is split from CL: https://codereview.webrtc.org/2854123003/

TBR=magjed@webrtc.org
BUG=webrtc:7613

Review-Url: https://codereview.webrtc.org/2939203002
Cr-Commit-Position: refs/heads/master@{#18647}
Committed:
https://chromium.googlesource.com/external/webrtc/+/af6293517fa80247f227fa808...
==========

[magjed_webrtc, 2017-06-19 07:56:04]

magjed@webrtc.org changed reviewers:
+ magjed@webrtc.org, sakal@webrtc.org

[magjed_webrtc, 2017-06-19 16:48:16]

On 2017/06/18 05:31:29, commit-bot: I haz the power wrote:
> Committed patchset #5 (id:220001) as
>
https://chromium.googlesource.com/external/webrtc/+/af6293517fa80247f227fa808...

I had some comments on this CL, but I decided it was faster and easier to just
fix it myself. See https://chromium-review.googlesource.com/c/539456/ if you are
interested in what I changed.