Title: [267386] trunk
Revision
267386
Author
[email protected]
Date
2020-09-21 17:17:33 -0700 (Mon, 21 Sep 2020)

Log 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:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (267385 => 267386)


--- trunk/LayoutTests/ChangeLog	2020-09-21 23:49:41 UTC (rev 267385)
+++ trunk/LayoutTests/ChangeLog	2020-09-22 00:17:33 UTC (rev 267386)
@@ -1,5 +1,16 @@
 2020-09-21  Chris Dumez  <[email protected]>
 
+        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  <[email protected]>
+
         Unreviewed, unskip webaudio/Analyser/handle-silent-inputs.html.
 
         This test should no longer be flaky now that it is passing.

Modified: trunk/LayoutTests/webaudio/AudioBufferSource/audiobuffersource-loop-grain-no-duration-expected.txt (267385 => 267386)


--- trunk/LayoutTests/webaudio/AudioBufferSource/audiobuffersource-loop-grain-no-duration-expected.txt	2020-09-21 23:49:41 UTC (rev 267385)
+++ trunk/LayoutTests/webaudio/AudioBufferSource/audiobuffersource-loop-grain-no-duration-expected.txt	2020-09-22 00:17:33 UTC (rev 267386)
@@ -7,13 +7,7 @@
 PASS   The output of actual and expected loops is identical to the array [0,0.125,0.25,0.375,0.5,0.625,0.75,0.875,0,0.125,0.25,0.375,0.5,0.625,0.75,0.875...]. 
 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. 
 

Modified: trunk/Source/WebCore/ChangeLog (267385 => 267386)


--- trunk/Source/WebCore/ChangeLog	2020-09-21 23:49:41 UTC (rev 267385)
+++ trunk/Source/WebCore/ChangeLog	2020-09-22 00:17:33 UTC (rev 267386)
@@ -1,5 +1,27 @@
 2020-09-21  Chris Dumez  <[email protected]>
 
+        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  <[email protected]>
+
         Values returned by FFTFrame::doFFT() are twice as large as they should be
         https://bugs.webkit.org/show_bug.cgi?id=216781
 

Modified: trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp (267385 => 267386)


--- trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp	2020-09-21 23:49:41 UTC (rev 267385)
+++ trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp	2020-09-22 00:17:33 UTC (rev 267386)
@@ -450,6 +450,11 @@
 
     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 { };
 }
 
@@ -458,21 +463,15 @@
     return output(0)->numberOfChannels();
 }
 
-ExceptionOr<void> AudioBufferSourceNode::startLater(double when, double grainOffset, Optional<double> optionalGrainDuration)
+ExceptionOr<void> AudioBufferSourceNode::startLater(double when, double grainOffset, Optional<double> grainDuration)
 {
-    double grainDuration = 0;
-    if (optionalGrainDuration)
-        grainDuration = optionalGrainDuration.value();
-    else if (buffer())
-        grainDuration = buffer()->duration() - grainOffset;
-
     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();
 
@@ -485,37 +484,55 @@
     if (!std::isfinite(grainOffset) || (grainOffset < 0))
         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();
+    adjustGrainParameters();
 
-        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());
-    }
     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());
+}
+
 double AudioBufferSourceNode::totalPitchRate()
 {
     double dopplerRate = 1.0;

Modified: trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h (267385 => 267386)


--- trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h	2020-09-21 23:49:41 UTC (rev 267385)
+++ trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h	2020-09-22 00:17:33 UTC (rev 267386)
@@ -99,7 +99,8 @@
     virtual double legacyGainValue() const { return 1.0; }
     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.
     bool renderFromBuffer(AudioBus*, unsigned destinationFrameOffset, size_t numberOfFrames, double startFrameOffset);
@@ -134,6 +135,7 @@
     bool m_isGrain { false };
     double m_grainOffset { 0 }; // in seconds
     double m_grainDuration; // in seconds
+    double m_wasGrainDurationGiven { false };
 
     // totalPitchRate() returns the instantaneous pitch rate (non-time preserving).
     // It incorporates the base pitch rate, any sample-rate conversion factor from the buffer, and any doppler shift from an associated panner node.
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to