Changeset 267386 in webkit

Timestamp:

Sep 21, 2020, 5:17:33 PM (6 years ago)

Author:

Chris Dumez

Message:

AudioBufferSourceNode should update grain parameters when buffer is set after rendering has started
​https://bugs.webkit.org/show_bug.cgi?id=216808

Reviewed by Eric Carlson.

Source/WebCore:

AudioBufferSourceNode should update grain parameters when buffer is set after rendering has
started. The grain parameters need to be adjusted so that they make sense given the buffer
length. Previously, we would only update grain parameters in AudioBufferSourceNode::startPlaying(),
when a buffer is set. We would fail to update those grain parameters when the buffer is set,
in setBuffer(), if startPlaying() has already been called.

No new tests, rebaselined existing test.

• Modules/webaudio/AudioBufferSourceNode.cpp:

(WebCore::AudioBufferSourceNode::setBuffer):
(WebCore::AudioBufferSourceNode::startLater):
(WebCore::AudioBufferSourceNode::startPlaying):
(WebCore::AudioBufferSourceNode::updateGrainParameters):

• Modules/webaudio/AudioBufferSourceNode.h:

LayoutTests:

Rebaseline test that is now passing.

• webaudio/AudioBufferSource/audiobuffersource-loop-grain-no-duration-expected.txt:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/webaudio/AudioBufferSource/audiobuffersource-loop-grain-no-duration-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp (modified) (3 diffs)
• Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-09-21  Chris Dumez  <cdumez@apple.com>
+
+        AudioBufferSourceNode should update grain parameters when buffer is set after rendering has started
+        https://bugs.webkit.org/show_bug.cgi?id=216808
+
+        Reviewed by Eric Carlson.
+
+        Rebaseline test that is now passing.
+
+        * webaudio/AudioBufferSource/audiobuffersource-loop-grain-no-duration-expected.txt:
+
 2020-09-21  Chris Dumez  <cdumez@apple.com>
 

trunk/LayoutTests/webaudio/AudioBufferSource/audiobuffersource-loop-grain-no-duration-expected.txt

@@ -8 +8 @@
 PASS < [loop-count] All assertions passed. (total 1 assertions) 
 PASS > [delayed-start]  
-FAIL X The content of the left and right channel expected to be equal to the array [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0...] but differs in 8128 places:
-        Index   Actual                  Expected
-        [8193]  7.8125000000000000e-3   0.0000000000000000e+0
-        [8194]  1.5625000000000000e-2   0.0000000000000000e+0
-        [8195]  2.3437500000000000e-2   0.0000000000000000e+0
-        [8196]  3.1250000000000000e-2   0.0000000000000000e+0
-        ...and 8124 more errors. assert_true: expected true got false
-FAIL < [delayed-start] 1 out of 1 assertions were failed. assert_true: expected true got false
-FAIL # AUDIT TASK RUNNER FINISHED: 1 out of 2 tasks were failed. assert_true: expected true got false
+PASS   The content of the left and right channel is identical to the array [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0...]. 
+PASS < [delayed-start] All assertions passed. (total 1 assertions) 
+PASS # AUDIT TASK RUNNER FINISHED: 2 tasks ran successfully. 
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-09-21  Chris Dumez  <cdumez@apple.com>
+
+        AudioBufferSourceNode should update grain parameters when buffer is set after rendering has started
+        https://bugs.webkit.org/show_bug.cgi?id=216808
+
+        Reviewed by Eric Carlson.
+
+        AudioBufferSourceNode should update grain parameters when buffer is set after rendering has
+        started. The grain parameters need to be adjusted so that they make sense given the buffer
+        length. Previously, we would only update grain parameters in AudioBufferSourceNode::startPlaying(),
+        when a buffer is set. We would fail to update those grain parameters when the buffer is set,
+        in setBuffer(), if startPlaying() has already been called.
+
+        No new tests, rebaselined existing test.
+
+        * Modules/webaudio/AudioBufferSourceNode.cpp:
+        (WebCore::AudioBufferSourceNode::setBuffer):
+        (WebCore::AudioBufferSourceNode::startLater):
+        (WebCore::AudioBufferSourceNode::startPlaying):
+        (WebCore::AudioBufferSourceNode::updateGrainParameters):
+        * Modules/webaudio/AudioBufferSourceNode.h:
+
 2020-09-21  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp

@@ -451 +451 @@
     m_virtualReadIndex = 0;
     m_buffer = WTFMove(buffer);
+
+    // In case the buffer gets set after playback has started, we need to clamp the grain parameters now.
+    if (m_isGrain)
+        adjustGrainParameters();
+
     return { };
 }
@@ -459 +464 @@
 }
 
