Support RK NPU for SenseVoice non-streaming ASR models - #2589
Conversation
dosubot
Bot
added
the
size:XXL
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Caution Review failedThe pull request is closed. WalkthroughAdds RKNN support for SenseVoice: Python export/conversion/test tooling, a PyTorch SenseVoice model, RKNN C++ model and decoder implementations, build/config validation and recognizer routing for RKNN, plus a small exporter eval-mode tweak. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant PyExport as export-onnx.py
participant Torch as SenseVoiceSmall (PyTorch)
participant ONNX as ONNX File
User->>PyExport: Run with --input-len-in-seconds
PyExport->>PyExport: Load SentencePiece, CMVN, state_dict
PyExport->>Torch: Instantiate model, call model.eval()
PyExport->>PyExport: Build dummy x and prompt
PyExport->>ONNX: torch.onnx.export(model, (x, prompt)) with metadata
ONNX-->>PyExport: model.onnx + metadata
PyExport-->>User: tokens.txt, model.onnx
sequenceDiagram
autonumber
participant CLI
participant ORT as ONNX Runtime
participant RKNN as RKNN Toolkit
CLI->>ORT: Inspect model.onnx (shapes, custom_string)
ORT-->>CLI: shapes + metadata
CLI->>RKNN: load_onnx(model.onnx, custom_string)
RKNN-->>CLI: build() -> rknn model file
CLI-->>User: model.rknn
sequenceDiagram
autonumber
participant Client
participant Rec as OfflineRecognizerImpl
participant RKImpl as OfflineRecognizerSenseVoiceRknnImpl
participant Model as OfflineSenseVoiceModelRknn
participant Dec as OfflineCtcGreedySearchDecoderRknn
Client->>Rec: Create(config provider="rknn")
Rec->>RKImpl: Construct with config (#if compiled)
Client->>RKImpl: DecodeStreams(streams)
loop per stream
RKImpl->>Model: Run(features, language, itn_flag)
Model-->>RKImpl: logits (frames × vocab)
RKImpl->>Dec: Decode(logits)
Dec-->>RKImpl: tokens + timestamps
RKImpl->>RKImpl: ConvertSenseVoiceResult + post-process
end
RKImpl-->>Client: Results
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✨ Finishing touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sherpa-onnx/csrc/offline-recognizer-sense-voice-impl.h (1)
25-60: ODR: ConvertSenseVoiceResult is defined in a header — move implementation to a .cc or mark inline.Definition found at sherpa-onnx/csrc/offline-recognizer-sense-voice-impl.h:25; declaration at sherpa-onnx/csrc/rknn/offline-recognizer-sense-voice-rknn-impl.h:23 — will produce multiple-definition linker errors.
Apply one:
Option A (inline in header):
-OfflineRecognitionResult ConvertSenseVoiceResult( +inline OfflineRecognitionResult ConvertSenseVoiceResult( const OfflineCtcDecoderResult &src, const SymbolTable &sym_table, int32_t frame_shift_ms, int32_t subsampling_factor) {Option B (preferred): move the definition to a .cc and keep only a declaration in headers:
-OfflineRecognitionResult ConvertSenseVoiceResult( - const OfflineCtcDecoderResult &src, const SymbolTable &sym_table, - int32_t frame_shift_ms, int32_t subsampling_factor) { - ... -} +OfflineRecognitionResult ConvertSenseVoiceResult( + const OfflineCtcDecoderResult &src, const SymbolTable &sym_table, + int32_t frame_shift_ms, int32_t subsampling_factor);
🧹 Nitpick comments (23)
sherpa-onnx/csrc/rknn/offline-ctc-greedy-search-decoder-rknn.h (2)
8-11: Replace unused include and add the correct one for fixed-width ints.isn’t used in this header; add locally instead of relying on transitive includes.
-#include <vector> +#include <cstdint> #include "sherpa-onnx/csrc/offline-ctc-decoder.h"
19-21: Make return value non-discardable and validate args in the .cc.Add [[nodiscard]] to discourage silent result drops; also add basic range checks for blank_id_/vocab_size in the implementation.
- OfflineCtcDecoderResult Decode(const float *logits, int32_t num_frames, - int32_t vocab_size); + [[nodiscard]] OfflineCtcDecoderResult Decode(const float *logits, + int32_t num_frames, + int32_t vocab_size);Outside this header (in offline-ctc-greedy-search-decoder-rknn.cc), consider:
// At the start of Decode(...) if (logits == nullptr || num_frames <= 0 || vocab_size <= 0 || blank_id_ < 0 || blank_id_ >= vocab_size) { return {}; }scripts/sense-voice/rknn/test_onnx.py (4)
9-10: Remove duplicate import.Only
import onnxruntime as ortis needed.-import onnxruntime import onnxruntime as ort
93-94: Be robust to static/dynamic shapes: prefer metadatanum_input_frameswhen present.RKNN export sets it; falling back to shape keeps ONNX-only flow working.
- self.max_len = self.model.get_inputs()[0].shape[1] + self.max_len = int( + meta.get("num_input_frames", + self.model.get_inputs()[0].shape[1]) + )
151-153: Fix contiguity check for NumPy arrays.Use NumPy’s flags instead of
features.data.contiguous.- assert features.data.contiguous is True + assert features.flags.c_contiguous assert features.dtype == np.float32, features.dtype
223-224: Optional: make blank id configurable.If future models change the blank id, read it from metadata (if added) or a CLI flag; default to 0.
sherpa-onnx/csrc/offline-recognizer-sense-voice-impl.h (2)
34-39: Verify the “4-frame” offset assumptions.Both text token slicing (
i = 4) and timestamp offset (t-4) depend on a magic number. Confirm SenseVoice always emits 3 meta tokens (lang/emotion/event) plus one extra token, and that logits time index 0..3 map to padding, not speech.If correct, consider a named constant to document intent:
- for (int32_t i = 4; i < src.tokens.size(); ++i) { + constexpr int kPrefixLen = 4; + for (int32_t i = kPrefixLen; i < static_cast<int32_t>(src.tokens.size()); ++i) { ... - float time = frame_shift_s * (src.timestamps[i] - 4); + float time = frame_shift_s * (src.timestamps[i] - kPrefixLen);Also applies to: 44-47
49-56: Remove ineffective std::move from const source.
srcisconst;std::move(src.words)performs a copy. Prefer a plain copy for clarity.- r.words = std::move(src.words); + r.words = src.words;sherpa-onnx/csrc/rknn/offline-ctc-greedy-search-decoder-rknn.cc (1)
15-36: Add light input validation and pre-reserve for small wins.Avoid undefined behavior on bad shapes and reduce vector reallocations.
OfflineCtcDecoderResult OfflineCtcGreedySearchDecoderRknn::Decode( const float *logits, int32_t num_frames, int32_t vocab_size) { OfflineCtcDecoderResult ans; + if (!logits || num_frames <= 0 || vocab_size <= 0) { + return ans; + } + ans.tokens.reserve(num_frames); + ans.timestamps.reserve(num_frames); int64_t prev_id = -1;sherpa-onnx/csrc/offline-model-config.cc (1)
61-82: Provider-aware validation is a good addition; consider tightening RKNN rules.
- Nice: gating num_threads check and extension compatibility by provider.
- Optional: validate RK3588-specific num_threads domain here to fail fast on invalid values.
Apply after Line 92:
+ // Validate RK3588 core selection values early + if (provider == "rknn") { + switch (num_threads) { + case 1: // random core + case 0: // core 0 + case -1: // core 1 + case -2: // core 2 + case -3: // cores 0+1 + case -4: // cores 0+1+2 + break; + default: + SHERPA_ONNX_LOGE( + "For RK3588, num_threads must be one of {1,0,-1,-2,-3,-4}. " + "Given %d", + num_threads); + return false; + } + }Also, please update the help text to mention rknn as a valid provider (Lines 39-41):
- po->Register("provider", &provider, - "Specify a provider to use: cpu, cuda, coreml"); + po->Register("provider", &provider, + "Specify a provider to use: cpu, cuda, coreml, rknn");sherpa-onnx/csrc/offline-recognizer-impl.cc (2)
46-62: RKNN routing logic looks correct; small simplification.Change the second branch to plain else; the preceding if already ensures the non-empty case.
- } else if (!config.model_config.sense_voice.model.empty()) { + } else { return std::make_unique<OfflineRecognizerSenseVoiceRknnImpl>(config); }
254-270: Mirror the simplification in the Manager overload.- } else if (!config.model_config.sense_voice.model.empty()) { + } else { return std::make_unique<OfflineRecognizerSenseVoiceRknnImpl>(mgr, config); }sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.h (1)
11-11: Avoid leaking RKNN C API into the public header.The header doesn’t use RKNN types; include rknn_api.h in the .cc instead to reduce compile-time coupling.
-#include "rknn_api.h" // NOLINTscripts/sense-voice/rknn/export-onnx.py (2)
89-92: Load state dict onto CPU explicitly.Prevents device mismatch if checkpoints were saved on GPU.
- state_dict = torch.load("./model.pt") + state_dict = torch.load("./model.pt", map_location="cpu")
110-113: Compute exact LFR output length for fixed-shape export.Use the standard LFR formula to avoid off-by-one when baking the fixed T.
- # num_input_frames is an approximate number - num_input_frames = int(num_frames / lfr_window_shift + 0.5) + # num_input_frames using LFR: floor((N - m)/n) + 1; clamp to >=1 + num_input_frames = max(1, (num_frames - lfr_window_size) // lfr_window_shift + 1)sherpa-onnx/csrc/rknn/offline-recognizer-sense-voice-rknn-impl.h (2)
22-26: Fix misleading reference for ConvertSenseVoiceResult.-// defined in ../online-recognizer-sense-voice-impl.h +// defined in ../offline-recognizer-sense-voice-impl.h
74-78: Consider a batched/multi-stream path.Current loop decodes streams serially; batching could reduce overhead on some targets.
scripts/sense-voice/rknn/export-rknn.py (2)
105-107: Use SystemExit for consistency.- if not Path(filename).is_file(): - exit(f"{filename} does not exist") + if not Path(filename).is_file(): + raise SystemExit(f"{filename} does not exist")
141-149: Validate target platform early.def main(): args = get_parser().parse_args() print(vars(args)) + if args.target_platform not in g_platforms: + raise SystemExit( + f"Unsupported --target-platform '{args.target_platform}'. " + f"Supported values: {', '.join(g_platforms)}" + )sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.cc (1)
59-61: Consider makingRunmethod const-correct.The
Runmethod takes thefeaturesparameter by value and immediately moves it, which is good. However, this method modifies its input internally but is marked as non-const in the implementation while the public interface declares it as const. This inconsistency could lead to confusion.scripts/sense-voice/rknn/torch_model.py (3)
14-19: Add type hint for Optional parameter.The
positionsparameter should useOptional[torch.Tensor]instead of implicit optional.Apply this fix:
+from typing import Optional + def encode( self, - positions: torch.Tensor = None, + positions: Optional[torch.Tensor] = None, depth: int = None, dtype: torch.dtype = torch.float32, ):
89-92: Remove unused LoRA parameters.The LoRA-related parameters (
lora_list,lora_rank,lora_alpha,lora_dropout) are defined but never used in the implementation.Consider removing these unused parameters or implement LoRA functionality if intended:
def __init__( self, n_head, in_feat, n_feat, dropout_rate, kernel_size, sanm_shfit=0, - lora_list=None, - lora_rank=8, - lora_alpha=16, - lora_dropout=0.1, ):
485-487: Remove unused CTC parameters.The parameters
ctc_type,reduce, andignore_nan_gradare defined but never used.Remove the unused parameters:
def __init__( self, odim: int, encoder_output_size: int, dropout_rate: float = 0.0, - ctc_type: str = "builtin", - reduce: bool = True, - ignore_nan_grad: bool = True, extra_linear: bool = True, ):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
scripts/sense-voice/export-onnx.py(1 hunks)scripts/sense-voice/rknn/export-onnx.py(1 hunks)scripts/sense-voice/rknn/export-rknn.py(1 hunks)scripts/sense-voice/rknn/test_onnx.py(1 hunks)scripts/sense-voice/rknn/torch_model.py(1 hunks)sherpa-onnx/csrc/CMakeLists.txt(1 hunks)sherpa-onnx/csrc/offline-model-config.cc(2 hunks)sherpa-onnx/csrc/offline-recognizer-impl.cc(2 hunks)sherpa-onnx/csrc/offline-recognizer-sense-voice-impl.h(4 hunks)sherpa-onnx/csrc/online-recognizer-impl.cc(0 hunks)sherpa-onnx/csrc/rknn/offline-ctc-greedy-search-decoder-rknn.cc(1 hunks)sherpa-onnx/csrc/rknn/offline-ctc-greedy-search-decoder-rknn.h(1 hunks)sherpa-onnx/csrc/rknn/offline-recognizer-sense-voice-rknn-impl.h(1 hunks)sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.cc(1 hunks)sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.h(1 hunks)sherpa-onnx/csrc/rknn/online-zipformer-ctc-model-rknn.cc(0 hunks)sherpa-onnx/csrc/rknn/online-zipformer-transducer-model-rknn.cc(0 hunks)
💤 Files with no reviewable changes (3)
- sherpa-onnx/csrc/rknn/online-zipformer-transducer-model-rknn.cc
- sherpa-onnx/csrc/rknn/online-zipformer-ctc-model-rknn.cc
- sherpa-onnx/csrc/online-recognizer-impl.cc
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-06T04:18:47.981Z
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:18:47.981Z
Learning: In sherpa-onnx Java API, the native library names in Core.java (WIN_NATIVE_LIBRARY_NAME = "sherpa-onnx-jni.dll", UNIX_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.so", MACOS_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.dylib") are copied directly from the compiled binary filenames and should not be changed to match other libraries' naming conventions.
Applied to files:
sherpa-onnx/csrc/offline-model-config.cc
🧬 Code graph analysis (9)
sherpa-onnx/csrc/rknn/offline-ctc-greedy-search-decoder-rknn.h (3)
sherpa-onnx/csrc/rknn/offline-recognizer-sense-voice-rknn-impl.h (1)
sherpa_onnx(20-134)sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.h (1)
sherpa_onnx(15-43)sherpa-onnx/csrc/rknn/offline-ctc-greedy-search-decoder-rknn.cc (2)
Decode(15-36)Decode(15-16)
scripts/sense-voice/export-onnx.py (2)
scripts/sense-voice/rknn/export-onnx.py (1)
main(78-158)scripts/sense-voice/rknn/torch_model.py (1)
SenseVoiceSmall(539-612)
scripts/sense-voice/rknn/export-onnx.py (2)
scripts/sense-voice/rknn/torch_model.py (1)
SenseVoiceSmall(539-612)scripts/sense-voice/export-onnx.py (4)
add_meta_data(21-39)load_cmvn(79-94)generate_tokens(97-103)main(122-197)
sherpa-onnx/csrc/rknn/offline-recognizer-sense-voice-rknn-impl.h (1)
sherpa-onnx/csrc/offline-recognizer-sense-voice-impl.h (5)
sherpa_onnx(23-363)ConvertSenseVoiceResult(25-59)InitFeatConfig(304-311)- `` (103-105)
DecodeStreams(107-302)
sherpa-onnx/csrc/offline-recognizer-impl.cc (1)
sherpa-onnx/csrc/online-recognizer-impl.cc (4)
Create(36-92)Create(36-37)Create(95-152)Create(95-96)
sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.h (2)
sherpa-onnx/csrc/rknn/offline-recognizer-sense-voice-rknn-impl.h (1)
sherpa_onnx(20-134)sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.cc (15)
OfflineSenseVoiceModelRknn(214-214)OfflineSenseVoiceModelRknn(216-218)OfflineSenseVoiceModelRknn(221-223)OfflineSenseVoiceModelRknn(237-238)OfflineSenseVoiceModelRknn(242-243)Run(225-229)Run(225-227)features(59-104)features(59-60)GetModelMetadata(231-234)GetModelMetadata(232-232)Impl(29-34)Impl(36-43)Impl(36-36)Impl(46-53)
scripts/sense-voice/rknn/export-rknn.py (1)
scripts/sense-voice/rknn/export-onnx.py (1)
main(78-158)
sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.cc (1)
sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.h (1)
OfflineSenseVoiceModelRknn(17-41)
scripts/sense-voice/rknn/test_onnx.py (2)
scripts/sense-voice/rknn/export-onnx.py (2)
get_args(15-28)main(78-158)scripts/sense-voice/export-onnx.py (1)
main(122-197)
🪛 Ruff (0.12.2)
scripts/sense-voice/rknn/torch_model.py
17-17: PEP 484 prohibits implicit Optional
Convert to Optional[T]
(RUF013)
59-59: Do not perform function call torch.nn.ReLU in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
89-89: Unused method argument: lora_list
(ARG002)
90-90: Unused method argument: lora_rank
(ARG002)
91-91: Unused method argument: lora_alpha
(ARG002)
92-92: Unused method argument: lora_dropout
(ARG002)
461-461: Loop control variable layer_idx not used within loop body
Rename unused layer_idx to _layer_idx
(B007)
465-465: Loop control variable layer_idx not used within loop body
Rename unused layer_idx to _layer_idx
(B007)
471-471: Loop control variable layer_idx not used within loop body
Rename unused layer_idx to _layer_idx
(B007)
485-485: Unused method argument: ctc_type
(ARG002)
486-486: Unused method argument: reduce
(ARG002)
487-487: Unused method argument: ignore_nan_grad
(ARG002)
592-592: Undefined name language
(F821)
594-594: Undefined name text_norm
(F821)
🪛 Flake8 (7.2.0)
scripts/sense-voice/rknn/torch_model.py
[error] 592-592: undefined name 'language'
(F821)
[error] 594-594: undefined name 'text_norm'
(F821)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: ubuntu-latest Debug shared tts-ON
- GitHub Check: ubuntu-latest Debug static tts-ON
- GitHub Check: ubuntu-latest Release static tts-OFF
- GitHub Check: ubuntu-latest Release static tts-ON
- GitHub Check: ubuntu-latest Release shared tts-OFF
- GitHub Check: ubuntu-24.04 3.9
- GitHub Check: Debug shared tts-OFF
- GitHub Check: Release shared tts-OFF
- GitHub Check: Debug shared tts-ON
- GitHub Check: Release static tts-OFF
- GitHub Check: Release shared tts-ON
- GitHub Check: Debug shared-ON tts-ON
- GitHub Check: Debug shared-OFF tts-ON
- GitHub Check: Debug shared-OFF tts-OFF
- GitHub Check: Release shared-OFF tts-ON
- GitHub Check: Release shared-ON tts-OFF
- GitHub Check: Debug shared-ON tts-OFF
- GitHub Check: rknn shared OFF
- GitHub Check: Release shared-ON tts-ON
- GitHub Check: rknn shared ON
🔇 Additional comments (12)
scripts/sense-voice/export-onnx.py (1)
121-125: Good call: disable grad and set eval mode for export.This prevents training-time behavior and unnecessary autograd overhead during export.
sherpa-onnx/csrc/CMakeLists.txt (1)
176-177: RKNN SenseVoice sources wired correctly.The additions fit the existing RKNN block and link logic.
sherpa-onnx/csrc/offline-recognizer-sense-voice-impl.h (3)
14-14: Good: use SHERPA_ONNX macros header.This enables consistent logging/exit across platforms.
76-76: Good: replace exit() with SHERPA_ONNX_EXIT().Matches project convention and centralizes behavior.
98-98: Good: replace exit() with SHERPA_ONNX_EXIT().Same as above.
sherpa-onnx/csrc/offline-model-config.cc (2)
10-10: Needed include for EndsWith — good.
84-92: Model-provider extension check for RKNN — good.Prevents accidental .onnx with rknn. LGTM.
sherpa-onnx/csrc/offline-recognizer-impl.cc (1)
38-40: Conditional RKNN include — good guard usage.sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.h (1)
33-35: API shape is fine; pass-by-value enables move — LGTM.sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.cc (3)
36-43: LGTM! Correct RKNN lifecycle management.The destructor-constructor pair properly manages the RKNN context lifecycle with appropriate initialization and cleanup.
65-77: Validate prompt array indices against model expectations.The prompt array is hard-coded with indices
{language, 1, 2, text_norm}. The values1and2at indices 1 and 2 appear to be magic numbers. Please verify these are the expected values for the model.Could you confirm that indices 1 and 2 in the prompt array should always be set to the literal values 1 and 2? Consider adding a comment explaining what these values represent.
116-124: Macro SHERPA_ONNX_RKNN_READ_META_DATA_INT should be wrapped in do-while.Good use of the do-while pattern for the macro to ensure it behaves correctly in all contexts.
| vocab_size = sp.vocab_size() | ||
| generate_tokens(sp) | ||
|
|
There was a problem hiding this comment.
Guard against tokenizer/model vocab size mismatch (breaks decoding).
Metadata vocab_size must match the model’s output dim; derive from the model and assert equality with SentencePiece.
- vocab_size = sp.vocab_size()
+ vocab_size_sp = sp.vocab_size()
@@
- model = SenseVoiceSmall(neg_mean=neg_mean, inv_stddev=inv_stddev)
+ model = SenseVoiceSmall(neg_mean=neg_mean, inv_stddev=inv_stddev)
model.load_state_dict(state_dict)
+ assert getattr(model, "vocab_size", None) == vocab_size_sp, (
+ f"Tokenizer size ({vocab_size_sp}) != model.vocab_size ({getattr(model, 'vocab_size', None)})"
+ )
+ vocab_size = model.vocab_size
del state_dictAlso applies to: 98-101, 145-146
| language = 3 | ||
| text_norm = 15 | ||
| prompt = torch.tensor([language, 1, 2, text_norm], dtype=torch.int32) | ||
|
|
There was a problem hiding this comment.
Fix dtype for nn.Embedding indices (must be int64).
torch.nn.Embedding requires Long (int64) indices. Using int32 will raise at export/runtime.
- prompt = torch.tensor([language, 1, 2, text_norm], dtype=torch.int32)
+ prompt = torch.tensor([language, 1, 2, text_norm], dtype=torch.int64)📝 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.
| language = 3 | |
| text_norm = 15 | |
| prompt = torch.tensor([language, 1, 2, text_norm], dtype=torch.int32) | |
| language = 3 | |
| text_norm = 15 | |
| prompt = torch.tensor([language, 1, 2, text_norm], dtype=torch.int64) |
🤖 Prompt for AI Agents
In scripts/sense-voice/rknn/export-onnx.py around lines 116 to 119 the prompt
tensor is created with dtype=torch.int32 but nn.Embedding requires int64
indices; change the tensor to use torch.int64 (or torch.long) so the indices are
64-bit (e.g. create the tensor with dtype=torch.int64 or call .long() on it) to
avoid export/runtime errors.
| std::vector<float> logits = model_->Run(std::move(f), language, text_norm); | ||
| int32_t num_out_frames = logits.size() / meta_data.vocab_size; | ||
|
|
||
| auto result = | ||
| decoder_->Decode(logits.data(), num_out_frames, meta_data.vocab_size); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate logits shape against vocab size before decoding.
Prevents silent corruption if metadata vocab_size mismatches the model.
- std::vector<float> logits = model_->Run(std::move(f), language, text_norm);
- int32_t num_out_frames = logits.size() / meta_data.vocab_size;
+ std::vector<float> logits = model_->Run(std::move(f), language, text_norm);
+ if (meta_data.vocab_size <= 0 || (logits.size() % meta_data.vocab_size) != 0) {
+ SHERPA_ONNX_LOGE("Invalid logits size (%zu) or vocab_size (%d).",
+ logits.size(), meta_data.vocab_size);
+ return;
+ }
+ int32_t num_out_frames = logits.size() / meta_data.vocab_size;📝 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.
| std::vector<float> logits = model_->Run(std::move(f), language, text_norm); | |
| int32_t num_out_frames = logits.size() / meta_data.vocab_size; | |
| auto result = | |
| decoder_->Decode(logits.data(), num_out_frames, meta_data.vocab_size); | |
| std::vector<float> logits = model_->Run(std::move(f), language, text_norm); | |
| if (meta_data.vocab_size <= 0 || (logits.size() % meta_data.vocab_size) != 0) { | |
| SHERPA_ONNX_LOGE("Invalid logits size (%zu) or vocab_size (%d).", | |
| logits.size(), meta_data.vocab_size); | |
| return; | |
| } | |
| int32_t num_out_frames = logits.size() / meta_data.vocab_size; | |
| auto result = | |
| decoder_->Decode(logits.data(), num_out_frames, meta_data.vocab_size); |
🤖 Prompt for AI Agents
In sherpa-onnx/csrc/rknn/offline-recognizer-sense-voice-rknn-impl.h around lines
113 to 118, validate that meta_data.vocab_size is > 0 and that logits.size() is
a non-zero multiple of meta_data.vocab_size before calling decoder_->Decode; if
the check fails, log or report an error (including sizes) and return/abort
gracefully (e.g., return empty result or throw an exception) instead of
proceeding to Decode to prevent silent corruption.
| auto ret = rknn_dup_context(&ctx_, &ctx); | ||
| SHERPA_ONNX_RKNN_CHECK(ret, "Failed to duplicate the ctx"); | ||
|
|
||
| ret = rknn_inputs_set(ctx, inputs.size(), inputs.data()); | ||
| SHERPA_ONNX_RKNN_CHECK(ret, "Failed to set inputs"); | ||
|
|
||
| ret = rknn_run(ctx, nullptr); | ||
| SHERPA_ONNX_RKNN_CHECK(ret, "Failed to run the model"); | ||
|
|
||
| ret = rknn_outputs_get(ctx, outputs.size(), outputs.data(), nullptr); | ||
| SHERPA_ONNX_RKNN_CHECK(ret, "Failed to get model output"); | ||
|
|
||
| rknn_destroy(ctx); |
There was a problem hiding this comment.
Add error handling for context duplication cleanup.
While the function correctly duplicates the context and destroys it at the end, if any error occurs between lines 92-99, the duplicated context won't be cleaned up, leading to a resource leak.
Consider using RAII or adding proper error handling:
rknn_context ctx = 0;
auto ret = rknn_dup_context(&ctx_, &ctx);
SHERPA_ONNX_RKNN_CHECK(ret, "Failed to duplicate the ctx");
+// Ensure cleanup on any error
+struct ContextGuard {
+ rknn_context& ctx;
+ ~ContextGuard() { if (ctx) rknn_destroy(ctx); }
+} guard{ctx};
+
ret = rknn_inputs_set(ctx, inputs.size(), inputs.data());
SHERPA_ONNX_RKNN_CHECK(ret, "Failed to set inputs");
ret = rknn_run(ctx, nullptr);
SHERPA_ONNX_RKNN_CHECK(ret, "Failed to run the model");
ret = rknn_outputs_get(ctx, outputs.size(), outputs.data(), nullptr);
SHERPA_ONNX_RKNN_CHECK(ret, "Failed to get model output");
-rknn_destroy(ctx);
+guard.ctx = 0; // Disable cleanup after success
+rknn_destroy(ctx);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.cc around lines 89-101,
the duplicated RKNN context (ctx) is only destroyed at the end, so if any call
between rknn_dup_context and rknn_destroy fails the duplicated context is
leaked; update the function to ensure ctx is always cleaned up on error by
either wrapping ctx in an RAII wrapper (e.g., a unique_ptr with a custom deleter
that calls rknn_destroy) or by adding immediate cleanup before each early
return/error check (call rknn_destroy(ctx) when a subsequent rknn_* call fails
and before returning the error). Ensure the duplicated ctx variable is only
destroyed once and that error paths propagate the original error code after
cleanup.
| if (out_num_frames > num_input_frames_) { | ||
| SHERPA_ONNX_LOGE( | ||
| "Number of input frames %d is too large. Truncate it to %d frames.", | ||
| out_num_frames, num_input_frames_); | ||
|
|
||
| SHERPA_ONNX_LOGE( | ||
| "Recognition result may be truncated/incomplete. Please select a " | ||
| "model accepting longer audios."); | ||
|
|
||
| out_num_frames = num_input_frames_; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider making frame truncation behavior configurable.
When the number of input frames exceeds the model's capacity, the code logs a warning and truncates the input. This silent truncation could lead to incomplete recognition results. Consider making this behavior configurable (e.g., allow failing instead of truncating).
Would you like me to propose an implementation that adds a configuration option to control the truncation behavior (e.g., truncate vs. fail)?
| template <typename Manager> | ||
| OfflineSenseVoiceModelRknn(Manager *mgr, const OfflineModelConfig &config); | ||
|
|
There was a problem hiding this comment.
Template ctor definition visibility — likely link error.
You declare a templated Manager constructor in the header but define it in the .cc. Without explicit instantiations, call sites in other TUs will fail to link. Keep the definition in the .cc, but add explicit instantiations there for the supported managers.
Add to sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.cc (guarded):
// At top (for type visibility)
#if __ANDROID_API__ >= 9
#include "android/asset_manager.h"
#endif
#if __OHOS__
#include "rawfile/raw_file_manager.h"
#endif
// Near the end of the .cc
#if __ANDROID_API__ >= 9
template OfflineSenseVoiceModelRknn::OfflineSenseVoiceModelRknn(
AAssetManager* mgr, const OfflineModelConfig& config);
#endif
#if __OHOS__
template OfflineSenseVoiceModelRknn::OfflineSenseVoiceModelRknn(
NativeResourceManager* mgr, const OfflineModelConfig& config);
#endif🤖 Prompt for AI Agents
In sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.h around lines 23-25 you
declared a templated constructor but defined it in the .cc which will cause link
errors unless explicitly instantiated; fix by adding guarded explicit template
instantiations in sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.cc for
each supported Manager type and include the necessary platform headers at the
top of that .cc (e.g., AAssetManager header for Android and
NativeResourceManager/raw file header for OHOS) so the instantiations compile.
There was a problem hiding this comment.
Pull Request Overview
This PR adds RK NPU support for SenseVoice non-streaming ASR models by implementing a new RKNN backend with greedy CTC decoding.
- Adds offline RKNN implementation for SenseVoice models including model loading, feature processing, and CTC decoding
- Introduces export tools to convert SenseVoice models from PyTorch to ONNX and then to RKNN format
- Implements provider-aware validation to prevent mismatched model and provider usage
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.h | Header for RKNN SenseVoice model interface |
| sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.cc | Implementation of RKNN SenseVoice model with LFR processing |
| sherpa-onnx/csrc/rknn/offline-recognizer-sense-voice-rknn-impl.h | Header for RKNN SenseVoice recognizer implementation |
| sherpa-onnx/csrc/rknn/offline-ctc-greedy-search-decoder-rknn.h | Header for RKNN CTC greedy decoder |
| sherpa-onnx/csrc/rknn/offline-ctc-greedy-search-decoder-rknn.cc | Implementation of CTC greedy search decoder for RKNN |
| sherpa-onnx/csrc/offline-recognizer-impl.cc | Adds RKNN backend routing for offline recognition |
| sherpa-onnx/csrc/offline-model-config.cc | Adds provider validation and RK3588 NPU core selection |
| sherpa-onnx/csrc/CMakeLists.txt | Includes new RKNN source files in build |
| scripts/sense-voice/rknn/torch_model.py | PyTorch implementation of SenseVoice model for export |
| scripts/sense-voice/rknn/test_onnx.py | ONNX test runner for SenseVoice transcription |
| scripts/sense-voice/rknn/export-rknn.py | CLI to convert ONNX models to RKNN format |
| scripts/sense-voice/rknn/export-onnx.py | CLI to export SenseVoice from PyTorch to ONNX |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| class SinusoidalPositionEncoder(nn.Module): | ||
| """ """ | ||
|
|
||
| def __int__(self, d_model=80, dropout_rate=0.1): |
There was a problem hiding this comment.
Method name should be __init__ instead of __int__. This is a typo that would prevent proper initialization of the SinusoidalPositionEncoder class.
| def __int__(self, d_model=80, dropout_rate=0.1): | |
| def __init__(self, d_model=80, dropout_rate=0.1): |
| def export_rknn(rknn, filename): | ||
| ret = rknn.export_rknn(filename) | ||
| if ret != 0: | ||
| exit("Export rknn model to {filename} failed!") |
There was a problem hiding this comment.
The f-string formatting is missing the 'f' prefix. The variable filename will not be interpolated into the string. Should be f\"Export rknn model to {filename} failed!\"
| exit("Export rknn model to {filename} failed!") | |
| exit(f"Export rknn model to {filename} failed!") |
|
|
||
| ret = rknn.build(do_quantization=False) | ||
| if ret != 0: | ||
| exit("Build model {filename} failed!") |
There was a problem hiding this comment.
The f-string formatting is missing the 'f' prefix. The variable filename will not be interpolated into the string. Should be f\"Build model {filename} failed!\"
| exit("Build model {filename} failed!") | |
| exit(f"Build model {filename} failed!") |
|
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sherpa-onnx/csrc/offline-model-config.cc (2)
39-41: Update provider help text to mention rknn.CLI help omits the newly supported provider and can confuse users.
Apply:
- po->Register("provider", &provider, - "Specify a provider to use: cpu, cuda, coreml"); + po->Register("provider", &provider, + "Specify a provider to use: cpu, cuda, coreml, rknn");
33-35: Document rknn-specific num_threads semantics in help.Expose the RK3588 mapping so users don’t guess.
Apply:
- po->Register("num-threads", &num_threads, - "Number of threads to run the neural network"); + po->Register("num-threads", &num_threads, + "Number of threads to run the neural network. " + "For provider=rknn on RK3588, values map to NPU cores: " + "1=random core; 0=core0; -1=core1; -2=core2; " + "-3=cores0+1; -4=cores0+1+2");
♻️ Duplicate comments (6)
sherpa-onnx/csrc/offline-model-config.cc (1)
71-74: Duplicate num_threads check removal looks resolved.The extra non-rknn num_threads < 1 validation mentioned earlier is no longer duplicated.
scripts/sense-voice/rknn/export-rknn.py (4)
92-95: Use SystemExit and proper f-string; avoidexit().Same issue noted previously;
exit()with a string is for interactive shells and masks non-zero codes.Apply:
def export_rknn(rknn, filename): ret = rknn.export_rknn(filename) if ret != 0: - exit(f"Export rknn model to {filename} failed!") + raise SystemExit(f"Export rknn model to {filename} failed!")
112-114: Fix build-failure exit; preferSystemExitand f-string.Same as prior comment; message formatting and termination semantics.
- if ret != 0: - exit(f"Build model {filename} failed!") + if ret != 0: + raise SystemExit(f"Build model {filename} failed!")
108-111: Also fix load failure termination.Align with above; avoid
exit().- if ret != 0: - exit(f"Load model {filename} failed!") + if ret != 0: + raise SystemExit(f"Load model {filename} failed!")
105-106: Raise on missing input file; avoidexit().
scriptsshould exit with non-zero and a clear message.- if not Path(filename).is_file(): - exit(f"{filename} does not exist") + if not Path(filename).is_file(): + raise SystemExit(f"{filename} does not exist")scripts/sense-voice/rknn/torch_model.py (1)
11-13: Initializenn.Moduleand wire dropout; currentpassbreaks.eval()recursion.Without
super().__init__(), calling.eval()on the parent will error.- def __init__(self, d_model=80, dropout_rate=0.1): - pass + def __init__(self, d_model=80, dropout_rate=0.1): + super().__init__() + self.dropout = nn.Dropout(dropout_rate) + self.d_model = d_model
🧹 Nitpick comments (10)
sherpa-onnx/csrc/offline-model-config.cc (2)
75-81: Handle file extensions case-insensitively (optional).Users may supply “.RKNN”/“.ONNX”. Consider accepting both cases.
Apply:
- if (!sense_voice.model.empty() && (EndsWith(sense_voice.model, ".rknn"))) { + if (!sense_voice.model.empty() && + (EndsWith(sense_voice.model, ".rknn") || + EndsWith(sense_voice.model, ".RKNN"))) {and
- if (!sense_voice.model.empty() && (EndsWith(sense_voice.model, ".onnx"))) { + if (!sense_voice.model.empty() && + (EndsWith(sense_voice.model, ".onnx") || + EndsWith(sense_voice.model, ".ONNX"))) {If there is an EndsWithIgnoreCase helper available, prefer that.
84-92: Add tests for new validation paths.Please add unit tests covering:
- provider!=rknn with .rknn model -> fail
- provider=rknn with .onnx model -> fail
- provider=rknn with invalid num_threads -> fail
- provider=rknn without SenseVoice or with mixed model configs -> fail
I can draft gtest cases targeting OfflineModelConfig::Validate(). Want me to open a follow-up PR?
scripts/sense-voice/rknn/export-rknn.py (3)
30-36: Constrain--target-platformvia argparsechoices.Prevents typos and gives immediate CLI feedback.
parser.add_argument( "--target-platform", type=str, - required=True, - help=f"Supported values are: {','.join(g_platforms)}", + required=True, + choices=g_platforms, + help="Target RK platform", )
67-75: Use logging instead of prints for model IO introspection.These are noisy; route to INFO to allow
-O/--quietcontrol.- for i in m.get_inputs(): - print(i) + for i in m.get_inputs(): + logging.info("input: %s", i) print("-----") - for i in m.get_outputs(): - print(i) - print() + for i in m.get_outputs(): + logging.info("output: %s", i)
76-86: Make metadata string deterministic and guard length using<= 1024.Dict iteration order may vary; also allow 1024 exactly (common cap).
- meta = m.get_modelmeta().custom_metadata_map - s = "" - sep = "" - for key, value in meta.items(): + meta = m.get_modelmeta().custom_metadata_map or {} + parts = [] + for key in sorted(meta.keys()): + value = meta[key] if key in ("neg_mean", "inv_stddev"): continue - s = s + sep + f"{key}={value}" - sep = ";" - assert len(s) < 1024, len(s) + parts.append(f"{key}={value}") + s = ";".join(parts) + assert len(s) <= 1024, f"custom_string too long: {len(s)}" - print("len(s)", len(s), s) + logging.info("custom_string length=%d", len(s))Please confirm RKNN’s exact limit is 1024 bytes (not 1023). If different, adjust accordingly.
scripts/sense-voice/rknn/torch_model.py (5)
41-47: Apply dropout to positional encoding output.Standard PE uses dropout after addition.
- def forward(self, x): + def forward(self, x): batch_size, timesteps, input_dim = x.size() positions = torch.arange(1, timesteps + 1, device=x.device)[None, :] position_encoding = self.encode(positions, input_dim, x.dtype).to(x.device) - - return x + position_encoding + return self.dropout(x + position_encoding)
14-19: UseOptional[Tensor]in type hints.Silences RUF013 and clarifies nullable.
- def encode( - self, - positions: torch.Tensor = None, + def encode( + self, + positions: "Optional[torch.Tensor]" = None,Add at file top:
+from typing import Optional
463-476: Rename unused loop variable or dropenumerate.Silences lints and clarifies intent.
- for layer_idx, encoder_layer in enumerate(self.encoders0): + for _, encoder_layer in enumerate(self.encoders0): encoder_outs = encoder_layer(xs_pad, masks) xs_pad, masks = encoder_outs[0], encoder_outs[1] - for layer_idx, encoder_layer in enumerate(self.encoders): + for _, encoder_layer in enumerate(self.encoders): encoder_outs = encoder_layer(xs_pad, masks) xs_pad, masks = encoder_outs[0], encoder_outs[1] - for layer_idx, encoder_layer in enumerate(self.tp_encoders): + for _, encoder_layer in enumerate(self.tp_encoders): encoder_outs = encoder_layer(xs_pad, masks) xs_pad, masks = encoder_outs[0], encoder_outs[1]
83-95: Drop or underscore unused LoRA args.They’re unused and trigger lints.
- lora_list=None, - lora_rank=8, - lora_alpha=16, - lora_dropout=0.1, + _lora_list=None, + _lora_rank=8, + _lora_alpha=16, + _lora_dropout=0.1,
482-491: Remove unused ctor params or document them.
ctc_type,reduce,ignore_nan_gradare accepted but unused; either wire them or drop to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.gitignore(1 hunks)scripts/sense-voice/rknn/export-onnx.py(1 hunks)scripts/sense-voice/rknn/export-rknn.py(1 hunks)scripts/sense-voice/rknn/test_onnx.py(1 hunks)scripts/sense-voice/rknn/torch_model.py(1 hunks)sherpa-onnx/csrc/offline-model-config.cc(2 hunks)sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.cc(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (3)
- scripts/sense-voice/rknn/export-onnx.py
- sherpa-onnx/csrc/rknn/offline-sense-voice-model-rknn.cc
- scripts/sense-voice/rknn/test_onnx.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-06T04:18:47.981Z
Learnt from: litongjava
PR: k2-fsa/sherpa-onnx#2440
File: sherpa-onnx/java-api/src/main/java/com/k2fsa/sherpa/onnx/core/Core.java:4-6
Timestamp: 2025-08-06T04:18:47.981Z
Learning: In sherpa-onnx Java API, the native library names in Core.java (WIN_NATIVE_LIBRARY_NAME = "sherpa-onnx-jni.dll", UNIX_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.so", MACOS_NATIVE_LIBRARY_NAME = "libsherpa-onnx-jni.dylib") are copied directly from the compiled binary filenames and should not be changed to match other libraries' naming conventions.
Applied to files:
sherpa-onnx/csrc/offline-model-config.cc
🧬 Code graph analysis (1)
scripts/sense-voice/rknn/export-rknn.py (1)
scripts/sense-voice/rknn/export-onnx.py (1)
main(78-159)
🪛 Ruff (0.12.2)
scripts/sense-voice/rknn/torch_model.py
17-17: PEP 484 prohibits implicit Optional
Convert to Optional[T]
(RUF013)
91-91: Unused method argument: lora_list
(ARG002)
92-92: Unused method argument: lora_rank
(ARG002)
93-93: Unused method argument: lora_alpha
(ARG002)
94-94: Unused method argument: lora_dropout
(ARG002)
463-463: Loop control variable layer_idx not used within loop body
Rename unused layer_idx to _layer_idx
(B007)
467-467: Loop control variable layer_idx not used within loop body
Rename unused layer_idx to _layer_idx
(B007)
473-473: Loop control variable layer_idx not used within loop body
Rename unused layer_idx to _layer_idx
(B007)
487-487: Unused method argument: ctc_type
(ARG002)
488-488: Unused method argument: reduce
(ARG002)
489-489: Unused method argument: ignore_nan_grad
(ARG002)
🔇 Additional comments (2)
sherpa-onnx/csrc/offline-model-config.cc (1)
10-10: Include for EndsWith is correct.Including text-utils.h to use EndsWith is appropriate here.
scripts/sense-voice/rknn/torch_model.py (1)
250-331: Early return whenin_size != sizeskips FFN/norm; confirm intent.This path (hit by the first layer: 560→512) returns after attention+dropout, bypassing the FFN and second norm. If intentional as a pre-proj, OK; otherwise it likely hurts quality.
Would you like a patch to factor a dedicated projection layer for the first block and keep the full block structure?
| def main(): | ||
| args = get_parser().parse_args() | ||
| print(vars(args)) | ||
|
|
||
| model = RKNNModel( | ||
| model=args.in_model, | ||
| target_platform=args.target_platform, | ||
| ) | ||
|
|
||
| model.export_rknn( | ||
| model=args.out_model, | ||
| ) | ||
|
|
||
| model.release() | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Always release RKNN; ensure output dir exists.
Prevent resource leaks and path errors.
def main():
args = get_parser().parse_args()
- print(vars(args))
+ logging.info("%s", vars(args))
+ # Ensure parent dir exists
+ out_path = Path(args.out_model)
+ out_path.parent.mkdir(parents=True, exist_ok=True)
- model = RKNNModel(
- model=args.in_model,
- target_platform=args.target_platform,
- )
-
- model.export_rknn(
- model=args.out_model,
- )
-
- model.release()
+ model = None
+ try:
+ model = RKNNModel(
+ model=args.in_model,
+ target_platform=args.target_platform,
+ )
+ model.export_rknn(model=str(out_path))
+ finally:
+ if model is not None:
+ model.release()📝 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.
| def main(): | |
| args = get_parser().parse_args() | |
| print(vars(args)) | |
| model = RKNNModel( | |
| model=args.in_model, | |
| target_platform=args.target_platform, | |
| ) | |
| model.export_rknn( | |
| model=args.out_model, | |
| ) | |
| model.release() | |
| if __name__ == "__main__": | |
| main() | |
| def main(): | |
| args = get_parser().parse_args() | |
| logging.info("%s", vars(args)) | |
| # Ensure parent dir exists | |
| out_path = Path(args.out_model) | |
| out_path.parent.mkdir(parents=True, exist_ok=True) | |
| model = None | |
| try: | |
| model = RKNNModel( | |
| model=args.in_model, | |
| target_platform=args.target_platform, | |
| ) | |
| model.export_rknn(model=str(out_path)) | |
| finally: | |
| if model is not None: | |
| model.release() | |
| if __name__ == "__main__": | |
| main() |
🤖 Prompt for AI Agents
In scripts/sense-voice/rknn/export-rknn.py around lines 141 to 158, the export
logic may leak the RKNN resource and fail if the output directory doesn't exist;
wrap the export and any subsequent work in a try/finally so model.release() is
always called (even on exceptions) and before calling model.export_rknn ensure
the directory for args.out_model exists by creating
os.path.dirname(args.out_model) with os.makedirs(..., exist_ok=True) (handle the
case of no dirname), then call export and finally release the model in the
finally block.
| self.neg_mean = neg_mean.unsqueeze(0).unsqueeze(0) | ||
| self.inv_stddev = inv_stddev.unsqueeze(0).unsqueeze(0) | ||
|
|
There was a problem hiding this comment.
Fix CMVN broadcasting for LFR (560 = 80*7).
As written, (B,T,560) ⊕ (1,1,80) will crash. Repeat stats to 560.
- self.neg_mean = neg_mean.unsqueeze(0).unsqueeze(0)
- self.inv_stddev = inv_stddev.unsqueeze(0).unsqueeze(0)
+ base = neg_mean.reshape(-1)
+ stdv = inv_stddev.reshape(-1)
+ assert base.numel() > 0 and base.numel() == stdv.numel()
+ repeat = (self.input_size // base.numel())
+ assert self.input_size % base.numel() == 0, "input_size must be a multiple of CMVN dim"
+ self.neg_mean = base.repeat(repeat).view(1, 1, self.input_size)
+ self.inv_stddev = stdv.repeat(repeat).view(1, 1, self.input_size)📝 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.
| self.neg_mean = neg_mean.unsqueeze(0).unsqueeze(0) | |
| self.inv_stddev = inv_stddev.unsqueeze(0).unsqueeze(0) | |
| base = neg_mean.reshape(-1) | |
| stdv = inv_stddev.reshape(-1) | |
| assert base.numel() > 0 and base.numel() == stdv.numel() | |
| repeat = (self.input_size // base.numel()) | |
| assert self.input_size % base.numel() == 0, "input_size must be a multiple of CMVN dim" | |
| self.neg_mean = base.repeat(repeat).view(1, 1, self.input_size) | |
| self.inv_stddev = stdv.repeat(repeat).view(1, 1, self.input_size) |
🤖 Prompt for AI Agents
In scripts/sense-voice/rknn/torch_model.py around lines 552 to 554, the CMVN
stats are shaped as (1,1,80) which cannot broadcast with inputs of shape
(B,T,560); modify the code so neg_mean and inv_stddev are expanded/repeated
along the feature dimension to match 560 (e.g., repeat each of the 80 stats 7
times to produce shape (1,1,560) or explicitly expand to (1,1,80*7)) before
applying to the (B,T,560) tensor so broadcasting succeeds.
| def forward(self, x, prompt): | ||
| input_query = self.embed(prompt).unsqueeze(0) | ||
|
|
||
| # for export, we always assume x and self.neg_mean are on CPU | ||
| x = (x + self.neg_mean) * self.inv_stddev | ||
| x = torch.cat((input_query, x), dim=1) | ||
|
|
||
| encoder_out = self.encoder(x) |
There was a problem hiding this comment.
Cast prompt to long for Embedding indices.
nn.Embedding requires int64 indices; int32 will fail.
- def forward(self, x, prompt):
- input_query = self.embed(prompt).unsqueeze(0)
+ def forward(self, x, prompt):
+ input_query = self.embed(prompt.to(torch.long)).unsqueeze(0)
# for export, we always assume x and self.neg_mean are on CPU
x = (x + self.neg_mean) * self.inv_stddev
x = torch.cat((input_query, x), dim=1)📝 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.
| def forward(self, x, prompt): | |
| input_query = self.embed(prompt).unsqueeze(0) | |
| # for export, we always assume x and self.neg_mean are on CPU | |
| x = (x + self.neg_mean) * self.inv_stddev | |
| x = torch.cat((input_query, x), dim=1) | |
| encoder_out = self.encoder(x) | |
| def forward(self, x, prompt): | |
| input_query = self.embed(prompt.to(torch.long)).unsqueeze(0) | |
| # for export, we always assume x and self.neg_mean are on CPU | |
| x = (x + self.neg_mean) * self.inv_stddev | |
| x = torch.cat((input_query, x), dim=1) | |
| encoder_out = self.encoder(x) |
🤖 Prompt for AI Agents
In scripts/sense-voice/rknn/torch_model.py around lines 592 to 599, the code
passes prompt directly to nn.Embedding which requires int64 indices; cast prompt
to torch.long (int64) before calling self.embed to avoid type errors, e.g.,
ensure prompt = prompt.long() (and keep device consistent if needed) then call
self.embed(prompt). This ensures embedding receives proper index dtype and
prevents runtime failures.
| // For RK NPU, we reinterpret num_threads: | ||
| // | ||
| // For RK3588 only | ||
| // num_threads == 1 -> Select a core randomly | ||
| // num_threads == 0 -> Use NPU core 0 | ||
| // num_threads == -1 -> Use NPU core 1 | ||
| // num_threads == -2 -> Use NPU core 2 | ||
| // num_threads == -3 -> Use NPU core 0 and core 1 | ||
| // num_threads == -4 -> Use NPU core 0, core 1, and core 2 | ||
| if (provider != "rknn") { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate rknn-only num_threads domain.
For provider=rknn we accept special values; reject anything else early to avoid undefined routing.
Apply inside the rknn branch (see next comment’s diff) or equivalently right after entering it:
+ // Validate allowed values for RK3588 rknn backend
+ if (!(num_threads == 1 || num_threads == 0 || num_threads == -1 ||
+ num_threads == -2 || num_threads == -3 || num_threads == -4)) {
+ SHERPA_ONNX_LOGE(
+ "For provider=rknn on RK3588, num_threads must be one of "
+ "{1, 0, -1, -2, -3, -4}. Given %d",
+ num_threads);
+ return false;
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In sherpa-onnx/csrc/offline-model-config.cc around lines 61 to 70, when provider
== "rknn" you must validate that num_threads is one of the supported special
values for RK3588 (1, 0, -1, -2, -3, -4); if it is not, return an error (or
throw) immediately instead of letting it fall through to undefined routing. Add
this check inside the rknn branch and produce a clear error message indicating
the invalid num_threads value and that only the listed values are allowed.
| if (provider != "rknn") { | ||
| if (num_threads < 1) { | ||
| SHERPA_ONNX_LOGE("num_threads should be > 0. Given %d", num_threads); | ||
| return false; | ||
| } | ||
| if (!sense_voice.model.empty() && (EndsWith(sense_voice.model, ".rknn"))) { | ||
| SHERPA_ONNX_LOGE( | ||
| "--provider is %s, which is not rknn, but you pass a rknn model " | ||
| "filename. model: '%s'", | ||
| provider.c_str(), sense_voice.model.c_str()); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| if (provider == "rknn") { | ||
| if (!sense_voice.model.empty() && (EndsWith(sense_voice.model, ".onnx"))) { | ||
| SHERPA_ONNX_LOGE( | ||
| "--provider is rknn, but you pass an onnx model " | ||
| "filename. model: '%s'", | ||
| sense_voice.model.c_str()); | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Tighten rknn branch: require SenseVoice and forbid mixing other model types.
With provider=rknn, only SenseVoice offline models are supported. Fail fast if SenseVoice isn’t set or if other model configs are also provided. Also fold the second provider check into an else-if for clarity.
Apply:
- if (provider == "rknn") {
+ } else if (provider == "rknn") {
+ // Validate allowed values for RK3588 rknn backend
+ if (!(num_threads == 1 || num_threads == 0 || num_threads == -1 ||
+ num_threads == -2 || num_threads == -3 || num_threads == -4)) {
+ SHERPA_ONNX_LOGE(
+ "For provider=rknn on RK3588, num_threads must be one of "
+ "{1, 0, -1, -2, -3, -4}. Given %d",
+ num_threads);
+ return false;
+ }
+
+ // SenseVoice is the only supported model family for rknn
+ if (sense_voice.model.empty()) {
+ SHERPA_ONNX_LOGE(
+ "--provider is rknn, but sense_voice.model is empty. "
+ "Please provide a SenseVoice RKNN model.");
+ return false;
+ }
+ if (!paraformer.model.empty() || !nemo_ctc.model.empty() ||
+ !whisper.encoder.empty() || !fire_red_asr.encoder.empty() ||
+ !tdnn.model.empty() || !zipformer_ctc.model.empty() ||
+ !wenet_ctc.model.empty() || !moonshine.preprocessor.empty() ||
+ !dolphin.model.empty() || !canary.encoder.empty() ||
+ !transducer.encoder_filename.empty() || !telespeech_ctc.empty()) {
+ SHERPA_ONNX_LOGE(
+ "With provider=rknn, only SenseVoice offline models are supported. "
+ "Unset other model configs.");
+ return false;
+ }
if (!sense_voice.model.empty() && (EndsWith(sense_voice.model, ".onnx"))) {
SHERPA_ONNX_LOGE(
"--provider is rknn, but you pass an onnx model "
"filename. model: '%s'",
sense_voice.model.c_str());
return false;
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In sherpa-onnx/csrc/offline-model-config.cc around lines 70-92, tighten the
provider logic by folding the second provider check into an else-if and making
the rknn branch strict: when provider == "rknn" fail fast if sense_voice.model
is empty (log an error and return false) and also fail if any other model
configuration is provided (e.g., any other model path strings are non-empty or
if sense_voice.model has an .onnx suffix), and when provider != "rknn" keep
rejecting .rknn filenames as before; update error messages to clearly state
which condition failed.
TODOs
Summary by CodeRabbit
New Features
Improvements
Build
Chores