Bug 77609 – AudioBus need to support stereo->mono down mix in copyFrom sumFrom etc.

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

[Raymond]

Hi

Usually this is not a big issue. But in the situation that :

A OfflineContext with mono channel buffer is connected to a RealtimeAnalyserNode, things go bad. Since RealtimeAnalyserNode is hardcoded to stereo, in the debug mode, this will lead to crash by ASSERT in AudioBuscopyFrom. And the best case is in release mode, it just return, thus no music is outputed. And this could also happen for other nodes if source and destination's channels not match.

But in this very case, the user might never know what is going on, for even the source buffer is mono, offlineContext buffer is mono, it still go wrong, he will never know that realtimeAnalyserNode rune everything.

I think the solution is :

1. make RealtimeAnalyserNode more clever and can adjust its output channel to suit for input channel
2. support down mix in audioBus

2 is a must. while 1 might be a addon.

[Raymond]

a crash example can be found at :
https://bugs.webkit.org/show_bug.cgi?id=77515#c3

[Wei James (wistoch)]

Created attachment 125088 [details]
Patch

[Chris Rogers]

Thanks James, the changes look good.  Could you please write a very simple layout test which demonstrates that the ASSERT no longer happens?  I think it could be a very simple test with a mono OfflineAudioContext and a RealtimeAnalyserNode (or DynamicsCompressorNode also I think could work) as Raymond describes in his initial comment.

[Wei James (wistoch)]

Created attachment 125271 [details]
Patch

[Wei James (wistoch)]

(In reply to comment #3)
> Thanks James, the changes look good.  Could you please write a very simple layout test which demonstrates that the ASSERT no longer happens?  I think it could be a very simple test with a mono OfflineAudioContext and a RealtimeAnalyserNode (or DynamicsCompressorNode also I think could work) as Raymond describes in his initial comment.

chris, patch update including a layout test. please review. thanks

[Wei James (wistoch)]

Created attachment 125281 [details]
Patch

[Chris Rogers]

Comment on attachment 125281 [details]
Patch

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

Looks good, if you add a few minor explanatory comments:

> LayoutTests/webaudio/stereo2mono-down-mixing.html:53
> +    context = new webkitAudioContext(1, sampleRate * renderLengthSeconds, sampleRate);

could add comment:

// Explicitly create a mono AudioContext so that the destination is mono.

> LayoutTests/webaudio/stereo2mono-down-mixing.html:58
> +

could add comment:

// We create an analyser here, because it has a hard-coded stereo output.

[Wei James (wistoch)]

Created attachment 125467 [details]
Patch

[Wei James (wistoch)]

(In reply to comment #7)
> (From update of attachment 125281 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=125281&action=review
> 
> Looks good, if you add a few minor explanatory comments:
> 
> > LayoutTests/webaudio/stereo2mono-down-mixing.html:53
> > +    context = new webkitAudioContext(1, sampleRate * renderLengthSeconds, sampleRate);
> 
> could add comment:
> 
> // Explicitly create a mono AudioContext so that the destination is mono.
> 
> > LayoutTests/webaudio/stereo2mono-down-mixing.html:58
> > +
> 
> could add comment:
> 
> // We create an analyser here, because it has a hard-coded stereo output.

thanks. patch updated to including the comments. thanks

[Chris Rogers]

Looks good.

[Kenneth Russell]

Comment on attachment 125467 [details]
Patch

rs=me

[WebKit Review Bot]

Comment on attachment 125467 [details]
Patch

Clearing flags on attachment: 125467

Committed r106858: <http://trac.webkit.org/changeset/106858>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.