Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge AudioBufferSourceNode loop fixes from Blink 
https://bugs.webkit.org/show_bug.cgi?id=216849

Reviewed by Eric Carlson.

Source/WebCore:

Merge AudioBufferSourceNode loop fixes from Blink, the following one in particular:
- https://codereview.chromium.org/723823002

This allows us to pass all the checks in:
webaudio/AudioBufferSource/audiobuffersource-loop-comprehensive.html.

No new tests, rebaselined existing test.

* Modules/webaudio/AudioBufferSourceNode.cpp:
(WebCore::AudioBufferSourceNode::renderFromBuffer):
(WebCore::AudioBufferSourceNode::adjustGrainParameters):
Merge loop fixes from Blink.

* Modules/webaudio/AudioScheduledSourceNode.cpp:
(WebCore::AudioScheduledSourceNode::stopLater):
Stop throwing when m_endTime is already set. Our behavior did not match
Chrome and Firefox here. This was causing some existing tests to start
failing now that providing a grain duration when calling start() would
set m_endTime if loop() returns true.

LayoutTests:

* webaudio/AudioBufferSource/audiobuffersource-loop-comprehensive-expected.txt:
Rebaseline test that is now passing.

* webaudio/Oscillator/oscillator-basic-expected.txt:
* webaudio/dom-exceptions-expected.txt:
Rebaseline tests because the exception message was improved.

* webaudio/audiobuffersource-exception-expected.txt:
* webaudio/audiobuffersource-exception.html:
Stop expecting an exception to be thrown when AudioBufferSourceNode.stop() gets called
more than once. I have verified that Chrome and Firefox do not throw in this case.

Canonical link: https://commits.webkit.org/229640@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@267443 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
@cdumez
cdumez committed Sep 22, 2020
1 parent 16e23d7 commit 4b3d294
Show file tree
Hide file tree
Showing 11 changed files with 85 additions and 60 deletions.
19 changes: 19 additions & 0 deletions LayoutTests/ChangeLog
View file
@@ -1,3 +1,22 @@
2020-09-22 Chris Dumez <cdumez@apple.com>

Merge AudioBufferSourceNode loop fixes from Blink
https://bugs.webkit.org/show_bug.cgi?id=216849

Reviewed by Eric Carlson.

* webaudio/AudioBufferSource/audiobuffersource-loop-comprehensive-expected.txt:
Rebaseline test that is now passing.

* webaudio/Oscillator/oscillator-basic-expected.txt:
* webaudio/dom-exceptions-expected.txt:
Rebaseline tests because the exception message was improved.

* webaudio/audiobuffersource-exception-expected.txt:
* webaudio/audiobuffersource-exception.html:
Stop expecting an exception to be thrown when AudioBufferSourceNode.stop() gets called
more than once. I have verified that Chrome and Firefox do not throw in this case.

2020-09-22 Hector Lopez <hector_i_lopez@apple.com>

