Improve C/C++ API documentation, examples, and CI - #3626
Merged
Merged
Conversation
dosubot
Bot
added
the
size:XXL
📝 WalkthroughWalkthroughRefactors CI workflows to centralize build artifacts and library paths; adds many C/C++ example programs and CMake targets; consolidates example config patterns; introduces C++ audio-analysis RAII wrappers (language ID, speaker embedding/manager, diarization); and expands Doxygen docs with model-specific pages and header cross-links. ChangesCI Workflows and Build Infrastructure
C API Examples
C++ API Examples
C++ API New Audio Analysis Features
API Documentation Infrastructure
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs:
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![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 adds several new C and C++ API examples for various models, including NeMo CTC, GigaAM v2, streaming Nemotron, and others, along with corresponding documentation updates. My review identified a compilation error in the Piper TTS example where a non-existent field was accessed, as well as some minor code quality improvements regarding namespace usage and error handling in the speaker identification example.
|
|
||
| #include "sherpa-onnx/c-api/cxx-api.h" | ||
|
|
||
| using namespace sherpa_onnx::cxx; // NOLINT |
There was a problem hiding this comment.
Using using namespace at the global scope in a source file is generally discouraged as it can lead to name collisions. It is better to move it inside main() to be consistent with other examples in this pull request.
| Wave wave = ReadWave(wav_filename); | ||
| if (wave.samples.empty()) { | ||
| std::cerr << "Failed to read " << wav_filename << "\n"; | ||
| exit(-1); |
There was a problem hiding this comment.
Using exit(-1) is non-standard. For failure, it is recommended to use exit(EXIT_FAILURE) from or return a non-zero value from main(). This also applies to line 42.
Suggested change
| exit(-1); | |
| exit(EXIT_FAILURE); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
.github/workflows/c-api.yaml (1)
1650-1665: 💤 Low valueRemove disabled step with constant
if: falsecondition.The static analysis tool (actionlint) flags the
if: falseas a constant expression. If the ffmpeg test is not intended to run, it's cleaner to remove or comment out the entire step rather than leaving it with a hardcoded false condition.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/c-api.yaml around lines 1650 - 1665, Remove the disabled GitHub Actions step named "Test ffmpeg" that uses the constant condition if: false (the whole YAML block starting with the step name "Test ffmpeg"); either delete the entire step or comment it out so actionlint no longer sees a constant expression, and keep any needed test logic elsewhere or behind a proper conditional matrix entry or input-driven if condition.cxx-api-examples/streaming-paraformer-cxx-api.cc (1)
17-22: ⚡ Quick winInclude
<algorithm>explicitly forstd::min.Line 71 uses
std::min, which requires<algorithm>. Avoid relying on transitive includes.Proposed fix
`#include` <chrono> // NOLINT +#include <algorithm> `#include` <cstdio> `#include` <iostream> `#include` <string>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cxx-api-examples/streaming-paraformer-cxx-api.cc` around lines 17 - 22, The code uses std::min (referenced at the call to std::min in streaming-paraformer-cxx-api.cc) but doesn't include <algorithm>, so add an explicit `#include` <algorithm> to the top include block (alongside <chrono>, <cstdio>, <iostream>, <string>) to avoid relying on transitive includes and ensure std::min is declared.cxx-api-examples/offline-tts-piper-cxx-api.cc (2)
62-62: 💤 Low valueClarify silence_scale initialization.
gen_config.silence_scaleis assigned fromconfig.silence_scale, butconfig.silence_scalewas never explicitly set. This relies on default initialization, which may be intentional but is unclear to readers.♻️ Consider explicit initialization
Either set
config.silence_scaleexplicitly before this line, or setgen_config.silence_scaledirectly:-gen_config.silence_scale = config.silence_scale; +gen_config.silence_scale = 1.0; // or appropriate default🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cxx-api-examples/offline-tts-piper-cxx-api.cc` at line 62, The assignment gen_config.silence_scale = config.silence_scale uses config.silence_scale without it being explicitly initialized; either initialize config.silence_scale to the intended default value earlier (e.g., set config.silence_scale = <desired_value> before the assignment) or avoid relying on config and set gen_config.silence_scale directly to the intended default/constant when constructing gen_config so the value is explicit and readable.
58-58: ⚡ Quick winAdd error checking after TTS creation.
The code doesn't verify that
OfflineTts::Createsucceeded. If the model files are missing or invalid, subsequent operations onttsmay fail or crash.🛡️ Proposed validation check
auto tts = OfflineTts::Create(config); +if (!tts.Get()) { + std::cerr << "Failed to create TTS model\n"; + return -1; +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cxx-api-examples/offline-tts-piper-cxx-api.cc` at line 58, The code calls OfflineTts::Create(config) but doesn't check the return value; verify that the returned pointer or optional (variable tts) is valid before using it and handle failure by logging an error and exiting or returning an error status. Locate the OfflineTts::Create call and after it check if tts is null/empty or indicates failure (depending on its return type) and then call process logger/print an error like "Failed to create OfflineTts" along with any error info and abort/return to avoid subsequent dereference of tts.cxx-api-examples/speaker-identification-cxx-api.cc (1)
62-62: ⚡ Quick winAdd error checking after manager creation.
For consistency with the extractor creation (lines 54-57), verify that
SpeakerEmbeddingManager::Createsucceeded before proceeding with enrollment operations.🛡️ Proposed validation check
SpeakerEmbeddingManager manager = SpeakerEmbeddingManager::Create(dim); +if (!manager.Get()) { + std::cerr << "Failed to create speaker embedding manager\n"; + return -1; +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cxx-api-examples/speaker-identification-cxx-api.cc` at line 62, Verify the result of SpeakerEmbeddingManager::Create(dim) just like the extractor creation: check that the returned manager is valid (non-null or has_value depending on its return type) before using it for enrollment, and if creation failed log an error via the same logger and exit/return early; update references to manager in enrollment code to assume a valid manager only after this validation.sherpa-onnx/c-api/cxx-api.h (1)
1867-1888: ⚡ Quick winAdd buffer-size and termination documentation to the C++ wrapper header.
The C API documentation (in
c-api.h) clearly specifies thatAddList()expects a NULL-terminated pointer array,AddListFlattened()'snparameter is the embedding count (not float count), andAdd()/Search()/Verify()operate on vectors of exactlydimelements. However, the C++ wrapper header comments at lines 1869–1887 omit these details, leaving C++ callers to either guess the contract or dig into the C API docs. Add explicit dimension and termination requirements to the C++ method comments to match the clarity of the C API layer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sherpa-onnx/c-api/cxx-api.h` around lines 1867 - 1888, Update the C++ wrapper comments for the SpeakerManager methods to explicitly document buffer sizes and termination: for Add(const std::string &name, const float *v) state that v must point to exactly dim floats (embedding length); for AddList(const std::string &name, const float **v) state that v is a NULL-terminated array of pointers, each pointing to an embedding of exactly dim floats; for AddListFlattened(const std::string &name, const float *v, int32_t n) state that v is a flat array containing n embeddings (so length = n * dim) and that n is the number of embeddings (not number of floats); and for Search(const float *v, float threshold) and Verify(const std::string &name, const float *v, float threshold) state that v must point to exactly dim floats; update the brief comments above those method declarations (Add, AddList, AddListFlattened, Search, Verify) to include these requirements.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cxx-api-examples/offline-speaker-diarization-cxx-api.cc`:
- Line 15: The wget command in offline-speaker-diarization-cxx-api.cc contains a
misspelling in the release path: replace "speaker-recongition-models" with
"speaker-recognition-models" in the commented download URL; update the comment
line that starts with "wget
https://github.com/k2-fsa/sherpa-onnx/releases/download/..." so the path is
corrected to the proper release directory name.
In `@sherpa-onnx/c-api/cxx-api.cc`:
- Around line 1448-1454: The bridge DiarizationProgressCallback currently
dereferences and invokes OfflineSpeakerDiarizationProgressCallback
unconditionally and lets exceptions cross the C boundary; update the bridge
(DiarizationProgressCallback) to first check that the pointer is non-null and
that the std::function is non-empty before calling, and wrap the invocation in a
try/catch that swallows or logs exceptions so they don't unwind past the C
callback boundary. Additionally, when registering the callback before calling
SherpaOnnxOfflineSpeakerDiarizationProcessWithCallback, only pass the pointer if
the supplied OfflineSpeakerDiarizationProgressCallback is non-empty (otherwise
pass nullptr) so the bridge can skip invocation safely. Ensure you reference the
symbols DiarizationProgressCallback, OfflineSpeakerDiarizationProgressCallback,
and SherpaOnnxOfflineSpeakerDiarizationProcessWithCallback when making the
changes.
In `@sherpa-onnx/c-api/docs/punctuation.dox`:
- Around line 26-30: Update the doc snippet to use the consistent
SherpaOnnxOffline* API names: replace SherpaOfflinePunctuationAddPunct with
SherpaOnnxOfflinePunctuationAddPunct and replace
SherpaOfflinePunctuationFreeText with SherpaOnnxOfflinePunctuationFreeText so
all offline punctuation calls (e.g., the call that produces result and the free
call) match the SherpaOnnxOffline naming used elsewhere (leave
SherpaOnnxDestroyOfflinePunctuation as-is).
In `@sherpa-onnx/c-api/docs/source-separation.dox`:
- Around line 28-30: The examples create a SherpaOnnxOfflineSourceSeparation
instance via SherpaOnnxCreateOfflineSourceSeparation(&config) but never destroy
it; add a call to SherpaOnnxDestroyOfflineSourceSeparation(ss) after the usage
of the ss variable in both snippets (the snippet around the creation at lines
28-30 and the other example at 47-49) to properly free resources and avoid
leaking the SherpaOnnxOfflineSourceSeparation object.
In `@sherpa-onnx/c-api/docs/speech-enhancement.dox`:
- Around line 45-47: Add explicit destroy calls for every denoiser/audio
allocation shown in the snippets: after
SherpaOnnxCreateOfflineSpeechDenoiser(&config) call invoke
SherpaOnnxDestroyOfflineSpeechDenoiser(sd); do the same for the online denoiser
example by pairing SherpaOnnxCreateOnlineSpeechDenoiser(...) with
SherpaOnnxDestroyOnlineSpeechDenoiser(...) and for any denoised audio buffers
pair their creation with SherpaOnnxDestroyDenoisedAudio(...); place the destroy
calls immediately after the last use of each object to make ownership and
lifetimes explicit.
---
Nitpick comments:
In @.github/workflows/c-api.yaml:
- Around line 1650-1665: Remove the disabled GitHub Actions step named "Test
ffmpeg" that uses the constant condition if: false (the whole YAML block
starting with the step name "Test ffmpeg"); either delete the entire step or
comment it out so actionlint no longer sees a constant expression, and keep any
needed test logic elsewhere or behind a proper conditional matrix entry or
input-driven if condition.
In `@cxx-api-examples/offline-tts-piper-cxx-api.cc`:
- Line 62: The assignment gen_config.silence_scale = config.silence_scale uses
config.silence_scale without it being explicitly initialized; either initialize
config.silence_scale to the intended default value earlier (e.g., set
config.silence_scale = <desired_value> before the assignment) or avoid relying
on config and set gen_config.silence_scale directly to the intended
default/constant when constructing gen_config so the value is explicit and
readable.
- Line 58: The code calls OfflineTts::Create(config) but doesn't check the
return value; verify that the returned pointer or optional (variable tts) is
valid before using it and handle failure by logging an error and exiting or
returning an error status. Locate the OfflineTts::Create call and after it check
if tts is null/empty or indicates failure (depending on its return type) and
then call process logger/print an error like "Failed to create OfflineTts" along
with any error info and abort/return to avoid subsequent dereference of tts.
In `@cxx-api-examples/speaker-identification-cxx-api.cc`:
- Line 62: Verify the result of SpeakerEmbeddingManager::Create(dim) just like
the extractor creation: check that the returned manager is valid (non-null or
has_value depending on its return type) before using it for enrollment, and if
creation failed log an error via the same logger and exit/return early; update
references to manager in enrollment code to assume a valid manager only after
this validation.
In `@cxx-api-examples/streaming-paraformer-cxx-api.cc`:
- Around line 17-22: The code uses std::min (referenced at the call to std::min
in streaming-paraformer-cxx-api.cc) but doesn't include <algorithm>, so add an
explicit `#include` <algorithm> to the top include block (alongside <chrono>,
<cstdio>, <iostream>, <string>) to avoid relying on transitive includes and
ensure std::min is declared.
In `@sherpa-onnx/c-api/cxx-api.h`:
- Around line 1867-1888: Update the C++ wrapper comments for the SpeakerManager
methods to explicitly document buffer sizes and termination: for Add(const
std::string &name, const float *v) state that v must point to exactly dim floats
(embedding length); for AddList(const std::string &name, const float **v) state
that v is a NULL-terminated array of pointers, each pointing to an embedding of
exactly dim floats; for AddListFlattened(const std::string &name, const float
*v, int32_t n) state that v is a flat array containing n embeddings (so length =
n * dim) and that n is the number of embeddings (not number of floats); and for
Search(const float *v, float threshold) and Verify(const std::string &name,
const float *v, float threshold) state that v must point to exactly dim floats;
update the brief comments above those method declarations (Add, AddList,
AddListFlattened, Search, Verify) to include these requirements.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3d7c18b5-ded6-4686-84d0-a832bfe448a0
📒 Files selected for processing (55)
.github/workflows/c-api.yaml.github/workflows/cxx-api.yamlc-api-examples/CMakeLists.txtc-api-examples/add-punctuation-c-api.cc-api-examples/fire-red-asr-ctc-c-api.cc-api-examples/funasr-nano-c-api.cc-api-examples/keywords-spotter-buffered-tokens-keywords-c-api.cc-api-examples/medasr-ctc-c-api.cc-api-examples/nemo-ctc-c-api.cc-api-examples/nemo-giga-am-v2-c-api.cc-api-examples/omnilingual-asr-ctc-c-api.cc-api-examples/paraformer-c-api.cc-api-examples/qwen3-asr-c-api.cc-api-examples/sense-voice-c-api.cc-api-examples/sense-voice-with-hr-c-api.cc-api-examples/streaming-ctc-buffered-tokens-c-api.cc-api-examples/streaming-nemotron-c-api.cc-api-examples/streaming-paraformer-buffered-tokens-c-api.cc-api-examples/streaming-paraformer-c-api.cc-api-examples/streaming-t-one-ctc-c-api.cc-api-examples/streaming-zipformer-buffered-tokens-hotwords-c-api.cc-api-examples/streaming-zipformer-c-api.cc-api-examples/vad-sense-voice-c-api.cc-api-examples/wenet-ctc-c-api.cc-api-examples/whisper-c-api.cc-api-examples/zipformer-c-api.ccxx-api-examples/CMakeLists.txtcxx-api-examples/nemo-ctc-cxx-api.cccxx-api-examples/nemo-giga-am-v2-cxx-api.cccxx-api-examples/offline-speaker-diarization-cxx-api.cccxx-api-examples/offline-tts-piper-cxx-api.cccxx-api-examples/paraformer-cxx-api.cccxx-api-examples/speaker-identification-cxx-api.cccxx-api-examples/spoken-language-identification-cxx-api.cccxx-api-examples/streaming-nemotron-cxx-api.cccxx-api-examples/streaming-paraformer-cxx-api.ccsherpa-onnx/c-api/Doxyfilesherpa-onnx/c-api/c-api.hsherpa-onnx/c-api/cxx-api.ccsherpa-onnx/c-api/cxx-api.hsherpa-onnx/c-api/docs/audio-tagging.doxsherpa-onnx/c-api/docs/keyword-spotting.doxsherpa-onnx/c-api/docs/offline-asr.doxsherpa-onnx/c-api/docs/online-asr.doxsherpa-onnx/c-api/docs/punctuation.doxsherpa-onnx/c-api/docs/resampler.doxsherpa-onnx/c-api/docs/source-separation.doxsherpa-onnx/c-api/docs/speaker-diarization.doxsherpa-onnx/c-api/docs/speaker-embedding.doxsherpa-onnx/c-api/docs/speech-enhancement.doxsherpa-onnx/c-api/docs/spoken-language-id.doxsherpa-onnx/c-api/docs/tts.doxsherpa-onnx/c-api/docs/vad.doxsherpa-onnx/c-api/mainpage.mdsherpa-onnx/c-api/sherpa-onnx-symbols-c.exp
| // tar xvf sherpa-onnx-pyannote-segmentation-3-0.tar.bz2 | ||
| // rm sherpa-onnx-pyannote-segmentation-3-0.tar.bz2 | ||
| // | ||
| // wget https://github.com/k2-fsa/sherpa-onnx/releases/download/speaker-recongition-models/3dspeaker_speech_eres2net_base_sv_zh-cn_3dspeaker_16k.onnx |
There was a problem hiding this comment.
Fix typo in the release URL path.
Line 15 uses speaker-recongition-models; this typo is likely to break the example download command.
Proposed fix
-// wget https://github.com/k2-fsa/sherpa-onnx/releases/download/speaker-recongition-models/3dspeaker_speech_eres2net_base_sv_zh-cn_3dspeaker_16k.onnx
+// wget https://github.com/k2-fsa/sherpa-onnx/releases/download/speaker-recognition-models/3dspeaker_speech_eres2net_base_sv_zh-cn_3dspeaker_16k.onnx📝 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
| // wget https://github.com/k2-fsa/sherpa-onnx/releases/download/speaker-recongition-models/3dspeaker_speech_eres2net_base_sv_zh-cn_3dspeaker_16k.onnx | |
| // wget https://github.com/k2-fsa/sherpa-onnx/releases/download/speaker-recognition-models/3dspeaker_speech_eres2net_base_sv_zh-cn_3dspeaker_16k.onnx |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cxx-api-examples/offline-speaker-diarization-cxx-api.cc` at line 15, The wget
command in offline-speaker-diarization-cxx-api.cc contains a misspelling in the
release path: replace "speaker-recongition-models" with
"speaker-recognition-models" in the commented download URL; update the comment
line that starts with "wget
https://github.com/k2-fsa/sherpa-onnx/releases/download/..." so the path is
corrected to the proper release directory name.
Comment on lines
+26
to
+30
| const char *result = SherpaOfflinePunctuationAddPunct( | ||
| punct, "how are you i am fine thank you"); | ||
| printf("%s\n", result); | ||
| SherpaOfflinePunctuationFreeText(result); | ||
| SherpaOnnxDestroyOfflinePunctuation(punct); |
There was a problem hiding this comment.
Fix offline punctuation function names in the snippet.
The offline sample uses inconsistent symbols and likely won’t compile as written (SherpaOffline... vs SherpaOnnx... used elsewhere). Please use the SherpaOnnxOffline... function names.
Proposed doc fix
-const char *result = SherpaOfflinePunctuationAddPunct(
+const char *result = SherpaOnnxOfflinePunctuationAddPunct(
punct, "how are you i am fine thank you");
printf("%s\n", result);
-SherpaOfflinePunctuationFreeText(result);
+SherpaOnnxOfflinePunctuationFreeText(result);
SherpaOnnxDestroyOfflinePunctuation(punct);📝 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
| const char *result = SherpaOfflinePunctuationAddPunct( | |
| punct, "how are you i am fine thank you"); | |
| printf("%s\n", result); | |
| SherpaOfflinePunctuationFreeText(result); | |
| SherpaOnnxDestroyOfflinePunctuation(punct); | |
| const char *result = SherpaOnnxOfflinePunctuationAddPunct( | |
| punct, "how are you i am fine thank you"); | |
| printf("%s\n", result); | |
| SherpaOnnxOfflinePunctuationFreeText(result); | |
| SherpaOnnxDestroyOfflinePunctuation(punct); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sherpa-onnx/c-api/docs/punctuation.dox` around lines 26 - 30, Update the doc
snippet to use the consistent SherpaOnnxOffline* API names: replace
SherpaOfflinePunctuationAddPunct with SherpaOnnxOfflinePunctuationAddPunct and
replace SherpaOfflinePunctuationFreeText with
SherpaOnnxOfflinePunctuationFreeText so all offline punctuation calls (e.g., the
call that produces result and the free call) match the SherpaOnnxOffline naming
used elsewhere (leave SherpaOnnxDestroyOfflinePunctuation as-is).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/test-python.sh:
- Line 252: The script downloads the SenseVoice int8 archive but later calls the
CLI with --sense-voice=$repo/model.onnx which doesn't exist in that archive;
update the references so the subtitle-generation steps use $repo/model.int8.onnx
(or add simple detection logic to prefer model.int8.onnx when present) instead
of $repo/model.onnx; change both occurrences of --sense-voice=$repo/model.onnx
to --sense-voice=$repo/model.int8.onnx (or implement a short check that picks
model.int8.onnx if tokens.txt plus model.int8.onnx are present).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fa6ce343-68d9-46fd-abcf-9a30bf9c0f6f
📒 Files selected for processing (19)
.github/scripts/test-c-api.sh.github/scripts/test-cxx-api.sh.github/scripts/test-python.sh.github/workflows/c-api.yaml.github/workflows/cxx-api.yamlc-api-examples/sense-voice-c-api.cc-api-examples/sense-voice-with-hr-c-api.cc-api-examples/vad-sense-voice-c-api.ccxx-api-examples/sense-voice-cxx-api.cccxx-api-examples/sense-voice-simulate-streaming-alsa-cxx-api.cccxx-api-examples/sense-voice-simulate-streaming-microphone-cxx-api.cccxx-api-examples/sense-voice-with-hr-cxx-api.ccpython-api-examples/generate-subtitles.pypython-api-examples/non_streaming_server.pypython-api-examples/offline-sense-voice-ctc-decode-files-with-hr.pypython-api-examples/offline-sense-voice-ctc-decode-files.pypython-api-examples/simulate-streaming-sense-voice-microphone.pypython-api-examples/two-pass-wss.pypython-api-examples/vad-with-non-streaming-asr.py
✅ Files skipped from review due to trivial changes (5)
- python-api-examples/simulate-streaming-sense-voice-microphone.py
- python-api-examples/two-pass-wss.py
- python-api-examples/vad-with-non-streaming-asr.py
- python-api-examples/generate-subtitles.py
- python-api-examples/non_streaming_server.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/scripts/test-python.sh (1)
282-282: 💤 Low valueConsider quoting variable expansions for defensive coding.
Shellcheck recommends quoting
$repo/model.int8.onnxto prevent potential word splitting or globbing issues. While unlikely to cause problems in this controlled CI environment, it's a defensive best practice.🛡️ Proposed fix
python3 ./python-api-examples/generate-subtitles.py \ --silero-vad-model=./silero_vad.onnx \ - --sense-voice=$repo/model.int8.onnx \ + --sense-voice="$repo/model.int8.onnx" \ --tokens=$repo/tokens.txt \ @@ python3 ./python-api-examples/generate-subtitles.py \ --silero-vad-model=./silero_vad.onnx \ - --sense-voice=$repo/model.int8.onnx \ + --sense-voice="$repo/model.int8.onnx" \ --tokens=$repo/tokens.txt \Note: Many other variable expansions throughout this script also lack quotes. A comprehensive fix would quote them all, but that's beyond the scope of this PR.
Also applies to: 296-296
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/scripts/test-python.sh at line 282, The command flag --sense-voice=$repo/model.int8.onnx should use a quoted variable expansion to avoid word-splitting or globbing; update the invocation that sets --sense-voice to use "--sense-voice=\"$repo/model.int8.onnx\"" (and likewise quote the analogous expansion at the other occurrence mentioned) so the shell treats the path as a single argument.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/scripts/test-python.sh:
- Line 282: The command flag --sense-voice=$repo/model.int8.onnx should use a
quoted variable expansion to avoid word-splitting or globbing; update the
invocation that sets --sense-voice to use
"--sense-voice=\"$repo/model.int8.onnx\"" (and likewise quote the analogous
expansion at the other occurrence mentioned) so the shell treats the path as a
single argument.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c3aa1141-b1e9-4d5f-9da2-8e8338629bf1
📒 Files selected for processing (1)
.github/scripts/test-python.sh
jimmy1984xu
pushed a commit
to jimmy1984xu/sherpa-onnx
that referenced
this pull request
Jul 29, 2026
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
1 participant
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.
Summary
Comprehensive C API documentation and example improvements for sherpa-onnx.
Documentation
(2 models), punctuation, speech enhancement (GTCRN + DPDFNet), source separation (Spleeter + UVR), speaker diarization, speaker embedding, spoken language identification,
keyword spotting, and linear resampler
@seecross-references to all 66 key structs/functions in c-api.h and to all 13 dox filesCED), speech denoiser (GTCRN + DPDFNet), source separation (Spleeter + UVR)
paths, bad formatting
New C++ API wrappers (cxx-api.h / cxx-api.cc)
New examples
C API:
C++ API:
CI improvements
streaming CTC buffered tokens, streaming paraformer buffered tokens, streaming zipformer buffered tokens hotwords, keywords spotter buffered tokens, NeMo CTC, Paraformer CXX,
Offline TTS Piper, streaming paraformer CXX
keywords file for KWS test
Code cleanup
Summary by CodeRabbit
New Features
Tests
Documentation