Bug 46532 – Add LowPass2FilterNode files

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Chris Rogers]

Add LowPass2FilterNode files

[Chris Rogers]

Created attachment 68774 [details]
Patch

[Kenneth Russell]

Comment on attachment 68774 [details]
Patch

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

The code looks OK, but I'm marking this r- for the license update as discussed offline. Please also consider getting the Web Audio spec up to date with this change before landing it.

> WebCore/webaudio/LowPass2FilterNode.idl:34
> +    ] LowPass2FilterNode : AudioNode {

http://chromium.googlecode.com/svn/trunk/samples/audio/specification/specification.html#APIOverview-section says that LowPass2FilterNode "would be" a subclass of AudioNode; I think you should modify the spec to include the API definition for this class before committing it.

[Chris Rogers]

Created attachment 69689 [details]
Patch

[Chris Rogers]

Fixed the license.  Also I put a FIXME in the .idl file about moving to a BiquadNode implementation.

[Kenneth Russell]

Comment on attachment 69689 [details]
Patch

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

Sorry I didn't catch the m_biquadProcessor issue earlier but please change it here as well as in HighPass2FilterNode.

> WebCore/webaudio/LowPass2FilterNode.cpp:38
> +    m_processor = lowpassFilter.release();

Per below, the weak m_biquadProcessor pointer is very confusing. Please change this to "m_processor = adoptPtr(...)" and add a helper function to fetch the processor as a BiquadProcessor.

> WebCore/webaudio/LowPass2FilterNode.h:48
> +    BiquadProcessor* m_biquadProcessor;

This weak pointer makes the lifetime of this object confusing. Make this a helper function "BiquadProcessor* biquadProcessor()" returning a static_cast of the result of processor().

[Chris Rogers]

Created attachment 69698 [details]
Patch

[Chris Rogers]

You're right, your suggestions make it much cleaner -- FIXED.

[Kenneth Russell]

Comment on attachment 69698 [details]
Patch

Looks good to me.

[WebKit Commit Bot]

Comment on attachment 69698 [details]
Patch

Clearing flags on attachment: 69698

Committed r69087: <http://trac.webkit.org/changeset/69087>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.