perf(tts): pipeline sync per-sentence synthesis with playback - #74301
MahdiHedhli wants to merge 1 commit into
Conversation
|
Thanks for the focused performance improvement. The premise is confirmed on current main: No verified correctness or design-fit problems found in the reviewed diff. The production file has not changed since the PR base; only older tests were pruned from This is an automated hermes-sweeper review. |
The universal sync fallback in stream_tts_to_speaker ran strictly serially
per sentence — synthesize, play, and only then start synthesizing the next
sentence — so every sentence boundary added a full synthesis-time of dead
air. Chunked streamers (elevenlabs/openai/gemini/xai) already avoid this;
every other provider (edge, piper, plugin providers) paid it on each reply
in voice mode and the wake-word loop.
_SyncSentencePipeline overlaps the two: one single-threaded synthesis
worker (sentences stay FIFO; providers never see concurrent calls from
this loop — same effective concurrency as before) feeds one playback
worker through a small bounded queue, so sentence n+1 synthesizes while
sentence n plays. Lookahead is bounded (backpressure + at most a couple of
temp files), stop_event short-circuits both stages, synthesis failures are
isolated per sentence, temp files are always unlinked, and the finally
block flushes the pipeline BEFORE tts_done_event fires so continuous voice
mode never reopens the mic over its own voice. synthesize/play are
resolved late so existing monkeypatch-based tests work unchanged.
Measured with a real local model provider (OmniVoice plugin, Apple
Silicon), same 3-sentence reply, playback simulated at the produced clips'
true durations, best-of-2 interleaved runs under identical load:
serial pipelined
time to first word 10.8s 4.4s
mid-reply dead air 11.2s 1.8s (second gap: 0.03s)
full reply wall 33.2s 17.0s
Tests: 4 new (timestamp-proven overlap, order + per-sentence failure
isolation, stop skips queued playback, temp-file hygiene); the existing
sync-fallback and display-callback tests pass unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
77d2094 to
178c0d6
Compare
|
Thanks @MahdiHedhli — the measurement was right and the gap is real: main's prefetch rework covers only chunked streamers, so edge/piper/plugin providers still paid full per-sentence dead air on the serial sync path. Salvaged into #77355 with your authorship preserved via cherry-pick; conflict resolution kept all of main's prefetch machinery intact, and the ordering/backpressure/shutdown semantics of your pipeline passed a dedicated audit (single-worker FIFO + bounded queue + drain-then-shutdown close, running before tts_done_event.set). 245 tts tests green. Closing in favor of the salvage. |
What does this PR do?
Removes the per-sentence dead air from voice-mode / wake-word replies for every provider without a chunked streaming API.
stream_tts_to_speaker's universal sync fallback ran strictly serially per sentence — synthesize, play, and only then start synthesizing the next sentence — so every sentence boundary added a full synthesis-time of silence. The chunked streamers (elevenlabs/openai/gemini/xai) already avoid this; edge, piper, and plugin providers all paid it on every reply. For local model providers the cost dominates the conversation: at real-time-factor ≈ 1, the reply is silent between sentences for about as long as it speaks.This replaces the serial
_speak_via_syncwith_SyncSentencePipeline: one single-threaded synthesis worker (sentences stay FIFO — providers never see concurrent calls from this loop, same effective concurrency as before) feeding one playback worker through a small bounded queue. While sentence n plays, sentence n+1 is already synthesizing.Behavior preserved deliberately:
tts_done_eventsemantics — thefinallyblock flushes the pipeline before the event fires, so continuous voice mode never reopens the mic over its own voice.synthesize/playare resolved late (module global / import in worker), so the existing monkeypatch-based tests pass unchanged.Related Issue
No open issue found — searched open+merged PRs and issues for sentence/pipeline/TTS-latency phrasings per CONTRIBUTING's search-first rule; closest prior art is the chunked-streamer work this composes with (streamer path untouched).
Type of Change
Changes Made
tools/tts_tool.py— add module-level_SyncSentencePipeline; construct it instream_tts_to_speakerwhen no chunked streamer resolves; route_speak_sentence's sync branch through it; retire the serial_speak_via_sync; flush the pipeline infinallybeforetts_done_event.set().tests/tools/test_tts_streaming.py— 4 new tests: timestamp-proven overlap (synth of sentence 2 starts before playback of sentence 1 ends), order preservation + per-sentence failure isolation, stop-event skipping queued playback, temp-file cleanup.How to Test
pytest tests/tools/test_tts_streaming.py -q— 34 pass here (the pre-existingtest_streamer_path_writes_pcm_to_outputfailure on macOS is unrelated: the Darwin TCC guard forcesoutput_stream = None, so that test can only pass on Linux CI).test_tts_streaming_e2e.py,test_voice_mode.py,test_tts_speed.py,test_tts_plugin_dispatch.py): 193 passed / 8 failed — the identical 8 (WSL2/PowerShell) fail on unpatchedmainon macOS; zero regressions.Fast cloud providers benefit less in absolute terms but the boundary stall (network round-trip per sentence) still disappears behind playback.
Checklist
stream_tts_to_speakerunchanged)cli-config.yaml.example— N/A (no new config keys)ThreadPoolExecutor,queue,threading) — no platform-specific code; Windows/macOS/Linux identical