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.