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.
a crash example can be found at : https://bugs.webkit.org/show_bug.cgi?id=77515#c3
Created attachment 125088 [details] Patch
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.
Created attachment 125271 [details] Patch
(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
Created attachment 125281 [details] Patch
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.
Created attachment 125467 [details] Patch
(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
Looks good.
Comment on attachment 125467 [details] Patch rs=me
Comment on attachment 125467 [details] Patch Clearing flags on attachment: 125467 Committed r106858: <http://trac.webkit.org/changeset/106858>
All reviewed patches have been landed. Closing bug.