Bug 75119 – MediaElementAudioSourceNode should support multi-channel > stereo

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Audio

[Chris Rogers]

Both up and down mixing paths for multi-channel > stereo need to be added into AudioBus::processWithGainFrom(), etc.
And then the checks in AudioBufferSourceNode and MediaElementAudioSourceNode need to be removed.

We should have layout tests (at least for AudioBufferSourceNode)

[Wei James (wistoch)]

(In reply to comment #0)
> Both up and down mixing paths for multi-channel > stereo need to be added into AudioBus::processWithGainFrom(), etc.
> And then the checks in AudioBufferSourceNode and MediaElementAudioSourceNode need to be removed.
> 
> We should have layout tests (at least for AudioBufferSourceNode)

roger, I'm interested in this feature implementation. 

If this task is not urgent and not so complicated for a newbie, can I have a try for it? thanks

[Wei James (wistoch)]

for 5.1 to stereo downmixing, there is no algorithm in https://dvcs.w3.org/hg/audio/raw-file/tip/webaudio/specification.html#UpMix-section, which algorithm should I use? thanks

[Chris Rogers]

Hi James, thanks for offering to do this.  I think it's something you could probably tackle.  But we'll have to first sort out the details of your other vsmul() optimizations in that same part of the code.  If you see my latest comment there, I think we should maybe simplify the denormal handling:

https://bugs.webkit.org/show_bug.cgi?id=75334

As far as the 5.1 to stereo case, the only question is the exact scaling factors (constant multipliers), so you can prepare a patch with close to the exact code (except the numeric values) and we can sort that out later.

[Wei James (wistoch)]

(In reply to comment #3)
> Hi James, thanks for offering to do this.  I think it's something you could probably tackle.  But we'll have to first sort out the details of your other vsmul() optimizations in that same part of the code.  If you see my latest comment there, I think we should maybe simplify the denormal handling:
> 
> https://bugs.webkit.org/show_bug.cgi?id=75334
> 
> As far as the 5.1 to stereo case, the only question is the exact scaling factors (constant multipliers), so you can prepare a patch with close to the exact code (except the numeric values) and we can sort that out later.

roger, thanks for the guidance. I will work on #75334 firstly. 

for up and down mixing, for source and destination, we both have 5 options: mono, stereo, quad, 5.0, 5.1.  Current we only have 3 options: mono->mono, mono->stereo, stereo->stereo. 

do we need to support all channel up and down mixing(that is all 25 options)?

[Chris Rogers]

(In reply to comment #4)
> (In reply to comment #3)
> > Hi James, thanks for offering to do this.  I think it's something you could probably tackle.  But we'll have to first sort out the details of your other vsmul() optimizations in that same part of the code.  If you see my latest comment there, I think we should maybe simplify the denormal handling:
> > 
> > https://bugs.webkit.org/show_bug.cgi?id=75334
> > 
> > As far as the 5.1 to stereo case, the only question is the exact scaling factors (constant multipliers), so you can prepare a patch with close to the exact code (except the numeric values) and we can sort that out later.
> 
> roger, thanks for the guidance. I will work on #75334 firstly. 
> 
> for up and down mixing, for source and destination, we both have 5 options: mono, stereo, quad, 5.0, 5.1.  Current we only have 3 options: mono->mono, mono->stereo, stereo->stereo. 
> 
> do we need to support all channel up and down mixing(that is all 25 options)?

We don't have to support all of them to start with.  After all we only currently support a few of them, so anything is an improvement.  Quad (4-channels) we can probably skip for now (and get to it later).  Also, the most important are the up-mixing options (as opposed to down-mixing).  For up-mixing to a number of channels which isn't known as having a specific channel layout such as 3 channels (which isn't 1,2,4,6 - for mono,stereo,quad,5.1) , the default up-mix should just be to copy/sum the channels into the first available channels, and then just ignore (or zero) the remaining channels.

For example, if up-mixing stereo to 3 channels:
channel0 -> channel0
channel1 -> channel1
ignore/silence -> channel2

This type of up-mix may also be interesting for 1,2,4,6, but we don't have to worry about that for now.

[Wei James (wistoch)]

Created attachment 142608 [details]
Patch

[Chris Rogers]

Comment on attachment 142608 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=142608&action=review

Thanks James! - looks good with nit

> Source/WebCore/Modules/webaudio/MediaElementAudioSourceNode.cpp:71
> +        if (!numberOfChannels || numberOfChannels > AudioContext::maxNumberOfChannels() \

style nit: Please remove continuation character '\' and leave everything on one long line

[Wei James (wistoch)]

Created attachment 142618 [details]
Patch

[Wei James (wistoch)]

Comment on attachment 142608 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=142608&action=review

>> Source/WebCore/Modules/webaudio/MediaElementAudioSourceNode.cpp:71
>> +        if (!numberOfChannels || numberOfChannels > AudioContext::maxNumberOfChannels() \
> 
> style nit: Please remove continuation character '\' and leave everything on one long line

fixed. thanks. Just back from chromium coding style. :)

[WebKit Review Bot]

Comment on attachment 142618 [details]
Patch

Clearing flags on attachment: 142618

Committed r117541: <http://trac.webkit.org/changeset/117541>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

Comment on attachment 142608 [details]
Patch

Cleared review? from obsolete attachment 142608 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).