[webkit-reviews] review granted: [Bug 214851] Added constructor methods to ChannelMergerNode, ChannelSplitterNode : [Attachment 405354] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Clark Wang <clark_wang at apple.com>'s
request for review:
Bug 214851: Added constructor methods to ChannelMergerNode, ChannelSplitterNode
https://bugs.webkit.org/show_bug.cgi?id=214851

Attachment 405354: Patch

https://bugs.webkit.org/attachment.cgi?id=405354&action=review




--- Comment #3 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 405354
  --> https://bugs.webkit.org/attachment.cgi?id=405354
Patch

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

> Source/WebCore/Modules/webaudio/ChannelMergerNode.cpp:48
> +    if (options.numberOfInputs > AudioContext::maxNumberOfChannels() ||
options.numberOfInputs <= 0)

|| !options.numberOfInputs

numberOfInputs is unsigned so it cannot be negative.

> Source/WebCore/Modules/webaudio/ChannelSplitterNode.cpp:42
> +    if (options.numberOfOutputs > AudioContext::maxNumberOfChannels() ||
options.numberOfOutputs <= 0)

|| !options.numberOfOutputs

> Source/WebCore/Modules/webaudio/ChannelSplitterNode.cpp:64
> +    , m_numberOfOutputs(numberOfOutputs)

Why do we need this data member? Looks like you could use existing
numberOfOutputs() ?