From e2da233cd1baa859a5721542fc8b80d9f3f880e7 Mon Sep 17 00:00:00 2001 From: historia Date: Thu, 10 Sep 2026 00:16:00 -0400 Subject: fix: smart chunking, extraction, and process-safety issues --- app/converter/audio.py | 13 +++++- app/converter/chunking.py | 97 +++++++++++++++++++++++++++++++++++++------ app/converter/clients/base.py | 5 +++ app/converter/converter.py | 94 ++++++++++++++++++++++++++++++++++++++++- app/converter/extractors.py | 96 ++++++++++++++++++++++++++++++++++-------- 5 files changed, 272 insertions(+), 33 deletions(-) (limited to 'app/converter') diff --git a/app/converter/audio.py b/app/converter/audio.py index 9df7aff..9536788 100644 --- a/app/converter/audio.py +++ b/app/converter/audio.py @@ -6,6 +6,7 @@ threads them through, so there is no module-global path to mutate. """ import logging +import math import re import shutil import subprocess @@ -29,7 +30,12 @@ def atempo_filters(speed: float) -> str: ``atempo`` accepts 0.5..2.0 per filter; values outside that range are handled by chaining multiple filters. Returns "" when ``speed`` is 1.0. + Non-finite values (``inf``, ``nan``) are rejected: they pass a bare + ``> 0`` check and would either loop forever (``inf / 2.0`` stays + ``inf``) or produce an invalid tempo expression. """ + if not math.isfinite(speed): + raise ValueError(f"Speed must be a finite number, got {speed}") if speed <= 0: raise ValueError(f"Speed must be a positive number, got {speed}") if abs(speed - 1.0) < SPEED_EPSILON: @@ -252,7 +258,10 @@ def build_concat_command(concat_list: Path, output_path: Path, output_format: st raise ValueError("speed_path is required when speed is not 1.0") return [ "ffmpeg", "-y", *inputs, "-filter_complex", - f"[0:a]split=2[base][spd];[spd]{filters}[spdout]", + # asplit (not split): split is a video filter and rejects + # audio streams, which would fail every speed-adjusted + # assembly after synthesis had already completed. + f"[0:a]asplit=2[base][spd];[spd]{filters}[spdout]", "-map", "[base]", *encode, *tags, *cover_block, *container, str(output_path), "-map", "[spdout]", *encode, *tags, *cover_block, *container, str(speed_path), ] @@ -290,7 +299,7 @@ def build_m4b_chapters_command(concat_list: Path, metadata_file: Path, output_pa if speed_path is not None: return [ "ffmpeg", "-y", *inputs, "-filter_complex", - f"[0:a]split=2[base][spd];[spd]{atempo_filters(speed)}[spdout]", + f"[0:a]asplit=2[base][spd];[spd]{atempo_filters(speed)}[spdout]", "-map", "[base]", *cover_block, "-map_metadata", "1", "-map_chapters", "1", *encode, *tags, *container, str(output_path), "-map", "[spdout]", *cover_block, "-map_metadata", "2", "-map_chapters", "2", diff --git a/app/converter/chunking.py b/app/converter/chunking.py index 0ffaa21..bf104bc 100644 --- a/app/converter/chunking.py +++ b/app/converter/chunking.py @@ -21,7 +21,8 @@ _FLOOR_RATIO = 0.7 # boundary; if such a token runs longer than this many characters per # allowed word it is sliced at the hard character cap (a last-resort # bound for punctuation-free input such as some CJK text or damage from -# text extraction). +# text extraction). The slice also honors the word limit itself, since +# every non-spaced-script character counts as one word. _MAX_CHARS_PER_WORD = 8 _ABBREVIATIONS = frozenset( @@ -203,8 +204,8 @@ def _split_oversized(text: str, max_words: int) -> List[str]: First at clause punctuation (kept attached for TTS prosody, and never at commas without whitespace so tokens like "1,000,000" and "12:30" survive), then at word boundaries. A token with no - whitespace at all is sliced at the hard character cap so a - punctuation-free run cannot exceed the bound. + whitespace at all is sliced at the hard character cap and the word + limit so a punctuation-free run cannot exceed either bound. """ parts = re.split(r"(?:(?<=[,;:])\s+|(?<=[，、；：]))", text) pieces: List[str] = [] @@ -220,21 +221,50 @@ def _split_oversized(text: str, max_words: int) -> List[str]: return pieces +def _slice_token(word: str, max_words: int, cap: int) -> List[str]: + """Slice a single whitespace-free token into packable pieces. + + Cuts before the character cap is exceeded and before the word count + is exceeded: each non-spaced-script character counts as one word, + so a punctuation-free CJK token is bounded by the word limit too, + not only by the character cap. + """ + slices: List[str] = [] + cur: List[str] = [] + cur_chars = 0 + # Any slice is itself one whitespace-delimited word before its + # non-spaced-script characters are counted on top. + cur_words = 1 + for ch in word: + ch_words = 1 if _NON_SPACED_RUN.match(ch) else 0 + if cur and (cur_chars + 1 > cap or cur_words + ch_words > max_words): + slices.append("".join(cur)) + cur, cur_chars, cur_words = [], 0, 1 + cur.append(ch) + cur_chars += 1 + cur_words += ch_words + if cur: + slices.append("".join(cur)) + return slices + + def _hard_slices(text: str, max_words: int, cap: int) -> List[str]: - """Slice TEXT at word boundaries, then at CAP characters when a - single token has no whitespace to break at. Buffers never exceed - the word limit or the character cap, so every piece is packable.""" + """Slice TEXT at word boundaries, then within tokens when a single + token (no whitespace to break at) exceeds the character cap or the + word limit on its own. Buffers never exceed the word limit or the + character cap, so every piece is packable.""" pieces: List[str] = [] buf: List[str] = [] buf_words = 0 buf_chars = 0 for word in text.split(): - if len(word) > cap: + if len(word) > cap or _count_words(word) > max_words: + # A single token too big to buffer: slice it at the hard + # character cap and the word limit together. if buf: pieces.append(" ".join(buf)) buf, buf_words, buf_chars = [], 0, 0 - for start in range(0, len(word), cap): - pieces.append(word[start:start + cap]) + pieces.extend(_slice_token(word, max_words, cap)) continue words = _count_words(word) if buf and buf_words + words <= max_words \ @@ -266,6 +296,41 @@ def _joined_chars(units: List[_Unit]) -> int: return total +def _quote_states(pieces: List[str], in_quote_end: bool) -> List[bool]: + """The quote state after each piece of an oversized sentence. + + Pieces of one sentence can sit inside a quotation the sentence does + not close, so ending a chunk at such a piece would sever the quote + (and marking them clean lets the target flush cut mid-quote). The + scanner's quote tracking is replayed over the pieces from the state + the sentence started in; that start state is unknown, so both + possibilities are replayed and the one whose end state matches the + unit's known end state wins (a unit rarely starts mid-quote). When + neither matches, every piece takes the unit's conservative end + state. + """ + def simulate(in_quote: bool, depth: int) -> List[bool]: + states: List[bool] = [] + for piece in pieces: + for ch in piece: + if ch == '"': + in_quote = not in_quote + elif ch in "“«": + depth += 1 + elif ch in "”»": + depth = max(0, depth - 1) + states.append(in_quote or depth > 0) + return states + + states = simulate(False, 0) + if states and states[-1] == in_quote_end: + return states + states = simulate(True, 0) + if states and states[-1] == in_quote_end: + return states + return [in_quote_end] * len(pieces) + + def _split_smart(text: str, max_words: int) -> List[str]: """Smart strategy: fill chunks up to MAX_WORDS at sentence boundaries, end them at the target once a quotation has closed or @@ -318,9 +383,14 @@ def _split_smart(text: str, max_words: int) -> List[str]: cur_chars = _joined_chars(cur) continue # One piece exceeds the limit itself: send its pieces out - # packed as tightly as the pieces allow. + # packed as tightly as the pieces allow, carrying the + # sentence's quotation state through the fallback so a chunk + # can end at a clean in-sentence boundary — and a piece still + # inside a quote is never mistaken for one. flush() - for piece_text in _split_oversized(unit.text, max_words): + piece_texts = _split_oversized(unit.text, max_words) + quote_states = _quote_states(piece_texts, unit.in_quote_end) + for index, piece_text in enumerate(piece_texts): if _count_words(piece_text) > max_words \ or len(piece_text) > char_cap: # A character-sliced piece of degenerate input: its own @@ -328,7 +398,10 @@ def _split_smart(text: str, max_words: int) -> List[str]: flush() chunks.append(piece_text) continue - piece = _Unit(piece_text, _count_words(piece_text), False, False) + piece = _Unit( + piece_text, _count_words(piece_text), + unit.para_after and index == len(piece_texts) - 1, + quote_states[index]) if cur and cur_words + piece.words <= max_words \ and cur_chars + 1 + len(piece_text) <= char_cap: cur.append(piece) diff --git a/app/converter/clients/base.py b/app/converter/clients/base.py index 9cf6979..7e9d919 100644 --- a/app/converter/clients/base.py +++ b/app/converter/clients/base.py @@ -133,6 +133,11 @@ class BaseTTSClient: try: result = self.generate_chunk(text, chunk_num) if result and Path(result).exists(): + # A cancel that arrived while this request was in + # flight must still stop the run: the generated audio + # is scratch (cleaned per book) and must not be + # returned for assembly. + self._check_cancelled() return Path(result) logger.warning("Chunk %d attempt %d failed", chunk_num, attempt + 1) except ConversionCancelled: diff --git a/app/converter/converter.py b/app/converter/converter.py index f8815d8..88e17a9 100644 --- a/app/converter/converter.py +++ b/app/converter/converter.py @@ -1,7 +1,9 @@ """Orchestrates book-to-audiobook conversion.""" +import contextlib import glob import logging +import math import re import shutil import sys @@ -64,6 +66,50 @@ CHUNKS_FOLDER = APP_DIR / "chunks" # Per-chunk scratch audio, cleaned per book LOGS_FOLDER = logging_kit.LOG_DIR DEBUG_FOLDER = APP_DIR / "debug" # --debug dumps, kept across runs + +@contextlib.contextmanager +def _conversion_lock(): + """An exclusive cross-process lock over the shared scratch folder. + + Every converter run from this checkout synthesizes into the same + ``app/chunks`` directory (fixed chunk_NNNN names, cleaned per book), so + two overlapping runs would delete or overwrite each other's audio. + The lock makes a second run fail fast instead. It is an advisory + fcntl/msvcrt lock that dies with its process, so a crashed run never + leaves a stale lock wedging later starts; platforms without file + locking degrade to the old unlocked behavior. + """ + CHUNKS_FOLDER.mkdir(parents=True, exist_ok=True) + handle = open(CHUNKS_FOLDER / ".conversion.lock", "w") + handle.write("0") + handle.flush() + try: + busy: Optional[OSError] = None + try: + import fcntl + try: + fcntl.flock(handle.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) + except OSError as exc: + busy = exc + except ImportError: + try: + import msvcrt + try: + msvcrt.locking(handle.fileno(), msvcrt.LK_NBLCK, 1) + except OSError as exc: + busy = exc + except ImportError: + logger.warning("No file locking on this platform; the " + "conversion lock is not enforced") + if busy is not None: + raise RuntimeError( + "Another conversion is already running from this " + "installation: the shared scratch folder (app/chunks) is " + "in use. Wait for the run to finish or stop it first.") + yield + finally: + handle.close() # releases the lock + # Output containers and supported input formats. AUDIO_FORMATS = ("mp3", "m4b", "ogg", "flac") SUPPORTED_FORMATS = [".txt", ".pdf", ".epub"] @@ -322,8 +368,12 @@ class AudiobookConverter: unload_models: Optional[bool] = None, progress: Optional[Callable[[dict], None]] = None, cancel=None): - if speed <= 0: - raise ValueError(f"Speed must be a positive number, got {speed}") + # Non-finite speeds pass a bare "> 0" check (and float("inf") is + # "positive"): they would loop forever in atempo filter chaining, + # so they are rejected here at the boundary. + if not math.isfinite(speed) or speed <= 0: + raise ValueError( + f"Speed must be a finite positive number, got {speed}") if output_format not in AUDIO_FORMATS: raise ValueError(f"Unsupported output format: {output_format}") if backend is None: @@ -767,6 +817,9 @@ class AudiobookConverter: return False output_path = AUDIOBOOKS_FOLDER / f"{stem}.{self.output_format}" + # Same rule as _convert_text: honor a cancel that arrived during + # the final chapter's synthesis instead of assembling anyway. + self._check_cancelled() if not audio.combine_chapters_to_m4b(chapter_files, titles, output_path, chunks_dir=CHUNKS_FOLDER, speed=self.speed, @@ -910,6 +963,9 @@ class AudiobookConverter: successful_chunks, total_chunks) return False + # A cancel that arrived while the last chunk was generating + # must still stop the run here, before any output is produced. + self._check_cancelled() success = audio.combine_chunks(total_chunks, output_path, chunk_results=results, chunks_dir=CHUNKS_FOLDER, @@ -1087,6 +1143,27 @@ class AudiobookConverter: tag = f"{name_tag}_{narrator_tag}" if name_tag else narrator_tag names.append((book_file, f"{name}_{tag}")) + # The stem-collision suffix above can itself collide with another + # book's natural stem ("book.txt" and "book_txt.txt" both produce + # "book_txt_"), so make every planned name unique across the + # batch: two books sharing an output name would silently overwrite + # each other (ffmpeg writes with -y and both are approved here, + # before any output exists). Later occurrences get a numeric + # suffix; the list is sorted, so the result is deterministic. + used: set = set() + unique: List[Tuple[Path, str]] = [] + for book_file, name in names: + if name in used: + suffix = 2 + while f"{name}_{suffix}" in used: + suffix += 1 + print(f"[INFO] Output name {name} is already planned for " + f"another book; {book_file.name} becomes {name}_{suffix}") + name = f"{name}_{suffix}" + used.add(name) + unique.append((book_file, name)) + names = unique + # Ask every overwrite question up front, before any conversion # starts, so the rest of the run is unattended. planned: List[Tuple[Path, str]] = [] @@ -1136,6 +1213,19 @@ class AudiobookConverter: self._emit({"kind": "done", "ok": 0, "total": 0}) return True + # The scratch folder is shared by every run from this checkout; + # hold the cross-process lock for the whole conversion. + with _conversion_lock(): + return self._convert_planned_books(book_files, planned, run_start) + + def _convert_planned_books(self, book_files, planned, + run_start: float) -> bool: + """Convert PLANNED books sequentially (the body of ``run``). + + BOOK_FILES/PLANNED come from the pre-flight (outer or caller); + RUN_START timestamps the console summary. Returns True when every + planned book converted. + """ self._say(f"[INFO] Converting {len(planned)} of {len(book_files)} book(s)") # Per-book outcome ({book file name: ok}), published on the diff --git a/app/converter/extractors.py b/app/converter/extractors.py index 8b01372..0e30ac1 100644 --- a/app/converter/extractors.py +++ b/app/converter/extractors.py @@ -2,11 +2,13 @@ import codecs import logging +import posixpath import re import zipfile from html import unescape from pathlib import Path -from typing import List, NamedTuple, Optional +from typing import Dict, List, NamedTuple, Optional +from urllib.parse import unquote try: from bs4 import BeautifulSoup @@ -216,8 +218,13 @@ def _epub_spine_order(epub_zip: zipfile.ZipFile, html_names: List[str]): Parses the package OPF (located via META-INF/container.xml, else the only *.opf member): manifest item id -> href, then the spine's - idrefs. Returns member paths limited to HTML_NAMES; nav documents - (``properties`` containing "nav") and non-HTML items are excluded. + idrefs. Hrefs are URL-encoded, may carry "./" or "../" segments and + XML entities, and are relative to the OPF's folder — they are + normalized before matching against the zip's member names, so valid + chapters are not silently dropped when their written form differs + from the archived path. Returns member paths limited to HTML_NAMES; + nav documents (``properties`` containing "nav") and non-HTML items + are excluded. """ container = "META-INF/container.xml" opf_name = None @@ -246,6 +253,25 @@ def _epub_spine_order(epub_zip: zipfile.ZipFile, html_names: List[str]): base = "/".join(opf_name.split("/")[:-1]) item_tags = re.findall(r"]*>", opf) + # Zip member names indexed under their normalized spellings, so an + # exact archive path can be found from any equivalent href form. + # (Percent-encoded and plain spellings both map to the same member.) + members: Dict[str, str] = {} + for name in html_names: + members.setdefault(posixpath.normpath(name), name) + members.setdefault(posixpath.normpath(unquote(name)), name) + + def member_for_href(href: str) -> Optional[str]: + """The zip member an OPF manifest HREF points at (best effort).""" + path = unescape(href).strip() + path = path.split("#", 1)[0] + path = unquote(path).strip() + if not path: + return None + if base: + path = f"{base}/{path}" + return members.get(posixpath.normpath(path).lstrip("/")) + def is_nav(item_tag: str) -> bool: properties = attr(item_tag, "properties") or "" return "nav" in properties.split() @@ -262,10 +288,9 @@ def _epub_spine_order(epub_zip: zipfile.ZipFile, html_names: List[str]): href = attr(match, "href") if not href: continue - path = (f"{base}/{href}" if base else href) - path = re.sub(r"#.*$", "", path) - if path in html_names and path not in order: - order.append(path) + member = member_for_href(href) + if member and member not in order: + order.append(member) return order or None @@ -287,45 +312,82 @@ def _read_epub_manual(file_path: Path): def clean_text(text: str) -> str: - """Normalize whitespace and strip standalone page numbers. + """Normalize whitespace, strip standalone page numbers, and keep + paragraph breaks. Page numbers are removed only when a short number (up to three digits) appears alone on its own line (before whitespace collapsing), so inline numbers like "42 years", "1,000" or "3.5" are preserved, as are four-digit standalone lines, which are usually years ("1984") or chapter numbers rather than page numbers. + + Runs of blank lines survive as a single paragraph break (``\\n\\n``) so + the smart chunker can still end requests at paragraph boundaries and + reset its quotation state there; single line breaks (soft wraps) and + all other whitespace collapse to spaces. """ if not text: return "" # Standalone page numbers (digits alone on a line) must go BEFORE the - # newline-collapsing step below. + # whitespace-collapsing steps below. text = re.sub(r"(?m)^\s*\d{1,3}\s*$", " ", text) - text = re.sub(r"\s+", " ", text) + # Normalize line breaks so the steps below see plain "\n". + text = re.sub(r"\r\n?", "\n", text) + # Any run of blank lines becomes exactly one paragraph break. + text = re.sub(r"\n[^\S\n]*\n(?:[^\S\n]*\n)*", "\n\n", text) + # Remaining single line breaks (soft wraps) become spaces. + text = re.sub(r"(?believable" stays one word). The lookahead keeps +# lookalike tags (, ) out of the match. +_BLOCK_BREAK_RE = re.compile( + r"])" + r"[^>]*>", + re.IGNORECASE) + + def clean_html(html_content: str) -> str: - """Strip markup, scripts and styles from HTML content.""" + """Strip markup, scripts and styles from HTML content. + + Block-level tags become paragraph breaks (so the chunker sees the + document's paragraphs); inline tags vanish without adding spaces, so + no artificial word boundaries are injected. + """ if not html_content: return "" + html_content = _BLOCK_BREAK_RE.sub("\n\n", html_content) + if BS4_AVAILABLE: try: soup = BeautifulSoup(html_content, "html.parser") for tag in soup(["script", "style"]): tag.decompose() - text = soup.get_text(separator=" ") - return re.sub(r"\s+", " ", text).strip() + return clean_text(soup.get_text(separator="")) except Exception as exc: logger.debug("BeautifulSoup cleaning failed, falling back to regex: %s", exc) - # Fallback regex cleaning + # Fallback regex cleaning. Block tags are already paragraph breaks; + # every other tag (inline) disappears without adding a space, so no + # artificial word boundary is injected. html_content = re.sub(r"]*>.*?", "", html_content, flags=re.DOTALL | re.IGNORECASE) html_content = re.sub(r"]*>.*?", "", html_content, flags=re.DOTALL | re.IGNORECASE) - html_content = re.sub(r"<[^>]+>", " ", html_content) + html_content = re.sub(r"<[^>]+>", "", html_content) html_content = unescape(html_content) - html_content = re.sub(r"\s+", " ", html_content) - return html_content.strip() + return clean_text(html_content) def _natural_key(name: str): -- cgit v1.2.3