[webkit-reviews] review granted: [Bug 62078] Implement BiquadFilterNode for filter types: LOWPASS, HIGHPASS, BANDPASS, LOWSHELF, HIGHSHELF, PEAKING, NOTCH, ALLPASS : [Attachment 96002] Patch

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has granted Chris Rogers
<crogers at google.com>'s request for review:
Bug 62078: Implement BiquadFilterNode for filter types: LOWPASS, HIGHPASS,
BANDPASS, LOWSHELF, HIGHSHELF, PEAKING, NOTCH, ALLPASS
https://bugs.webkit.org/show_bug.cgi?id=62078

Attachment 96002: Patch
https://bugs.webkit.org/attachment.cgi?id=96002&action=review

------- Additional Comments from Kenneth Russell <kbr at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=96002&action=review

I'm not qualified to review the math for the coefficients but have a couple of
minor comments.

Also, when are you going to delete the LowPass2FilterNode and
HighPass2FilterNode, or at least implement them in terms of the
Biquad2FilterNode? If they're deprecated then their construction should
probably log a warning message to the console that they will be removed soon.
There are not enough users of the Web Audio API at this point to warrant
keeping them around permanently.

> Source/WebCore/platform/audio/Biquad.cpp:230
> +    double a0Inverse = 1.0 / a0;

All of the .0's in this file seem unnecessary and so should be removed per
WebKit style.

> Source/WebCore/platform/audio/Biquad.cpp:250
> +    // FIXME: optimize the common terms

Why not at least define aPlusOne and aMinusOne now rather than adding this
FIXME?

> Source/WebCore/platform/audio/Biquad.cpp:272
> +    // FIXME: optimize the common terms

Same question about common terms.