Skip to content
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.

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

Don't force TextDe/Encoder #3329

Merged
merged 1 commit into from
Feb 27, 2023
Merged

Conversation

daxpedda
Copy link
Collaborator

Currently the generated JS shim calls TextDe/Encoder, even if it's not required by the user.
This forces users to provide a polyfill even when not needed.
Even if just a stub is provided as a polyfill, like done in the wasm-audio-worklet example, it would still require the class TextDe/Encoder to be defined, which prevents support detection by downstream users.

This adjust calls to TextDe/Encoder to do nothing if it's not available.
I don't think this has any use-case outside audio worklets.
This doesn't prevent users from using an actual polyfill if desired.

Cc @lukaslihotzki.
Fixes #2367.

Copy link
Collaborator

@Liamolucko Liamolucko left a comment

Choose a reason for hiding this comment

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

Thanks!

@Liamolucko Liamolucko merged commit 78421dc into rustwasm:main Feb 27, 2023
@daxpedda daxpedda mentioned this pull request Feb 27, 2023
@BlinkyStitt
Copy link

So I agree that TextDe/Encoder isn't needed outside of audio worklets, but your diff here removed the code from audio worklets where I do need it. Was that intentional? What is the best way to get this polyfilled inside of an Audio worklet?

78421dc#diff-07cf5528c3f9b4e22f2489de3616bd3ec5adbfeea002b3cad8d7f9d9cd5c68b0

@daxpedda
Copy link
Collaborator Author

It was intentional indeed, as its not required anymore to get audio worklets to run.

Using the usual Worklet.addModule() beforehand should perfectly suffice to inject any polyfill.

In general though I would recommend against using it in audio worklets, as any kind of memory allocation inside the audio worklet can cause stuttering among other issues, which is why the API was removed there to begin with. See WebAudio/web-audio-api#2499 (comment) for more information.

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.

Unblock AudioWorklets: Find an alternative to TextEncoder / TextDecoder
3 participants