From 614a8c5ae406bf94ca8e2ded4302d422c5755b01 Mon Sep 17 00:00:00 2001 From: imSp4rky Date: Thu, 15 Oct 2026 16:39:19 -0600 Subject: [PATCH] fix(subtitles): sort cues by start time when converting from SSA/ASS Convert a time-sorted copy of the SSA/ASS source, so that SubtitleEdit and pysubs2 write SRT cues in order. SSA/ASS targets keep the file order. --- docs/guide/subtitles.md | 5 ++ tests/tracks/test_subtitle_convert.py | 32 +++++++++++++ unshackle/core/tracks/subtitle_convert.py | 57 ++++++++++++++++------- 3 files changed, 78 insertions(+), 16 deletions(-) diff --git a/docs/guide/subtitles.md b/docs/guide/subtitles.md index 90ccfa66..ec7e7a74 100644 --- a/docs/guide/subtitles.md +++ b/docs/guide/subtitles.md @@ -247,6 +247,11 @@ unshackle dl --sub-format original EXAMPLE 'https://www.example.com/...' Conversion only runs when the source format differs from the target, so `--sub-format srt` on a track that is already SRT is a no-op. +When unshackle converts SSA/ASS to a format that is not SSA/ASS, it writes the cues in +start-time order. SSA/ASS files often list layered sign events out of time order. The +SSA/ASS source file keeps its order, because the order of events on one layer sets the +order in which they are drawn. + !!! warning "Down-converting styled subtitles is lossy" Converting styled SSA/ASS subtitles to SRT throws away positioning, colours, and italics. unshackle never does this automatically. It only happens when you diff --git a/tests/tracks/test_subtitle_convert.py b/tests/tracks/test_subtitle_convert.py index 6fad00e7..1cca3b95 100644 --- a/tests/tracks/test_subtitle_convert.py +++ b/tests/tracks/test_subtitle_convert.py @@ -49,6 +49,14 @@ Dialogue: 0,0:00:03.00,0:00:04.00,Default,,0,0,0,,World """ +LAYERED_ASS_SAMPLE = ASS_SAMPLE.replace( + "Dialogue: 0,0:00:01.00", "Dialogue: 1,0:00:05.00,0:00:06.00,Default,,0,0,0,,Sign\nDialogue: 0,0:00:01.00" +) + +# Read before the autouse fixture below hides it. +INSTALLED_SUBTITLEEDIT = binaries.SubtitleEdit + + @pytest.fixture(autouse=True) def no_subtitleedit(monkeypatch): """Default every test to a SubtitleEdit-less environment; tests opt in when needed.""" @@ -147,6 +155,30 @@ def test_ass_to_srt_converts_when_forced(tmp_path, monkeypatch): assert "{\\" not in out.read_text("utf8") # override tags stripped +@pytest.mark.parametrize("target", [Codec.SubRip, Codec.WebVTT]) +@pytest.mark.parametrize("backend", ["pysubs2", "subtitleedit"]) +def test_layered_ass_converts_in_start_time_order(tmp_path, monkeypatch, backend, target): + if backend == "subtitleedit": + if not INSTALLED_SUBTITLEEDIT: + pytest.skip("SubtitleEdit is not installed") + monkeypatch.setattr(binaries, "SubtitleEdit", INSTALLED_SUBTITLEEDIT) + monkeypatch.setattr("unshackle.core.config.config.subtitle", {"conversion_method": backend}, raising=False) + sub = make_sub(tmp_path, "x.ass", LAYERED_ASS_SAMPLE, Codec.SubStationAlphav4) + out = sub.convert(target, forced=True) + starts = re.findall(r"(\d+:\d\d:\d\d[.,]\d{3}) -->", out.read_text("utf-8-sig")) + assert len(starts) == 3 + assert starts == sorted(starts) + assert [p.name for p in tmp_path.iterdir()] == [out.name] # sorted copy removed + + +def test_ass_to_ssa_keeps_file_order(tmp_path, monkeypatch): + monkeypatch.setattr("unshackle.core.config.config.subtitle", {}, raising=False) + sub = make_sub(tmp_path, "x.ass", LAYERED_ASS_SAMPLE, Codec.SubStationAlphav4) + out = sub.convert(Codec.SubStationAlpha, forced=True) + text = out.read_text("utf8") + assert text.index("Sign") < text.index("Hello") + + def test_webvtt_to_srt_conversion(tmp_path, monkeypatch): monkeypatch.setattr("unshackle.core.config.config.subtitle", {}, raising=False) sub = make_sub(tmp_path, "x.vtt", VTT_SAMPLE, Codec.WebVTT) diff --git a/unshackle/core/tracks/subtitle_convert.py b/unshackle/core/tracks/subtitle_convert.py index 13df5ee6..256fab8e 100644 --- a/unshackle/core/tracks/subtitle_convert.py +++ b/unshackle/core/tracks/subtitle_convert.py @@ -19,6 +19,7 @@ import logging import re import subprocess import time +from itertools import pairwise from pathlib import Path from typing import Optional, Protocol @@ -114,6 +115,8 @@ def subtitleedit_args( return args +SUBSTATION: frozenset[Codec] = frozenset({Codec.SubStationAlpha, Codec.SubStationAlphav4}) + # Styled SubStation formats flattened to SRT lose positioning/colours/italics. # Never performed automatically, only when the user explicitly forces a target format. LOSSY_DOWNCONVERTS: frozenset[tuple[Codec, Codec]] = frozenset( @@ -155,10 +158,7 @@ class SubtitleEditBackend: return source in self.reads and target in self.writes def rank(self, source: Codec, target: Codec) -> int: - if source in (Codec.SubStationAlpha, Codec.SubStationAlphav4) and target in ( - Codec.TimedTextMarkupLang, - Codec.WebVTT, - ): + if source in SUBSTATION and target in (Codec.TimedTextMarkupLang, Codec.WebVTT): return 3 return 0 @@ -183,7 +183,7 @@ class SubtitleEditBackend: # SE5 names the output ., which may differ from our target # suffix (e.g. timedtext1.0 -> .ttml). Normalise it onto `out`. if not out.exists(): - produced = next((p for p in src.parent.glob(f"{src.stem}.*") if p not in (src, out)), None) + produced = next((p for p in out.parent.glob(f"{src.stem}.*") if p not in (src, out)), None) if produced is None: raise FileNotFoundError(f"SubtitleEdit produced no output for {src.name} -> {target.name}") produced.replace(out) @@ -203,7 +203,7 @@ class Pysubs2Backend: def rank(self, source: Codec, target: Codec) -> int: # Preferred reader for styled SubStation sources; solid general fallback otherwise. - return 1 if source in (Codec.SubStationAlpha, Codec.SubStationAlphav4) else 2 + return 1 if source in SUBSTATION else 2 def convert(self, source: Codec, src: Path, target: Codec, out: Path) -> None: subs = pysubs2.load(str(src), encoding="utf-8-sig") @@ -327,6 +327,26 @@ def finalize(sub: Subtitle, target: Codec, out: Path) -> Path: return out +def time_sorted_copy(src: Path) -> Optional[Path]: + """ + Copy an SSA/ASS file with its events in start-time order, or return None when it is already in order. + + Layered sign events are often out of time order, and SubtitleEdit and the pysubs2 SRT writer keep file order. + The source stays as it is, because ASS draws the events of one layer in file order. + """ + try: + subs = pysubs2.load(str(src), encoding="utf-8-sig") + except (pysubs2.exceptions.Pysubs2Error, UnicodeDecodeError, ValueError) as e: + log.debug(f"Cannot read {src.name} to sort it by start time, converting it unsorted: {e}") + return None + if all(a.start <= b.start for a, b in pairwise(subs.events)): + return None + subs.sort() + copy = src.with_name(f"{src.stem}.sorted{src.suffix}") + subs.save(str(copy), format_=subs.format, encoding="utf-8") + return copy + + def run_conversion(sub: Subtitle, target: Codec, *, pin: Optional[str] = None, forced: bool = False) -> Path: """ Convert ``sub`` to ``target`` using the best available backend, falling back through the @@ -356,16 +376,21 @@ def run_conversion(sub: Subtitle, target: Codec, *, pin: Optional[str] = None, f raise NotImplementedError(f"Cannot convert {source.name} to {target.name}.") out = src.with_suffix(f".{target.value.lower()}") + sorted_src = time_sorted_copy(src) if source in SUBSTATION and target not in SUBSTATION else None last_exc: Optional[Exception] = None - for backend in chain: - try: - backend.convert(source, src, target, out) - except Exception as e: - 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) + try: + for backend in chain: + try: + backend.convert(source, sorted_src or src, target, out) + except Exception as e: + 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) + finally: + if sorted_src: + sorted_src.unlink(missing_ok=True) raise RuntimeError(f"All subtitle backends failed for {source.name}->{target.name}") from last_exc