Make the Pyannote segmentation window shift configurable - #3769
Conversation
dosubot
Bot
added
the
size:M
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Pyannote speaker segmentation configuration now supports a validated ChangesPyannote window shift configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a configurable window shift ratio (pyannote-window-shift-ratio) for the Pyannote offline speaker segmentation model, replacing the previously hardcoded value of 0.1. It adds validation to ensure the ratio is within the range (0, 1] and clamps the computed window shift to valid bounds. The review feedback points out a potential off-by-one truncation issue when casting the computed floating-point window shift to an integer, suggesting rounding to the nearest integer instead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| window_shift, meta_data_.window_size, meta_data_.window_size); | ||
| meta_data_.window_shift = meta_data_.window_size; | ||
| } else { | ||
| meta_data_.window_shift = static_cast<int32_t>(window_shift); |
There was a problem hiding this comment.
Using static_cast<int32_t> directly on window_shift truncates the fractional part. Due to floating-point precision limitations, certain ratios can result in a computed window_shift that is slightly less than the expected integer value (for example, 0.13f * 160000 results in 20799.999237..., which truncates to 20799 instead of 20800).
To prevent these off-by-one errors, round the computed value to the nearest integer by adding 0.5 before casting.
meta_data_.window_shift = static_cast<int32_t>(window_shift + 0.5);
csukuangfj
left a comment
csukuangfj
left a comment
There was a problem hiding this comment.
Thanks! Left a minor comment.
| const double window_shift = | ||
| static_cast<double>(config_.pyannote.window_shift_ratio) * | ||
| meta_data_.window_size; | ||
| if (!(window_shift >= 1)) { |
There was a problem hiding this comment.
Please use
if (window_shift < 1)
It is more readable.
There was a problem hiding this comment.
Done in f3a6816, thanks. That reads better.
I kept the NaN case explicit rather than leaving it to the negation:
if (std::isnan(window_shift) || window_shift < 1) {Validate() already rejects a NaN ratio, so this only bites library callers
that build the config directly and skip validation. Without the check a NaN
falls past both branches into static_cast<int32_t>, which is undefined. If
you would rather keep the line simple, I will drop it.
Replace the negated comparison with the more readable form suggested in review, keeping the NaN case explicit so a ratio that bypasses Validate() still clamps instead of reaching the cast.
csukuangfj
left a comment
csukuangfj
left a comment
There was a problem hiding this comment.
Thank you for your contribution!
2 participants
The pyannote segmentation sliding-window shift is currently hardcoded to 10% of the window size (a 10 s window sliding every 1 s, i.e. 90% overlap):
The shift directly scales the total work of the whole diarization pipeline: the number of chunks fed to BOTH the segmentation model and the embedding extractor is roughly proportional to 1/ratio. On CPU-only devices where offline diarization runs at RTF 0.25+, this is the single biggest speed knob, and it is not exposed anywhere today.
This PR adds
window_shift_ratiotoOfflineSpeakerSegmentationPyannoteModelConfig(CLI:--segmentation.pyannote-window-shift-ratio), default0.1:int32_tshift as before (for the standardwindow_size=160000, both the old0.1 * 160000and the new float-stored ratio promoted to double truncate to 16000), so existing users see no change.Validate()rejects values outside(0, 1](NaN-safe); the computed shift is additionally clamped to[1, window_size]with a warning for library callers that bypassValidate(), so a degenerate ratio can never produce a zero shift and an infinite chunk loop.Measured effect (644 s two-speaker English WAV,
sherpa-onnx-pyannote-segmentation-3-0+ CAM++ zh_en advanced embedding,--clustering.cluster-threshold=0.90, 4 threads each, macOS arm64, this repo's CLI):(Agreement computed on a 10 ms grid over the full file including silence, best label mapping; segment boundaries move by at most a few tens of milliseconds on this material. Quality on harder audio will degrade sooner as overlap drops — which is exactly why the default stays 0.1 and this is opt-in.)
Summary by CodeRabbit