-
Notifications
You must be signed in to change notification settings - Fork 0
Improve TTS streaming chunking for better prosody and gapless playback #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
3f88ff7
fc73c27
469f70a
406b95f
bce7061
a4ed68b
afeb431
6894cfe
8b89b98
5e0eaa1
f275fe6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -461,22 +461,86 @@ def speech_worker(): | |||||||||||||||||||||||||||||||||
| speech_queue.put(None) # Stop the worker | ||||||||||||||||||||||||||||||||||
| speech_thread.join() | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def speak_streaming_async(self, text_generator, chunk_on: str = ".", print_text: bool = True, | ||||||||||||||||||||||||||||||||||
| @staticmethod | ||||||||||||||||||||||||||||||||||
| def _get_audio_duration(file_path: str) -> float: | ||||||||||||||||||||||||||||||||||
| """Get the duration of an audio file in seconds using pygame. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Falls back to a file-size estimate (~4 KB/s for Google TTS MP3) if | ||||||||||||||||||||||||||||||||||
| pygame.mixer.Sound cannot determine the length. | ||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||
| sound = pygame.mixer.Sound(file_path) | ||||||||||||||||||||||||||||||||||
| duration = sound.get_length() | ||||||||||||||||||||||||||||||||||
| del sound | ||||||||||||||||||||||||||||||||||
| if duration > 0: | ||||||||||||||||||||||||||||||||||
| return duration | ||||||||||||||||||||||||||||||||||
| except Exception: | ||||||||||||||||||||||||||||||||||
| pass | ||||||||||||||||||||||||||||||||||
| # Fallback: estimate from file size assuming ~32 kbps MP3 (4 KB/s) | ||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||
| return os.path.getsize(file_path) / 4000.0 | ||||||||||||||||||||||||||||||||||
| except Exception: | ||||||||||||||||||||||||||||||||||
| return 0.0 | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def speak_streaming_async(self, text_generator, chunk_on: str = ",.!?", print_text: bool = True, | ||||||||||||||||||||||||||||||||||
| min_chunk_size: int = 15): | ||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||
| Speak text as it's being generated with truly parallel processing. | ||||||||||||||||||||||||||||||||||
| Multiple sentences can be synthesized and queued while others are playing. | ||||||||||||||||||||||||||||||||||
| This provides the fastest response time. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Chunking strategy | ||||||||||||||||||||||||||||||||||
| ----------------- | ||||||||||||||||||||||||||||||||||
| * **Chunk 0 (fast start):** fire synthesis at the first punctuation | ||||||||||||||||||||||||||||||||||
| boundary (including commas) to minimise time-to-first-sound. | ||||||||||||||||||||||||||||||||||
| * **Chunk 1+ (quality chunks):** accumulate tokens and trigger synthesis | ||||||||||||||||||||||||||||||||||
| at the first sentence-ending boundary (``.!?``) where the remaining | ||||||||||||||||||||||||||||||||||
| playback time of previously queued audio drops to within | ||||||||||||||||||||||||||||||||||
| ``avg_synthesis_time + 200 ms``. This maximises input length for | ||||||||||||||||||||||||||||||||||
| better prosody while keeping playback gapless. | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
| * **Chunk 1+ (quality chunks):** accumulate tokens and trigger synthesis | |
| at the first sentence-ending boundary (``.!?``) where the remaining | |
| playback time of previously queued audio drops to within | |
| ``avg_synthesis_time + 200 ms``. This maximises input length for | |
| better prosody while keeping playback gapless. | |
| * **Chunk 1+ (quality chunks):** accumulate tokens and, once the | |
| remaining playback time of previously queued audio drops to within | |
| ``avg_synthesis_time + 200 ms``, trigger synthesis at the last | |
| available sentence-ending boundary (``.!?``). This maximises input | |
| length for better prosody while keeping playback gapless. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed — updated the docstring to say "trigger synthesis at the last available sentence-ending boundary" which matches the actual find_sentence_boundary (default: last boundary) behavior.
Generated by Claude Code
Copilot
AI
Apr 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code comment/strategy says chunk 0 should fire on the first punctuation boundary (including commas), but with the current default chunk_on="." and the any(c in buffer for c in chunk_on) check, chunk 0 will only trigger on periods unless callers explicitly pass commas/other punctuation. Consider changing the default chunk_on to include the intended punctuation (e.g., ",.!?") or hard-coding the additional delimiters for chunk 0 so the documented behavior matches the default behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed — changed the default chunk_on to ",.!?" so chunk 0 fires on commas, periods, and other sentence-ending punctuation out of the box.
Generated by Claude Code
Copilot
AI
Apr 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring for chunk_on says the default is ".", but the function signature now defaults to ",.!?". This makes the public API documentation inaccurate for callers. Update the docstring (and/or the param description) to reflect the actual default and clarify that chunk_on is only used for chunk 0 boundaries.
| :param chunk_on: Character to chunk on for chunk 0 (default: ".") | |
| :param chunk_on: Characters used to detect chunk 0 boundaries only | |
| (default: `",.!?"`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed — updated the docstring to Characters used to detect chunk 0 boundaries only (default: ",.!?").
Generated by Claude Code
Copilot
AI
Apr 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_remaining_playback_time() recomputes sum(state["queued_durations"]) each time it is called. Since this runs in the hot token-processing loop, it can become expensive when the queue grows. Consider tracking a single queued_total_duration float in state and updating it on append/pop so remaining time is computed in O(1).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed — replaced the queued_durations list with a single queued_total float that is incremented on append and decremented on pop, making remaining-time computation O(1).
Generated by Claude Code
Copilot
AI
Apr 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_update_avg is described as a rolling average, but it appends to an unbounded list and recomputes sum(...) on every update. Over long streams this becomes O(n^2) time and unbounded memory growth. Prefer an incremental mean (keep count + running sum) and/or a fixed-size window (e.g., collections.deque(maxlen=...)) to keep the update O(1) and memory bounded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed — replaced the unbounded list with an incremental mean using synth_count + synth_sum stored in the shared state dict. O(1) time and constant memory per update.
Generated by Claude Code
Copilot
AI
Apr 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
avg_synthesis_time (and synthesis_count/synthesis_sum) are updated in synthesis_worker but read in the main token-processing thread without any synchronization. This is a cross-thread data race that can yield inconsistent thresholds. Store/read these values under a lock (e.g., reuse state_lock or a dedicated lock) or use a thread-safe primitive (e.g., queue/atomic pattern) for publishing the latest average.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed — moved synth_count, synth_sum, and avg_synth into the shared state dict so all reads and writes happen under state_lock, eliminating the cross-thread data race.
Generated by Claude Code
Copilot
AI
Apr 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Temporary MP3 files can be leaked when synthesis fails. A temp file is created (delete=False) before synthesize_to_file, but if audio_file is falsy (quota block / error) or an exception is raised, there’s no cleanup of temp_filename, and playback never removes it. Add a cleanup path to delete the temp file when synthesis doesn’t produce a playable file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed — added cleanup of temp_filename in both the else branch (when synthesize_to_file returns falsy, e.g. quota block) and the except branch (on unexpected errors), preventing temp file leaks.
Generated by Claude Code
Copilot
AI
Apr 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
state["play_start"] is set before pygame.mixer.music.load() / .play(). If load/play takes non-trivial time, elapsed playback will be overcounted and _remaining_playback_time() will underestimate remaining audio, causing chunk 1+ to trigger earlier than intended. Set play_start immediately after .play() returns (or at least after load) so timing reflects real playback start.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Moved play_start assignment to immediately after pygame.mixer.music.play() returns, so elapsed-time calculations reflect actual playback start rather than including load latency.
Generated by Claude Code
Copilot
AI
Apr 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If pygame.mixer.music.load() or .play() raises, the queued_total value is never decremented for this item because the decrement happens inside this lock block after the load/play calls. That will cause _remaining_playback_time() to overestimate remaining audio and can delay subsequent synthesis triggers. Consider decrementing queued_total as soon as the item is dequeued (and track it as “active” during load/play), or use a flag so you always subtract estimated_dur in finally when playback fails before the state update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 5e0eaa1 — queued_total is now decremented immediately on dequeue (before try), so failures in load()/play() don't leave stale values. play_duration is also reset to 0.0 in the finally block so it doesn't linger after failure.
Generated by Claude Code
Copilot
AI
Apr 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If pygame.mixer.music.load/play raises or another playback exception occurs, the temporary audio_file is not removed, which can leak files over time. Consider moving the os.remove(audio_file) cleanup into a finally block (or attempt cleanup in the exception handler as well), while still ensuring state["playback_active"] is cleared.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed — moved both state["playback_active"] = False and os.remove(audio_file) into a finally block so they execute regardless of whether an exception occurred during load/play.
Generated by Claude Code
Copilot
AI
Apr 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The “Chunk 0 (fast start)” description says synthesis fires at the first punctuation boundary, but the implementation uses _find_sentence_boundary(...) which returns the last valid boundary in the current buffer (see chunking.find_sentence_boundary). This can delay time-to-first-audio if multiple boundaries arrive before the first synthesis is queued. Consider changing chunk 0 to select the first valid boundary, or update the doc/PR description if “last boundary” is intended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Added a first_only parameter to find_sentence_boundary() in chunking.py and chunk 0 now calls it with first_only=True so synthesis fires at the earliest valid boundary, minimising time-to-first-audio.
Generated by Claude Code
Copilot
AI
Apr 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After chunk 0 is enqueued, buffer may still contain sentence-ending punctuation from the same incoming token (e.g., a token containing multiple sentences). Because chunk 1+ only scans for boundaries when a future token contains .?!, pending_boundary may stay -1 and delay synthesis until the next punctuation or end-of-stream. Consider scanning the post-split remainder immediately when transitioning to chunk 1+ (or loop to consume multiple boundaries within the current token).
| chunk_index += 1 | |
| chunk_index += 1 | |
| # The remainder may already contain one or more | |
| # sentence-ending delimiters from the same token. | |
| # Cache the next boundary now so chunk 1+ does not | |
| # wait for a future token containing punctuation. | |
| pending_boundary = self._find_sentence_boundary(buffer, ".!?") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed — immediately after chunk 0 fires and the buffer is sliced, we now scan the remainder with self._find_sentence_boundary(buffer, ".!?") and seed pending_boundary. This ensures chunk 1+ doesn't stall waiting for a future token if the remainder already contains a sentence boundary.
Generated by Claude Code
Copilot
AI
Apr 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the chunk 1+ path, _find_sentence_boundary(buffer, ".!?") is called on every token even when the newly appended token contains no boundary punctuation. Because find_sentence_boundary scans the entire buffer, this turns long no-punctuation stretches into O(n^2) work as the buffer grows. Consider short-circuiting based on the incoming token (e.g., only scan when the new token contains ., !, or ?) or tracking the last scanned index to avoid rescanning the whole buffer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed — added a short-circuit check: the chunk 1+ path only calls _find_sentence_boundary when token.content contains ., !, or ?. Tokens without sentence-ending punctuation skip the scan entirely.
Generated by Claude Code
Copilot
AI
Apr 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The chunk 1+ short-circuit (continue when the incoming token has no .?!) can prevent synthesis from ever triggering on an already-present sentence boundary. Example: after a . arrives you may skip enqueuing because remaining > threshold; subsequent tokens without punctuation will continue and never re-check remaining for that existing boundary, which can delay synthesis until the next punctuation and increase stall risk. Consider tracking a pending boundary index and re-evaluating the remaining <= threshold condition on subsequent tokens (even if the token has no punctuation), or only skipping the buffer scan while still checking a previously found boundary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Replaced the hard continue with a pending_boundary cache. The buffer scan only runs when the new token contains .!?, but the timing condition (remaining <= threshold) is re-evaluated on every token against the cached boundary. Once consumed, pending_boundary resets to -1.
Generated by Claude Code
Copilot
AI
Apr 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When chunk 0 has been queued for synthesis but no audio has been produced/queued yet, _remaining_playback_time() will return 0.0, which makes remaining <= threshold true and can cause chunk 1+ to be enqueued as soon as a sentence boundary appears—before there is any playback window to compare against. If the intent is to wait until there is actual queued/playing audio to keep chunks larger, gate the chunk 1+ logic on state["playback_active"] or state["queued_durations"] being non-empty (or seed an initial estimated playback window).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Added a has_audio flag to the shared state that is set to True once the first synthesis result is queued. The chunk 1+ path now skips until has_audio is true, preventing premature firing when remaining==0 just because no audio exists yet.
Generated by Claude Code
Copilot
AI
Apr 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The “Edge case” guard is currently unreachable: it’s nested under if remaining <= threshold: but then checks remaining > threshold * 2, which can never be true (threshold is positive). As a result, the intended behavior of holding back very short chunks when there’s still plenty of playback window won’t ever happen. Consider revising the condition/placement so the short-chunk suppression can actually trigger (or remove the dead code if it’s not needed).
| if remaining <= threshold: | |
| to_speak = buffer[:pending_boundary + 1].strip() | |
| # Edge case: chunk 0 still playing with time to spare | |
| # and buffer text is very short – hold for more tokens | |
| if remaining > threshold * 2 and len(to_speak) < min_chunk_size: | |
| continue | |
| to_speak = buffer[:pending_boundary + 1].strip() | |
| # Edge case: chunk 0 still playing with time to spare | |
| # and buffer text is very short – hold for more tokens | |
| if remaining > threshold * 2 and len(to_speak) < min_chunk_size: | |
| continue | |
| if remaining <= threshold: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already fixed in f275fe6 — moved the short-chunk suppression guard before the if remaining <= threshold: block, exactly as suggested.
Generated by Claude Code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_get_audio_duration()callspygame.mixer.Sound(...)from the synthesis worker thread while playback concurrently usespygame.mixer.musicin the playback worker. This introduces concurrent pygame mixer calls from multiple threads, which can be unstable depending on the SDL_mixer backend. To avoid cross-thread mixer access, consider computing duration without pygame (e.g., parse MP3 headers with a pure-Python library) or move duration measurement into the playback thread (so all pygame calls happen on one thread).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed — removed
pygame.mixer.Soundfrom the synthesis thread entirely. Renamed to_estimate_audio_duration(file-size based, thread-safe). The accurate duration is now measured viapygame.mixer.Soundin the playback worker right before playback, keeping all pygame/SDL_mixer calls on a single thread.Generated by Claude Code