Skip to content
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

AudioWorkletProcessor in Bitcrusher example calls super() with options #2491

Open
chrisguttandin opened this issue May 23, 2022 · 6 comments · May be fixed by #2492
Open

AudioWorkletProcessor in Bitcrusher example calls super() with options #2491

chrisguttandin opened this issue May 23, 2022 · 6 comments · May be fixed by #2492

Comments

@chrisguttandin
Copy link
Contributor

Describe the issue

The IDL of an AudioWorkletProcessor defines its constructor without any arguments.

https://webaudio.github.io/web-audio-api/#AudioWorkletProcessor

But the Bitcrusher example calls super() with the options passed into the Bitcrusher constructor.

https://webaudio.github.io/web-audio-api/#the-bitcrusher-node

The processor of the VU Meter example doesn't pass on the options.

Where Is It

It's in line 19 of EXAMPLE 16.

Additional Information

I'm happy to provide a PR which just removes line 17-18 of EXAMPLE 16 and replaces line 19 with a call to super() without any arguments.

@hoch
Copy link
Member

hoch commented May 23, 2022

I would appreciate if you submit a PR for the group! :)

@hoch
Copy link
Member

hoch commented May 23, 2022

I gave a second look on this. Here's what we're really missing:

https://webaudio.github.io/web-audio-api/#dom-audioworkletnode-audioworkletnode

  • In step 13, the algorithm invokes the AWP constructor with processor construction data which has nodeName, node, serializedOptions, serializedProcessorPort.

https://webaudio.github.io/web-audio-api/#dom-audioworkletprocessor-audioworkletprocessor

  • The algorithm doesn't have a way to access serializedOptions from the algorithm above.

Thus, this can be fixed by saying - invoke the constructor of AWP with the argument of processorOptions which is serializedOptions passed from the AWN constructor.

Sorry that I came up with this idea after your created a PR. I hope we can discuss this in the teleconference.

@chrisguttandin
Copy link
Contributor Author

I noticed the inconsistency in the examples when I double-checked the spec for answering this question on Stack Overflow.

I thought it is actually a feature that it is possible to access the options in the custom processor but without having the ability to alter them. It ensures that the AudioWorkletNode and the AudioWorkletProcessor have the same configuration.

As far as I know all current implementations work somehow like this:

class CustomProcessor extends AudioWorkletProcessor {
    constructor(options) {
        // the options can be accessed here...

        // but the call to super doesn't accept any argument (or ignores it)
        // and uses an unmodified copy of the options
        super();
    }
}

@guest271314

This comment was marked as off-topic.

@hoch
Copy link
Member

hoch commented Jun 2, 2022

@chrisguttandin Your PR looks great and has nothing to do with the problem that I described in #2491 (comment). I'll merge it after adding some markups around the change.

Spec-wise, we need to explain how the processorOptions from the AudioWorkletNode side gets passed to the subclass constructor of AudioWorkletProcessor. The super() of AWP doesn't accept any argument, but the subclass can take advantage of that. Fortunately, the current implementation of Chrome and FireFox supports this but this behavior is not clearly defined in the spec.

We believe a simple IDL fix won't be enough to explain, so we need to come up with a short prose for it.

Thanks for pointing that out!

@hoch
Copy link
Member

hoch commented Sep 14, 2022

We have a plan to add steps to consume processorOptions in AudioWorkletProcessor constructor (which is super() call):
https://webaudio.github.io/web-audio-api/#AudioWorketProcessor-constructors

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants