options should be options.processorOptions at Web Audio worklet-recorder.js

[guest271314]

At https://github.com/web-platform-tests/wpt/commit/d5be80a86d4f938250c075ac12414ad47516969c#diff-f3873c920bd541f45825be596c3a4a6fR10 options should be options.processorOptions else the properties passed at a plain object not having property processorOptions will not be defined at AudioWorkletProcessor.

const aw = new AudioWorkletNode(context, "recorder-processor", {0:1})

results in

console.log(options) // {numberOfInputs: 1, numberOfOutputs: 1}

where

const aw = new AudioWorkletNode(context, "recorder-processor", {processorOptions:{0:1}})

results in

console.log(options) // {numberOfInputs: 1, numberOfOutputs: 1, processorOptions: {0: 1}}

within AudioWorkletProcessor.

Specification https://webaudio.github.io/web-audio-api/#dom-audioworkletnodeoptions-processoroptions

cc @yutakahirano

[yutakahirano]

cc: @hoch

[hoch]

Per the instantiation algorithm step 2 in the second box, the behavior shown above is correct.

The node option is passed to the processor as-is.

[karlt]

I assume the issue is that the RecorderProcessor constructor tests for options.duration, which will always be undefined.

(The test for options.channelCount is correct because AudioNodeOptions has a channelCount member.)

But also, processorOptions has a default value of null. With

const aw = new AudioWorkletNode(context, "recorder-processor", {0:1})

I don't know why console.log(options) in the processor does not show

{numberOfInputs: 1, numberOfOutputs: 1, processorOptions: null}

but the console.log issue is not a problem with the test.

[guest271314]

The node option is passed to the processor as-is.

The linked code implies that a plain JavaScript object having any properties. That is not the case. The object MUST have a property named processorOptions. A plain JavaScript object having a value duration will not be defined within the constructor at AudioWorkletProcessor. Consider createMediaStreamDesitnation() with a live MediaStreamTrack of indeterminate duration being connected to the AudioWorkletNode. Where will the value of duration be derived from.

Can you post code where a plain JavaScript object having a property duration const aw = new AudioWorkletNode(context, "recorder-processor", {duration:1}) will be defined at AudioWorkletProcessor?

[hoch]

Again, why is it a MUST? processorOptions is by default null and it's nullable and not required: https://webaudio.github.io/web-audio-api/#dictdef-audioworkletnodeoptions

See here for the fix: https://github.com/web-platform-tests/wpt/commit/d5be80a86d4f938250c075ac12414ad47516969c#r34434171

[karlt]

options will always have a processorOptions member because that member has a default value, but options.processorOptions is by default not an object, and so should be tested before trying to access its members. i.e. assuming options.processorOptions.duration is never 0,

this._recordDuration =
    (options.processorOptions && options.processorOptions.duration) || 1;

e.g.

{ let processorOptions = null; processorOptions.duration; }
=> TypeError: processorOptions is null

[guest271314]

Again, why is it a MUST? processorOptions is by default null and it's nullable and not required: https://webaudio.github.io/web-audio-api/#dictdef-audioworkletnodeoptions

See here for the fix: d5be80a#r34434171

For clarity. No, processorOptions is NOT null at the actual implementation at Chromium 77 (FWIW, allowing for the fact that Chromium implementations and crashes are meaningless in the scheme of testing and implementer concern).

The language used at the linked comment is the why as to MUST. That is, if clarity is an important factor of your specification.

Neither AudioWorkletNodeOptions or AudioNodeOptions has duration as a valid dictionary member. So at the l.18 it is undefined and falls back to 1. So the test runs without error. However, I agree that this needs to be fixed.

The result of the code should obvious why clarification is needed in the specification. IF a plain JavaScript object passed as optionsProcessor DOES NOT have a property set named "processorOptions" no options will be defined other than numberOfChannels and numberOfInputs

{numberOfInputs: 1, numberOfOutputs: 1}

That fact is not immediately clear at the specification. That is why when tested the code in this repository filed this issue.

If you want your specification to be clear on what MUST be a property of the plain JavaScript object, then state that unambiguously at your specification.

It is not clear what you mean by "this needs to be fixed" at the linked code? That the code at the example for AudioWorklet re this issue needs to be fixed is obvious https://plnkr.co/edit/GN6He8?p=preview. Do you mean that the implementation needs to be fixed to allow any JavaScript plain object, or that, again, referring to the fact that processorOptions is NOT null, the implementation needs to be fixed so that processorOption is a default property of an options object that MUST be a defined parameter options that MUST be a default parameter to AudioWorkletNode constructor?

[hoch]

Sorry about the confusion. I think we're dealing with two problems:

1. On the spec, AudioWorkletNodeOptions.processorOptions should be neither nullable nor defaulting to null. This is not the behavior we want to have, and I think this is an oversight on the spec. I filed an issue to fix.
2. The test code also should be fixed as shown in https://github.com/web-platform-tests/wpt/issues/17662#issuecomment-514813953.

[guest271314]

@hoch The referenced code in this repository has not yet been updated, correct?

Should this issue be closed?

[hoch]

1 is done now. I'll fix 2 soon.