Improve TTS streaming chunking for better prosody and gapless playback#3
Conversation
Chunk 0 fires at the first punctuation boundary (fast start), while subsequent chunks accumulate tokens and trigger synthesis based on a rolling-average synthesis time vs remaining playback window. This maximises text per synthesis call (better Chirp3-HD prosody) while keeping playback gapless. Key changes in speak_streaming_async: - Two-phase chunking: fast-start chunk 0 + timing-aware chunk 1+ - Rolling average synthesis time (seeded at 400ms) drives trigger - Playback duration tracking via _get_audio_duration helper - Stall detection with logged warnings for self-correcting average - All state is per-call (reset each invocation) https://claude.ai/code/session_01E7isyc2DwdLiHppMT8gvA1
There was a problem hiding this comment.
Pull request overview
This PR updates speak_streaming_async to improve streaming TTS chunking so playback starts faster while later chunks are larger (for better prosody) and timed to avoid gaps.
Changes:
- Adds two-phase chunking: a fast-start initial chunk followed by timing-aware sentence chunks.
- Tracks synthesis time and estimated queued playback duration to decide when to trigger the next synthesis call.
- Adds
_get_audio_duration()helper to estimate audio duration per synthesized chunk.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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. | ||
|
|
||
| :param text_generator: Generator that yields text tokens | ||
| :param chunk_on: Character to chunk on (default: "." for sentences) | ||
| :param chunk_on: Character to chunk on for chunk 0 (default: ".") |
There was a problem hiding this comment.
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.
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
| def _update_avg(duration: float): | ||
| nonlocal avg_synthesis_time | ||
| synthesis_times.append(duration) | ||
| avg_synthesis_time = sum(synthesis_times) / len(synthesis_times) | ||
|
|
There was a problem hiding this comment.
_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.
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
| def _remaining_playback_time() -> float: | ||
| """Total seconds of audio still to play (current + queued).""" | ||
| with state_lock: | ||
| remaining = 0.0 | ||
| if state["playback_active"]: | ||
| elapsed = time.monotonic() - state["play_start"] | ||
| remaining = max(0.0, state["play_duration"] - elapsed) | ||
| remaining += sum(state["queued_durations"]) | ||
| return remaining |
There was a problem hiding this comment.
_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.
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
| else: | ||
| # --- Chunk 1+: quality chunks – sentence boundaries only --- | ||
| sentence_boundary = self._find_sentence_boundary(buffer, ".!?") | ||
| if sentence_boundary < 0: | ||
| continue | ||
|
|
||
| remaining = _remaining_playback_time() | ||
| threshold = avg_synthesis_time + 0.2 # 200 ms margin | ||
|
|
||
| if remaining <= threshold: | ||
| to_speak = buffer[:sentence_boundary + 1].strip() | ||
|
|
There was a problem hiding this comment.
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.
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
| pygame.mixer.music.load(audio_file) | ||
| pygame.mixer.music.play() | ||
|
|
||
| # Wait for playback to finish | ||
|
|
||
| while pygame.mixer.music.get_busy(): | ||
| time.sleep(0.1) | ||
|
|
||
| # Clean up the temporary file | ||
| time.sleep(0.05) | ||
|
|
||
| last_end = time.monotonic() | ||
|
|
||
| with state_lock: | ||
| state["playback_active"] = False | ||
|
|
||
| try: | ||
| os.remove(audio_file) | ||
| except: | ||
| except Exception: | ||
| pass | ||
| except Exception as e: | ||
| print(f"\n❌ Playback error: {e}") | ||
|
|
||
| with state_lock: | ||
| state["playback_active"] = False | ||
|
|
There was a problem hiding this comment.
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.
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
- Fix chunk_on default to ",.!?" so chunk 0 fires on commas as documented - Replace unbounded synthesis_times list with O(1) incremental mean - Replace queued_durations list with a single queued_total float for O(1) remaining-time computation - Gate chunk 1+ trigger on has_audio flag so we don't fire prematurely before chunk 0 synthesis completes - Move playback temp-file cleanup and state reset into finally block to prevent file leaks on exceptions https://claude.ai/code/session_01E7isyc2DwdLiHppMT8gvA1
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if chunk_index == 0: | ||
| # --- Chunk 0: fast start – fire at first boundary (incl. commas) --- | ||
| if any(c in buffer for c in chunk_on): | ||
| last_chunk_idx = self._find_sentence_boundary(buffer, chunk_on) | ||
| if last_chunk_idx >= 0: | ||
| to_speak = buffer[:last_chunk_idx + 1].strip() | ||
| delimiter = buffer[last_chunk_idx] | ||
| effective_min = 5 if delimiter in '.!?' else min_chunk_size |
There was a problem hiding this comment.
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.
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
| def _update_avg(duration: float): | ||
| nonlocal avg_synthesis_time, synthesis_count, synthesis_sum | ||
| synthesis_count += 1 | ||
| synthesis_sum += duration | ||
| avg_synthesis_time = synthesis_sum / synthesis_count | ||
|
|
There was a problem hiding this comment.
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.
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
| else: | ||
| # --- Chunk 1+: quality chunks – sentence boundaries only --- | ||
| sentence_boundary = self._find_sentence_boundary(buffer, ".!?") | ||
| if sentence_boundary < 0: | ||
| continue |
There was a problem hiding this comment.
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.
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
| with tempfile.NamedTemporaryFile(suffix=".mp3", delete=False) as temp_file: | ||
| temp_filename = temp_file.name | ||
| # Synthesize speech to the temp file | ||
|
|
||
| t0 = time.monotonic() | ||
| audio_file = self.synthesize_to_file(text_to_speak, temp_filename) | ||
| synth_dur = time.monotonic() - t0 | ||
|
|
||
| if audio_file: | ||
| playback_queue.put(audio_file) | ||
| audio_dur = self._get_audio_duration(audio_file) | ||
| _update_avg(synth_dur) | ||
|
|
There was a problem hiding this comment.
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.
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
- Chunk 0 now uses first_only=True boundary search so synthesis fires at the earliest valid boundary, minimising time-to-first-audio - Moved synthesis timing state (count/sum/avg) into shared state dict under state_lock to eliminate cross-thread data race - Short-circuit chunk 1+ boundary scan: skip full buffer rescan when the incoming token contains no sentence-ending punctuation (.!?) - Clean up temp MP3 file when synthesis returns falsy or throws, preventing file leaks on quota blocks or errors https://claude.ai/code/session_01E7isyc2DwdLiHppMT8gvA1
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # --- Chunk 1+: quality chunks – sentence boundaries only --- | ||
| # Short-circuit: skip the full buffer scan when the new | ||
| # token contains no sentence-ending punctuation. | ||
| if not any(c in token.content for c in ".!?"): | ||
| continue | ||
| sentence_boundary = self._find_sentence_boundary(buffer, ".!?") | ||
| if sentence_boundary < 0: | ||
| continue |
There was a problem hiding this comment.
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.
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
| 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() |
There was a problem hiding this comment.
_get_audio_duration() calls pygame.mixer.Sound(...) from the synthesis worker thread while playback concurrently uses pygame.mixer.music in 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.
Fixed — removed pygame.mixer.Sound from the synthesis thread entirely. Renamed to _estimate_audio_duration (file-size based, thread-safe). The accurate duration is now measured via pygame.mixer.Sound in the playback worker right before playback, keeping all pygame/SDL_mixer calls on a single thread.
Generated by Claude Code
- Chunk 1+ now caches `pending_boundary` and re-evaluates the remaining-time condition on every token, even when the token has no punctuation. Only the buffer scan is skipped (not the timing check), so an existing boundary fires as soon as the window closes. - Replaced `_get_audio_duration` (which called pygame.mixer.Sound from the synthesis thread) with `_estimate_audio_duration` (file-size only, thread-safe). Accurate duration via pygame.mixer.Sound is now measured in the playback worker, keeping all pygame/SDL_mixer calls on a single thread. https://claude.ai/code/session_01E7isyc2DwdLiHppMT8gvA1
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| :param text_generator: Generator that yields text tokens | ||
| :param chunk_on: Character to chunk on (default: "." for sentences) | ||
| :param chunk_on: Character to chunk on for chunk 0 (default: ".") |
There was a problem hiding this comment.
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.
Fixed — updated the docstring to Characters used to detect chunk 0 boundaries only (default: ",.!?").
Generated by Claude Code
| with state_lock: | ||
| # Correct queued_total: remove the estimate, the | ||
| # play_duration field now uses the real value. | ||
| state["queued_total"] = max(0.0, state["queued_total"] - estimated_dur) | ||
| state["play_start"] = time.monotonic() | ||
| state["play_duration"] = actual_dur | ||
| state["playback_active"] = True |
There was a problem hiding this comment.
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.
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
| @staticmethod | ||
| def _estimate_audio_duration(file_path: str) -> float: | ||
| """Estimate the duration of an MP3 file from its size. | ||
|
|
||
| Uses ~32 kbps (4 KB/s), which is typical for Google Cloud TTS MP3 | ||
| output. This avoids calling any pygame API so it is safe to use | ||
| from any thread. |
There was a problem hiding this comment.
PR description mentions a _get_audio_duration helper, but the implementation introduces _estimate_audio_duration. If _get_audio_duration is referenced elsewhere (docs, callers, or future work), this naming mismatch can cause confusion. Consider aligning the helper name with the PR description or updating the description to match the code.
There was a problem hiding this comment.
Fair point — the PR description was written before the rename. The method was originally _get_audio_duration but was renamed to _estimate_audio_duration in a later commit to reflect that it now only does file-size estimation (no pygame). This is a description-vs-code mismatch, not a code issue. I'll update the PR description to match.
Generated by Claude Code
- Update chunk_on docstring to reflect actual default ",.!?" - Move play_start assignment to after pygame.mixer.music.play() so elapsed-time calculations reflect real playback, not load latency https://claude.ai/code/session_01E7isyc2DwdLiHppMT8gvA1
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # --- Chunk 0: fast start – fire at first boundary (incl. commas) --- | ||
| if any(c in buffer for c in chunk_on): | ||
| first_idx = find_sentence_boundary(buffer, chunk_on, first_only=True) | ||
| if first_idx >= 0: | ||
| to_speak = buffer[:first_idx + 1].strip() | ||
| delimiter = buffer[first_idx] | ||
| effective_min = 5 if delimiter in '.!?' else min_chunk_size | ||
| if len(to_speak) >= effective_min: | ||
| buffer = buffer[first_idx + 1:] | ||
| synthesis_queue.put(to_speak) | ||
| chunk_index += 1 |
There was a problem hiding this comment.
Chunk 0 uses first_only=True, so if the earliest valid boundary yields a chunk shorter than effective_min, the code never advances past that boundary (each loop iteration will keep re-selecting the same first_idx and to_speak), potentially delaying time-to-first-audio indefinitely. Consider selecting the earliest boundary that also satisfies the minimum-length requirement (e.g., iterate over boundaries until one meets effective_min, or fall back to the last boundary when the first is too short).
| # --- Chunk 0: fast start – fire at first boundary (incl. commas) --- | |
| if any(c in buffer for c in chunk_on): | |
| first_idx = find_sentence_boundary(buffer, chunk_on, first_only=True) | |
| if first_idx >= 0: | |
| to_speak = buffer[:first_idx + 1].strip() | |
| delimiter = buffer[first_idx] | |
| effective_min = 5 if delimiter in '.!?' else min_chunk_size | |
| if len(to_speak) >= effective_min: | |
| buffer = buffer[first_idx + 1:] | |
| synthesis_queue.put(to_speak) | |
| chunk_index += 1 | |
| # --- Chunk 0: fast start – fire at the earliest acceptable boundary (incl. commas) --- | |
| if any(c in buffer for c in chunk_on): | |
| selected_idx = -1 | |
| selected_text = "" | |
| search_start = 0 | |
| while search_start < len(buffer): | |
| next_idx = find_sentence_boundary(buffer[search_start:], chunk_on, first_only=True) | |
| if next_idx < 0: | |
| break | |
| boundary_idx = search_start + next_idx | |
| candidate_text = buffer[:boundary_idx + 1].strip() | |
| delimiter = buffer[boundary_idx] | |
| effective_min = 5 if delimiter in '.!?' else min_chunk_size | |
| if len(candidate_text) >= effective_min: | |
| selected_idx = boundary_idx | |
| selected_text = candidate_text | |
| break | |
| search_start = boundary_idx + 1 | |
| if selected_idx >= 0: | |
| buffer = buffer[selected_idx + 1:] | |
| synthesis_queue.put(selected_text) | |
| chunk_index += 1 |
There was a problem hiding this comment.
Good catch — chunk 0 now iterates over boundaries (calling find_sentence_boundary on successive sub-slices) until it finds one whose candidate text meets effective_min. This avoids the infinite re-selection of a too-short first boundary.
Generated by Claude Code
| if len(to_speak) >= effective_min: | ||
| buffer = buffer[first_idx + 1:] | ||
| synthesis_queue.put(to_speak) | ||
| chunk_index += 1 |
There was a problem hiding this comment.
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.
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
| * **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. |
There was a problem hiding this comment.
Docstring says chunk 1+ triggers at the first sentence-ending boundary where the remaining playback window is small enough, but the implementation uses _find_sentence_boundary(...) / find_sentence_boundary(...) which returns the last valid boundary by default. Please align the docstring with the actual behavior, or change the boundary selection to match the documented “first boundary” semantics.
| * **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.
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
| fast-start chunking). Otherwise the last valid boundary is returned | ||
| (maximises chunk length for better prosody). |
There was a problem hiding this comment.
find_sentence_boundary(..., first_only=True) changes boundary-selection behavior in a way that’s easy to regress (and affects time-to-first-audio). Since this function is pure/deterministic, adding unit tests covering both first_only=True and the default “last boundary” behavior (including commas/abbrev/decimal cases) would help prevent future regressions without requiring any external TTS/audio dependencies.
| fast-start chunking). Otherwise the last valid boundary is returned | |
| (maximises chunk length for better prosody). | |
| fast-start chunking). Otherwise the last valid boundary is returned | |
| (maximises chunk length for better prosody). | |
| The examples below act as regression coverage for the boundary-selection | |
| behavior and common edge cases: | |
| >>> text = "Hello world. Another sentence!" | |
| >>> find_sentence_boundary(text) | |
| 29 | |
| >>> find_sentence_boundary(text, first_only=True) | |
| 11 | |
| Decimals should not be split at the period: | |
| >>> find_sentence_boundary("The value is 3.14 and rising.") | |
| 28 | |
| >>> find_sentence_boundary("Version 2.0") | |
| -1 | |
| Initials/abbreviations should stay connected rather than treating the first | |
| period as a sentence boundary: | |
| >>> find_sentence_boundary("Meet J. T. Smith.") | |
| 16 | |
| >>> find_sentence_boundary("Meet J. T. Smith.", first_only=True) | |
| 16 | |
| When commas are enabled as chunk characters, weak commas are ignored and | |
| stronger commas still participate in first-vs-last selection: | |
| >>> comma_text = "Alpha, beta gamma delta epsilon zeta, final stop." | |
| >>> find_sentence_boundary(comma_text, chunk_chars=",.!?") | |
| 50 | |
| >>> find_sentence_boundary(comma_text, chunk_chars=",.!?", first_only=True) | |
| 37 | |
| >>> find_sentence_boundary("Count 1, 2, 3", chunk_chars=",") | |
| -1 |
There was a problem hiding this comment.
Added doctests covering both first_only=True and default (last boundary), plus edge cases for decimals, initials/abbreviations, and weak commas. All 8 doctests pass via python -m doctest.
Generated by Claude Code
…dd doctests - Chunk 0 now iterates over boundaries to find the first one meeting the min-length requirement, avoiding infinite stall on short first boundaries - After chunk 0 fires, immediately scan the remainder for sentence- ending boundaries and seed pending_boundary so chunk 1+ doesn't wait for a future token containing punctuation - Align chunk 1+ docstring: triggers at the *last* available sentence boundary (not "first") to maximise chunk length - Add doctests to find_sentence_boundary covering first_only, decimals, initials/abbreviations, and weak commas https://claude.ai/code/session_01E7isyc2DwdLiHppMT8gvA1
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fast-start chunking). Otherwise the last valid boundary is returned | ||
| (maximises chunk length for better prosody). | ||
|
|
||
| >>> text = "Hello world. Another sentence!" | ||
| >>> find_sentence_boundary(text) | ||
| 29 | ||
| >>> find_sentence_boundary(text, first_only=True) | ||
| 11 | ||
|
|
||
| Decimals should not be split at the period: | ||
|
|
||
| >>> find_sentence_boundary("The value is 3.14 and rising.") | ||
| 28 | ||
| >>> find_sentence_boundary("Version 2.0") | ||
| -1 | ||
|
|
||
| Initials / abbreviations stay connected: | ||
|
|
||
| >>> find_sentence_boundary("Meet J. T. Smith.") | ||
| 16 | ||
| >>> find_sentence_boundary("Meet J. T. Smith.", first_only=True) | ||
| 16 | ||
|
|
||
| When commas are enabled, weak commas are skipped: | ||
|
|
||
| >>> find_sentence_boundary("Count 1, 2, 3", chunk_chars=",") | ||
| -1 |
There was a problem hiding this comment.
The new doctest examples in this docstring won’t be executed by the current CI/test configuration (CI runs python -m pytest -q and pyproject.toml doesn’t enable --doctest-modules). That means the first_only behavior and edge cases can regress without any automated signal. Consider either enabling doctest collection in pytest (e.g., add --doctest-modules in [tool.pytest.ini_options].addopts) or moving these cases into an explicit tests/test_chunking.py unit test.
| fast-start chunking). Otherwise the last valid boundary is returned | |
| (maximises chunk length for better prosody). | |
| >>> text = "Hello world. Another sentence!" | |
| >>> find_sentence_boundary(text) | |
| 29 | |
| >>> find_sentence_boundary(text, first_only=True) | |
| 11 | |
| Decimals should not be split at the period: | |
| >>> find_sentence_boundary("The value is 3.14 and rising.") | |
| 28 | |
| >>> find_sentence_boundary("Version 2.0") | |
| -1 | |
| Initials / abbreviations stay connected: | |
| >>> find_sentence_boundary("Meet J. T. Smith.") | |
| 16 | |
| >>> find_sentence_boundary("Meet J. T. Smith.", first_only=True) | |
| 16 | |
| When commas are enabled, weak commas are skipped: | |
| >>> find_sentence_boundary("Count 1, 2, 3", chunk_chars=",") | |
| -1 | |
| fast-start chunking). Otherwise the last valid boundary is returned | |
| (maximises chunk length for better prosody). | |
| Example: | |
| text = "Hello world. Another sentence!" | |
| find_sentence_boundary(text) -> 29 | |
| find_sentence_boundary(text, first_only=True) -> 11 | |
| Decimals should not be split at the period: | |
| find_sentence_boundary("The value is 3.14 and rising.") -> 28 | |
| find_sentence_boundary("Version 2.0") -> -1 | |
| Initials / abbreviations stay connected: | |
| find_sentence_boundary("Meet J. T. Smith.") -> 16 | |
| find_sentence_boundary("Meet J. T. Smith.", first_only=True) -> 16 | |
| When commas are enabled, weak commas are skipped: | |
| find_sentence_boundary("Count 1, 2, 3", chunk_chars=",") -> -1 |
There was a problem hiding this comment.
Good point. Created tests/tts/test_chunking.py with 11 pytest test cases covering first_only, last boundary, decimals, initials/abbreviations, weak commas, and edge cases. These run in CI via the standard pytest invocation — no need to enable --doctest-modules globally. The doctests remain in the docstring as inline documentation.
Generated by Claude Code
Addresses Copilot feedback that doctests won't run in CI without --doctest-modules. Creates tests/tts/test_chunking.py with 11 pytest cases covering first_only, last boundary, decimals, initials, abbreviations, weak commas, and edge cases (empty string, no boundary). https://claude.ai/code/session_01E7isyc2DwdLiHppMT8gvA1
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| with state_lock: | ||
| # Correct queued_total: remove the estimate, the | ||
| # play_duration field now uses the real value. | ||
| state["queued_total"] = max(0.0, state["queued_total"] - estimated_dur) | ||
| state["play_duration"] = actual_dur | ||
| state["playback_active"] = True | ||
|
|
There was a problem hiding this comment.
playback_active is set to True before play_start is updated (it remains 0.0 until the later lock block after .play()). During that window, _remaining_playback_time() can compute elapsed = time.monotonic() - 0.0 and treat the current chunk as already fully elapsed, temporarily driving remaining to 0 and potentially triggering chunk 1+ too early. Consider updating play_start and setting playback_active=True in the same locked state update (after .play()), or have _remaining_playback_time() treat play_start==0.0 as “not started yet” even if playback_active is true.
There was a problem hiding this comment.
Good catch. Moved playback_active = True into the same lock acquisition as play_start, both set after .play() returns. The earlier lock block now only updates queued_total and play_duration, so _remaining_playback_time() never sees playback_active=True with a stale play_start.
Generated by Claude Code
Set playback_active=True and play_start together in a single lock acquisition after .play() returns, so _remaining_playback_time() never sees playback_active=True with a stale play_start of 0.0. https://claude.ai/code/session_01E7isyc2DwdLiHppMT8gvA1
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| with state_lock: | ||
| # Correct queued_total: remove the estimate, the | ||
| # play_duration field now uses the real value. | ||
| state["queued_total"] = max(0.0, state["queued_total"] - estimated_dur) | ||
| state["play_duration"] = actual_dur | ||
|
|
||
| pygame.mixer.music.load(audio_file) | ||
| pygame.mixer.music.play() | ||
|
|
||
| # Wait for playback to finish | ||
|
|
||
| # Set play_start and playback_active together after | ||
| # .play() so _remaining_playback_time() never sees | ||
| # playback_active=True with a stale play_start. | ||
| with state_lock: | ||
| state["play_start"] = time.monotonic() | ||
| state["playback_active"] = True |
There was a problem hiding this comment.
_remaining_playback_time() is documented as “current + queued”, but in playback_worker the current chunk’s estimate is removed from state["queued_total"] before state["playback_active"] becomes True (it’s only set after pygame.mixer.music.play()). During the load()/play() window, remaining playback time can be undercounted (often to ~0 when the queue was otherwise empty), which can cause chunk 1+ to trigger earlier than intended and reduce chunk sizes/prosody. Consider deferring the queued_total -= estimated_dur adjustment until the same lock block where play_start/playback_active are set (or keep the estimate in queued_total until playback is marked active, or add a separate “loading_duration” included in remaining-time calculations).
There was a problem hiding this comment.
Fixed in 5e0eaa1 — queued_total is now decremented immediately on dequeue (before the try block), and play_duration is set at the same time. _remaining_playback_time() includes play_duration even when playback_active is False (during the load/play window), so the chunk is always accounted for.
Generated by Claude Code
Defer the queued_total decrement until the same lock block where playback_active and play_start are set, so _remaining_playback_time() never sees a gap where the loading chunk is unaccounted for. https://claude.ai/code/session_01E7isyc2DwdLiHppMT8gvA1
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| with state_lock: | ||
| state["queued_total"] = max(0.0, state["queued_total"] - estimated_dur) | ||
| state["play_duration"] = actual_dur | ||
| state["play_start"] = time.monotonic() | ||
| state["playback_active"] = True |
There was a problem hiding this comment.
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.
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
Decrement queued_total immediately on dequeue and track the chunk via play_duration. _remaining_playback_time() now accounts for the chunk during loading (play_duration when !playback_active) and during playback (play_duration - elapsed). play_duration is reset in finally so it never lingers after a chunk finishes or fails. https://claude.ai/code/session_01E7isyc2DwdLiHppMT8gvA1
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| 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 | ||
|
|
There was a problem hiding this comment.
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.
Already fixed in f275fe6 — moved the short-chunk suppression guard before the if remaining <= threshold: block, exactly as suggested.
Generated by Claude Code
Move the edge-case check (hold short chunks when remaining playback time is ample) before the `remaining <= threshold` gate so it can actually trigger. https://claude.ai/code/session_01E7isyc2DwdLiHppMT8gvA1
Chunk 0 fires at the first punctuation boundary (fast start), while
subsequent chunks accumulate tokens and trigger synthesis based on a
rolling-average synthesis time vs remaining playback window. This
maximises text per synthesis call (better Chirp3-HD prosody) while
keeping playback gapless.
Key changes in speak_streaming_async:
https://claude.ai/code/session_01E7isyc2DwdLiHppMT8gvA1