From b24008237b57afd743d7c47c353bf8fd2dd2de16 Mon Sep 17 00:00:00 2001 From: sudacode Date: Thu, 24 Sep 2026 21:32:56 -0700 Subject: [PATCH] fix(jellyfin): keep selecting subtitles when a track download fails (#272) --- changes/jellyfin-fast-subtitle-selection.md | 1 + src/core/services/jellyfin.test.ts | 13 ++++- src/core/services/jellyfin.ts | 13 +++++ .../runtime/jellyfin-subtitle-preload.test.ts | 50 +++++++++++++++++++ src/main/runtime/jellyfin-subtitle-preload.ts | 32 ++++++------ 5 files changed, 91 insertions(+), 18 deletions(-) diff --git a/changes/jellyfin-fast-subtitle-selection.md b/changes/jellyfin-fast-subtitle-selection.md index 80c2113e..0bd9bec5 100644 --- a/changes/jellyfin-fast-subtitle-selection.md +++ b/changes/jellyfin-fast-subtitle-selection.md @@ -2,3 +2,4 @@ type: fixed area: jellyfin - Jellyfin playback now selects the Japanese subtitle track, and starts subtitle annotations, as soon as that track downloads instead of waiting for every other subtitle track. Tracks now download in parallel, so a slow embedded track Jellyfin has to extract no longer delays the primary subtitles. +- Jellyfin episodes with image-based embedded subtitles (PGS, DVD, DVB) now auto-select the Japanese and English tracks. SubMiner no longer requests those tracks as text, and a subtitle track that fails to download no longer cancels selection of the others. diff --git a/src/core/services/jellyfin.test.ts b/src/core/services/jellyfin.test.ts index c6340c29..301f109d 100644 --- a/src/core/services/jellyfin.test.ts +++ b/src/core/services/jellyfin.test.ts @@ -377,6 +377,14 @@ test('listSubtitleTracks returns all subtitle streams with delivery urls', async DeliveryUrl: 'https://cdn.example.com/subs.srt', IsExternalUrl: true, }, + { + Type: 'Subtitle', + Index: 5, + Codec: 'PGSSUB', + Language: 'eng', + DisplayTitle: 'English PGS', + DeliveryMethod: 'Embed', + }, ], }, ], @@ -395,10 +403,10 @@ test('listSubtitleTracks returns all subtitle streams with delivery urls', async clientInfo, 'movie-1', ); - assert.equal(tracks.length, 3); + assert.equal(tracks.length, 4); assert.deepEqual( tracks.map((track) => track.index), - [2, 3, 4], + [2, 3, 4, 5], ); assert.equal( tracks[0]!.deliveryUrl, @@ -409,6 +417,7 @@ test('listSubtitleTracks returns all subtitle streams with delivery urls', async 'http://jellyfin.local/Videos/movie-1/ms-1/Subtitles/3/Stream.srt?ApiKey=token', ); assert.equal(tracks[2]!.deliveryUrl, 'https://cdn.example.com/subs.srt'); + assert.equal(tracks[3]!.deliveryUrl, null, 'bitmap subtitles cannot be fetched as text'); } finally { globalThis.fetch = originalFetch; } diff --git a/src/core/services/jellyfin.ts b/src/core/services/jellyfin.ts index e960604c..2d421287 100644 --- a/src/core/services/jellyfin.ts +++ b/src/core/services/jellyfin.ts @@ -146,6 +146,17 @@ function setApiKeyParam(url: URL, accessToken: string): void { url.searchParams.set('ApiKey', accessToken); } +const IMAGE_SUBTITLE_CODECS = new Set([ + 'pgssub', + 'pgs', + 'hdmv_pgs_subtitle', + 'dvdsub', + 'dvd_subtitle', + 'dvbsub', + 'dvb_subtitle', + 'xsub', +]); + function resolveDeliveryUrl( session: JellyfinAuthSession, stream: JellyfinMediaStream, @@ -163,6 +174,8 @@ function resolveDeliveryUrl( const streamIndex = asIntegerOrNull(stream.Index); if (streamIndex === null || !itemId || !mediaSourceId) return null; const codec = ensureString(stream.Codec).toLowerCase(); + // Jellyfin cannot convert bitmap subtitles to text, so requesting them always fails. + if (IMAGE_SUBTITLE_CODECS.has(codec)) return null; const ext = codec === 'subrip' ? 'srt' diff --git a/src/main/runtime/jellyfin-subtitle-preload.test.ts b/src/main/runtime/jellyfin-subtitle-preload.test.ts index 570109e4..4c78169d 100644 --- a/src/main/runtime/jellyfin-subtitle-preload.test.ts +++ b/src/main/runtime/jellyfin-subtitle-preload.test.ts @@ -471,6 +471,56 @@ test('preload jellyfin subtitles does not lock in a fallback japanese track duri ]); }); +test('preload jellyfin subtitles still selects remaining tracks when one download fails', async () => { + const commands: Array> = []; + const preload = createPreloadJellyfinExternalSubtitlesHandler( + makeDeps({ + listJellyfinSubtitleTracks: async () => [ + { index: 0, language: 'eng', title: 'English', deliveryUrl: 'https://sub/eng.srt' }, + { index: 1, language: 'jpn', title: 'Japanese', deliveryUrl: 'https://sub/jpn.srt' }, + { index: 5, language: 'eng', title: 'English PGS', deliveryUrl: 'https://sub/pgs.srt' }, + ], + getMpvClient: () => ({ + requestProperty: async () => [ + { + type: 'sub', + id: 1, + lang: 'eng', + title: 'English', + external: true, + 'external-filename': '/tmp/subminer-jellyfin-subtitles/0.srt', + }, + { + type: 'sub', + id: 2, + lang: 'jpn', + title: 'Japanese', + external: true, + 'external-filename': '/tmp/subminer-jellyfin-subtitles/1.srt', + }, + ], + }), + cacheSubtitleTrack: async (track) => { + if (track.index === 5) { + throw new Error('Failed to download Jellyfin subtitle (HTTP 400)'); + } + return { + path: `/tmp/subminer-jellyfin-subtitles/${track.index}.srt`, + cleanupDir: '/tmp/subminer-jellyfin-subtitles', + }; + }, + sendMpvCommand: (command) => commands.push(command), + }), + ); + + await preload({ session, clientInfo, itemId: 'item-1' }); + + assert.deepEqual(setPropertyCommandsExceptTrackAutoSelection(commands), [ + ['set_property', 'sid', 2], + ['set_property', 'secondary-sid', 1], + ]); +}); + test('preload jellyfin subtitles clears managed delay when no external tracks are available', async () => { const commands: Array> = []; const preload = createPreloadJellyfinExternalSubtitlesHandler( diff --git a/src/main/runtime/jellyfin-subtitle-preload.ts b/src/main/runtime/jellyfin-subtitle-preload.ts index 54e1a9be..7d9f6b7c 100644 --- a/src/main/runtime/jellyfin-subtitle-preload.ts +++ b/src/main/runtime/jellyfin-subtitle-preload.ts @@ -367,15 +367,22 @@ export function createPreloadJellyfinExternalSubtitlesHandler(deps: { // Download every track at once and add each to mpv as soon as it lands. Jellyfin has // to extract embedded tracks from the container, so one slow track must not hold up - // the Japanese primary that annotations depend on. + // the Japanese primary that annotations depend on. A failed track is skipped rather + // than failing the whole preload, so the remaining tracks still get selected. const cachedTracks: CachedExternalSubtitleTrack[] = []; const downloads = new Map( uniqueTracks.map((track) => [ track, - (async (): Promise => { + (async (): Promise => { const labelBase = (track.title || track.language || '').trim(); const label = labelBase || `Jellyfin Subtitle ${track.index}`; - const cached = { ...(await deps.cacheSubtitleTrack(track)), source: track }; + let cached: CachedExternalSubtitleTrack; + try { + cached = { ...(await deps.cacheSubtitleTrack(track)), source: track }; + } catch (error) { + deps.logDebug(`Failed to download Jellyfin subtitle track ${track.index}`, error); + return null; + } activeCacheDirs.add(cached.cleanupDir); cachedTracks.push(cached); deps.sendMpvCommand(['sub-add', cached.path, 'auto', label, track.language || '']); @@ -383,20 +390,17 @@ export function createPreloadJellyfinExternalSubtitlesHandler(deps: { })(), ]), ); - // Settled up front so a failed download is not flagged as unhandled while the - // Japanese track is still being selected. - const allDownloads = Promise.allSettled(downloads.values()); + const allDownloads = Promise.all(downloads.values()); try { let subtitleTracks: MpvSubtitleTrack[] = []; let japanesePrimaryId: number | null | undefined; const preferredJapaneseSource = pickPreferredJapaneseSource(uniqueTracks); - const preferredJapaneseDownload = preferredJapaneseSource - ? downloads.get(preferredJapaneseSource) - : undefined; - if (preferredJapaneseDownload) { - const preferredJapanese = await preferredJapaneseDownload; + const preferredJapanese = preferredJapaneseSource + ? await downloads.get(preferredJapaneseSource) + : null; + if (preferredJapanese) { await deps.wait(TRACK_SELECTION_INITIAL_WAIT_MS); subtitleTracks = (await waitForPreferredSubtitleTracks(deps, true, [preferredJapanese.path])) ?? []; @@ -418,11 +422,7 @@ export function createPreloadJellyfinExternalSubtitlesHandler(deps: { } } - const results = await allDownloads; - const failedDownload = results.find((result) => result.status === 'rejected'); - if (failedDownload) { - throw failedDownload.reason; - } + await allDownloads; const cachedPaths = cachedTracks.map((track) => track.path); if (!hasExpectedExternalSubtitleTracks(subtitleTracks, cachedPaths)) { await deps.wait(TRACK_SELECTION_INITIAL_WAIT_MS);