Conversation
The offline CTC greedy decoder produced argmax token IDs only and discarded the log-probability values, so the existing ys_log_probs field on OfflineRecognitionResult (and the matching C struct field) was always NULL for CTC models — even though it was already populated for the offline transducer path by k2-fsa#2843. Changes: - OfflineCtcDecoderResult: add token_log_probs (per-emitted-token max log-prob) and vocab_log_probs (full vocab distribution per emitted token). - OfflineCtcGreedySearchDecoder::Decode: populate both vectors alongside the existing tokens/timestamps. Blank and repeated frames are excluded (same predicate as the existing token emission). - OfflineRecognitionResult: add vocab_log_probs for parity with the C API addition. - offline-recognizer-ctc-impl Convert(): copy the new fields from the decoder result into the outer recognition result. - C API: add vocab_log_probs (flattened row-major float matrix) and vocab_size to SherpaOnnxOfflineRecognizerResult, populate them in SherpaOnnxGetOfflineStreamResult, free them in SherpaOnnxDestroyOfflineRecognizerResult. The existing ys_log_probs C-side wiring (added in k2-fsa#2843) is now also populated for offline CTC because the recognizer copies src.token_log_probs into r.ys_log_probs. Motivation: enables entropy-based confidence estimation (e.g. Tsallis entropy over the vocab distribution) for offline CTC models without requiring a second inference pass via raw onnxruntime.
dosubot
Bot
added
the
size:M
📝 WalkthroughWalkthroughThis PR extends the offline ASR result pipeline to capture and expose full vocabulary log-probability distributions for each emitted token. The feature propagates from greedy CTC decoder through C++ recognition result to the C API, with proper memory management for C callers. ChangesVocabulary log-probability capture and propagation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@sherpa-onnx/c-api/c-api.cc`:
- Around line 868-879: The code flattens result.vocab_log_probs into flat
assuming every row matches vocab_size and multiplies r->count*vocab_size without
overflow checks; to fix, before allocating and copying, validate that
result.vocab_log_probs.size() == static_cast<size_t>(r->count), compute
vocab_size from row 0, then loop over i and verify
result.vocab_log_probs[i].size() == static_cast<size_t>(vocab_size) (handle
mismatch by returning an error/cleanup), and check/mask the multiplication
r->count * vocab_size for overflow (or use size_t and bounds-check against
numeric_limits<size_t>::max()) before new float[...] to avoid buffer overruns
when filling flat and ensure safe memory allocation for flat and correct
assignment to r->vocab_size.
In `@sherpa-onnx/csrc/offline-recognizer-ctc-impl.h`:
- Around line 80-88: Convert() is copying src.token_log_probs and
src.vocab_log_probs directly into r.ys_log_probs and r.vocab_log_probs using
src.tokens indices, which breaks alignment when Convert() filters tokens before
assigning r.tokens; update the logic in Convert() to build r.ys_log_probs and
r.vocab_log_probs by iterating the same filtered token indices used to produce
r.tokens and push the corresponding entries from src.token_log_probs and
src.vocab_log_probs (only when those source arrays are non-empty and have
sufficient size), so the resulting r.ys_log_probs and r.vocab_log_probs match
r.tokens in length and order.
🪄 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: 0d40099f-4349-47b3-8376-cc80ee1087b0
📒 Files selected for processing (6)
sherpa-onnx/c-api/c-api.ccsherpa-onnx/c-api/c-api.hsherpa-onnx/csrc/offline-ctc-decoder.hsherpa-onnx/csrc/offline-ctc-greedy-search-decoder.ccsherpa-onnx/csrc/offline-recognizer-ctc-impl.hsherpa-onnx/csrc/offline-stream.h
| // Copy vocab_log_probs (flattened row-major: count * vocab_size) | ||
| if (!result.vocab_log_probs.empty() && | ||
| static_cast<int32_t>(result.vocab_log_probs.size()) == r->count && | ||
| !result.vocab_log_probs[0].empty()) { | ||
| int32_t vocab_size = | ||
| static_cast<int32_t>(result.vocab_log_probs[0].size()); | ||
| r->vocab_size = vocab_size; | ||
| float *flat = new float[r->count * vocab_size]; | ||
| for (int32_t i = 0; i < r->count; ++i) { | ||
| std::copy(result.vocab_log_probs[i].begin(), | ||
| result.vocab_log_probs[i].end(), flat + i * vocab_size); | ||
| } |
There was a problem hiding this comment.
Validate every row width before flattening to avoid buffer corruption.
At Line 877, the copy assumes every result.vocab_log_probs[i] has vocab_size elements (from Line 873), but only row 0 is validated. A wider row can overrun flat; a shorter row yields partial/uninitialized output. Also, Line 875 multiplies int32_t dimensions without overflow guarding.
Suggested fix
- if (!result.vocab_log_probs.empty() &&
- static_cast<int32_t>(result.vocab_log_probs.size()) == r->count &&
- !result.vocab_log_probs[0].empty()) {
- int32_t vocab_size =
- static_cast<int32_t>(result.vocab_log_probs[0].size());
- r->vocab_size = vocab_size;
- float *flat = new float[r->count * vocab_size];
- for (int32_t i = 0; i < r->count; ++i) {
- std::copy(result.vocab_log_probs[i].begin(),
- result.vocab_log_probs[i].end(), flat + i * vocab_size);
- }
- r->vocab_log_probs = flat;
+ if (!result.vocab_log_probs.empty() &&
+ static_cast<int32_t>(result.vocab_log_probs.size()) == r->count &&
+ !result.vocab_log_probs[0].empty()) {
+ const size_t rows = result.vocab_log_probs.size();
+ const size_t vocab_size = result.vocab_log_probs[0].size();
+ const bool uniform = std::all_of(
+ result.vocab_log_probs.begin(), result.vocab_log_probs.end(),
+ [vocab_size](const std::vector<float> &row) {
+ return row.size() == vocab_size;
+ });
+
+ if (uniform && rows > 0 &&
+ vocab_size <= std::numeric_limits<size_t>::max() / rows) {
+ r->vocab_size = static_cast<int32_t>(vocab_size);
+ float *flat = new float[rows * vocab_size];
+ for (size_t i = 0; i < rows; ++i) {
+ std::copy(result.vocab_log_probs[i].begin(),
+ result.vocab_log_probs[i].end(), flat + i * vocab_size);
+ }
+ r->vocab_log_probs = flat;
+ } else {
+ r->vocab_log_probs = nullptr;
+ r->vocab_size = 0;
+ }
} else {
r->vocab_log_probs = nullptr;
r->vocab_size = 0;
}🤖 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/c-api.cc` around lines 868 - 879, The code flattens
result.vocab_log_probs into flat assuming every row matches vocab_size and
multiplies r->count*vocab_size without overflow checks; to fix, before
allocating and copying, validate that result.vocab_log_probs.size() ==
static_cast<size_t>(r->count), compute vocab_size from row 0, then loop over i
and verify result.vocab_log_probs[i].size() == static_cast<size_t>(vocab_size)
(handle mismatch by returning an error/cleanup), and check/mask the
multiplication r->count * vocab_size for overflow (or use size_t and
bounds-check against numeric_limits<size_t>::max()) before new float[...] to
avoid buffer overruns when filling flat and ensure safe memory allocation for
flat and correct assignment to r->vocab_size.
| if (!src.token_log_probs.empty() && | ||
| src.token_log_probs.size() == src.tokens.size()) { | ||
| r.ys_log_probs = src.token_log_probs; | ||
| } | ||
|
|
||
| if (!src.vocab_log_probs.empty() && | ||
| src.vocab_log_probs.size() == src.tokens.size()) { | ||
| r.vocab_log_probs = src.vocab_log_probs; | ||
| } |
There was a problem hiding this comment.
Keep log-prob arrays aligned with filtered output tokens
Convert() filters tokens before writing r.tokens, but these assignments copy arrays indexed by src.tokens. That can break the per-token alignment contract for r.ys_log_probs/r.vocab_log_probs when tokens are skipped.
💡 Suggested fix
@@
OfflineRecognitionResult r;
r.tokens.reserve(src.tokens.size());
r.timestamps.reserve(src.timestamps.size());
+ std::vector<int32_t> kept_indices;
+ kept_indices.reserve(src.tokens.size());
@@
auto sym = sym_table[src.tokens[i]];
@@
r.tokens.push_back(std::move(sym));
+ kept_indices.push_back(i);
}
@@
- if (!src.token_log_probs.empty() &&
- src.token_log_probs.size() == src.tokens.size()) {
- r.ys_log_probs = src.token_log_probs;
+ if (!src.token_log_probs.empty() &&
+ src.token_log_probs.size() == src.tokens.size()) {
+ r.ys_log_probs.reserve(kept_indices.size());
+ for (auto idx : kept_indices) {
+ r.ys_log_probs.push_back(src.token_log_probs[idx]);
+ }
}
if (!src.vocab_log_probs.empty() &&
src.vocab_log_probs.size() == src.tokens.size()) {
- r.vocab_log_probs = src.vocab_log_probs;
+ r.vocab_log_probs.reserve(kept_indices.size());
+ for (auto idx : kept_indices) {
+ r.vocab_log_probs.push_back(src.vocab_log_probs[idx]);
+ }
}🤖 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/csrc/offline-recognizer-ctc-impl.h` around lines 80 - 88,
Convert() is copying src.token_log_probs and src.vocab_log_probs directly into
r.ys_log_probs and r.vocab_log_probs using src.tokens indices, which breaks
alignment when Convert() filters tokens before assigning r.tokens; update the
logic in Convert() to build r.ys_log_probs and r.vocab_log_probs by iterating
the same filtered token indices used to produce r.tokens and push the
corresponding entries from src.token_log_probs and src.vocab_log_probs (only
when those source arrays are non-empty and have sufficient size), so the
resulting r.ys_log_probs and r.vocab_log_probs match r.tokens in length and
order.
![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 implements the capture and exposure of vocabulary log-probabilities within the offline CTC decoder and C-API to support entropy-based confidence estimation. Key changes involve updating data structures and the greedy search decoder to record these probabilities. Feedback points out a potential integer overflow during memory allocation in the C-API and suggests optimizing memory consumption by making the full vocabulary distribution recording optional and using a flattened vector structure.
| float *flat = new float[r->count * vocab_size]; | ||
| for (int32_t i = 0; i < r->count; ++i) { | ||
| std::copy(result.vocab_log_probs[i].begin(), | ||
| result.vocab_log_probs[i].end(), flat + i * vocab_size); | ||
| } | ||
| r->vocab_log_probs = flat; |
There was a problem hiding this comment.
Potential integer overflow in the calculation of the buffer size and pointer arithmetic. r->count and vocab_size are both int32_t, so their product is calculated as a 32-bit signed integer. If the product exceeds INT32_MAX (approx. 2.1 billion), it will overflow, leading to an incorrect allocation size and undefined behavior during indexing in the subsequent loop. This is a realistic scenario for long audio files or large vocabularies.
float *flat = new float[static_cast<size_t>(r->count) * vocab_size];
for (int32_t i = 0; i < r->count; ++i) {
std::copy(result.vocab_log_probs[i].begin(),
result.vocab_log_probs[i].end(),
flat + static_cast<size_t>(i) * vocab_size);
}
r->vocab_log_probs = flat;| r.tokens.push_back(y); | ||
| r.timestamps.push_back(t); | ||
| r.token_log_probs.push_back(log_prob); | ||
| r.vocab_log_probs.emplace_back(p_log_probs, p_log_probs + vocab_size); |
There was a problem hiding this comment.
Unconditionally populating vocab_log_probs for every emitted token can lead to excessive memory consumption, especially for long audio inputs and models with large vocabularies. For instance, a 1-hour recording with 10,000 tokens and a 50,000-word vocabulary would require approximately 2GB of additional memory. Since this data is only needed for specific use cases like entropy-based confidence estimation, it should be optional. Additionally, using std::vector<std::vector> results in many small allocations; a single flat vector would be more efficient.
1 participant
Summary
The offline CTC greedy decoder (
offline-ctc-greedy-search-decoder.cc) currently runsstd::max_elementto find the argmax token but discards the log-probability value at that position. As a result,ys_log_probsonOfflineRecognitionResult(added by #2843 for the offline transducer path) is always empty for CTC models, and no API surface ever sees the full vocab distribution.This PR exposes both:
token_log_probs) — keeps the value thatstd::max_elementalready computed. Costs one extrafloatper emitted token. Routed into the existingOfflineRecognitionResult::ys_log_probs+ C structys_log_probsfield via the existing copy logic — no new C API surface needed for this.vocab_log_probs) — copy of the full softmax row at each emitted frame. New fields onOfflineRecognitionResult, the C decoder result struct, and the C API result struct (vocab_log_probsflat row-major[count, vocab_size]+vocab_size).Motivation
Per-token confidence + full vocab distributions are required for entropy-based confidence estimation (e.g. Tsallis / Rényi over the vocab row — NVIDIA's recommended approach for ASR confidence). PR #2897 attempted a broad version of this but was closed when the author's ROVER fusion benchmark didn't show a measurable win in their (multi-system, clinical-audio) setting. We're using it for single-model entropy on where the use case is fundamentally different, and the patch was good. This PR re-submits a focused subset covering only offline CTC, which is the architecture we use.
Backwards compatibility
OfflineRecognitionResultandSherpaOnnxOfflineRecognizerResultonly gain trailing fields. Existing consumers see no API/ABI changes.nullptrwhen callers don't consume them. Memory footprint cost:O(count * vocab_size * sizeof(float))per decode — opt-in via reading the field, but unconditionally populated. If that's a concern, happy to gate it on a config flag.ys_log_probs(which Add token-level confidence scores (ys_probs) for offline transducer models #2843 added for offline transducer) now also flows for offline CTC, matching the field's documentation.Testing
Built and tested on Android arm64-v8a (NDK r27) against a NeMo Conformer-CTC model (
EncDecCTCModelBPE, vocab_size 1025, blank = 1024). On real device, confidence values pass sanity checks:Happy to add a regression test if the maintainers want one — pointer to the right test fixture would help.
Not included in this PR (deliberately)
PR #2897's scope covered Whisper, Moonshine, Canary, NeMo transducer, online transducer, and Flutter/Dart bindings in addition to offline CTC. This PR keeps scope tight to offline CTC to maximise the chance of merge; happy to follow up with parallel PRs for the other decoders if this lands and the maintainer is open to it.
Closes / supersedes: a focused subset of #2897.
Summary by CodeRabbit
Release Notes