From 1d8b83c37ced9e0c1dfa78089da7371bedef6f8f Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 25 Sep 2026 15:20:31 +0200 Subject: [PATCH] fix(plex): a sidecar subtitle in a legacy code page rendered its accents as U+FFFD MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Hungarian .srt beside the video (Windows-1250) was decoded as UTF-8 with errors=replace: one episode served 1391 U+FFFD and no accented letter. decode_subtitle now lets a BOM decide, keeps valid UTF-8, and otherwise reads the legacy code page of the stream's language (cp1250 for Central European, cp1251 for Cyrillic, else cp1252) — on the ffmpeg path too. The cache has no eviction, so its key is versioned (sub2_): a track cached broken is read again once instead of being served for good. --- backend/app/plex/subtitles.py | 53 ++++++++++++- backend/tests/test_plex_subtitle_encoding.py | 81 ++++++++++++++++++++ backend/tests/test_plex_subtitle_prepare.py | 5 +- 3 files changed, 133 insertions(+), 6 deletions(-) create mode 100644 backend/tests/test_plex_subtitle_encoding.py diff --git a/backend/app/plex/subtitles.py b/backend/app/plex/subtitles.py index 374a731e..8bb38dd5 100644 --- a/backend/app/plex/subtitles.py +++ b/backend/app/plex/subtitles.py @@ -32,6 +32,7 @@ window still took the full 145 s and emitted the whole track), so windows are ex zero-based clock and shifted back to absolute time here with `shift_vtt`. """ +import codecs import json import re import subprocess @@ -217,7 +218,10 @@ _FULL_END_S = 1_000_000_000.0 # stand-in for "to the end of the file" (JSON can def cache_key(rating_key: str, sub_ord: int) -> str: - return f"sub_{rating_key}_{sub_ord}" + # `sub2_`: versioned when sidecars in a legacy code page turned out to have been cached as + # U+FFFD (`decode_subtitle`). The cache has no eviction, so every track is read again once + # rather than a broken one being served for good; the old `sub_` files are just left behind. + return f"sub2_{rating_key}_{sub_ord}" def _vtt_path(key: str) -> Path: @@ -904,15 +908,56 @@ def _full_pass_start(rating_key: str, targets: dict[int, int]) -> float: return max(0.0, float(f"{min(ends) - WINDOW_PRE_S:.3f}")) +# The legacy code page a subtitle in this language (Plex's ISO 639-2 `languageCode`) was most likely +# saved in; anything not listed falls to Windows-1252. +_CP1250 = { + "hun", + "ces", + "cze", + "pol", + "slk", + "slo", + "slv", + "hrv", + "bos", + "ron", + "rum", + "sqi", + "alb", +} +_CP1251 = {"rus", "ukr", "bel", "bul", "mkd", "mac", "srp"} + + +def decode_subtitle(raw: bytes, language_code: str | None) -> str: + """A sidecar subtitle file as text. SubRip carries no charset, and a Hungarian release is + typically Windows-1250: decoded as UTF-8 with `replace`, every ő/ű/é came out as U+FFFD (found + live, 1391 of them in one episode). So: a BOM decides; else valid UTF-8 stays UTF-8 (a legacy + code page with accents is almost never valid UTF-8); else the language's legacy code page.""" + if raw.startswith(codecs.BOM_UTF8): + return raw[len(codecs.BOM_UTF8) :].decode("utf-8", "replace") + if raw.startswith((codecs.BOM_UTF16_LE, codecs.BOM_UTF16_BE)): + return raw.decode("utf-16", "replace") + try: + return raw.decode("utf-8") + except UnicodeDecodeError: + # silent-ok: not UTF-8 is the expected answer for a legacy-code-page file, decoded below. + pass + lang = (language_code or "").lower() + legacy = "cp1250" if lang in _CP1250 else "cp1251" if lang in _CP1251 else "cp1252" + return raw.decode(legacy, "replace") + + def _sidecar_vtt(db: Session, stream: dict) -> str: - """An external subtitle file, fetched through Plex and converted to WebVTT.""" + """An external subtitle file, fetched through Plex and converted to WebVTT. Its bytes are made + UTF-8 first (`decode_subtitle`), on the ffmpeg path too: ffmpeg reads a text subtitle as UTF-8.""" with PlexClient(db) as plex: raw = plex.raw_get(stream["key"]) + text = decode_subtitle(raw, stream.get("languageCode")) codec = (stream.get("codec") or stream.get("format") or "").lower() if codec in ("srt", "subrip", ""): - return srt_to_vtt(raw.decode("utf-8", "replace")) + return srt_to_vtt(text) with tempfile.NamedTemporaryFile(suffix="." + (codec or "sub"), delete=False) as fh: - fh.write(raw) + fh.write(text.encode("utf-8")) tmp = fh.name try: return ffmpeg_to_vtt(tmp, None) diff --git a/backend/tests/test_plex_subtitle_encoding.py b/backend/tests/test_plex_subtitle_encoding.py new file mode 100644 index 00000000..6222787b --- /dev/null +++ b/backend/tests/test_plex_subtitle_encoding.py @@ -0,0 +1,81 @@ +"""A sidecar subtitle in a legacy code page must render its accents, not U+FFFD. + +Found live on Arrow S03E15 (a Hungarian .srt beside the .mp4): the served WebVTT held 1391 U+FFFD and +not one accented letter, because `_sidecar_vtt` decoded the bytes as UTF-8 with `errors="replace"` +while the file was Windows-1250 (ő = 0xF5, ű = 0xFB). Legacy-code-page .srt files are the norm for +Hungarian releases, and SubRip carries no charset of its own, so the stream's language is the only hint. +No Postgres: Plex and ffmpeg are stubbed. +""" + +from pathlib import Path +from typing import cast + +import pytest +from sqlalchemy.orm import Session + +from app.plex import subtitles + +HU = "1\r\n00:00:01,000 --> 00:00:02,000\r\nMiért nem szóltál? Ősz, űr.\r\n" +FR = "1\r\n00:00:01,000 --> 00:00:02,000\r\nDéjà vu, garçon.\r\n" +RU = "1\r\n00:00:01,000 --> 00:00:02,000\r\nПривет, мир.\r\n" + + +class _Plex: + def __init__(self, raw: bytes): + self.raw = raw + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + def raw_get(self, key: str) -> bytes: + return self.raw + + +def _sidecar(monkeypatch, raw: bytes, lang: str | None, codec: str = "srt") -> str: + monkeypatch.setattr(subtitles, "PlexClient", lambda db: _Plex(raw)) + stream = {"key": "/library/streams/1", "codec": codec, "languageCode": lang} + return subtitles._sidecar_vtt(cast(Session, None), stream) + + +@pytest.mark.parametrize( + ("text", "encoding", "lang"), + [ + (HU, "cp1250", "hun"), # the file that was found broken + (FR, "cp1252", "fre"), # a Western legacy file + (RU, "cp1251", "rus"), # a Cyrillic one + (HU, "utf-8", "hun"), # already UTF-8: untouched + (HU, "utf-8-sig", "hun"), # UTF-8 with a BOM + (HU, "utf-16", "hun"), # UTF-16 (carries its BOM) + ], + ids=["cp1250-hu", "cp1252-fr", "cp1251-ru", "utf8", "utf8-bom", "utf16"], +) +def test_a_sidecar_srt_is_decoded_in_its_own_encoding(monkeypatch, text, encoding, lang): + vtt = _sidecar(monkeypatch, text.encode(encoding), lang) + assert "\ufffd" not in vtt + assert vtt.startswith("WEBVTT\n\n") + assert text.splitlines()[2] in vtt + + +def test_a_non_srt_sidecar_reaches_ffmpeg_as_utf8(monkeypatch): + seen: dict[str, bytes] = {} + + def fake_ffmpeg(src: str, sub_ord): + seen["bytes"] = Path(src).read_bytes() + return "WEBVTT\n" + + monkeypatch.setattr(subtitles, "ffmpeg_to_vtt", fake_ffmpeg) + _sidecar(monkeypatch, "[Events]\nDialogue: Ősz\n".encode("cp1250"), "hun", codec="ass") + assert seen["bytes"].decode("utf-8") == "[Events]\nDialogue: Ősz\n" + + +def test_a_track_cached_before_the_encoding_fix_is_not_served(tmp_path, monkeypatch): + """The cache has no eviction, so a mis-decoded track would have stayed broken for good: the key + is versioned, and a track cached under the old one is read again (once) instead of served.""" + monkeypatch.setattr(subtitles, "CACHE_DIR", tmp_path) + (tmp_path / "sub_14881_1.vtt").write_text( + "WEBVTT\n\n1\n00:00:01.000 --> 00:00:02.000\nAz�rt\n", "utf-8" + ) + assert subtitles.cached_track("14881", 1)[0] is None diff --git a/backend/tests/test_plex_subtitle_prepare.py b/backend/tests/test_plex_subtitle_prepare.py index c6ccc90f..f0a7c50a 100644 --- a/backend/tests/test_plex_subtitle_prepare.py +++ b/backend/tests/test_plex_subtitle_prepare.py @@ -311,8 +311,9 @@ def test_covered_ranges_coalesce_and_answer_membership(): def test_a_cached_vtt_from_an_older_build_counts_as_complete(tmp_path): """Before this change the cache only ever held whole tracks, and none of them has a sidecar. Reading one as "covers nothing" would re-extract every already-cached track after the upgrade.""" - (tmp_path / "sub_98614_0.vtt").write_text(_vtt(_cue("00:01.000", "00:02.000", "x")), "utf-8") - assert subtitles.read_meta("sub_98614_0") == ([[0.0, 1_000_000_000.0]], True) + key = subtitles.cache_key("98614", 0) + (tmp_path / f"{key}.vtt").write_text(_vtt(_cue("00:01.000", "00:02.000", "x")), "utf-8") + assert subtitles.read_meta(key) == ([[0.0, 1_000_000_000.0]], True) # --- the deep-review round: the seams between the window and the full pass ----------------------