From 1f96a4bd78df7e2d9432a003f5c5e9b36c41616c Mon Sep 17 00:00:00 2001 From: imSp4rky Date: Mon, 24 Aug 2026 15:55:06 -0600 Subject: [PATCH] fix(subtitle): keep WebVTT-only cue markup out of converted subtitles Strip cue classes, //ruby and karaoke timestamps before conversion, decode entities for SRT/SSA/ASS, load pysubs2 as utf-8-sig, and route sdh_method: filter-subs through convert_before_strip instead of raising. --- docs/guide/subtitles.md | 65 +++++++++++++++++++++++ docs/reference/configuration/download.md | 4 +- tests/tracks/test_webvtt_cue_classes.py | 62 +++++++++++++++++++++ unshackle/core/tracks/subtitle.py | 40 ++++++++------ unshackle/core/tracks/subtitle_convert.py | 30 ++++++++++- 5 files changed, 182 insertions(+), 19 deletions(-) create mode 100644 tests/tracks/test_webvtt_cue_classes.py diff --git a/docs/guide/subtitles.md b/docs/guide/subtitles.md index b7e97c92..319f29c1 100644 --- a/docs/guide/subtitles.md +++ b/docs/guide/subtitles.md @@ -55,6 +55,9 @@ format-appropriate clean-up to make a single, valid file: that confuse downstream parsers, and merges multi-line cues that a service split into separate overlapping cues. By default unshackle keeps the original styling and layout (see [`preserve_formatting`](#subtitle-configuration)). +- **Cue markup normalisation**: unshackle removes the WebVTT-only parts of a cue + payload, so no player draws them as dialogue. See + [Cue markup](#cue-markup-and-html-entities). - **TTML for muxing**: Matroska cannot carry TTML. If you do not request a specific output format, unshackle automatically converts any plain TTML track to WebVTT before muxing (the closest lossless equivalent). Boxed `fTTML` is likewise unwrapped to TTML @@ -188,6 +191,20 @@ itself. The default (`auto`) prefers the `subby` cleaner for SRT, falls back to SubtitleEdit when you install it, and otherwise uses the built-in `filter-subs` approach. +Every engine removes the same three things: bracketed sound effects (`[MUSIC PLAYING]`, +`(door creaks)`), speaker labels (`MAN:`), and font tags. Every engine keeps plain +dialogue and inline `` styling. They differ on music and lyric lines: + +| `sdh_method` | Engine that runs | Lyric lines (`♪ ... ♪`) | +| --- | --- | --- | +| `auto` | `subby` for an SRT track. SubtitleEdit for any other format when you install it, otherwise `filter-subs`. | Kept, unless the fallback reaches `filter-subs`. | +| `subby` | `subby`, but only for a track that is already SRT. Any other format falls through to `filter-subs`. | Kept for SRT, removed otherwise. | +| `subtitleedit` | SubtitleEdit when you install it, otherwise `filter-subs`. | Kept with SubtitleEdit, removed without it. | +| `filter-subs` | `filter-subs` always. | Removed. | + +Pick `filter-subs` when you want lyrics gone as well. Pick `subby` or install +SubtitleEdit when you want to read along with the songs. + !!! note "Why `convert_before_strip` exists" `subby`'s SDH stripper only operates on SRT. Handed any other codec it returns silently without stripping anything, not an error but a no-op that leaves the SDH @@ -288,6 +305,28 @@ Reading the table: Install [SubtitleEdit / `seconv`](#subtitle-configuration) if you need colour or the highest → SRT fidelity. +### Cue markup and HTML entities + +Styling is not the only thing a conversion has to handle. WebVTT carries markup that no +other format understands, and a player that does not know a tag draws it as dialogue. +unshackle removes that markup before any backend runs, so the result is the same for +every `conversion_method`: + +| WebVTT markup | Result | +| --- | --- | +| `text` | Tags removed, text kept. A class only selects a CSS `::cue` rule that no player ever receives, so it carries no styling. | +| ``, ``, `` | Class list removed, the styling tag kept. | +| ``, ``, ``, `` | Tags removed, text kept. | +| `<00:00:02.000>` | Removed. These are karaoke timings that no subtitle player renders. | +| `&`, ` `, `'` | Decoded to real characters for SRT, SSA, and ASS output. Kept escaped for WebVTT and TTML, which need them. | + +!!! warning "A class list breaks more than you expect" + FFmpeg reads SubRip markup as a tag only while the tag name stays inside + `[0-9a-zA-Z_/]`. The dot in `` fails that test, so FFmpeg prints the opening + tag on screen and then closes an italic run that never opened. The FFmpeg WebVTT + reader matches `i`, `b`, and `u` exactly, so there the same class silently loses the + styling. mpv, VLC, Plex, and Jellyfin all use one of those two readers. + !!! note "Config wins over a service's preference" A service may set a `preferred_conversion_method` on its own tracks (for example when it ships subtitles that a particular backend handles best). An explicit @@ -381,6 +420,32 @@ subtitle: | `group_by` | `type` | `type`, `language` | Whether all tracks of one type stay together, or each language stays next to its own variants. | | `language_priority` | *(unset)* | a list of languages | Languages to sort to the top. The rest keep the default order. Nothing is removed. | +### Which settings to change + +The defaults give a clean, player-safe file, so most people change nothing. Start here +only when you want something the defaults do not do: + +| If you want | Set | +| --- | --- | +| Every original style kept, with no re-encode at all | `--sub-format original`, and `sidecar_format: original` for sidecars | +| The highest conversion fidelity, including colour | Install SubtitleEdit. `conversion_method` stays `auto` | +| Music and lyric lines stripped along with the sound effects | `sdh_method: filter-subs` | +| Lyric lines kept | `sdh_method: subby`, or install SubtitleEdit | +| No stripped copies made at all | `strip_sdh: false` | +| One backend for every conversion, whatever you install | `conversion_method: ` | + +Two keys read as if they control markup, and do not: + +- **`preserve_formatting`** decides whether unshackle re-encodes a WebVTT file after + the repair pipeline. Left `true`, the cue text passes through as written. Set to + `false`, unshackle re-encodes the file through pycaption, which merges identical cues + and drops empty ones but also flattens styling. Neither setting leaves WebVTT-only + markup in the file: [cue markup normalisation](#cue-markup-and-html-entities) runs + either way. +- **`convert_before_strip`** applies to SDH stripping, not to your output format. It + converts the working copy to SRT so a non-SubtitleEdit engine can read it. Turning it + off makes `subby` and `filter-subs` no-ops on anything that is not already SRT. + !!! tip "SubtitleEdit is optional but recommended" Several high-fidelity paths (the best-quality conversions and the SubtitleEdit SDH stripper) only activate when you install the SubtitleEdit CLI and put it on your `PATH`. diff --git a/docs/reference/configuration/download.md b/docs/reference/configuration/download.md index 85682d0c..2b3fcd74 100644 --- a/docs/reference/configuration/download.md +++ b/docs/reference/configuration/download.md @@ -43,7 +43,7 @@ Common config keys. This is a useful subset, and every `dl` flag works: | `sub_format` | str | *(unset)* | Convert subtitles to this format (`srt`, `vtt`, `original`, ...). | | `forced_subs` | bool | `false` | Include forced subtitle tracks. | | `forced_s_lang` | list | `[]` | Forced subtitle language(s); implies `forced_subs`. A `-` prefix excludes. | -| `no_subs` / `no_audio` / `no_chapters` | bool | `false` | Skip that track type. | +| `no_subs` / `no_audio` / `no_chapters` / `no_attachments` | bool | `false` | Skip that track type. `no_attachments` also skips subtitle font attaching. | | `downloads` | int | `1` | Tracks downloaded concurrently. | | `workers` | int | *(downloader default)* | Threads per track. | | `speed_limit` | str | *(unlimited)* | Total download speed cap across all downloads combined, e.g. `500k`, `5M`, `1.5G` or plain bytes/sec. Values are bytes, not bits (`5M` = 5.0 MB/s). | @@ -132,7 +132,7 @@ in or writes them as sidecar files. See [Subtitles](../../guide/subtitles.md) fo |---------|------|---------|-------------| | `strip_sdh` | bool | `true` | Strip SDH/CC cues (hearing-impaired annotations) into a clean track. | | `sdh_method` | str | `"auto"` | SDH-stripping backend to use. | -| `preserve_formatting` | bool | `true` | Keep styling/positioning when converting or stripping. | +| `preserve_formatting` | bool | `true` | Pass WebVTT cue text through as written. `false` re-encodes it through pycaption, which merges identical cues but flattens styling. unshackle removes WebVTT-only cue markup either way. | | `convert_before_strip` | bool | `true` | Convert to the working format before stripping SDH. | | `conversion_method` | str | `"auto"` | Subtitle conversion backend. | | `output_mode` | str | `"mux"` | `mux` embeds subtitles in the MKV; `sidecar` writes separate files; `both` writes sidecars and still muxes. | diff --git a/tests/tracks/test_webvtt_cue_classes.py b/tests/tracks/test_webvtt_cue_classes.py new file mode 100644 index 00000000..407e1f70 --- /dev/null +++ b/tests/tracks/test_webvtt_cue_classes.py @@ -0,0 +1,62 @@ +from unshackle.core.tracks.subtitle import Subtitle + + +def test_strip_webvtt_cue_classes(): + text = ( + "WEBVTT\n\n" + "00:00:01.000 --> 00:00:04.000\n" + "Mor, da vi blev gift,\n" + "kom Ben og jeg til dig.\n" + "Keep these and this\n" + "class on styling\n" + "voice jp \u6f22\u304b\u3093\n" + "kara <00:00:01.500>timing <00:01.500>short\n" + ) + out = Subtitle.strip_webvtt_cue_classes(text) + assert "bg_transparent" not in out and "" not in out and "" not in out + assert "Mor, da vi blev gift," in out + assert "Keep these and this" in out + # class list dropped, the styling tag itself kept + assert "class on styling" in out + # WebVTT-only tags go, their text stays + assert "voice jp \u6f22\u304b\u3093" in out + assert "kara timing short" in out + + +def test_decode_entities(tmp_path): + from unshackle.core.tracks.subtitle_convert import decode_entities + + out = tmp_path / "a.srt" + out.write_text( + "1\n00:00:01,000 --> 00:00:04,000\nROBERT & RA'ED  x 'q'\nTom ¬ sure, AT&T\n", + encoding="utf8", + ) + decode_entities(out) + text = out.read_text(encoding="utf8") + assert "ROBERT & RA'ED \xa0x 'q'" in text + # no-semicolon legacy forms stay as written + assert "Tom ¬ sure, AT&T" in text + + +def test_filter_subs_converts_before_stripping(tmp_path, monkeypatch): + """``sdh_method: filter-subs`` used to hand subtitle-filter a .vtt and raise OSError.""" + from unshackle.core import binaries + from unshackle.core.config import config + + monkeypatch.setattr(binaries, "SubtitleEdit", None) + monkeypatch.setattr(config, "subtitle", {"sdh_method": "filter-subs"}, raising=False) + + path = tmp_path / "sub.vtt" + path.write_text( + "WEBVTT\n\n00:00:01.000 --> 00:00:03.000\n[MUSIC PLAYING]\n\n" + "00:00:04.000 --> 00:00:06.000\nPlain dialogue stays.\n", + encoding="utf8", + ) + sub = Subtitle(url="https://example.test/x", language="en", codec=Subtitle.Codec.WebVTT) + sub.path = path + sub.strip_hearing_impaired() + + assert sub.codec == Subtitle.Codec.SubRip + text = sub.path.read_text(encoding="utf8") + assert "MUSIC PLAYING" not in text + assert "Plain dialogue stays." in text diff --git a/unshackle/core/tracks/subtitle.py b/unshackle/core/tracks/subtitle.py index 314f5590..d4aa19cb 100644 --- a/unshackle/core/tracks/subtitle.py +++ b/unshackle/core/tracks/subtitle.py @@ -288,6 +288,7 @@ class Subtitle(Track): text = Subtitle.sanitize_webvtt_timestamps(text) text = Subtitle.sanitize_webvtt_cue_identifiers(text) text = Subtitle.merge_overlapping_webvtt_cues(text) + text = Subtitle.strip_webvtt_cue_classes(text) preserve_formatting = config.subtitle.get("preserve_formatting", True) @@ -315,6 +316,29 @@ class Subtitle(Track): # Keep the sanitized version even if parsing failed self.path.write_text(text, encoding="utf8") + @staticmethod + def strip_webvtt_cue_classes(text: str) -> str: + """ + Reduce a cue payload to the markup every target format shares: ````, ```` + and ```` without their class list. It removes class spans (````), + ````, ````, ````/```` and karaoke timestamps + (``<00:00:01.000>``). + + A class only selects a CSS ``::cue`` rule that no player receives, so it never + carries styling, but it does break rendering. FFmpeg's SubRip reader treats a tag + name as a tag only while it matches ``[0-9a-zA-Z_/]``. The dot fails that test, so + FFmpeg prints ```` as cue text and then emits the closing italic-off with + nothing opened. FFmpeg's WebVTT reader matches ``i``/``b``/``u`` exactly and drops + the rest, so there a class silently loses the styling instead. The tags this + method removes outright are the ones both readers already discard, and that + libass would draw as text if a conversion carried them into ASS. + """ + text = re.sub(r"]*)?(?:\s[^>]*)?>", "", text) + text = re.sub(r"(]*", r"\1\2", text) + text = re.sub(r"]*)?(?:\s[^>]*)?>", "", text) + text = re.sub(r"", "", text) + return re.sub(r"<\d{2,}:\d{2}(?::\d{2})?\.\d{3}>", "", text) + @staticmethod def strip_webvtt_timestamp_map(text: str) -> str: """ @@ -1000,22 +1024,6 @@ class Subtitle(Track): return elif sdh_method == "subtitleedit" and binaries.SubtitleEdit: pass # Continue to SubtitleEdit section below - elif sdh_method == "filter-subs": - sub = Subtitles(self.path) - try: - sub.filter(rm_fonts=True, rm_ast=True, rm_music=True, rm_effects=True, rm_names=True, rm_author=True) - except ValueError as e: - if "too many values to unpack" in str(e): - # Retry without name removal if the error is due to multiple colons in time references - # This can happen with lines like "at 10:00 and 2:00" - sub = Subtitles(self.path) - sub.filter( - rm_fonts=True, rm_ast=True, rm_music=True, rm_effects=True, rm_names=False, rm_author=True - ) - else: - raise - sub.save() - return elif sdh_method == "auto": if self.codec == Subtitle.Codec.SubRip: try: diff --git a/unshackle/core/tracks/subtitle_convert.py b/unshackle/core/tracks/subtitle_convert.py index a8e54963..13df5ee6 100644 --- a/unshackle/core/tracks/subtitle_convert.py +++ b/unshackle/core/tracks/subtitle_convert.py @@ -14,7 +14,9 @@ the codec enum, ``parse``, sanitizers and cue helpers (the collaborators backend from __future__ import annotations +import html import logging +import re import subprocess import time from pathlib import Path @@ -204,7 +206,7 @@ class Pysubs2Backend: return 1 if source in (Codec.SubStationAlpha, Codec.SubStationAlphav4) else 2 def convert(self, source: Codec, src: Path, target: Codec, out: Path) -> None: - subs = pysubs2.load(str(src), encoding="utf-8") + subs = pysubs2.load(str(src), encoding="utf-8-sig") subs.save(str(out), format_=PYSUBS2_FORMATS[target], encoding="utf-8") @@ -289,6 +291,30 @@ def resolve_backends(source: Codec, target: Codec, *, pin: Optional[str] = None) return sorted(available, key=lambda b: b.rank(source, target)) +# Formats with no entity syntax of their own. WebVTT and TTML keep theirs escaped. +PLAIN_TEXT_TARGETS: frozenset[Codec] = frozenset({Codec.SubRip, Codec.SubStationAlpha, Codec.SubStationAlphav4}) + + +def decode_entities(out: Path) -> None: + """ + Turn HTML entities into real characters in a format that has no entity syntax. + + FFmpeg's SubRip reader (mpv, VLC, Plex, Jellyfin) and libass draw ``&`` and the + other entities as literal text. Backends differ here: subby and SubtitleEdit + decode, pysubs2 does not. + """ + text = out.read_text(encoding="utf8") + if "&" not in text: + return + decoded = re.sub( + r"&(?:#\d{1,7}|#[xX][0-9a-fA-F]{1,6}|[A-Za-z][A-Za-z0-9]{1,31});", + lambda m: html.unescape(m.group(0)), + text, + ) + if decoded != text: + out.write_text(decoded, encoding="utf8") + + def finalize(sub: Subtitle, target: Codec, out: Path) -> Path: """Swap the track onto the converted file and fire the OnConverted callback.""" original = sub.path @@ -338,6 +364,8 @@ def run_conversion(sub: Subtitle, target: Codec, *, pin: Optional[str] = None, f last_exc = e log.debug(f"Subtitle backend {backend.name} failed ({source.name}->{target.name}): {e}") continue + if target in PLAIN_TEXT_TARGETS: + decode_entities(out) return finalize(sub, target, out) raise RuntimeError(f"All subtitle backends failed for {source.name}->{target.name}") from last_exc