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

[bugzilla-daemon at webkit.org]

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





--- Comment #21 from xingnan.wang at intel.com  2012-02-07 19:00:47 PST ---
(In reply to comment #20)
> (From update of attachment 125775 [details])
> 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.
> 
That`s OK.
> > 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%"
> 
Ok.
> > Source/WebCore/platform/audio/Biquad.cpp:56
> > +
> 
> nit:  Remove extra blank line.
> 
OK.
> > 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.
> 
ippsIIRInit64f_BiQuad_32f normalizes the coefficients internally, so it does not 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.
> 
Agree, it`s incorrect. Thanks for correcting me and I`ll remove them.
> This is probably the source of some of the test failures.
> 
But it seems it`s not the root cause of test failures. In ipp m_b0, m_b1 and others are not used while processing, and the tests still failed after removing 323-324.
> > Source/WebCore/platform/audio/Biquad.h:113
> > +    Ipp8u* m_buffer;
> 
> nit:  Is there a more descriptive name for m_buffer?
How about m_ippInternalBuffer?

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