Changeset 269108 in webkit


Ignore:
Timestamp:
Oct 28, 2020 10:40:03 AM (21 months ago)
Author:
Chris Dumez
Message:

AudioBuffer channels should be neuterable / detachable
​https://bugs.webkit.org/show_bug.cgi?id=218286

Reviewed by Eric Carlson.

LayoutTests/imported/w3c:

Rebaseline WPT test that is now passing. This test is also passing in Firefox. It is crashing in Blink.

  • web-platform-tests/webaudio/the-audio-api/the-convolvernode-interface/transferred-buffer-output-expected.txt:

Source/WebCore:

AudioBuffer channels should be neuterable / detachable:

When the one of the channels' buffers gets detached/neutered, the array returned by
getChannelData(0) is neutered and thus has zero length. Internally though, if
channelData() gets called and ANY of the channels' buffers are detached, we return
a new empty array, as per the specification. This makes sure we end up with silence
on all channels when any of the channels gets detached, which is consistent with the
specification and Firefox (Blink seems to crash).

To make the AudioBuffer API less error-prone when used natively, I updated length()
and duration() to return 0 whenever any of the channels is detached. This is safer
since channelData() returns 0-length arrays in this case. The Web API keeps returning
the original values though so they rely on new originalLength() / originalDuration()
getters.

No new tests, rebaselined / updated existing tests.

  • Modules/webaudio/AudioBuffer.cpp:

(WebCore::AudioBuffer::create):
(WebCore::AudioBuffer::AudioBuffer):
(WebCore::AudioBuffer::invalidate):
(WebCore::AudioBuffer::channelData):
(WebCore::AudioBuffer::zero):
(WebCore::AudioBuffer::hasDetachedChannelBuffer const):

  • Modules/webaudio/AudioBuffer.h:

(WebCore::AudioBuffer::originalLength const):
(WebCore::AudioBuffer::originalDuration const):
(WebCore::AudioBuffer::length const):
(WebCore::AudioBuffer::duration const):

  • Modules/webaudio/AudioBuffer.idl:
  • Modules/webaudio/ConvolverNode.cpp:

(WebCore::ConvolverNode::setBuffer):

  • Modules/webaudio/ScriptProcessorNode.cpp:

(WebCore::ScriptProcessorNode::initialize):

LayoutTests:

  • webaudio/audiobuffer-neuter-expected.txt:
  • webaudio/audiobuffer-neuter.html:

Extend layout test coverage. I have verified that this new version of the test is fully passing in
Gecko. In Blink, the initial checks all pass but it then crashes during rendering.

  • webaudio/resources/audio-testing.js:

(createConstantBuffer):
Improve createConstantBuffer() so that it can construct buffers with multiple channels.

Location:
trunk
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r269104 r269108  
     12020-10-28  Chris Dumez  <cdumez@apple.com>
     2
     3        AudioBuffer channels should be neuterable / detachable
     4        https://bugs.webkit.org/show_bug.cgi?id=218286
     5
     6        Reviewed by Eric Carlson.
     7
     8        * webaudio/audiobuffer-neuter-expected.txt:
     9        * webaudio/audiobuffer-neuter.html:
     10        Extend layout test coverage. I have verified that this new version of the test is fully passing in
     11        Gecko. In Blink, the initial checks all pass but it then crashes during rendering.
     12
     13        * webaudio/resources/audio-testing.js:
     14        (createConstantBuffer):
     15        Improve createConstantBuffer() so that it can construct buffers with multiple channels.
     16
    1172020-10-28  Philippe Normand  <pnormand@igalia.com>
    218

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r269081 r269108  
     12020-10-28  Chris Dumez  <cdumez@apple.com>
     2
     3        AudioBuffer channels should be neuterable / detachable
     4        https://bugs.webkit.org/show_bug.cgi?id=218286
     5
     6        Reviewed by Eric Carlson.
     7
     8        Rebaseline WPT test that is now passing. This test is also passing in Firefox. It is crashing in Blink.
     9
     10        * web-platform-tests/webaudio/the-audio-api/the-convolvernode-interface/transferred-buffer-output-expected.txt:
     11
    1122020-10-27  Chris Dumez  <cdumez@apple.com>
    213

  • trunk/LayoutTests/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-convolvernode-interface/transferred-buffer-output-expected.txt

    r268685 r269108  
    44PASS Audit report
    55PASS > [Test Convolver with transferred buffer] Output should be all zeroes
    6 FAIL X Convolver channel 0 output[0:1279]: Expected 0 for all values but found 1280 unexpected values:
    7         Index   Actual
    8         [0]     1
    9         [1]     2
    10         [2]     3
    11         [3]     4
    12         ...and 1276 more errors. assert_true: expected true got false
    13 FAIL X Convolver channel 1 output[0:1279]: Expected 0 for all values but found 1280 unexpected values:
    14         Index   Actual
    15         [0]     2
    16         [1]     4
    17         [2]     6
    18         [3]     8
    19         ...and 1276 more errors. assert_true: expected true got false
    20 FAIL < [Test Convolver with transferred buffer] 2 out of 2 assertions were failed. assert_true: expected true got false
    21 FAIL # AUDIT TASK RUNNER FINISHED: 1 out of 1 tasks were failed. assert_true: expected true got false
     6PASS   Convolver channel 0 output[0:1279] contains only the constant 0.
     7PASS   Convolver channel 1 output[0:1279] contains only the constant 0.
     8PASS < [Test Convolver with transferred buffer] All assertions passed. (total 2 assertions)
     9PASS # AUDIT TASK RUNNER FINISHED: 1 tasks ran successfully.
    2210

  • trunk/LayoutTests/webaudio/audiobuffer-neuter-expected.txt

    r217243 r269108  
    1 Tests that neutered AudioBuffers do not produce garbage
     1Tests that neutered AudioBuffers produce silence
     2PASS sourceBuffer.length is numberOfFrames
     3PASS sourceBuffer.getChannelData(0).length is 0
     4PASS sourceBuffer.getChannelData(0) === sourceBuffer.getChannelData(0) is true
     5PASS sourceBuffer.getChannelData(0) === originalFirstChannel is true
     6PASS sourceBuffer.getChannelData(1).length is numberOfFrames
     7PASS sourceBuffer.getChannelData(1) === sourceBuffer.getChannelData(1) is true
     8PASS sourceBuffer.getChannelData(1) === originalSecondChannel is true
    29PASS Output matches expectations.
    310PASS successfullyParsed is true

  • trunk/LayoutTests/webaudio/audiobuffer-neuter.html

    r267504 r269108  
    1515            jsTestIsAsync = true;
    1616
    17             var numberOfFrames = sampleRate * lengthInSeconds;
    18             context = new OfflineAudioContext(1, numberOfFrames, sampleRate);
    19             sourceBuffer = createConstantBuffer(context, numberOfFrames, 0.5);
     17            numberOfFrames = sampleRate * lengthInSeconds;
     18            context = new OfflineAudioContext(2, numberOfFrames, sampleRate);
     19            sourceBuffer = createConstantBuffer(context, numberOfFrames, [0.5, 0.25]);
     20            originalFirstChannel = sourceBuffer.getChannelData(0);
     21            originalSecondChannel = sourceBuffer.getChannelData(1);
    2022            var data = sourceBuffer.getChannelData(0).buffer;
    2123
    … …  
    3335            worker.onmessage = workerReply;
    3436            worker.postMessage(data, [data]);
     37
     38            // The length of the AudioBuffer should be unchanged, even if some of its audio channels buffers
     39            // have been detached.
     40            shouldBe("sourceBuffer.length", "numberOfFrames");
     41
     42            // getChannelData() should return a 0-length array if the channel buffer has been detached.
     43            shouldBe("sourceBuffer.getChannelData(0).length", "0");
     44            shouldBeTrue("sourceBuffer.getChannelData(0) === sourceBuffer.getChannelData(0)");
     45            shouldBeTrue("sourceBuffer.getChannelData(0) === originalFirstChannel");
     46
     47            // getChannelData() should return accurate channel data for non detached channels.
     48            shouldBe("sourceBuffer.getChannelData(1).length", "numberOfFrames");
     49            shouldBeTrue("sourceBuffer.getChannelData(1) === sourceBuffer.getChannelData(1)");
     50            shouldBeTrue("sourceBuffer.getChannelData(1) === originalSecondChannel");
     51            secondChannel = sourceBuffer.getChannelData(1);
     52            for (var i = 0; i < numberOfFrames; ++i) {
     53                if (secondChannel[i] != 0.25) {
     54                    testFailed("Second channel contained unexpected value: " + secondChannel[i]);
     55                    break;
     56                }
     57            }
    3558        }
    3659
    … …  
    4770            var renderedBuffer = event.renderedBuffer;
    4871            var numberOfFrames = sampleRate * lengthInSeconds;
    49             var expectedBuffer = createConstantBuffer(context, numberOfFrames, 0.5);
     72            // We expect silence on both channels.
     73            var expectedBuffer = createConstantBuffer(context, numberOfFrames, [0, 0]);
    5074
    51             var renderedData = renderedBuffer.getChannelData(0);
    52             var expectedData = expectedBuffer.getChannelData(0);
     75            for (let channelIndex = 0; channelIndex < 2; channelIndex++) {
     76                var renderedData = renderedBuffer.getChannelData(channelIndex);
     77                var expectedData = expectedBuffer.getChannelData(channelIndex);
    5378
    54             for (var i = 0; i < numberOfFrames; ++i) {
    55                 if (expectedData[i] != renderedData[i]) {
    56                     testFailed('expected: ' + expectedData[i] + ' actual: ' + renderedData[i]);
    57                     finishJSTest();
     79                for (var i = 0; i < numberOfFrames; ++i) {
     80                    if (expectedData[i] != renderedData[i]) {
     81                        testFailed('Channel ' + channelIndex + ' - expected: ' + expectedData[i] + ' actual: ' + renderedData[i]);
     82                        finishJSTest();
     83                    }
    5884                }
    5985            }
    … …  
    6692</head>
    6793<body onload="runTest()">
    68     <div>Tests that neutered AudioBuffers do not produce garbage</div>
     94    <div>Tests that neutered AudioBuffers produce silence</div>
    6995    <div id="console"></div>
    7096</body>

  • trunk/LayoutTests/webaudio/resources/audio-testing.js

    r205065 r269108  
    130130// Create a buffer of the given length having a constant value.
    131131function createConstantBuffer(context, sampleFrameLength, constantValue) {
    132     var audioBuffer = context.createBuffer(1, sampleFrameLength, context.sampleRate);
    133     var n = audioBuffer.length;
    134     var dataL = audioBuffer.getChannelData(0);
    135 
    136     for (var i = 0; i < n; ++i)
    137         dataL[i] = constantValue;
     132    let channels;
     133    let values;
     134
     135    if (typeof constantValue === 'number') {
     136        channels = 1;
     137        values = [constantValue];
     138    } else {
     139        channels = constantValue.length;
     140        values = constantValue;
     141    }
     142
     143    let audioBuffer = context.createBuffer(channels, sampleFrameLength, context.sampleRate);
     144    let n = audioBuffer.length;
     145
     146    for (let c = 0; c < channels; ++c) {
     147        let data = audioBuffer.getChannelData(c);
     148        for (let i = 0; i < n; ++i)
     149            data[i] = values[c];
     150    }
    138151
    139152    return audioBuffer;

  • trunk/Source/WebCore/ChangeLog

    r269104 r269108  
     12020-10-28  Chris Dumez  <cdumez@apple.com>
     2
     3        AudioBuffer channels should be neuterable / detachable
     4        https://bugs.webkit.org/show_bug.cgi?id=218286
     5
     6        Reviewed by Eric Carlson.
     7
     8        AudioBuffer channels should be neuterable / detachable:
     9        - https://webaudio.github.io/web-audio-api/#acquire-the-content
     10
     11        When the one of the channels' buffers gets detached/neutered, the array returned by
     12        getChannelData(0) is neutered and thus has zero length. Internally though, if
     13        channelData() gets called and ANY of the channels' buffers are detached, we return
     14        a new empty array, as per the specification. This makes sure we end up with silence
     15        on all channels when any of the channels gets detached, which is consistent with the
     16        specification and Firefox (Blink seems to crash).
     17
     18        To make the AudioBuffer API less error-prone when used natively, I updated length()
     19        and duration() to return 0 whenever any of the channels is detached. This is safer
     20        since channelData() returns 0-length arrays in this case. The Web API keeps returning
     21        the original values though so they rely on new originalLength() / originalDuration()
     22        getters.
     23
     24        No new tests, rebaselined / updated existing tests.
     25
     26        * Modules/webaudio/AudioBuffer.cpp:
     27        (WebCore::AudioBuffer::create):
     28        (WebCore::AudioBuffer::AudioBuffer):
     29        (WebCore::AudioBuffer::invalidate):
     30        (WebCore::AudioBuffer::channelData):
     31        (WebCore::AudioBuffer::zero):
     32        (WebCore::AudioBuffer::hasDetachedChannelBuffer const):
     33        * Modules/webaudio/AudioBuffer.h:
     34        (WebCore::AudioBuffer::originalLength const):
     35        (WebCore::AudioBuffer::originalDuration const):
     36        (WebCore::AudioBuffer::length const):
     37        (WebCore::AudioBuffer::duration const):
     38        * Modules/webaudio/AudioBuffer.idl:
     39        * Modules/webaudio/ConvolverNode.cpp:
     40        (WebCore::ConvolverNode::setBuffer):
     41        * Modules/webaudio/ScriptProcessorNode.cpp:
     42        (WebCore::ScriptProcessorNode::initialize):
     43
    1442020-10-28  Philippe Normand  <pnormand@igalia.com>
    245

  • trunk/Source/WebCore/Modules/webaudio/AudioBuffer.cpp

    r269081 r269108  
    4141namespace WebCore {
    4242
    43 RefPtr<AudioBuffer> AudioBuffer::create(unsigned numberOfChannels, size_t numberOfFrames, float sampleRate)
     43RefPtr<AudioBuffer> AudioBuffer::create(unsigned numberOfChannels, size_t numberOfFrames, float sampleRate, LegacyPreventNeutering preventNeutering)
    4444{
    4545    if (!BaseAudioContext::isSupportedSampleRate(sampleRate) || !numberOfChannels || numberOfChannels > AudioContext::maxNumberOfChannels() || !numberOfFrames)
    4646        return nullptr;
    4747
    48     auto buffer = adoptRef(*new AudioBuffer(numberOfChannels, numberOfFrames, sampleRate));
    49     if (!buffer->m_length)
     48    auto buffer = adoptRef(*new AudioBuffer(numberOfChannels, numberOfFrames, sampleRate, preventNeutering));
     49    if (!buffer->originalLength())
    5050        return nullptr;
    5151
    … …  
    6868   
    6969    auto buffer = adoptRef(*new AudioBuffer(options.numberOfChannels, options.length, options.sampleRate));
    70     if (!buffer->length())
     70    if (!buffer->originalLength())
    7171        return Exception { NotSupportedError, "Channel was not able to be created."_s };
    7272   
    … …  
    8282}
    8383
    84 AudioBuffer::AudioBuffer(unsigned numberOfChannels, size_t length, float sampleRate)
     84AudioBuffer::AudioBuffer(unsigned numberOfChannels, size_t length, float sampleRate, LegacyPreventNeutering preventNeutering)
    8585    : m_sampleRate(sampleRate)
    86     , m_length(length)
     86    , m_originalLength(length)
    8787{
    8888    m_channels.reserveCapacity(numberOfChannels);
    8989
    9090    for (unsigned i = 0; i < numberOfChannels; ++i) {
    91         auto channelDataArray = Float32Array::tryCreate(m_length);
     91        auto channelDataArray = Float32Array::tryCreate(m_originalLength);
    9292        if (!channelDataArray) {
    9393            invalidate();
    … …  
    9595        }
    9696
    97         channelDataArray->setNeuterable(false);
     97        if (preventNeutering == LegacyPreventNeutering::Yes)
     98            channelDataArray->setNeuterable(false);
     99
    98100        m_channels.append(WTFMove(channelDataArray));
    99101    }
    … …  
    103105AudioBuffer::AudioBuffer(AudioBus& bus)
    104106    : m_sampleRate(bus.sampleRate())
    105     , m_length(bus.length())
     107    , m_originalLength(bus.length())
    106108{
    107109    // Copy audio data from the bus to the Float32Arrays we manage.
    … …  
    109111    m_channels.reserveCapacity(numberOfChannels);
    110112    for (unsigned i = 0; i < numberOfChannels; ++i) {
    111         auto channelDataArray = Float32Array::tryCreate(m_length);
     113        auto channelDataArray = Float32Array::tryCreate(m_originalLength);
    112114        if (!channelDataArray) {
    113115            invalidate();
    … …  
    115117        }
    116118
    117         channelDataArray->setNeuterable(false);
    118         channelDataArray->setRange(bus.channel(i)->data(), m_length, 0);
     119        channelDataArray->setRange(bus.channel(i)->data(), m_originalLength, 0);
    119120        m_channels.append(WTFMove(channelDataArray));
    120121    }
    … …  
    125126{
    126127    releaseMemory();
    127     m_length = 0;
     128    m_originalLength = 0;
    128129}
    129130
    … …  
    160161}
    161162
    162 Float32Array* AudioBuffer::channelData(unsigned channelIndex)
     163RefPtr<Float32Array> AudioBuffer::channelData(unsigned channelIndex)
    163164{
    164165    if (channelIndex >= m_channels.size())
    165166        return nullptr;
    166     return m_channels[channelIndex].get();
     167    if (hasDetachedChannelBuffer())
     168        return Float32Array::create(0);
     169    return m_channels[channelIndex].copyRef();
    167170}
    168171
    … …  
    226229{
    227230    for (auto& channel : m_channels)
    228         channel->zeroRange(0, length());
     231        channel->zeroFill();
    229232}
    230233
    … …  
    243246}
    244247
     248bool AudioBuffer::hasDetachedChannelBuffer() const
     249{
     250    for (auto& channel : m_channels) {
     251        if (channel->isNeutered())
     252            return true;
     253    }
     254    return false;
     255}
     256
    245257} // namespace WebCore
    246258

  • trunk/Source/WebCore/Modules/webaudio/AudioBuffer.h

    r269081 r269108  
    4242
    4343class AudioBuffer : public RefCounted<AudioBuffer> {
    44 public:   
    45     static RefPtr<AudioBuffer> create(unsigned numberOfChannels, size_t numberOfFrames, float sampleRate);
     44public:
     45    enum class LegacyPreventNeutering : bool { No, Yes };
     46    static RefPtr<AudioBuffer> create(unsigned numberOfChannels, size_t numberOfFrames, float sampleRate, LegacyPreventNeutering = LegacyPreventNeutering::No);
    4647    static ExceptionOr<Ref<AudioBuffer>> create(const AudioBufferOptions&);
    4748    // Returns nullptr if data is not a valid audio file.
    … …  
    4950
    5051    // Format
    51     size_t length() const { return m_length; }
     52    size_t originalLength() const { return m_originalLength; }
     53    double originalDuration() const { return originalLength() / static_cast<double>(sampleRate()); }
     54    float sampleRate() const { return m_sampleRate; }
     55
     56    // The following function may start returning 0 if any of the underlying channel buffers gets detached.
     57    size_t length() const { return hasDetachedChannelBuffer() ? 0 : m_originalLength; }
    5258    double duration() const { return length() / static_cast<double>(sampleRate()); }
    53     float sampleRate() const { return m_sampleRate; }
    5459
    5560    // Channel data access
    … …  
    5863    ExceptionOr<void> copyFromChannel(Ref<Float32Array>&&, unsigned channelNumber, unsigned bufferOffset);
    5964    ExceptionOr<void> copyToChannel(Ref<Float32Array>&&, unsigned channelNumber, unsigned startInChannel);
    60     Float32Array* channelData(unsigned channelIndex);
     65
     66    // Native channel data access.
     67    RefPtr<Float32Array> channelData(unsigned channelIndex);
    6168    void zero();
    6269
    … …  
    7178   
    7279private:
    73     AudioBuffer(unsigned numberOfChannels, size_t length, float sampleRate);
     80    AudioBuffer(unsigned numberOfChannels, size_t length, float sampleRate, LegacyPreventNeutering = LegacyPreventNeutering::No);
    7481    explicit AudioBuffer(AudioBus&);
    7582
    7683    void invalidate();
    7784
     85    bool hasDetachedChannelBuffer() const;
     86
    7887    float m_sampleRate;
    7988    mutable Lock m_channelsLock;
    80     size_t m_length;
     89    size_t m_originalLength;
    8190    Vector<RefPtr<Float32Array>> m_channels;
    8291    Vector<JSValueInWrappedObject> m_channelWrappers;

  • trunk/Source/WebCore/Modules/webaudio/AudioBuffer.idl

    r269081 r269108  
    3636] interface AudioBuffer {
    3737    [EnabledBySetting=ModernUnprefixedWebAudio] constructor(AudioBufferOptions options);
    38     readonly attribute unsigned long length; // in sample-frames
    39     readonly attribute double duration; // in seconds
     38    [ImplementedAs=originalLength] readonly attribute unsigned long length; // in sample-frames
     39    [ImplementedAs=originalDuration] readonly attribute double duration; // in seconds
    4040    readonly attribute float sampleRate; // in sample-frames per second
    4141    readonly attribute unsigned long numberOfChannels;

  • trunk/Source/WebCore/Modules/webaudio/ConvolverNode.cpp

    r268812 r269108  
    131131        return Exception { NotSupportedError, "Buffer should have 1, 2 or 4 channels"_s };
    132132
    133     if (!bufferLength)
    134         return Exception { NotSupportedError, "Buffer length cannot be 0"_s };
    135 
    136133    // Wrap the AudioBuffer by an AudioBus. It's an efficient pointer set and not a memcpy().
    137134    // This memory is simply used in the Reverb constructor and no reference to it is kept for later use in that class.

  • trunk/Source/WebCore/Modules/webaudio/ScriptProcessorNode.cpp

    r268728 r269108  
    9191    // These AudioBuffers will be directly accessed in the main thread by JavaScript.
    9292    for (unsigned i = 0; i < 2; ++i) {
    93         auto inputBuffer = m_numberOfInputChannels ? AudioBuffer::create(m_numberOfInputChannels, bufferSize(), sampleRate) : 0;
    94         auto outputBuffer = m_numberOfOutputChannels ? AudioBuffer::create(m_numberOfOutputChannels, bufferSize(), sampleRate) : 0;
     93        // We prevent neutering the AudioBuffers here since we pass those to JS and reuse them.
     94        auto inputBuffer = m_numberOfInputChannels ? AudioBuffer::create(m_numberOfInputChannels, bufferSize(), sampleRate, AudioBuffer::LegacyPreventNeutering::Yes) : 0;
     95        auto outputBuffer = m_numberOfOutputChannels ? AudioBuffer::create(m_numberOfOutputChannels, bufferSize(), sampleRate, AudioBuffer::LegacyPreventNeutering::Yes) : 0;
    9596
    9697        m_inputBuffers.append(inputBuffer);
Note: See TracChangeset for help on using the changeset viewer.