[Webkit-unassigned] [Bug 46527] Add BiquadProcessor files

[bugzilla-daemon at webkit.org]

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


Kenneth Russell <kbr at google.com> changed:

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




--- Comment #3 from Kenneth Russell <kbr at google.com>  2010-10-04 14:40:52 PST ---
(From update of attachment 68769)
View in context: https://bugs.webkit.org/attachment.cgi?id=68769&action=review

Several issues. Also please update the license text per offline discussion.

> WebCore/webaudio/BiquadProcessor.cpp:58
> +        m_parameter3 = AudioParam::create("unused", 0.0, 0.0, 1.0);

These are the same parameters as LowPass2. Something looks wrong.

> WebCore/webaudio/BiquadProcessor.cpp:68
> +        m_parameter3 = AudioParam::create("unused", 0.0, 0.0, 1.0);

These parameters look the same as those for Peaking aside from m_parameter3 being "unused" instead of "gain". I don't know what the right values should be but please double-check both.

> WebCore/webaudio/BiquadProcessor.cpp:73
> +        m_parameter3 = AudioParam::create("unused", 0.0, 0.0, 1.0);

Two assignments to m_parameter3 and none to m_parameter2.

> WebCore/webaudio/BiquadProcessor.cpp:78
> +        m_parameter3 = AudioParam::create("unused", 0.0, 0.0, 1.0);

Two assignments to m_parameter3 and none to m_parameter2.

> WebCore/webaudio/BiquadProcessor.h:65
> +    AudioParam& parameter3() { return *m_parameter3; }

Especially since subclasses take the addresses of these references, I think you should return pointers here. You can indicate in the documentation that the BiquadProcessor owns the storage for the parameters.

> WebCore/webaudio/BiquadProcessor.h:70
> +    FilterType m_type; // lowpass, highpass, etc.

As many of these protected fields should be made private as possible. Also, this comment is unnecessary because the FilterType enum is self-documenting.

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