fix(subtitle): keep WebVTT-only cue markup out of converted subtitles

Strip cue classes, <v>/<lang>/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

@@ -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 `<i>` 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 |
+| --- | --- |
+| `<c.bg_transparent>text</c>` | Tags removed, text kept. A class only selects a CSS `::cue` rule that no player ever receives, so it carries no styling. |
+| `<i.loud>`, `<b.x>`, `<u.y>` | Class list removed, the styling tag kept. |
+| `<v Bob>`, `<lang ja>`, `<ruby>`, `<rt>` | Tags removed, text kept. |
+| `<00:00:02.000>` | Removed. These are karaoke timings that no subtitle player renders. |
+| `&amp;`, `&nbsp;`, `&#39;` | 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 `<i.loud>` 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: <backend>` |
+
+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`.

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. |

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"
+        "<c.bg_transparent>Mor, da vi blev gift,</c>\n"
+        "<c>kom Ben</c> <c.a.b>og jeg</c.a.b> til dig.\n"
+        "<i>Keep</i> <b>these</b> and <code>this</code>\n"
+        "<i.loud>class</i> <b.x>on</b> <u.y>styling</u>\n"
+        "<v.z Bob>voice</v> <lang ja>jp</lang> <ruby>\u6f22<rt>\u304b\u3093</rt></ruby>\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 "</c>" not in out and "<c>" not in out
+    assert "Mor, da vi blev gift," in out
+    assert "<i>Keep</i> <b>these</b> and <code>this</code>" in out
+    # class list dropped, the styling tag itself kept
+    assert "<i>class</i> <b>on</b> <u>styling</u>" 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 &amp; RA'ED &nbsp;x &#39;q&#39;\nTom &not 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 &not 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

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: ``<i>``, ``<b>``
+        and ``<u>`` without their class list. It removes class spans (``<c.foo>``),
+        ``<v>``, ``<lang>``, ``<ruby>``/``<rt>`` 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 ``<i.loud>`` 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"</?c(?:\.[^\s>]*)?(?:\s[^>]*)?>", "", text)
+        text = re.sub(r"(</?)([ibu])\.[^\s>]*", r"\1\2", text)
+        text = re.sub(r"</?(?:v|lang)(?:\.[^\s>]*)?(?:\s[^>]*)?>", "", text)
+        text = re.sub(r"</?(?:ruby|rt)>", "", 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:

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