| Summary: | AudioWorkletProcessor fails to load when some parameter descriptors with minValue/maxValue are specified | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Casey Primozic <casey> |
| Component: | Web Audio | Assignee: | Nobody <webkit-unassigned> |
| Status: | NEW --- | ||
| Severity: | Minor | CC: | casey, cdumez, chrisguttandin, jer.noble, karlcow, webkit-bug-importer |
| Priority: | P2 | Keywords: | BrowserCompat, InRadar |
| Version: | Safari 16 | ||
| Hardware: | Mac (Apple Silicon) | ||
| OS: | macOS 13 | ||
| URL: | https://webaudio.github.io/web-audio-api/#dom-audioworkletglobalscope-registerprocessor | ||
One thing I forgot to mention is that when this bug happens, the promise returned by `ctx.audioWorklet.addModule` itself doesn't reject. The AWP registration failure happens silently, and the bug manifests when trying to create a `AudioWorkletNode` with the name of the processor. Likely due to the following logic in AudioWorkletGlobalScope::registerProcessor():
```
if (!parameterDescriptorsValue.isUndefined()) {
parameterDescriptors = convert<IDLSequence<IDLDictionary<AudioParamDescriptor>>>(*globalObject, parameterDescriptorsValue);
RETURN_IF_EXCEPTION(scope, Exception { ExistingExceptionError });
UNUSED_PARAM(parameterDescriptors);
HashSet<String> paramNames;
for (auto& descriptor : parameterDescriptors) {
auto addResult = paramNames.add(descriptor.name);
if (!addResult.isNewEntry)
return Exception { NotSupportedError, makeString("parameterDescriptors contain duplicate AudioParam name: ", name) };
if (descriptor.defaultValue < descriptor.minValue)
return Exception { InvalidStateError, makeString("AudioParamDescriptor with name '", name, "' has a defaultValue that is less than the minValue") };
if (descriptor.defaultValue > descriptor.maxValue)
return Exception { InvalidStateError, makeString("AudioParamDescriptor with name '", name, "' has a defaultValue that is greater than the maxValue") };
}
}
```
Relevant specification: https://webaudio.github.io/web-audio-api/#dom-audioworkletglobalscope-registerprocessor ``` For each descriptor of parameterDescriptorSequence: 1. Let paramName be the value of the member name in descriptor. Throw a NotSupportedError if paramNames already contains paramName value. 2. Append paramName to the paramNames array. 3. Let defaultValue be the value of the member defaultValue in descriptor. 4. Let minValue be the value of the member minValue in descriptor. 5. Let maxValue be the value of the member maxValue in descriptor. 6. If the expresstion minValue <= defaultValue <= maxValue is false, throw an InvalidStateError. ``` So it seems our implementation is close to the specification text. What may be surprising though is that the addModule() promise apparently doesn't get rejected, even though the module script got an exception when running. (In reply to Chris Dumez from comment #5) > So it seems our implementation is close to the specification text. What may > be surprising though is that the addModule() promise apparently doesn't get > rejected, even though the module script got an exception when running. Hmm, it actually seems it is per specification to not reject the promise if the audio worklet script throws an exception. See https://wpt.live/worklets/audio-worklet-import.https.html Test: Importing a script which throws should still resolve the given promise. If I forward the exception from the audioworklet to reject the promise, I start failing this standards test. (In reply to Chris Dumez from comment #6) > (In reply to Chris Dumez from comment #5) > > So it seems our implementation is close to the specification text. What may > > be surprising though is that the addModule() promise apparently doesn't get > > rejected, even though the module script got an exception when running. > > Hmm, it actually seems it is per specification to not reject the promise if > the audio worklet script throws an exception. > > See https://wpt.live/worklets/audio-worklet-import.https.html > Test: Importing a script which throws should still resolve the given promise. > > If I forward the exception from the audioworklet to reject the promise, I > start failing this standards test. Relevant specification for addModule(): https://html.spec.whatwg.org/multipage/worklets.html#dom-worklet-addmodule Chrome/Blink indeed doesn't throw for invalid parameter descriptor values, I looked at their code and they have a FIXME comment about it:
// TODO(crbug.com/1078546): The steps 7.3.3 ~ 7.3.6 are missing.
Steps 7.3.3 ~ 7.3.6 are exactly the ones about validating the parameter values.
I filed https://github.com/WebAudio/web-audio-api/issues/2534 and cc'd the Blink engineer who works on this, and who is also an editor for this spec. At the moment, our behavior seems the most spec-compliant, though not interoperable with Blink. This is not an ideal situation but it is not clear yet how we should proceed. |
When defining `AudioWorkletProcessor`s that have parameter descriptors which define `minValue` or `maxValue` can cause the AWP to fail to register in some cases. It happens when the `defaultValue` for the param descriptor is out of the range implied by `minValue` and `maxValue`. For example, defining an AWP with this param descriptor fails: ``` static get parameterDescriptors() { return [{ name: "x", minValue: 0.01 }]; } ``` I believe this happens because the default `defaultValue` is 0, which is below the set `minValue`. Changing it to this makes it register successfully: ``` static get parameterDescriptors() { return [{ name: "x", minValue: 0.01, defaultValue: 0.4 }]; } ``` The same thing happens for `maxValue` if `defaultValue` is set higher than `maxValue`. I'm not sure what the spec says about situations like this, but it works in other browsers I've tested. Here is a working minimal repro of the bug: index.html: ``` <html> <head> <script type="text/javascript"> const ctx = new AudioContext(); ctx.audioWorklet .addModule("/awp.js") .then(() => { const node = new AudioWorkletNode(ctx, "bug-repro-awp"); document.write("SUCCESS"); }) .catch((e) => { document.write("ERROR: " + e); }); </script> </head> </html> ``` awp.js: ``` class BugReproAWP extends AudioWorkletProcessor { static get parameterDescriptors() { return [{ name: "bottom_ratio", minValue: 0.01 }]; } constructor(_options) {} process(inputs, outputs, params) { return true; } } registerProcessor("bug-repro-awp", BugReproAWP); ```