Skip to content

GitLab

T
tangled
Commit fcdf8258 authored by Raymond Toy's avatar Raymond Toy Committed by Commit Bot
Browse files
Options

Make sure Oscillator read index is in valid range. 

In ProcessARateVectorKernel we forgot to mask v_read0 to contain the
index in a valid range.  v_read1 was handled correctly, and
ProcessKRateVector also masked the indices.  Apply the mask.

Manually tested this agains the repro case from the bug and no issues
occur.  Previously, I could easily reproduce the issue.

Added simple test case where detune overflows.  Output should be zero.

Bug: 1115907
Change-Id: Id7af713b935bd7cc14669630ddb1d04c3b1b2c39
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2358593
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799646}
parent 3ae83ac4
Hide whitespace changes
Inline Side-by-side
Showing with 53 additions and 5 deletions
third_party/blink/renderer/modules/webaudio/cpu/x86/oscillator_kernel_sse2.cc
View file @ fcdf8258
......@@ -96,7 +96,8 @@ std::tuple<int, double> OscillatorHandler::ProcessKRateVector(
int n_loops = n / 4;
for (int loop = 0; loop < n_loops; ++loop, k += 4) {
// Compute indices for the samples and contain within the valid range.
// Compute indices for the samples. Clamp the index to lie in the range 0
// to periodic_wave_size-1 by applying a mask to the index.
const __m128i read_index_0 =
_mm_and_si128(_mm_cvttps_epi32(v_virt_index), v_read_mask);
const __m128i read_index_1 =
......@@ -226,14 +227,20 @@ double OscillatorHandler::ProcessARateVectorKernel(
// Convert the virtual read index (parts) to an integer, and carefully
// merge them into one vector.
const __m128i v_read0 = reinterpret_cast<__m128i>(_mm_movelh_ps(
__m128i v_read0 = reinterpret_cast<__m128i>(_mm_movelh_ps(
reinterpret_cast<__m128>(_mm_cvttpd_epi32(v_read_index_lo)),
reinterpret_cast<__m128>(_mm_cvttpd_epi32(v_read_index_hi))));
// Get index to next element being sure to wrap the index around if needed.
const __m128i v_read1 =
_mm_and_si128(_mm_add_epi32(v_read0, _mm_set1_epi32(1)),
_mm_set1_epi32(read_index_mask));
__m128i v_read1 = _mm_add_epi32(v_read0, _mm_set1_epi32(1));
// Make sure the index lies in 0 to periodic_wave_size - 1 (the size of the
// arrays) by applying a mask to the values.
{
const __m128i v_mask = _mm_set1_epi32(read_index_mask);
v_read0 = _mm_and_si128(v_read0, v_mask);
v_read1 = _mm_and_si128(v_read1, v_mask);
}
float sample1_lower[4] __attribute__((aligned(16)));
float sample2_lower[4] __attribute__((aligned(16)));
......
third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-oscillatornode-interface/detune-overflow.html 0 → 100644
View file @ fcdf8258
<!doctype html>
<html>
<head>
<title>Test Osc.detune Overflow</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/webaudio/resources/audit.js"></script>
<script src="/webaudio/resources/audit-util.js"></script>
</head>
<body>
<script>
const sampleRate = 44100;
const renderLengthFrames = RENDER_QUANTUM_FRAMES;
let audit = Audit.createTaskRunner();
audit.define('detune overflow', async (task, should) => {
let context =
new OfflineAudioContext(1, renderLengthFrames, sampleRate);
// This value of frequency and detune results in a computed frequency of
// 440*2^(153600/1200) = 1.497e41. The frequency needs to be clamped to
// Nyquist. But a sine wave at Nyquist frequency is all zeroes. Verify
// the output is 0.
let osc = new OscillatorNode(context, {frequency: 440, detune: 153600});
osc.connect(context.destination);
let buffer = await context.startRendering();
let output = buffer.getChannelData(0);
should(output, 'Osc freq and detune outside nominal range')
.beConstantValueOf(0);
task.done();
});
audit.run();
</script>
</body>
</html>
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment