Repository navigation
[Model] Support MOSS-TTS-local-v1.5 - #4664
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Signed-off-by: gcanlin <canlinguosdu@gmail.com>
hsliuustc0106
left a comment
There was a problem hiding this comment.
🔴 PR Body Incomplete — Required Evidence Missing
This PR is substantial (>1000 LOC / >10 files). Please provide:
- vLLM Version — vLLM base version used
- vLLM-Omni Commit — Specific commit hash tested against
- L3 Test Results — Please run the L3 tests locally and paste results
- Manual Test — Example command and output showing the model works
See CONTRIBUTING.md for test guidelines.
linyueqian
left a comment
There was a problem hiding this comment.
Thanks for the PR. I ran this end to end on a single datacenter GPU (vllm 0.23.0, this PR's head 18c8f1be), covering offline, online voice clone, an existing-variant regression check, and a concurrency sweep. Summary below.
Verified green (single stream)
| Check | Result |
|---|---|
| Local-v1.5 offline (48 kHz stereo) | PASS, 5.04s, ASR near-verbatim |
Local-v1.5 online /v1/audio/speech voice clone |
PASS, HTTP 200, ASR exact |
| Talker weight load | PASS, 330/330 params, 0 skipped |
Codec (official v2 remote class, stereo batch_decode) |
PASS |
Regression: existing Realtime through the new _build_codec path |
PASS, ASR exact verbatim |
Two notes on the codec change for other reviewers. Since both v1 and v2 codecs ship auto_map, _build_codec now loads the official remote class for all variants, so the vendored audio_tokenizer.py in_proj Identity-to-Linear change is fallback only and does not affect the default path (Realtime verified intact). The official MossAudioTokenizerModel does expose batch_decode and config.codebook_size, so the wrapper API lines up.
Serious: concurrent distinct-text corruption (verified)
With two concurrent requests carrying different text, 6 of 6 iterations produced wrong audio. Examples:
req0 expected "She bought seven red apples..." -> "The rocket launched into space..." (req1's text)
req0 -> "She launched into space early this morning" (mid-stream mix of both)
req1 -> empty / req1 -> repetition loop
Run sequentially, every text transcribes exactly, so this is server-side per-request state crossing in the batched talker, not sampling noise. A perf sweep that reuses the same text for every request hides it, because swapping identical audio looks fine.
Likely cause: the even-split row assignment rows_per_req = num_rows // len(info_dicts) in make_omni_output, plus compute_logits which uses no request_token_spans at all. When requests co-batch with mismatched row counts (staggered prefill/decode), the forced text token (continue vs im_end) and the hidden[row_end - 1] slice land on the wrong request, giving premature stop (empty) or lost conditioning (wrong text). Suggest keying both functions off the real per-request spans rather than an even split.
Scope note: this is not unique to this PR. The existing Realtime variant (its talker is untouched here) also fails the same two-way distinct-text test 6/6 with an empty-output symptom, so concurrency robustness looks like a broader MOSS-stack issue. Flagging it here because this model is aimed at concurrent serving.
Performance (single GPU)
| Concurrency | Throughput | Mean latency | RTF |
|---|---|---|---|
| 1 | 4.14 audio_s/s | 2.37s | 4.15x |
| 8 | 5.72 audio_s/s | 13.99s | 0.75x |
| 16 | 5.71 audio_s/s | 21.1s | 0.56x |
Single stream is strong (RTF 4.15x). Throughput saturates near 5.7 to 6.0 audio_s/s. The ceiling is stage 0, not the codec: raising stage-1 codec max_num_seqs from 1 to 8 moved throughput under 5 percent. In make_omni_output the depth transformer generate_frame is called per request with batch 1, even though it already accepts (B, H); batching it across co-scheduled requests is the main throughput lever. Those concurrent numbers measure GPU work, not usable audio, given the corruption above.
Lower severity
- MEDIUM: audio sampling params are hardcoded in
make_omni_output(temp=0.8, top_k=30, top_p=0.6, rep_penalty=1.1), so the deploy YAML stage-0 sampling params are dead for audio quality. Suggest sourcing them from config or sampling_params. - MEDIUM: debug scaffolding in the per-frame hot path:
import os as _os, aMOSS_TTS_DEBUG_STOPenv hook, and an inlineimport logginginsidegenerate_frame(runs every audio frame). Worth removing or hoisting. - LOW: this PR removes the
MOSS_TTS_CODEC_PATHenv override, butrecipes/OpenMOSS/MOSS-TTS.mdstill documents it. - LOW: existing variants now depend on
trust_remote_codefor codec loading (a vendored fallback exists). Worth one line in the PR description. - LOW: no unit test for the Local variant.
Recommendation
Single-stream quality and the architecture port look good. Before relying on concurrent serving, please fix the per-request row routing in compute_logits and make_omni_output and re-verify with distinct concurrent texts. The depth-transformer batching and the hardcoded-sampling and debug cleanups are good follow-ups.
Verified on a single datacenter GPU, vllm 0.23.0.
|
tagging @Sy0307 to avoid duplicate efforts |
| "MossTTSRealtimeTalkerForGeneration", | ||
| ), | ||
| # MossTTSLocalModel: MOSS-TTS-Local-Transformer-v1.5 | ||
| "MossTTSLocalModel": ( |
There was a problem hiding this comment.
This registers a new model, but the PR body is still the template and there are no tests in the diff. Please add profiling evidence, an upstream/HF baseline comparison, and at least one runnable/e2e test that loads the checkpoint and verifies valid 48 kHz stereo audio before merge.
@linyueqian Do you have the script that can reproduce this bug? I test 2 requests with multiple times and don't reproduce it. The audios are normal. |
Signed-off-by: gcanlin <canlinguosdu@gmail.com>
|
I think there are two issues that should be fixed before this PR is merged. First, the row/request mapping in
Under continuous batching, different requests can have different token counts in the same step, especially in mixed prefill/decode or staggered-request scenarios. Using an even split here can apply one request's state to hidden/logit rows that belong to another request, which means forced text tokens or stop tokens can be written to the wrong request. Since We verified this on H20 with PR head Please change Bad audio files like (Origin prompts = "Request zero mentions copper coins and a quiet museum."): Second, the Local-v1.5 streaming WAV header is written as mono, but this model is a stereo codec path.
wav_header = _create_wav_header(
sample_rate=sample_rate_val,
num_channels=1,
bits_per_sample=16,
)I verified this on H20 with PR head
This makes clients parse stereo PCM as a mono sample stream instead of a valid stereo WAV. Playback and downstream audio processing will receive incorrect audio. Please write the correct channel count in the streaming WAV header, or do not wrap stereo PCM as WAV until the header can represent it correctly. Other parts LGTM and it is good beginning for Moss-tts 1.5 support. |
|
And plz check script below: Concurrent repro script#!/usr/bin/env python3
"""
MOSS-TTS-Local-v1.5 concurrent request repro script.
Purpose:
Stress staggered/continuous batching for /v1/audio/speech and save each WAV.
Use ASR or manual listening afterward to compare each output with its prompt.
Before running:
1. Start vLLM-Omni server for MOSS-TTS-Local-v1.5.
2. Set SERVER, MODEL, and REF_AUDIO below.
3. Run: python3 repro_moss_local_concurrency.py
"""
import base64
import concurrent.futures
import json
import time
import urllib.error
import urllib.request
import wave
SERVER = "http://127.0.0.1:PORT" # e.g. http://127.0.0.1:8320
MODEL = "PATH_OR_MODEL_ID_TO_MOSS_TTS_LOCAL_V1_5"
REF_AUDIO = "PATH_TO_REFERENCE_AUDIO_WAV"
OUTPUT_PREFIX = "/tmp/moss_local_concurrency_repro"
texts = [
"Request zero mentions copper coins and a quiet museum.",
"Request one mentions orange flowers beside a river.",
"Request two mentions purple mountains under winter snow.",
"Request three mentions black coffee and a glass window.",
"Request four is deliberately longer and talks about a patient scientist measuring clouds, counting stars, and writing careful notes for the morning report.",
"Request five says a silver bicycle rolled past the school.",
"Request six says a yellow taxi stopped near the station.",
"Request seven says the final lantern glowed above the bridge.",
]
# Stagger requests so the server is more likely to mix prefill/decode work.
delays = [0.0, 0.05, 0.10, 0.15, 0.25, 0.35, 0.45, 0.60]
with open(REF_AUDIO, "rb") as f:
ref_audio_b64 = base64.b64encode(f.read()).decode()
def request_one(i: int) -> dict:
time.sleep(delays[i])
out_path = f"{OUTPUT_PREFIX}_{i}.wav"
payload = {
"model": MODEL,
"input": texts[i],
"voice": "test",
"ref_audio": "data:audio/wav;base64," + ref_audio_b64,
"response_format": "wav",
"max_new_tokens": 96,
}
req = urllib.request.Request(
SERVER + "/v1/audio/speech",
data=json.dumps(payload).encode("utf-8"),
headers={"Content-Type": "application/json"},
)
started = time.time()
try:
with urllib.request.urlopen(req, timeout=600) as resp:
data = resp.read()
status = resp.status
with open(out_path, "wb") as f:
f.write(data)
try:
with wave.open(out_path, "rb") as wav:
meta = {
"channels": wav.getnchannels(),
"sample_rate": wav.getframerate(),
"frames": wav.getnframes(),
"duration_s": round(wav.getnframes() / wav.getframerate(), 2),
}
except Exception as exc:
meta = {
"wav_error": type(exc).__name__,
"message": str(exc),
"first_40_bytes_hex": data[:40].hex(),
}
return {
"i": i,
"status": status,
"bytes": len(data),
"seconds": round(time.time() - started, 2),
"out": out_path,
"meta": meta,
"prompt": texts[i],
}
except urllib.error.HTTPError as exc:
return {
"i": i,
"error": "HTTPError",
"status": exc.code,
"body": exc.read().decode("utf-8", errors="ignore")[:1000],
"prompt": texts[i],
}
except Exception as exc:
return {
"i": i,
"error": type(exc).__name__,
"message": str(exc),
"prompt": texts[i],
}
def main() -> None:
started = time.time()
with concurrent.futures.ThreadPoolExecutor(max_workers=len(texts)) as executor:
futures = [executor.submit(request_one, i) for i in range(len(texts))]
for future in concurrent.futures.as_completed(futures):
print("RESULT", json.dumps(future.result(), ensure_ascii=False), flush=True)
print("TOTAL", round(time.time() - started, 2), flush=True)
if __name__ == "__main__":
main() |
Signed-off-by: gcanlin <canlinguosdu@gmail.com>
Signed-off-by: gcanlin <canlinguosdu@gmail.com>
|
@Sy0307 @linyueqian I push 6964d5e to fix the batch bug. Tested the script by @Sy0307 and the audios are normal now. The root cause is that we don't maintain the state dict for per-request. I add a helper function to resolve the spans temporarily. Before, the code incorrectly resolved the span evenly. The correct way is that split hidden rows by the real spans that records the range of rows for per-request. |
| ) | ||
| wav_header = _create_wav_header(sample_rate=sample_rate_val, num_channels=1, bits_per_sample=16) | ||
| num_channels = _infer_audio_num_channels(np.asarray(chunk_np)) | ||
| wav_header = _create_wav_header( |
There was a problem hiding this comment.
By the way, I think we really need to refactor the whole serving_speech.py. To be honest, it's hard to read and maintain now. And it has 4000+ lines now. @linyueqian @Sy0307 @hsliuustc0106 @alex-jw-brooks
There was a problem hiding this comment.
Maybe proposaling a new RFC can be a good start for making this file clear and easy to reuse. Thoughts? @linyueqian
There was a problem hiding this comment.
indeed. i try to refactor some but need more work
There was a problem hiding this comment.
@gcanlin completely agree, I have actually been meaning to refactor some of the entrypoints once I have some bandwidth. I think we need to decouple model specific code from individual entrypoints as much as possible. It will simplify this entrypoint a lot and make testing easier.
serving_chat is also quite hard to look at and 3.7k lines too 😬 I think we need to make sure tests are solid for current behavior and should simplify them both as soon as possible
There was a problem hiding this comment.
lets create a new RFC for that
|
Have checked latest fix and it works as expected now. Nice work and thx @gcanlin |
Signed-off-by: gcanlin <canlinguosdu@gmail.com>
PLEASE FILL IN THE PR DESCRIPTION HERE.
Purpose
Support https://huggingface.co/OpenMOSS-Team/MOSS-TTS-Local-Transformer-v1.5.
Test Plan
vLLM Version: 0.23.0
vLLM-Omni Commit: main
Test Result
BEFORE SUBMITTING: read CONTRIBUTING.md and run the precheck-pr skill with the code agent for a self-check against project conventions.
(anything written below this line will be removed by GitHub Actions)