Add high-quality band-limited audio resampling algorithm
Created attachment 86240 [details] Patch
The code has been tested a fair bit by processing exponentially swept sine waves and diffing against the ideal generated result. The code can be optimized further, but runs well enough to get us started. Note: this code will not be used in the mac port, since CoreAudio's resampler is very good and better optimized.
Comment on attachment 86240 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=86240&action=review There are a bunch of signed/unsigned mismatches that need to be dealt with because I'm pretty sure they'll be flagged as errors on some platforms (I think Windows). Some assertions are also needed to ensure no out-of-bounds accesses. > Source/WebCore/platform/audio/SincResampler.cpp:98 > + unsigned n = m_kernelSize; > + int halfSize = n / 2; Mixing of signed/unsigned; bad idea. > Source/WebCore/platform/audio/SincResampler.cpp:102 > + for (int offsetIndex = 0; offsetIndex <= m_numberOfKernelOffsets; ++offsetIndex) { Mixing of signed/unsigned. > 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. > 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 > 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? > Source/WebCore/platform/audio/SincResampler.cpp:178 > + float sum1 = 0.0; > + float sum2 = 0.0; The .0's here are unnecessary; see "Floating point literals" in http://www.webkit.org/coding/coding-style.html . > 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. > Source/WebCore/platform/audio/SincResampler.cpp:325 > + memcpy(r1, r3, sizeof(float) * (m_kernelSize / 2) ); > + memcpy(r2, r4, sizeof(float) * (m_kernelSize / 2) ); Extra spaces before closing paren.
Created attachment 86490 [details] Patch
Comment on attachment 86240 [details] Patch 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
Comment on attachment 86490 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=86490&action=review Looks better, thanks. Couple of additional minor issues you can fix upon landing. > Source/WebCore/platform/audio/SincResampler.cpp:165 > + while (numberOfDestinationFrames > 0) { numberOfDestinationFrames is unsigned, so this test is not well written. Would be better as "while (numberOfDestinationFrames)". > Source/WebCore/platform/audio/SincResampler.cpp:316 > + numberOfDestinationFrames--; In WebKit, prefer predecrement (--numberOfDestinationFrames).
Committed r81702: <http://trac.webkit.org/changeset/81702>