Add support for unmixed audio from remote WebRTC remote tracks.

Add support for unmixed audio from remote WebRTC remote tracks.
To begin with WebAudio will have support for this with more areas to come (e.g. MediaStreamRecording)

BUG=121673
Committed: https://crrev.com/30b55651cf9500b1ed8917e4d040513559ad3e83
Cr-Commit-Position: refs/heads/master@{#364885}

[tommi (sloooow) - chröme, 2015-12-11 09:31:37]

tommi@chromium.org changed reviewers:
+ jochen@chromium.org, perkj@chromium.org

[tommi (sloooow) - chröme, 2015-12-11 09:31:38]

Per: Main reviewer.
Jochen: 	content/public/renderer/media_stream_audio_sink.cc (owner)

[tommi (sloooow) - chröme, 2015-12-11 09:58:40]

Rebase

[tommi (sloooow) - chröme, 2015-12-11 10:00:34]

The CQ bit was checked by tommi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-11 10:01:23]

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

[commit-bot: I haz the power, 2015-12-11 10:45:54]

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

[commit-bot: I haz the power, 2015-12-11 10:45:55]

Dry run: Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[perkj_chrome, 2015-12-11 12:31:38]

Looks like a very good step in the correct direction.

I think you might want to switch reviewer from jochen to guido or grunell.

https://codereview.chromium.org/1514143003/diff/20001/content/public/renderer...
File content/public/renderer/media_stream_audio_sink.cc (right):

https://codereview.chromium.org/1514143003/diff/20001/content/public/renderer...
content/public/renderer/media_stream_audio_sink.cc:9: #include
"content/renderer/media/remote_media_stream_impl.h"
why this include? Shouldn't be needed.

