[Webkit-unassigned] [Bug 84687] Add multichannel support for input of JavaScriptAudioNode

[bugzilla-daemon at webkit.org]

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





--- Comment #3 from Chris Rogers <crogers at google.com>  2012-04-26 12:50:21 PST ---
(From update of attachment 138525)
View in context: https://bugs.webkit.org/attachment.cgi?id=138525&action=review

Thanks Xingnan, this looks pretty good.  I like your approach of using the "m_internalInputBus" and think this should work much better than what we have now.
But we still need to handle the case where numberOfInputChannels==0 (if the JavaScriptAudioNode is only synthesizing audio and doesn't care about input).
And the test cases need to be split out into separate test files.

> Source/WebCore/Modules/webaudio/JavaScriptAudioNode.cpp:61
> +    if (!numberOfInputChannels || numberOfInputChannels > AudioContext::maxNumberOfChannels())

In the Web Audio API spec, it is allowed for *either* numberOfInputChannels or numberOfOutputChannels to be zero.  It is only illegal if *both* are zero at the same time.

So, we need to allow numberOfInputChannels==0

> Source/WebCore/Modules/webaudio/JavaScriptAudioNode.cpp:-108
> -        m_inputBuffers.append(AudioBuffer::create(2, bufferSize(), sampleRate));

Probably need to check m_internalInputBus.numberOfChannels() > 0 before appending to m_inputBuffers here, and instead append(0) (if there are no input channels)

> Source/WebCore/Modules/webaudio/JavaScriptAudioNode.cpp:171
> +

You need to check numberOfInputChannels > 0 before calling copyFrom below

> LayoutTests/webaudio/javascriptaudionode.html:54
>  

I think we should keep the original javascriptaudionode.html test-file the same, and create one or two additional tests using your new testing code here.

You can create a new .js helper test file in the "resources" directory called "javascriptaudionode-testing.js" with the fillData() and checkData() methods above.

Then you can create two new test files "javascriptaudionode-2channel-input.html" and "javascriptaudionode-6channel-input.html"

It looks like you're trying to test both cases (2-channel and 6-channel) in the same test which is a bit confusing and I'm not sure it can work properly.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.