fix(jellyfin): select Japanese subtitles before slower tracks finish (#270)

2026-10-01 05:40:50 -07:00 · 2026-09-24 20:42:14 -07:00
parent 23af9c6464
commit ba0155e429
3 changed files with 247 additions and 66 deletions
@@ -363,6 +363,114 @@ test('preload jellyfin subtitles waits for delayed external japanese track inste
  ]);
 });

+test('preload jellyfin subtitles selects japanese before slower tracks finish downloading', async () => {
+  const commands: Array<Array<string | number>> = [];
+  let releaseSlowTrack!: () => void;
+  const slowTrackBlocked = new Promise<void>((resolve) => {
+    releaseSlowTrack = resolve;
+  });
+  const mpvTracks: Array<Record<string, unknown>> = [];
+  const preload = createPreloadJellyfinExternalSubtitlesHandler(
+    makeDeps({
+      listJellyfinSubtitleTracks: async () => [
+        { index: 0, language: 'eng', title: 'English', deliveryUrl: 'https://sub/eng.ass' },
+        { index: 1, language: 'jpn', title: 'Japanese', deliveryUrl: 'https://sub/jpn.srt' },
+      ],
+      getMpvClient: () => ({ requestProperty: async () => mpvTracks }),
+      cacheSubtitleTrack: async (track) => {
+        if (track.index === 0) {
+          await slowTrackBlocked;
+        }
+        return {
+          path: `/tmp/subminer-jellyfin-subtitles/${track.index}.srt`,
+          cleanupDir: '/tmp/subminer-jellyfin-subtitles',
+        };
+      },
+      sendMpvCommand: (command) => {
+        commands.push(command);
+        if (command[0] === 'sub-add') {
+          mpvTracks.push({
+            type: 'sub',
+            id: mpvTracks.length + 1,
+            lang: command[4],
+            title: command[3],
+            external: true,
+            'external-filename': command[1],
+          });
+        }
+      },
+    }),
+  );
+
+  const done = preload({ session, clientInfo, itemId: 'item-1' });
+  const hasJapanesePrimary = () =>
+    commands.some(
+      (command) => command[0] === 'set_property' && command[1] === 'sid' && command[2] === 1,
+    );
+  for (let tick = 0; tick < 1000 && !hasJapanesePrimary(); tick += 1) {
+    await new Promise((resolve) => setImmediate(resolve));
+  }
+  const selectedBeforeSlowTrack = setPropertyCommandsExceptTrackAutoSelection(commands);
+  // Release before asserting so a regression fails here instead of leaving the preload hanging.
+  releaseSlowTrack();
+  await done;
+
+  assert.deepEqual(selectedBeforeSlowTrack, [['set_property', 'sid', 1]]);
+  assert.deepEqual(setPropertyCommandsExceptTrackAutoSelection(commands), [
+    ['set_property', 'sid', 1],
+    ['set_property', 'secondary-sid', 2],
+  ]);
+});
+
+test('preload jellyfin subtitles does not lock in a fallback japanese track during early selection', async () => {
+  const commands: Array<Array<string | number>> = [];
+  let requestCount = 0;
+  const fallbackJapanese = {
+    type: 'sub',
+    id: 5,
+    lang: 'jpn',
+    title: 'Japanese SDH',
+    external: true,
+    'external-filename': '/tmp/subminer-jellyfin-subtitles/0.srt',
+  };
+  const preferredJapanese = {
+    type: 'sub',
+    id: 6,
+    lang: 'jpn',
+    title: 'Japanese',
+    external: true,
+    'external-filename': '/tmp/subminer-jellyfin-subtitles/1.srt',
+  };
+  const preload = createPreloadJellyfinExternalSubtitlesHandler(
+    makeDeps({
+      listJellyfinSubtitleTracks: async () => [
+        { index: 0, language: 'jpn', title: 'Japanese SDH', deliveryUrl: 'https://sub/sdh.srt' },
+        {
+          index: 1,
+          language: 'jpn',
+          title: 'Japanese',
+          isDefault: true,
+          deliveryUrl: 'https://sub/jpn.srt',
+        },
+      ],
+      getMpvClient: () => ({
+        requestProperty: async () => {
+          requestCount += 1;
+          // mpv lists the preferred track only after the early selection poll gives up.
+          return requestCount <= 10 ? [fallbackJapanese] : [fallbackJapanese, preferredJapanese];
+        },
+      }),
+      sendMpvCommand: (command) => commands.push(command),
+    }),
+  );
+
+  await preload({ session, clientInfo, itemId: 'item-1' });
+
+  assert.deepEqual(setPropertyCommandsExceptTrackAutoSelection(commands), [
+    ['set_property', 'sid', 6],
+  ]);
+});
+
 test('preload jellyfin subtitles clears managed delay when no external tracks are available', async () => {
  const commands: Array<Array<string | number>> = [];
  const preload = createPreloadJellyfinExternalSubtitlesHandler(
@@ -129,24 +129,48 @@ function pickBestCachedTrackId(
        : false,
    )
    .filter(({ track }) => track.id !== excludeId)
-    .map(({ track, cached }) => {
-      const title = cached?.source.title || track.title;
-      return {
-        track,
-        score:
-          (track.external ? 100 : 0) +
-          (cached?.source.isDefault ? 35 : 0) +
-          (cached?.source.isExternal === false ? 25 : 0) +
-          (cached?.source.isExternal === true ? -10 : 0) +
-          (cached?.source.isForced ? -25 : 0) +
-          (isLikelyHearingImpaired(title) ? -10 : 10) +
-          (/\bdefault\b/i.test(title) ? 3 : 0),
-      };
-    })
+    .flatMap(({ track, cached }) =>
+      cached
+        ? [
+            {
+              track,
+              score:
+                (track.external ? 100 : 0) +
+                scoreJellyfinSource(cached.source, cached.source.title || track.title),
+            },
+          ]
+        : [],
+    )
    .sort((a, b) => b.score - a.score);
  return ranked[0]?.track.id ?? null;
 }

+// Ranks Jellyfin subtitle sources by metadata alone, so the preferred track is known before download.
+function scoreJellyfinSource(source: JellyfinSubtitleTrack, title: string): number {
+  return (
+    (source.isDefault ? 35 : 0) +
+    (source.isExternal === false ? 25 : 0) +
+    (source.isExternal === true ? -10 : 0) +
+    (source.isForced ? -25 : 0) +
+    (isLikelyHearingImpaired(title) ? -10 : 10) +
+    (/\bdefault\b/i.test(title) ? 3 : 0)
+  );
+}
+
+function pickPreferredJapaneseSource(
+  sources: JellyfinSubtitleTrack[],
+): JellyfinSubtitleTrack | null {
+  const ranked = sources
+    .filter((source) => isJapanese(source.language || '') || isJapanese(source.title || ''))
+    .map((source) => ({ source, score: scoreJellyfinSource(source, source.title || '') }))
+    .sort((a, b) => b.score - a.score);
+  return ranked[0]?.source ?? null;
+}
+
+function findMpvTrackIdByPath(tracks: MpvSubtitleTrack[], filePath: string): number | null {
+  return tracks.find((track) => track.externalFilename === filePath)?.id ?? null;
+}
+
 function findCachedTrackForMpvTrackId(
  tracks: MpvSubtitleTrack[],
  cachedTracks: CachedExternalSubtitleTrack[],
@@ -282,6 +306,22 @@ export function createPreloadJellyfinExternalSubtitlesHandler(deps: {
      });
  }

+  function selectJapanesePrimary(
+    subtitleTracks: MpvSubtitleTrack[],
+    cachedTracks: CachedExternalSubtitleTrack[],
+    trackId: number | null,
+  ): void {
+    if (trackId === null) {
+      deps.sendMpvCommand(['set_property', 'sid', 'no']);
+      return;
+    }
+    deps.sendMpvCommand(['set_property', 'sid', trackId]);
+    const selectedCachedTrack = findCachedTrackForMpvTrackId(subtitleTracks, cachedTracks, trackId);
+    if (selectedCachedTrack) {
+      startSubtitlePrefetchForCachedTrack(selectedCachedTrack.path);
+    }
+  }
+
  function cleanupActiveCache(): void {
    const dirs = [...activeCacheDirs];
    if (dirs.length === 0) return;
@@ -319,64 +359,93 @@ export function createPreloadJellyfinExternalSubtitlesHandler(deps: {
      deps.sendMpvCommand(['set_property', 'secondary-sub-visibility', 'no']);
      await deps.wait(300);
      const seenUrls = new Set<string>();
-      const cachedTracks: CachedExternalSubtitleTrack[] = [];
-      for (const track of externalTracks) {
-        if (!track.deliveryUrl || seenUrls.has(track.deliveryUrl)) {
-          continue;
-        }
+      const uniqueTracks = externalTracks.filter((track) => {
+        if (!track.deliveryUrl || seenUrls.has(track.deliveryUrl)) return false;
        seenUrls.add(track.deliveryUrl);
-        const labelBase = (track.title || track.language || '').trim();
-        const label = labelBase || `Jellyfin Subtitle ${track.index}`;
-        const cached = await deps.cacheSubtitleTrack(track);
-        activeCacheDirs.add(cached.cleanupDir);
-        cachedTracks.push({ ...cached, source: track });
-        deps.sendMpvCommand(['sub-add', cached.path, 'auto', label, track.language || '']);
-      }
+        return true;
+      });

-      await deps.wait(TRACK_SELECTION_INITIAL_WAIT_MS);
-      const shouldWaitForExternalJapanese = externalTracks.some(
-        (track) => isJapanese(track.language || '') || isJapanese(track.title || ''),
+      // 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.
+      const cachedTracks: CachedExternalSubtitleTrack[] = [];
+      const downloads = new Map(
+        uniqueTracks.map((track) => [
+          track,
+          (async (): Promise<CachedExternalSubtitleTrack> => {
+            const labelBase = (track.title || track.language || '').trim();
+            const label = labelBase || `Jellyfin Subtitle ${track.index}`;
+            const cached = { ...(await deps.cacheSubtitleTrack(track)), source: track };
+            activeCacheDirs.add(cached.cleanupDir);
+            cachedTracks.push(cached);
+            deps.sendMpvCommand(['sub-add', cached.path, 'auto', label, track.language || '']);
+            return cached;
+          })(),
+        ]),
      );
-      const subtitleTracks = await waitForPreferredSubtitleTracks(
-        deps,
-        shouldWaitForExternalJapanese,
-        cachedTracks.map((track) => track.path),
-      );
-      if (
-        shouldWaitForExternalJapanese &&
-        (!subtitleTracks || !hasExternalJapaneseTrack(subtitleTracks))
-      ) {
-        deps.logDebug('Timed out waiting for Jellyfin Japanese subtitle track', {
-          itemId: params.itemId,
-        });
-        return;
-      }
+      // 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 resolvedSubtitleTracks = subtitleTracks ?? [];
-      const japanesePrimaryId =
-        pickBestCachedTrackId(resolvedSubtitleTracks, cachedTracks, isJapanese) ??
-        pickBestTrackId(resolvedSubtitleTracks, isJapanese);
-      const englishSecondaryId =
-        pickBestCachedTrackId(resolvedSubtitleTracks, cachedTracks, isEnglish, japanesePrimaryId) ??
-        pickBestTrackId(resolvedSubtitleTracks, isEnglish, japanesePrimaryId);
-      if (japanesePrimaryId !== null) {
-        const selectedCachedTrack = findCachedTrackForMpvTrackId(
-          resolvedSubtitleTracks,
-          cachedTracks,
-          japanesePrimaryId,
-        );
-        if (selectedCachedTrack) {
-          deps.sendMpvCommand(['set_property', 'sid', japanesePrimaryId]);
-          startSubtitlePrefetchForCachedTrack(selectedCachedTrack.path);
-        } else {
-          deps.sendMpvCommand(['set_property', 'sid', japanesePrimaryId]);
+      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;
+          await deps.wait(TRACK_SELECTION_INITIAL_WAIT_MS);
+          subtitleTracks =
+            (await waitForPreferredSubtitleTracks(deps, true, [preferredJapanese.path])) ?? [];
+          if (!hasExternalJapaneseTrack(subtitleTracks)) {
+            deps.logDebug('Timed out waiting for Jellyfin Japanese subtitle track', {
+              itemId: params.itemId,
+            });
+            return;
+          }
+          // Only commit early to the preferred track. If mpv has not listed it yet, leave the
+          // choice to the full ranking below instead of locking in a lower-ranked fallback.
+          const preferredJapaneseTrackId = findMpvTrackIdByPath(
+            subtitleTracks,
+            preferredJapanese.path,
+          );
+          if (preferredJapaneseTrackId !== null) {
+            japanesePrimaryId = preferredJapaneseTrackId;
+            selectJapanesePrimary(subtitleTracks, cachedTracks, japanesePrimaryId);
+          }
        }
-      } else {
-        deps.sendMpvCommand(['set_property', 'sid', 'no']);
-      }

-      if (englishSecondaryId !== null) {
-        deps.sendMpvCommand(['set_property', 'secondary-sid', englishSecondaryId]);
+        const results = await allDownloads;
+        const failedDownload = results.find((result) => result.status === 'rejected');
+        if (failedDownload) {
+          throw failedDownload.reason;
+        }
+        const cachedPaths = cachedTracks.map((track) => track.path);
+        if (!hasExpectedExternalSubtitleTracks(subtitleTracks, cachedPaths)) {
+          await deps.wait(TRACK_SELECTION_INITIAL_WAIT_MS);
+          subtitleTracks = (await waitForPreferredSubtitleTracks(deps, false, cachedPaths)) ?? [];
+        }
+
+        if (japanesePrimaryId === undefined) {
+          japanesePrimaryId =
+            pickBestCachedTrackId(subtitleTracks, cachedTracks, isJapanese) ??
+            pickBestTrackId(subtitleTracks, isJapanese);
+          selectJapanesePrimary(subtitleTracks, cachedTracks, japanesePrimaryId);
+        }
+
+        const englishSecondaryId =
+          pickBestCachedTrackId(subtitleTracks, cachedTracks, isEnglish, japanesePrimaryId) ??
+          pickBestTrackId(subtitleTracks, isEnglish, japanesePrimaryId);
+        if (englishSecondaryId !== null) {
+          deps.sendMpvCommand(['set_property', 'secondary-sid', englishSecondaryId]);
+        }
+      } finally {
+        // Keep this run in the queue until every download has registered its cache dir, so
+        // the next run's cleanup sees them all.
+        await allDownloads;
      }
    } catch (error) {
      deps.logDebug('Failed to preload Jellyfin external subtitles', error);