[ macOS ] webgl/2.0.0/conformance/textures/misc/texture-upload-size.html is a flaky failure
Expand Down
2 changes: 1 addition & 1 deletion ...io/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-basic-expected.txt
View file
Expand Up @@ -6,7 +6,7 @@ PASS > [start/stop exceptions]
PASS start(NaN) threw TypeError: "The provided value is non-finite".
PASS start(Infinity) threw TypeError: "The provided value is non-finite".
PASS start(-Infinity) threw TypeError: "The provided value is non-finite".
PASS Calling stop() before start() threw InvalidStateError: "The object is in an invalid state.".
PASS Calling stop() before start() threw InvalidStateError: "cannot call stop without calling start first.".
PASS start(-1) threw RangeError: "when value should be positive".
PASS start(0,-1) threw RangeError: "offset value should be positive".
PASS start(0,0,-1) threw RangeError: "duration value should be positive".
Expand Down
2 changes: 1 addition & 1 deletion ...ebaudio/the-audio-api/the-constantsourcenode-interface/constant-source-basic-expected.txt
View file
Expand Up @@ -34,7 +34,7 @@ PASS > [start/stop exceptions]
PASS start(NaN) threw TypeError: "The provided value is non-finite".
PASS start(Infinity) threw TypeError: "The provided value is non-finite".
PASS start(-Infinity) threw TypeError: "The provided value is non-finite".
PASS Calling stop() before start() threw InvalidStateError: "The object is in an invalid state.".
PASS Calling stop() before start() threw InvalidStateError: "cannot call stop without calling start first.".
PASS start(-1) threw RangeError: "when value should be positive".
PASS Calling start() twice threw InvalidStateError: "The object is in an invalid state.".
PASS stop(-1) threw RangeError: "when value should be positive".
Expand Down
51 changes: 9 additions & 42 deletions LayoutTests/webaudio/AudioBufferSource/audiobuffersource-loop-comprehensive-expected.txt
View file
Expand Up @@ -20,13 +20,7 @@ PASS Case 6: loop from 4 -> 6 with playbackRate of 0.5 is identical to the arr
PASS Case 6: loop from 4 -> 6 with playbackRate of 0.5: tail contains only the constant 0.
PASS Case 7: loop from 4 -> 6 with playbackRate of 1.5 is identical to the array [0,1.5,3,4.5,4,5.5,5,4.5,4,5.5,5,4.5,4,5.5,5,4.5...].
PASS Case 7: loop from 4 -> 6 with playbackRate of 1.5: tail contains only the constant 0.
FAIL X Case 8: loop from 2 -> 5 with offset 6 expected to be equal to the array [2,3,4,2,3,4,2,3,4,2,3,4,2,3,4,2...] but differs in 16 places:
Index Actual Expected
[0] 3.0000000000000000e+0 2.0000000000000000e+0
[1] 4.0000000000000000e+0 3.0000000000000000e+0
[2] 2.0000000000000000e+0 4.0000000000000000e+0
[3] 3.0000000000000000e+0 2.0000000000000000e+0
...and 12 more errors. assert_true: expected true got false
PASS Case 8: loop from 2 -> 5 with offset 6 is identical to the array [2,3,4,2,3,4,2,3,4,2,3,4,2,3,4,2...].
PASS Case 8: loop from 2 -> 5 with offset 6: tail contains only the constant 0.
PASS Case 9: loop from 3 -> 6 with offset 1 is identical to the array [1,2,3,4,5,3,4,5,3,4,5,3,4,5,3,4...].
PASS Case 9: loop from 3 -> 6 with offset 1: tail contains only the constant 0.
Expand All @@ -42,39 +36,12 @@ PASS Case 14: illegal loop: loopStartFrame < 0 is identical to the array [0,1,
PASS Case 14: illegal loop: loopStartFrame < 0: tail contains only the constant 0.
PASS Case 15: illegal loop: loopEndFrame > bufferLength is identical to the array [0,1,2,3,4,5,6,7,0,1,2,3,4,5,6,7...].
PASS Case 15: illegal loop: loopEndFrame > bufferLength: tail contains only the constant 0.
FAIL X Case 16: loop from 3 -> 6 with offset 1 for 20 frames expected to be equal to the array [1,2,3,4,5,3,4,5,3,4,5,3,4,5,3,4...] but differs in 10 places:
Index Actual Expected
[20] 3.0000000000000000e+0 0.0000000000000000e+0
[21] 4.0000000000000000e+0 0.0000000000000000e+0
[22] 5.0000000000000000e+0 0.0000000000000000e+0
[23] 3.0000000000000000e+0 0.0000000000000000e+0
...and 6 more errors. assert_true: expected true got false
FAIL X Case 16: loop from 3 -> 6 with offset 1 for 20 frames: tail: Expected 0 for all values but found 2 unexpected values:
Index Actual
[0] 4
[1] 5 assert_true: expected true got false
FAIL X Case 17: loop from 3 -> 8 with offset 1 for 3 frames expected to be equal to the array [1,2,3,0,0,0,0,0,0,0,0,0,0,0,0,0...] but differs in 30 places:
Index Actual Expected
[0] 4.0000000000000000e+0 1.0000000000000000e+0
[1] 6.0000000000000000e+0 2.0000000000000000e+0
[2] 8.0000000000000000e+0 3.0000000000000000e+0
[3] 6.0000000000000000e+0 0.0000000000000000e+0
...and 26 more errors. assert_true: expected true got false
FAIL X Case 17: loop from 3 -> 8 with offset 1 for 3 frames: tail: Expected 0 for all values but found 2 unexpected values:
Index Actual
[0] 6
[1] 7 assert_true: expected true got false
FAIL X Case 18: loop from 3 -> 8 with offset 7 for 3 frames expected to be equal to the array [7,3,4,0,0,0,0,0,0,0,0,0,0,0,0,0...] but differs in 30 places:
Index Actual Expected
[0] 1.5000000000000000e+1 7.0000000000000000e+0
[1] 9.0000000000000000e+0 3.0000000000000000e+0
[2] 1.1000000000000000e+1 4.0000000000000000e+0
[3] 1.3000000000000000e+1 0.0000000000000000e+0
...and 26 more errors. assert_true: expected true got false
FAIL X Case 18: loop from 3 -> 8 with offset 7 for 3 frames: tail: Expected 0 for all values but found 2 unexpected values:
Index Actual
[0] 15
[1] 9 assert_true: expected true got false
FAIL < [AudioBufferSource looping test] 7 out of 39 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 Case 16: loop from 3 -> 6 with offset 1 for 20 frames is identical to the array [1,2,3,4,5,3,4,5,3,4,5,3,4,5,3,4...].
PASS Case 16: loop from 3 -> 6 with offset 1 for 20 frames: tail contains only the constant 0.
PASS Case 17: loop from 3 -> 8 with offset 1 for 3 frames is identical to the array [1,2,3,0,0,0,0,0,0,0,0,0,0,0,0,0...].
PASS Case 17: loop from 3 -> 8 with offset 1 for 3 frames: tail contains only the constant 0.
PASS Case 18: loop from 3 -> 8 with offset 7 for 3 frames is identical to the array [7,3,4,0,0,0,0,0,0,0,0,0,0,0,0,0...].
PASS Case 18: loop from 3 -> 8 with offset 7 for 3 frames: tail contains only the constant 0.
PASS < [AudioBufferSource looping test] All assertions passed. (total 39 assertions)
PASS # AUDIT TASK RUNNER FINISHED: 1 tasks ran successfully.

2 changes: 1 addition & 1 deletion LayoutTests/webaudio/Oscillator/oscillator-basic-expected.txt
View file
Expand Up @@ -18,7 +18,7 @@ PASS > [start/stop exceptions]
PASS start(NaN) threw TypeError: "The provided value is non-finite".
PASS start(Infinity) threw TypeError: "The provided value is non-finite".
PASS start(-Infinity) threw TypeError: "The provided value is non-finite".
PASS Calling stop() before start() threw InvalidStateError: "The object is in an invalid state.".
PASS Calling stop() before start() threw InvalidStateError: "cannot call stop without calling start first.".
PASS start(-1) threw RangeError: "when value should be positive".
PASS Calling start() twice threw InvalidStateError: "The object is in an invalid state.".
PASS stop(-1) threw RangeError: "when value should be positive".
Expand Down
4 changes: 2 additions & 2 deletions LayoutTests/webaudio/audiobuffersource-exception-expected.txt
View file
Expand Up @@ -2,9 +2,9 @@ Tests that AudioBufferSourceNode validates start and stop calls.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".

PASS bufferSource.stop(0) threw exception InvalidStateError: The object is in an invalid state..
PASS bufferSource.stop(0) threw exception InvalidStateError: cannot call stop without calling start first..
PASS bufferSource.start(0) threw exception InvalidStateError: Cannot call start more than once..
PASS bufferSource.stop(0) threw exception InvalidStateError: The object is in an invalid state..
PASS InvalidStateError did not throw exception.
PASS successfullyParsed is true

TEST COMPLETE
Expand Down
5 changes: 2 additions & 3 deletions LayoutTests/webaudio/audiobuffersource-exception.html
View file
Expand Up @@ -22,7 +22,7 @@

var sampleRate = 44100.0;
var numberOfFrames = 32;
var context = new webkitOfflineAudioContext(1, numberOfFrames, sampleRate);
var context = new OfflineAudioContext(1, numberOfFrames, sampleRate);
bufferSource = context.createBufferSource();
bufferSource.buffer = createTestBuffer(context, numberOfFrames);
bufferSource.connect(context.destination);
Expand All @@ -35,8 +35,7 @@
shouldThrowErrorName("bufferSource.start(0)", "InvalidStateError");
bufferSource.stop(0);

// 'stop' should be called only once.
shouldThrowErrorName("bufferSource.stop(0)", "InvalidStateError");
shouldNotThrow("bufferSource.stop(0)", "InvalidStateError");

context.oncomplete = finishJSTest;
context.startRendering();
Expand Down
4 changes: 2 additions & 2 deletions LayoutTests/webaudio/dom-exceptions-expected.txt
View file
Expand Up @@ -189,7 +189,7 @@ PASS source4 = context.createBufferSource() did not throw an exception.
PASS source4.start() did not throw an exception.
PASS source5 = context.createBufferSource() did not throw an exception.
PASS source5.buffer = buffer did not throw an exception.
PASS source5.stop() threw InvalidStateError: "The object is in an invalid state.".
PASS source5.stop() threw InvalidStateError: "cannot call stop without calling start first.".
PASS source6 = context.createBufferSource() did not throw an exception.
PASS source6.buffer = buffer did not throw an exception.
PASS source6.start() did not throw an exception.
Expand All @@ -212,7 +212,7 @@ PASS source8.stop(-Infinity) threw TypeError: "The provided value is non-finit
PASS source8.stop(NaN) threw TypeError: "The provided value is non-finite".
PASS source8.stop() did not throw an exception.
PASS osc = context.createOscillator() did not throw an exception.
PASS osc.stop() threw InvalidStateError: "The object is in an invalid state.".
PASS osc.stop() threw InvalidStateError: "cannot call stop without calling start first.".
PASS osc1 = context.createOscillator() did not throw an exception.
PASS osc1.start() did not throw an exception.
PASS osc1.stop() did not throw an exception.
Expand Down
27 changes: 27 additions & 0 deletions Source/WebCore/ChangeLog
View file
@@ -1,3 +1,30 @@
2020-09-22 Chris Dumez <cdumez@apple.com>

Merge AudioBufferSourceNode loop fixes from Blink
https://bugs.webkit.org/show_bug.cgi?id=216849

Reviewed by Eric Carlson.

Merge AudioBufferSourceNode loop fixes from Blink, the following one in particular:
- https://codereview.chromium.org/723823002

This allows us to pass all the checks in:
webaudio/AudioBufferSource/audiobuffersource-loop-comprehensive.html.

No new tests, rebaselined existing test.

* Modules/webaudio/AudioBufferSourceNode.cpp:
(WebCore::AudioBufferSourceNode::renderFromBuffer):
(WebCore::AudioBufferSourceNode::adjustGrainParameters):
Merge loop fixes from Blink.

* Modules/webaudio/AudioScheduledSourceNode.cpp:
(WebCore::AudioScheduledSourceNode::stopLater):
Stop throwing when m_endTime is already set. Our behavior did not match
Chrome and Firefox here. This was causing some existing tests to start
failing now that providing a grain duration when calling start() would
set m_endTime if loop() returns true.

2020-09-22 Sihui Liu <sihui_liu@apple.com>

REGRESSION(r266075): WebContent process crashes at TextManipulationController::getPath
Expand Down
25 changes: 19 additions & 6 deletions Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp
View file
Expand Up @@ -242,6 +242,13 @@ bool AudioBufferSourceNode::renderFromBuffer(AudioBus* bus, unsigned destination
virtualDeltaFrames = virtualMaxFrame - virtualMinFrame;
}

// If we're looping and the offset (virtualReadIndex) is past the end of the loop, wrap back to the
// beginning of the loop. For other cases, nothing needs to be done.
if (loop() && m_virtualReadIndex >= virtualMaxFrame) {
m_virtualReadIndex = (m_loopStart < 0) ? 0 : (m_loopStart * buffer()->sampleRate());
m_virtualReadIndex = std::min(m_virtualReadIndex, static_cast<double>(bufferLength - 1));
}

// Sanity check that our playback rate isn't larger than the loop size.
if (std::abs(pitchRate) > virtualDeltaFrames)
return false;
Expand Down Expand Up @@ -516,12 +523,18 @@ void AudioBufferSourceNode::adjustGrainParameters()

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;
if (!m_wasGrainDurationGiven)
m_grainDuration = bufferDuration - m_grainOffset;

if (m_wasGrainDurationGiven && loop()) {
// We're looping a grain with a grain duration specified. Schedule the loop
// to stop after grainDuration seconds after starting, possibly running the
// loop multiple times if grainDuration is larger than the buffer duration.
// The net effect is as if the user called stop(when + grainDuration).
m_grainDuration = clampTo(m_grainDuration, 0.0, std::numeric_limits<double>::infinity());
m_endTime = m_startTime + m_grainDuration;
} else
m_grainDuration = clampTo(m_grainDuration, 0.0, bufferDuration - m_grainOffset);

// 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.
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/Modules/webaudio/AudioScheduledSourceNode.cpp
View file
Expand Up @@ -174,8 +174,8 @@ ExceptionOr<void> AudioScheduledSourceNode::stopLater(double when)
ASSERT(isMainThread());
ALWAYS_LOG(LOGIDENTIFIER, when);

if (m_playbackState == UNSCHEDULED_STATE || m_endTime != UnknownTime)
return Exception { InvalidStateError };
if (m_playbackState == UNSCHEDULED_STATE)
return Exception { InvalidStateError, "cannot call stop without calling start first."_s };

if (!std::isfinite(when) || when < 0)
return Exception { RangeError, "when value should be positive"_s };
Expand Down

0 comments on commit 4b3d294

Please sign in to comment.