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 for AUDIO_WORKLET + MODULARIZE #20249

Merged
merged 5 commits into from
Oct 17, 2023

Conversation

imiskolee
Copy link
Contributor

@imiskolee imiskolee commented Sep 14, 2023

move the js code to fix syntax error.

Fixes: #19796

@sbc100
Copy link
Collaborator

sbc100 commented Sep 14, 2023

I guess we should add a test case for AUDIO_WORKLET + MODULARIZE? Perhaps modify one of the existing tests using @parameterize? (And verify that it fails without this patch).

@sbc100 sbc100 requested a review from juj September 14, 2023 16:04
@sbc100 sbc100 changed the title fix bug: https://github.com/emscripten-core/emscripten/issues/19796 Fox for AUDIO_WORKLET + MODULARIZE Sep 14, 2023
@sbc100
Copy link
Collaborator

sbc100 commented Sep 14, 2023

(I hope you don't mind, I re-titled the PR to describe what it does)

@imiskolee
Copy link
Contributor Author

I guess we should add a test case for AUDIO_WORKLET + MODULARIZE? Perhaps modify one of the existing tests using @parameterize? (And verify that it fails without this patch).

@imiskolee imiskolee closed this Sep 14, 2023
@sbc100 sbc100 changed the title Fox for AUDIO_WORKLET + MODULARIZE Fix for AUDIO_WORKLET + MODULARIZE Sep 14, 2023
@imiskolee imiskolee reopened this Sep 14, 2023
@imiskolee
Copy link
Contributor Author

@sbc100 I has a compile issue now:

  1. when I used bash '-sAUDIO_WORKLET', '-sWASM_WORKERS','-sMODULARIZE=1' , it's showing error:
test/runner browser.test_audio_worklet_modularize

emcc: error: Due to collision in variable name "Module", the shell file "/Users/miskolee/develop/src/github.com/emscripten/src/shell.html" is not compatible with build options "-sMODULARIZE -sEXPORT_NAME=Module". Either provide your own shell file, change the name of the export to something else to avoid the name collision. (see https://github.com/emscripten-core/emscripten/issues/7950 for details)

I also tried

'-sAUDIO_WORKLET', '-sWASM_WORKERS','-sMODULARIZE=1','-sEXPORT_NAME=MyModule'

emcc: error: Customizing EXPORT_NAME requires that the HTML be customized to use that name (see https://github.com/emscripten-core/emscripten/issues/10086)
'-sAUDIO_WORKLET', '-sWASM_WORKERS','-sEXPORT_ES6=1'
image

@imiskolee
Copy link
Contributor Author

imiskolee commented Sep 16, 2023

AudioWorkletGlobalScope have not wasmTable field. so *.aw.js file also failed. Uncaught TypeError: Cannot read properties of undefined (reading 'get')
at new WasmAudioWorkletProcessor (TestWasm.aw.js:30:51)

Looks like the wasmTable can not pass to worker, but the aw.js depends this.

@imiskolee imiskolee closed this Sep 16, 2023
@imiskolee imiskolee force-pushed the fix/generated-js-issue-19796 branch from 94a4e38 to c65a321 Compare September 16, 2023 15:26
@imiskolee imiskolee reopened this Sep 16, 2023
@imiskolee
Copy link
Contributor Author

Hi,Team: I already add the test case and can be test pass with

test/runner browser.test_audio_worklet_modularize

please take a look this.

@imiskolee imiskolee force-pushed the fix/generated-js-issue-19796 branch 3 times, most recently from e28351b to 481bd3c Compare September 16, 2023 16:27
@imiskolee
Copy link
Contributor Author

@juj please check this.

emcc.py Outdated Show resolved Hide resolved
emcc.py Outdated Show resolved Hide resolved
emcc.py Outdated Show resolved Hide resolved
@imiskolee imiskolee force-pushed the fix/generated-js-issue-19796 branch 2 times, most recently from bcbc8cf to 85f0865 Compare September 22, 2023 07:44
@imiskolee imiskolee requested review from kripken and sbc100 September 22, 2023 14:53
@imiskolee imiskolee force-pushed the fix/generated-js-issue-19796 branch 2 times, most recently from ed48ad3 to 560bdad Compare October 10, 2023 03:15
@imiskolee imiskolee force-pushed the fix/generated-js-issue-19796 branch from 560bdad to 1f9ca55 Compare October 10, 2023 03:17
@sbc100
Copy link
Collaborator

sbc100 commented Oct 10, 2023

Hmm.. it looks like there is some kind circleci issue going on here where it trying to use you plan to run these CI jobs.. @kripken do you know about this issue?

@sbc100
Copy link
Collaborator

sbc100 commented Oct 10, 2023

Something must have changed or be broken with circleci because it didn't do this for the previous run in this PR

@imiskolee
Copy link
Contributor Author

what's next step?

@sbc100
Copy link
Collaborator

sbc100 commented Oct 16, 2023

We just need the CI tests to pass.

In this case it looks like we are being effected by a circleci configuration issue: #20430

@kripken
Copy link
Member

kripken commented Oct 17, 2023

Looks like the CircleCI issue is resolved here, landing.

@kripken kripken merged commit 635bb90 into emscripten-core:main Oct 17, 2023
@imiskolee imiskolee deleted the fix/generated-js-issue-19796 branch October 18, 2023 02:16
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.

Invalid js file when compiling with AUDIO_WORKLET + MODULARIZE
3 participants