WebAudio / web-audio-api

The Web Audio API v1.0, developed by the W3C Audio WG
https://webaudio.github.io/web-audio-api/
Other
1.04k stars 165 forks source link

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

Open chrisguttandin opened 2 years ago

chrisguttandin commented 2 years ago

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 commented 2 years ago

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

hoch commented 2 years ago

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

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

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

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 commented 2 years ago

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 commented 2 years ago

There is indeed inconsistency. numberOfInputs and numberOfOutputs are properties of AudioWorkletNode after construction, not outputChannelCount nor processorOptions.

numberOfInputs, numberOfOutputs, outputChannelCount and processorOptions are not assigned as properties of extended AudioWorkletProcessor class. numberOfInputs, numberOfOutputs, outputChannelCount and processorOptions should be assigned as properties of both AudioWorkletNode and AudioWorkletProcessor extended class instances, and be atomically updated between each other when the properties are set, similar to a proxy object, and reflected in process(inputs, outputs).

This should work something like

class Test {
    constructor({
        numberOfInputs = 1,
        numberOfOutputs = 1,
        outputChannelCount = [1],
    } = {}) {
        Object.assign(this, ...arguments);
    }
}

class TestDefaultOptions extends Test {
    constructor() {
        super({
            numberOfInputs: 1,
            numberOfOutputs: 1,
            outputChannelCount: [2],
        });
        console.log(this.outputChannelCount); // [2]
    }
}

new TestDefaultOptions(); 
hoch commented 2 years ago

@chrisguttandin Your PR looks great and has nothing to do with the problem that I described in https://github.com/WebAudio/web-audio-api/issues/2491#issuecomment-1135152927. 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 commented 1 year ago

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