Add chromium support for MediaStreamAudioDestinationNode

Add chromium support for MediaStreamAudioDestinationNode

We add smarts into MediaStreamDependencyFactory::CreateNativeLocalMediaStream()
to handle MediaStreams originating from WebAudio.

Please see companion WebKit patches:
https://bugs.webkit.org/show_bug.cgi?id=101815
https://bugs.webkit.org/show_bug.cgi?id=106053

BUG=none
TEST=manual test
http://www.corp.google.com/~henrika/WebAudio/MediaStreamAudioDestinationNode.html

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=177330

[Chris Rogers, 2012-11-10 01:14:46]

We have sound!

Early test indicates we can transmit from a WebAudio node to a PeerConnection.

[henrika (OOO until Aug 14), 2012-11-12 09:24:55]

Great work!

Chis, would it be possible to provide some more details so I can try it out as
well? I am not sure how to apply the WebKit patch and it would be great if you
could distribute the modified demo as well so we could test it.

[tommi (sloooow) - chröme, 2012-11-12 09:46:18]

Great!  How far away are we from landing?

https://codereview.chromium.org/11369171/diff/1/content/renderer/media/rtc_pe...
File content/renderer/media/rtc_peer_connection_handler.cc (right):

https://codereview.chromium.org/11369171/diff/1/content/renderer/media/rtc_pe...
content/renderer/media/rtc_peer_connection_handler.cc:322:
webaudio_capturer_source_ = new WebAudioCapturerSource();
Should we first DCHECK that webaudioÖcapturer_source_ is NULL?

https://codereview.chromium.org/11369171/diff/1/content/renderer/media/rtc_pe...
content/renderer/media/rtc_peer_connection_handler.cc:324:
dependency_factory_->GetWebRtcAudioDevice()->capturer();
no chance that GetWebRtcAudioDevice() returns NULL or would that be a serious
error?

https://codereview.chromium.org/11369171/diff/1/content/renderer/media/rtc_pe...
content/renderer/media/rtc_peer_connection_handler.cc:348: if
(webaudio_capturer_source_.get())
nit: scoped_refptr supports checking validity without .get() and that seems to
be the norm now.

https://codereview.chromium.org/11369171/diff/1/content/renderer/media/rtc_pe...
File content/renderer/media/rtc_peer_connection_handler.h (right):

https://codereview.chromium.org/11369171/diff/1/content/renderer/media/rtc_pe...
content/renderer/media/rtc_peer_connection_handler.h:82: virtual void
consumeAudio(const WebKit::WebVector<const float*>&,
missing param name (I know webkit is different)

https://codereview.chromium.org/11369171/diff/1/content/renderer/media/webaud...
File content/renderer/media/webaudio_capturer_source.cc (right):

https://codereview.chromium.org/11369171/diff/1/content/renderer/media/webaud...
content/renderer/media/webaudio_capturer_source.cc:62: // Wrap.
comment doesn't add much :)

https://codereview.chromium.org/11369171/diff/1/content/renderer/media/webaud...
content/renderer/media/webaudio_capturer_source.cc:64: for (unsigned i = 0; i <
audio_data.size(); ++i)
s/unsigned/size_t or int?

[henrika (OOO until Aug 14), 2012-11-12 10:23:54]

Chris, 

sorry if I don't know the details here.

How is this work related to the existing live-audio I/O work? Do you call
createMediaStreamSource(stream) in your example, and if so, does that not
require the unified audio (I/O) audio backend?

[perkj_chrome, 2012-11-12 10:35:11]

http://codereview.chromium.org/11369171/diff/1/content/renderer/media/rtc_pee...
File content/renderer/media/rtc_peer_connection_handler.cc (right):

http://codereview.chromium.org/11369171/diff/1/content/renderer/media/rtc_pee...
content/renderer/media/rtc_peer_connection_handler.cc:315: if
(audioComponents.size() > 0) {
This seems to be in the wrong place and should be done when the underlaying
source is constructed in the media_stream_dependency_factory one way or the
other. Maybe via the MediaStreamCenter.

How does this work conceptually?
1. GetUserMedia creates a Ms with tracks where the source is a camera and a mic.


2. What is this doing? Are you replacing the mic source in the the ms from 1
with WebAudio or is this a new ms or is it a new audio track with a new source
you have added to 1? 

If you are replacing the the source in an existing audio track you should be
able to do that through the MediaStreamCenter and the
MediaStreamDependencyFactory.

If the output from WebAudio is a new MediaStream with a new audio source (the
output from the webaudio filter) - the MediaStream should be constructed through
MediaStreamCenter. Please see the existing constructor in MediaStreamCenter.

[henrika (OOO until Aug 14), 2012-11-12 17:00:30]

To add some more flesh to my previous question. What if we now do 
[modified_media_stream].connect(context.destination)? Will that trigger a mix of
audio rendering where one goes trough PeerConnection and the other through Web
Audio rendering?

Just trying to learn here.

[Chris Rogers, 2012-11-12 23:18:15]

On 2012/11/12 09:24:55, henrika wrote:
> Great work!
> 
> Chis, would it be possible to provide some more details so I can try it out as
> well? I am not sure how to apply the WebKit patch and it would be great if you
> could distribute the modified demo as well so we could test it.

Henrik, here's a link to the modified demo:
http://chromium.googlecode.com/svn/trunk/samples/audio/peerconnection.html

It's a bit half-baked right now since the peerConnection.addStream(...) call
needs to be made later after other calls are made.  Plus it relies on a previous
call to addStream for the microphone's localStream itself, which should not be
necessary. Hopefully, you guys will be able to help me refine this.

So when running it, you have to do this sequence:
1) Click on Start
2) agree to use "microphone" --- this part can be removed in this test later
on...
3) Click on Call
4) Click on "Web Audio" (which actually calls peerConnection.addStream(...))

WARNING: this will not yet work when "Web Audio Input" is enabled, so must
disable to test this.

To get any WebKit patch, simply click on the "Diff" link for the patch, then
click on the "Raw Unified" link in the upper left corner.  You can then copy the
patch text and use the normal UNIX "patch" tool to apply from the WebKit
directory (in "third_party").  For example, in this case, the link is:
https://bugs.webkit.org/attachment.cgi?id=173403&action=diff&context=patch&co...

I've noticed that Shijing removed the WebRtcAudioDeviceImpl::capturer() method,
so I needed to add it back in so it will compile with this CL.

[Chris Rogers, 2012-11-12 23:23:06]

On 2012/11/12 10:35:11, perkj wrote:
>
http://codereview.chromium.org/11369171/diff/1/content/renderer/media/rtc_pee...
> File content/renderer/media/rtc_peer_connection_handler.cc (right):
> 
>
http://codereview.chromium.org/11369171/diff/1/content/renderer/media/rtc_pee...
> content/renderer/media/rtc_peer_connection_handler.cc:315: if
> (audioComponents.size() > 0) {
> This seems to be in the wrong place and should be done when the underlaying
> source is constructed in the media_stream_dependency_factory one way or the
> other. Maybe via the MediaStreamCenter.
> 
> How does this work conceptually?
> 1. GetUserMedia creates a Ms with tracks where the source is a camera and a
mic.
> 

Actually, this should be able to work without *any* call to getUserMedia(),
since the WebAudio node creates
its own MediaStream.  WebAudio is able to generate an audio stream on its own.

> 
> 2. What is this doing? Are you replacing the mic source in the the ms from 1
> with WebAudio or is this a new ms or is it a new audio track with a new source
> you have added to 1? 

It's a new MediaStream created by the MediaStreamAudioDestinationNode.  You can
see the exact details in the companion WebKit patch.

> 
> If you are replacing the the source in an existing audio track you should be
> able to do that through the MediaStreamCenter and the
> MediaStreamDependencyFactory.
> 
> If the output from WebAudio is a new MediaStream with a new audio source (the
> output from the webaudio filter) - the MediaStream should be constructed
through
> MediaStreamCenter. Please see the existing constructor in MediaStreamCenter.

I took a look at MediaStreamCenter in WebCore/platform/mediastream, but it's not
clear to me what you're proposing.  I suggest we talk offline, since this is
likely to be a complex discussion.

[henrika (OOO until Aug 14), 2012-11-13 14:14:57]

Thanks for the links Chris. Is the CL patched now so it matches what Shijing
just landed?

I'll pause any further comments until you have sorted out the "big picture" with
Per and Tommy.

[Chris Rogers, 2012-12-14 21:43:24]

This latest CL is an iteration based on TommiW's improved WebKit-side
architecture using WebAudioDestinationConsumer:
http://trac.webkit.org/changeset/135985

perkj, xians1, henrika: Please have another look.  You should be able to apply
the patch and try the demo, as long as you *strictly* follow the sequence
(Start, Call, "approve getUserMedia", WebAudio).  The problem is that the WebRTC
engine appears to REQUIRE having the MediaStream from getUserMedia() attached to
the peer for this to work at all.  There is some engine setup which I don't
fully understand, and the WebRtcAudioDeviceImpl doesn't appear to be running
otherwise.  Please help!  Obviously, we would prefer that this demo work without
any use of getUserMedia() at all...

perkj, tommyw: A second problem is that in WebKit,
createMediaStreamDestination() (implemented in
MediaStreamAudioDestinationNode::create()) causes this call:
    MediaStreamCenter::instance().didCreateMediaStream(m_stream->descriptor());

To happen, but we fail right away in
MediaStreamDependencyFactory::CreateNativeLocalMediaStream() on the first
DCHECK:
  DCHECK(PeerConnectionFactoryCreated());

So tommyw, what lazy initialization call do we need to make in WebKit so this
DCHECK doesn't happen?

Thanks for everybody's help and attention!

[Chris Rogers, 2012-12-14 21:43:58]

+tommyw

[henrika (OOO until Aug 14), 2012-12-15 16:32:49]

Chris, I have not checked this CL in a while and might have missed some details
but was not the original intention:

getUserMedia() -> local MediaStream -> WebAudio -> Peerconnection?

It is not clear to me why we now discuss in terms of avoiding getUserMedia? Or
should both alternatives should be possible?

[henrika (OOO until Aug 14), 2012-12-15 16:43:45]

I think this CL must be rebased to match top of the tree.

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/web...
File content/renderer/media/webrtc_audio_device_impl.h (right):

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/web...
content/renderer/media/webrtc_audio_device_impl.h:398: WebRtcAudioCapturer*
capturer() { return capturer_; }
I think you must rebase since we already have this part in latest. See
http://code.google.com/searchframe#OAMlx_jo-ck/src/content/renderer/media/web...

[Chris Rogers, 2012-12-15 19:45:23]

On 2012/12/15 16:32:49, henrika wrote:
> Chris, I have not checked this CL in a while and might have missed some
details
> but was not the original intention:
> 
> getUserMedia() -> local MediaStream -> WebAudio -> Peerconnection?
> 
> It is not clear to me why we now discuss in terms of avoiding getUserMedia? Or
> should both alternatives should be possible?

The idea is that *any* audio being generated from WebAudio should be able to be
sent to a PeerConnection.  This *could* be from live audio input with
processing, but in my simple example, I'm just playing a pre-recorded sound of a
woman speaking.  Since we're adding the MediaStream to the peer with
addStream(), it should "just work".  It shouldn't matter if the MediaStream is
from getUserMedia(), or from another source (WebAudio in this case).

[henrika (OOO until Aug 14), 2012-12-17 08:37:16]

Hence, #3 here: 

https://dvcs.w3.org/hg/audio/raw-file/tip/webaudio/webrtc-integration.html

is not covered by this CL yet. Correct?

[perkj_chrome, 2012-12-17 10:59:20]

Awsome - I think this is the right approach. I can't review the audio specifics
but I looked at the integration with MediaStreams.

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/med...
File content/renderer/media/media_stream_dependency_factory.cc (right):

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/med...
content/renderer/media/media_stream_dependency_factory.cc:231:
DCHECK(PeerConnectionFactoryCreated());
Change this to  if (!EnsurePeerConnectionFactory())
log and return; 
That should solve your problem of having to call getusermedia/ or creating a
peerconnection before creating the WebAudio MS. Prior to this cl -
EnsurePeerConnectionFactory was triggered by getusermedia or when a
peerconnection was created.

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/med...
content/renderer/media/media_stream_dependency_factory.cc:253:
webaudio_capturer_source_ = new WebAudioCapturerSource();
Why is webauido_capturer_source_ stored at all in MediaStreamDependencyFactory?
If it is never used again just loos this reference. If you need it again I
suggest you create  a MediaStreamSourceExtraData and store it in there. That way
Webkit controls the lifetime and it can be fetched everywhere a MediaStream is
used.

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/med...
content/renderer/media/media_stream_dependency_factory.cc:261:
source.addAudioConsumer(webaudio_capturer_source_.get());
Does addAudioConsumer ref count webaudio_capturer_source_ so it is guaranteed to
live even if a user creates another WebAudio MediaStream.

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/med...
content/renderer/media/media_stream_dependency_factory.cc:265: // MediaStreams
to a single peer, mixing their results.
Multiple MediaStreams will result in multiple calls to
MediaStreamDependencyFactory::CreateNativeLocalMediaStream. You can end up here
if a JS creates a new MediaStream based on another MediaStream. Ex creating a
copy of a MediaStream.

[perkj_chrome, 2012-12-17 15:38:23]

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/med...
File content/renderer/media/media_stream_dependency_factory.cc (right):

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/med...
content/renderer/media/media_stream_dependency_factory.cc:284:
scoped_refptr<webrtc::LocalAudioTrackInterface> audio_track(
line 284 to 287 must be done even for webaudio sources. The native
repressentation of the track must be created and added to the native stream.

[henrika (OOO until Aug 14), 2012-12-17 16:04:01]

Chris,

I have applied the patch on Windows and also did the changes that Per proposed
but failed to get any audio playing. I copied the JS file to a local disc and
also the mp4 file but get Uncaught Error: SyntaxError: DOM Exception 12 on
source.buffer = context.createBuffer(request.response, false) which is bad
start.

I also modified the script (after discussions with Per) and removed all parts
related to the local media stream:

All is in Call now:

function call() {
  btn2.disabled = true;
  btn3.disabled = false;
  trace("Starting call");
  var servers = null;
  pc1 = new webkitRTCPeerConnection(servers);
  trace("Created local peer connection object pc1");
  pc1.onicecandidate = iceCallback1; 
  pc2 = new webkitRTCPeerConnection(servers);
  trace("Created remote peer connection object pc2");
  pc2.onicecandidate = iceCallback2;
  pc2.onaddstream = gotRemoteStream;
  
  trace("mediaStreamDestination = context.createMediaStreamDestination();");
  mediaStreamDestination = context.createMediaStreamDestination();
  
  trace("pc1.addStream(mediaStreamDestination.stream)");
  pc1.addStream(mediaStreamDestination.stream);
    
  trace("source.connect(mediaStreamDestination)");
  source.connect(mediaStreamDestination);
  
  trace("source.start(0)");
  source.start(0);
 
  pc1.createOffer(gotDescription1);
}

I then expect:

unction gotRemoteStream(e){
  vid2.src = webkitURL.createObjectURL(e.stream);
  trace("Received remote stream");
}

which should use the file as source and render the remote stream from PC.

BUT

I get no call to gotRemoteStream().

One more input, I initially tried the complete script (with invalid use of
getUserMedia) and then it fails in LOG(ERROR) <<
"WebAudioCapturerSource::Consume() : Channel mismatch.";

The reason is that the reference channel layout is taken from my mic
(2-channels) but the audio_data source is mono. Guess some more work is needed
here to compensate for mismatch.

[henrika (OOO until Aug 14), 2012-12-17 16:13:08]

One more comment. When you rebase and try latest you will hit warnings in
WebRtcAudioCapturer if you try to attach a local media stream to a video tag (a
local renderer will ask for data but the capturer does not provide it). I am not
sure if it is a bug since we should avoid it in the script but I'll see if I can
improve that area anyhow.

[henrika (OOO until Aug 14), 2012-12-17 16:14:41]

I assume that you will:

- Rebase
- Do changes as Per proposes.
- Clean up the script and remove all parts related to getUserMedia.
- Perhaps try on Windows as well to see if there are differences.

[no longer working on chromium, 2012-12-17 22:17:58]

Hi Chris,

I am afraid I can't help much for this CL since I will be on vocation after
Tomorrow. Hope it is fine to defer my part to Henrik and Per.

BR,
SX

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/web...
File content/renderer/media/webaudio_capturer_source.cc (right):

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/web...
content/renderer/media/webaudio_capturer_source.cc:9: using
media::AudioParameters;
nit, move it under media::AudioFifo;

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/web...
content/renderer/media/webaudio_capturer_source.cc:18: : callback_(0),
NULL

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/web...
content/renderer/media/webaudio_capturer_source.cc:29: base::AutoLock
auto_lock(lock_);
When this Initialize() needs to be called? Is it called only once and before
another other method?

[henrika (OOO until Aug 14), 2012-12-18 12:23:02]

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/med...
File content/renderer/media/media_stream_dependency_factory.cc (right):

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/med...
content/renderer/media/media_stream_dependency_factory.cc:254:
WebRtcAudioCapturer* capturer = GetWebRtcAudioDevice()->capturer();
Note that this guy can return NULL for non-supported sample rates, e.g. 88200,
22050 etc. We must ensure that these cases does not crash.

[henrika (OOO until Aug 14), 2012-12-18 13:35:15]

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/med...
File content/renderer/media/media_stream_dependency_factory.cc (right):

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/med...
content/renderer/media/media_stream_dependency_factory.cc:284:
scoped_refptr<webrtc::LocalAudioTrackInterface> audio_track(
I tried this but it did not work. Per adviced me to try out:

scoped_refptr<webrtc::LocalAudioTrackInterface> audio_track(
            CreateLocalAudioTrack(label + "a0", NULL));

and that worked.

[henrika (OOO until Aug 14), 2012-12-18 13:41:09]

I am now able to do MediaStreamDestination -> PC -> VoE -> PC -> render in
looback on Windows.

It sounds very bad and I don't think that iSAC is the only bad guy.

This is what I had to do to make it work on ToT:

- Apply all changes above
- Select a Windows audio device which support mono input (*very* rare case on
Windows)
- Use WAV file instead of MP4 file as source (mp4 is not supported in
non-official Chrome)

Then I could make it work without any usage of getUserMedia or references to
additional local streams. Here is a simplified script that uses a <audio> tag
and has a Call button only:

<!DOCTYPE html>
<html>
<head>
<title>MediaStreamAudioDestinationNode Demo</title>
<style>
video {
  border:5px solid black;
  width:480px;
  height:360px;
}
button {
  font: 18px sans-serif;
  padding: 8px;
}
textarea {
  font-family: monospace;
  margin: 2px;
  width:480px;
  height:640px;
}
</style>
</head>
<body>
<audio id="aud" controls autoplay></audio>
<br>
<button id="btn2" onclick="call()">Call</button>
<button id="btn3" onclick="hangup()">Hang Up</button>
<br>
<xtextarea id="ta1"></textarea>
<xtextarea id="ta2"></textarea>
<script>

window.onload = init;

btn2.disabled = false;
btn3.disabled = true;
var pc1, pc2;
var context = 0;
var source = 0;
var mediaStreamDestination = 0;

function trace(text) {
  if (text[text.length - 1] == '\n') {
    text = text.substring(0, text.length - 1);
  }
}  
  
function call() {
  btn2.disabled = true;
  btn3.disabled = false;
  trace("Starting call");
  
  var servers = null;
  pc1 = new webkitRTCPeerConnection(servers);
  trace("Created local peer connection object pc1");
  pc1.onicecandidate = iceCallback1; 
  pc2 = new webkitRTCPeerConnection(servers);
  trace("Created remote peer connection object pc2");
  pc2.onicecandidate = iceCallback2;
  pc2.onaddstream = gotRemoteStream;
  
  trace("mediaStreamDestination = context.createMediaStreamDestination();");
  mediaStreamDestination = context.createMediaStreamDestination();
  
  trace("pc1.addStream(mediaStreamDestination.stream)");
  pc1.addStream(mediaStreamDestination.stream);
    
  trace("source.connect(mediaStreamDestination)");
  source.connect(mediaStreamDestination);
  
  trace("source.start(0)");
  source.start(0);

  pc1.createOffer(gotDescription1);
}

function gotDescription1(desc){
  pc1.setLocalDescription(desc);
  trace("Offer from pc1 \n" + desc.sdp);
  pc2.setRemoteDescription(desc);
  pc2.createAnswer(gotDescription2);
}

function gotDescription2(desc){
  pc2.setLocalDescription(desc);
  trace("Answer from pc2 \n" + desc.sdp);
  pc1.setRemoteDescription(desc);
}

function hangup() {
  trace("Ending call");
  pc1.close(); 
  pc2.close();
  pc1 = null;
  pc2 = null;
  btn3.disabled = true;
  btn2.disabled = false;
}

function gotRemoteStream(e){
  aud.src = webkitURL.createObjectURL(e.stream);
  trace("Received remote stream");
}

function iceCallback1(event){
  if (event.candidate) {
    pc2.addIceCandidate(new RTCIceCandidate(event.candidate));
    trace("Local ICE candidate: \n" + event.candidate.candidate);
  }
}
      
function iceCallback2(event){
  if (event.candidate) {
    pc1.addIceCandidate(new RTCIceCandidate(event.candidate));
    trace("Remote ICE candidate: \n " + event.candidate.candidate);
  }
}

function loadAudioBuffer(url) {
    var request = new XMLHttpRequest();
    request.open("GET", url, true);
    request.responseType = "arraybuffer";

    request.onload = function() { 
        source = context.createBufferSource();
        source.buffer = context.createBuffer(request.response, false);
    }

    request.send();
}

function init() {
    context = new webkitAudioContext();
    loadAudioBuffer("sounds/hyper-reality/human-voice.wav");
}

</script>
</body>
</html>

[henrika (OOO until Aug 14), 2012-12-18 13:43:49]

What's next?

- I will try to force usage of Opus.

Chris & Per: I hope you guys can sort out the last details and make this stable.
I think we are pretty close.

Chris: stereo input must be supported on Windows as well.

[perkj_chrome, 2012-12-18 14:09:04]

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/med...
File content/renderer/media/media_stream_dependency_factory.cc (right):

https://codereview.chromium.org/11369171/diff/9001/content/renderer/media/med...
content/renderer/media/media_stream_dependency_factory.cc:284:
scoped_refptr<webrtc::LocalAudioTrackInterface> audio_track(
On 2012/12/18 13:35:15, henrika wrote:
> I tried this but it did not work. Per adviced me to try out:
> 
> scoped_refptr<webrtc::LocalAudioTrackInterface> audio_track(
>             CreateLocalAudioTrack(label + "a0", NULL));
> 
> and that worked.

Yes- the tracks labels must follow the msid rfc (see dreft alvestrand 02) a0
stand for the first audio track in the stream with label |label|. It does not
make sence that chrome must know this, so feel free to add a todo with my name
to remove the label from track creation.

[henrika (OOO until Aug 14), 2012-12-18 14:09:53]

Forced Opus:

function transform(sdp) {
    // Remove all other codecs (not the video codecs though).
    sdp = sdp.replace(/m=audio (\d+) RTP\/SAVPF.*\r\n/g,
                      'm=audio $1 RTP/SAVPF 111\r\n');
    sdp = sdp.replace(/a=rtpmap:(?!111)\d{1,3} (?!VP8|red|ulpfec).*\r\n/g, '');
    return sdp;
}

function gotDescription1(desc){
  trace("Offer from pc1 \n" + desc.sdp);
  var modifiedOffer = new RTCSessionDescription( {type: 'offer',
                                                  sdp: transform(desc.sdp)});
  pc1.setLocalDescription(modifiedOffer);
  trace("Offer from pc1 \n" + modifiedOffer.sdp);
  pc2.setRemoteDescription(modifiedOffer);
  pc2.createAnswer(gotDescription2);

  // pc1.setLocalDescription(desc);
  // trace("Offer from pc1 \n" + desc.sdp);
  // pc2.setRemoteDescription(desc);
  // pc2.createAnswer(gotDescription2);
}

=> verify in console:

a=rtpmap:111 opus/48000/2

Still gives "robot audio". Can be that the driving source creates extreme
jitter. Also, guess we need stereo input here as well.

Pausing for now.

[Chris Rogers, 2012-12-19 02:23:57]

Hi Henrik, thanks for digging in a little bit.  I've also been wrestling with
this today.  I tried out your simplified script, but am not able to get it to
work with your Opus hack.  I've checked in a new and slightly improved version
of my original script which includes your Opus hack and it sounds great --- few
or no artifacts heard :)

http://chromium.googlecode.com/svn/trunk/samples/audio/peerconnection.html

Here are some points:
* I think the critical difference between my version (which still requires
getUserMedia()) and yours is I'm calling
"pc1.addStream(mediaStreamDestination.stream);"
AFTER "pc1.createOffer(gotDescription1);" instead of BEFORE as in your
simplified script. 

* I've found that I can't get your script to work with the Opus hack, probably
related to this ordering.

* I've added the ability to enable or disable both getUserMedia() and Opus
selection using the variables "useGetUserMedia" and "useOpus" variables.  The
basic result I'm seeing is that getUserMedia() is required.

* I found that the first time I enabled Opus and ran the demo, that it sounded
good!  But, that even after I removed the Opus selection code that it continued
(apparently) to use the Opus codec because the quality still sounded very high
to me!!!  There's something sticky / stateful / persistent about this codec
selection, and I don't understand anything about it.  It seems tweaky to me.

* In my new version, I've also added the ability to trigger a percussion sound
(snare drum) with the keyboard which mixes in along with the woman's voice.  It
gives a sense of the added latency - not that bad for local connection.

* For number-of-channels (mono vs. stereo), we need to have a discussion since
there are so many details revolving around this.  What determines the
number-of-channels of the stream we send to the PeerConnection? ---  In other
words, what is the number of channels that WebRtcAudioDeviceImpl::CaptureData()
is expecting to receive????  It would be nice if we could make sure that this
number is equal to the number of channels of the MediaStreamAudioDestinationNode
(which I could make configurable to 1, 2, ...) in the
createMediaStreamDestination() method.  I have found that on OSX that
WebRtcAudioDeviceImpl::CaptureData() is expecting mono (since it's so tied into
the microphone code :) --- thus I've hard-coded it to mono in WebKit.   We need
WebKit to be able to tell the PeerConnection what the number of channels is (in
the addStream() implementation) I think.

I'm not able to get any farther at this point because I understand little of the
details of the WebRTC engine code.  It would be nice to understand more about
how the order of method calls affects the outcome, the apparent statefulness of
the Opus selection, and the number-of-channels selection.

We'll get it working, but I really need your help :)  The good news is that the
Opus codec makes it sound really good (although still mono).  I'm not hearing
the artifacts you're hearing on Windows, not sure what the difference is...

[henrika (OOO until Aug 14), 2012-12-19 10:01:53]

Chris,

I have done a first test on Windows using your new script and unfortunately I
don't think it is correct. I can be wrong here (will double check on Mac) but I
think that you are using only the local renderer here and *not* any codec at all
and that's why you get good audio.

On Windows, I don't get any gotRemoteStream() callback and that's a must to be
able to state that your stream has taken the "PC path".

[henrika (OOO until Aug 14), 2012-12-19 10:21:38]

* I think the critical difference between my version (which still requires
getUserMedia()) and yours is I'm calling
"pc1.addStream(mediaStreamDestination.stream);"
AFTER "pc1.createOffer(gotDescription1);" instead of BEFORE as in your
simplified script. 

That *is* the difference, yes. But you *must* call pc1.addStream() BEFORE
pc1.createOffer() otherwise no stream is included in the offer we make and hence
no remote stream will be created and we are in the void.

Spec states: 

"The createOffer method generates a blob of SDP that contains a RFC
 3264 offer with the supported configurations for the session,
 including ==>> descriptions of the local MediaStreams attached to this
 PeerConnection <<== , the codec/RTP/RTCP options supported by ...."

[henrika (OOO until Aug 14), 2012-12-19 13:36:48]

Chris,

to further illustrate my point, I have sent you two files:

1) A modified media_stream_impl.cc in content\renderer\media
2) A new test page which plays the file using WebAudio Media Stream locally
*without using any PC at all*.

1+2 does the same thing as your last script version but it is more clear what
actually happens. What I do is to:

- create a local WebAudio media stream (I do NOT call getUserMedia())
- add a file source to the stream
- add that stream to an audio tag
- and render it locally
- you can also add the drum sound using the keyboard

The extra patch was needed to ensure that we don't try to use any microphone as
source.

IMHO, the correct way forward from here is to base the work on my previous demo
(adding the local stream *before* calling createOffer). That demo is the only
demo which will give you the correct (encoded) audio stream.

[henrika (OOO until Aug 14), 2012-12-20 15:09:24]

Chris,

I've done more work today and will upload the file for you (see e.mail).

Summary:
========

If I disabled the AEC in WebRTC, the audio is perfect for the
mediaStreamDestination + PC case where no getUserMedia is utilized.

Remaining issues:

- How to disable the AEC?
- Why is the AEC used at all?
- I have done a fix to support stereo input but we are still querying the capure
device and that is not correct.
- There is no stop() method on the mediaStreamDestination.stream. Chris: we need
this, right?

I have also written a JS where I (requires the web-audio-input flag):

- call getUserMedia
- feed the local stream to MediaStreamSource
- connects the source with an HP filter
- connects the filter to a MediaStreamDestination 
- attach MediaStreamDestination.stream to a PC
- send the filtered and encoded signal in loopback over PC
- renders the remote media stream on an audio tag

Remaining issues:

- AEC
- still behind a flag
- channel limitations

[henrika (OOO until Aug 14), 2012-12-21 12:01:44]

Work continued here:

https://codereview.chromium.org/11669004

[Chris Rogers, 2013-01-04 01:57:06]

Building on work from Henrik's last iteration:

WebAudioCapturerSource constructor now takes WebRtcAudioCapturer, so that it can
can control the call to SetCapturerSource(), giving it the channels and
sample-rate.

WebRtcAudioCapturer::SetCapturerSource() now takes channels and sample-rate
which it gets from WebKit

Please note that a WebKit patch implementing setFormat() must be landed (or
applied locally) for this to build:
https://bugs.webkit.org/show_bug.cgi?id=106053

[Chris Rogers, 2013-01-05 22:50:51]

FYI: setFormat() support has now landed in WebKit:
http://trac.webkit.org/changeset/138895

[henrika (OOO until Aug 14), 2013-01-07 10:14:19]

Chris,

just added a minor comment since I was not able to build today. Do you know an
approximate ETA for this WebKit patch in Chrome. If the delay is long I could
try to apply it manually.

perkj: You mentioned a comment in an off-line discussion today on how we perhaps
could improve this CL. Care to add some details?

[henrika (OOO until Aug 14), 2013-01-07 10:14:27]

https://codereview.chromium.org/11369171/diff/39001/content/renderer/media/we...
File content/renderer/media/webrtc_audio_capturer.cc (right):

https://codereview.chromium.org/11369171/diff/39001/content/renderer/media/we...
content/renderer/media/webrtc_audio_capturer.cc:140: 440); // requires knowledge
about WebRTC @ 10ms
Chris, what range of sample rates can we expect from WebAudio? WebRTC does not
support ant rate. As an example, a rate of 11025 or 22050 will fail.

