Add AudioChannelSplitter files
Created attachment 68414 [details] Patch
This is the implementation for AudioChannelSplitter as described in the web audio API: http://chromium.googlecode.com/svn/trunk/samples/audio/specification/specification.html#AudioChannelSplitter-section
Comment on attachment 68414 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=68414&action=review This basically looks good but I'm marking it r- because of the initialize() / uninitialize() refactoring issue. Couple of other minor details. > WebCore/webaudio/AudioChannelSplitter.cpp:40 > +// This is considering that 5.1 (6 channels is the largest we'll ever deal with). This isn't a complete sentence. > WebCore/webaudio/AudioChannelSplitter.cpp:58 > +void AudioChannelSplitter::process(size_t /*framesToProcess*/) In the .cpp, since the argument is completely unused, just leave the name off rather than commenting it out. > WebCore/webaudio/AudioChannelSplitter.cpp:85 > +void AudioChannelSplitter::initialize() It looks like every AudioNode subclass has this exact same initialize() and uninitialize() method. They should be refactored into the base class. At that point m_isInitialized could be private in AudioNode. > WebCore/webaudio/AudioChannelSplitter.h:47 > + virtual void process(size_t /*framesToProcess*/); I think the argument name should be uncommented.
Created attachment 69123 [details] Patch
Comment on attachment 68414 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=68414&action=review >> WebCore/webaudio/AudioChannelSplitter.cpp:40 >> +// This is considering that 5.1 (6 channels is the largest we'll ever deal with). > > This isn't a complete sentence. FIXED >> WebCore/webaudio/AudioChannelSplitter.cpp:58 >> +void AudioChannelSplitter::process(size_t /*framesToProcess*/) > > In the .cpp, since the argument is completely unused, just leave the name off rather than commenting it out. I've added the following just for a little extra safety: ASSERT_UNUSED(framesToProcess, framesToProcess == source->length()); >> WebCore/webaudio/AudioChannelSplitter.cpp:85 >> +void AudioChannelSplitter::initialize() > > It looks like every AudioNode subclass has this exact same initialize() and uninitialize() method. They should be refactored into the base class. At that point m_isInitialized could be private in AudioNode. FIXED: I've changed AudioNode as you suggest (which I'll post as a separate patch) >> WebCore/webaudio/AudioChannelSplitter.h:47 >> + virtual void process(size_t /*framesToProcess*/); > > I think the argument name should be uncommented. FIXED
Comment on attachment 69123 [details] Patch Looks good.
Comment on attachment 69123 [details] Patch Clearing flags on attachment: 69123 Committed r68736: <http://trac.webkit.org/changeset/68736>
All reviewed patches have been landed. Closing bug.