Skip to content

Commit a910f2e

Browse files
shaypal5claude
andauthored
fix(tts): #83 partial — revert inter-word breaks (low-intensity scenes only) (#86)
* Revert "fix(tts): Azure SSML parsing error on adjacent break elements (#67) (#71)" This reverts commit 0bdb217. * Revert "fix(tts): insert inter-word <break> tags to prevent Hebrew word merging (#70)" This reverts commit d0c273b. * restore(tts): reinstate hardenings from #71 unrelated to #70 + add #83 regression test The initial revert in this branch (revert PR #70 + PR #71 wholesale) was too aggressive: PR #71 bundled three independent hardenings, only one of which was caused by PR #70. This commit restores the two hardenings that have nothing to do with inter-word break injection, and narrows the third to the residual case that survives #70's revert. Also adds the #83 regression test that pins the bisect finding. Restored from #71 (and explicitly verified to NOT re-introduce #83): - `_sanitize_text` + `_XML_INVALID_CHARS_RE` regex. Defends against an LLM emitting XML 1.0 control characters that otherwise make the SSML unparseable by Azure. Independent bug class from per-word breaks. - Azure-range prosody clamping in `_semitones_to_percent`, `_rate_to_string`, `_volume_to_string`, plus warning logs on clamp activation. `speaker_BYS_F_6-10_001.yaml` ships `pitch_delta_st=+9` → +54% unclamped, which Azure rejects. Independent bug class. - Adjacent `<break>` merging in `_apply_phrase_prosody`, narrowed to the phrase-after / phrase-before case (the only adjacent-break source that survives #70's revert). The original #71 logic also had a word-break branch that is no longer reachable. Added: - `test_no_per_word_breaks_in_default_ssml` regression test pinned to #83. The default multi-word SSML must not contain `<break>` tags; per-word break injection (PR #70) tripped Whisper's silence-detection heuristic and produced the WER regression. Any future Hebrew word- merge mitigation (#62) must not re-introduce per-word breaks. - `test_text_with_invalid_xml_chars_sanitized`, `test_prosody_pitch_clamped_to_azure_range`, `test_prosody_rate_clamped_to_azure_range`, `test_prosody_volume_clamped_to_azure_range`, `test_adjacent_phrase_breaks_are_merged` — pin the restored hardenings. All three of these were independently flagged by Copilot's review on this PR (resolves three Copilot review threads). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 588cf05 commit a910f2e

3 files changed

Lines changed: 92 additions & 546 deletions

File tree

‎synthbanshee/tts/ssml_builder.py‎

Lines changed: 25 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@
66
``supports_style_tags=False``)
77
- <prosody> elements for rate, pitch, and volume control
88
- Nested per-phrase <prosody> + <break> elements (M2b)
9-
- Inter-word ``<break time="50ms"/>`` elements to prevent Hebrew word
10-
merging (#62)
119
1210
Azure SSML reference:
1311
https://learn.microsoft.com/en-us/azure/ai-services/speech-service/speech-synthesis-markup-voice
@@ -28,12 +26,6 @@
2826
_MSTTS_XMLNS = "http://www.w3.org/2001/mstts"
2927
_SPEAK_LANG = "he-IL"
3028

31-
# Inter-word break duration in milliseconds. 50 ms is the initial estimate
32-
# for signalling a word boundary to Azure / Google he-IL without introducing
33-
# an audible pause. This value needs empirical validation with real TTS
34-
# output — it may need per-provider tuning if engines respond differently.
35-
_WORD_BREAK_MS = 50
36-
3729
# Azure prosody attribute limits (documented ranges).
3830
_AZURE_RATE_MIN_PCT = -50 # rate="-50%" → 0.5x
3931
_AZURE_RATE_MAX_PCT = 200 # rate="+200%" → 3.0x
@@ -57,74 +49,6 @@ def _sanitize_text(text: str) -> str:
5749
return _XML_INVALID_CHARS_RE.sub("", text)
5850

5951

60-
def _inject_word_breaks(
61-
parent: ET.Element,
62-
text: str,
63-
after: ET.Element | None = None,
64-
) -> ET.Element | None:
65-
"""Insert *text* into *parent* with ``<break>`` tags between words.
66-
67-
Hebrew TTS engines (Azure he-IL, Google Chirp) merge adjacent words into
68-
unintelligible speech when no explicit boundary cue exists. This function
69-
splits *text* on whitespace and inserts a short ``<break>`` element between
70-
every pair of consecutive words.
71-
72-
Args:
73-
parent: The XML element to add content to.
74-
text: The text to inject (may contain multi-word Hebrew).
75-
after: If provided, the first text chunk is appended to
76-
``after.tail`` instead of ``parent.text``.
77-
78-
Returns:
79-
The last child element added to *parent*, or *after* if no ``<break>``
80-
elements were created (single word or empty text).
81-
"""
82-
if not text or not text.strip():
83-
# Pure whitespace or empty — append as-is without inserting breaks.
84-
if text:
85-
if after is None:
86-
parent.text = (parent.text or "") + text
87-
else:
88-
after.tail = (after.tail or "") + text
89-
return after
90-
91-
words = text.split()
92-
last = after
93-
94-
# Preserve any leading whitespace (e.g. space before a text fragment
95-
# that follows a <break> or <prosody> element).
96-
leading = text[: len(text) - len(text.lstrip())]
97-
if leading:
98-
if last is None:
99-
parent.text = (parent.text or "") + leading
100-
else:
101-
last.tail = (last.tail or "") + leading
102-
103-
for i, word in enumerate(words):
104-
if i == 0:
105-
# First word: append directly (no break needed before it).
106-
if last is None:
107-
parent.text = (parent.text or "") + word
108-
else:
109-
last.tail = (last.tail or "") + word
110-
else:
111-
# Subsequent words: insert <break/> then the word with a
112-
# leading space in the tail to preserve normal spacing.
113-
brk = ET.SubElement(parent, "break", attrib={"time": f"{_WORD_BREAK_MS}ms"})
114-
brk.tail = " " + word
115-
last = brk
116-
117-
# Preserve any trailing whitespace.
118-
trailing = text[len(text.rstrip()) :]
119-
if trailing:
120-
if last is None:
121-
parent.text = (parent.text or "") + trailing
122-
else:
123-
last.tail = (last.tail or "") + trailing
124-
125-
return last
126-
127-
12852
@dataclass
12953
class UtteranceSpec:
13054
"""Specifies one TTS utterance to be included in an SSML document."""
@@ -188,10 +112,13 @@ def _apply_phrase_prosody(
188112
189113
Splits *text* around phrase spans and wraps each phrase in a nested
190114
``<prosody>`` element with optional ``<break time="…"/>`` elements
191-
inserted before and/or after. Inter-word ``<break>`` elements are
192-
inserted within each text fragment to prevent Hebrew word merging (#62).
193-
Overlapping spans are skipped (the span whose ``char_start`` falls
194-
before the previous span's ``char_end`` is silently dropped).
115+
inserted before and/or after. Adjacent ``<break>`` elements are merged
116+
(durations summed) to avoid Azure SSML parser error 0x80045003 (#67):
117+
when a phrase carries ``break_after_ms`` and the next phrase carries
118+
``break_before_ms``, two ``<break>`` siblings would otherwise appear
119+
back-to-back. Overlapping spans are skipped (the span whose
120+
``char_start`` falls before the previous span's ``char_end`` is silently
121+
dropped).
195122
196123
Args:
197124
parent: The XML element that will receive the mixed text/element content.
@@ -202,16 +129,26 @@ def _apply_phrase_prosody(
202129
prev: ET.Element | None = None
203130

204131
def _append_text(s: str) -> None:
205-
"""Append *s* with inter-word ``<break>`` elements."""
206132
nonlocal prev
207133
if not s:
208134
return
209-
result = _inject_word_breaks(parent, s, after=prev)
210-
if result is not None:
211-
prev = result
135+
if prev is None:
136+
parent.text = (parent.text or "") + s
137+
else:
138+
prev.tail = (prev.tail or "") + s
212139

213140
def _append_break(ms: int) -> ET.Element:
141+
"""Add a ``<break time="{ms}ms"/>`` or merge into the preceding break.
142+
143+
Azure rejects adjacent ``<break>`` siblings (#67); when the most
144+
recently appended element is itself a ``<break>``, we sum the
145+
durations into the existing element instead of creating a new one.
146+
"""
214147
nonlocal prev
148+
if prev is not None and prev.tag == "break":
149+
prev_ms = int(prev.attrib.get("time", "0ms").replace("ms", ""))
150+
prev.attrib["time"] = f"{prev_ms + ms}ms"
151+
return prev
215152
el = ET.SubElement(parent, "break", attrib={"time": f"{ms}ms"})
216153
prev = el
217154
return el
@@ -227,23 +164,9 @@ def _append_break(ms: int) -> ET.Element:
227164
if phrase.char_start > cursor:
228165
_append_text(text[cursor : phrase.char_start])
229166

230-
# Optional break before the phrase. Merge with the preceding
231-
# <break> element (if any) to avoid adjacent breaks that Azure's SSML
232-
# parser rejects with error 0x80045003 (#67).
167+
# Optional break before the phrase.
233168
if phrase.break_before_ms > 0:
234-
if prev is not None and prev.tag == "break":
235-
prev_time = prev.attrib.get("time", "0ms")
236-
prev_ms = int(prev_time.replace("ms", ""))
237-
if prev_ms == _WORD_BREAK_MS:
238-
# Word-boundary break: replace with the phrase break
239-
# (the phrase break subsumes the word-boundary intent).
240-
prev.attrib["time"] = f"{phrase.break_before_ms}ms"
241-
else:
242-
# Semantic break (e.g. break_after from prior phrase):
243-
# sum durations to preserve both intents.
244-
prev.attrib["time"] = f"{prev_ms + phrase.break_before_ms}ms"
245-
else:
246-
_append_break(phrase.break_before_ms)
169+
_append_break(phrase.break_before_ms)
247170

248171
# Phrase-level prosody wrapper (omitted when only breaks are requested).
249172
phrase_attrs: dict[str, str] = {}
@@ -257,7 +180,7 @@ def _append_break(ms: int) -> ET.Element:
257180
phrase_text = text[phrase.char_start : phrase.char_end]
258181
if phrase_attrs:
259182
pe = ET.SubElement(parent, "prosody", attrib=phrase_attrs)
260-
_inject_word_breaks(pe, phrase_text)
183+
pe.text = phrase_text
261184
prev = pe
262185
else:
263186
_append_text(phrase_text)
@@ -350,7 +273,7 @@ def build_multi(
350273
if utt.phrase_prosody:
351274
_apply_phrase_prosody(inner, utt_text, utt.phrase_prosody)
352275
else:
353-
_inject_word_breaks(inner, utt_text)
276+
inner.text = utt_text
354277

355278
raw = ET.tostring(speak, encoding="unicode", xml_declaration=False)
356279
return '<?xml version="1.0" encoding="UTF-8"?>\n' + raw

‎tests/unit/test_phrase_prosody.py‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -333,13 +333,8 @@ class TestApplyPhraseProsody:
333333
def test_no_phrases_sets_text(self) -> None:
334334
parent = _make_parent()
335335
_apply_phrase_prosody(parent, "hello world", [])
336-
# Inter-word <break> splits text: parent.text = "hello",
337-
# then <break time="50ms"/> with tail " world".
338-
assert parent.text == "hello"
339-
children = list(parent)
340-
assert len(children) == 1
341-
assert children[0].tag == "break"
342-
assert children[0].tail == " world"
336+
assert parent.text == "hello world"
337+
assert len(list(parent)) == 0 # no children
343338

344339
def test_single_phrase_mid_text(self) -> None:
345340
parent = _make_parent()
@@ -377,12 +372,8 @@ def test_zero_length_phrase_skipped(self) -> None:
377372
parent = _make_parent()
378373
phrase = PhraseProsody("p0", 3, 3) # zero-length span
379374
_apply_phrase_prosody(parent, "hello world", [phrase])
380-
# Zero-length phrase skipped → text set with inter-word breaks
381-
assert parent.text == "hello"
382-
children = list(parent)
383-
assert len(children) == 1
384-
assert children[0].tag == "break"
385-
assert children[0].tail == " world"
375+
# Zero-length phrase skipped → text is set as-is
376+
assert parent.text == "hello world"
386377

387378
def test_phrase_with_pitch_attribute(self) -> None:
388379
# Covers `if phrase.pitch is not None: phrase_attrs["pitch"] = ...` (line 113-114).

0 commit comments

Comments
 (0)