Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Dec 8, 2025

This API was added when audioworklets were first added, but it doesn't
look like it was intended to work with invalid inputs: 5402fc9.

At least none of the original usages in 5402fc9 checked for

@sbc100 sbc100 requested review from cwoffenden and juj December 8, 2025 22:04
@sbc100 sbc100 force-pushed the emscriptenGetAudioObject branch from 8e311af to 587a47f Compare December 8, 2025 22:05
This API was added when audioworklets were first added, but it doesn't
look like it was intended to work with invalid inputs: 5402fc9.

At least none of the original usages in 5402fc9 checked for
undefined return values.
@sbc100 sbc100 force-pushed the emscriptenGetAudioObject branch from 587a47f to 60e5204 Compare December 8, 2025 22:12
$emscriptenGetAudioObject: (objectHandle) => {
#if ASSERTIONS || WEBAUDIO_DEBUG
emAudioExpectNodeOrContext(objectHandle, 'emscriptenGetAudioObject');
#endif
Copy link
Collaborator

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

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

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

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?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants