Bug 44795 – audio engine: add Biquad files

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

[Chris Rogers]

audio engine: add Biquad files

[Chris Rogers]

Created attachment 65768 [details]
Patch

[Kenneth Russell]

Comment on attachment 65768 [details]
Patch

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

I trust that the math is correct and is in some generally standard form for signal processing which is why the equations are in certain forms with certain variable names. I have one main question, which we've discussed offline, about a double negative which might imply changing where some negation occurs throughout the code. Also a couple of minor formatting issues. Will wait for your feedback before updating the review status of the patch.

> WebCore/platform/audio/Biquad.cpp:69
> +
Unnecessary newline

> WebCore/platform/audio/Biquad.cpp:165
> +    destP[1] = destP[framesToProcess - 1 + 2];
These are subtle since they would look like out of bounds accesses on arrays simply of length framesToProcess. Could you add a comment referencing the allocation of two extra samples for the input and output buffers in the constructor?

> WebCore/platform/audio/Biquad.cpp:190
> +        resonance = 0.0; // for now can't go negative
resonance = std::max(0.0, resonance);

> WebCore/platform/audio/Biquad.cpp:212
> +        resonance = 0.0; // for now can't go negative
Same std::max as above.

> WebCore/platform/audio/Biquad.cpp:266
> +    m_b1 = - (-2.0 * pole.real());
Why do you have a double negative here? Why not just write "2.0 * pole.real()"?

> WebCore/platform/audio/Biquad.cpp:269
> +    m_b2 = - (poleMag * poleMag);
Extra space between - and (

> WebCore/platform/audio/Biquad.h:90
> +
Unnecessary newline?

[Chris Rogers]

Created attachment 66967 [details]
Patch

[Chris Rogers]

Ken, I've addressed your comments.  I've also fixed an issue where the filter coefficients (m_b1 and m_b2) were being used with negative values from what is normally used in biquad filter difference equations.  I've negated their use both in the processing math and when the coefficient values are calculated, so overall there is no change mathematically, but the code now looks cleaner and is in a more standard form.

[Kenneth Russell]

Comment on attachment 66967 [details]
Patch

Looks good.

[WebKit Commit Bot]

Comment on attachment 66967 [details]
Patch

Clearing flags on attachment: 66967

Committed r67132: <http://trac.webkit.org/changeset/67132>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.