padenot
commented
5 years ago [...] The way this algorithm is written right now, the result is somewhere between "undefined behavior" and "the entire rendering-a-graph algorithm invocation is aborted".
This should result in having onprocessorerror called on the control thread for the right AudioWorkletNode. Silence for this AudioWorkletProcessor should be output, and this AudioWorkletProcessor should be made inactive.
There's also additional confusion because some of the arguments to this function are defined as IDL values (e.g. outputs is defined as sequence<sequence
>) but there's nothing here that makes it clear that an IDL-to-ES conversion should happen.
This we're having a look at in https://github.com/WebAudio/web-audio-api/issues/1933#issuecomment-505551737. I think what we really want is FrozenArray<FrozenArray<Float32Array>>. This will solve a number of things (such as having authors mutating the array while in the process method, and allowing to reuse the objects although I'm not quite sure about that). I used the notation Float32Array[][], but this is not valid WebIDL.
I don't quite know what behavior this code is aiming is, especially in terms of microtask behavior; step 4.4.2.4 is not really clear to me in terms of what it's saying (e.g. is that a normative requirement, or an informative note? Normally microtask queuing normative bits are handled by the various existing ES infrastructure...). But it seems like what's really going on here is that processor is being treated as a Web IDL callback interface with a single method named process, and it might be worth actually defining it that way. Except the [[callable process]] bit has no corresponding equivalent in https://heycam.github.io/webidl/#call-a-user-objects-operation. Maybe we can refactor that algorithm in some way to make that behavior configurable?
The behaviour this should have is to do a microtask checkpoint after all the worklet process methods have been executed.
Conceptually, each rendering quantum iteration (the big algorithm in 2.4) is an event loop task, and microtask checkpoint happens in between those iterations (step 4 of the algorithm). Multiple process methods can be called during in each rendering quantum iteration (exactly one per instance of AudioWorkletProcessor). Any microtask spawed as a result of calling any process method will be executed after all the process method calls for this iteration, and before any process method calls for the next iteration.
This was discussed at length in https://github.com/WebAudio/web-audio-api/issues/1511, and the behaviour was decided there, but wasn't expressed adequately in the text.
@hoch, did I capture the intent correctly?
hoch
bzbarsky
karlt
svgeesus
https://webaudio.github.io/web-audio-api/#rendering-loop step 4.4.2.1 invokes an ES
Get. This requires a bunch of setup of the sort you see in https://html.spec.whatwg.org/multipage/webappapis.html#prepare-to-run-script and https://html.spec.whatwg.org/multipage/webappapis.html#prepare-to-run-a-callback and then cleanup afterwards. Similar for the function invocation at step 4.4.2.3. The lack of cleanup makes it unclear what happens with exceptions, if anything, for example. As a specific example, say theGetthrows an exception. The way this algorithm is written right now, the result is somewhere between "undefined behavior" and "the entire rendering-a-graph algorithm invocation is aborted".There's also additional confusion because some of the arguments to this function are defined as IDL values (e.g. outputs is defined as
sequence<sequence<Float32Array>>) but there's nothing here that makes it clear that an IDL-to-ES conversion should happen.I don't quite know what behavior this code is aiming is, especially in terms of microtask behavior; step 4.4.2.4 is not really clear to me in terms of what it's saying (e.g. is that a normative requirement, or an informative note? Normally microtask queuing normative bits are handled by the various existing ES infrastructure...). But it seems like what's really going on here is that
processoris being treated as a Web IDL callback interface with a single method namedprocess, and it might be worth actually defining it that way. Except the[[callable process]]bit has no corresponding equivalent in https://heycam.github.io/webidl/#call-a-user-objects-operation. Maybe we can refactor that algorithm in some way to make that behavior configurable?Note that I am currently trying to review code that attempts to implement this stuff in Gecko, but I'm somewhat blocked by the above issues....
@karlt @domenic