Diff
Modified: trunk/LayoutTests/ChangeLog (269107 => 269108)
--- trunk/LayoutTests/ChangeLog 2020-10-28 17:32:26 UTC (rev 269107)
+++ trunk/LayoutTests/ChangeLog 2020-10-28 17:40:03 UTC (rev 269108)
@@ -1,3 +1,19 @@
+2020-10-28 Chris Dumez <[email protected]>
+
+ AudioBuffer channels should be neuterable / detachable
+ https://bugs.webkit.org/show_bug.cgi?id=218286
+
+ Reviewed by Eric Carlson.
+
+ * 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.
+
2020-10-28 Philippe Normand <[email protected]>
[GStreamer] Multi-channel (>2) support for the AudioFileReader
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (269107 => 269108)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2020-10-28 17:32:26 UTC (rev 269107)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2020-10-28 17:40:03 UTC (rev 269108)
@@ -1,3 +1,14 @@
+2020-10-28 Chris Dumez <[email protected]>
+
+ AudioBuffer channels should be neuterable / detachable
+ https://bugs.webkit.org/show_bug.cgi?id=218286
+
+ Reviewed by Eric Carlson.
+
+ 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:
+
2020-10-27 Chris Dumez <[email protected]>
AudioBuffer.getChannelData(x) should keep returning the same JS wrapper for a given channel
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-convolvernode-interface/transferred-buffer-output-expected.txt (269107 => 269108)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-convolvernode-interface/transferred-buffer-output-expected.txt 2020-10-28 17:32:26 UTC (rev 269107)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-convolvernode-interface/transferred-buffer-output-expected.txt 2020-10-28 17:40:03 UTC (rev 269108)
@@ -3,20 +3,8 @@
PASS Executing "Test Convolver with transferred buffer"
PASS Audit report
PASS > [Test Convolver with transferred buffer] Output should be all zeroes
-FAIL X Convolver channel 0 output[0:1279]: Expected 0 for all values but found 1280 unexpected values:
- Index Actual
- [0] 1
- [1] 2
- [2] 3
- [3] 4
- ...and 1276 more errors. assert_true: expected true got false
-FAIL X Convolver channel 1 output[0:1279]: Expected 0 for all values but found 1280 unexpected values:
- Index Actual
- [0] 2
- [1] 4
- [2] 6
- [3] 8
- ...and 1276 more errors. assert_true: expected true got false
-FAIL < [Test Convolver with transferred buffer] 2 out of 2 assertions were failed. assert_true: expected true got false
-FAIL # AUDIT TASK RUNNER FINISHED: 1 out of 1 tasks were failed. assert_true: expected true got false
+PASS Convolver channel 0 output[0:1279] contains only the constant 0.
+PASS Convolver channel 1 output[0:1279] contains only the constant 0.
+PASS < [Test Convolver with transferred buffer] All assertions passed. (total 2 assertions)
+PASS # AUDIT TASK RUNNER FINISHED: 1 tasks ran successfully.
Modified: trunk/LayoutTests/webaudio/audiobuffer-neuter-expected.txt (269107 => 269108)
--- trunk/LayoutTests/webaudio/audiobuffer-neuter-expected.txt 2020-10-28 17:32:26 UTC (rev 269107)
+++ trunk/LayoutTests/webaudio/audiobuffer-neuter-expected.txt 2020-10-28 17:40:03 UTC (rev 269108)
@@ -1,4 +1,11 @@
-Tests that neutered AudioBuffers do not produce garbage
+Tests that neutered AudioBuffers produce silence
+PASS sourceBuffer.length is numberOfFrames
+PASS sourceBuffer.getChannelData(0).length is 0
+PASS sourceBuffer.getChannelData(0) === sourceBuffer.getChannelData(0) is true
+PASS sourceBuffer.getChannelData(0) === originalFirstChannel is true
+PASS sourceBuffer.getChannelData(1).length is numberOfFrames
+PASS sourceBuffer.getChannelData(1) === sourceBuffer.getChannelData(1) is true
+PASS sourceBuffer.getChannelData(1) === originalSecondChannel is true
PASS Output matches expectations.
PASS successfullyParsed is true
Modified: trunk/LayoutTests/webaudio/audiobuffer-neuter.html (269107 => 269108)
--- trunk/LayoutTests/webaudio/audiobuffer-neuter.html 2020-10-28 17:32:26 UTC (rev 269107)
+++ trunk/LayoutTests/webaudio/audiobuffer-neuter.html 2020-10-28 17:40:03 UTC (rev 269108)
@@ -14,9 +14,11 @@
{
jsTestIsAsync = true;
- var numberOfFrames = sampleRate * lengthInSeconds;
- context = new OfflineAudioContext(1, numberOfFrames, sampleRate);
- sourceBuffer = createConstantBuffer(context, numberOfFrames, 0.5);
+ numberOfFrames = sampleRate * lengthInSeconds;
+ context = new OfflineAudioContext(2, numberOfFrames, sampleRate);
+ sourceBuffer = createConstantBuffer(context, numberOfFrames, [0.5, 0.25]);
+ originalFirstChannel = sourceBuffer.getChannelData(0);
+ originalSecondChannel = sourceBuffer.getChannelData(1);
var data = ""
var string = [
@@ -32,6 +34,27 @@
var worker = new Worker(blobURL);
worker._onmessage_ = workerReply;
worker.postMessage(data, [data]);
+
+ // The length of the AudioBuffer should be unchanged, even if some of its audio channels buffers
+ // have been detached.
+ shouldBe("sourceBuffer.length", "numberOfFrames");
+
+ // getChannelData() should return a 0-length array if the channel buffer has been detached.
+ shouldBe("sourceBuffer.getChannelData(0).length", "0");
+ shouldBeTrue("sourceBuffer.getChannelData(0) === sourceBuffer.getChannelData(0)");
+ shouldBeTrue("sourceBuffer.getChannelData(0) === originalFirstChannel");
+
+ // getChannelData() should return accurate channel data for non detached channels.
+ shouldBe("sourceBuffer.getChannelData(1).length", "numberOfFrames");
+ shouldBeTrue("sourceBuffer.getChannelData(1) === sourceBuffer.getChannelData(1)");
+ shouldBeTrue("sourceBuffer.getChannelData(1) === originalSecondChannel");
+ secondChannel = sourceBuffer.getChannelData(1);
+ for (var i = 0; i < numberOfFrames; ++i) {
+ if (secondChannel[i] != 0.25) {
+ testFailed("Second channel contained unexpected value: " + secondChannel[i]);
+ break;
+ }
+ }
}
function workerReply(event) {
@@ -46,15 +69,18 @@
function finishTest(event) {
var renderedBuffer = event.renderedBuffer;
var numberOfFrames = sampleRate * lengthInSeconds;
- var expectedBuffer = createConstantBuffer(context, numberOfFrames, 0.5);
+ // We expect silence on both channels.
+ var expectedBuffer = createConstantBuffer(context, numberOfFrames, [0, 0]);
- var renderedData = renderedBuffer.getChannelData(0);
- var expectedData = expectedBuffer.getChannelData(0);
+ for (let channelIndex = 0; channelIndex < 2; channelIndex++) {
+ var renderedData = renderedBuffer.getChannelData(channelIndex);
+ var expectedData = expectedBuffer.getChannelData(channelIndex);
- for (var i = 0; i < numberOfFrames; ++i) {
- if (expectedData[i] != renderedData[i]) {
- testFailed('expected: ' + expectedData[i] + ' actual: ' + renderedData[i]);
- finishJSTest();
+ for (var i = 0; i < numberOfFrames; ++i) {
+ if (expectedData[i] != renderedData[i]) {
+ testFailed('Channel ' + channelIndex + ' - expected: ' + expectedData[i] + ' actual: ' + renderedData[i]);
+ finishJSTest();
+ }
}
}
@@ -65,7 +91,7 @@
</script>
</head>
<body _onload_="runTest()">
- <div>Tests that neutered AudioBuffers do not produce garbage</div>
+ <div>Tests that neutered AudioBuffers produce silence</div>
<div id="console"></div>
</body>
Modified: trunk/LayoutTests/webaudio/resources/audio-testing.js (269107 => 269108)
--- trunk/LayoutTests/webaudio/resources/audio-testing.js 2020-10-28 17:32:26 UTC (rev 269107)
+++ trunk/LayoutTests/webaudio/resources/audio-testing.js 2020-10-28 17:40:03 UTC (rev 269108)
@@ -129,13 +129,26 @@
// Create a buffer of the given length having a constant value.
function createConstantBuffer(context, sampleFrameLength, constantValue) {
- var audioBuffer = context.createBuffer(1, sampleFrameLength, context.sampleRate);
- var n = audioBuffer.length;
- var dataL = audioBuffer.getChannelData(0);
+ let channels;
+ let values;
- for (var i = 0; i < n; ++i)
- dataL[i] = constantValue;
+ if (typeof constantValue === 'number') {
+ channels = 1;
+ values = [constantValue];
+ } else {
+ channels = constantValue.length;
+ values = constantValue;
+ }
+ let audioBuffer = context.createBuffer(channels, sampleFrameLength, context.sampleRate);
+ let n = audioBuffer.length;
+
+ for (let c = 0; c < channels; ++c) {
+ let data = ""
+ for (let i = 0; i < n; ++i)
+ data[i] = values[c];
+ }
+
return audioBuffer;
}
Modified: trunk/Source/WebCore/ChangeLog (269107 => 269108)
--- trunk/Source/WebCore/ChangeLog 2020-10-28 17:32:26 UTC (rev 269107)
+++ trunk/Source/WebCore/ChangeLog 2020-10-28 17:40:03 UTC (rev 269108)
@@ -1,3 +1,46 @@
+2020-10-28 Chris Dumez <[email protected]>
+
+ AudioBuffer channels should be neuterable / detachable
+ https://bugs.webkit.org/show_bug.cgi?id=218286
+
+ Reviewed by Eric Carlson.
+
+ AudioBuffer channels should be neuterable / detachable:
+ - https://webaudio.github.io/web-audio-api/#acquire-the-content
+
+ 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):
+
2020-10-28 Philippe Normand <[email protected]>
[GStreamer] Multi-channel (>2) support for the AudioFileReader
Modified: trunk/Source/WebCore/Modules/webaudio/AudioBuffer.cpp (269107 => 269108)
--- trunk/Source/WebCore/Modules/webaudio/AudioBuffer.cpp 2020-10-28 17:32:26 UTC (rev 269107)
+++ trunk/Source/WebCore/Modules/webaudio/AudioBuffer.cpp 2020-10-28 17:40:03 UTC (rev 269108)
@@ -40,13 +40,13 @@
namespace WebCore {
-RefPtr<AudioBuffer> AudioBuffer::create(unsigned numberOfChannels, size_t numberOfFrames, float sampleRate)
+RefPtr<AudioBuffer> AudioBuffer::create(unsigned numberOfChannels, size_t numberOfFrames, float sampleRate, LegacyPreventNeutering preventNeutering)
{
if (!BaseAudioContext::isSupportedSampleRate(sampleRate) || !numberOfChannels || numberOfChannels > AudioContext::maxNumberOfChannels() || !numberOfFrames)
return nullptr;
- auto buffer = adoptRef(*new AudioBuffer(numberOfChannels, numberOfFrames, sampleRate));
- if (!buffer->m_length)
+ auto buffer = adoptRef(*new AudioBuffer(numberOfChannels, numberOfFrames, sampleRate, preventNeutering));
+ if (!buffer->originalLength())
return nullptr;
return buffer;
@@ -67,7 +67,7 @@
return Exception { NotSupportedError, "Sample rate is not in the supported range."_s };
auto buffer = adoptRef(*new AudioBuffer(options.numberOfChannels, options.length, options.sampleRate));
- if (!buffer->length())
+ if (!buffer->originalLength())
return Exception { NotSupportedError, "Channel was not able to be created."_s };
return buffer;
@@ -81,20 +81,22 @@
return adoptRef(*new AudioBuffer(*bus));
}
-AudioBuffer::AudioBuffer(unsigned numberOfChannels, size_t length, float sampleRate)
+AudioBuffer::AudioBuffer(unsigned numberOfChannels, size_t length, float sampleRate, LegacyPreventNeutering preventNeutering)
: m_sampleRate(sampleRate)
- , m_length(length)
+ , m_originalLength(length)
{
m_channels.reserveCapacity(numberOfChannels);
for (unsigned i = 0; i < numberOfChannels; ++i) {
- auto channelDataArray = Float32Array::tryCreate(m_length);
+ auto channelDataArray = Float32Array::tryCreate(m_originalLength);
if (!channelDataArray) {
invalidate();
break;
}
- channelDataArray->setNeuterable(false);
+ if (preventNeutering == LegacyPreventNeutering::Yes)
+ channelDataArray->setNeuterable(false);
+
m_channels.append(WTFMove(channelDataArray));
}
m_channelWrappers.resize(m_channels.size());
@@ -102,20 +104,19 @@
AudioBuffer::AudioBuffer(AudioBus& bus)
: m_sampleRate(bus.sampleRate())
- , m_length(bus.length())
+ , m_originalLength(bus.length())
{
// Copy audio data from the bus to the Float32Arrays we manage.
unsigned numberOfChannels = bus.numberOfChannels();
m_channels.reserveCapacity(numberOfChannels);
for (unsigned i = 0; i < numberOfChannels; ++i) {
- auto channelDataArray = Float32Array::tryCreate(m_length);
+ auto channelDataArray = Float32Array::tryCreate(m_originalLength);
if (!channelDataArray) {
invalidate();
break;
}
- channelDataArray->setNeuterable(false);
- channelDataArray->setRange(bus.channel(i)->data(), m_length, 0);
+ channelDataArray->setRange(bus.channel(i)->data(), m_originalLength, 0);
m_channels.append(WTFMove(channelDataArray));
}
m_channelWrappers.resize(m_channels.size());
@@ -124,7 +125,7 @@
void AudioBuffer::invalidate()
{
releaseMemory();
- m_length = 0;
+ m_originalLength = 0;
}
void AudioBuffer::releaseMemory()
@@ -159,11 +160,13 @@
channelWrapper.visit(visitor);
}
-Float32Array* AudioBuffer::channelData(unsigned channelIndex)
+RefPtr<Float32Array> AudioBuffer::channelData(unsigned channelIndex)
{
if (channelIndex >= m_channels.size())
return nullptr;
- return m_channels[channelIndex].get();
+ if (hasDetachedChannelBuffer())
+ return Float32Array::create(0);
+ return m_channels[channelIndex].copyRef();
}
ExceptionOr<void> AudioBuffer::copyFromChannel(Ref<Float32Array>&& destination, unsigned channelNumber, unsigned bufferOffset)
@@ -225,7 +228,7 @@
void AudioBuffer::zero()
{
for (auto& channel : m_channels)
- channel->zeroRange(0, length());
+ channel->zeroFill();
}
size_t AudioBuffer::memoryCost() const
@@ -242,6 +245,15 @@
return cost;
}
+bool AudioBuffer::hasDetachedChannelBuffer() const
+{
+ for (auto& channel : m_channels) {
+ if (channel->isNeutered())
+ return true;
+ }
+ return false;
+}
+
} // namespace WebCore
#endif // ENABLE(WEB_AUDIO)
Modified: trunk/Source/WebCore/Modules/webaudio/AudioBuffer.h (269107 => 269108)
--- trunk/Source/WebCore/Modules/webaudio/AudioBuffer.h 2020-10-28 17:32:26 UTC (rev 269107)
+++ trunk/Source/WebCore/Modules/webaudio/AudioBuffer.h 2020-10-28 17:40:03 UTC (rev 269108)
@@ -41,23 +41,30 @@
class AudioBus;
class AudioBuffer : public RefCounted<AudioBuffer> {
-public:
- static RefPtr<AudioBuffer> create(unsigned numberOfChannels, size_t numberOfFrames, float sampleRate);
+public:
+ enum class LegacyPreventNeutering : bool { No, Yes };
+ static RefPtr<AudioBuffer> create(unsigned numberOfChannels, size_t numberOfFrames, float sampleRate, LegacyPreventNeutering = LegacyPreventNeutering::No);
static ExceptionOr<Ref<AudioBuffer>> create(const AudioBufferOptions&);
// Returns nullptr if data is not a valid audio file.
static RefPtr<AudioBuffer> createFromAudioFileData(const void* data, size_t dataSize, bool mixToMono, float sampleRate);
// Format
- size_t length() const { return m_length; }
- double duration() const { return length() / static_cast<double>(sampleRate()); }
+ size_t originalLength() const { return m_originalLength; }
+ double originalDuration() const { return originalLength() / static_cast<double>(sampleRate()); }
float sampleRate() const { return m_sampleRate; }
+ // The following function may start returning 0 if any of the underlying channel buffers gets detached.
+ size_t length() const { return hasDetachedChannelBuffer() ? 0 : m_originalLength; }
+ double duration() const { return length() / static_cast<double>(sampleRate()); }
+
// Channel data access
unsigned numberOfChannels() const { return m_channels.size(); }
ExceptionOr<JSC::JSValue> getChannelData(JSDOMGlobalObject&, unsigned channelIndex);
ExceptionOr<void> copyFromChannel(Ref<Float32Array>&&, unsigned channelNumber, unsigned bufferOffset);
ExceptionOr<void> copyToChannel(Ref<Float32Array>&&, unsigned channelNumber, unsigned startInChannel);
- Float32Array* channelData(unsigned channelIndex);
+
+ // Native channel data access.
+ RefPtr<Float32Array> channelData(unsigned channelIndex);
void zero();
// Because an AudioBuffer has a _javascript_ wrapper, which will be garbage collected, it may take a while for this object to be deleted.
@@ -70,14 +77,16 @@
void visitChannelWrappers(JSC::SlotVisitor&);
private:
- AudioBuffer(unsigned numberOfChannels, size_t length, float sampleRate);
+ AudioBuffer(unsigned numberOfChannels, size_t length, float sampleRate, LegacyPreventNeutering = LegacyPreventNeutering::No);
explicit AudioBuffer(AudioBus&);
void invalidate();
+ bool hasDetachedChannelBuffer() const;
+
float m_sampleRate;
mutable Lock m_channelsLock;
- size_t m_length;
+ size_t m_originalLength;
Vector<RefPtr<Float32Array>> m_channels;
Vector<JSValueInWrappedObject> m_channelWrappers;
};
Modified: trunk/Source/WebCore/Modules/webaudio/AudioBuffer.idl (269107 => 269108)
--- trunk/Source/WebCore/Modules/webaudio/AudioBuffer.idl 2020-10-28 17:32:26 UTC (rev 269107)
+++ trunk/Source/WebCore/Modules/webaudio/AudioBuffer.idl 2020-10-28 17:40:03 UTC (rev 269108)
@@ -35,8 +35,8 @@
Exposed=Window
] interface AudioBuffer {
[EnabledBySetting=ModernUnprefixedWebAudio] constructor(AudioBufferOptions options);
- readonly attribute unsigned long length; // in sample-frames
- readonly attribute double duration; // in seconds
+ [ImplementedAs=originalLength] readonly attribute unsigned long length; // in sample-frames
+ [ImplementedAs=originalDuration] readonly attribute double duration; // in seconds
readonly attribute float sampleRate; // in sample-frames per second
readonly attribute unsigned long numberOfChannels;
Modified: trunk/Source/WebCore/Modules/webaudio/ConvolverNode.cpp (269107 => 269108)
--- trunk/Source/WebCore/Modules/webaudio/ConvolverNode.cpp 2020-10-28 17:32:26 UTC (rev 269107)
+++ trunk/Source/WebCore/Modules/webaudio/ConvolverNode.cpp 2020-10-28 17:40:03 UTC (rev 269108)
@@ -130,9 +130,6 @@
if (!isChannelCountGood)
return Exception { NotSupportedError, "Buffer should have 1, 2 or 4 channels"_s };
- if (!bufferLength)
- return Exception { NotSupportedError, "Buffer length cannot be 0"_s };
-
// Wrap the AudioBuffer by an AudioBus. It's an efficient pointer set and not a memcpy().
// This memory is simply used in the Reverb constructor and no reference to it is kept for later use in that class.
auto bufferBus = AudioBus::create(numberOfChannels, bufferLength, false);
Modified: trunk/Source/WebCore/Modules/webaudio/ScriptProcessorNode.cpp (269107 => 269108)
--- trunk/Source/WebCore/Modules/webaudio/ScriptProcessorNode.cpp 2020-10-28 17:32:26 UTC (rev 269107)
+++ trunk/Source/WebCore/Modules/webaudio/ScriptProcessorNode.cpp 2020-10-28 17:40:03 UTC (rev 269108)
@@ -90,8 +90,9 @@
// Create double buffers on both the input and output sides.
// These AudioBuffers will be directly accessed in the main thread by _javascript_.
for (unsigned i = 0; i < 2; ++i) {
- auto inputBuffer = m_numberOfInputChannels ? AudioBuffer::create(m_numberOfInputChannels, bufferSize(), sampleRate) : 0;
- auto outputBuffer = m_numberOfOutputChannels ? AudioBuffer::create(m_numberOfOutputChannels, bufferSize(), sampleRate) : 0;
+ // We prevent neutering the AudioBuffers here since we pass those to JS and reuse them.
+ auto inputBuffer = m_numberOfInputChannels ? AudioBuffer::create(m_numberOfInputChannels, bufferSize(), sampleRate, AudioBuffer::LegacyPreventNeutering::Yes) : 0;
+ auto outputBuffer = m_numberOfOutputChannels ? AudioBuffer::create(m_numberOfOutputChannels, bufferSize(), sampleRate, AudioBuffer::LegacyPreventNeutering::Yes) : 0;
m_inputBuffers.append(inputBuffer);
m_outputBuffers.append(outputBuffer);
