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

add WebAssembly for Kws #648

Merged
merged 19 commits into from
Mar 11, 2024
Merged

add WebAssembly for Kws #648

merged 19 commits into from
Mar 11, 2024

Conversation

lovemefan
Copy link
Contributor

No description provided.

```

You should have the following files in `assets` before you can run
`build-wasm-simd-kws.sh`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please also check in this file.

You need to use

cd sherpa-onnx
git add -f ./build-wasm-simd-kws.sh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👌


#ifdef SHERPA_ONNX_ENABLE_WASM_KWS
// Due to the limitations of the wasm file system,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you pass a keyword file from wasm?

We have been doing this for model files, such as tokens.txt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, but it needs to be recompiled when the keywords is modified, which is very inconvenient. Because token.txt and keywords.txt are both packaged into the sherpa-onnx-wasm-kws-main.data file, they cannot be modified and can only be recompiled to generate. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, I see. Then please keep using your current approach.

@@ -0,0 +1,55 @@
if(NOT $ENV{SHERPA_ONNX_IS_USING_BUILD_WASM_SH})
message(FATAL_ERROR "Please use ./build-wasm-kws.sh to build for wasm KWS")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
message(FATAL_ERROR "Please use ./build-wasm-kws.sh to build for wasm KWS")
message(FATAL_ERROR "Please use ./build-wasm-simd-kws.sh to build for wasm KWS")

Copy link
Collaborator

@csukuangfj csukuangfj left a comment

Choose a reason for hiding this comment

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

Thank you! Left some minor comments.

@@ -17,14 +17,13 @@ concurrency:

jobs:
build_wheels_aarch64:
name: ${{ matrix.manylinux }} ${{ matrix.python-version }}
name: ${{ matrix.python-version }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use changes from the latest master for *.yaml files that you have changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, I forgot to merge other files

@@ -473,10 +473,16 @@ SherpaOnnxKeywordSpotter* CreateKeywordSpotter(
SHERPA_ONNX_LOGE("%s\n", spotter_config.ToString().c_str());
}

#ifndef SHERPA_ONNX_ENABLE_WASM_KWS
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you disable only the check for hotword file?
We can still validate model files and tokens.txt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, thank you for your suggestions

@csukuangfj
Copy link
Collaborator

Thank your for your first-time contribution!

@csukuangfj csukuangfj merged commit 009ed2c into k2-fsa:master Mar 11, 2024
4 of 226 checks passed
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.

2 participants