Out of bounds read in DelayNode causes NaNs to be output

I encountered a problem where if I used a DelayNode with an oscillating delay and a BiquadFilterNode after it, after a minute or so one or both channels would randomly drop out and become NaN for the rest of the playback:

highPass = new Tone.Filter(100, 'highpass').toDestination(),
chorus = new Tone.Chorus(2, 2.4, 1.2).connect(highPass),

Here's a repo which reproduces that, just run "npm install" and "npm start" to start rendering audio:

https://github.com/thenickdude/web-audio-js-bug

The bug happens in DisplayNodeDSP at currentSampleFrame == 3897344.

kernel.computeAccurateDelayIndices() is called, and the first delayTime it sees is 4.558883937022529e-9, a small positive number. It computes a delayIndex of -0.00020104678162269352 for it, and since this is negative it's added to the size of the delay buffer (44288) to give 44287.99979895322. So far so good. This is that code:

const delayTime = Math.max(0, Math.min(delayTimes[i], maxDelayTime));

let delayIndex = virtualReadIndex + i - delayTime * sampleRate;

if (delayIndex < 0) {
  delayIndex += delayBufferLength;
}

delayIndices[i] = delayIndex;

But 44287.99979895322 can't be stored accurately enough in delayIndices, since delayIndices is only a Float32Array. It's so close to 44288 that it ends up being rounded up to 44288.

Next in kernel.processWithAccurateDelayIndices(), this code runs:

 const ix = delayIndices[i];
 const i0 = ix | 0;
 const ia = ix % 1;

 if (ia === 0) {
   output[i] = delayBuffer[i0];
  } else ...

ix is the 44288 value that computeAccurateDelayIndices stored in there, and its fractional part is zero, so this if statement is entered and attempts to return delayBuffer[44288], which is one past the end of the array, and so returns NaN. This breaks the Biquad filter because that NaN gets mixed into its current state, which silences the channel from then onwards.

To fix this I changed the if statement in processWithAccurateDelayIndices like so:

if (ia === 0) {
  if (i0 === delayBufferLength) {
    output[i] = delayBuffer[delayBufferLength - 1]; // Avoid out of bounds read
  } else {
    output[i] = delayBuffer[i0];
  }
} else ...

processWithStaticDelayIndices might have the same issue but I'm not sure about that one, I'm not familiar enough with the code.

With this fix in place no NaNs are generated and the audio now renders correctly:

descriptinc / web-audio-js

Out of bounds read in DelayNode causes NaNs to be output #25