[Chris Rogers, 2013-01-07 20:57:30]

On 2013/01/07 10:14:19, henrika wrote:
> Chris,
> 
> just added a minor comment since I was not able to build today. Do you know an
> approximate ETA for this WebKit patch in Chrome. If the delay is long I could
> try to apply it manually.
> 
> perkj: You mentioned a comment in an off-line discussion today on how we
perhaps
> could improve this CL. Care to add some details?

The patch is landed in WebKit already, but WebKit might not yet have been rolled
in DEPS.  It should be by the end of the day.

[Chris Rogers, 2013-01-07 20:58:48]

On 2013/01/07 10:14:27, henrika wrote:
>
https://codereview.chromium.org/11369171/diff/39001/content/renderer/media/we...
> File content/renderer/media/webrtc_audio_capturer.cc (right):
> 
>
https://codereview.chromium.org/11369171/diff/39001/content/renderer/media/we...
> content/renderer/media/webrtc_audio_capturer.cc:140: 440); // requires
knowledge
> about WebRTC @ 10ms
> Chris, what range of sample rates can we expect from WebAudio? WebRTC does not
> support ant rate. As an example, a rate of 11025 or 22050 will fail.

It should present a sample-rate value which is the hardware sample-rate, which
(I believe) is usually what WebRTC also will be using.

