Repository navigation
Make speech streaming default to SSE - #4679
Conversation
Signed-off-by: syd520zy <529477025@qq.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
There was a problem hiding this comment.
Code review
The routing logic itself is correct. I checked the full stream x stream_format truth table and every combination resolves cleanly: stream_format="audio" still wins over the new SSE default, nothing is double-classified, and hoisting the word_timestamps guard to is_streaming() is sound. Making SSE the default for stream=True (issue #4612 "Option A") is the intended direction.
The remaining ask is to migrate the in-repo consumers and docs that still assume the old stream=True -> raw-PCM contract, since none are updated here (the PR only touches 3 files).
blocking: example/demo scripts that POST stream=true and read raw PCM
fish_speech/speech_client.py, plus the same pattern incosyvoice3/speech_client.pyandming_tts/openai_speech_client.py: they setpayload["stream"] = True+response_format="pcm", then writeresp.iter_bytes()to a.pcmfile.qwen3_tts/tts_common.py(stream=Trueby default, consumed as raw PCM chunks) and thevoxcpm2/moss_tts_nanogradio demos (proxy raw PCM to a browser AudioWorklet).- README curl pipes
"stream": truestraight intoplay -t raw ...:README.md(two examples, L89 and L529).
After this change all of these receive text/event-stream SSE where they expect raw PCM, producing garbage/silent audio. Please update them to pass stream_format="audio" for the raw path.
blocking: docs are now inaccurate
docs/serving/speech_api.mdL125 still documentsstream | bool | false | Stream raw PCM chunks as they are decoded. After this PR,stream=truestreams SSE, not raw PCM. The table (and any raw-stream snippets) should be updated, with raw PCM now described understream_format="audio".
not affected: e2e CI. The streaming TTS e2e tests route through send_audio_speech_request, which uses stream only to choose the client-side with_streaming_response read mode and never forwards it to the server, so they exercise the unchanged non-streaming path. The PR's own updated unit tests in test_serving_speech.py cover the new SSE behavior, so unit/lint CI stays green.
It would also help to note in the PR description that this intentionally reverses the stream=True -> raw-audio contract that #4490 introduced (its test_stream_true_prefers_raw_audio_over_sse asserted exactly that, and is renamed/inverted here).
Minor wording/docstring notes left inline.
| return self.stream_format == "audio" | ||
|
|
||
| def is_sse_stream(self) -> bool: | ||
| return self.stream_format == "sse" and not self.is_raw_audio_stream() | ||
| return (self.stream or self.stream_format == "sse") and not self.is_raw_audio_stream() | ||
|
|
||
| def is_streaming(self) -> bool: | ||
| return self.is_raw_audio_stream() or self.stream_format == "sse" | ||
| return self.is_raw_audio_stream() or self.is_sse_stream() |
There was a problem hiding this comment.
blocking (root cause): this block is where stream=True switches from raw-audio to SSE. The logic is correct and this is the intended Option A default. What is missing is the matching update to the in-repo consumers that still expect stream=True to return raw PCM. They are not in this PR's diff (so GitHub can't offer one-click suggestions on them); the exact edits are below.
Example / demo clients — add stream_format="audio" next to stream:
fish_speech/speech_client.py (identical in cosyvoice3/speech_client.py and ming_tts/openai_speech_client.py):
if args.stream:
payload["stream"] = True
payload["stream_format"] = "audio" # keep raw PCM streaming
payload["response_format"] = "pcm"qwen3_tts/tts_common.py:
payload: dict = {
"input": text.strip(),
"response_format": "pcm" if stream else response_format,
"stream": stream,
}
if stream:
payload["stream_format"] = "audio"voxcpm2/gradio_demo.py and moss_tts_nano/gradio_demo.py: add "stream_format": "audio" to the streaming payload dict the same way.
README (examples/online_serving/text_to_speech/README.md, both curl blocks, L89 and L529):
"stream": true,
+ "stream_format": "audio",
"response_format": "pcm"Docs (docs/serving/speech_api.md L125) — the stream row is now inaccurate:
-| `stream` | bool | false | Stream raw PCM chunks as they are decoded (requires `response_format="pcm"`) |
+| `stream` | bool | false | When true, stream OpenAI `speech.audio.*` SSE events (requires `response_format="pcm"` or `"wav"`). For raw PCM/WAV byte streaming, set `stream_format="audio"`. |
+| `stream_format` | string | null | `"audio"` = raw PCM/WAV bytes; `"sse"` (or `stream=true`) = OpenAI `speech.audio.*` SSE events. || Streaming is supported via the ``stream=True`` switch or ``stream_format='sse'``, | ||
| which return OpenAI ``speech.audio.*`` SSE events. ``stream_format='audio'`` | ||
| opts into raw audio streaming with ``response_format='pcm'`` or ``'wav'``. |
There was a problem hiding this comment.
nit: the two lines just below this (Each Code2Wav chunk is yielded as raw audio bytes ... / For WAV format, a header with placeholder size values is emitted first.) now describe only the stream_format='audio' path, not the new stream=True SSE default. Worth scoping them to the raw-audio case so the docstring isn't read as applying to SSE.
| "'sse' streams OpenAI speech.audio.* SSE events. Omit for non-streaming, " | ||
| "or use stream=true to stream SSE events." |
There was a problem hiding this comment.
nit: Omit for non-streaming, or use stream=true to stream SSE events reads as slightly self-contradictory, since omitting stream_format no longer implies non-streaming once stream=true. Minor wording.
Signed-off-by: syd520zy <529477025@qq.com>
|
I updated the PR to address the review feedback around the Changes made:
I also updated the raw streaming e2e request configs to include Note: this intentionally changes the compatibility behavior introduced by #4490, where |
… streaming_speech_default_to_sse Signed-off-by: syd520zy <529477025@qq.com> # Conflicts: # docs/serving/speech_api.md
… streaming_speech_default_to_sse Signed-off-by: syd520zy <529477025@qq.com> # Conflicts: # tests/e2e/online_serving/test_voxtral_tts.py
linyueqian
left a comment
There was a problem hiding this comment.
Re-review (round 2). The contract flip itself is correct and the round-1 comments are all addressed: the audio.py is_*_stream() truth table is sound (verified all six stream x stream_format combinations), the word_timestamps guard is now hoisted to is_streaming() so it also covers the SSE path, and the new unit/e2e coverage is solid. Forwarding stream_format in runtime.py also means the e2e streaming tests now actually exercise the raw-audio path instead of silently running non-streaming.
Two things still block merge.
[blocking] Merge conflict against current main. main has moved past this branch's last merge and renamed tests/e2e/online_serving/test_voxtral_tts.py to test_voxtral_tts_expansion.py, so this PR's edit to the old path is a modify/delete conflict (that is the current CONFLICTING state). See the inline note for the resolution.
[important] The Option A migration is incomplete. The description says consumers, docs, and benchmarks were all updated, but a few raw-audio consumers still send stream=true without stream_format="audio" and will now receive SSE instead of raw bytes:
recipes/Qwen/Qwen3-TTS.mdL150-152:stream:true+pcmpiped intoplay -t raw(and the prose at L166). Now feeds SSE into the player.recipes/mistralai/Voxtral-TTS.mdL82-83:stream:true+pcmwritten tovoxtral_stream.pcm, thenffmpeg -f s16le. SSE text lands in the.pcm.recipes/OpenMOSS/MOSS-TTS.mdL99-109:stream:true+wavwritten tooutput.wav. SSE text lands in the.wav.benchmarks/fish-speech/bench_speaker_cache.pyL203-204 and L242-243: postsstream=True, response_format=pcmover aiohttp expecting raw PCM.benchmarks/patch/patch.pywas fixed, but this standalone script was missed.
Each just needs "stream_format": "audio" added next to stream.
[suggestion] tests/e2e/online_serving/test_indextts2.py L77-82: the stream=True streaming test has no stream_format. It will not fail CI (the send_audio_speech_request helper never forwards stream to the server, so it runs non-streaming), but it no longer exercises streaming and is inconsistent with the e2e tests this PR did update. Worth adding "stream_format": "audio" for parity.
Non-blocking: diffusion-mode TTS (_create_diffusion_speech) ignores streaming entirely, but that is pre-existing and out of scope here.
| "voice": "casual_female", | ||
| "language": "English", | ||
| "stream": True, | ||
| "stream_format": "audio", |
There was a problem hiding this comment.
[blocking] main renamed this file to test_voxtral_tts_expansion.py after this branch's last merge, so this edit is now a modify/delete conflict (the current CONFLICTING state). When you merge main, drop this file and instead add "stream_format": "audio" to test_speech_english_streaming in test_voxtral_tts_expansion.py -- its streaming payload still has stream=True, response_format=pcm with no stream_format.
Signed-off-by: syd520zy <529477025@qq.com>
Updated the remaining raw-audio streaming consumers from the round-2 review. Changes made:
I also did another pass for the same raw-byte streaming pattern and fixed two additional affected examples:
These examples pipe or write raw PCM/WAV bytes, so they should explicitly opt into the raw audio stream format now that Validation run:
For the two outdated-file items:
Could you please re-check these two items against the latest PR head? |
linyueqian
left a comment
There was a problem hiding this comment.
Approving. All round-2 items are resolved: the voxtral merge conflict is fixed (old file dropped for upstream's rename, stream_format="audio" re-applied in test_voxtral_tts_expansion.py), and the remaining raw-audio consumers (recipes Qwen/Voxtral/MOSS/VoxCPM2, the MOSS-TTS-Nano block in the examples README, and bench_speaker_cache.py) plus test_indextts2.py now pass stream_format="audio".
Re-verified this pass: the is_*_stream() routing and the word_timestamps guard in serving_speech.py are unchanged and correct (all six stream x stream_format combinations check out), a full-tree sweep finds no remaining consumer that streams raw audio without stream_format, and the lightweight CI (DCO, build 3.11/3.12, docs, pre-commit) is green. The only caveat is the unrun full e2e suite already noted in the description; the e2e configs themselves are updated correctly.
Signed-off-by: syd520zy <529477025@qq.com>
Head branch was pushed to by a user without write access
The failing test was still mocking the old I updated the test mock target to match the current production path. I also checked for other stale |
Integrate upstream main 0899a1a (11 commits since 1b318d1) — orchestrator inter-stage/client output split (vllm-project#4527), diffusion request-level batching (vllm-project#4079), speech SSE default (vllm-project#4679), and assorted model/example migrations. Auto-merged with no conflicts. Signed-off-by: chickeyton <ngton2014@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: syd520zy <529477025@qq.com> Signed-off-by: Wojciech Kutak <wkutak@nvidia.com>
Signed-off-by: syd520zy <529477025@qq.com>
Summary
Make
/v1/audio/speechstreaming requests default to SSE.This implements Option A from #4612:
stream=Truenow returns OpenAI-stylespeech.audio.*SSE events.stream_format="sse"continues to return SSE events.stream_format="audio"remains available as an explicit opt-in for raw PCM/WAV byte streaming.stream_format="audio".Motivation
SSE provides structured stream events, which makes it possible to carry stream-level metadata such as usage, done events, and error events. Raw audio byte streaming cannot represent this metadata cleanly.
Issue #4612 discusses the server-side default behavior for speech streaming format. This PR follows the selected Option A direction: make speech streaming default to SSE while keeping raw audio streaming available through an explicit
stream_format="audio"request.Behavior
The updated behavior is:
stream, nostream_formatstream=Truestream_format="sse"stream=True+stream_format="sse"stream_format="audio"stream=True+stream_format="audio"word_timestamps=trueis still rejected for HTTP speech streaming, because word timestamp streaming is currently implemented by the WebSocket/v1/audio/speech/streampath.Notes
This intentionally changes the compatibility behavior introduced by #4490, where
stream=Truepreferred raw audio. Raw audio streaming remains available throughstream_format="audio".Tests
Ran:
pre-commit run ruff-format --files ...on the modified speech API, raw-streaming example/client, benchmark, helper, and test filespre-commit run ruff-check --files ...on the same modified Python filesgit diff --checkpython -m py_compile ...on the modified Python filesNot run:
stream_format="audio"for raw audio streaming, but model/hardware-backed e2e validation was not run locally.