[Webkit-unassigned] [Bug 77509] Enable IPP for Biquad filter

[bugzilla-daemon at webkit.org]

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





--- Comment #20 from Raymond Toy <rtoy at chromium.org>  2012-02-07 10:01:44 PST ---
(From update of attachment 125775)
View in context: https://bugs.webkit.org/attachment.cgi?id=125775&action=review

This new patch looks much simpler than the previous one.  Thanks for applying this patch over the other one that added tests.

> Source/WebCore/ChangeLog:8
> +        Used IIR filter in IPP and imporved ~27% performance in linux.

"Used" -> "Use"
"and imporved..." -> "which improves performance in linux by ~27%"

> Source/WebCore/platform/audio/Biquad.cpp:56
> +

nit:  Remove extra blank line.

> Source/WebCore/platform/audio/Biquad.cpp:317
> +

Question:  Should ipp normalize the coefficients by dividing everything by a0 like we do for non-ipp?  Makes the actual coefficients consistent between implementations.  Perhaps this doesn't matter.

> Source/WebCore/platform/audio/Biquad.cpp:324
> +        return;

This breaks my expectations on what setNormalizedCoefficients should do and is, I think, incorrect.  In setLowShelfParams, there is the call setNormalizedCoefficients(1,0,0,1,0,0).  Since a0=1, nothing is done, including setting up the actual filter parameters.

I think 323-324 should be removed.  Or at least be sure you still set the filter parameters in this case.

This is probably the source of some of the test failures.

> Source/WebCore/platform/audio/Biquad.h:113
> +    Ipp8u* m_buffer;

nit:  Is there a more descriptive name for m_buffer?

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