Bug 46528 - Add BiquadDSPKernel files
Summary: Add BiquadDSPKernel files
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-24 15:26 PDT by Chris Rogers
Modified: 2010-10-05 01:27 PDT (History)
8 users (show)

See Also:


Attachments
Patch (6.96 KB, patch)
2010-09-24 15:28 PDT, Chris Rogers
no flags Details | Formatted Diff | Diff
Patch (6.52 KB, patch)
2010-10-04 17:11 PDT, Chris Rogers
no flags Details | Formatted Diff | Diff
Patch (6.59 KB, patch)
2010-10-04 17:16 PDT, Chris Rogers
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Rogers 2010-09-24 15:26:58 PDT
Add BiquadDSPKernel files
Comment 1 Chris Rogers 2010-09-24 15:28:10 PDT
Created attachment 68770 [details]
Patch
Comment 2 Kenneth Russell 2010-10-04 15:07:38 PDT
Comment on attachment 68770 [details]
Patch

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

Code generally looks good; one change, plus please update the copyright notice.

> WebCore/webaudio/BiquadDSPKernel.h:56
> +    BiquadProcessor* m_biquadProcessor;

Please remove this weak pointer and instead provide a helper method which downcasts the superclass's pointer. Having multiple pointers in multiple classes all pointing to the same storage makes understanding the lifetime difficult. Also, the superclass doesn't maintain an OwnPtr; who actually owns the AudioDSPKernelProcessor* in the superclass?
Comment 3 Chris Rogers 2010-10-04 17:11:08 PDT
Created attachment 69723 [details]
Patch
Comment 4 Chris Rogers 2010-10-04 17:16:41 PDT
Created attachment 69724 [details]
Patch
Comment 5 Chris Rogers 2010-10-04 17:17:27 PDT
Updated LICENSE and added biquadProcessor() method as suggested.
Comment 6 Kenneth Russell 2010-10-04 17:19:25 PDT
Comment on attachment 69724 [details]
Patch

Looks good to me.
Comment 7 WebKit Commit Bot 2010-10-05 01:27:31 PDT
Comment on attachment 69724 [details]
Patch

Clearing flags on attachment: 69724

Committed r69095: <http://trac.webkit.org/changeset/69095>
Comment 8 WebKit Commit Bot 2010-10-05 01:27:37 PDT
All reviewed patches have been landed.  Closing bug.