fix(dub): auto-assign per-speaker voices in multi-speaker dubbing (#486) - #490
Conversation
Multi-speaker dubs detected speakers and built per-speaker clones (Voice
dropdown showed "From Video → Speaker N"), but most segments stayed on
"Default" voice and had to be set by hand — inconsistently across runs.
Root cause: after diarization, dub_core stamped each long line (the
default-on per-segment-ref path) with `auto-seg:{id}` as its profile_id.
The dub editor's Voice <select> (and the Cast panel) only render `auto:`
options, so an `auto-seg:` value matched no <option> and silently showed
"Default". Short lines (<3s) fell through to `auto:{speaker}`, which DID
render — hence "sometimes the cloned voice is picked".
Fix: bind every segment to the UI-visible `auto:{speaker}` whenever its
detected speaker has a clone; only fall back to `auto-seg:{id}` when the
speaker has no per-speaker clone at all. The per-segment-ref quality win
is preserved: dub_generate's `auto:` branch now transparently prefers
THIS segment's own per-segment ref (segment_clones[seg_id]) when present,
else the per-speaker clone. Manual overrides and the no-clone path are
untouched; existing jobs that persisted `auto-seg:` ids still resolve.
Tests: tests/test_dub_multispeaker_voice_486.py — assignment binds to
auto:{speaker} (not auto-seg:), never clobbers manual overrides, falls
back to auto-seg: only when the speaker has no clone; generate-time
resolution prefers per-segment ref then per-speaker clone. Green
alongside the existing dub generate/incremental/segmentation suites.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughFixes multi-speaker auto-clone ChangesMulti-speaker auto-voice profile resolution fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Panel review — specific line-level concerns: ML inference (dub_generate.py, lines 218–242) The new per-segment lookup does Audio DSP (dub_core.py, lines 802–830)
Desktop systems / product polish (tests/test_dub_multispeaker_voice_486.py, lines 108–185)
🚥 Pre-merge checks | ✅ 9✅ Passed checks (9 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
| Filename | Overview |
|---|---|
| backend/api/routers/dub_core.py | Assignment loop reordered to prefer auto:{speaker} over auto-seg:{id} when a per-speaker clone exists; per-segment fallback preserved for the no-clone edge case. Logic is correct and backward-compatible. |
| backend/api/routers/dub_generate.py | New auto: branch transparently prefers segment_clones[seg_id] before falling back to the per-speaker clone; closure over seg_id is safe due to immediate await. One misleading comment (loop direction) flagged. |
| tests/test_dub_multispeaker_voice_486.py | Good hermetic coverage of both the assignment and generate-time resolution paths; Part 1 helpers duplicate production code (acknowledged risk), Part 2 exercises real dub_generate path with a stub model. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[diarization complete\nspeaker_clones + seg_clones] --> B{segment has\nmanual profile_id?}
B -- yes --> Z[keep as-is]
B -- no --> C{spk in\nspeaker_clones?}
C -- yes --> D["assign auto:{speaker}\n← NEW default"]
C -- no --> E{seg_id in\nseg_clones?}
E -- yes --> F["assign auto-seg:{id}\n(renders as Default)"]
E -- no --> G[no profile assigned]
D --> H[dub_generate auto: branch]
H --> I{segment_clones\nhas this seg_id?}
I -- yes --> J[use per-segment ref\nhigh-prosody clone]
I -- no --> K[use per-speaker ref\nfallback clone]
style D fill:#22c55e,color:#fff
style H fill:#3b82f6,color:#fff
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[diarization complete\nspeaker_clones + seg_clones] --> B{segment has\nmanual profile_id?}
B -- yes --> Z[keep as-is]
B -- no --> C{spk in\nspeaker_clones?}
C -- yes --> D["assign auto:{speaker}\n← NEW default"]
C -- no --> E{seg_id in\nseg_clones?}
E -- yes --> F["assign auto-seg:{id}\n(renders as Default)"]
E -- no --> G[no profile assigned]
D --> H[dub_generate auto: branch]
H --> I{segment_clones\nhas this seg_id?}
I -- yes --> J[use per-segment ref\nhigh-prosody clone]
I -- no --> K[use per-speaker ref\nfallback clone]
style D fill:#22c55e,color:#fff
style H fill:#3b82f6,color:#fff
Reviews (1): Last reviewed commit: "fix(dub): auto-assign per-speaker voices..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| continue | ||
| sid = str(s.get("id", "")) | ||
| if sid and sid in seg_clones: | ||
| s["profile_id"] = f"auto-seg:{sid}" | ||
| return final_segs | ||
|
|
||
|
|
||
| def test_segments_bind_to_speaker_clone_not_auto_seg(): | ||
| """Long lines with a per-segment ref still get the UI-visible auto:{spk}.""" | ||
| clones = { | ||
| "Speaker 1": {"ref_audio": "/v/spk1.wav", "ref_text": "one"}, | ||
| "Speaker 2": {"ref_audio": "/v/spk2.wav", "ref_text": "two"}, | ||
| } |
There was a problem hiding this comment.
_assign_default_profiles is a verbatim copy of the loop in dub_core.py, not a call to it. As the comment acknowledges, if the production loop is refactored (e.g., the continue ordering changes or a third profile prefix is added), these four unit tests will keep passing on stale logic while the real path regresses. The generate-time tests in Part 2 do hit real code, so the user-visible path is guarded, but the dub_core assignment tests give false confidence. Consider extracting the assignment loop into a standalone helper that both dub_core.py and the test can import directly.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/api/routers/dub_generate.py (1)
227-227: 💤 Low value[DSP/Product] Closure captures loop variable
seg_idby reference—works today, breaks on refactor.Line 227 references
seg_idfrom the outer loop (line 140). Theawaiton line 404 currently guarantees_gencompletes before the next iteration, so this is not a runtime bug. However, if a future change removes the await (e.g., to parallelize segment generation), the closure will silently see the wrong segment id.Bind the variable at definition time to make the code future-proof:
♻️ Defensive fix for closure binding
- def _gen(text, lang, instruct_str, dur_s, nstep, cfg, spd, profile_id, effect_preset): + def _gen(text, lang, instruct_str, dur_s, nstep, cfg, spd, profile_id, effect_preset, *, _seg_id=seg_id): ref_audio = None ref_text = None used_seed = None @@ -224,7 +224,7 @@ # `auto:` id the dub editor's Voice dropdown can actually # render ("From Video → Speaker N"). `seg_id` is closed over # from the per-segment loop below. - seg_ref = (job.get("segment_clones") or {}).get(str(seg_id)) + seg_ref = (job.get("segment_clones") or {}).get(str(_seg_id))🤖 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 `@backend/api/routers/dub_generate.py` at line 227, The closure on line 227 that references `seg_id` from the outer loop (line 140) captures the variable by reference. While the await on line 404 currently prevents issues, this will break silently if the code is refactored to parallelize segment generation. Bind `seg_id` at definition time by using a default argument pattern (e.g., in a lambda or factory function) rather than capturing it from the enclosing scope, so that each closure iteration has its own independent copy of the segment id value.Source: Linters/SAST tools
🤖 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 `@backend/api/routers/dub_generate.py`:
- Line 227: The closure on line 227 that references `seg_id` from the outer loop
(line 140) captures the variable by reference. While the await on line 404
currently prevents issues, this will break silently if the code is refactored to
parallelize segment generation. Bind `seg_id` at definition time by using a
default argument pattern (e.g., in a lambda or factory function) rather than
capturing it from the enclosing scope, so that each closure iteration has its
own independent copy of the segment id value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c48d0e3a-b05b-4bde-857e-4e032f914ef5
📒 Files selected for processing (3)
backend/api/routers/dub_core.pybackend/api/routers/dub_generate.pytests/test_dub_multispeaker_voice_486.py
…) (#616) Segmentation groups words into sentences BEFORE diarization, so a two-speaker exchange can land in one segment; assign_speakers_* then only relabels it with the majority speaker, losing the turn boundary (the second half of #486 — the per-speaker voice auto-assign was fixed in #490). Add a post-diarization pass that re-splits any segment whose words span >1 speaker at the word-level boundary, assigning each piece its speaker: - backend/services/segmentation.py: resplit_segments_by_diarization / resplit_segments_by_turns + a pure _resplit_core. Single-speaker segments are returned BYTE-FOR-BYTE UNCHANGED (same dict/id/text/start/end) — the no-single-speaker-regression guarantee. Pieces keep the segment's outer start/end (preserving onset-snap) and use word times for interior splits, so they exactly cover the original span. A lone mis-attributed word is smoothed, not split (diarization noise). - backend/api/routers/dub_core.py: accumulate global-timeline words alongside segments; apply the re-split after both the pyannote and FunASR-turns assign. Heuristic fallback (no word-speaker data) is untouched. 8 regression tests pin the invariant + the split/3-way/noise-smoothing/label behaviour. Full suite: 1836 passed. Co-authored-by: mergetest <test@local> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Problem
Dubbing a multi-speaker video: diarization detects both speakers and per-speaker clones get created (the Voice dropdown shows From Video → Speaker 1 / Speaker 2), but most segments are left on Default voice instead of being auto-bound to their detected speaker's clone. A row correctly labelled
Speaker 1reads Voice =Default. Inconsistent across runs — short lines sometimes show the cloned voice, long lines don't. (Sub-bug 1 of #486.)Root cause
After diarization,
backend/api/routers/dub_core.pydefaults each segment'sprofile_id. With per-segment refs on (the default), every line ≥3s was stampedauto-seg:{id}. The dub editor's Voice<select>(and the Cast panel) inDubSegmentRow.jsx/DubTab.jsxonly renderauto:{speaker}options ("From Video → Speaker N") — anauto-seg:value matches no<option>, so the row silently falls back to displaying "Default". Short lines (<3s) had no per-segment ref and fell through toauto:{speaker}, which does render — exactly the "sometimes it's picked" inconsistency. The frontend has zero handling of theauto-seg:prefix.Fix
dub_core.py: bind every segment to the UI-visibleauto:{speaker}whenever its detected speaker has a clone. Only fall back toauto-seg:{id}when the speaker has no per-speaker clone at all (rare: too little usable audio overall, but a single long line still has its own ref).dub_generate.py: the per-segment-ref quality win is preserved transparently — theauto:resolution branch now prefers THIS segment's own per-segment ref (segment_clones[seg_id]) when present, else the per-speaker clone. So a row shown as "Speaker 1" still clones from its own line's source audio when long enough.Backward-compatible: manual overrides and the no-clone path are untouched; existing jobs that already persisted
auto-seg:ids still resolve via the retainedauto-seg:branch. No schema change, no version bump, no user-facing strings (no i18n/docs impact).Sub-bug 2 (speaker turns merged onto one line) — investigated, not fixed here
Root cause is clear but the fix is not low-risk, so it's documented rather than shipped: segmentation runs before diarization (
segment_transcript→_build_segments_from_wordsgroups words by sentence/duration only), andassign_speakers_from_diarizationonly relabels existing segments by majority overlap — it never re-splits a segment that spans two speakers. When the merge/stitch passes run, every segment still carries the placeholderspeaker_id="Speaker 1", so the "never merge across a speaker boundary" rule is a no-op. A correct fix needs diarization turns fed back as split boundaries (or a speaker-aware re-split pass), which touches the broadcast-grade segmentation rules and risks regressing single-speaker timing. Left for a dedicated change.Tests
tests/test_dub_multispeaker_voice_486.py(top-level,asyncio.run):auto:{speaker}(notauto-seg:) when a speaker clone existsauto-seg:only when the speaker has no clone; leaves Default when no clones existGreen alongside
test_segment_refs,test_smart_fit_generate,test_redub_incremental,test_assign_speakers_*,test_segmentation,test_dub_transcribe,test_dub_timing_strategy.🤖 Generated with Claude Code
Summary
Fixes automatic voice assignment in multi-speaker dubbing workflows where segments were inconsistently bound to the "Default" voice despite having detected speaker clones available.
The Problem
When diarization detects multiple speakers and creates per-speaker voice clones:
auto-seg:{segment_id}as theirprofile_id(per-segment reference)auto:{speaker}("From Video → Speaker N")auto-seg:doesn't match any rendered<option>, the UI silently fell back to "Default"auto:{speaker}, which renders properlyThe Fix
Two coordinated changes restore correct behavior while preserving per-segment voice quality:
1. Segment Binding (
dub_core.py)After diarization completes, the assignment loop now:
profile_idoverride:auto:{speaker}(UI-visible)auto-seg:{segment_id}(fallback)This ensures every segment carries a renderable
auto:{speaker}binding when its speaker has a clone.2. Generate-Time Resolution (
dub_generate.py)When an
auto:{speaker}binding is encountered during voice synthesis:job["segment_clones"][segment_id]for a per-segment referencejob["speaker_clones"]This preserves the Wave 3.2 per-segment-reference quality advantage while keeping the UI-visible binding stable.
Resolution Flow
Backward Compatibility
auto-seg:IDs continue to resolve correctly in generationTesting
New test suite (
tests/test_dub_multispeaker_voice_486.py) validates:auto:{speaker}(notauto-seg:)profile_idoverrides are preserveddub_generatepath with patched model