From 08e2ee8b85bbb7a6bdbe4d21adbed3150a0f80c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deniz=20=C5=9Eafak?= Date: Mon, 7 Sep 2026 17:29:27 +0300 Subject: [PATCH] fix(normalization): improve punctuation handling and spacing in text normalization fix(subtitles): enhance ellipsis and paragraph break handling in subtitle processing feat(gui): implement background update check for new versions test(tests): add tests for ellipsis handling and paragraph breaks preservation --- abogen/domain/split_pattern.py | 6 +- abogen/domain/subtitle_generation.py | 45 ++++++-- abogen/kokoro_text_normalization.py | 30 ++++-- abogen/pyqt/gui.py | 154 ++++++++++++++++----------- tests/test_regression_fixes.py | 7 ++ tests/test_subtitle_scenarios.py | 56 ++++++++++ 6 files changed, 219 insertions(+), 79 deletions(-) diff --git a/abogen/domain/split_pattern.py b/abogen/domain/split_pattern.py index 9f2bf5d..165b1db 100644 --- a/abogen/domain/split_pattern.py +++ b/abogen/domain/split_pattern.py @@ -5,10 +5,10 @@ from __future__ import annotations from abogen.domain.enums import Language, SubtitleMode # Canonical punctuation sets covering all supported scripts: -# ASCII (. ! ?), Arabic ؟, CJK (。!?), Devanagari । -PUNCTUATION_SENTENCE = r".!?؟。!?।" +# ASCII (. ! ?), ellipsis (…), Arabic ؟, CJK (。!?), Devanagari । +PUNCTUATION_SENTENCE = r".!?…؟。!?।" # Commas: ASCII , CJK fullwidth ,CJK ideographic 、 -PUNCTUATION_SENTENCE_COMMA = r".!?,?。!?،,、।" +PUNCTUATION_SENTENCE_COMMA = r".!?…,?。!?،,、।" PUNCTUATION_COMMAS = ",,、" diff --git a/abogen/domain/subtitle_generation.py b/abogen/domain/subtitle_generation.py index 2cd3d0d..ff48d26 100644 --- a/abogen/domain/subtitle_generation.py +++ b/abogen/domain/subtitle_generation.py @@ -153,7 +153,10 @@ def _process_karaoke_highlighting( if t.get("end") is not None and t.get("start") is not None else 0.5 ) - duration_cs = int(duration * 100) + try: + duration_cs = int(duration * 100) + except (ValueError, OverflowError, TypeError): + duration_cs = 50 # Add karaoke effect karaoke_text += f"{{\\kf{duration_cs}}}{t.get('text', '')}{t.get('whitespace', '') or ''}" @@ -174,7 +177,10 @@ def _process_karaoke_highlighting( karaoke_text = "" for t in current_sentence: duration = t["end"] - t["start"] if t.get("end") and t.get("start") else 0.5 - duration_cs = int(duration * 100) + try: + duration_cs = int(duration * 100) + except (ValueError, OverflowError, TypeError): + duration_cs = 50 karaoke_text += f"{{\\kf{duration_cs}}}{t.get('text', '')}{t.get('whitespace', '') or ''}" text_stripped = karaoke_text.strip() if text_stripped: @@ -230,6 +236,19 @@ def _process_spacy_sentences( set(sentence_boundaries + comma_positions) ) + # spaCy does not treat ellipsis ("...", "..", "…") as a sentence + # boundary ("Lorem ipsum... Lorem..." stays one sentence), so ellipsis + # runs followed by whitespace/end would merge into a single subtitle + # entry. Add explicit boundaries after them. Single dots ("Mr.") stay + # spaCy's responsibility so abbreviations don't regress. + for m in re.finditer(r"\.{2,}(?=[\s\"'”’»›)\]}]|$)|…(?=[\s\"'”’»›)\]}]|$)", full_text): + sentence_boundaries.append(m.end()) + # Double newlines are paragraph breaks: always split, even when spaCy + # sees no sentence boundary. + for m in re.finditer(r"\n{2,}", full_text): + sentence_boundaries.append(m.end()) + sentence_boundaries = sorted(set(sentence_boundaries)) + # Multi-sentence single FakeToken handling if len(tokens) == 1 and len(sentence_boundaries) > 1: single = tokens[0] @@ -257,7 +276,12 @@ def _process_spacy_sentences( if prev_pos < len(full_text): remainder = full_text[prev_pos:].strip() if remainder: - subtitle_entries.append((cur_start, end_time, remainder)) + remainder_end = end_time + if remainder_end is None: + remainder_end = fallback_end_time + if remainder_end is None: + remainder_end = cur_start + subtitle_entries.append((cur_start, remainder_end, remainder)) _apply_fallback_end_time(subtitle_entries, fallback_end_time) return @@ -379,15 +403,24 @@ def _process_regex_sentences( if len(parts) > 1: d = (end_time - start_time) if (end_time is not None and start_time is not None and end_time > start_time) else 0.0 total_len = max(len(sentence_text), 1) - cur_s = start_time + cur_s = start_time if start_time is not None else 0.0 for i, p in enumerate(parts): - e = end_time if i == len(parts) - 1 else cur_s + d * len(p) / total_len + if i == len(parts) - 1 and end_time is not None: + e = end_time + else: + e = cur_s + d * len(p) / total_len subtitle_entries.append((cur_s, e, p)) cur_s = e current_sentence = [] if current_sentence and sentence_text: - subtitle_entries.append((start_time, end_time, sentence_text)) + safe_start = start_time if start_time is not None else 0.0 + safe_end = end_time + if safe_end is None: + safe_end = fallback_end_time + if safe_end is None: + safe_end = safe_start + subtitle_entries.append((safe_start, safe_end, sentence_text)) # Fallback for last entry _apply_fallback_end_time(subtitle_entries, fallback_end_time) diff --git a/abogen/kokoro_text_normalization.py b/abogen/kokoro_text_normalization.py index 8d24231..4e7735e 100644 --- a/abogen/kokoro_text_normalization.py +++ b/abogen/kokoro_text_normalization.py @@ -704,7 +704,9 @@ def _cleanup_spacing(text: str) -> str: text = re.sub(rf"\s+([\"\'])([\s{_STANDARD_PUNCT_CLASS}{_CLOSING_PUNCT_CLASS}]|$)", r"\1\2", text) # Ensure spaces exist after sentence punctuation when followed by a word/quote. - text = re.sub(rf"([{_STANDARD_PUNCT_CLASS}])(?![\s{_CLOSING_PUNCT_CLASS}\"\'”’»›)])", r"\1 ", text) + # Runs of punctuation ("...", "?!?", "!!") must stay together: no space + # inside the run, only after it ("a...b" -> "a... b"). + text = re.sub(rf"([{_STANDARD_PUNCT_CLASS}])(?![\s{_STANDARD_PUNCT_CLASS}{_CLOSING_PUNCT_CLASS}\"\'”’»›)])", r"\1 ", text) # Ensure space after unambiguous closing quote when followed by a word (e.g. '”Next' -> '” Next') text = re.sub(rf"([{_CLOSING_PUNCT_CLASS}])(?![\s{_STANDARD_PUNCT_CLASS}{_CLOSING_PUNCT_CLASS}\"\'”’»›)])", r"\1 ", text) # Straight double quote closing (preceded by non-whitespace) followed directly by a word/number/opening @@ -715,8 +717,10 @@ def _cleanup_spacing(text: str) -> str: # Tighten hyphen/em dash spacing between word characters. text = re.sub(r"(?<=\w)\s*([-–—])\s*(?=\w)", r"\1", text) - # Normalize multiple spaces. - text = re.sub(r"\s{2,}", " ", text) + # Normalize multiple spaces, preserving paragraph breaks (double + # newlines must survive so the TTS engine can split on them). + text = re.sub(r"[^\S\n]{2,}", " ", text) + text = re.sub(r"\n{3,}", "\n\n", text) return text.strip() @@ -1856,7 +1860,10 @@ def _normalize_grouped_numbers(text: str, cfg: ApostropheConfig) -> str: for digit in trimmed_fraction: if not digit.isdigit(): return token - digit_words.append(_DIGIT_WORDS[int(digit)]) + try: + digit_words.append(_DIGIT_WORDS[int(digit)]) + except (ValueError, IndexError): + return token spoken = f"{integer_words} point {' '.join(digit_words)}" return f"minus {spoken}" if is_negative else spoken @@ -1878,18 +1885,27 @@ def _normalize_grouped_numbers(text: str, cfg: ApostropheConfig) -> str: # Magnitude case: $2.5 million -> two point five million dollars if "." in amount_str: integer_part, fraction_part = amount_str.split(".", 1) - integer_val = int(integer_part) + try: + integer_val = int(integer_part) + except ValueError: + return match.group(0) integer_words = _int_to_words(integer_val, language) # Spell out fraction digits digit_words = [] for digit in fraction_part: if digit.isdigit(): - digit_words.append(_DIGIT_WORDS[int(digit)]) + try: + digit_words.append(_DIGIT_WORDS[int(digit)]) + except (ValueError, IndexError): + return match.group(0) amount_spoken = f"{integer_words} point {' '.join(digit_words)}" else: - amount_spoken = _int_to_words(int(amount), language) + try: + amount_spoken = _int_to_words(int(amount), language) + except (ValueError, OverflowError): + return match.group(0) currency_names = { "$": "dollars", diff --git a/abogen/pyqt/gui.py b/abogen/pyqt/gui.py index 46b0d93..3d5c249 100644 --- a/abogen/pyqt/gui.py +++ b/abogen/pyqt/gui.py @@ -137,6 +137,28 @@ class ThreadSafeLogSignal(QObject): self.log_signal.emit(message) +_UPDATE_CHECK_URL = "https://raw.githubusercontent.com/denizsafak/abogen/refs/heads/main/abogen/VERSION" +_UPDATE_CHECK_TIMEOUT = 8 # seconds; bounds offline/DNS hangs so the GUI never blocks + + +class _UpdateCheckThread(QThread): + """Fetch the remote VERSION file off the GUI thread.""" + + succeeded = pyqtSignal(str) + failed = pyqtSignal(str) + + def run(self): + import urllib.request + + try: + with urllib.request.urlopen( + _UPDATE_CHECK_URL, timeout=_UPDATE_CHECK_TIMEOUT + ) as response: + self.succeeded.emit(response.read().decode().strip()) + except Exception as exc: # offline, DNS hang, HTTP error, ... + self.failed.emit(str(exc)) + + class IconProvider(QFileIconProvider): def icon(self, fileInfo): return super().icon(fileInfo) @@ -4077,75 +4099,85 @@ Categories=AudioVideo;Audio;Utility; self.check_for_updates_startup() def check_for_updates_startup(self): - import urllib.request - - def show_update_message(remote_version, local_version): - msg_box = QMessageBox(self) - msg_box.setIcon(QMessageBox.Icon.Information) - msg_box.setWindowTitle("Update Available") - msg_box.setText( - f"A new version of {PROGRAM_NAME} is available! ({local_version} > {remote_version})" - ) - msg_box.setInformativeText( - f"If you installed via pip, update by running:\n" - f"pip install --upgrade {PROGRAM_NAME}\n\n" - f"If you're using the Windows portable version, run 'WINDOWS_INSTALL.bat' again.\n\n" - "Alternatively, visit the GitHub repository for more information. " - "Would you like to view the changelog?" - ) - msg_box.setStandardButtons( - QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No - ) - msg_box.setDefaultButton(QMessageBox.StandardButton.Yes) - if msg_box.exec() == QMessageBox.StandardButton.Yes: - try: - QDesktopServices.openUrl(QUrl(GITHUB_URL + "/releases/latest")) - except Exception: - pass - - # Reset flag to track if we should show "no updates" message + # Network I/O runs in a worker thread: urlopen without a timeout on + # the GUI thread froze the whole app when offline (DNS/connect can + # hang for minutes). Results return via signals on the GUI thread. + thread = getattr(self, "_update_check_thread", None) + if thread is not None: + try: + if thread.isRunning(): + return + except RuntimeError: + pass # previous thread already finished/deleted show_result = ( hasattr(self, "_show_update_check_result") and self._show_update_check_result ) self._show_update_check_result = False + self._update_check_thread = _UpdateCheckThread(self) + self._update_check_thread.succeeded.connect( + lambda remote_raw: self._on_update_check_done(remote_raw, show_result) + ) + self._update_check_thread.failed.connect( + lambda err: self._on_update_check_failed(err, show_result) + ) + self._update_check_thread.finished.connect( + self._update_check_thread.deleteLater + ) + self._update_check_thread.start() + def _on_update_check_done(self, remote_raw, show_result): + remote_version = remote_raw.strip() + local_version = VERSION try: - update_url = "https://raw.githubusercontent.com/denizsafak/abogen/refs/heads/main/abogen/VERSION" - with urllib.request.urlopen(update_url) as response: - remote_raw = response.read().decode().strip() - local_raw = VERSION + remote_num = int("".join(remote_version.split("."))) + local_num = int("".join(local_version.split("."))) + except ValueError: + return + if remote_num > local_num: + # Use QTimer to ensure UI is ready, then show update message. + QTimer.singleShot( + 1000, + lambda: self._show_update_message(remote_version, local_version), + ) + elif show_result: + QMessageBox.information( + self, + "Up to Date", + f"You are running the latest version of {PROGRAM_NAME} ({local_version}).", + ) - # Parse version numbers - remote_version = remote_raw - local_version = local_raw + def _on_update_check_failed(self, err, show_result): + if show_result: + QMessageBox.warning( + self, + "Update Check Failed", + f"Could not check for updates:\n{err}", + ) + def _show_update_message(self, remote_version, local_version): + msg_box = QMessageBox(self) + msg_box.setIcon(QMessageBox.Icon.Information) + msg_box.setWindowTitle("Update Available") + msg_box.setText( + f"A new version of {PROGRAM_NAME} is available! ({local_version} > {remote_version})" + ) + msg_box.setInformativeText( + f"If you installed via pip, update by running:\n" + f"pip install --upgrade {PROGRAM_NAME}\n\n" + f"If you're using the Windows portable version, run 'WINDOWS_INSTALL.bat' again.\n\n" + "Alternatively, visit the GitHub repository for more information. " + "Would you like to view the changelog?" + ) + msg_box.setStandardButtons( + QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No + ) + msg_box.setDefaultButton(QMessageBox.StandardButton.Yes) + if msg_box.exec() == QMessageBox.StandardButton.Yes: try: - remote_num = int("".join(remote_version.split("."))) - local_num = int("".join(local_version.split("."))) - except ValueError as ve: - return - - if remote_num > local_num: - # Use QTimer to ensure UI is ready, then show update message. - QTimer.singleShot( - 1000, lambda: show_update_message(remote_version, local_version) - ) - elif show_result: - # Show "no updates" message if manually checking - QMessageBox.information( - self, - "Up to Date", - f"You are running the latest version of {PROGRAM_NAME} ({local_version}).", - ) - except Exception as e: - if show_result: - QMessageBox.warning( - self, - "Update Check Failed", - f"Could not check for updates:\n{str(e)}", - ) - pass + QDesktopServices.openUrl(QUrl(GITHUB_URL + "/releases/latest")) + except Exception: + pass def clear_cache_files(self): """Clear cache files created by the program.""" @@ -4248,8 +4280,6 @@ Categories=AudioVideo;Audio;Utility; def set_max_log_lines(self): """Open a dialog to set the maximum lines in the log window.""" - from PyQt6.QtWidgets import QInputDialog - value, ok = QInputDialog.getInt( self, "Max Lines in Log Window", @@ -4271,8 +4301,6 @@ Categories=AudioVideo;Audio;Utility; def set_max_subtitle_words(self): """Open a dialog to set the maximum words per subtitle""" - from PyQt6.QtWidgets import QInputDialog - current_value = self.config.get("max_subtitle_words", _DEFAULTS["max_subtitle_words"]) value, ok = QInputDialog.getInt( diff --git a/tests/test_regression_fixes.py b/tests/test_regression_fixes.py index babc2f8..1c52870 100644 --- a/tests/test_regression_fixes.py +++ b/tests/test_regression_fixes.py @@ -90,3 +90,10 @@ def test_manual_override_normalization(): assert normalize_manual_override_token("The") == "the" assert normalize_manual_override_token(" A ") == "a" assert normalize_manual_override_token("word") == "word" + + +def test_paragraph_breaks_and_ellipsis_preserved(): + normalized = normalize("Test. Lorem ipsum...\n\nLorem...\n\nLorem ...") + assert "\n\n" in normalized + assert ". . ." not in normalized + assert "Lorem..." in normalized diff --git a/tests/test_subtitle_scenarios.py b/tests/test_subtitle_scenarios.py index 60d11a6..2b18cf5 100644 --- a/tests/test_subtitle_scenarios.py +++ b/tests/test_subtitle_scenarios.py @@ -344,3 +344,59 @@ class TestComplexParagraphsAndEdgeCases: assert len(entries) == 2 assert entries[0][2] == '"I wonder..."' assert entries[1][2] == "he mused." + + +class TestEllipsisAndParagraphBreaks: + """Regression: '...' sentences merged into one entry; '\\n\\n' flattened. + + spaCy does not treat ellipsis as a sentence boundary, so + 'Lorem ipsum... Lorem...' stayed a single subtitle entry. And + _cleanup_spacing collapsed paragraph breaks before TTS. + """ + + @staticmethod + def _tok(text_ws, dur=0.5): + toks, t = [], 0.0 + for text, ws in text_ws: + toks.append({"start": t, "end": t + dur, "text": text, "whitespace": ws}) + t += dur + return toks, t + + def test_spacy_splits_ellipsis_sentences(self): + # Kokoro-style tokens: '...' arrives as 3 dot tokens. + toks, end = self._tok([ + ("Test", ""), (".", " "), ("Lorem", " "), ("ipsum", ""), + (".", ""), (".", ""), (".", " "), + ("Lorem", ""), (".", ""), (".", ""), (".", ""), + ]) + entries = [] + process_subtitle_tokens( + toks, entries, max_subtitle_words=50, subtitle_mode="Sentence", + language=Language.EN_US, use_spacy_segmentation=True, + fallback_end_time=end, + ) + assert [e[2] for e in entries] == ["Test.", "Lorem ipsum...", "Lorem..."] + + def test_spacy_keeps_abbreviations_intact(self): + toks, end = self._tok([ + ("Mr.", " "), ("Smith", " "), ("went", " "), ("home", ""), + (".", " "), ("He", " "), ("slept", ""), (".", ""), + ]) + entries = [] + process_subtitle_tokens( + toks, entries, max_subtitle_words=50, subtitle_mode="Sentence", + language=Language.EN_US, use_spacy_segmentation=True, + fallback_end_time=end, + ) + assert [e[2] for e in entries] == ["Mr. Smith went home.", "He slept."] + + def test_spacy_splits_faketoken_ellipsis(self): + entries = [] + process_subtitle_tokens( + [{"start": 0.0, "end": 3.0, + "text": "Lorem ipsum... Lorem... Lorem...", "whitespace": ""}], + entries, max_subtitle_words=50, subtitle_mode="Sentence", + language=Language.EN_US, use_spacy_segmentation=True, + fallback_end_time=3.0, + ) + assert [e[2] for e in entries] == ["Lorem ipsum...", "Lorem...", "Lorem..."]