Fix incorrect delay of DelayNode

WrapPositionVector was not wrapping the position around when it equaled
the end of the buffer.  This caused an extra zero value to be output
in this case.

Examination of WrapIndexVector also indicates a related logic error
because there's no >= intrinsic, so we used < but got the logic wrong.

The NEON version appears to be correct, so just added additional
comments to match the SSE2 version.

Added one test to make sure the delay is correct (based on the bug
report).

Bug: 1123023
Test: the-delaynode-interface/delay-test.html
Change-Id: Ibcb09573c53d62e926a332307d07601110b91aa3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2382656Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803289}

third_party/blink/renderer/platform/audio/cpu/arm/audio_delay_dsp_kernel_neon.cc

@@ -14,28 +14,35 @@ namespace blink {
 static ALWAYS_INLINE int32x4_t WrapIndexVector(int32x4_t v_write_index,
                                                int32x4_t v_buffer_length) {
   // Wrap the write_index if any index is past the end of the buffer.
+  // This implements
+  //
+  //   if (write_index >= buffer_length)
+  //     write_index -= buffer_length
 
-  // cmp = 0xffffffff if buffer length < write index and 0 otherwise.  (That is,
-  // 0xffffffff if index >= buffer length.)
+  // If write_index >= buffer_length, cmp = 0xffffffff.  Otherwise 0.
   int32x4_t cmp =
       reinterpret_cast<int32x4_t>(vcgeq_s32(v_write_index, v_buffer_length));
 
-  // Bitwise and cmp with buffer length to get buffer length or 0 depending on
-  // whether buffer length < index or not.  Subtract this from the index to wrap
-  // the index appropriately.
+  // Bitwise-and cmp with buffer length to get buffer length or 0 depending on
+  // whether write_index >= buffer_length or not.  Subtract this from the index
+  // to wrap the index appropriately.
   return vsubq_s32(v_write_index, vandq_s32(cmp, v_buffer_length));
 }
 
 static ALWAYS_INLINE float32x4_t
 WrapPositionVector(float32x4_t v_position, float32x4_t v_buffer_length) {
   // Wrap the read position if it exceed the buffer length.
+  // This implements
+  //
+  //   if (position >= buffer_length)
+  //     read_position -= buffer_length
 
-  // If buffer length < read_position, set cmp to 0xffffffff.  Otherwise zero.
+  // If position >= buffer length, set cmp = 0xffffffff.  Otherwise 0.
   uint32x4_t cmp = vcgeq_f32(v_position, v_buffer_length);
 
-  // Bitwise and buffer_length with cmp to get buffer_length or 0 depending on
+  // Bitwise-and buffer_length with cmp to get buffer_length or 0 depending on
   // whether read_position >= buffer length or not.  Then subtract from the
-  // psoition to wrap it around if needed.
+  // position to wrap it around if needed.
   return vsubq_f32(v_position,
                    reinterpret_cast<float32x4_t>(vandq_u32(
                        reinterpret_cast<uint32x4_t>(v_buffer_length), cmp)));

third_party/blink/renderer/platform/audio/cpu/x86/audio_delay_dsp_kernel_sse2.cc

@@ -11,27 +11,40 @@ namespace blink {
 static ALWAYS_INLINE __m128i WrapIndexVector(__m128i v_write_index,
                                              __m128i v_buffer_length) {
   // Wrap the write_index if any index is past the end of the buffer.
-
-  // cmp = 0xffffffff if buffer length < write index and 0 otherwise.  (That is,
-  // 0xffffffff if index >= buffer length.)
-  __m128i cmp = _mm_cmplt_epi32(v_buffer_length, v_write_index);
-
-  // Bitwise and cmp with buffer length to get buffer length or 0 depending on
-  // whether buffer length < index or not.  Subtract this from the index to wrap
-  // the index appropriately.
-  return _mm_sub_epi32(v_write_index, _mm_and_si128(cmp, v_buffer_length));
+  // This implements
+  //
+  //   if (write_index >= buffer_length)
+  //     write_index -= buffer_length
+
+  // There's no mm_cmpge_epi32, so we need to use mm_cmplt_epi32.  Thus, the
+  // above becomes
+  //
+  //   if (!(write_index < buffer_length))
+  //     write_index -= buffer_length
+
+  // If write_index < buffer_length, set cmp = 0xffffffff.  Otherwise 0.
+  __m128i cmp = _mm_cmplt_epi32(v_write_index, v_buffer_length);
+
+  // Invert cmp and bitwise-and with buffer_length to get buffer_length or 0
+  // depending on whether write_index >= buffer_length or not.  Subtract from
+  // write_index to wrap it.
+  return _mm_sub_epi32(v_write_index, _mm_andnot_si128(cmp, v_buffer_length));
 }
 
 static ALWAYS_INLINE __m128 WrapPositionVector(__m128 v_position,
                                                __m128 v_buffer_length) {
   // Wrap the read position if it exceed the buffer length.
+  // This implements
+  //
+  //   if (position >= buffer_length)
+  //     read_position -= buffer_length
 
-  // If buffer length < read_position, set cmp to 0xffffffff.  Otherwise zero.
-  __m128 cmp = _mm_cmplt_ps(v_buffer_length, v_position);
+  // If position >= buffer length, set cmp = 0xffffffff.  Otherwise 0.
+  __m128 cmp = _mm_cmpge_ps(v_position, v_buffer_length);
 
-  // Bitwise and buffer_length with cmp to get buffer_length or 0 depending on
+  // Bitwise-and buffer_length with cmp to get buffer_length or 0 depending on
   // whether read_position >= buffer length or not.  Then subtract from the
-  // psoition to wrap it around if needed.
+  // position to wrap it.
   return _mm_sub_ps(v_position, _mm_and_ps(v_buffer_length, cmp));
 }
 

third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-delaynode-interface/delay-test.html

+<!doctype html>
+<html>
+  <head>
+    <title>Test DelayNode Delay</title>
+    <script src="/resources/testharness.js"></script>
+    <script src="/resources/testharnessreport.js"></script>
+    <script src="/webaudio/resources/audit-util.js"></script>
+    <script src="/webaudio/resources/audit.js"></script>
+  </head>
+
+  <body>
+    <script>
+      let audit = Audit.createTaskRunner();
+
+      audit.define(
+          {label: 'test0', description: 'Test delay of 3 frames'},
+          async (task, should) => {
+            // Only need a few outputs samples.  The sample rate is arbitrary.
+            const context =
+                new OfflineAudioContext(1, RENDER_QUANTUM_FRAMES, 8192);
+            let src;
+            let delay;
+
+            should(
+                () => {
+                  src = new ConstantSourceNode(context);
+                  delay = new DelayNode(context);
+                },
+                'Creating ConstantSourceNode(context) and DelayNode(context)')
+                .notThrow();
+
+            // The number of frames to delay for the DelayNode.  Should be a
+            // whole number, but is otherwise arbitrary.
+            const delayFrames = 3;
+
+            should(() => {
+              delay.delayTime.value = delayFrames / context.sampleRate;
+            }, `Setting delayTime to ${delayFrames} frames`).notThrow();
+
+            src.connect(delay).connect(context.destination);
+
+            src.start();
+
+            let buffer = await context.startRendering();
+            let output = buffer.getChannelData(0);
+
+            // Verify output was delayed the correct number of frames.
+            should(output.slice(0, delayFrames), `output[0:${delayFrames - 1}]`)
+                .beConstantValueOf(0);
+            should(
+                output.slice(delayFrames),
+                `output[${delayFrames}:${output.length - 1}]`)
+                .beConstantValueOf(1);
+
+            task.done();
+          });
+
+      audit.run();
+    </script>
+  </body>
+</html>