[Webkit-unassigned] [Bug 56692] Add high-quality band-limited audio resampling algorithm

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Chris Rogers <crogers at google.com>  2011-03-22 12:26:37 PST ---
(From update of attachment 86240)
View in context: https://bugs.webkit.org/attachment.cgi?id=86240&action=review

>> Source/WebCore/platform/audio/SincResampler.cpp:98
>> +    int halfSize = n / 2;
> 
> Mixing of signed/unsigned; bad idea.

FIXED

>> Source/WebCore/platform/audio/SincResampler.cpp:102
>> +    for (int offsetIndex = 0; offsetIndex <= m_numberOfKernelOffsets; ++offsetIndex) {
> 
> Mixing of signed/unsigned.

FIXED

>> Source/WebCore/platform/audio/SincResampler.cpp:105
>> +        for (int i = 0; i < n; ++i) {
> 
> Signed/unsigned mismatch. I think you should consider just using signed ints everywhere.

n is now int instead of unsigned

>> Source/WebCore/platform/audio/SincResampler.cpp:141
>> +    // Setup various region pointers in the buffer (see diagram above).
> 
> Please add some assertions to prevent out-of-bounds accesses. As far as I can tell the following are requirements:
>   m_blockSize >= m_kernelSize / 2
>   numberOfSourceFrames >= m_blockSize + m_kernelSize / 2
>   m_inputBuffer.size() >= m_blockSize + m_kernelSize
>   m_kernelSize % 2 == 0

Added your suggested assertions, except the one about numberOfSourceFrames.  It can be smaller than you suggest because consumeSource() will zero-pad if there aren't enough source frames.

>> Source/WebCore/platform/audio/SincResampler.cpp:152
>> +    unsigned numberOfDestinationFrames = numberOfSourceFrames / m_scaleFactor;
> 
> Is there a warning about loss of precision because the denominator is a double?

I added a static_cast<> here

>> Source/WebCore/platform/audio/SincResampler.cpp:178
>> +            float sum2 = 0.0;
> 
> The .0's here are unnecessary; see "Floating point literals" in http://www.webkit.org/coding/coding-style.html .

FIXED

>> Source/WebCore/platform/audio/SincResampler.cpp:198
>> +                // Optimize size 32 and size 64 kernels by unrolling the while loop.
> 
> Might be worth mentioning the percentage speedup you get by doing this.

DONE

>> Source/WebCore/platform/audio/SincResampler.cpp:325
>> +        memcpy(r2, r4, sizeof(float) * (m_kernelSize / 2) );
> 
> Extra spaces before closing paren.

FIXED

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