[Webkit-unassigned] [Bug 44795] audio engine: add Biquad files

[bugzilla-daemon at webkit.org]

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





--- Comment #2 from Kenneth Russell <kbr at google.com>  2010-09-07 19:18:53 PST ---
(From update of attachment 65768)
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?

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