Repository navigation
Update language bindings for FireRedASR CTC models - #3224
Merged
Merged
Conversation
dosubot
Bot
added
the
size:XXL
Summary of ChangesHello @csukuangfj, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the project's automatic speech recognition capabilities by integrating FireRedASR CTC models across a wide array of programming language bindings. The changes ensure that developers using various languages can now leverage these new models, complete with dedicated examples and updated build processes, enhancing the overall accessibility and functionality of the ASR toolkit. Highlights
Changelog
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds FireRedASR CTC support across the codebase: new config types in C/C++ and language bindings, example programs (offline/streaming), run scripts, CI/workflow steps, model download automation, and many test-script reorderings and cleanups. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the language bindings for all 12 programming languages to support the new FireRedASR CTC models. The changes are extensive, including new examples, updated build configurations, and modified test scripts. Overall, the implementation is solid and aligns with the project's structure. I've identified a few minor issues, mainly copy-paste errors in comments and a potentially unintentional test removal, which I've detailed in the review comments.
Comment on lines
+1
to
+5
| // cxx-api-examples/medasr-ctc-cxx-api.cc | ||
| // Copyright (c) 2025 Xiaomi Corporation | ||
|
|
||
| // | ||
| // This file demonstrates how to use MedASR with sherpa-onnx's C++ API. |
There was a problem hiding this comment.
The file header comment and the description appear to be from another file, mentioning MedASR instead of FireRedASR CTC. Please update these comments to accurately reflect the content of this file.
Suggested change
| // cxx-api-examples/medasr-ctc-cxx-api.cc | |
| // Copyright (c) 2025 Xiaomi Corporation | |
| // | |
| // This file demonstrates how to use MedASR with sherpa-onnx's C++ API. | |
| // cxx-api-examples/fire-red-asr-ctc-cxx-api.cc | |
| // Copyright (c) 2025 Xiaomi Corporation | |
| // | |
| // This file demonstrates how to use FireRedASR CTC with sherpa-onnx's C++ API. |
| const sherpa_onnx = require('sherpa-onnx-node'); | ||
|
|
||
| /** | ||
| * Create an OfflineRecognizer with FunASR Nano model asynchronously. |
There was a problem hiding this comment.
The comment incorrectly states that this function creates a recognizer for the FunASR Nano model. It should be updated to mention the FireRedASR CTC model to avoid confusion.
Suggested change
| * Create an OfflineRecognizer with FunASR Nano model asynchronously. | |
| * Create an OfflineRecognizer with FireRedASR CTC model asynchronously. |
There was a problem hiding this comment.
Pull request overview
Updates the multi-language bindings and examples to support FireRedASR CTC offline models across the project.
Changes:
- Adds
OfflineFireRedAsrCtcModelConfig/fireRedAsrCtcto C/C++ core configs and propagates it to language bindings (Rust, Java/Kotlin, Go, Dart/Flutter, Swift, Pascal, WASM). - Introduces new FireRedASR CTC example programs and CI coverage across multiple ecosystems.
- Adjusts debug logging behavior in JNI to avoid noisy logs and handle long strings on Android.
Reviewed changes
Copilot reviewed 69 out of 72 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| wasm/nodejs/sherpa-onnx-wasm-nodejs.cc | Extends WASM Node.js binding config printing and struct-size checks for FireRedASR CTC. |
| wasm/asr/sherpa-onnx-asr.js | Adds FireRedASR CTC config init/free + packing into offline model config for WASM JS. |
| swift-api-examples/run-fire-red-asr-ctc.sh | Adds Swift runner script for FireRedASR CTC example. |
| swift-api-examples/fire-red-asr-ctc.swift | Adds Swift FireRedASR CTC decoding example. |
| swift-api-examples/SherpaOnnx.swift | Adds Swift helper + extends offline model config to include FireRedASR CTC. |
| swift-api-examples/.gitignore | Ignores built Swift FireRedASR CTC example binary. |
| sherpa-onnx/rust/sherpa-onnx/src/offline_asr.rs | Adds Rust wrapper struct and plumbing for FireRedASR CTC in offline model config. |
| sherpa-onnx/rust/sherpa-onnx/Cargo.toml | Bumps Rust crate and sys dependency versions to 0.1.8. |
| sherpa-onnx/rust/sherpa-onnx-sys/src/offline_asr.rs | Adds raw FFI struct + embeds it into offline model config. |
| sherpa-onnx/rust/sherpa-onnx-sys/Cargo.toml | Bumps sys crate version to 0.1.8. |
| sherpa-onnx/pascal-api/sherpa_onnx.pas | Adds Pascal structs and conversion for FireRedASR CTC config. |
| sherpa-onnx/kotlin-api/OfflineRecognizer.kt | Adds Kotlin data classes + model selection entry for FireRedASR CTC. |
| sherpa-onnx/jni/online-recognizer.cc | Gates config logging behind debug and handles Android log truncation. |
| sherpa-onnx/jni/offline-tts.cc | Minor whitespace/formatting cleanup; gates config logging behind debug and handles Android log truncation. |
| sherpa-onnx/jni/offline-recognizer.cc | Reads FireRedASR CTC config from Java + gates config logging behind debug with Android chunking. |
| sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OfflineModelConfig.java | Adds FireRedASR CTC field/getter/setter wiring in Java OfflineModelConfig. |
| sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OfflineFireRedAsrCtcModelConfig.java | Introduces Java FireRedASR CTC model config class. |
| sherpa-onnx/java-api/Makefile | Includes the new Java FireRedASR CTC config class in build. |
| sherpa-onnx/c-api/cxx-api.h | Adds C++ API struct for FireRedASR CTC and embeds in OfflineModelConfig. |
| sherpa-onnx/c-api/cxx-api.cc | Converts C++ FireRedASR CTC config into C API config struct. |
| sherpa-onnx/c-api/c-api.h | Adds C API struct for FireRedASR CTC and embeds in OfflineModelConfig. |
| sherpa-onnx/c-api/c-api.cc | Wires FireRedASR CTC model path into recognizer config parsing. |
| scripts/go/sherpa_onnx.go | Adds Go binding struct + passes/free C string for FireRedASR CTC model path. |
| scripts/go/_internal/non-streaming-fire-red-asr-ctc-decode-files/run.sh | Adds internal Go script entry to run FireRedASR CTC decode example. |
| scripts/go/_internal/non-streaming-fire-red-asr-ctc-decode-files/main.go | Adds internal Go script entry to build FireRedASR CTC decode example. |
| scripts/go/_internal/non-streaming-fire-red-asr-ctc-decode-files/go.mod | Adds internal Go module for FireRedASR CTC decode example. |
| scripts/dotnet/OfflineModelConfig.cs | Extends .NET offline model config struct with FireRedASR CTC. |
| scripts/dotnet/OfflineFireRedAsrCtcModel.cs | Adds .NET FireRedASR CTC config struct for interop. |
| scripts/apk/generate-vad-asr-apk-script.py | Adds FireRedASR CTC model entry for APK script generation. |
| rust-api-examples/run-fire-red-asr-ctc.sh | Adds Rust runner script for FireRedASR CTC example. |
| rust-api-examples/examples/fire_red_asr_ctc.rs | Adds Rust FireRedASR CTC offline decoding example. |
| rust-api-examples/Cargo.toml | Bumps Rust examples crate and dependency to 0.1.8. |
| pascal-api-examples/non-streaming-asr/run-fire-red-asr-ctc.sh | Adds Pascal runner script for FireRedASR CTC example. |
| pascal-api-examples/non-streaming-asr/fire_red_asr_ctc.pas | Adds Pascal FireRedASR CTC offline decoding example. |
| pascal-api-examples/non-streaming-asr/.gitignore | Ignores Pascal FireRedASR CTC example outputs. |
| nodejs-examples/test-offline-fire-red-asr-ctc.js | Adds Node.js NPM example for FireRedASR CTC offline decode. |
| nodejs-examples/README.md | Documents how to run the new Node.js FireRedASR CTC example. |
| nodejs-addon-examples/test_asr_non_streaming_fire_red_asr_ctc_async.js | Adds Node addon async example for FireRedASR CTC offline decode. |
| nodejs-addon-examples/test_asr_non_streaming_fire_red_asr_ctc.js | Adds Node addon sync example for FireRedASR CTC offline decode. |
| kotlin-api-examples/test_offline_fire_red_asr_ctc.kt | Adds Kotlin example for FireRedASR CTC offline decode. |
| kotlin-api-examples/run.sh | Adds Kotlin example runner for FireRedASR CTC and runs it in the script. |
| java-api-examples/run-non-streaming-decode-file-fire-red-asr-ctc.sh | Adds Java runner script for FireRedASR CTC offline decode example. |
| java-api-examples/README.md | Updates Java examples README to include FireRedASR CTC and other reordered entries. |
| java-api-examples/NonStreamingDecodeFileFireRedAsrCtc.java | Adds Java FireRedASR CTC offline decode example. |
| harmony-os/SherpaOnnxHar/sherpa_onnx/src/main/cpp/non-streaming-asr.cc | Adds HarmonyOS N-API parsing/freeing for FireRedASR CTC config. |
| go-api-examples/non-streaming-fire-red-asr-ctc-decode-files/run.sh | Adds Go example runner for FireRedASR CTC offline decode. |
| go-api-examples/non-streaming-fire-red-asr-ctc-decode-files/main.go | Adds Go FireRedASR CTC offline decoding example. |
| go-api-examples/non-streaming-fire-red-asr-ctc-decode-files/go.mod | Adds Go module for FireRedASR CTC example. |
| flutter/sherpa_onnx/lib/src/sherpa_onnx_bindings.dart | Adds Dart FFI struct bindings for FireRedASR CTC config. |
| flutter/sherpa_onnx/lib/src/offline_recognizer.dart | Adds Dart model config class + FFI wiring + JSON support for FireRedASR CTC. |
| dotnet-examples/offline-decode-files/run-fire-red-asr-ctc.sh | Adds .NET example runner for FireRedASR CTC offline decode. |
| dotnet-examples/offline-decode-files/Program.cs | Adds CLI option and config mapping for FireRedASR CTC model path. |
| dart-api-examples/non-streaming-asr/run-fire-red-asr-ctc.sh | Adds Dart example runner for FireRedASR CTC offline decode. |
| dart-api-examples/non-streaming-asr/bin/fire-red-asr-ctc.dart | Adds Dart FireRedASR CTC offline decoding example. |
| cxx-api-examples/fire-red-asr-ctc-simulate-streaming-microphone-cxx-api.cc | Adds C++ example simulating streaming (mic + VAD) using offline FireRedASR CTC decoding. |
| cxx-api-examples/fire-red-asr-ctc-simulate-streaming-alsa-cxx-api.cc | Adds ALSA-based C++ example simulating streaming using offline FireRedASR CTC decoding. |
| cxx-api-examples/fire-red-asr-ctc-cxx-api.cc | Adds C++ offline decode example for FireRedASR CTC. |
| cxx-api-examples/CMakeLists.txt | Adds targets for FireRedASR CTC C++ examples (offline + simulate streaming). |
| c-api-examples/fire-red-asr-ctc-c-api.c | Adds C API FireRedASR CTC offline decode example. |
| c-api-examples/CMakeLists.txt | Adds build target for the new C API FireRedASR CTC example. |
| .gitignore | Ignores FireRedASR CTC model directory and Go example binary output. |
| .github/workflows/test-go.yaml | Adds CI job step to build/run Go FireRedASR CTC decode example. |
| .github/workflows/run-java-test.yaml | Adds Java CI step to run FireRedASR CTC offline decode example. |
| .github/workflows/pascal.yaml | Adds Pascal CI step to run FireRedASR CTC example. |
| .github/workflows/cxx-api.yaml | Adds C++ CI step to build/run FireRedASR CTC example. |
| .github/workflows/c-api.yaml | Adds C CI step to build/run FireRedASR CTC example. |
| .github/scripts/test-swift.sh | Adds Swift CI script step to run FireRedASR CTC example. |
| .github/scripts/test-nodejs-npm.sh | Adds Node.js NPM CI script step to run FireRedASR CTC example. |
| .github/scripts/test-nodejs-addon-npm.sh | Adds Node addon CI script steps to run FireRedASR CTC sync/async examples. |
| .github/scripts/test-dot-net.sh | Reorders .NET CI script flow and adds FireRedASR CTC offline decode run. |
| .github/scripts/test-dart.sh | Adds Dart CI step for FireRedASR CTC and reorders test sections. |
Comments suppressed due to low confidence (6)
sherpa-onnx/c-api/cxx-api.cc:1
- The assignments for
funasr_nano.top_pandfunasr_nano.seedare duplicated (set at lines 292–293 and again at 299–300). Remove one set to avoid confusion and reduce the risk of future edits only updating one copy.
java-api-examples/NonStreamingDecodeFileFireRedAsrCtc.java:22 - The local variable name
medasris misleading in a FireRedASR CTC example. Rename it to something likefireRedAsrCtcto match the actual type and usage.
OfflineFireRedAsrCtcModelConfig medasr =
OfflineFireRedAsrCtcModelConfig.builder().setModel(model).build();
java-api-examples/NonStreamingDecodeFileFireRedAsrCtc.java:26
- The local variable name
medasris misleading in a FireRedASR CTC example. Rename it to something likefireRedAsrCtcto match the actual type and usage.
.setFireRedAsrCtc(medasr)
nodejs-addon-examples/test_asr_non_streaming_fire_red_asr_ctc_async.js:1
- The doc comment mentions 'FunASR Nano', but this file/config is for FireRedASR CTC (
fireRedAsrCtc). Update the comment to reflect FireRedASR CTC to avoid confusing users.
cxx-api-examples/fire-red-asr-ctc-cxx-api.cc:1 - The header comments reference
medasr-ctc-cxx-api.ccand 'MedASR', but this file is a FireRedASR CTC example. Update the filename/comment text to avoid misleading documentation.
// cxx-api-examples/medasr-ctc-cxx-api.cc
cxx-api-examples/fire-red-asr-ctc-cxx-api.cc:5
- The header comments reference
medasr-ctc-cxx-api.ccand 'MedASR', but this file is a FireRedASR CTC example. Update the filename/comment text to avoid misleading documentation.
// This file demonstrates how to use MedASR with sherpa-onnx's C++ API.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Test FireRedASR CTC | ||
| shell: bash | ||
| run: | | ||
| name=fire-red-asr-ctc-ctc-cxx-api |
There was a problem hiding this comment.
The workflow sets name=fire-red-asr-ctc-ctc-cxx-api and then compiles ./cxx-api-examples/$name.cc, but the added example file is cxx-api-examples/fire-red-asr-ctc-cxx-api.cc (single -ctc-). This will fail CI because the source file path won’t exist. Rename name to fire-red-asr-ctc-cxx-api (or compile the correct filename explicitly).
Suggested change
| name=fire-red-asr-ctc-ctc-cxx-api | |
| name=fire-red-asr-ctc-cxx-api |
Comment on lines
+45
to
+46
| file, _ := os.Open(filename) | ||
| defer file.Close() |
There was a problem hiding this comment.
This example ignores the error from os.Open(). If the open fails, file will be nil and defer file.Close() can panic. Handle the os.Open error (and consider checking err from reader.Read) to make the example robust and avoid crashing with a nil dereference.
| reader.Duration() // so that it initializes reader.Size | ||
|
|
||
| buf := make([]byte, reader.Size) | ||
| n, err := reader.Read(buf) |
There was a problem hiding this comment.
This example ignores the error from os.Open(). If the open fails, file will be nil and defer file.Close() can panic. Handle the os.Open error (and consider checking err from reader.Read) to make the example robust and avoid crashing with a nil dereference.
|
|
||
| cd ../offline-decode-files | ||
| ./run-fire-red-asr-ctc.sh | ||
| rm -rf sherpa-onnx-fire-red-asr-ctc-* |
There was a problem hiding this comment.
The cleanup glob sherpa-onnx-fire-red-asr-ctc-* does not match the downloaded model directory name used elsewhere in this PR (e.g., sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25). This likely leaves large model artifacts behind in CI. Adjust the glob to match the actual extracted directory name/prefix.
Suggested change
| rm -rf sherpa-onnx-fire-red-asr-ctc-* | |
| rm -rf sherpa-onnx-fire-red-asr2-ctc-* |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (13)
sherpa-onnx/jni/online-recognizer.cc (1)
251-259: Consider extracting duplicated debug-log block into a helper.The same debug logging logic appears in both constructors. A small local helper would reduce duplication and keep future log-format changes in one place.
Also applies to: 282-290
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sherpa-onnx/jni/online-recognizer.cc` around lines 251 - 259, Extract the duplicated debug-logging block in online-recognizer.cc into a single helper (e.g., a static/local function named LogConfigDebug or a lambda) and call it from both constructors instead of repeating the code; the helper should take the config (or config.ToString()) and implement the existing conditional compilation behavior (`#if` __ANDROID_API__ split into 128-byte lines and loop vs. single SHERPA_ONNX_LOGE call) so both constructor sites (the blocks around the current config.model_config.debug checks at the two locations) simply call LogConfigDebug(config).sherpa-onnx/jni/offline-tts.cc (1)
375-384: Good improvement: debug logging is now conditional.Two minor observations:
Line 376 uses
#if __ANDROID_API__while lines 359 and 387 use#if __ANDROID_API__ >= 9. Consider using#if defined(__ANDROID_API__)for clarity since the intent here is checking "is Android" rather than a specific API level.
newFromFile(line 409) still logs unconditionally, whilenewFromAssetnow respects the debug flag. If this is intentional (e.g., desktop debugging scenarios), it's fine, but if consistency is preferred, consider guarding both.♻️ Optional: Align preprocessor check style
if (config.model.debug) { -#if __ANDROID_API__ +#if defined(__ANDROID_API__) auto str_vec = sherpa_onnx::SplitString(config.ToString(), 128); for (const auto &s : str_vec) { SHERPA_ONNX_LOGE("%s", s.c_str()); } `#else` SHERPA_ONNX_LOGE("%s", config.ToString().c_str()); `#endif` }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sherpa-onnx/jni/offline-tts.cc` around lines 375 - 384, Use a clearer Android check and make logging consistent: replace the bare `#if __ANDROID_API__` in the debug block that prints `config.ToString()` (which also uses `SplitString` and `config.model.debug`) with a defined check such as `#if defined(__ANDROID_API__)` to match the other checks, and if you want consistent behavior between asset- and file-based model creation, wrap the unconditional logging in `newFromFile` with the same `if (config.model.debug)` guard (the other function is `newFromAsset`) so both only log model config when debugging is enabled.scripts/go/_internal/non-streaming-fire-red-asr-ctc-decode-files/go.mod (1)
1-1: Module naming convention.The module name lacks a domain prefix (e.g.,
github.com/k2-fsa/sherpa-onnx/...). While this is acceptable for internal scripts and examples not intended for external import, consider using a fully qualified module path if this code might be referenced elsewhere in the future.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/go/_internal/non-streaming-fire-red-asr-ctc-decode-files/go.mod` at line 1, The module declaration in go.mod uses a bare name "non-streaming-fire-red-asr-ctc-decode-files"; if this package may be imported externally, change the module line to a fully qualified module path (e.g., github.com/your-org/your-repo/non-streaming-fire-red-asr-ctc-decode-files) so tools and consumers can resolve it; update any import paths in the code that reference this module (search for imports currently matching this module name) to the new fully qualified path and run go mod tidy to refresh dependencies.swift-api-examples/fire-red-asr-ctc.swift (2)
2-5: Make model/token/wav paths configurable instead of hardcoded.Hardcoded local paths make this example fragile across environments. Accepting the three paths from
CommandLine.argumentswill make it reusable without source edits.♻️ Proposed refactor
func run() { - let model = - "./sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25/model.int8.onnx" - let tokens = - "./sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25/tokens.txt" + let args = CommandLine.arguments + guard args.count == 4 else { + print("Usage: fire-red-asr-ctc <model.onnx> <tokens.txt> <wave.wav>") + return + } + + let model = args[1] + let tokens = args[2] @@ - let filePath = "./sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25/test_wavs/1.wav" + let filePath = args[3]Also applies to: 25-25
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@swift-api-examples/fire-red-asr-ctc.swift` around lines 2 - 5, Replace the hardcoded model/tokens/wav paths by reading CommandLine.arguments: check for at least three args and assign them to the existing variables model, tokens and wav (or use the current hardcoded defaults when args are not provided); validate argument count and print a short usage message when insufficient arguments are given to guide users. Locate the top-level declarations of model and tokens in the file and update them to take values from CommandLine.arguments (e.g., arguments[1..3]) and ensure any downstream code still references the same variable names model, tokens and wav.
26-29: Add a guard for invalid WAV input before decode.Fail fast when
audio.samplesis empty oraudio.sampleRateis invalid to avoid confusing decode results.🛡️ Proposed guard
let filePath = "./sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25/test_wavs/1.wav" let audio = SherpaOnnxWaveWrapper.readWave(filename: filePath) + guard !audio.samples.isEmpty, audio.sampleRate > 0 else { + print("Failed to read valid audio from \(filePath)") + return + } let result = recognizer.decode(samples: audio.samples, sampleRate: audio.sampleRate)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@swift-api-examples/fire-red-asr-ctc.swift` around lines 26 - 29, Add a guard after calling SherpaOnnxWaveWrapper.readWave to validate the WAV data: check that audio.samples is not empty and audio.sampleRate is a valid positive value before calling recognizer.decode; if validation fails, log a clear error (including which value was invalid) and exit or return early to avoid passing bad input into recognizer.decode.dart-api-examples/non-streaming-asr/bin/fire-red-asr-ctc.dart (1)
40-55: Make recognizer/stream cleanup exception-safeIf an exception is thrown between Line 40 and Line 52,
free()is skipped. Wrap usage intry/finallyso native resources are always released.Suggested fix
final config = sherpa_onnx.OfflineRecognizerConfig(model: modelConfig); final recognizer = sherpa_onnx.OfflineRecognizer(config); - - final waveData = sherpa_onnx.readWave(inputWav); - final stream = recognizer.createStream(); - - stream.acceptWaveform( - samples: waveData.samples, - sampleRate: waveData.sampleRate, - ); - recognizer.decode(stream); - - final result = recognizer.getResult(stream); - print(result.text); - - stream.free(); - recognizer.free(); + sherpa_onnx.OfflineStream? stream; + try { + final waveData = sherpa_onnx.readWave(inputWav); + stream = recognizer.createStream(); + stream.acceptWaveform( + samples: waveData.samples, + sampleRate: waveData.sampleRate, + ); + recognizer.decode(stream); + final result = recognizer.getResult(stream); + print(result.text); + } finally { + stream?.free(); + recognizer.free(); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dart-api-examples/non-streaming-asr/bin/fire-red-asr-ctc.dart` around lines 40 - 55, The code creates native resources (recognizer and stream) but currently calls stream.free() and recognizer.free() only at the end, so exceptions during acceptWaveform/decode/getResult will leak; fix by allocating the stream via recognizer.createStream() and then wrapping the processing (acceptWaveform, decode, getResult, print) in a try/finally where the finally calls stream.free() and recognizer.free() (guarding null or already-freed cases if necessary) so both recognizer and stream are always released even on error.dart-api-examples/non-streaming-asr/run-fire-red-asr-ctc.sh (1)
3-10: Improve curl download robustness with explicit error handling and retriesThe current download can fail silently if the HTTP request returns an error (e.g., 404, 500); curl without
--failexits with code 0, causingtarto fail on corrupted HTML content. Add--failto catch HTTP errors immediately, and include--retryflags to improve resilience on transient network issues in CI.Consider:
- curl -SL -O https://github.com/k2-fsa/sherpa-onnx/releases/download/asr-models/sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25.tar.bz2 + curl --fail --location --retry 3 --retry-all-errors --retry-delay 2 \ + -O https://github.com/k2-fsa/sherpa-onnx/releases/download/asr-models/sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25.tar.bz2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dart-api-examples/non-streaming-asr/run-fire-red-asr-ctc.sh` around lines 3 - 10, The curl download in run-fire-red-asr-ctc.sh that fetches sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25.tar.bz2 can silently succeed with an HTTP error and produce a broken archive (breaks tar and downstream steps); update the curl invocation that runs when tokens.txt is missing to include --fail and retry options (e.g., --retry and/or --retry-delay and --retry-connrefused) so HTTP errors cause immediate non-zero exit and transient network failures are retried, ensuring the subsequent tar extraction only runs on a valid download.dotnet-examples/offline-decode-files/run-fire-red-asr-ctc.sh (1)
3-8: Harden the model download for CI stability.The current fetch command can be brittle for transient network failures and may not fail early on HTTP errors.
♻️ Proposed reliability tweak
-set -ex +set -euxo pipefail @@ - curl -SL -O https://github.com/k2-fsa/sherpa-onnx/releases/download/asr-models/sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25.tar.bz2 + curl --fail --location --retry 3 --retry-delay 2 \ + --connect-timeout 15 --max-time 300 -O \ + https://github.com/k2-fsa/sherpa-onnx/releases/download/asr-models/sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25.tar.bz2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dotnet-examples/offline-decode-files/run-fire-red-asr-ctc.sh` around lines 3 - 8, Replace the brittle curl call that downloads sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25.tar.bz2 with a hardened download: use curl with --fail --show-error --location and retry flags (for example --retry 5 --retry-delay 2 --retry-connrefused) and write to a deterministic output (-o), check the downloaded file is non-empty before extracting, and verify tar extraction succeeds (exit on error) before removing the tarball; update the block that references the model filename model.int8.onnx and the tarball name sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25.tar.bz2 to use these safer curl and post-download checks.nodejs-examples/test-offline-fire-red-asr-ctc.js (1)
3-4: Unused imports.
fsandReadableare imported but never used in this file.🧹 Proposed fix
// Copyright (c) 2026 Xiaomi Corporation (authors: Fangjun Kuang) // -const fs = require('fs'); -const {Readable} = require('stream'); - const sherpa_onnx = require('sherpa-onnx');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nodejs-examples/test-offline-fire-red-asr-ctc.js` around lines 3 - 4, The top-level imports "fs" and "Readable" are unused; remove the unused require statements (references to fs and Readable) from the module head (or alternatively use them where intended) so there are no unused variables; search for require('fs') and const {Readable} to locate and delete or replace those lines and run lint/tests to confirm no further references remain.go-api-examples/non-streaming-fire-red-asr-ctc-decode-files/main.go (1)
68-72: Consider checking the error fromreader.Read.The error from
reader.Readis assigned but not checked before validating the byte count. While the byte count check would catch most read failures, an explicit error check provides clearer diagnostics.♻️ Suggested improvement
buf := make([]byte, reader.Size) n, err := reader.Read(buf) + if err != nil { + log.Fatalf("Failed to read audio data: %v", err) + } if n != int(reader.Size) { log.Fatalf("Failed to read %v bytes. Returned %v bytes\n", reader.Size, n) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go-api-examples/non-streaming-fire-red-asr-ctc-decode-files/main.go` around lines 68 - 72, The code reads into buf using reader.Read but ignores the returned err; update the block around reader.Read (buf, reader.Read, reader.Size, n, err) to first check err (e.g., if err != nil && err != io.EOF) and log/handle that error (include reader.Size and err in the log) before asserting n == int(reader.Size); this ensures clear diagnostics on read failures while preserving the existing byte-count validation.kotlin-api-examples/test_offline_fire_red_asr_ctc.kt (1)
11-15: Prefervalovervarfor non-reassigned variables.In Kotlin,
valis preferred for variables that are not reassigned. Bothstreamandresultare assigned once and never modified.♻️ Suggested fix
- var stream = recognizer.createStream() + val stream = recognizer.createStream() stream.acceptWaveform(waveData.samples, sampleRate=waveData.sampleRate) recognizer.decode(stream) - var result = recognizer.getResult(stream) + val result = recognizer.getResult(stream)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kotlin-api-examples/test_offline_fire_red_asr_ctc.kt` around lines 11 - 15, Replace mutable declarations with immutable ones: change the local variables declared with "var stream" and "var result" to use "val" since neither is reassigned. Update the lines around recognizer.createStream(), stream.acceptWaveform(...), recognizer.decode(stream) and recognizer.getResult(stream) so "stream" and "result" are declared with "val" to reflect immutability.java-api-examples/NonStreamingDecodeFileFireRedAsrCtc.java (1)
21-27: Renamemedasrto a FireRed-specific name for clarity.The current variable name suggests a different model family and can mislead future edits.
✏️ Suggested rename
- OfflineFireRedAsrCtcModelConfig medasr = + OfflineFireRedAsrCtcModelConfig fireRedAsrCtc = OfflineFireRedAsrCtcModelConfig.builder().setModel(model).build(); ... - .setFireRedAsrCtc(medasr) + .setFireRedAsrCtc(fireRedAsrCtc)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@java-api-examples/NonStreamingDecodeFileFireRedAsrCtc.java` around lines 21 - 27, Rename the variable medasr to a FireRed-specific name (e.g., fireRedAsrConfig or fireRedModelConfig) to avoid confusion; update its declaration of OfflineFireRedAsrCtcModelConfig medasr = OfflineFireRedAsrCtcModelConfig.builder().setModel(model).build() and all subsequent uses (notably the OfflineModelConfig.builder().setFireRedAsrCtc(medasr).setTokens(tokens) call) to the new identifier so references remain consistent.sherpa-onnx/c-api/cxx-api.cc (1)
292-300: Pre-existing duplicate assignments fortop_pandseed.Lines 292-293 assign
top_pandseed, but lines 299-300 assign them again. This appears to be a pre-existing issue, not introduced by this PR, but the redundant assignments are wasteful.♻️ Proposed fix to remove duplicate assignments
c.model_config.funasr_nano.top_p = config.model_config.funasr_nano.top_p; c.model_config.funasr_nano.seed = config.model_config.funasr_nano.seed; c.model_config.funasr_nano.language = config.model_config.funasr_nano.language.c_str(); c.model_config.funasr_nano.itn = config.model_config.funasr_nano.itn ? 1 : 0; c.model_config.funasr_nano.hotwords = config.model_config.funasr_nano.hotwords.c_str(); - c.model_config.funasr_nano.top_p = config.model_config.funasr_nano.top_p; - c.model_config.funasr_nano.seed = config.model_config.funasr_nano.seed; c.model_config.medasr.model = config.model_config.medasr.model.c_str();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sherpa-onnx/c-api/cxx-api.cc` around lines 292 - 300, The block copies funasr_nano fields twice; remove the redundant second assignments to c.model_config.funasr_nano.top_p and c.model_config.funasr_nano.seed so each field is set only once from config.model_config.funasr_nano.top_p and config.model_config.funasr_nano.seed; locate the duplicate lines setting c.model_config.funasr_nano.top_p and c.model_config.funasr_nano.seed and delete the later occurrences to eliminate the wasteful redundancy.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/cxx-api.yaml:
- Around line 84-86: The CI step sets name=fire-red-asr-ctc-ctc-cxx-api but the
g++ invocation uses that name to build
./cxx-api-examples/fire-red-asr-ctc-ctc-cxx-api.cc which doesn't exist; change
the name variable and the g++ target to the correct base name
(fire-red-asr-ctc-cxx-api) so the output binary and source path match (adjust
the name assignment and the g++ source/output references that use name).
In `@cxx-api-examples/fire-red-asr-ctc-cxx-api.cc`:
- Around line 1-6: The file header comments still reference "MedASR" and the old
filename "medasr-ctc-cxx-api.cc"; update those header lines in
fire-red-asr-ctc-cxx-api.cc to correctly reference "FireRedASR CTC" and the
matching filename (e.g., replace "medasr-ctc-cxx-api.cc" and "MedASR"
occurrences with "fire-red-asr-ctc-cxx-api.cc" and "FireRedASR CTC") so the
top-of-file comment accurately reflects this example and model assets.
In `@cxx-api-examples/fire-red-asr-ctc-simulate-streaming-alsa-cxx-api.cc`:
- Around line 37-43: The signal handler and unsynchronized access to stop cause
a data race and use of non-signal-safe APIs; change the global bool stop to
std::atomic<bool> stop; remove the call to condition_variable.notify_one() from
Handler and only set stop.store(true) inside the handler, and update all
reads/writes (in RecordCallback and the main loop) to use stop.load()/store() so
they are atomic; additionally, move any
condition_variable.notify_one()/notify_all() calls out of the signal handler
into normal thread context (e.g., have the main thread or the recording thread
detect stop.load() and then call condition_variable.notify_one()) so no
non-signal-safe functions run in Handler.
In `@cxx-api-examples/fire-red-asr-ctc-simulate-streaming-microphone-cxx-api.cc`:
- Around line 129-133: The stderr log prints mic_sample_rate before it is
updated from SHERPA_ONNX_MIC_SAMPLE_RATE, so it reports the old value; update
the code in this block to parse sample_rate_str into mic_sample_rate first (use
atof or equivalent) and then call fprintf to log the resulting mic_sample_rate
(or alternatively log the raw sample_rate_str), ensuring the variables
sample_rate_str and mic_sample_rate are referenced in that order in this
snippet.
- Around line 112-118: Pa_GetDeviceCount() can return negative PortAudio error
codes; update the logic around the call in the main flow that assigns
PaDeviceIndex num_devices = Pa_GetDeviceCount() to first check for num_devices <
0 and handle that as a failure (log an error including PortAudio's message using
Pa_GetErrorText(num_devices) or similar) before the existing num_devices == 0
check, then return an error code to abort initialization.
- Around line 39-43: The SIGINT handler "Handler" performs non-async-signal-safe
actions (calling condition_variable.notify_one() and fprintf()) which is
undefined behavior; change the handler to only set a volatile sig_atomic_t flag
(e.g., stop) and return immediately, and move the
condition_variable.notify_one() and the stderr message (or any logging) into the
normal program flow where you detect the flag (e.g., main loop or the thread
waiting on the condition variable) so you can safely call
condition_variable.notify_one() and print an exit message.
In `@dart-api-examples/non-streaming-asr/bin/fire-red-asr-ctc.dart`:
- Around line 9-23: The call to initSherpaOnnx() is happening before CLI
parsing/validation so parser.parse(arguments) can throw a FormatException before
usage is printed; move initSherpaOnnx() to after you parse and validate
arguments in main(List<String> arguments) so invalid flags are handled first.
Specifically, call parser.parse(arguments) and check res['model'],
res['tokens'], and res['input-wav'] for null and print parser.usage/exit(1)
before invoking initSherpaOnnx(); update references in main to ensure any
runtime setup (initSherpaOnnx) occurs only after successful argument validation.
In `@dart-api-examples/non-streaming-asr/run-fire-red-asr-ctc.sh`:
- Around line 7-11: The current download guard only checks for tokens.txt so
missing files like model.int8.onnx or test_wavs/1.wav can still cause failures;
update the shell guard around the download/extract block to verify the presence
of all required artifacts (tokens.txt, model.int8.onnx, and test_wavs/1.wav)
before skipping the download—e.g., test each with -f (or loop the required
filenames) and only skip download if all exist, otherwise perform the
curl/tar/remove steps as before.
In `@go-api-examples/non-streaming-fire-red-asr-ctc-decode-files/main.go`:
- Around line 44-46: The function readWave currently ignores the error from
os.Open which can leave file nil and cause panics on file.Close or
wav.NewReader; update readWave to check the error returned by os.Open (and
return an error from readWave or handle it appropriately), only defer file.Close
after confirming file is non-nil, and propagate any errors from
wav.NewReader/reading operations back to the caller (e.g., change signature to
return (samples []float32, sampleRate int, err error) and use error checks for
os.Open and wav.NewReader inside readWave).
In `@nodejs-addon-examples/test_asr_non_streaming_fire_red_asr_ctc_async.js`:
- Around line 9-21: The createRecognizerAsync function is ignoring the modelDir
argument and uses hardcoded paths; modify createRecognizerAsync to build the
model and tokens paths from the provided modelDir (instead of the current
'./sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25/...') so main(modelDir)
actually selects the intended files—update references
modelConfig.fireRedAsrCtc.model and modelConfig.fireRedAsrCtc.tokens (or tokens)
to use path joining (e.g., path.join(modelDir, 'model.int8.onnx') and
path.join(modelDir, 'tokens.txt')) and ensure any other places in
createRecognizerAsync that reference those hardcoded filenames use the
constructed paths.
- Around line 7-8: Update the stale docstring that mentions “FunASR Nano model”
to accurately describe the configured model `fireRedAsrCtc` in the comment above
the OfflineRecognizer setup; locate the comment in
test_asr_non_streaming_fire_red_asr_ctc_async.js (the block that starts with
"Create an OfflineRecognizer...") and change the text to reference the
fireRedAsrCtc model so the docstring matches the actual configuration used by
the function.
In `@nodejs-addon-examples/test_asr_non_streaming_fire_red_asr_ctc.js`:
- Around line 39-41: The real-time factor calculation can divide by zero when
wave.samples.length is 0; after computing duration (duration =
wave.samples.length / wave.sampleRate) add a guard: if duration === 0 set
real_time_factor to null (or a sentinel like 0/NaN per project convention)
instead of performing elapsed_seconds / duration, otherwise compute
real_time_factor = elapsed_seconds / duration; update the code around
elapsed_seconds, duration and real_time_factor to use this conditional branch to
avoid Infinity/NaN.
In `@rust-api-examples/examples/fire_red_asr_ctc.rs`:
- Around line 30-48: Remove the unused CLI args by deleting the `language:
String` and `use_itn: bool` fields (and their #[arg(...)] attributes) from the
CLI struct so they are no longer parsed, and update any help/comments
accordingly; if you prefer to keep them for future use instead, wire them into
recognizer configuration by adding corresponding fields to
`OfflineFireRedAsrCtcModelConfig` and passing them into the recognizer setup
where the model is configured (referencing `OfflineFireRedAsrCtcModelConfig` and
the recognizer initialization code that currently ignores these args).
---
Nitpick comments:
In `@dart-api-examples/non-streaming-asr/bin/fire-red-asr-ctc.dart`:
- Around line 40-55: The code creates native resources (recognizer and stream)
but currently calls stream.free() and recognizer.free() only at the end, so
exceptions during acceptWaveform/decode/getResult will leak; fix by allocating
the stream via recognizer.createStream() and then wrapping the processing
(acceptWaveform, decode, getResult, print) in a try/finally where the finally
calls stream.free() and recognizer.free() (guarding null or already-freed cases
if necessary) so both recognizer and stream are always released even on error.
In `@dart-api-examples/non-streaming-asr/run-fire-red-asr-ctc.sh`:
- Around line 3-10: The curl download in run-fire-red-asr-ctc.sh that fetches
sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25.tar.bz2 can silently succeed
with an HTTP error and produce a broken archive (breaks tar and downstream
steps); update the curl invocation that runs when tokens.txt is missing to
include --fail and retry options (e.g., --retry and/or --retry-delay and
--retry-connrefused) so HTTP errors cause immediate non-zero exit and transient
network failures are retried, ensuring the subsequent tar extraction only runs
on a valid download.
In `@dotnet-examples/offline-decode-files/run-fire-red-asr-ctc.sh`:
- Around line 3-8: Replace the brittle curl call that downloads
sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25.tar.bz2 with a hardened
download: use curl with --fail --show-error --location and retry flags (for
example --retry 5 --retry-delay 2 --retry-connrefused) and write to a
deterministic output (-o), check the downloaded file is non-empty before
extracting, and verify tar extraction succeeds (exit on error) before removing
the tarball; update the block that references the model filename model.int8.onnx
and the tarball name sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25.tar.bz2
to use these safer curl and post-download checks.
In `@go-api-examples/non-streaming-fire-red-asr-ctc-decode-files/main.go`:
- Around line 68-72: The code reads into buf using reader.Read but ignores the
returned err; update the block around reader.Read (buf, reader.Read,
reader.Size, n, err) to first check err (e.g., if err != nil && err != io.EOF)
and log/handle that error (include reader.Size and err in the log) before
asserting n == int(reader.Size); this ensures clear diagnostics on read failures
while preserving the existing byte-count validation.
In `@java-api-examples/NonStreamingDecodeFileFireRedAsrCtc.java`:
- Around line 21-27: Rename the variable medasr to a FireRed-specific name
(e.g., fireRedAsrConfig or fireRedModelConfig) to avoid confusion; update its
declaration of OfflineFireRedAsrCtcModelConfig medasr =
OfflineFireRedAsrCtcModelConfig.builder().setModel(model).build() and all
subsequent uses (notably the
OfflineModelConfig.builder().setFireRedAsrCtc(medasr).setTokens(tokens) call) to
the new identifier so references remain consistent.
In `@kotlin-api-examples/test_offline_fire_red_asr_ctc.kt`:
- Around line 11-15: Replace mutable declarations with immutable ones: change
the local variables declared with "var stream" and "var result" to use "val"
since neither is reassigned. Update the lines around recognizer.createStream(),
stream.acceptWaveform(...), recognizer.decode(stream) and
recognizer.getResult(stream) so "stream" and "result" are declared with "val" to
reflect immutability.
In `@nodejs-examples/test-offline-fire-red-asr-ctc.js`:
- Around line 3-4: The top-level imports "fs" and "Readable" are unused; remove
the unused require statements (references to fs and Readable) from the module
head (or alternatively use them where intended) so there are no unused
variables; search for require('fs') and const {Readable} to locate and delete or
replace those lines and run lint/tests to confirm no further references remain.
In `@scripts/go/_internal/non-streaming-fire-red-asr-ctc-decode-files/go.mod`:
- Line 1: The module declaration in go.mod uses a bare name
"non-streaming-fire-red-asr-ctc-decode-files"; if this package may be imported
externally, change the module line to a fully qualified module path (e.g.,
github.com/your-org/your-repo/non-streaming-fire-red-asr-ctc-decode-files) so
tools and consumers can resolve it; update any import paths in the code that
reference this module (search for imports currently matching this module name)
to the new fully qualified path and run go mod tidy to refresh dependencies.
In `@sherpa-onnx/c-api/cxx-api.cc`:
- Around line 292-300: The block copies funasr_nano fields twice; remove the
redundant second assignments to c.model_config.funasr_nano.top_p and
c.model_config.funasr_nano.seed so each field is set only once from
config.model_config.funasr_nano.top_p and config.model_config.funasr_nano.seed;
locate the duplicate lines setting c.model_config.funasr_nano.top_p and
c.model_config.funasr_nano.seed and delete the later occurrences to eliminate
the wasteful redundancy.
In `@sherpa-onnx/jni/offline-tts.cc`:
- Around line 375-384: Use a clearer Android check and make logging consistent:
replace the bare `#if __ANDROID_API__` in the debug block that prints
`config.ToString()` (which also uses `SplitString` and `config.model.debug`)
with a defined check such as `#if defined(__ANDROID_API__)` to match the other
checks, and if you want consistent behavior between asset- and file-based model
creation, wrap the unconditional logging in `newFromFile` with the same `if
(config.model.debug)` guard (the other function is `newFromAsset`) so both only
log model config when debugging is enabled.
In `@sherpa-onnx/jni/online-recognizer.cc`:
- Around line 251-259: Extract the duplicated debug-logging block in
online-recognizer.cc into a single helper (e.g., a static/local function named
LogConfigDebug or a lambda) and call it from both constructors instead of
repeating the code; the helper should take the config (or config.ToString()) and
implement the existing conditional compilation behavior (`#if` __ANDROID_API__
split into 128-byte lines and loop vs. single SHERPA_ONNX_LOGE call) so both
constructor sites (the blocks around the current config.model_config.debug
checks at the two locations) simply call LogConfigDebug(config).
In `@swift-api-examples/fire-red-asr-ctc.swift`:
- Around line 2-5: Replace the hardcoded model/tokens/wav paths by reading
CommandLine.arguments: check for at least three args and assign them to the
existing variables model, tokens and wav (or use the current hardcoded defaults
when args are not provided); validate argument count and print a short usage
message when insufficient arguments are given to guide users. Locate the
top-level declarations of model and tokens in the file and update them to take
values from CommandLine.arguments (e.g., arguments[1..3]) and ensure any
downstream code still references the same variable names model, tokens and wav.
- Around line 26-29: Add a guard after calling SherpaOnnxWaveWrapper.readWave to
validate the WAV data: check that audio.samples is not empty and
audio.sampleRate is a valid positive value before calling recognizer.decode; if
validation fails, log a clear error (including which value was invalid) and exit
or return early to avoid passing bad input into recognizer.decode.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
rust-api-examples/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (71)
.github/scripts/test-dart.sh.github/scripts/test-dot-net.sh.github/scripts/test-nodejs-addon-npm.sh.github/scripts/test-nodejs-npm.sh.github/scripts/test-swift.sh.github/workflows/c-api.yaml.github/workflows/cxx-api.yaml.github/workflows/pascal.yaml.github/workflows/run-java-test.yaml.github/workflows/test-go.yaml.gitignorec-api-examples/CMakeLists.txtc-api-examples/fire-red-asr-ctc-c-api.ccxx-api-examples/CMakeLists.txtcxx-api-examples/fire-red-asr-ctc-cxx-api.cccxx-api-examples/fire-red-asr-ctc-simulate-streaming-alsa-cxx-api.cccxx-api-examples/fire-red-asr-ctc-simulate-streaming-microphone-cxx-api.ccdart-api-examples/non-streaming-asr/bin/fire-red-asr-ctc.dartdart-api-examples/non-streaming-asr/run-fire-red-asr-ctc.shdotnet-examples/offline-decode-files/Program.csdotnet-examples/offline-decode-files/run-fire-red-asr-ctc.shflutter/sherpa_onnx/lib/src/offline_recognizer.dartflutter/sherpa_onnx/lib/src/sherpa_onnx_bindings.dartgo-api-examples/non-streaming-fire-red-asr-ctc-decode-files/go.modgo-api-examples/non-streaming-fire-red-asr-ctc-decode-files/main.gogo-api-examples/non-streaming-fire-red-asr-ctc-decode-files/run.shharmony-os/SherpaOnnxHar/sherpa_onnx/src/main/cpp/non-streaming-asr.ccjava-api-examples/NonStreamingDecodeFileFireRedAsrCtc.javajava-api-examples/README.mdjava-api-examples/run-non-streaming-decode-file-fire-red-asr-ctc.shkotlin-api-examples/run.shkotlin-api-examples/test_offline_fire_red_asr_ctc.ktnodejs-addon-examples/test_asr_non_streaming_fire_red_asr_ctc.jsnodejs-addon-examples/test_asr_non_streaming_fire_red_asr_ctc_async.jsnodejs-examples/README.mdnodejs-examples/test-offline-fire-red-asr-ctc.jspascal-api-examples/non-streaming-asr/.gitignorepascal-api-examples/non-streaming-asr/fire_red_asr_ctc.paspascal-api-examples/non-streaming-asr/run-fire-red-asr-ctc.shrust-api-examples/Cargo.tomlrust-api-examples/examples/fire_red_asr_ctc.rsrust-api-examples/run-fire-red-asr-ctc.shscripts/apk/generate-vad-asr-apk-script.pyscripts/dotnet/OfflineFireRedAsrCtcModel.csscripts/dotnet/OfflineModelConfig.csscripts/go/_internal/non-streaming-fire-red-asr-ctc-decode-files/go.modscripts/go/_internal/non-streaming-fire-red-asr-ctc-decode-files/main.goscripts/go/_internal/non-streaming-fire-red-asr-ctc-decode-files/run.shscripts/go/sherpa_onnx.gosherpa-onnx/c-api/c-api.ccsherpa-onnx/c-api/c-api.hsherpa-onnx/c-api/cxx-api.ccsherpa-onnx/c-api/cxx-api.hsherpa-onnx/java-api/Makefilesherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OfflineFireRedAsrCtcModelConfig.javasherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/OfflineModelConfig.javasherpa-onnx/jni/offline-recognizer.ccsherpa-onnx/jni/offline-tts.ccsherpa-onnx/jni/online-recognizer.ccsherpa-onnx/kotlin-api/OfflineRecognizer.ktsherpa-onnx/pascal-api/sherpa_onnx.passherpa-onnx/rust/sherpa-onnx-sys/Cargo.tomlsherpa-onnx/rust/sherpa-onnx-sys/src/offline_asr.rssherpa-onnx/rust/sherpa-onnx/Cargo.tomlsherpa-onnx/rust/sherpa-onnx/src/offline_asr.rsswift-api-examples/.gitignoreswift-api-examples/SherpaOnnx.swiftswift-api-examples/fire-red-asr-ctc.swiftswift-api-examples/run-fire-red-asr-ctc.shwasm/asr/sherpa-onnx-asr.jswasm/nodejs/sherpa-onnx-wasm-nodejs.cc
Comment on lines
+37
to
+43
| bool stop = false; | ||
|
|
||
| static void Handler(int32_t /*sig*/) { | ||
| stop = true; | ||
| condition_variable.notify_one(); | ||
| fprintf(stderr, "\nCaught Ctrl + C. Exiting...\n"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify file exists and check line count
wc -l cxx-api-examples/fire-red-asr-ctc-simulate-streaming-alsa-cxx-api.ccRepository: k2-fsa/sherpa-onnx
Length of output: 134
🏁 Script executed:
# Check the code around lines 37-43 (stop declaration and Handler)
sed -n '30,50p' cxx-api-examples/fire-red-asr-ctc-simulate-streaming-alsa-cxx-api.ccRepository: k2-fsa/sherpa-onnx
Length of output: 672
🏁 Script executed:
# Check the code around lines 167-172 (also applies)
sed -n '160,180p' cxx-api-examples/fire-red-asr-ctc-simulate-streaming-alsa-cxx-api.ccRepository: k2-fsa/sherpa-onnx
Length of output: 520
🏁 Script executed:
# Search for all accesses to 'stop' variable to confirm thread-access patterns
rg -n '\bstop\b' cxx-api-examples/fire-red-asr-ctc-simulate-streaming-alsa-cxx-api.ccRepository: k2-fsa/sherpa-onnx
Length of output: 217
🏁 Script executed:
# Check what includes are present (especially atomic)
head -20 cxx-api-examples/fire-red-asr-ctc-simulate-streaming-alsa-cxx-api.ccRepository: k2-fsa/sherpa-onnx
Length of output: 795
Fix signal-handler safety and data race on stop flag.
stop is declared as a plain bool but is accessed from multiple threads and within a signal handler:
- Signal handler writes to
stopand callscondition_variable.notify_one()(not signal-safe) - Main thread and
RecordCallbackthread readstopwithout synchronization
This creates both a data race and violates signal-handler safety rules.
🔧 Proposed fix
+#include <atomic>
...
-bool stop = false;
+std::atomic<bool> stop{false};
static void Handler(int32_t /*sig*/) {
- stop = true;
- condition_variable.notify_one();
- fprintf(stderr, "\nCaught Ctrl + C. Exiting...\n");
+ stop.store(true, std::memory_order_relaxed);
}
static void RecordCallback(sherpa_onnx::Alsa *alsa) {
int32_t chunk = 0.1 * alsa->GetActualSampleRate();
- while (!stop) {
+ while (!stop.load(std::memory_order_relaxed)) {
std::vector<float> samples = alsa->Read(chunk);
...
- while (!stop) {
+ while (!stop.load(std::memory_order_relaxed)) {
{
std::unique_lock<std::mutex> lock(mutex);
- while (samples_queue.empty() && !stop) {
- condition_variable.wait(lock);
+ while (samples_queue.empty() &&
+ !stop.load(std::memory_order_relaxed)) {
+ condition_variable.wait_for(lock, std::chrono::milliseconds(100));
}
- if (stop) {
+ if (stop.load(std::memory_order_relaxed)) {
break;
}Also applies to: lines 167–173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cxx-api-examples/fire-red-asr-ctc-simulate-streaming-alsa-cxx-api.cc` around
lines 37 - 43, The signal handler and unsynchronized access to stop cause a data
race and use of non-signal-safe APIs; change the global bool stop to
std::atomic<bool> stop; remove the call to condition_variable.notify_one() from
Handler and only set stop.store(true) inside the handler, and update all
reads/writes (in RecordCallback and the main loop) to use stop.load()/store() so
they are atomic; additionally, move any
condition_variable.notify_one()/notify_all() calls out of the signal handler
into normal thread context (e.g., have the main thread or the recording thread
detect stop.load() and then call condition_variable.notify_one()) so no
non-signal-safe functions run in Handler.
Comment on lines
+39
to
+43
| static void Handler(int32_t /*sig*/) { | ||
| stop = true; | ||
| condition_variable.notify_one(); | ||
| fprintf(stderr, "\nCaught Ctrl + C. Exiting...\n"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "fire-red-asr-ctc-simulate-streaming-microphone-cxx-api.cc" -type fRepository: k2-fsa/sherpa-onnx
Length of output: 138
🏁 Script executed:
# Check the file size first
wc -l cxx-api-examples/fire-red-asr-ctc-simulate-streaming-microphone-cxx-api.ccRepository: k2-fsa/sherpa-onnx
Length of output: 140
🏁 Script executed:
# Read the specific lines around the handler (lines 30-50 for context)
sed -n '30,60p' cxx-api-examples/fire-red-asr-ctc-simulate-streaming-microphone-cxx-api.ccRepository: k2-fsa/sherpa-onnx
Length of output: 1179
🏁 Script executed:
rg -n "signal|Handler" cxx-api-examples/fire-red-asr-ctc-simulate-streaming-microphone-cxx-api.ccRepository: k2-fsa/sherpa-onnx
Length of output: 157
🏁 Script executed:
# Check for signal registration and how Handler is used
sed -n '1,242p' cxx-api-examples/fire-red-asr-ctc-simulate-streaming-microphone-cxx-api.cc | tail -100Repository: k2-fsa/sherpa-onnx
Length of output: 2816
🌐 Web query:
async-signal-safe functions POSIX C standard condition_variable notify
💡 Result:
-
Async-signal-safe (POSIX) means a function can be called from an asynchronous signal handler without invoking undefined behavior (POSIX defines a specific whitelist; see
signal-safety(7)). [1] -
Condition variables are not async-signal-safe:
- POSIX explicitly says it is not safe to call
pthread_cond_signal()from an asynchronously-invoked signal handler, and concludes that mutexes and condition variables are not suitable for waking a waiting thread from a signal handler. [2] - Linux man pages likewise warn that
pthread_cond_signal()/pthread_cond_broadcast()from a signal handler may deadlock. [3]
- POSIX explicitly says it is not safe to call
-
Therefore,
std::condition_variable::notify_one()/notify_all()are not async-signal-safe in practice on POSIX systems, because common C++ standard libraries implement them usingpthread_cond_*(so the same restriction applies). [4] -
What to use instead (typical POSIX patterns):
- Use an async-signal-safe “wake mechanism”, e.g.
sem_post()(explicitly async-signal-safe) to release a thread blocked insem_wait(). [5] - Or use a self-pipe/eventfd-style approach: the handler does a
write()to wake a thread blocked inpoll/select/read(those are on the async-signal-safe whitelist). [1]
- Use an async-signal-safe “wake mechanism”, e.g.
Sources: signal-safety(7) [1], POSIX/Open Group pthread_cond_signal rationale [2], Linux pthread_cond_signal(3) man page [3], discussion of C++ condition_variable relying on pthread condvars [4], sem_post(3) man page [5].
🌐 Web query:
volatile sig_atomic_t signal handler C++ best practice
💡 Result:
Best practice: what to do in a C++ signal handler
1) Keep the handler minimal
For an asynchronous signal (e.g., SIGINT arriving “at any time”), the portable pattern is:
- set a flag
- return
- let your normal code notice the flag and do the real work
C/C++ only guarantees safe communication via:
volatile std::sig_atomic_t(classic, portable), and/or- lock-free atomics (allowed by the standard/POSIX when lock-free) [1][2][3].
2) Don’t call “normal” code from the handler
In an async signal handler, calling most library functions is undefined behavior (C/C++), and on POSIX you must restrict yourself to async-signal-safe functions (e.g., write, _exit, etc.). printf, new/malloc, std::cout, mutexes, logging frameworks, etc. are not safe. See the POSIX/Linux async-signal-safe lists/details. [1][2]
3) Prefer sigaction() over signal() (POSIX)
signal() has historical/portability quirks; POSIX recommends sigaction() for well-defined behavior and options (signal masks, flags like SA_RESTART, etc.). [1]
4) If you need to wake a blocked thread: use the “self-pipe”/fd notification pattern (POSIX)
If your main loop might be blocked in poll/epoll/select, have the handler do only an async-signal-safe write() of 1 byte to a pipe (or similar), and let the main loop read it and shut down gracefully. (The key point: write() is async-signal-safe; complex work stays outside the handler.) [2]
5) If you touch errno, save/restore it
POSIX notes errno access is async-signal-safe if you save it on entry and restore it before returning. [3]
Minimal, idiomatic examples
Portable “set a flag” (C/C++)
`#include` <csignal>
static volatile std::sig_atomic_t g_stop = 0;
extern "C" void on_signal(int) {
g_stop = 1; // do nothing else
}This matches the standard’s constraints for async handlers (only volatile sig_atomic_t / lock-free atomics + very limited calls). [1]
POSIX “self-pipe” wakeup (sketch)
Handler does only write(pipefd[1], &byte, 1); main loop watches pipefd[0] in poll/epoll/select and performs shutdown outside the handler. (write is async-signal-safe.) [2]
Rule of thumb
If you’re thinking “I’ll just log / allocate / lock / throw / call a callback” inside the handler: don’t. Set a flag (or write() to a pipe) and handle it later. [1][2][4]
Sources: C/C++ signal-handler restrictions and allowed operations [1]; Linux/POSIX async-signal-safe functions concept and list [2]; POSIX requirements and errno guidance [3]; CERT guidance on only using volatile sig_atomic_t or lock-free atomics for shared objects in handlers [4].
References
[1] cppreference: std::signal restrictions (async handlers, volatile sig_atomic_t, lock-free atomics) (cppreference.cn)
[2] Linux man-pages: signal-safety(7) async-signal-safe functions (man7.org)
[3] The Open Group / POSIX (signal safety, lock-free atomics, errno save/restore) (pubs.opengroup.org)
[4] CERT (SIG31-C) guidance on shared objects in signal handlers (mathworks.com)
Citations:
- 1: https://cppreference.cn/w/cpp/utility/program/signal?utm_source=openai
- 2: https://man7.org/linux/man-pages/man7/signal-safety.7.html?utm_source=openai
- 3: https://pubs.opengroup.org/onlinepubs/9799919799/functions/V2_chap02.html?utm_source=openai
- 4: https://www.mathworks.com/help/bugfinder/ref/certcsig31c.html?utm_source=openai
Remove all non-async-signal-safe operations from the SIGINT handler.
Calling condition_variable.notify_one() (line 41) and fprintf() (line 42) in a signal handler is undefined behavior and can deadlock or crash. The POSIX standard prohibits pthread_cond_signal() from async signal handlers due to potential deadlock when the signal interrupts a thread holding the mutex. The handler must only set a volatile sig_atomic_t flag; all other work belongs in normal program flow.
Suggested fix
-std::condition_variable condition_variable;
-std::mutex mutex;
-bool stop = false;
+std::condition_variable condition_variable;
+std::mutex mutex;
+volatile std::sig_atomic_t stop = 0;
static void Handler(int32_t /*sig*/) {
- stop = true;
- condition_variable.notify_one();
- fprintf(stderr, "\nCaught Ctrl + C. Exiting...\n");
+ stop = 1;
}- while (samples_queue.empty() && !stop) {
- condition_variable.wait(lock);
+ while (samples_queue.empty() && !stop) {
+ condition_variable.wait_for(lock, std::chrono::milliseconds(100));
}+ if (stop) {
+ fprintf(stderr, "\nCaught Ctrl + C. Exiting...\n");
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cxx-api-examples/fire-red-asr-ctc-simulate-streaming-microphone-cxx-api.cc`
around lines 39 - 43, The SIGINT handler "Handler" performs
non-async-signal-safe actions (calling condition_variable.notify_one() and
fprintf()) which is undefined behavior; change the handler to only set a
volatile sig_atomic_t flag (e.g., stop) and return immediately, and move the
condition_variable.notify_one() and the stderr message (or any logging) into the
normal program flow where you detect the flag (e.g., main loop or the thread
waiting on the condition variable) so you can safely call
condition_variable.notify_one() and print an exit message.
Comment on lines
+112
to
+118
| PaDeviceIndex num_devices = Pa_GetDeviceCount(); | ||
| if (num_devices == 0) { | ||
| std::cerr | ||
| << " If you are using Linux, please try " | ||
| "./build/bin/fire-red-asr-ctc-simulate-streaming-alsa-cxx-api\n"; | ||
| return -1; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PortAudio Pa_GetDeviceCount() error handling negative return values
💡 Result:
Pa_GetDeviceCount() returns either:
>= 0: the number of devices (may be 0), or< 0: aPaError(PortAudio error codes are always negative). [1][2]
Recommended handling pattern
PaError err;
PaDeviceIndex n = Pa_GetDeviceCount();
if (n < 0) {
err = (PaError)n;
fprintf(stderr, "Pa_GetDeviceCount failed: %s\n", Pa_GetErrorText(err));
if (err == paUnanticipatedHostError) {
const PaHostErrorInfo *info = Pa_GetLastHostErrorInfo();
if (info) {
fprintf(stderr, "Host API %d, code %ld: %s\n",
info->hostApiType, info->errorCode,
info->errorText ? info->errorText : "(no text)");
}
}
/* handle/return error */
} else {
/* n is the device count */
}Common cause
If PortAudio hasn’t been initialized, Pa_GetDeviceCount() can return paNotInitialized (negative). Ensure you call Pa_Initialize() successfully before querying devices. [2][3]
Sources: PortAudio API overview (error handling) [1], Pa_GetDeviceCount reference docs [2], initialization docs [3].
🏁 Script executed:
# Search for Pa_GetDeviceCount usage in the codebase
rg "Pa_GetDeviceCount" -A 5 -B 2
# Search for Pa_GetErrorText usage
rg "Pa_GetErrorText"
# Check for paError type definition
rg "paError|PaError" -B 2 -A 2
# Find the file in question and examine its content
fd "fire-red-asr-ctc-simulate-streaming-microphone-cxx-api.cc"Repository: k2-fsa/sherpa-onnx
Length of output: 45437
🌐 Web query:
PortAudio Pa_GetDeviceCount returns error code negative value
💡 Result:
Pa_GetDeviceCount() returns a PaDeviceIndex on success (>= 0), but if it returns a negative value, that value is a PortAudio error code (PaError) (all errors are negative). (portaudio.com)
Most common cause
You called Pa_GetDeviceCount() before Pa_Initialize() → PortAudio returns paNotInitialized. PortAudio requires Pa_Initialize() before almost all API calls. (portaudio.com)
What to do
- Initialize first
- Print the error text (and optionally host details)
PaError err = Pa_Initialize();
if (err != paNoError) {
fprintf(stderr, "Pa_Initialize failed: %s\n", Pa_GetErrorText(err));
return 1;
}
PaDeviceIndex n = Pa_GetDeviceCount();
if (n < 0) { // n is actually a PaError
fprintf(stderr, "Pa_GetDeviceCount failed: %s (%d)\n",
Pa_GetErrorText((PaError)n), (int)n);
// optional: check Pa_GetLastHostErrorInfo() for host-API specific details
Pa_Terminate();
return 1;
}
printf("devices: %d\n", (int)n);
Pa_Terminate();PortAudio’s own “Querying Devices” doc shows the same < 0 check pattern for Pa_GetDeviceCount(). (files.portaudio.com)
Citations:
- 1: https://www.portaudio.com/docs/v19-doxydocs/portaudio_8h.html?utm_source=openai
- 2: https://portaudio.com/docs/v19-doxydocs-dev/portaudio_8h.html?utm_source=openai
- 3: https://files.portaudio.com/docs/v19-doxydocs/querying_devices.html?utm_source=openai
🏁 Script executed:
# Find and examine the file
fd "fire-red-asr-ctc" --type f
# Search for Pa_GetDeviceCount usage in the codebase
rg "Pa_GetDeviceCount" -A 5 -B 2
# Search for error handling patterns with PortAudio functions
rg "Pa_.*Error|paError" -B 2 -A 2
# Look for PortAudio header includes to understand API
rg "portaudio|pa\.h|Pa_" --type cc --type cpp | head -30Repository: k2-fsa/sherpa-onnx
Length of output: 50418
Add check for negative Pa_GetDeviceCount() error codes.
Pa_GetDeviceCount() returns negative error codes on failure (all PortAudio errors are negative). The current code only checks == 0 and will continue with invalid state if an error occurs. Add an explicit check before the zero-device check:
Suggested fix
PaDeviceIndex num_devices = Pa_GetDeviceCount();
+ if (num_devices < 0) {
+ std::cerr << "PortAudio error: " << Pa_GetErrorText(num_devices) << "\n";
+ return -1;
+ }
if (num_devices == 0) {
std::cerr
<< " If you are using Linux, please try "
"./build/bin/fire-red-asr-ctc-simulate-streaming-alsa-cxx-api\n";
return -1;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cxx-api-examples/fire-red-asr-ctc-simulate-streaming-microphone-cxx-api.cc`
around lines 112 - 118, Pa_GetDeviceCount() can return negative PortAudio error
codes; update the logic around the call in the main flow that assigns
PaDeviceIndex num_devices = Pa_GetDeviceCount() to first check for num_devices <
0 and handle that as a failure (log an error including PortAudio's message using
Pa_GetErrorText(num_devices) or similar) before the existing num_devices == 0
check, then return an error code to abort initialization.
Comment on lines
+44
to
+46
| func readWave(filename string) (samples []float32, sampleRate int) { | ||
| file, _ := os.Open(filename) | ||
| defer file.Close() |
There was a problem hiding this comment.
Ignored error from os.Open could cause a nil pointer panic.
If the file doesn't exist or can't be opened, file will be nil, and the subsequent file.Close() and wav.NewReader(file) calls will panic. The error should be checked.
🐛 Proposed fix
func readWave(filename string) (samples []float32, sampleRate int) {
- file, _ := os.Open(filename)
+ file, err := os.Open(filename)
+ if err != nil {
+ log.Fatalf("Failed to open file %s: %v", filename, err)
+ }
defer file.Close()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func readWave(filename string) (samples []float32, sampleRate int) { | |
| file, _ := os.Open(filename) | |
| defer file.Close() | |
| func readWave(filename string) (samples []float32, sampleRate int) { | |
| file, err := os.Open(filename) | |
| if err != nil { | |
| log.Fatalf("Failed to open file %s: %v", filename, err) | |
| } | |
| defer file.Close() |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go-api-examples/non-streaming-fire-red-asr-ctc-decode-files/main.go` around
lines 44 - 46, The function readWave currently ignores the error from os.Open
which can leave file nil and cause panics on file.Close or wav.NewReader; update
readWave to check the error returned by os.Open (and return an error from
readWave or handle it appropriately), only defer file.Close after confirming
file is non-nil, and propagate any errors from wav.NewReader/reading operations
back to the caller (e.g., change signature to return (samples []float32,
sampleRate int, err error) and use error checks for os.Open and wav.NewReader
inside readWave).
Comment on lines
+9
to
+21
| async function createRecognizerAsync(modelDir, numThreads = 2, debug = 1) { | ||
| const config = { | ||
| featConfig: { | ||
| sampleRate: 16000, | ||
| featureDim: 80, | ||
| }, | ||
| modelConfig: { | ||
| fireRedAsrCtc: { | ||
| model: | ||
| './sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25/model.int8.onnx', | ||
| }, | ||
| tokens: | ||
| './sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25/tokens.txt', |
There was a problem hiding this comment.
modelDir parameter is ignored, making recognizer creation misleading.
main() passes modelDir (Line 46), but createRecognizerAsync() always uses hardcoded paths (Lines 18 and 21). This breaks expected behavior when a different model directory is provided.
Suggested fix
async function createRecognizerAsync(modelDir, numThreads = 2, debug = 1) {
const config = {
@@
modelConfig: {
fireRedAsrCtc: {
- model:
- './sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25/model.int8.onnx',
+ model: path.join(modelDir, 'model.int8.onnx'),
},
- tokens:
- './sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25/tokens.txt',
+ tokens: path.join(modelDir, 'tokens.txt'),
numThreads,
provider: 'cpu',
debug,Also applies to: 43-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nodejs-addon-examples/test_asr_non_streaming_fire_red_asr_ctc_async.js`
around lines 9 - 21, The createRecognizerAsync function is ignoring the modelDir
argument and uses hardcoded paths; modify createRecognizerAsync to build the
model and tokens paths from the provided modelDir (instead of the current
'./sherpa-onnx-fire-red-asr2-ctc-zh_en-int8-2026-02-25/...') so main(modelDir)
actually selects the intended files—update references
modelConfig.fireRedAsrCtc.model and modelConfig.fireRedAsrCtc.tokens (or tokens)
to use path joining (e.g., path.join(modelDir, 'model.int8.onnx') and
path.join(modelDir, 'tokens.txt')) and ensure any other places in
createRecognizerAsync that reference those hardcoded filenames use the
constructed paths.
Comment on lines
+39
to
+41
| const elapsed_seconds = (stop - start) / 1000; | ||
| const duration = wave.samples.length / wave.sampleRate; | ||
| const real_time_factor = elapsed_seconds / duration; |
There was a problem hiding this comment.
Guard RTF calculation for zero-duration input.
If wave.samples.length is 0, duration becomes 0 and real_time_factor becomes Infinity/NaN. Add a zero check before division.
Suggested fix
const elapsed_seconds = (stop - start) / 1000;
const duration = wave.samples.length / wave.sampleRate;
-const real_time_factor = elapsed_seconds / duration;
+const real_time_factor = duration > 0 ? elapsed_seconds / duration : 0;
console.log('Wave duration', duration.toFixed(3), 'seconds')
console.log('Elapsed', elapsed_seconds.toFixed(3), 'seconds')
console.log(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nodejs-addon-examples/test_asr_non_streaming_fire_red_asr_ctc.js` around
lines 39 - 41, The real-time factor calculation can divide by zero when
wave.samples.length is 0; after computing duration (duration =
wave.samples.length / wave.sampleRate) add a guard: if duration === 0 set
real_time_factor to null (or a sentinel like 0/NaN per project convention)
instead of performing elapsed_seconds / duration, otherwise compute
real_time_factor = elapsed_seconds / duration; update the code around
elapsed_seconds, duration and real_time_factor to use this conditional branch to
avoid Infinity/NaN.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/scripts/test-dot-net.sh (1)
63-106: Consider extracting repeated module steps into a small helper.Lines 63-106 add several repeated
cd/run/cleanup patterns. A helper would reduce duplication and make future test additions less error-prone.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/scripts/test-dot-net.sh around lines 63 - 106, Extract the repeated cd/run/ls/remove pattern in .github/scripts/test-dot-net.sh into a helper shell function (e.g., run_module) that accepts the target directory and the command to run (and an optional cleanup pattern), then replace each block that uses cd ../<module> + ./run*.sh + ls -lh + rm -... with calls to run_module "module-dir" "./run.sh" "cleanup-pattern"; update calls that use different run scripts (./run-ten-vad.sh, ./run-kitten.sh) to pass the correct command and cleanup arg so all repeated sequences (pocket-tts-zero-shot, vad-non-streaming-funasr-nano, non-streaming-funasr-nano-decode-files, version-test, offline-audio-tagging, kitten-tts, vad-non-streaming-asr-paraformer, non-streaming-canary-decode-files) are consolidated using the new run_module helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cxx-api-examples/fire-red-asr-ctc-cxx-api.cc`:
- Around line 42-46: After calling ReadWave and validating wave.samples, also
guard against an invalid sample rate by checking wave.sample_rate <= 0 before
computing duration or rtf; if invalid, print an error like "Invalid sample rate"
and return -1. Update both places where duration/rtf are computed (references:
ReadWave, wave.samples, wave.sample_rate, duration, rtf) to perform this
defensive check to avoid division-by-zero/inf.
- Around line 15-19: Add a proper C stdio header and/or use the C++ stream API
to fix the undefined printf usage: include <cstdio> at the top of the
translation unit (so std::printf is available) and update the existing printf
calls to std::printf, or replace those printf calls with std::cout usage for
consistency with the existing <iostream> include; target the occurrences of
printf in the file and the top-of-file includes near the existing `#include`
"sherpa-onnx/c-api/cxx-api.h".
---
Nitpick comments:
In @.github/scripts/test-dot-net.sh:
- Around line 63-106: Extract the repeated cd/run/ls/remove pattern in
.github/scripts/test-dot-net.sh into a helper shell function (e.g., run_module)
that accepts the target directory and the command to run (and an optional
cleanup pattern), then replace each block that uses cd ../<module> + ./run*.sh +
ls -lh + rm -... with calls to run_module "module-dir" "./run.sh"
"cleanup-pattern"; update calls that use different run scripts
(./run-ten-vad.sh, ./run-kitten.sh) to pass the correct command and cleanup arg
so all repeated sequences (pocket-tts-zero-shot, vad-non-streaming-funasr-nano,
non-streaming-funasr-nano-decode-files, version-test, offline-audio-tagging,
kitten-tts, vad-non-streaming-asr-paraformer, non-streaming-canary-decode-files)
are consolidated using the new run_module helper.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/scripts/test-dot-net.sh.github/scripts/test-rust.sh.github/workflows/cxx-api.yamlcxx-api-examples/fire-red-asr-ctc-cxx-api.cc
Comment on lines
+42
to
+46
| Wave wave = ReadWave(wave_filename); | ||
| if (wave.samples.empty()) { | ||
| std::cerr << "Failed to read: '" << wave_filename << "'\n"; | ||
| return -1; | ||
| } |
There was a problem hiding this comment.
Guard against invalid sample rate before computing duration/RTF.
If wave.sample_rate <= 0, duration/rtf becomes invalid (division by zero/inf). Add a defensive check after ReadWave.
Suggested patch
Wave wave = ReadWave(wave_filename);
if (wave.samples.empty()) {
std::cerr << "Failed to read: '" << wave_filename << "'\n";
return -1;
}
+ if (wave.sample_rate <= 0) {
+ std::cerr << "Invalid sample rate: " << wave.sample_rate << "\n";
+ return -1;
+ }
@@
- float duration = wave.samples.size() / static_cast<float>(wave.sample_rate);
- float rtf = elapsed_seconds / duration;
+ float duration = wave.samples.size() / static_cast<float>(wave.sample_rate);
+ float rtf = duration > 0 ? (elapsed_seconds / duration) : 0.0f;Also applies to: 64-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cxx-api-examples/fire-red-asr-ctc-cxx-api.cc` around lines 42 - 46, After
calling ReadWave and validating wave.samples, also guard against an invalid
sample rate by checking wave.sample_rate <= 0 before computing duration or rtf;
if invalid, print an error like "Invalid sample rate" and return -1. Update both
places where duration/rtf are computed (references: ReadWave, wave.samples,
wave.sample_rate, duration, rtf) to perform this defensive check to avoid
division-by-zero/inf.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All 12 programming language APIs have been updated.
Summary by CodeRabbit
New Features
Chores
Documentation