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

Fix broken JS glue for AUDIO_WORKLETS with EXPORT_ES6 #21192

Merged
merged 1 commit into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions src/postamble_minimal.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,19 +197,19 @@ WebAssembly.instantiate(Module['wasm'], imports).then((output) => {
#if AUDIO_WORKLET
// If we are in the audio worklet environment, we can only access the Module object
// and not the global scope of the main JS script. Therefore we need to export
// all functions that the audio worklet scope needs onto the Module object.
Module['wasmTable'] = wasmTable;
// all symbols that the audio worklet scope needs onto the Module object.
#if ASSERTIONS
// In ASSERTIONS-enabled builds, the following symbols have gotten read-only getters
// saved to the Module. Remove those getters so we can manually export the stack
// functions here.
// In ASSERTIONS-enabled builds, the needed symbols have gotten read-only getters
// saved to the Module. Remove the getters so we can manually export them here.
delete Module['stackSave'];
delete Module['stackAlloc'];
delete Module['stackRestore'];
delete Module['wasmTable'];
#endif
Module['stackSave'] = stackSave;
Module['stackAlloc'] = stackAlloc;
Module['stackRestore'] = stackRestore;
Module['wasmTable'] = wasmTable;
#endif

#if !IMPORTED_MEMORY
Expand Down
11 changes: 2 additions & 9 deletions src/preamble.js
Original file line number Diff line number Diff line change
Expand Up @@ -608,14 +608,14 @@ function instrumentWasmTableWithAbort() {
#endif

var wasmBinaryFile;
#if EXPORT_ES6 && USE_ES6_IMPORT_META && !SINGLE_FILE
#if EXPORT_ES6 && USE_ES6_IMPORT_META && !SINGLE_FILE && !AUDIO_WORKLET
if (Module['locateFile']) {
Copy link
Member

Choose a reason for hiding this comment

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

This ifdef seems to only affect locateFile, how does that related to new URL()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello, thanks for reviewing.

The idea was to show that this ifdef and the next ifdef are linked/were changed for the same reason.

But it seems having it worded like that might be worse than simply not having the comment there at all. I'll remove it.

Copy link
Member

Choose a reason for hiding this comment

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

This change looks not removed yet?

#endif
wasmBinaryFile = '{{{ WASM_BINARY_FILE }}}';
if (!isDataURI(wasmBinaryFile)) {
wasmBinaryFile = locateFile(wasmBinaryFile);
}
#if EXPORT_ES6 && USE_ES6_IMPORT_META && !SINGLE_FILE // in single-file mode, repeating WASM_BINARY_FILE would emit the contents again
#if EXPORT_ES6 && USE_ES6_IMPORT_META && !SINGLE_FILE && !AUDIO_WORKLET // In single-file mode, repeating WASM_BINARY_FILE would emit the contents again. For an Audio Worklet, we cannot use `new URL()`.
} else {
#if ENVIRONMENT_MAY_BE_SHELL
if (ENVIRONMENT_IS_SHELL)
Expand Down Expand Up @@ -1000,13 +1000,6 @@ function createWasm() {
#if ASSERTIONS && !PURE_WASI
assert(wasmTable, 'table not found in wasm exports');
#endif

#if AUDIO_WORKLET
// If we are in the audio worklet environment, we can only access the Module object
// and not the global scope of the main JS script. Therefore we need to export
// all functions that the audio worklet scope needs onto the Module object.
Module['wasmTable'] = wasmTable;
#endif
#endif

#if hasExportedSymbol('__wasm_call_ctors')
Expand Down
2 changes: 2 additions & 0 deletions test/test_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -5747,6 +5747,8 @@ def test_full_js_library_strict(self):
'pthreads_and_closure': (['-pthread', '--closure', '1', '-Oz'],),
'minimal_runtime': (['-sMINIMAL_RUNTIME'],),
'minimal_runtime_pthreads_and_closure': (['-sMINIMAL_RUNTIME', '-pthread', '--closure', '1', '-Oz'],),
'pthreads_es6': (['-pthread', '-sPTHREAD_POOL_SIZE=2', '-sEXPORT_ES6'],),
'es6': (['-sEXPORT_ES6'],),
})
def test_audio_worklet(self, args):
if '-sMEMORY64' in args and is_firefox():
Expand Down
21 changes: 13 additions & 8 deletions tools/link.py
Original file line number Diff line number Diff line change
Expand Up @@ -1339,9 +1339,12 @@ def phase_linker_setup(options, state, newargs):
settings.AUDIO_WORKLET_FILE = unsuffixed(os.path.basename(target)) + '.aw.js'
settings.JS_LIBRARIES.append((0, shared.path_from_root('src', 'library_webaudio.js')))
if not settings.MINIMAL_RUNTIME:
# If we are in the audio worklet environment, we can only access the Module object
# and not the global scope of the main JS script. Therefore we need to export
# all symbols that the audio worklet scope needs onto the Module object.
# MINIMAL_RUNTIME exports these manually, since this export mechanism is placed
# in global scope that is not suitable for MINIMAL_RUNTIME loader.
settings.EXPORTED_RUNTIME_METHODS += ['stackSave', 'stackAlloc', 'stackRestore']
settings.EXPORTED_RUNTIME_METHODS += ['stackSave', 'stackAlloc', 'stackRestore', 'wasmTable']

if settings.FORCE_FILESYSTEM and not settings.MINIMAL_RUNTIME:
# when the filesystem is forced, we export by default methods that filesystem usage
Expand Down Expand Up @@ -2349,13 +2352,15 @@ def modularize():
'script_url_node': script_url_node,
'src': src,
}
# Given the async nature of how the Module function and Module object
# come into existence in AudioWorkletGlobalScope, store the Module
# function under a different variable name so that AudioWorkletGlobalScope
# will be able to reference it without aliasing/conflicting with the
# Module variable name.
if settings.AUDIO_WORKLET and settings.MODULARIZE:
src += f'globalThis.AudioWorkletModule = {settings.EXPORT_NAME};'

# Given the async nature of how the Module function and Module object
# come into existence in AudioWorkletGlobalScope, store the Module
# function under a different variable name so that AudioWorkletGlobalScope
# will be able to reference it without aliasing/conflicting with the
# Module variable name. This should happen even in MINIMAL_RUNTIME builds
# for MODULARIZE and EXPORT_ES6 to work correctly.
if settings.AUDIO_WORKLET and settings.MODULARIZE:
src += f'globalThis.AudioWorkletModule = {settings.EXPORT_NAME};'

# Export using a UMD style export, or ES6 exports if selected
if settings.EXPORT_ES6:
Expand Down
Loading