[AUDIO_WORKLET] Support locateFile when loading audio worklet js file#20318
[AUDIO_WORKLET] Support locateFile when loading audio worklet js file#20318FloRecisio wants to merge 2 commits intoemscripten-core:mainfrom
Conversation
RReverser
left a comment
RReverser
left a comment
There was a problem hiding this comment.
I'm running into this issue as well. The fix looks correct IMO.
|
Needs rebase & conflict fix. |
|
|
||
| // TODO: In MINIMAL_RUNTIME builds, read this file off of a preloaded Blob, and/or embed from a string like with WASM_WORKERS==2 mode. | ||
| audioWorklet.addModule('{{{ TARGET_BASENAME }}}.aw.js').then(() => { | ||
| audioWorklet.addModule(locateFile('{{{ TARGET_BASENAME }}}.aw.js')).then(() => { |
There was a problem hiding this comment.
Does locateFile exist under MINIMAL_RUNTIME?
Can we get a test for this?
There was a problem hiding this comment.
That too, yeah.
Ideally, I'd rather merge aw.js into main JS, just like we did for pthread workers - there's nothing stopping it, and it would result in one less tiny file to download and worry about.
For now, any fix would be great though, given how long it's been broken.
There was a problem hiding this comment.
I updated the PR, however I don't now how do you want to test this.
There was a problem hiding this comment.
Finally it didn't pass the existing tests anyway
There was a problem hiding this comment.
For testing, you could copy one of the existing audio worklet tests in test_browser.py but generate the JS in a subfolder - that should reproduce the original issue.
There was a problem hiding this comment.
The existing tests are already parameterised over various compile options, so should be able to check MINIMAL_RUNTIME vs MODULARIZE vs others as well.
…into audio_worklets_module_locate_file # Conflicts: # src/lib/libwebaudio.js
sbc100
changed the title
sbc100
changed the title
sbc100
changed the title
|
This change would be of great help for me! Would love to see this added <3 |
| // TODO: In MINIMAL_RUNTIME builds, read this file off of a preloaded Blob, | ||
| // and/or embed from a string like with WASM_WORKERS==2 mode. | ||
| audioWorklet.addModule('{{{ TARGET_BASENAME }}}.aw.js').then(() => { | ||
| audioWorklet.addModule(locateFile('{{{ TARGET_BASENAME }}}.aw.js')).then(() => { |
There was a problem hiding this comment.
How about
audioWorklet.addModule(
#if MINIMAL_RUNTIME
'{{{ TARGET_BASENAME }}}.aw.js'
#else
locateFile('{{{ TARGET_BASENAME }}}.aw.js')
#endif
).then(() => {
The locateFile function does not currently exist in MINIMAL_RUNTIME.
There was a problem hiding this comment.
I think we also need the new URL(...) pattern here as well for ES6 bundlers.
Essentially copy the pattern from regular ww.js workers.
There was a problem hiding this comment.
@sbc100 At this point, maybe this should be a parseTools macro for arbitrary resources?
There was a problem hiding this comment.
If you can think of way to express that that SGTM
There was a problem hiding this comment.
Since original PR author might not be around, I posted a PR #24044 to gate the MINIMAL_RUNTIME part, and add a test.
parseTools macro and ES6 bundlers are good ideas that can be added in later PRs.
5 participants
Fixes: #20316