Title: [265256] trunk
Revision
265256
Author
cdu...@apple.com
Date
2020-08-04 12:26:58 -0700 (Tue, 04 Aug 2020)

Log Message

Align AudioBufferSourceNode's start() / stop() with the specification
https://bugs.webkit.org/show_bug.cgi?id=215130

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Rebaseline a couple of web-platform-tests now that they are passing.

* web-platform-tests/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-basic-expected.txt:
* web-platform-tests/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-grain-expected.txt:

Source/WebCore:

Align AudioBufferSourceNode's start() / stop() with the specification:
- https://www.w3.org/TR/webaudio/#dom-audiobuffersourcenode-start
- https://www.w3.org/TR/webaudio/#dom-audioscheduledsourcenode-stop

In particular, we should throw a RangeError when the parameters are invalid,
not an InvalidStateError. Also, we should not return early if there is no
buffer.

No new tests, rebaselined existing tests.

* Modules/webaudio/AudioBufferSourceNode.cpp:
(WebCore::AudioBufferSourceNode::startPlaying):
* Modules/webaudio/AudioScheduledSourceNode.cpp:
(WebCore::AudioScheduledSourceNode::startLater):
(WebCore::AudioScheduledSourceNode::stopLater):

LayoutTests:

Rebaseline test because exception message changed.

* webaudio/audiobuffersource-exception-expected.txt:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (265255 => 265256)


--- trunk/LayoutTests/ChangeLog	2020-08-04 19:24:44 UTC (rev 265255)
+++ trunk/LayoutTests/ChangeLog	2020-08-04 19:26:58 UTC (rev 265256)
@@ -1,3 +1,14 @@
+2020-08-04  Chris Dumez  <cdu...@apple.com>
+
+        Align AudioBufferSourceNode's start() / stop() with the specification
+        https://bugs.webkit.org/show_bug.cgi?id=215130
+
+        Reviewed by Darin Adler.
+
+        Rebaseline test because exception message changed.
+
+        * webaudio/audiobuffersource-exception-expected.txt:
+
 2020-08-04  Hector Lopez  <hector_i_lo...@apple.com>
 
         [ iOS ] fast/canvas/draw-focus-if-needed.html is passing and expectations need to be removed

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (265255 => 265256)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2020-08-04 19:24:44 UTC (rev 265255)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2020-08-04 19:26:58 UTC (rev 265256)
@@ -1,3 +1,15 @@
+2020-08-04  Chris Dumez  <cdu...@apple.com>
+
+        Align AudioBufferSourceNode's start() / stop() with the specification
+        https://bugs.webkit.org/show_bug.cgi?id=215130
+
+        Reviewed by Darin Adler.
+
+        Rebaseline a couple of web-platform-tests now that they are passing.
+
+        * web-platform-tests/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-basic-expected.txt:
+        * web-platform-tests/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-grain-expected.txt:
+
 2020-08-04  Rob Buis  <rb...@igalia.com>
 
         Performance.getEntriesByName/Type should match case sensitive

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-basic-expected.txt (265255 => 265256)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-basic-expected.txt	2020-08-04 19:24:44 UTC (rev 265255)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-basic-expected.txt	2020-08-04 19:26:58 UTC (rev 265256)
@@ -7,14 +7,14 @@
 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.". 
-FAIL X start(-1) threw "InvalidStateError" instead of EcmaScript error RangeError. assert_true: expected true got false
-FAIL X start(0,-1) threw "InvalidStateError" instead of EcmaScript error RangeError. assert_true: expected true got false
-FAIL X start(0,0,-1) threw "InvalidStateError" instead of EcmaScript error RangeError. assert_true: expected true got false
-FAIL X Calling start() twice did not throw an exception. assert_true: expected true got false
-FAIL X stop(-1) threw "InvalidStateError" instead of EcmaScript error RangeError. assert_true: expected true got false
+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". 
+PASS   Calling start() twice threw InvalidStateError: "Cannot call start more than once.". 
+PASS   stop(-1) threw RangeError: "when value should be positive". 
 PASS   stop(NaN) threw TypeError: "The provided value is non-finite". 
 PASS   stop(Infinity) threw TypeError: "The provided value is non-finite". 
 PASS   stop(-Infinity) threw TypeError: "The provided value is non-finite". 
-FAIL < [start/stop exceptions] 5 out of 12 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 < [start/stop exceptions] All assertions passed. (total 12 assertions) 
+PASS # AUDIT TASK RUNNER FINISHED: 1 tasks ran successfully. 
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-grain-expected.txt (265255 => 265256)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-grain-expected.txt	2020-08-04 19:24:44 UTC (rev 265255)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiobuffersourcenode-interface/audiobuffersource-grain-expected.txt	2020-08-04 19:26:58 UTC (rev 265256)
@@ -3,7 +3,7 @@
 PASS Executing "Test setting the source buffer after starting the grain" 
 PASS Audit report 
 PASS > [Test setting the source buffer after starting the grain]  
-FAIL X Buffer was played is not true. Got false. assert_true: expected true got false
-FAIL < [Test setting the source buffer after starting the grain] 1 out of 1 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   Buffer was played is true. 
+PASS < [Test setting the source buffer after starting the grain] All assertions passed. (total 1 assertions) 
+PASS # AUDIT TASK RUNNER FINISHED: 1 tasks ran successfully. 
 

Modified: trunk/LayoutTests/webaudio/audiobuffersource-exception-expected.txt (265255 => 265256)


--- trunk/LayoutTests/webaudio/audiobuffersource-exception-expected.txt	2020-08-04 19:24:44 UTC (rev 265255)
+++ trunk/LayoutTests/webaudio/audiobuffersource-exception-expected.txt	2020-08-04 19:26:58 UTC (rev 265256)
@@ -3,7 +3,7 @@
 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.start(0) threw exception InvalidStateError: The object is in an invalid state..
+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 successfullyParsed is true
 

Modified: trunk/Source/WebCore/ChangeLog (265255 => 265256)


--- trunk/Source/WebCore/ChangeLog	2020-08-04 19:24:44 UTC (rev 265255)
+++ trunk/Source/WebCore/ChangeLog	2020-08-04 19:26:58 UTC (rev 265256)
@@ -1,3 +1,26 @@
+2020-08-04  Chris Dumez  <cdu...@apple.com>
+
+        Align AudioBufferSourceNode's start() / stop() with the specification
+        https://bugs.webkit.org/show_bug.cgi?id=215130
+
+        Reviewed by Darin Adler.
+
+        Align AudioBufferSourceNode's start() / stop() with the specification:
+        - https://www.w3.org/TR/webaudio/#dom-audiobuffersourcenode-start
+        - https://www.w3.org/TR/webaudio/#dom-audioscheduledsourcenode-stop
+
+        In particular, we should throw a RangeError when the parameters are invalid,
+        not an InvalidStateError. Also, we should not return early if there is no
+        buffer.
+
+        No new tests, rebaselined existing tests.
+
+        * Modules/webaudio/AudioBufferSourceNode.cpp:
+        (WebCore::AudioBufferSourceNode::startPlaying):
+        * Modules/webaudio/AudioScheduledSourceNode.cpp:
+        (WebCore::AudioScheduledSourceNode::startLater):
+        (WebCore::AudioScheduledSourceNode::stopLater):
+
 2020-08-04  Andres Gonzalez  <andresg...@apple.com>
 
         Add the ability of comparing the accessibility tree with isolated tree mode on and off.

Modified: trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp (265255 => 265256)


--- trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp	2020-08-04 19:24:44 UTC (rev 265255)
+++ trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp	2020-08-04 19:26:58 UTC (rev 265256)
@@ -471,45 +471,45 @@
     context().nodeWillBeginPlayback();
 
     if (m_playbackState != UNSCHEDULED_STATE)
-        return Exception { InvalidStateError };
+        return Exception { InvalidStateError, "Cannot call start more than once."_s };
 
     if (!std::isfinite(when) || (when < 0))
-        return Exception { InvalidStateError };
+        return Exception { RangeError, "when value should be positive"_s };
 
     if (!std::isfinite(grainOffset) || (grainOffset < 0))
-        return Exception { InvalidStateError };
+        return Exception { RangeError, "offset value should be positive"_s };
 
     if (!std::isfinite(grainDuration) || (grainDuration < 0))
-        return Exception { InvalidStateError };
+        return Exception { RangeError, "duration value should be positive"_s };
 
-    if (!buffer())
-        return { };
-
     m_isGrain = playbackMode == Partial;
-    if (m_isGrain) {
-        // Do sanity checking of grain parameters versus buffer size.
-        double bufferDuration = buffer()->duration();
+    m_grainOffset = grainOffset;
+    m_grainDuration = grainDuration;
+    m_startTime = when;
 
-        m_grainOffset = std::min(bufferDuration, grainOffset);
+    if (buffer()) {
+        if (m_isGrain) {
+            // Do sanity checking of grain parameters versus buffer size.
+            double bufferDuration = buffer()->duration();
 
-        double maxDuration = bufferDuration - m_grainOffset;
-        m_grainDuration = std::min(maxDuration, grainDuration);
-    } else {
-        m_grainOffset = 0.0;
-        m_grainDuration = buffer()->duration();
+            m_grainOffset = std::min(bufferDuration, grainOffset);
+
+            double maxDuration = bufferDuration - m_grainOffset;
+            m_grainDuration = std::min(maxDuration, grainDuration);
+        } else {
+            m_grainOffset = 0.0;
+            m_grainDuration = buffer()->duration();
+        }
+
+        // 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 (totalPitchRate() < 0)
+            m_virtualReadIndex = AudioUtilities::timeToSampleFrame(m_grainOffset + m_grainDuration, buffer()->sampleRate()) - 1;
+        else
+            m_virtualReadIndex = AudioUtilities::timeToSampleFrame(m_grainOffset, buffer()->sampleRate());
     }
-
-    m_startTime = when;
-    
-    // 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 (totalPitchRate() < 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 { };

Modified: trunk/Source/WebCore/Modules/webaudio/AudioScheduledSourceNode.cpp (265255 => 265256)


--- trunk/Source/WebCore/Modules/webaudio/AudioScheduledSourceNode.cpp	2020-08-04 19:24:44 UTC (rev 265255)
+++ trunk/Source/WebCore/Modules/webaudio/AudioScheduledSourceNode.cpp	2020-08-04 19:26:58 UTC (rev 265256)
@@ -145,8 +145,9 @@
 
     if (m_playbackState != UNSCHEDULED_STATE)
         return Exception { InvalidStateError };
+
     if (!std::isfinite(when) || when < 0)
-        return Exception { InvalidStateError };
+        return Exception { RangeError, "when value should be positive"_s };
 
     m_startTime = when;
     m_playbackState = SCHEDULED_STATE;
@@ -161,8 +162,9 @@
 
     if (m_playbackState == UNSCHEDULED_STATE || m_endTime != UnknownTime)
         return Exception { InvalidStateError };
+
     if (!std::isfinite(when) || when < 0)
-        return Exception { InvalidStateError };
+        return Exception { RangeError, "when value should be positive"_s };
 
     m_endTime = when;
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to