-ExceptionOr<void> AudioBufferSourceNode::startLater(double when, double grainOffset, Optional<double> optionalGrainDuration)
-{
-    double grainDuration = 0;
-    if (optionalGrainDuration)
-        grainDuration = optionalGrainDuration.value();
-    else if (buffer())
-        grainDuration = buffer()->duration() - grainOffset;
-
+ExceptionOr<void> AudioBufferSourceNode::startLater(double when, double grainOffset, Optional<double> grainDuration)
+{
     return startPlaying(when, grainOffset, grainDuration);
 }
 
-ExceptionOr<void> AudioBufferSourceNode::startPlaying(double when, double grainOffset, double grainDuration)
+ExceptionOr<void> AudioBufferSourceNode::startPlaying(double when, double grainOffset, Optional<double> grainDuration)
 {
     ASSERT(isMainThread());
-    ALWAYS_LOG(LOGIDENTIFIER, "when = ", when, ", offset = ", grainOffset, ", duration = ", grainDuration);
+    ALWAYS_LOG(LOGIDENTIFIER, "when = ", when, ", offset = ", grainOffset, ", duration = ", grainDuration.valueOr(0));
 
     context().nodeWillBeginPlayback();
@@ -486 +485 @@
         return Exception { RangeError, "offset value should be positive"_s };
 
-    if (!std::isfinite(grainDuration) || (grainDuration < 0))
+    if (grainDuration && (!std::isfinite(*grainDuration) || (*grainDuration < 0)))
         return Exception { RangeError, "duration value should be positive"_s };
+
+    // This synchronizes with process().
+    auto locker = holdLock(m_processMutex);
 
     m_isGrain = true;
     m_grainOffset = grainOffset;
-    m_grainDuration = grainDuration;
+    m_grainDuration = grainDuration.valueOr(0);
+    m_wasGrainDurationGiven = !!grainDuration;
     m_startTime = when;
 
-    if (buffer()) {
-        // Do sanity checking of grain parameters versus buffer size.
-        double bufferDuration = buffer()->duration();
-
-        m_grainOffset = std::min(bufferDuration, grainOffset);
-
-        double maxDuration = bufferDuration - m_grainOffset;
-        m_grainDuration = std::min(maxDuration, grainDuration);
-
-        // We call timeToSampleFrame here since at playbackRate == 1 we don't want to go through linear interpolation
-        // at a sub-sample position since it will degrade the quality.
-        // When aligned to the sample-frame the playback will be identical to the PCM data stored in the buffer.
-        // Since playbackRate == 1 is very common, it's worth considering quality.
-        if (playbackRate().value() < 0)
-            m_virtualReadIndex = AudioUtilities::timeToSampleFrame(m_grainOffset + m_grainDuration, buffer()->sampleRate()) - 1;
-        else
-            m_virtualReadIndex = AudioUtilities::timeToSampleFrame(m_grainOffset, buffer()->sampleRate());
-    }
+    adjustGrainParameters();
+
     m_playbackState = SCHEDULED_STATE;
 
     return { };
+}
+
+void AudioBufferSourceNode::adjustGrainParameters()
+{
+    ASSERT(m_processMutex.isHeld());
+
+    auto buffer = this->buffer();
+    if (!buffer)
+        return;
+
+    // Do sanity checking of grain parameters versus buffer size.
+    double bufferDuration = buffer->duration();
+
+    m_grainOffset = std::min(bufferDuration, m_grainOffset);
+
+    double maxDuration = bufferDuration - m_grainOffset;
+
+    if (m_wasGrainDurationGiven)
+        m_grainDuration = std::min(m_grainDuration, maxDuration);
+    else
+        m_grainDuration = maxDuration;
+
+    // We call timeToSampleFrame here since at playbackRate == 1 we don't want to go through linear interpolation
+    // at a sub-sample position since it will degrade the quality.
+    // When aligned to the sample-frame the playback will be identical to the PCM data stored in the buffer.
+    // Since playbackRate == 1 is very common, it's worth considering quality.
+    if (playbackRate().value() < 0)
+        m_virtualReadIndex = AudioUtilities::timeToSampleFrame(m_grainOffset + m_grainDuration, buffer->sampleRate()) - 1;
+    else
+        m_virtualReadIndex = AudioUtilities::timeToSampleFrame(m_grainOffset, buffer->sampleRate());
 }
 

trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h

@@ -100 +100 @@
     virtual bool shouldThrowOnAttemptToOverwriteBuffer() const { return true; }
 
-    ExceptionOr<void> startPlaying(double when, double grainOffset, double grainDuration);
+    ExceptionOr<void> startPlaying(double when, double grainOffset, Optional<double> grainDuration);
+    void adjustGrainParameters();
 
     // Returns true on success.
@@ -135 +136 @@
     double m_grainOffset { 0 }; // in seconds
     double m_grainDuration; // in seconds
+    double m_wasGrainDurationGiven { false };
 
     // totalPitchRate() returns the instantaneous pitch rate (non-time preserving).