Changeset 270157 in webkit
- Timestamp:
- Nov 21, 2020 8:51:13 PM (3 years ago)
- Location:
- trunk
- Files:
-
- 4 added
- 6 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/LayoutTests/ChangeLog
r270155 r270157 1 2020-11-21 Chris Dumez <cdumez@apple.com> 2 3 Poor resampling quality when using AudioContext sampleRate parameter 4 https://bugs.webkit.org/show_bug.cgi?id=219201 5 6 Reviewed by Geoff Garen. 7 8 Add layout test coverage that would hit assertions in debug. 9 10 * webaudio/audiocontext-large-samplerate-expected.txt: Added. 11 * webaudio/audiocontext-large-samplerate.html: Added. 12 * webaudio/audiocontext-low-samplerate-expected.txt: Added. 13 * webaudio/audiocontext-low-samplerate.html: Added. 14 1 15 2020-11-21 Chris Dumez <cdumez@apple.com> 2 16 -
trunk/Source/WebCore/ChangeLog
r270156 r270157 1 2020-11-21 Chris Dumez <cdumez@apple.com> 2 3 Poor resampling quality when using AudioContext sampleRate parameter 4 https://bugs.webkit.org/show_bug.cgi?id=219201 5 6 Reviewed by Geoff Garen. 7 8 MultiChannelResampler uses a SincResampler per audio channel. In MultiChannelResampler::process(), 9 it was calling SincResampler::process() for each channel, which would potentially end up calling 10 MultiChannelResampler::ChannelProvider::provideInput() to provide channel data used for resampling. 11 The issue was that MultiChannelResampler::ChannelProvider::provideInput() is implemented in such 12 a way that things will break if provideInput() gets called more than once per channel. When using 13 an AudioContext's sample rate larger than the hardware sample rate, provideInput() was getting 14 called more than once per channel and this resulted in very poor resampling quality. 15 16 To address the issue, MultiChannelResampler::process() now processes the data in chunks that 17 are small enough to guarantee that MultiChannelResampler::ChannelProvider::provideInput() will 18 never get called more than once per audio channel. 19 20 The fix is based on the corresponding MultiChannelResampler / SincResampler implementation in 21 Chrome: 22 - https://github.com/chromium/chromium/blob/master/media/base/multi_channel_resampler.cc 23 - https://github.com/chromium/chromium/blob/master/media/base/sinc_resampler.cc 24 25 Tests: webaudio/audiocontext-large-samplerate.html 26 webaudio/audiocontext-low-samplerate.html 27 28 * platform/audio/MultiChannelResampler.cpp: 29 (WebCore::MultiChannelResampler::ChannelProvider::setProvider): 30 (WebCore::MultiChannelResampler::ChannelProvider::setCurrentChannel): 31 (WebCore::MultiChannelResampler::process): 32 * platform/audio/MultiChannelResampler.h: 33 * platform/audio/SincResampler.cpp: 34 (WebCore::calculateChunkSize): 35 (WebCore::SincResampler::updateRegions): 36 * platform/audio/SincResampler.h: 37 1 38 2020-11-21 Simon Fraser <simon.fraser@apple.com> 2 39 -
trunk/Source/WebCore/platform/audio/MultiChannelResampler.cpp
r270155 r270157 39 39 // ChannelProvider provides a single channel of audio data (one channel at a time) for each channel 40 40 // of data provided to us in a multi-channel provider. 41 class MultiChannelResampler::ChannelProvider : public AudioSourceProvider{41 class MultiChannelResampler::ChannelProvider { 42 42 WTF_MAKE_FAST_ALLOCATED; 43 43 public: 44 explicit ChannelProvider(unsigned numberOfChannels )45 : m_ numberOfChannels(numberOfChannels)44 explicit ChannelProvider(unsigned numberOfChannels, unsigned requestFrames) 45 : m_multiChannelBus(AudioBus::create(numberOfChannels, requestFrames)) 46 46 { 47 47 } … … 49 49 void setProvider(AudioSourceProvider* multiChannelProvider) 50 50 { 51 m_currentChannel = 0;52 m_framesToProcess = 0;53 51 m_multiChannelProvider = multiChannelProvider; 54 52 } … … 56 54 // provideInput() will be called once for each channel, starting with the first channel. 57 55 // Each time it's called, it will provide the next channel of data. 58 void provideInput (AudioBus* bus, size_t framesToProcess) override56 void provideInputForChannel(AudioBus* bus, size_t framesToProcess, unsigned channelIndex) 59 57 { 58 ASSERT(channelIndex < m_multiChannelBus->numberOfChannels()); 59 ASSERT(framesToProcess <= m_multiChannelBus->length()); 60 if (framesToProcess > m_multiChannelBus->length()) 61 return; 62 60 63 bool isBusGood = bus && bus->numberOfChannels() == 1; 61 64 ASSERT(isBusGood); … … 65 68 // Get the data from the multi-channel provider when the first channel asks for it. 66 69 // For subsequent channels, we can just dish out the channel data from that (stored in m_multiChannelBus). 67 if (! m_currentChannel) {70 if (!channelIndex) { 68 71 m_framesToProcess = framesToProcess; 69 m_multiChannelBus = AudioBus::create(m_numberOfChannels, framesToProcess);70 72 m_multiChannelProvider->provideInput(m_multiChannelBus.get(), framesToProcess); 71 73 } 72 74 73 75 // All channels must ask for the same amount. This should always be the case, but let's just make sure. 74 bool isGood = m_multiChannelBus.get() &&framesToProcess == m_framesToProcess;76 bool isGood = framesToProcess == m_framesToProcess; 75 77 ASSERT(isGood); 76 78 if (!isGood) … … 78 80 79 81 // Copy the channel data from what we received from m_multiChannelProvider. 80 ASSERT(m_currentChannel <= m_numberOfChannels); 81 if (m_currentChannel < m_numberOfChannels) { 82 memcpy(bus->channel(0)->mutableData(), m_multiChannelBus->channel(m_currentChannel)->data(), sizeof(float) * framesToProcess); 83 ++m_currentChannel; 84 } 82 memcpy(bus->channel(0)->mutableData(), m_multiChannelBus->channel(channelIndex)->data(), sizeof(float) * framesToProcess); 85 83 } 86 84 … … 88 86 AudioSourceProvider* m_multiChannelProvider { nullptr }; 89 87 RefPtr<AudioBus> m_multiChannelBus; 90 unsigned m_numberOfChannels { 0 };91 unsigned m_currentChannel { 0 };92 88 size_t m_framesToProcess { 0 }; // Used to verify that all channels ask for the same amount. 93 89 }; 94 90 95 MultiChannelResampler::MultiChannelResampler(double scaleFactor, unsigned numberOfChannels, Optional<unsigned>requestFrames)91 MultiChannelResampler::MultiChannelResampler(double scaleFactor, unsigned numberOfChannels, unsigned requestFrames) 96 92 : m_numberOfChannels(numberOfChannels) 97 , m_channelProvider(makeUnique<ChannelProvider>(m_numberOfChannels ))93 , m_channelProvider(makeUnique<ChannelProvider>(m_numberOfChannels, requestFrames)) 98 94 { 99 95 // Create each channel's resampler. … … 113 109 m_channelProvider->setProvider(provider); 114 110 115 for (unsigned channelIndex = 0; channelIndex < m_numberOfChannels; ++channelIndex) { 116 // Depending on the sample-rate scale factor, and the internal buffering used in a SincResampler 117 // kernel, this call to process() will only sometimes call provideInput() on the channelProvider. 118 // However, if it calls provideInput() for the first channel, then it will call it for the remaining 119 // channels, since they all buffer in the same way and are processing the same number of frames. 120 m_kernels[channelIndex]->process(m_channelProvider.get(), 121 destination->channel(channelIndex)->mutableData(), 122 framesToProcess); 111 // We need to ensure that SincResampler only calls provideInput() once for each channel or it will confuse the logic 112 // inside ChannelProvider. To ensure this, we chunk the number of requested frames into SincResampler::chunkSize() 113 // sized chunks. SincResampler guarantees it will only call provideInput() once once we resample this way. 114 m_outputFramesReady = 0; 115 while (m_outputFramesReady < framesToProcess) { 116 size_t chunkSize = m_kernels[0]->chunkSize(); 117 size_t framesThisTime = std::min(framesToProcess - m_outputFramesReady, chunkSize); 118 119 for (unsigned channelIndex = 0; channelIndex < m_numberOfChannels; ++channelIndex) { 120 ASSERT(chunkSize == m_kernels[channelIndex]->chunkSize()); 121 bool wasProvideInputCalled = false; 122 m_kernels[channelIndex]->process(destination->channel(channelIndex)->mutableData() + m_outputFramesReady, framesThisTime, [this, channelIndex, &wasProvideInputCalled](AudioBus* bus, size_t framesToProcess) { 123 ASSERT_WITH_MESSAGE(!wasProvideInputCalled, "provideInputForChannel should only be called once"); 124 wasProvideInputCalled = true; 125 m_channelProvider->provideInputForChannel(bus, framesToProcess, channelIndex); 126 }); 127 } 128 129 m_outputFramesReady += framesThisTime; 123 130 } 124 131 -
trunk/Source/WebCore/platform/audio/MultiChannelResampler.h
r270155 r270157 42 42 public: 43 43 // requestFrames constrols the size of the buffer in frames when AudioSourceProvider::provideInput() is called. 44 explicit MultiChannelResampler(double scaleFactor, unsigned numberOfChannels, Optional<unsigned> requestFrames = WTF::nullopt);44 explicit MultiChannelResampler(double scaleFactor, unsigned numberOfChannels, unsigned requestFrames = SincResampler::defaultRequestFrames); 45 45 ~MultiChannelResampler(); 46 46 … … 60 60 class ChannelProvider; 61 61 std::unique_ptr<ChannelProvider> m_channelProvider; 62 size_t m_outputFramesReady { 0 }; 62 63 }; 63 64 -
trunk/Source/WebCore/platform/audio/SincResampler.cpp
r270155 r270157 112 112 namespace WebCore { 113 113 114 constexpr unsigned defaultRequestFrames { 512 };115 114 constexpr unsigned kernelSize { 32 }; 116 115 constexpr unsigned numberOfKernelOffsets { 32 }; 117 116 118 SincResampler::SincResampler(double scaleFactor, Optional<unsigned> requestFrames) 117 static size_t calculateChunkSize(unsigned blockSize, double scaleFactor) 118 { 119 return blockSize / scaleFactor; 120 } 121 122 SincResampler::SincResampler(double scaleFactor, unsigned requestFrames) 119 123 : m_scaleFactor(scaleFactor) 120 124 , m_kernelStorage(kernelSize * (numberOfKernelOffsets + 1)) 121 , m_requestFrames(requestFrames .valueOr(defaultRequestFrames))125 , m_requestFrames(requestFrames) 122 126 , m_inputBuffer(m_requestFrames + kernelSize) // See input buffer layout above. 123 127 , m_r1(m_inputBuffer.data()) … … 138 142 m_r4 = m_r0 + m_requestFrames - kernelSize / 2; 139 143 m_blockSize = m_r4 - m_r2; 144 m_chunkSize = calculateChunkSize(m_blockSize, m_scaleFactor); 140 145 141 146 // m_r1 at the beginning of the buffer. … … 188 193 } 189 194 190 void SincResampler::consumeSource(float* buffer, unsigned numberOfSourceFrames )191 { 192 ASSERT( m_sourceProvider);193 if (! m_sourceProvider)195 void SincResampler::consumeSource(float* buffer, unsigned numberOfSourceFrames, const Function<void(AudioBus* bus, size_t framesToProcess)>& provideInput) 196 { 197 ASSERT(provideInput); 198 if (!provideInput) 194 199 return; 195 200 … … 201 206 m_internalBus->setChannelMemory(0, buffer, numberOfSourceFrames); 202 207 203 m_sourceProvider->provideInput(m_internalBus.get(), numberOfSourceFrames);208 provideInput(m_internalBus.get(), numberOfSourceFrames); 204 209 } 205 210 206 211 namespace { 207 212 208 // BufferSourceProvider is an AudioSourceProvider wrapping an in-memory buffer.209 210 class BufferSourceProvider : public AudioSourceProvider{213 // BufferSourceProvider is an audio source provider wrapping an in-memory buffer. 214 215 class BufferSourceProvider { 211 216 public: 212 217 explicit BufferSourceProvider(const float* source, size_t numberOfSourceFrames) … … 217 222 218 223 // Consumes samples from the in-memory buffer. 219 void provideInput(AudioBus* bus, size_t framesToProcess) override224 void provideInput(AudioBus* bus, size_t framesToProcess) 220 225 { 221 226 ASSERT(m_source && bus); … … 254 259 while (remaining) { 255 260 unsigned framesThisTime = std::min(remaining, m_requestFrames); 256 process(&sourceProvider, destination, framesThisTime); 261 process(destination, framesThisTime, [&sourceProvider](AudioBus* bus, size_t framesToProcess) { 262 sourceProvider.provideInput(bus, framesToProcess); 263 }); 257 264 258 265 destination += framesThisTime; … … 261 268 } 262 269 263 void SincResampler::process( AudioSourceProvider* sourceProvider, float* destination, size_t framesToProcess)264 { 265 ASSERT( sourceProvider);266 if (! sourceProvider)270 void SincResampler::process(float* destination, size_t framesToProcess, const Function<void(AudioBus* bus, size_t framesToProcess)>& provideInput) 271 { 272 ASSERT(provideInput); 273 if (!provideInput) 267 274 return; 268 269 m_sourceProvider = sourceProvider;270 275 271 276 unsigned numberOfDestinationFrames = framesToProcess; … … 274 279 // Prime the input buffer at the start of the input stream. 275 280 if (!m_isBufferPrimed) { 276 consumeSource(m_r0, m_requestFrames );281 consumeSource(m_r0, m_requestFrames, provideInput); 277 282 m_isBufferPrimed = true; 278 283 } … … 522 527 // Step (5) 523 528 // Refresh the buffer with more input. 524 consumeSource(m_r0, m_requestFrames );529 consumeSource(m_r0, m_requestFrames, provideInput); 525 530 } 526 531 } -
trunk/Source/WebCore/platform/audio/SincResampler.h
r270155 r270157 42 42 WTF_MAKE_FAST_ALLOCATED; 43 43 public: 44 static constexpr unsigned defaultRequestFrames { 512 }; 45 44 46 // scaleFactor == sourceSampleRate / destinationSampleRate 45 47 // requestFrames controls the size in frames of the buffer requested by each provideInput() call. 46 SincResampler(double scaleFactor, Optional<unsigned> requestFrames = WTF::nullopt);48 SincResampler(double scaleFactor, unsigned requestFrames = defaultRequestFrames); 47 49 50 size_t chunkSize() const { return m_chunkSize; } 51 48 52 // Processes numberOfSourceFrames from source to produce numberOfSourceFrames / scaleFactor frames in destination. 49 53 void process(const float* source, float* destination, unsigned numberOfSourceFrames); 50 54 51 // Process with input sourcecallback function for streaming applications.52 void process( AudioSourceProvider*, float* destination, size_t framesToProcess);55 // Process with provideInput callback function for streaming applications. 56 void process(float* destination, size_t framesToProcess, const Function<void(AudioBus* bus, size_t framesToProcess)>& provideInput); 53 57 54 58 protected: 55 59 void initializeKernel(); 56 void consumeSource(float* buffer, unsigned numberOfSourceFrames );60 void consumeSource(float* buffer, unsigned numberOfSourceFrames, const Function<void(AudioBus* bus, size_t framesToProcess)>& provideInput); 57 61 void updateRegions(bool isSecondLoad); 58 62 … … 73 77 unsigned m_blockSize { 0 }; 74 78 79 size_t m_chunkSize { 0 }; 80 75 81 // Source is copied into this buffer for each processing pass. 76 82 AudioFloatArray m_inputBuffer; … … 86 92 const float* m_source { nullptr }; 87 93 unsigned m_sourceFramesAvailable { 0 }; 88 89 // m_sourceProvider is used to provide the audio input stream to the resampler.90 AudioSourceProvider* m_sourceProvider { nullptr };91 94 92 95 // The buffer is primed once at the very beginning of processing.
Note: See TracChangeset
for help on using the changeset viewer.