From 604a8633dd92c1dedf14b9001fc6413f63e11ba5 Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 23 Aug 2026 11:47:01 +0200 Subject: [PATCH] fix(playback): plausibility-based legacy subtitle encoding + packaged subtitle IPC smoke MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review round 2 on #1471: - Encoding detection now scores the Windows-1251 candidate's decoded text instead of any byte ratio: genuinely Cyrillic files decode into pure-Cyrillic words, while misread Latin text produces words mixing Cyrillic and ASCII letters ("était" -> "йtait") — so accent-dense CP1252 dialogue ("Été") stays Latin and short CP1251 dialogue stays Cyrillic. - The packaged Linux frame-copy smoke now drives addEmbeddedMpvSubtitle with a fixture .srt against the real helper (asserting the track appears in the session snapshot) and round-trips the subtitle delay/style IPC; the native file dialog itself cannot be automated and is documented as manually verified. Co-Authored-By: Claude Fable 5 --- .../embedded-mpv-frame-copy-packaged.e2e.ts | 53 +++++++++++++++++++ docs/architecture/player-controls-contract.md | 7 +++ .../external-subtitle-cues.util.spec.ts | 17 ++++++ .../external-subtitle-cues.util.ts | 51 +++++++++--------- 4 files changed, 103 insertions(+), 25 deletions(-) diff --git a/apps/electron-backend-e2e/src/embedded-mpv-frame-copy-packaged.e2e.ts b/apps/electron-backend-e2e/src/embedded-mpv-frame-copy-packaged.e2e.ts index b9a43f860..0a900e5ca 100644 --- a/apps/electron-backend-e2e/src/embedded-mpv-frame-copy-packaged.e2e.ts +++ b/apps/electron-backend-e2e/src/embedded-mpv-frame-copy-packaged.e2e.ts @@ -5,6 +5,7 @@ import { test, type LaunchedElectronApp, } from './electron-test-fixtures'; +import { writeFileSync } from 'fs'; import { join } from 'path'; import { assertNativeFallbackPrerequisites, @@ -197,6 +198,58 @@ test.describe('Packaged Linux embedded MPV frame-copy runtime', () => { ) .toBe('playing'); + // Advanced subtitle support (#1408): exercise the real preload + // IPC, main-process validation, and the helper's `sub-add` + // protocol command against the packaged runtime. The native file + // dialog cannot be automated, so the path-based IPC is driven + // directly with a fixture file; mpv parses and auto-selects it, + // and the helper's track-list observation must report it back. + const subtitlePath = join(dataDir, 'packaged-smoke.srt'); + writeFileSync( + subtitlePath, + '1\n00:00:00,000 --> 00:00:10,000\nPackaged subtitle smoke\n' + ); + const withSubtitle = await launchedFrameCopyApp.mainWindow.evaluate( + async ({ sessionId, path }) => + window.electron.addEmbeddedMpvSubtitle?.(sessionId, path), + { sessionId: created.id, path: subtitlePath } + ); + expect(withSubtitle?.id).toBe(created.id); + await expect + .poll( + async () => { + const session = await getLatestSession( + launchedFrameCopyApp, + created.id + ); + return session?.subtitleTracks?.length ?? 0; + }, + { timeout: 15000 } + ) + .toBeGreaterThan(0); + + // Delay and style ride the same session-guarded IPC; mpv does not + // echo the values in the snapshot, so a session reply for the live + // session (instead of a rejection) is the observable contract. + const delayReply = await launchedFrameCopyApp.mainWindow.evaluate( + (sessionId) => + window.electron.setEmbeddedMpvSubtitleDelay?.( + sessionId, + 1.5 + ), + created.id + ); + expect(delayReply?.id).toBe(created.id); + const styleReply = await launchedFrameCopyApp.mainWindow.evaluate( + (sessionId) => + window.electron.setEmbeddedMpvSubtitleStyle?.(sessionId, { + sizePercent: 150, + color: '#ffe94f', + }), + created.id + ); + expect(styleReply?.id).toBe(created.id); + await launchedFrameCopyApp.mainWindow.evaluate( (sessionId) => window.electron.setEmbeddedMpvPaused(sessionId, true), diff --git a/docs/architecture/player-controls-contract.md b/docs/architecture/player-controls-contract.md index e4edf37ef..b0c12fbff 100644 --- a/docs/architecture/player-controls-contract.md +++ b/docs/architecture/player-controls-contract.md @@ -728,6 +728,13 @@ Per-engine implementations: renderer-side values: the delay resets per session, and a non-default persisted style is re-applied to each new session. `sub-color` affects mpv's text-subtitle rendering; ASS files keep their embedded styling. + Runtime coverage: the packaged Linux frame-copy smoke + (`apps/electron-backend-e2e/src/embedded-mpv-frame-copy-packaged.e2e.ts`) + drives `addEmbeddedMpvSubtitle` with a fixture file against the real + packaged helper and asserts the track appears in the session snapshot, + plus round-trips the delay/style IPC. The native file dialog itself + (`selectEmbeddedMpvSubtitleFile`) cannot be automated and is verified + manually. - **Not wired (capabilities stay false):** Video.js shared mode (its emulated text-track display needs a separate remote-track + CSS integration — a follow-up), the vendor-chrome (preference-off) web players by design, the diff --git a/libs/ui/playback/src/lib/web-video-support/external-subtitle-cues.util.spec.ts b/libs/ui/playback/src/lib/web-video-support/external-subtitle-cues.util.spec.ts index dc0477b91..589c1f187 100644 --- a/libs/ui/playback/src/lib/web-video-support/external-subtitle-cues.util.spec.ts +++ b/libs/ui/playback/src/lib/web-video-support/external-subtitle-cues.util.spec.ts @@ -59,6 +59,23 @@ describe('external-subtitle-cues.util', () => { ); }); + it('keeps accent-dense CP1252 words Latin ("Été" must not become Cyrillic)', () => { + // É=0xC9, t, é=0xE9 — more accented than ASCII letters, so a + // byte-ratio heuristic flips to CP1251 and renders "Йtй". The + // mixed-script plausibility check must keep this Windows-1252. + const cp1252 = [ + 0xc9, 0x74, 0xe9, // Été + ...Array.from(' 1\n00:00:01,000 --> 00:00:02,000\n').map((c) => + c.charCodeAt(0) + ), + 0xc0, // À + ...Array.from(' table !').map((c) => c.charCodeAt(0)), + ]; + const decoded = decodeExternalSubtitleBytes(toBuffer(cp1252)); + expect(decoded).toContain('Été'); + expect(decoded).toContain('À table'); + }); + it('decodes mostly-ASCII Windows-1252 bytes (sparse accents)', () => { // "resume: cafe" with two accented letters among ASCII. const cp1252 = [ diff --git a/libs/ui/playback/src/lib/web-video-support/external-subtitle-cues.util.ts b/libs/ui/playback/src/lib/web-video-support/external-subtitle-cues.util.ts index 1e822056c..ee3eff435 100644 --- a/libs/ui/playback/src/lib/web-video-support/external-subtitle-cues.util.ts +++ b/libs/ui/playback/src/lib/web-video-support/external-subtitle-cues.util.ts @@ -44,9 +44,8 @@ export function detectExternalSubtitleFormat( * silent U+FFFD substitution, which turns the still-common legacy-encoded SRT * files (Windows-1251 Cyrillic, Windows-1252 Western European) and Windows' * UTF-16 saves into mojibake that parses "successfully". Best-effort order: - * UTF-16 BOMs, then strict UTF-8, then a single-byte fallback chosen by how - * much of the text is non-ASCII — a non-Latin script (Cyrillic) makes high - * bytes dominate, while Latin text with accents stays mostly ASCII. + * UTF-16 BOMs, then strict UTF-8, then a single-byte fallback chosen by the + * plausibility of the decoded text (see {@link chooseLegacySingleByteDecode}). */ export function decodeExternalSubtitleBytes(buffer: ArrayBuffer): string { const bytes = new Uint8Array(buffer); @@ -64,35 +63,37 @@ export function decodeExternalSubtitleBytes(buffer: ArrayBuffer): string { // Not valid UTF-8: a legacy single-byte encoding. } - // Compare only letter-carrying bytes: cue counters and timing lines are - // ASCII digits/punctuation and identical in every candidate encoding, so - // ratios over the whole file would drown short dialogue ("Да"/"Нет") in - // timing bytes. In a non-Latin-script file virtually every letter is a - // high byte; Latin text with accents stays dominated by ASCII letters. - let highBytes = 0; - let asciiLetters = 0; - for (const byte of bytes) { - if (byte >= 0x80) { - highBytes += 1; - } else if ( - (byte >= 0x41 && byte <= 0x5a) || - (byte >= 0x61 && byte <= 0x7a) - ) { - asciiLetters += 1; - } - } - const encoding = - highBytes / Math.max(1, highBytes + asciiLetters) > 0.3 - ? 'windows-1251' - : 'windows-1252'; try { - return new TextDecoder(encoding).decode(buffer); + return chooseLegacySingleByteDecode(buffer); } catch { // Runtime without legacy decoders: non-fatal UTF-8 is the last resort. return new TextDecoder().decode(buffer); } } +/** + * Picks between the two dominant legacy encodings by the PLAUSIBILITY of the + * Windows-1251 candidate, not a byte ratio (a ratio drowns short dialogue in + * ASCII timing bytes one way, and over-weights accent-dense Latin words like + * "Été" the other way). CP1251 maps every high letter byte into the Cyrillic + * block, so a genuinely Cyrillic file decodes into pure-Cyrillic words, while + * misread Latin text decodes into words that MIX Cyrillic and ASCII letters + * ("était" → "йtait") — a shape real subtitles never contain. + */ +function chooseLegacySingleByteDecode(buffer: ArrayBuffer): string { + const decoded1251 = new TextDecoder('windows-1251').decode(buffer); + let score = 0; + for (const word of decoded1251.split(/[^\p{L}]+/u)) { + if (!/[\u0400-\u04ff]/.test(word)) { + continue; + } + score += /[A-Za-z]/.test(word) ? -word.length : word.length; + } + return score > 0 + ? decoded1251 + : new TextDecoder('windows-1252').decode(buffer); +} + // SRT uses "00:00:01,500"; VTT uses "00:00:01.500" and allows a missing hour // part ("01:23.456"). One pattern covers both. const TIMESTAMP_PATTERN =