https://codereview.chromium.org/1514143003/diff/20001/content/public/renderer...
content/public/renderer/media_stream_audio_sink.cc:10: #include
"content/renderer/media/webrtc_local_audio_track.h"
and remove this webrtc_local_audio_track.h"?

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
File content/renderer/media/media_stream_center.cc (right):

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
content/renderer/media/media_stream_center.cc:130: return nullptr;
Do we want to log this ?

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
File content/renderer/media/media_stream_track.h (right):

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
content/renderer/media/media_stream_track.h:49: class CONTENT_EXPORT
MediaStreamAudioTrack : public MediaStreamTrack {
please move to separate file. See MediaStreamVideoTrack

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
File content/renderer/media/media_stream_video_track.cc (left):

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
content/renderer/media/media_stream_video_track.cc:13: #include
"third_party/libjingle/source/talk/app/webrtc/mediastreaminterface.h"
thanks

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
File content/renderer/media/remote_media_stream_impl.cc (right):

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
content/renderer/media/remote_media_stream_impl.cc:234: // TODO(tommi): Set the
remote audio track extra data.
audio source

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
File content/renderer/media/webrtc/media_stream_remote_audio_track.h (right):

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
content/renderer/media/webrtc/media_stream_remote_audio_track.h:15: class
MediaStreamRemoteAudioTrack : public MediaStreamAudioTrack {
Do you in the end want to distinguish between local and remote tracks in chrome
content or do as video- have one track implementation and multiple source
implementations? 

Depending on the answer- add a todo to explain the future plan.

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
File content/renderer/media/webrtc/webrtc_media_stream_adapter.cc (right):

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
content/renderer/media/webrtc/webrtc_media_stream_adapter.cc:96:
audio_source->GetAudioCapturer()->EnablePeerConnectionMode();
Fyi, here is interesting code... This is were I would like to create adapter
PeerConnection needs and do whatever EnablePeerConnectionMode() is doing. 

Ie,  get rid of the native_track->GetAudioAdaptuer all together and create a
sink to the audio track that adapts to the webrtc interface. The other way
around from what you just do in the MediaStreamRemoteAudioTrack.

Anyway, not related to this cl.

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
File content/renderer/media/webrtc_local_audio_track.h (right):

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
content/renderer/media/webrtc_local_audio_track.h:39: class CONTENT_EXPORT
WebRtcLocalAudioTrack
todo rename to match your MediaStreamRemoteAudioTrack? (again depending on if
you want to separate local or remote tracks in the end or not.

[tommi (sloooow) - chröme, 2015-12-11 15:36:51]

https://codereview.chromium.org/1514143003/diff/20001/content/public/renderer...
File content/public/renderer/media_stream_audio_sink.cc (right):

https://codereview.chromium.org/1514143003/diff/20001/content/public/renderer...
content/public/renderer/media_stream_audio_sink.cc:9: #include
"content/renderer/media/remote_media_stream_impl.h"
On 2015/12/11 12:31:38, perkj wrote:
> why this include? Shouldn't be needed.

Ah, thanks. Removed.  Was probably there from some earlier version of the code.

https://codereview.chromium.org/1514143003/diff/20001/content/public/renderer...
content/public/renderer/media_stream_audio_sink.cc:10: #include
"content/renderer/media/webrtc_local_audio_track.h"
On 2015/12/11 12:31:38, perkj wrote:
> and remove this webrtc_local_audio_track.h"? 

Done.

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
File content/renderer/media/media_stream_center.cc (right):

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
content/renderer/media/media_stream_center.cc:130: return nullptr;
On 2015/12/11 12:31:38, perkj wrote:
> Do we want to log this ? 

Added a DLOG(ERROR)

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
File content/renderer/media/media_stream_track.h (right):

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
content/renderer/media/media_stream_track.h:49: class CONTENT_EXPORT
MediaStreamAudioTrack : public MediaStreamTrack {
On 2015/12/11 12:31:38, perkj wrote:
> please move to separate file. See MediaStreamVideoTrack

Done.

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
File content/renderer/media/remote_media_stream_impl.cc (right):

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
content/renderer/media/remote_media_stream_impl.cc:234: // TODO(tommi): Set the
remote audio track extra data.
On 2015/12/11 12:31:38, perkj wrote:
> audio source

Done.

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
File content/renderer/media/webrtc/media_stream_remote_audio_track.h (right):

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
content/renderer/media/webrtc/media_stream_remote_audio_track.h:15: class
MediaStreamRemoteAudioTrack : public MediaStreamAudioTrack {
On 2015/12/11 12:31:38, perkj wrote:
> Do you in the end want to distinguish between local and remote tracks in
chrome
> content or do as video- have one track implementation and multiple source
> implementations? 
> 
> Depending on the answer- add a todo to explain the future plan. 

In the end I'd like to have only one.  The video implementation makes sense to
me so I think I'll go for something similar.
This change, does make the local and remote audio tracks more similar and the
"local" source provider now works for both types.

Added a TODO.

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
File content/renderer/media/webrtc/webrtc_media_stream_adapter.cc (right):

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
content/renderer/media/webrtc/webrtc_media_stream_adapter.cc:96:
audio_source->GetAudioCapturer()->EnablePeerConnectionMode();
On 2015/12/11 12:31:38, perkj wrote:
> Fyi, here is interesting code... This is were I would like to create adapter
> PeerConnection needs and do whatever EnablePeerConnectionMode() is doing. 
> 
> Ie,  get rid of the native_track->GetAudioAdaptuer all together and create a
> sink to the audio track that adapts to the webrtc interface. The other way
> around from what you just do in the MediaStreamRemoteAudioTrack.
> 
> Anyway, not related to this cl.

Ack.  I need to wrap my head around what's going on inside
EnablePeerConnectionMode() and why it's needed.  At the moment there are big
differences for the default renderers in how local and remote audio is rendered
(e.g. because of WebRTC's mixer), so we have to continue to have some specific
handling.

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
File content/renderer/media/webrtc_local_audio_track.h (right):

https://codereview.chromium.org/1514143003/diff/20001/content/renderer/media/...
content/renderer/media/webrtc_local_audio_track.h:39: class CONTENT_EXPORT
WebRtcLocalAudioTrack
On 2015/12/11 12:31:38, perkj wrote:
> todo rename to match your MediaStreamRemoteAudioTrack? (again depending on if
> you want to separate local or remote tracks in the end or not.

Done. (added TODO since the full separation is a little further out)

[tommi (sloooow) - chröme, 2015-12-11 15:39:32]

Address comments

[tommi (sloooow) - chröme, 2015-12-11 15:43:37]

tommi@chromium.org changed reviewers:
+ jam@chromium.org
- jochen@chromium.org

[tommi (sloooow) - chröme, 2015-12-11 15:43:37]

Switching owner review to jam.
John, could you review the first three files in the cl?
content/content_renderer.gypi
content/renderer/BUILD.gn
content/public/renderer/media_stream_audio_sink.cc

[perkj_chrome, 2015-12-11 15:51:14]

lgtm

[perkj_chrome, 2015-12-11 15:54:41]

https://codereview.chromium.org/1514143003/diff/40001/content/renderer/media/...
File content/renderer/media/webrtc_local_audio_track.h (right):

https://codereview.chromium.org/1514143003/diff/40001/content/renderer/media/...
content/renderer/media/webrtc_local_audio_track.h:15: #include
"content/renderer/media/media_stream_track.h"
MediaStreamAudioTrack

[tommi (sloooow) - chröme, 2015-12-11 16:03:01]

https://codereview.chromium.org/1514143003/diff/40001/content/renderer/media/...
File content/renderer/media/webrtc_local_audio_track.h (right):

https://codereview.chromium.org/1514143003/diff/40001/content/renderer/media/...
content/renderer/media/webrtc_local_audio_track.h:15: #include
"content/renderer/media/media_stream_track.h"
On 2015/12/11 15:54:41, perkj wrote:
> MediaStreamAudioTrack

yup (doh!)

[tommi (sloooow) - chröme, 2015-12-11 16:03:58]

Fix include

[tommi (sloooow) - chröme, 2015-12-11 16:46:01]

Fix other include

[tommi (sloooow) - chröme, 2015-12-11 16:49:27]

The CQ bit was checked by tommi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-11 16:50:13]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1514143003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1514143003/80001

[commit-bot: I haz the power, 2015-12-11 18:28:19]

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

[commit-bot: I haz the power, 2015-12-11 18:28:20]

Dry run: This issue passed the CQ dry run.

[jam, 2015-12-11 22:01:01]

lgtm

[tommi (sloooow) - chröme, 2015-12-12 02:00:27]

The CQ bit was checked by tommi@chromium.org

[tommi (sloooow) - chröme, 2015-12-12 02:00:27]

The patchset sent to the CQ was uploaded after l-g-t-m from perkj@chromium.org
Link to the patchset: https://codereview.chromium.org/1514143003/#ps80001
(title: "Fix other include")

[commit-bot: I haz the power, 2015-12-12 02:05:25]

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

[commit-bot: I haz the power, 2015-12-12 03:44:00]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-12-12 03:45:11]

Description was changed from

==========
Add support for unmixed audio from remote WebRTC remote tracks.
To begin with WebAudio will have support for this with more areas to come (e.g.
MediaStreamRecording)

BUG=121673
==========

to

==========
Add support for unmixed audio from remote WebRTC remote tracks.
To begin with WebAudio will have support for this with more areas to come (e.g.
MediaStreamRecording)

BUG=121673

Committed: https://crrev.com/30b55651cf9500b1ed8917e4d040513559ad3e83
Cr-Commit-Position: refs/heads/master@{#364885}
==========

[commit-bot: I haz the power, 2015-12-12 03:45:12]

Patchset 5 (id:??) landed as
https://crrev.com/30b55651cf9500b1ed8917e4d040513559ad3e83
Cr-Commit-Position: refs/heads/master@{#364885}