Skip to content

Are Float32Arrays in inputs, outputs, and parameters arguments to process() reused between calls or created each time? #2381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
karlt opened this issue May 31, 2019 · 7 comments

Comments

@karlt
Copy link
Contributor

karlt commented May 31, 2019

This was originally filed as #1127 but the resolution is not clear to me.

https://webaudio.github.io/web-audio-api/#rendering-loop has

If this AudioNode has any AudioNodes connected to its input, sum the buffers made available for reading by all AudioNodes connected to this AudioNode. The resulting buffer is called the input buffer.

which implies that the "input buffer" used to "invoke processFunction" is a new object for each invocation. Is that the intention?

https://heycam.github.io/webidl/#es-sequence indicates that IDL sequence<T> types correspond to ECMAScript Array values. Conversion to ECMAScript values produces a new Array object, so the Arrays are new objects each time, if process() is invoked as a WebIDL callback. However, the Float32Array elements of these arrays could still either be reused or created each time, because they are not copied on conversion.

@hoch
Copy link
Member

hoch commented Jun 3, 2019

Is this question about AudioWorkletNode or a generic AudioNode case?

@karlt
Copy link
Contributor Author

karlt commented Jun 3, 2019

This issue was intended specifically for the Float32Arrays provided to AudioWorkletProcessor.process(). This case is special because this runs on the rendering thread, where I expect that minimizing garbage is preferable.

For completeness though, there are two other APIs in the spec that pass Float32Arrays to JS:

let buffer = new AudioBuffer({length: 1, sampleRate: 48000});
buffer.getChannelData(0)["expando"] = 1;
console.log("AudioBuffer.getChannelData() returns " +
            (buffer.getChannelData(0).hasOwnProperty("expando") ?
             "same object" : "new object"))
let context = new OfflineAudioContext({length: 1, sampleRate: 48000});
let a = new Float32Array(2);
a["expando"] = 1;
let shaper = new WaveShaperNode(context);
shaper.curve = a;
console.log("WaveShaperNode.curve returns " +
            (shaper.curve.hasOwnProperty("expando") ?
             "same object as setter" : "own object"))
shaper.curve["expando2"] = 1;
console.log("WaveShaperNode.curve returns " +
            (shaper.curve.hasOwnProperty("expando2") ?
             "same object" : "new object"))

AudioBuffer.getChannelData() provides the same object each time in both Chome and Firefox, though that is not clearly specified AFAIK.
The behavior of WaveShaperNode.curve differs between implementations. Neither return the object passed to the setter, but the behavior differs for repeated getter calls.
The Firefox behavior for WaveShaperNode.curve was changed at
https://hg.mozilla.org/mozilla-central/rev/8895dafe20fea0a2b339a8eeb556bcf4e3ac08ef#l5.23
I don't know the reasoning for the change because it appears to have snuck in with an unrelated change.

@karlt
Copy link
Contributor Author

karlt commented Jun 4, 2019

#1933 (comment) suspects a previous resolution based on the reasoning that objects being transferred to another thread would be too much of a problem.

I searched old issues and couldn't find such a resolution or discussion.
New objects will need to be created if and when buffers are neutered or can't be reused for other reasons, but this doesn't mean that new objects must be created on every call.

The most recent comment I found was #1127 (comment)

As a preview of the answer, buffer reuse across calls is likely to be permitted since otherwise there will be performance issues.

@hoch
Copy link
Member

hoch commented Jun 4, 2019

This issue was intended specifically for the Float32Arrays provided to AudioWorkletProcessor.process(). This case is special because this runs on the rendering thread, where I expect that minimizing garbage is preferable.

This issue has some discussion, so we certainly discussed it in the design process. (@padenot knows)

We all are aware that this will affect the GC, but it requires a new mechanism that prevents AWGS-generated (provided by audio subsystem) buffers from being transferred to the other realms. That seems like a non-trivial change, perhaps out of Web Audio API's boundary.

Another thought came up was to designate the WASM heap for this purpose, but also not feasible for V1.

@karlt
Copy link
Contributor Author

karlt commented Jun 6, 2019

Given there are other APIs that don't specify when a new object is created or reused, it seems acceptable to leave this unspecified for processFunction.

I don't think this needs to be resolved for v1. More specifics could perhaps be added when implementations have had more time to optimize.

@padenot
Copy link
Member

padenot commented Jun 27, 2019

See #1933 (comment) that is relevant to this discussion.

moz-v2v-gh referenced this issue in mozilla/gecko-dev Jul 18, 2019
….process() parameters r=padenot,bzbarsky

Objects are retained for re-use so as to reduce garbage generation and other
performance benefits.  This is currently unspecified.

https://github.com/WebAudio/web-audio-api/issues/1934
WebAudio/web-audio-api#1933

WebAudio/web-audio-api#1935 tracks specification of
zero-channels of input when inputs are not actively processing.

Differential Revision: https://phabricator.services.mozilla.com/D34836

--HG--
extra : moz-landing-system : lando
lissyx referenced this issue in lissyx/mozilla-central Jul 18, 2019
….process() parameters r=padenot,bzbarsky

Objects are retained for re-use so as to reduce garbage generation and other
performance benefits.  This is currently unspecified.

https://github.com/WebAudio/web-audio-api/issues/1934
WebAudio/web-audio-api#1933

WebAudio/web-audio-api#1935 tracks specification of
zero-channels of input when inputs are not actively processing.

Differential Revision: https://phabricator.services.mozilla.com/D34836
@mdjp mdjp transferred this issue from WebAudio/web-audio-api Sep 17, 2019
@padenot
Copy link
Member

padenot commented Jun 11, 2020

This is being handled in #1933. The answer to the question asked in this bug is "inputs, outputs and parameters are reused between calls".

1933 doesn't have prose yet, but there is agreement between implementers, and we to test implementation before writing the text. The outcome of this issue will not change however, objects are reused.

@padenot padenot closed this as completed Jun 11, 2020
@hoch hoch transferred this issue from WebAudio/web-audio-api-v2 Sep 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants