Changeset 270141 in webkit


Ignore:
Timestamp:
Nov 20, 2020 5:45:54 PM (3 years ago)
Author:
Chris Dumez
Message:

Poor resampling quality when using AudioContext sampleRate parameter
​https://bugs.webkit.org/show_bug.cgi?id=219201

Reviewed by Geoff Garen.

Source/WebCore:

MultiChannelResampler uses a SincResampler per audio channel. In MultiChannelResampler::process(),
it was calling SincResampler::process() for each channel, which would potentially end up calling
MultiChannelResampler::ChannelProvider::provideInput() to provide channel data used for resampling.
The issue was that MultiChannelResampler::ChannelProvider::provideInput() is implemented in such
a way that things will break if provideInput() gets called more than once per channel. When using
an AudioContext's sample rate larger than the hardware sample rate, provideInput() was getting
called more than once per channel and this resulted in very poor resampling quality.

To address the issue, MultiChannelResampler::process() now processes the data in chunks that
are small enough to guarantee that MultiChannelResampler::ChannelProvider::provideInput() will
never get called more than once per audio channel.

The fix is based on the corresponding MultiChannelResampler / SincResampler implementation in
Chrome:

Tests: webaudio/audiocontext-large-samplerate.html

webaudio/audiocontext-low-samplerate.html

  • platform/audio/MultiChannelResampler.cpp:

(WebCore::MultiChannelResampler::ChannelProvider::setProvider):
(WebCore::MultiChannelResampler::ChannelProvider::setCurrentChannel):
(WebCore::MultiChannelResampler::process):

  • platform/audio/MultiChannelResampler.h:
  • platform/audio/SincResampler.cpp:

(WebCore::calculateChunkSize):
(WebCore::SincResampler::updateRegions):

  • platform/audio/SincResampler.h:

LayoutTests:

Add layout test coverage that would hit assertions in debug.

  • webaudio/audiocontext-large-samplerate-expected.txt: Added.
  • webaudio/audiocontext-large-samplerate.html: Added.
  • webaudio/audiocontext-low-samplerate-expected.txt: Added.
  • webaudio/audiocontext-low-samplerate.html: Added.
Location:
trunk
Files:
4 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r270139 r270141  
     12020-11-20  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
    1152020-11-20  Chris Dumez  <cdumez@apple.com>
    216

  • trunk/Source/WebCore/ChangeLog

    r270136 r270141  
     12020-11-20  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
    1382020-11-20  Kate Cheney  <katherine_cheney@apple.com>
    239

  • trunk/Source/WebCore/platform/audio/MultiChannelResampler.cpp

    r267014 r270141  
    3434
    3535#include "AudioBus.h"
     36#include "AudioUtilities.h"
    3637
    3738namespace WebCore {
    … …  
    3940// ChannelProvider provides a single channel of audio data (one channel at a time) for each channel
    4041// of data provided to us in a multi-channel provider.
    41 class MultiChannelResampler::ChannelProvider : public AudioSourceProvider {
     42class MultiChannelResampler::ChannelProvider {
    4243    WTF_MAKE_FAST_ALLOCATED;
    4344public:
    4445    explicit ChannelProvider(unsigned numberOfChannels)
    45         : m_numberOfChannels(numberOfChannels)
     46        : m_multiChannelBus(AudioBus::create(numberOfChannels, AudioUtilities::renderQuantumSize))
    4647    {
    4748    }
    … …  
    4950    void setProvider(AudioSourceProvider* multiChannelProvider)
    5051    {
    51         m_currentChannel = 0;
    52         m_framesToProcess = 0;
    5352        m_multiChannelProvider = multiChannelProvider;
    5453    }
    … …  
    5655    // provideInput() will be called once for each channel, starting with the first channel.
    5756    // Each time it's called, it will provide the next channel of data.
    58     void provideInput(AudioBus* bus, size_t framesToProcess) override
     57    void provideInputForChannel(AudioBus* bus, size_t framesToProcess, unsigned channelIndex)
    5958    {
     59        ASSERT(channelIndex < m_multiChannelBus->numberOfChannels());
     60        ASSERT(framesToProcess <= AudioUtilities::renderQuantumSize);
     61        if (framesToProcess > AudioUtilities::renderQuantumSize)
     62            return;
     63
    6064        bool isBusGood = bus && bus->numberOfChannels() == 1;
    6165        ASSERT(isBusGood);
    … …  
    6569        // Get the data from the multi-channel provider when the first channel asks for it.
    6670        // For subsequent channels, we can just dish out the channel data from that (stored in m_multiChannelBus).
    67         if (!m_currentChannel) {
     71        if (!channelIndex) {
    6872            m_framesToProcess = framesToProcess;
    69             m_multiChannelBus = AudioBus::create(m_numberOfChannels, framesToProcess);
    7073            m_multiChannelProvider->provideInput(m_multiChannelBus.get(), framesToProcess);
    7174        }
    7275
    7376        // 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;
     77        bool isGood = framesToProcess == m_framesToProcess;
    7578        ASSERT(isGood);
    7679        if (!isGood)
    … …  
    7881
    7982        // 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         }
     83        memcpy(bus->channel(0)->mutableData(), m_multiChannelBus->channel(channelIndex)->data(), sizeof(float) * framesToProcess);
    8584    }
    8685
    … …  
    8887    AudioSourceProvider* m_multiChannelProvider { nullptr };
    8988    RefPtr<AudioBus> m_multiChannelBus;
    90     unsigned m_numberOfChannels { 0 };
    91     unsigned m_currentChannel { 0 };
    9289    size_t m_framesToProcess { 0 }; // Used to verify that all channels ask for the same amount.
    9390};
    … …  
    113110    m_channelProvider->setProvider(provider);
    114111
    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);
     112    // We need to ensure that SincResampler only calls provideInput() once for each channel or it will confuse the logic
     113    // inside ChannelProvider. To ensure this, we chunk the number of requested frames into SincResampler::chunkSize()
     114    // sized chunks. SincResampler guarantees it will only call provideInput() once once we resample this way.
     115    m_outputFramesReady = 0;
     116    while (m_outputFramesReady < framesToProcess) {
     117        size_t chunkSize = m_kernels[0]->chunkSize();
     118        size_t framesThisTime = std::min(framesToProcess - m_outputFramesReady, chunkSize);
     119
     120        for (unsigned channelIndex = 0; channelIndex < m_numberOfChannels; ++channelIndex) {
     121            ASSERT(chunkSize == m_kernels[channelIndex]->chunkSize());
     122            bool wasProvideInputCalled = false;
     123            m_kernels[channelIndex]->process(destination->channel(channelIndex)->mutableData() + m_outputFramesReady, framesThisTime, [this, channelIndex, &wasProvideInputCalled](AudioBus* bus, size_t framesToProcess) {
     124                ASSERT_WITH_MESSAGE(!wasProvideInputCalled, "provideInputForChannel should only be called once");
     125                wasProvideInputCalled = true;
     126                m_channelProvider->provideInputForChannel(bus, framesToProcess, channelIndex);
     127            });
     128        }
     129
     130        m_outputFramesReady += framesThisTime;
    123131    }
    124132

  • trunk/Source/WebCore/platform/audio/MultiChannelResampler.h

    r267014 r270141  
    6060    class ChannelProvider;
    6161    std::unique_ptr<ChannelProvider> m_channelProvider;
     62    size_t m_outputFramesReady { 0 };
    6263};
    6364

  • trunk/Source/WebCore/platform/audio/SincResampler.cpp

    r267014 r270141  
    116116constexpr unsigned numberOfKernelOffsets { 32 };
    117117
     118static size_t calculateChunkSize(unsigned blockSize, double scaleFactor)
     119{
     120    return blockSize / scaleFactor;
     121}
     122
    118123SincResampler::SincResampler(double scaleFactor, Optional<unsigned> requestFrames)
    119124    : m_scaleFactor(scaleFactor)
    … …  
    138143    m_r4 = m_r0 + m_requestFrames - kernelSize / 2;
    139144    m_blockSize = m_r4 - m_r2;
     145    m_chunkSize = calculateChunkSize(m_blockSize, m_scaleFactor);
    140146
    141147    // m_r1 at the beginning of the buffer.
    … …  
    188194}
    189195
    190 void SincResampler::consumeSource(float* buffer, unsigned numberOfSourceFrames)
    191 {
    192     ASSERT(m_sourceProvider);
    193     if (!m_sourceProvider)
     196void SincResampler::consumeSource(float* buffer, unsigned numberOfSourceFrames, const Function<void(AudioBus* bus, size_t framesToProcess)>& provideInput)
     197{
     198    ASSERT(provideInput);
     199    if (!provideInput)
    194200        return;
    195201
    … …  
    201207    m_internalBus->setChannelMemory(0, buffer, numberOfSourceFrames);
    202208   
    203     m_sourceProvider->provideInput(m_internalBus.get(), numberOfSourceFrames);
     209    provideInput(m_internalBus.get(), numberOfSourceFrames);
    204210}
    205211
    206212namespace {
    207213
    208 // BufferSourceProvider is an AudioSourceProvider wrapping an in-memory buffer.
    209 
    210 class BufferSourceProvider : public AudioSourceProvider {
     214// BufferSourceProvider is an audio source provider wrapping an in-memory buffer.
     215
     216class BufferSourceProvider {
    211217public:
    212218    explicit BufferSourceProvider(const float* source, size_t numberOfSourceFrames)
    … …  
    217223   
    218224    // Consumes samples from the in-memory buffer.
    219     void provideInput(AudioBus* bus, size_t framesToProcess) override
     225    void provideInput(AudioBus* bus, size_t framesToProcess)
    220226    {
    221227        ASSERT(m_source && bus);
    … …  
    254260    while (remaining) {
    255261        unsigned framesThisTime = std::min(remaining, m_requestFrames);
    256         process(&sourceProvider, destination, framesThisTime);
     262        process(destination, framesThisTime, [&sourceProvider](AudioBus* bus, size_t framesToProcess) {
     263            sourceProvider.provideInput(bus, framesToProcess);
     264        });
    257265       
    258266        destination += framesThisTime;
    … …  
    261269}
    262270
    263 void SincResampler::process(AudioSourceProvider* sourceProvider, float* destination, size_t framesToProcess)
    264 {
    265     ASSERT(sourceProvider);
    266     if (!sourceProvider)
     271void SincResampler::process(float* destination, size_t framesToProcess, const Function<void(AudioBus* bus, size_t framesToProcess)>& provideInput)
     272{
     273    ASSERT(provideInput);
     274    if (!provideInput)
    267275        return;
    268    
    269     m_sourceProvider = sourceProvider;
    270276
    271277    unsigned numberOfDestinationFrames = framesToProcess;
    … …  
    274280    // Prime the input buffer at the start of the input stream.
    275281    if (!m_isBufferPrimed) {
    276         consumeSource(m_r0, m_requestFrames);
     282        consumeSource(m_r0, m_requestFrames, provideInput);
    277283        m_isBufferPrimed = true;
    278284    }
    … …  
    522528        // Step (5)
    523529        // Refresh the buffer with more input.
    524         consumeSource(m_r0, m_requestFrames);
     530        consumeSource(m_r0, m_requestFrames, provideInput);
    525531    }
    526532}

  • trunk/Source/WebCore/platform/audio/SincResampler.h

    r267143 r270141  
    4646    SincResampler(double scaleFactor, Optional<unsigned> requestFrames = WTF::nullopt);
    4747   
     48    size_t chunkSize() const { return m_chunkSize; }
     49
    4850    // Processes numberOfSourceFrames from source to produce numberOfSourceFrames / scaleFactor frames in destination.
    4951    void process(const float* source, float* destination, unsigned numberOfSourceFrames);
    5052
    51     // Process with input source callback function for streaming applications.
    52     void process(AudioSourceProvider*, float* destination, size_t framesToProcess);
     53    // Process with provideInput callback function for streaming applications.
     54    void process(float* destination, size_t framesToProcess, const Function<void(AudioBus* bus, size_t framesToProcess)>& provideInput);
    5355
    5456protected:
    5557    void initializeKernel();
    56     void consumeSource(float* buffer, unsigned numberOfSourceFrames);
     58    void consumeSource(float* buffer, unsigned numberOfSourceFrames, const Function<void(AudioBus* bus, size_t framesToProcess)>& provideInput);
    5759    void updateRegions(bool isSecondLoad);
    5860   
    … …  
    7375    unsigned m_blockSize { 0 };
    7476
     77    size_t m_chunkSize { 0 };
     78
    7579    // Source is copied into this buffer for each processing pass.
    7680    AudioFloatArray m_inputBuffer;
    … …  
    8690    const float* m_source { nullptr };
    8791    unsigned m_sourceFramesAvailable { 0 };
    88    
    89     // m_sourceProvider is used to provide the audio input stream to the resampler.
    90     AudioSourceProvider* m_sourceProvider { nullptr };
    9192
    9293    // The buffer is primed once at the very beginning of processing.
Note: See TracChangeset for help on using the changeset viewer.