Skip to content

fix: added missing options to EmscriptenAudioWorkletNodeCreateOptions closes #23982 #25497

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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+35 −5
Open

fix: added missing options to EmscriptenAudioWorkletNodeCreateOptions closes #23982 #25497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
32ee9d9
fix: added missing options to EmscriptenAudioWorkletNodeCreateOptions
Oct 5, 2025
d9e8099
fix: updated channelCount, mode and interpretation with proper types …
Oct 6, 2025
9b5b7dc
added additional fields to struct for test cases
Oct 9, 2025
f9ad385
WIP pls work
Oct 9, 2025
2d676c5
I think im done now
Oct 9, 2025
8693ac7
this time fs
Oct 9, 2025
8a67632
nah bro istg this will work
Oct 9, 2025
1fd325d
life taught me lessons
Oct 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/lib/libwebaudio.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,9 @@ var LibraryWebAudio = {
numberOfInputs: {{{ makeGetValue('options', C_STRUCTS.EmscriptenAudioWorkletNodeCreateOptions.numberOfInputs, 'i32') }}},
numberOfOutputs: optionsOutputs,
outputChannelCount: readChannelCountArray({{{ makeGetValue('options', C_STRUCTS.EmscriptenAudioWorkletNodeCreateOptions.outputChannelCounts, 'i32*') }}}, optionsOutputs),
channelCount: (function(){ var v = {{{ makeGetValue('options', C_STRUCTS.EmscriptenAudioWorkletNodeCreateOptions.channelCount, 'u32') }}}; return v > 0 ? v : undefined; })(),
channelCountMode: (function(){ var v = {{{ makeGetValue('options', C_STRUCTS.EmscriptenAudioWorkletNodeCreateOptions.channelCountMode, 'i32') }}}; var arr = ['max','clamped-max','explicit']; return arr[v] || undefined; })(),
channelInterpretation: (function(){ var v = {{{ makeGetValue('options', C_STRUCTS.EmscriptenAudioWorkletNodeCreateOptions.channelInterpretation, 'i32') }}}; var arr = ['speakers','discrete']; return arr[v] || undefined; })(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The anonymous functions and || undefined should be unnecessary here, they take up code size for no benefit.

E.g. channelCountMode: ['max','clamped-max','explicit'][{{{ makeGetValue(...) }}}], looks like would be enough here?

One thing to verify here is how do older browsers behave (or Safari) that do not yet support these options? If you run in an older browser that does not support any of this, would that cause an error?

It would be good to have a short test of non-default channelCountMode and non-default channelInterpretation to ensure that it works.

processorOptions: {
callback,
userData,
Expand Down
7 changes: 5 additions & 2 deletions src/struct_info.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1260,7 +1260,7 @@
],
"EmscriptenWebSocketCreateAttributes": [
"protocols"
]
]
}
},
{
Expand Down Expand Up @@ -1293,7 +1293,10 @@
"EmscriptenAudioWorkletNodeCreateOptions": [
"numberOfInputs",
"numberOfOutputs",
"outputChannelCounts"
"outputChannelCounts",
"channelCount",
"channelCountMode",
"channelInterpretation"
]
}
},
Expand Down
5 changes: 4 additions & 1 deletion src/struct_info_generated.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,10 @@
"samplesPerChannel": 4
},
"EmscriptenAudioWorkletNodeCreateOptions": {
"__size__": 12,
"__size__": 24,
"channelCount": 12,
"channelCountMode": 16,
"channelInterpretation": 20,
"numberOfInputs": 0,
"numberOfOutputs": 4,
"outputChannelCounts": 8
Expand Down
5 changes: 4 additions & 1 deletion src/struct_info_generated_wasm64.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,10 @@
"samplesPerChannel": 4
},
"EmscriptenAudioWorkletNodeCreateOptions": {
"__size__": 16,
"__size__": 32,
"channelCount": 16,
"channelCountMode": 24,
"channelInterpretation": 28,
"numberOfInputs": 0,
"numberOfOutputs": 4,
"outputChannelCounts": 8
Expand Down
15 changes: 15 additions & 0 deletions system/include/emscripten/webaudio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,17 @@ typedef struct AudioParamFrame

typedef bool (*EmscriptenWorkletNodeProcessCallback)(int numInputs, const AudioSampleFrame *inputs, int numOutputs, AudioSampleFrame *outputs, int numParams, const AudioParamFrame *params, void *userData4);

typedef enum {
WEBAUDIO_CHANNEL_COUNT_MODE_MAX = 0,
WEBAUDIO_CHANNEL_COUNT_MODE_CLAMPED_MAX = 1,
WEBAUDIO_CHANNEL_COUNT_MODE_EXPLICIT = 2
} WEBAUDIO_CHANNEL_COUNT_MODE;

typedef enum {
WEBAUDIO_CHANNEL_INTERPRETATION_SPEAKERS = 0,
WEBAUDIO_CHANNEL_INTERPRETATION_DISCRETE = 1
} WEBAUDIO_CHANNEL_INTERPRETATION;

typedef struct EmscriptenAudioWorkletNodeCreateOptions
{
// How many audio nodes does this node take inputs from? Default=1
Expand All @@ -134,6 +145,10 @@ typedef struct EmscriptenAudioWorkletNodeCreateOptions
int numberOfOutputs;
// For each output, specifies the number of audio channels (1=mono/2=stereo/etc.) for that output. Default=an array of ones for each output channel.
int *outputChannelCounts;
unsigned long channelCount;
WEBAUDIO_CHANNEL_COUNT_MODE channelCountMode;
WEBAUDIO_CHANNEL_INTERPRETATION channelInterpretation;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add here a short comment describing the default values of each?

Hmm it seems that we get the struct initialization regression problem here.. that is, after this addition, all users must explicitly initialize these values to valid numbers; or their code will stop working? E.g. is existing users had code, say,

EmscriptenAudioWorkletNodeCreateOptions options = {
  .numberOfInputs = 2,
  .numberOfOutputs = 2,
};

then when they update Emscripten to recompile their code, they will get implicit

.channelCount = 0,
.channelCountMode = WEBAUDIO_CHANNEL_COUNT_MODE_MAX,
.channelInterpretation = WEBAUDIO_CHANNEL_INTERPRETATION_SPEAKERS

and the spec says, "If this value is set to zero [...] the implementation MUST throw a [NotSupportedError](https://webidl.spec.whatwg.org/#notsupportederror) exception."

So in this form, this update would cause all Web Audio utilizing code to break from running, unless they are updated.

That is a bit annoying, since there is necessarily not a good way for developers to figure that out, and they might raise a bug report "I updated Emscripten and now I am getting an NotSupportedError exception. It used to work in previous version of Emscripten, so what did you regress?"

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh now I see in the init code there was a v > 0 ? v : undefined added to work around this problem.

Hmm, I suppose that's ok. It would be good to have a comment, e.g.

// the number of channels used when up-mixing and down-mixing connections to any inputs to the node.
// Pass 0 to omit specifying this parameter altogether during context creation.
unsigned long channelCount;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cwoffenden what do you think about the struct init problem for the future?

Should we try to model the struct so that all-zero-init should be preserved as a valid struct init with defaults? E.g.

EmscriptenAudioWorkletNodeCreateOptions options = {};

would provide all zeros, and at JS side, we would always shoehorn the init to map to default values of an Audio Context?

In WebGPU I have been explicitly using global constant data default-initialized struct constants, e.g. here: https://github.com/juj/wasm_webgpu/blob/3ed529956d1ee3f14f64c07769385e1929908f68/lib/lib_webgpu_cpp20.cpp#L34-L45

which would mean we would enforce users to do

EmscriptenAudioWorkletNodeCreateOptions options = AUDIO_WORKLET_NODE_CREATE_DEFAULT_INITIALIZER;

if they want to ensure forward compatibility with default values into the future.

The default-zero-init for all fields would be preferred for code size and simplicity, though only if we can expect that any upcoming fields in the Web Audio spec would reasonably map from zero to the default. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m out for a week or so but jotting some quick thoughts down, I personally prefer (and use) the zero initialiser, with, for example, an enum UNDEFINED mapped to zero, and other special treatment of zeros. I find it cleaner than building a struct with all the defaults, and the constructor passed the struct is probably going to have all the sanity checks anyway, so using that to create sane defaults is a nice byproduct.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally prefer [...] the zero initialiser

Yeah, of course using zeroes is nice, though this is not a question of (just) programming style or preference. Rather the risk there is whether we will always be technically able to do this.

For example, in this specific PR case we were just barely saved by a lucky happenstance that the new added param channelCount has zero be an invalid value in the upstream Web Audio spec.

If that were not the case, then the above v > 0 ? v : undefined workaround would not be possible, but user breakage would have been inevitable, unless we also added some extra bool channelCountSpecified; variable.

Though I suppose the resolution here is clear that we don't want to go the route of initializer structs (or functions, like pthread_attr_init() for example has), and deal with the problematic case when/if one arises in the future.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we making the design changes and then merging this or is this good enough?


} EmscriptenAudioWorkletNodeCreateOptions;

// Instantiates the given AudioWorkletProcessor as an AudioWorkletNode, which continuously calls the specified processCallback() function on the browser's audio thread to perform audio processing.
Expand Down
5 changes: 4 additions & 1 deletion test/webaudio/audioworklet.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,10 @@ void AudioWorkletProcessorCreated(EMSCRIPTEN_WEBAUDIO_T audioContext, bool succe
EmscriptenAudioWorkletNodeCreateOptions options = {
.numberOfInputs = 0,
.numberOfOutputs = 1,
.outputChannelCounts = outputChannelCounts
.outputChannelCounts = outputChannelCounts,
.channelCount = 1,
.channelCountMode = WEBAUDIO_CHANNEL_COUNT_MODE_EXPLICIT,
.channelInterpretation = WEBAUDIO_CHANNEL_INTERPRETATION_SPEAKERS,
};

// Instantiate the noise-generator Audio Worklet Processor.
Expand Down