https://brianpeiris.github.io/spatial-audio-test/?ramped stops playin…

…g 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. Canonical link: https://commits.webkit.org/245900@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@287854 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Jan 10, 2022 · d4aceee330ebf9f7bacc7d183659b4d609344f87 · d4aceee
1 parent 4ff32c7
commit d4aceee330ebf9f7bacc7d183659b4d609344f87
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+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>
 
         ASSERTION FAILED in RenderLayer::updateClipRects

diff --git a/LayoutTests/webaudio/Panner/panner-cone-gain-nan-expected.txt b/LayoutTests/webaudio/Panner/panner-cone-gain-nan-expected.txt
@@ -0,0 +1,10 @@
+Tests that precision issues do not cause ConeEffect::gain() to return NaN
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS data.some(isNaN) is false
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/webaudio/Panner/panner-cone-gain-nan.html b/LayoutTests/webaudio/Panner/panner-cone-gain-nan.html
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test.js"></script>
+</head>
+<body>
+<script>
+description("Tests that precision issues do not cause ConeEffect::gain() to return NaN");
+jsTestIsAsync = true;
+
+function test() {
+    const audioContext = new OfflineAudioContext(1, 44100, 44100);
+
+    const panner = audioContext.createPanner();
+    panner.panningModel = "HRTF";
+    panner.refDistance = 50;
+    panner.coneInnerAngle = 0;
+    panner.coneOuterAngle = 180;
+    panner.coneOuterGain = 0.25;
+
+    const source = new ConstantSourceNode(audioContext);
+    source.connect(panner);
+    panner.connect(audioContext.destination);
+
+    audioContext.listener.setPosition(203.4781799316, 0, 200);
+    panner.setPosition(121.7528991699, 0, 137.7344207764);
+    panner.setOrientation(0.7954952121, 0, 0.6058534384);
+
+    source.start();
+    audioContext.startRendering().then((buffer) => {
+        data = buffer.getChannelData(0);
+        shouldBeFalse("data.some(isNaN)");
+        finishJSTest();
+  });
+}
+
+onload = test;
+
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,26 @@
+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>
 
         Modal container observer fails detection when rendered text is inserted after the container

diff --git a/Source/WebCore/platform/audio/Cone.cpp b/Source/WebCore/platform/audio/Cone.cpp
@@ -46,26 +46,28 @@ double ConeEffect::gain(FloatPoint3D sourcePosition, FloatPoint3D sourceOrientat
     FloatPoint3D sourceToListener = listenerPosition - sourcePosition;
     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);
 
     // Divide by 2.0 here since API is entire angle (not half-angle)
     double absInnerAngle = fabs(m_innerAngle) / 2.0;
     double absOuterAngle = fabs(m_outerAngle) / 2.0;
     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
         double x = (absAngle - absInnerAngle) / (absOuterAngle - absInnerAngle);