Open Bug 1576059 Opened 5 years ago Updated 2 years ago

Delay/spread ConvolverNode Reverb initialization across render quanta

Categories

(Core :: Web Audio, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: karlt, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: perf)

ReverbConvolver is careful to stagger its ReverbConvolverStages so that they are not performing FFTs on their inputs during the same render quantum.

However, the initialization of each stage involves comparable computation cost and all stages are initialized during the single first render quantum after the ConvolverNode.buffer is set.

The significant initialization operations can be delayed until the same render quanta as the periodic FFTs so that the costs are spread across quanta.

  1. Adding a bool FFTConvolver::processNeedsFFT() { return m_readWriteIndex == halfSize; } method would allow delayed initialization of ReverbConvolverStage::m_fftKernel. This would require storing the values passed to the constructor for an impulseResponse strong reference, stageOffset, and stageLength.

  2. FFTConvolver buffer zero-initialization can be delayed until process() by keeping the initial value of m_readWriteIndex. SilentChannel::ZeroChannel() could be returned until initialized.

  3. ReverbConvolver::m_accumulationBuffer should not require complete zero-initialization in one render quantum. The output of each stage is offset by ReverbConvolverStage::m_postDelayLength, which is non-decreasing through stages. That allows ReverbConvolver::process() to determine for each stage whether the output block in m_accumulationBuffer corresponding to the stage needs zero-initialization based on whether that has already been performed for the next stage (in the current or a previous rendering quantum).

  4. It should not be necessary to zero-initialize ReverbConvolver::m_inputBuffer.

Ideally the Reverb would be constructed on the main thread so that buffer allocations were performed there. I wonder whether zero-initialization of FFTBlock::mOutputBuffer on FFTBlock construction is really necessary.

Priority: -- → P3

Another option I considered instead of 1 and 2 was to refactor the ReverbConvolver() constructor a little to delay setting up m_backgroundStages until ReverbConvolver::backgroundThreadEntry().

The initialization of rendering thread stages would be performed on main thread.
There would be a small chunk of work but maybe not too much jank.

A different code path would be required for offline rendering, where there is no background thread.

I'm not so keen on this approach because, without a mechanism such as 1 and 2, there would still be huge chunk of computation to complete at the start of backgroundThreadEntry() before the background stages can get started. Presumably that interval is already somewhat limited or we would reduce it to reduce the rendering thread computation.

https://bugzilla.mozilla.org/show_bug.cgi?id=1493779#c3 has some notes on performance. It is not possible to spread calculation of the normalization factor over multiple render quanta and so that would be better performed on the main thread than the render thread. (If really necessary, decodeAudioData() could calculate this off main thread to support what is presumably the common path for large buffer construction.)

(In reply to Karl Tomlinson (:karlt) from comment #1)

  1. FFTConvolver buffer zero-initialization can be delayed until process() by keeping the initial value of m_readWriteIndex. SilentChannel::ZeroChannel() could be returned until initialized.

Probably better would be to write output directly to ReverbConvolverStage::m_accumulationBuffer (assuming alignment requirements can be satisfied) so that FFTConvolver::m_outputBuffer and m_lastOverlapBuffer are not required at all.

Similarly, ReverbConvolver::m_inputBuffer could be sized and initialized so that FFTConvolver::m_inputBuffer is not required.
(All stages share the same input and so the FFT of the input could even be shared between stages of the same size.)

Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.