Changeset 287854 in webkit

Timestamp:

Jan 10, 2022, 1:45:01 PM (5 years ago)

Author:

Chris Dumez

Message:

​https://brianpeiris.github.io/spatial-audio-test/?ramped stops playing after a few seconds
​https://bugs.webkit.org/show_bug.cgi?id=234979

Reviewed by Eric Carlson.

Source/WebCore:

In ConeEffect::gain(), due to precision issues and with certain panner node parameters, it was possible for
sourceToListener.dot(sourceOrientation) to return a value that is very slightly outside the [-1.0, 1.0]
range. We would then call acos() on the dot product and it would thus return NaN.

To addresss this, we now make sure the clamp the dot product to the expected range before calling acos().

I also made the following stylistic changes:

• Drop unnecessary normalizedSourceOrientation local variable since the sourceOrientation paramter is not const
• Add missing curly brackets in if conditions with more than one lines (due to comments)
• Call rad2deg() for readability instead of duplicating its logic here

Test: webaudio/Panner/panner-cone-gain-nan.html

• platform/audio/Cone.cpp:

(WebCore::ConeEffect::gain const):

LayoutTests:

Add layout test coverage.

• webaudio/Panner/panner-cone-gain-nan-expected.txt: Added.
• webaudio/Panner/panner-cone-gain-nan.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/webaudio/Panner/panner-cone-gain-nan-expected.txt (added)
• LayoutTests/webaudio/Panner/panner-cone-gain-nan.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/audio/Cone.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2022-01-10  Chris Dumez  <cdumez@apple.com>
+
+        https://brianpeiris.github.io/spatial-audio-test/?ramped stops playing after a few seconds
+        https://bugs.webkit.org/show_bug.cgi?id=234979
+
+        Reviewed by Eric Carlson.
+
+        Add layout test coverage.
+
+        * webaudio/Panner/panner-cone-gain-nan-expected.txt: Added.
+        * webaudio/Panner/panner-cone-gain-nan.html: Added.
+
 2022-01-10  Cathie Chen  <cathiechen@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2022-01-10  Chris Dumez  <cdumez@apple.com>
+
+        https://brianpeiris.github.io/spatial-audio-test/?ramped stops playing after a few seconds
+        https://bugs.webkit.org/show_bug.cgi?id=234979
+
+        Reviewed by Eric Carlson.
+
+        In ConeEffect::gain(), due to precision issues and with certain panner node parameters, it was possible for
+        `sourceToListener.dot(sourceOrientation)` to return a value that is very slightly outside the [-1.0, 1.0]
+        range. We would then call `acos()` on the dot product and it would thus return NaN.
+
+        To addresss this, we now make sure the clamp the dot product to the expected range before calling acos().
+
+        I also made the following stylistic changes:
+        - Drop unnecessary normalizedSourceOrientation local variable since the sourceOrientation paramter is not const
+        - Add missing curly brackets in if conditions with more than one lines (due to comments)
+        - Call rad2deg() for readability instead of duplicating its logic here
+
+        Test: webaudio/Panner/panner-cone-gain-nan.html
+
+        * platform/audio/Cone.cpp:
+        (WebCore::ConeEffect::gain const):
+
 2022-01-10  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Source/WebCore/platform/audio/Cone.cpp

@@ -47 +47 @@
     sourceToListener.normalize();
 
-    FloatPoint3D normalizedSourceOrientation = sourceOrientation;
-    normalizedSourceOrientation.normalize();
+    sourceOrientation.normalize();
+
+    // Due to precision issues, the dot product may be very slightly outside the range [-1.0, 1.0], which would
+    // acos() to return NaN. For this reason, we have to make sure we clamp the dot product before calling acos().
+    auto dotProduct = clampTo(sourceToListener.dot(sourceOrientation), -1.0, 1.0);
 
     // Angle between the source orientation vector and the source-listener vector
-    double dotProduct = sourceToListener.dot(normalizedSourceOrientation);
-    double angle = 180.0 * acos(dotProduct) / piDouble;
+    double angle = rad2deg(acos(dotProduct));
     double absAngle = fabs(angle);
 
@@ -60 +62 @@
     double gain = 1.0;
 
-    if (absAngle <= absInnerAngle)
+    if (absAngle <= absInnerAngle) {
         // No attenuation
         gain = 1.0;
-    else if (absAngle >= absOuterAngle)
+    } else if (absAngle >= absOuterAngle) {
         // Max attenuation
         gain = m_outerGain;
-    else {
+    } else {
         // Between inner and outer cones
         // inner -> outer, x goes from 0 -> 1