Repository navigation
[Bugfix] Fix cross-request codes.ref leak in Qwen3-TTS make_omni_output - #4373
Conversation
make_omni_output collapsed every request's reference codec frames into one last-writer-wins slot and emitted them as a length-1 list, which to_payload_element broadcasts to the whole batch (the element[0] fallback). Each utterance's first vocoder chunks then decoded with another request's voice as context: audible onset timbre deformation at any concurrency > 1, correct at concurrency 1. Keep codes.ref batch-aligned instead: one entry per request, with an empty placeholder for requests that carry no ref (the downstream numel() > 0 checks already treat an empty tensor as no-ref). Fixes vllm-project#4370 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: jjw <m@jjw.in>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Reviewed PR #4373: [Bugfix] Fix cross-request codes.ref leak in Qwen3-TTS make_omni_output |
linyueqian
left a comment
There was a problem hiding this comment.
LGTM. I verified both the defect and the fix independently rather than just reading the diff.
Root cause confirmed: make_omni_output collapsed every request's ref into a single last-writer-wins variable and emitted it as a length-1 list, and to_payload_element broadcasts element[0] to any request index past the end of the list. The prefix cache path has the same fallback in _unwrap_lists, so both paths leaked. With the batch-aligned list, element[idx] picks the right request on both paths, and the ordering assumption is the same one the audio codes concatenation already relies on.
I also checked the two subtle cases:
- The empty placeholder is safe at all three consumers (
_normalize_ref_code, the async-chunk path, and the orchestrator path instage_input_processors/qwen3_tts.py): each guards onnumel(). - Cross-step accumulation in full-payload mode: the ref is written into the request's persistent intermediate buffer at first prefill, so a request that has a ref re-emits it every step and the placeholder can never clobber a real ref in
accumulate_full_payload_output. The placeholder only ever applies to requests that genuinely carry no ref, which is exactly the case main got wrong.
Test results (H20-class node, vllm 0.22.0; these are CPU-only tests):
- New regression test against the PR base without the fix: 2/3 fail, leak reproduces exactly as described.
- On this branch: 3/3 pass, and the full
tests/model_executor/models/qwen3_tts/directory passes (114/114). - Cherry-picked onto current main (
9577ccb): applies cleanly, no new failures.
One note for CI triage: current main already fails 10 tests in test_qwen3_tts_talker_preprocess.py with AttributeError: '_embedding_dtype', introduced by #4283 (those tests build the talker via __new__ and skip __init__). If Buildkite comes back red on that file after the ready label is added, it is pre-existing and not this PR.
| ref_code_list.append(ref_code) | ||
| has_ref_code = True | ||
| else: | ||
| ref_code_list.append(torch.empty(0, dtype=torch.long)) |
There was a problem hiding this comment.
[minor] The placeholder is 1-D (0,) while real refs are 2-D (N, num_code_groups). Every current consumer only checks numel(), so this works today, but a (0, num_code_groups) placeholder would be shape-consistent if a downstream ever concatenates or reshapes these. Fine to leave as is.
| audio_codes_list: list[torch.Tensor] = [] | ||
| ref_code_len_list: list[torch.Tensor] = [] | ||
| ref_code_tensor: torch.Tensor | None = None | ||
| ref_code_list: list[torch.Tensor] = [] |
There was a problem hiding this comment.
Could pre-size this to len(info_dicts) and assign by index rather than appending — len(ref_code_list) == batch size then holds by construction, and both append(torch.empty(0)) sites further down drop out:
ref_code_list: list[torch.Tensor] = [
torch.empty(0, dtype=torch.long) for _ in info_dicts
]
...
for i, info in enumerate(info_dicts):
if not isinstance(info, dict):
continue
...
if isinstance(ref_code, torch.Tensor) and ref_code.numel() > 0:
ref_code_list[i] = ref_code
has_ref_code = TrueMinor — but it means a future continue added to this loop can't silently desync the list from the batch.
Resolve serving_speech.py conflict from Step-Audio2 (vllm-project#464): instead of the inline 'if self._tts_model_type == "step_audio2"' branch added on main, migrate Step-Audio2 onto the adapter framework (new StepAudio2Adapter delegating to the existing _validate_tts_request dispatcher + _build_step_audio2_prompt). Keeps the uniform adapter dispatch; registry now has 14 model-types. Also brings in vllm-project#4373 (Qwen3-TTS codes.ref leak), vllm-project#4397 (Ming-omni Mapping), Signed-off-by: Yueqian Lin <linyueqian@outlook.com> vllm-project#3763 (Wan2.2 SP) from main.
…ut (vllm-project#4373) Signed-off-by: jjw <m@jjw.in> Co-authored-by: jjw <m@jjw.in> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Yueqian Lin <70319226+linyueqian@users.noreply.github.com>
…ut (vllm-project#4373) Signed-off-by: jjw <m@jjw.in> Co-authored-by: jjw <m@jjw.in> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Yueqian Lin <70319226+linyueqian@users.noreply.github.com>
Purpose
Fix cross-request voice leakage in Qwen3-TTS voice clone under concurrency.
make_omni_outputcollapses every request'scodes["ref"]into a single last-writer-wins variable and emits it as a length-1 list.to_payload_elementindexes list payloads withelement[idx] if idx < len(element) else element[0], so that list is broadcast to every request in the batch: each utterance's first vocoder chunks decode with another request's reference voice as context. Audible result: onset timbre deformation converging over ~1-2 s on every voice-clone utterance at any concurrency > 1; correct at concurrency 1. Silent (no warning, no error).This PR keeps
codes.refbatch-aligned: one entry per request, with an empty placeholder for requests that carry no ref (the downstreamnumel() > 0checks already treat an empty tensor as "no ref").Fixes #4370 (full diagnosis, repro script, and forensics there). Same defect family as #4355, which proposes hardening the
element[0]fallback itself.Test Plan
vLLM Version: 0.22.0
vLLM-Omni Commit: 3fe3aa3 (main)
tests/model_executor/models/qwen3_tts/test_qwen3_tts_talker_ref_codes.py: CPU-only, no weights, constructs the talker via__new__like the existing preprocess tests. Covers per-request refs, the no-ref placeholder, and the no-refs-at-all case.Test Result
codes.refcomes back as a length-1 list holding the last request's ref for every request, including requests that have no ref of their own; on this branch, each request gets its own entry.ruff checkandruff format --checkpass on both changed files.qwen-ttspackage decoding the same segments on the same GPU. Throughput unchanged within noise (22.4x vs 22.8x realtime at concurrency 16).