[Webkit-unassigned] [Bug 46290] Add AudioChannelSplitter files

[bugzilla-daemon at webkit.org]

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


Kenneth Russell <kbr at google.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #68414|review?                     |review-
               Flag|                            |




--- Comment #3 from Kenneth Russell <kbr at google.com>  2010-09-28 10:33:53 PST ---
(From update of attachment 68414)
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.

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