[henrika (OOO until Aug 14), 2013-01-08 09:52:16]

Thanks Chris,

it works for me now as well but I have some more comments.

If you did some more cleaning up as well, we might be close to something soon.

There is a separate issue at
https://code.google.com/p/webrtc/issues/detail?id=1040 for the AEC part and
hclam@ is working on it already which is great. I will assist as well.

We should also extend the JS demos for this issue and verify that we can do
start/stop. Today, I am unable to confirm that your media stream supports
stop(). If I reload my demo page, it does not work and can sometimes crash. When
the CL is stable, I'll try to come up with some additional tests. 

Also, I guess we need something that adds usage of getUserMedia and
mediaStreamSource as well; right? I mean, where we transmit a recorded and
filtered version of a captured stream. 

Example (source is no longer a file but a filtered version of the local media
stream):

function gotStream(stream) {
  if (stream.audioTracks.length == 1 && stream.videoTracks.length == 0) {
      
      var servers = null;
      pc1 = new webkitRTCPeerConnection(servers);
 
      pc1.onicecandidate = iceCallback1; 
      pc2 = new webkitRTCPeerConnection(servers);
 
      pc2.onicecandidate = iceCallback2;
      pc2.onaddstream = gotRemoteStream;
  
      mediaStreamSource = context.createMediaStreamSource(stream);
      
      filter = context.createBiquadFilter();
      filter.type = filter.HIGHPASS;
      filter.frequency.value = 4000;
            
      mediaStreamSource.connect(filter);
      
      mediaStreamDestination = context.createMediaStreamDestination();
      pc1.addStream(mediaStreamDestination.stream);
      filter.connect(mediaStreamDestination);

      pc1.createOffer(gotDescription1);
    }
    
    localStream = stream;
}

[henrika (OOO until Aug 14), 2013-01-08 09:52:27]

https://codereview.chromium.org/11369171/diff/39001/content/renderer/media/we...
File content/renderer/media/webrtc_audio_capturer.cc (right):

https://codereview.chromium.org/11369171/diff/39001/content/renderer/media/we...
content/renderer/media/webrtc_audio_capturer.cc:125: // henrika:
Guess we could clean up here now. Also, I did not spend any on the locking in my
first proposal. Guess it needs some attention as well.

https://codereview.chromium.org/11369171/diff/39001/content/renderer/media/we...
content/renderer/media/webrtc_audio_capturer.cc:136:
params_.Reset(params_.format(),
I had anticipated that the sample rate would be the rate of the (file) source,
i.e., 44.1 in our example. It now seems like you use the native hardware rate
and resample the source to this rate; correct?

I guess that works as well, but in any case, the hard-coded size of 440 must be
modified (se below). I have tried it using 44.1, 48 and 96 on my Win 7 machine
and it works well.


int buffer_size = GetBufferSizeForSampleRate(sample_rate);
if (!buffer_size) {
  DLOG(ERROR) << "Unsupported platform";
  return;
}

params_.Reset(params_.format(),
              channel_layout,
              sample_rate,
              16,  // this value is not really used
              buffer_size); <<<==========

[Chris Rogers, 2013-01-14 23:12:13]

Hi Henrik PTAL, I've tidied this up a bit and am ready for a more final review.

Now that we dispatch the stream format details via setFormat(), I've removed the
extra channel mixing code.

https://codereview.chromium.org/11369171/diff/39001/content/renderer/media/we...
File content/renderer/media/webrtc_audio_capturer.cc (right):

https://codereview.chromium.org/11369171/diff/39001/content/renderer/media/we...
content/renderer/media/webrtc_audio_capturer.cc:125: // henrika:
On 2013/01/08 09:52:27, henrika wrote:
> Guess we could clean up here now. Also, I did not spend any on the locking in
my
> first proposal. Guess it needs some attention as well.

Done.

https://codereview.chromium.org/11369171/diff/39001/content/renderer/media/we...
content/renderer/media/webrtc_audio_capturer.cc:136:
params_.Reset(params_.format(),
On 2013/01/08 09:52:27, henrika wrote:
> I had anticipated that the sample rate would be the rate of the (file) source,
> i.e., 44.1 in our example. It now seems like you use the native hardware rate
> and resample the source to this rate; correct?
> 
> I guess that works as well, but in any case, the hard-coded size of 440 must
be
> modified (se below). I have tried it using 44.1, 48 and 96 on my Win 7 machine
> and it works well.
> 
> 
> int buffer_size = GetBufferSizeForSampleRate(sample_rate);
> if (!buffer_size) {
>   DLOG(ERROR) << "Unsupported platform";
>   return;
> }
> 
> params_.Reset(params_.format(),
>               channel_layout,
>               sample_rate,
>               16,  // this value is not really used
>               buffer_size); <<<==========

The sample-rate will generally be the hardware sample-rate (what the WebAudio
AudioContext uses for all processing).  All <audio> tags and AudioBuffers are
internally already sample-rate converted to this hardware sample-rate in WebKit.

I've fixed the code as you suggest to account for the sample-rate when computing
|buffer_size|.

https://codereview.chromium.org/11369171/diff/39001/content/renderer/media/we...
content/renderer/media/webrtc_audio_capturer.cc:140: 440); // requires knowledge
about WebRTC @ 10ms
On 2013/01/07 10:14:28, henrika wrote:
> Chris, what range of sample rates can we expect from WebAudio? WebRTC does not
> support ant rate. As an example, a rate of 11025 or 22050 will fail.

It will be the hardware sample-rate that the AudioContext uses.  This is
normally the same as what the rest of WebRTC uses, as I understand.

[henrika (OOO until Aug 14), 2013-01-16 17:25:15]

LGTM from my side.

Note that I am not an owner here.

Chris,

given your vacation plans; would it not be a good idea if you tried to to land
this CL before you go. There will most likely be conflicts with mine
(https://codereview.chromium.org/11783059/) but I can fix that and sort out any
remaining issues during your vacation.

This CL does not contain any parts that good break existing clients and should
be safe to land even if we are not 100% on a perfect QoS for the new
MediaStreamDestination API.

https://codereview.chromium.org/11369171/diff/75001/content/renderer/media/me...
File content/renderer/media/media_stream_dependency_factory.cc (right):

https://codereview.chromium.org/11369171/diff/75001/content/renderer/media/me...
content/renderer/media/media_stream_dependency_factory.cc:267:
scoped_refptr<webrtc::LocalAudioTrackInterface> audio_track(
This part is kind of magic. Could you sort out the details and provide some
comments here?

[Chris Rogers, 2013-01-16 18:38:54]

https://codereview.chromium.org/11369171/diff/75001/content/renderer/media/me...
File content/renderer/media/media_stream_dependency_factory.cc (right):

https://codereview.chromium.org/11369171/diff/75001/content/renderer/media/me...
content/renderer/media/media_stream_dependency_factory.cc:267:
scoped_refptr<webrtc::LocalAudioTrackInterface> audio_track(
On 2013/01/16 17:25:15, henrika wrote:
> This part is kind of magic. Could you sort out the details and provide some
> comments here?

This is code you gave me :)  I think Per helped you with it.  I'll add a comment
if you can help explain it

[DaleCurtis, 2013-01-16 21:39:29]

media/ lgtm

[Chris Rogers, 2013-01-16 21:43:27]

+jam: content OWNERS

[jam, 2013-01-17 00:46:31]

On 2013/01/16 21:43:27, Chris Rogers wrote:
> +jam: content OWNERS

gypi lgtm

[commit-bot: I haz the power, 2013-01-17 00:52:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/crogers@google.com/11369171/75001

[commit-bot: I haz the power, 2013-01-17 03:32:31]

Change committed as 177330

[henrika (OOO until Aug 14), 2013-01-17 08:13:09]

https://codereview.chromium.org/11369171/diff/75001/content/renderer/media/me...
File content/renderer/media/media_stream_dependency_factory.cc (right):

https://codereview.chromium.org/11369171/diff/75001/content/renderer/media/me...
content/renderer/media/media_stream_dependency_factory.cc:267:
scoped_refptr<webrtc::LocalAudioTrackInterface> audio_track(
I will check with Per and perhaps add more comments here when I merge our CLs. I
have been involve in too many CLs and reviews lately it seems like since I even
forget what I've written before ;-)