fix(tts): per-sentence prefetch pipeline with PortAudio resilience - #71084
fix(tts): per-sentence prefetch pipeline with PortAudio resilience#71084beardedeagle wants to merge 1 commit into
Conversation
a020925 to
5151125
Compare
There was a problem hiding this comment.
Pull request overview
Improves the low-latency streaming TTS path in tools/tts_tool.py by introducing a per-sentence prefetch pipeline and adding resilience around PortAudio write failures, with accompanying regression tests for the streaming dispatch behavior.
Changes:
- Add per-sentence prefetching so each sentence’s
streamer.stream()request fires immediately and is buffered for a single FIFO playback worker. - Add PortAudio write error handling with stream reinit attempts and fallback to temp-file playback when reinit is exhausted.
- Expand
tests/tools/test_tts_streaming.pywith new regression tests for misaligned PCM chunks, PortAudio failures, and per-sentence prefetch behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tools/tts_tool.py | Implements per-sentence streaming prefetch + PortAudio error recovery in stream_tts_to_speaker. |
| tests/tools/test_tts_streaming.py | Adds regression tests covering streaming dispatch, misaligned PCM chunks, and PortAudio error scenarios. |
Comments suppressed due to low confidence (2)
tools/tts_tool.py:3023
- The no-audio-device temp-file path also passes raw streaming PCM chunks straight into the WAV writer. If the provider yields odd-length chunks (common for HTTP chunking), the WAV output can contain partial int16 samples or drop bytes. Apply the same int16 alignment here before writing/playing the tempfile.
# Materialize the prefetched chunks for the temp-file path.
_chunks = []
while True:
chunk = chunk_queue.get()
if chunk is None:
break
_chunks.append(chunk)
_play_via_tempfile(
iter(_chunks), stop_event, streamer.sample_rate
)
tests/tools/test_tts_streaming.py:156
- These new OpenAIStreamer tests assume managed-gateway availability via tools.tts_tool._has_openai_audio_backend(), but OpenAIStreamer.available() currently only checks OPENAI_API_KEY (tools/tts_streaming.py:204-206) and never consults tts_tool. As written, this test will fail (get_env_value is patched to None, so available() returns False). Either implement the managed-gateway availability path in OpenAIStreamer (and include that file change in this PR) or update/remove this test.
def _drain_queue(sentences):
q = queue.Queue()
for s in sentences:
q.put(s)
q.put(None)
return q
def _sd_mock():
sd = MagicMock()
out = MagicMock()
sd.OutputStream.return_value = out
return sd, out
def test_streamer_path_writes_pcm_to_output(monkeypatch):
from tools import tts_tool
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Rewrite stream_tts_to_speaker to eliminate inter-sentence gaps and survive transient PortAudio/CoreAudio errors: Per-sentence prefetch pipeline: Each sentence gets its own streamer.stream() call the moment it completes. A background prefetch thread fires the HTTP request immediately, buffering PCM chunks into a per-segment queue. The single playback worker drains these queues in FIFO order. Sentence N+1's HTTP request fires WHILE sentence N is still playing, so audio is already arriving when the worker reaches it — no inter-sentence gap. PCM chunk alignment: OpenAI's streaming PCM API yields chunks on arbitrary byte boundaries not aligned to the int16 frame width (2 bytes). The old code called numpy.frombuffer directly on each chunk, which raised 'buffer size must be a multiple of element size' on odd-length chunks and silently dropped them. Carry leftover bytes into the next chunk so all audio plays. Use dtype='<i2' (explicit little-endian) to match the StreamingTTSProvider contract. PortAudio write error resilience: PortAudio/CoreAudio can raise transient errors (PaErrorCode -9986) on macOS device state changes, buffer underruns, or Bluetooth disconnects. Wrap output_stream.write() in try/except. On error, reinitialize the output stream (up to 3 attempts). If reinit is exhausted, fall back to temp-file playback for remaining sentences instead of dropping them. Worker join timeout: Increase the playback worker join timeout from 30s to 300s. A 1000-character response is ~70s of audio at 15 chars/s. The old 30s timeout expired mid-playback on longer responses, closing the output stream out from under the still-running worker.
5151125 to
b567650
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tackling the real sequential-streaming and PCM-alignment issues: current main still blocks on a sentence's streamer.stream() iterator at tools/tts_tool.py:3445-3468, and directly converts each chunk at tools/tts_tool.py:3463.
Problems
- Cancellation can strand a prefetch thread. In PR head
tools/tts_tool.py:2880,_consume_to_queue()unconditionally blocks onchunk_queue.put(None), but the worker skips a queued segment whenstop_eventis set (tools/tts_tool.py:2937-2938). If the bounded queue is full, nobody drains it and_prefetch_sem.release()is unreachable. - The branch predates main's deliberate macOS output policy.
a1bc12f191added the Darwin guard now attools/tts_tool.py:3393-3399; the PR baseline opensOutputStreamunconditionally. A salvage must retain that tempfile/afplay route and the current output-activity lifecycle (tools/tts_tool.py:3449-3465).
Suggested changes
- Make cancellation drain/unblock every segment queue and add a saturated-queue cancellation regression.
- Reapply the prefetch design onto current main while retaining the newer macOS and tempfile-cleanup behavior.
This is an automated hermes-sweeper review.
| exc, | ||
| ) | ||
| finally: | ||
| chunk_queue.put(None) # sentinel: no more chunks |
There was a problem hiding this comment.
Cancellation can deadlock this producer: _playback_worker skips a queued segment when stop_event is set, so after the bounded queue fills there is no consumer for this unconditional sentinel put. The thread then never reaches _prefetch_sem.release(). Drain cancelled queues or make completion signaling cancellation-aware, and add a saturated-queue stop regression.
Replaces the synchronous per-sentence streamer.stream() loop in stream_tts_to_speaker with a per-sentence prefetch pipeline. Each sentence gets its own background thread that fires the HTTP request immediately, buffering PCM chunks into a per-segment queue (capped at 3 concurrent via semaphore). A single playback worker drains segments in FIFO order with PortAudio error recovery (reinit up to 3 attempts, then temp-file fallback). Also fixes PCM chunk alignment for odd-byte HTTP chunks and increases the worker join timeout from 30s to 300s. Salvage of PR NousResearch#71084 onto current main. Closes NousResearch#71084
Start from main's 13 tests (renamed test_openai_available_reflects_key to test_openai_available_reflects_audio_key_resolution, added 4 new tests for xai oauth, elevenlabs secret resolver, openai configured key, stream cap). Append 12 new regression tests from PR NousResearch#71084 for the prefetch pipeline, PCM misalignment, and PortAudio resilience. Patch platform.system in stream-path tests for main's macOS guard.
Replaces the synchronous per-sentence streamer.stream() loop in stream_tts_to_speaker with a per-sentence prefetch pipeline. Each sentence gets its own background thread that fires the HTTP request immediately, buffering PCM chunks into a per-segment queue (capped at 3 concurrent via semaphore). A single playback worker drains segments in FIFO order with PortAudio error recovery (reinit up to 3 attempts, then temp-file fallback). Also fixes PCM chunk alignment for odd-byte HTTP chunks and increases the worker join timeout from 30s to 300s. Salvage of PR #71084 onto current main. Closes #71084
Start from main's 13 tests (renamed test_openai_available_reflects_key to test_openai_available_reflects_audio_key_resolution, added 4 new tests for xai oauth, elevenlabs secret resolver, openai configured key, stream cap). Append 12 new regression tests from PR #71084 for the prefetch pipeline, PCM misalignment, and PortAudio resilience. Patch platform.system in stream-path tests for main's macOS guard.
|
Merged via #76623. Your commits cherry-picked with authorship preserved — the prefetch pipeline architecture is unchanged, with fixes applied on top for regressions found during review (mark_audio_output_active, tmp.close, exception-path deadlock, PCM leftover bug) and code dedup (_create_output_stream, _align_int16_chunks). Thanks for the contribution! |
Replaces the synchronous per-sentence streamer.stream() loop in stream_tts_to_speaker with a per-sentence prefetch pipeline. Each sentence gets its own background thread that fires the HTTP request immediately, buffering PCM chunks into a per-segment queue (capped at 3 concurrent via semaphore). A single playback worker drains segments in FIFO order with PortAudio error recovery (reinit up to 3 attempts, then temp-file fallback). Also fixes PCM chunk alignment for odd-byte HTTP chunks and increases the worker join timeout from 30s to 300s. Salvage of PR NousResearch#71084 onto current main. Closes NousResearch#71084
Start from main's 13 tests (renamed test_openai_available_reflects_key to test_openai_available_reflects_audio_key_resolution, added 4 new tests for xai oauth, elevenlabs secret resolver, openai configured key, stream cap). Append 12 new regression tests from PR NousResearch#71084 for the prefetch pipeline, PCM misalignment, and PortAudio resilience. Patch platform.system in stream-path tests for main's macOS guard.
Replaces the synchronous per-sentence streamer.stream() loop in stream_tts_to_speaker with a per-sentence prefetch pipeline. Each sentence gets its own background thread that fires the HTTP request immediately, buffering PCM chunks into a per-segment queue (capped at 3 concurrent via semaphore). A single playback worker drains segments in FIFO order with PortAudio error recovery (reinit up to 3 attempts, then temp-file fallback). Also fixes PCM chunk alignment for odd-byte HTTP chunks and increases the worker join timeout from 30s to 300s. Salvage of PR NousResearch#71084 onto current main. Closes NousResearch#71084
Start from main's 13 tests (renamed test_openai_available_reflects_key to test_openai_available_reflects_audio_key_resolution, added 4 new tests for xai oauth, elevenlabs secret resolver, openai configured key, stream cap). Append 12 new regression tests from PR NousResearch#71084 for the prefetch pipeline, PCM misalignment, and PortAudio resilience. Patch platform.system in stream-path tests for main's macOS guard.
Problem
Three issues in stream_tts_to_speaker:
Inter-sentence gaps (3-5s) — each sentence triggered its own API call, but the HTTP request only fired when the playback worker reached the generator, not when the sentence was enqueued.
Scattered audio fragments — OpenAI streaming PCM yields chunks on arbitrary byte boundaries not aligned to int16 (2 bytes). numpy.frombuffer raised on odd-length chunks and silently dropped them.
Process hang on PortAudio errors — transient PaErrorCode -9986 errors on macOS device state changes killed the playback thread and hung the pipeline join. The 30s worker join timeout was also too short.
Solution
Per-sentence prefetch pipeline — each sentence gets its own streamer.stream() call immediately upon completion. A background thread fires the HTTP request right away, buffering PCM chunks into a per-segment queue. A single playback worker drains these in FIFO order — sentence N+1's HTTP fires while sentence N is still playing.
PCM chunk alignment — carry leftover bytes into the next chunk. Use dtype='<i2' (explicit little-endian) per the StreamingTTSProvider contract.
PortAudio write error resilience — wrap output_stream.write() in try/except. On error: reinitialize the stream (up to 3 attempts), re-attempt the failed write, fall back to temp-file playback if reinit is exhausted.
Worker join timeout — 30s to 300s (a 1000-char response is ~70s of audio).
Regression tests