Make WebAudio output as stereo as default

Make WebAudio output as stereo as default.


NOTRY=true

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169964

[no longer working on chromium, 2014-03-25 09:39:08]

Tommy, could you please review this small patch?

SX

[Tommy Widenflycht, 2014-03-25 09:49:26]

https://codereview.chromium.org/210713005/diff/1/Source/modules/webaudio/Audi...
File Source/modules/webaudio/AudioContext.cpp (right):

https://codereview.chromium.org/210713005/diff/1/Source/modules/webaudio/Audi...
Source/modules/webaudio/AudioContext.cpp:421: // FIXME: Add support for an
optional argument which specifies the number of channels.
I guess this comment can be removed as well

https://codereview.chromium.org/210713005/diff/1/Source/modules/webaudio/Audi...
Source/modules/webaudio/AudioContext.cpp:422: return
MediaStreamAudioDestinationNode::create(this, 2);
Can this be made into an enum? MediaStreamAudioDestinationNode::Stereo is much
better than 2.

[no longer working on chromium, 2014-03-25 10:08:29]

Tommy, PTAL.

Thanks,
SX

https://codereview.chromium.org/210713005/diff/1/Source/modules/webaudio/Audi...
File Source/modules/webaudio/AudioContext.cpp (right):

https://codereview.chromium.org/210713005/diff/1/Source/modules/webaudio/Audi...
Source/modules/webaudio/AudioContext.cpp:413: node->setFormat(2, sampleRate());
for example here

https://codereview.chromium.org/210713005/diff/1/Source/modules/webaudio/Audi...
Source/modules/webaudio/AudioContext.cpp:421: // FIXME: Add support for an
optional argument which specifies the number of channels.
On 2014/03/25 09:49:27, Tommy Widenflycht wrote:
> I guess this comment can be removed as well

Done with replacing the FIXME with a comment, as how it is done in other places.

https://codereview.chromium.org/210713005/diff/1/Source/modules/webaudio/Audi...
Source/modules/webaudio/AudioContext.cpp:422: return
MediaStreamAudioDestinationNode::create(this, 2);
On 2014/03/25 09:49:27, Tommy Widenflycht wrote:
> Can this be made into an enum? MediaStreamAudioDestinationNode::Stereo is much
> better than 2.

:) Looking at the whole file, they use 2 for the channel. I suggest we should
keep what it is, and leave it to WebAudio guys to do a thorough clean-up if they
think it is suitable.

https://codereview.chromium.org/210713005/diff/1/Source/modules/webaudio/Audi...
Source/modules/webaudio/AudioContext.cpp:428: return createScriptProcessor(0, 2,
2, exceptionState);
here, and elsewhere in all the webaudio code.

[Tommy Widenflycht, 2014-03-25 10:28:12]

lgtm

[no longer working on chromium, 2014-03-25 17:08:50]

Chris, can I have your stamp for this small fix?

SX

[Raymond Toy, 2014-03-25 17:14:29]

On 2014/03/25 17:08:50, xians1 wrote:
> Chris, can I have your stamp for this small fix?
> 
> SX

I'm not Chris, but lgtm from me.

[no longer working on chromium, 2014-03-25 17:15:43]

The CQ bit was checked by xians@chromium.org

[no longer working on chromium, 2014-03-25 17:15:45]

The CQ bit was unchecked by xians@chromium.org

[no longer working on chromium, 2014-03-25 17:16:06]

The CQ bit was checked by xians@chromium.org

[commit-bot: I haz the power, 2014-03-25 17:16:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xians@chromium.org/210713005/20001

[commit-bot: I haz the power, 2014-03-25 17:17:20]

Change committed as 169964

[no longer working on chromium, 2014-03-25 17:17:25]

Thanks Raymond, I am landing this now so that the fix can be in tomorrow's
canary and our tester can verify it before the cut.

Chris, feel free to have comment and I will be glad to make follow-up CLs.

[Chris Wilson, 2014-03-25 17:28:50]

On 2014/03/25 17:17:25, xians1 wrote:
> Thanks Raymond, I am landing this now so that the fix can be in tomorrow's
> canary and our tester can verify it before the cut.
> 
> Chris, feel free to have comment and I will be glad to make follow-up CLs.

LGTM.

[mantriur74_gmail.com, 2014-04-15 19:42:24]

I know this is slightly off topic, but ... is there currently a way 
(preferably in the stable release) to actually set the number of channels 
from the JavaScript end or are we stuck with mono until this patch hits 
(and then with stereo)? The way I understand it, a media stream destination 
should adjust to the available output channels and up/downmix accordingly.

channelCount doesn't seem to do anything at the moment and even defaults to 
2 despite the output being mono ...

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.