Skip to content

Add assertions to emscriptenGetAudioObject. NFC #25914

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
sbc100 wants to merge 1 commit into
base: main
Choose a base branch
from
+25 −27
Open

Add assertions to emscriptenGetAudioObject. NFC #25914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 6 additions & 1 deletion src/lib/libwebaudio.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,12 @@ var LibraryWebAudio = {

// Call this function from JavaScript to get the Web Audio object corresponding to the given
// Wasm handle ID.
$emscriptenGetAudioObject: (objectHandle) => emAudio[objectHandle],
$emscriptenGetAudioObject: (objectHandle) => {
#if ASSERTIONS || WEBAUDIO_DEBUG
emAudioExpectNodeOrContext(objectHandle, 'emscriptenGetAudioObject');
#endif
Copy link
Collaborator

@juj juj Dec 9, 2025

Choose a reason for hiding this comment

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

The above call to emAudioExpectNodeOrContext() seems restrictive.

It prevents users from testing if a given handle has been deleted? E.g. one would not be able to write code like this anymore:

if (emscriptenGetAudioObject(someHandle)) {
  console.log('someHandle exists, accessing it...');
} else {
  console.log('someHandle has been deleted, doing something else (maybe deinitializing some JS side structure at shutdown?)');
}

Copy link
Collaborator Author

@sbc100 sbc100 Dec 9, 2025

Choose a reason for hiding this comment

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

Indeed, that is the idea. None of our use cases for the API seems to use it in this way, and none of the other APIs in the this file allow invalid handles to be passed. Unless there is specific use case for allowing invalid handles here I suggest we be consistent. (At least until someone asks for this ability).

Copy link
Collaborator

@cwoffenden cwoffenden Dec 12, 2025

Choose a reason for hiding this comment

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

I'd be interested to know how others are using the API. I agree with putting the test with the handle request, but I only see my own use cases.

Copy link
Collaborator Author

@sbc100 sbc100 Dec 12, 2025

Choose a reason for hiding this comment

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

I'm only aware of the uses within the test code, but maybe you could find more via a github global search or some such?

return emAudio[objectHandle];
},

// Performs the work of getting the AudioContext's render quantum size.
$emscriptenGetContextQuantumSize: (contextHandle) => {
Expand Down
12 changes: 5 additions & 7 deletions test/webaudio/audioworklet_params_mixing.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,11 @@ bool process(int numInputs, const AudioSampleFrame* inputs, int numOutputs, Audi
// it's already fading up or down, we reverse the fade direction).
EM_JS(void, doFade, (EMSCRIPTEN_AUDIO_WORKLET_NODE_T workletID), {
var worklet = emscriptenGetAudioObject(workletID);
if (worklet) {
// Emscripten's API creates these from a C array, indexing them instead of a
// name. Chrome and FF work with 0 but Safari requires the correct "0".
var param = worklet.parameters.get("0");
if (param) {
param.setTargetAtTime((param.value > 0.5) ? 0 : 1, 0 /* same as context.currentTime */, 0.5);
}
// Emscripten's API creates these from a C array, indexing them instead of a
// name. Chrome and FF work with 0 but Safari requires the correct "0".
var param = worklet.parameters.get("0");
if (param) {
param.setTargetAtTime((param.value > 0.5) ? 0 : 1, 0 /* same as context.currentTime */, 0.5);
}
})

Expand Down
33 changes: 14 additions & 19 deletions test/webaudio/audioworklet_test_shared.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,30 +30,25 @@ EMSCRIPTEN_WEBAUDIO_T bassID = 0;
// registered as an internal audio object and the ID returned).
EM_JS(EMSCRIPTEN_WEBAUDIO_T, createTrack, (EMSCRIPTEN_WEBAUDIO_T ctxID, const char* url, bool looping), {
var context = emscriptenGetAudioObject(ctxID);
if (context) {
var audio = document.createElement('audio');
// Number() wrapper is a workaround for UTF8ToString() needing a JS number
// and from64() not being available in EM_JS macros. Fix in UTF8ToString?
audio.src = UTF8ToString(Number(url));
audio.loop = looping;
var track = context.createMediaElementSource(audio);
return emscriptenRegisterAudioObject(track);
}
return 0;
var audio = document.createElement('audio');
// Number() wrapper is a workaround for UTF8ToString() needing a JS number
// and from64() not being available in EM_JS macros. Fix in UTF8ToString?
audio.src = UTF8ToString(Number(url));
audio.loop = looping;
var track = context.createMediaElementSource(audio);
return emscriptenRegisterAudioObject(track);
})

// Toggles the play/pause of a MediaElementAudioSourceNode given its ID
EM_JS(void, toggleTrack, (EMSCRIPTEN_WEBAUDIO_T srcID), {
var source = emscriptenGetAudioObject(srcID);
if (source) {
var audio = source.mediaElement;
if (audio) {
if (audio.paused) {
audio.currentTime = 0;
audio.play();
} else {
audio.pause();
}
var audio = source.mediaElement;
if (audio) {
if (audio.paused) {
audio.currentTime = 0;
audio.play();
} else {
audio.pause();
}
}
})
Expand Down
Loading