ConvolverNode's "buffer" attribute doesn't actually define the setter steps

[bzbarsky]

https://webaudio.github.io/web-audio-api/#dom-convolvernode-buffer does not clearly define the setter steps. This is bad, because the actual implementation at least in Gecko seems to do https://webaudio.github.io/web-audio-api/#acquire-the-content as part of those steps, which is obviously an observable side-effect that needs to be clearly specified.

[rtoy]

Yes, we need to define the steps. But I think it's not necessary to acquire the contents. At least Chrome and Safari basically convert the buffer into a bunch of FFTs (except for maybe the first 128 frames of the response), and doesn't refer to the original data anymore. (I'll have to check the exact details on when this conversion is done.)

So, maybe it's enough to just say that the data is copied and the convolver doesn't have to acquire the contents?

[karlt]

https://webaudio.github.io/web-audio-api/#acquire-the-content

The acquire the contents of an AudioBuffer operation is invoked in the following cases:

• [...]
• When a ConvolverNode's buffer is set to an AudioBuffer while the node is connected to an output node, or a ConvolverNode is connected to an output node while the ConvolverNode's buffer is set to an AudioBuffer, it acquires the content of the AudioBuffer.
• [...]

Gecko doesn't have the "connected to an output node" condition. I don't know the reasoning behind this condition.

It's theoretically possible to delay some of the FFT calculation work when the impulse response is long enough that FFTs of the tail are not required immediately. Gecko doesn't yet do this, but is performing the FFTs on the rendering thread (which is not so good without the mentioned optimization).

Impulse responses buffers can be moderately large, but I haven't quantified the cost of a main thread copy.

Perhaps an advantage of keeping the acquire-the-content for ConvolverNode would be so that, for all similar APIs, these steps are consistently "invoked when the contents of an AudioBuffer are needed by some API implementation."

[rtoy]

Chrome doesn't acquire the contents and, even worse, potentially modifies the given impulse response when computing the normalization factor.

IIUC, acquiring the contents would mean two convolvers couldn't use the same impulse response array and would need to have separate copies. Given the size of some responses, this would be bad for low-memory mobile devices.

[karlt]

The acquire the content operation is designed in a way that is not observably different from a copy-on-write implementation, and so using two ConvolverNodes with a single AudioBuffer does not require a copy.

Step 4 "Attach ArrayBuffers containing copies of the data to the AudioBuffer, to be returned by the next call to getChannelData()." is not observable until getChannelData() is called and so copying of the data is not required unless and until the AudioBuffer is modified or getChannelData() makes it available to JS for modification.

[rtoy]

Oops. I always forget that nothing happens unless someone wants to write to the AudioBuffer.

[rtoy]

FWIW, I checked Chrome's code. While constructing the convolver, it computes all the required FFTs. The iniitial part of the impulse response uses a direct convolution instead of an FFT, but Chrome makes a copy of the initial part of the response. Chrome's webaudio impl is fairly careful not to allocate memory on the rendering thread, so delaying the tail FFT setup would not be good for Chrome.

So, effectively, it's as if an internal copy were made.

[rtoy]

Teleconf: @padenot to investigate how FireFox behaves when using the same buffer for two convolvers, but the buffer is modified before being given to the second convolver.

Generally, we might say the UA may acquire the contents but isn't required to. (But maybe it should behave as if it were copied. Both Chrome and Firefox basically do this by computing FFTs and not using the buffer values any more.)

[rtoy]

Here's a little test of the convolver and buffers, as mentioned in #1862 (comment).

If you hear something, then it means that the modified ArrayBuffer used for the second convolver actually worked. If you hear nothing, that means the buffer used for the second convolver is the all-zeros response used for the first.

With Chrome beta and Firefox nightly, I hear a tone.

[rtoy]

@padenot to look at writing the spec text for this to be compatible with what Chrome and Firefox do today.

[padenot]

In both Firefox and Chrome, printing out n1.buffer.getChannelData(0)[0] after running response.getChannelData(0)[0] = 1; prints 1, which means something slightly different than acquire the contents happens, which is expected. However, there is a single full-scale sine coming through, which is expected also.

All UAs are compatible here, and do the right thing and we "just" need to make the prose reflect this.

[padenot]

It's fine to acquire the contents in fact here. It has the correct semantics, I think.

[rtoy]

Just wanted to mention here that at the F2F when printing out the convolver buffer value (using console.log(n1.buffer) in fiddle), both chrome and firefox returned a non-zero buffer because the audio buffer had changed. If i were a developer, I would find this very weird because the returned buffer is NOT the buffer actually being used for the first convolver since the originally assigned buffer was all zeros and the convolver is actually always producing silence.

Not sure what do to here about this.

[padenot]

I don't know if we can do anything here. Having to keep the buffer set around is a bit costly for an edge case like this.

[rtoy]

Fixed in PR #1962