Changeset 270157 in webkit


Ignore:
Timestamp:
Nov 21, 2020 8:51:13 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

    r270155 r270157  
     12020-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
    1152020-11-21  Chris Dumez  <cdumez@apple.com>
    216
  • trunk/Source/WebCore/ChangeLog

    r270156 r270157  
     12020-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
    1382020-11-21  Simon Fraser  <simon.fraser@apple.com>
    239
  • trunk/Source/WebCore/platform/audio/MultiChannelResampler.cpp

    r270155 r270157  
    3939// ChannelProvider provides a single channel of audio data (one channel at a time) for each channel
    4040// of data provided to us in a multi-channel provider.
    41 class MultiChannelResampler::ChannelProvider : public AudioSourceProvider {
     41class MultiChannelResampler::ChannelProvider {
    4242    WTF_MAKE_FAST_ALLOCATED;
    4343public:
    44     explicit ChannelProvider(unsigned numberOfChannels)
    45         : m_numberOfChannels(numberOfChannels)
     44    explicit ChannelProvider(unsigned numberOfChannels, unsigned requestFrames)
     45        : m_multiChannelBus(AudioBus::create(numberOfChannels, requestFrames))
    4646    {
    4747    }
     
    4949    void setProvider(AudioSourceProvider* multiChannelProvider)
    5050    {
    51         m_currentChannel = 0;
    52         m_framesToProcess = 0;
    5351        m_multiChannelProvider = multiChannelProvider;
    5452    }
     
    5654    // provideInput() will be called once for each channel, starting with the first channel.
    5755    // Each time it's called, it will provide the next channel of data.
    58     void provideInput(AudioBus* bus, size_t framesToProcess) override
     56    void provideInputForChannel(AudioBus* bus, size_t framesToProcess, unsigned channelIndex)
    5957    {
     58        ASSERT(channelIndex < m_multiChannelBus->numberOfChannels());
     59        ASSERT(framesToProcess <= m_multiChannelBus->length());
     60        if (framesToProcess > m_multiChannelBus->length())
     61            return;
     62
    6063        bool isBusGood = bus && bus->numberOfChannels() == 1;
    6164        ASSERT(isBusGood);
     
    6568        // Get the data from the multi-channel provider when the first channel asks for it.
    6669        // For subsequent channels, we can just dish out the channel data from that (stored in m_multiChannelBus).
    67         if (!m_currentChannel) {
     70        if (!channelIndex) {
    6871            m_framesToProcess = framesToProcess;
    69             m_multiChannelBus = AudioBus::create(m_numberOfChannels, framesToProcess);
    7072            m_multiChannelProvider->provideInput(m_multiChannelBus.get(), framesToProcess);
    7173        }
    7274
    7375        // 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;
    7577        ASSERT(isGood);
    7678        if (!isGood)
     
    7880
    7981        // 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);
    8583    }
    8684
     
    8886    AudioSourceProvider* m_multiChannelProvider { nullptr };
    8987    RefPtr<AudioBus> m_multiChannelBus;
    90     unsigned m_numberOfChannels { 0 };
    91     unsigned m_currentChannel { 0 };
    9288    size_t m_framesToProcess { 0 }; // Used to verify that all channels ask for the same amount.
    9389};
    9490
    95 MultiChannelResampler::MultiChannelResampler(double scaleFactor, unsigned numberOfChannels, Optional<unsigned> requestFrames)
     91MultiChannelResampler::MultiChannelResampler(double scaleFactor, unsigned numberOfChannels, unsigned requestFrames)
    9692    : m_numberOfChannels(numberOfChannels)
    97     , m_channelProvider(makeUnique<ChannelProvider>(m_numberOfChannels))
     93    , m_channelProvider(makeUnique<ChannelProvider>(m_numberOfChannels, requestFrames))
    9894{
    9995    // Create each channel's resampler.
     
    113109    m_channelProvider->setProvider(provider);
    114110
    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;
    123130    }
    124131
  • trunk/Source/WebCore/platform/audio/MultiChannelResampler.h

    r270155 r270157  
    4242public:   
    4343    // 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);
    4545    ~MultiChannelResampler();
    4646
     
    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

    r270155 r270157  
    112112namespace WebCore {
    113113
    114 constexpr unsigned defaultRequestFrames { 512 };
    115114constexpr unsigned kernelSize { 32 };
    116115constexpr unsigned numberOfKernelOffsets { 32 };
    117116
    118 SincResampler::SincResampler(double scaleFactor, Optional<unsigned> requestFrames)
     117static size_t calculateChunkSize(unsigned blockSize, double scaleFactor)
     118{
     119    return blockSize / scaleFactor;
     120}
     121
     122SincResampler::SincResampler(double scaleFactor, unsigned requestFrames)
    119123    : m_scaleFactor(scaleFactor)
    120124    , m_kernelStorage(kernelSize * (numberOfKernelOffsets + 1))
    121     , m_requestFrames(requestFrames.valueOr(defaultRequestFrames))
     125    , m_requestFrames(requestFrames)
    122126    , m_inputBuffer(m_requestFrames + kernelSize) // See input buffer layout above.
    123127    , m_r1(m_inputBuffer.data())
     
    138142    m_r4 = m_r0 + m_requestFrames - kernelSize / 2;
    139143    m_blockSize = m_r4 - m_r2;
     144    m_chunkSize = calculateChunkSize(m_blockSize, m_scaleFactor);
    140145
    141146    // m_r1 at the beginning of the buffer.
     
    188193}
    189194
    190 void SincResampler::consumeSource(float* buffer, unsigned numberOfSourceFrames)
    191 {
    192     ASSERT(m_sourceProvider);
    193     if (!m_sourceProvider)
     195void SincResampler::consumeSource(float* buffer, unsigned numberOfSourceFrames, const Function<void(AudioBus* bus, size_t framesToProcess)>& provideInput)
     196{
     197    ASSERT(provideInput);
     198    if (!provideInput)
    194199        return;
    195200
     
    201206    m_internalBus->setChannelMemory(0, buffer, numberOfSourceFrames);
    202207   
    203     m_sourceProvider->provideInput(m_internalBus.get(), numberOfSourceFrames);
     208    provideInput(m_internalBus.get(), numberOfSourceFrames);
    204209}
    205210
    206211namespace {
    207212
    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
     215class BufferSourceProvider {
    211216public:
    212217    explicit BufferSourceProvider(const float* source, size_t numberOfSourceFrames)
     
    217222   
    218223    // Consumes samples from the in-memory buffer.
    219     void provideInput(AudioBus* bus, size_t framesToProcess) override
     224    void provideInput(AudioBus* bus, size_t framesToProcess)
    220225    {
    221226        ASSERT(m_source && bus);
     
    254259    while (remaining) {
    255260        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        });
    257264       
    258265        destination += framesThisTime;
     
    261268}
    262269
    263 void SincResampler::process(AudioSourceProvider* sourceProvider, float* destination, size_t framesToProcess)
    264 {
    265     ASSERT(sourceProvider);
    266     if (!sourceProvider)
     270void SincResampler::process(float* destination, size_t framesToProcess, const Function<void(AudioBus* bus, size_t framesToProcess)>& provideInput)
     271{
     272    ASSERT(provideInput);
     273    if (!provideInput)
    267274        return;
    268    
    269     m_sourceProvider = sourceProvider;
    270275
    271276    unsigned numberOfDestinationFrames = framesToProcess;
     
    274279    // Prime the input buffer at the start of the input stream.
    275280    if (!m_isBufferPrimed) {
    276         consumeSource(m_r0, m_requestFrames);
     281        consumeSource(m_r0, m_requestFrames, provideInput);
    277282        m_isBufferPrimed = true;
    278283    }
     
    522527        // Step (5)
    523528        // Refresh the buffer with more input.
    524         consumeSource(m_r0, m_requestFrames);
     529        consumeSource(m_r0, m_requestFrames, provideInput);
    525530    }
    526531}
  • trunk/Source/WebCore/platform/audio/SincResampler.h

    r270155 r270157  
    4242    WTF_MAKE_FAST_ALLOCATED;
    4343public:
     44    static constexpr unsigned defaultRequestFrames { 512 };
     45
    4446    // scaleFactor == sourceSampleRate / destinationSampleRate
    4547    // 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);
    4749   
     50    size_t chunkSize() const { return m_chunkSize; }
     51
    4852    // Processes numberOfSourceFrames from source to produce numberOfSourceFrames / scaleFactor frames in destination.
    4953    void process(const float* source, float* destination, unsigned numberOfSourceFrames);
    5054
    51     // Process with input source callback 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);
    5357
    5458protected:
    5559    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);
    5761    void updateRegions(bool isSecondLoad);
    5862   
     
    7377    unsigned m_blockSize { 0 };
    7478
     79    size_t m_chunkSize { 0 };
     80
    7581    // Source is copied into this buffer for each processing pass.
    7682    AudioFloatArray m_inputBuffer;
     
    8692    const float* m_source { nullptr };
    8793    unsigned m_sourceFramesAvailable { 0 };
    88    
    89     // m_sourceProvider is used to provide the audio input stream to the resampler.
    90     AudioSourceProvider* m_sourceProvider { nullptr };
    9194
    9295    // The buffer is primed once at the very beginning of processing.
Note: See TracChangeset for help on using the changeset viewer.