From 509dc5bf7f8f343a74c28595099038875de8af47 Mon Sep 17 00:00:00 2001 From: sudacode Date: Sun, 23 Aug 2026 01:56:49 -0700 Subject: [PATCH] fix(subtitles): improve secondary subtitle extraction and display (#215) --- .../dedupe-embedded-subtitle-extraction.md | 4 + ...remove-secondary-subtitle-display-limit.md | 4 + docs-site/mining-workflow.md | 4 +- docs/architecture/subtitle-overlay-priming.md | 11 +- src/main.ts | 14 +- src/main/main-wiring.test.ts | 20 ++- .../runtime/app-lifecycle-actions.test.ts | 11 +- src/main/runtime/app-lifecycle-actions.ts | 7 +- .../app-lifecycle-main-cleanup.test.ts | 4 + .../runtime/app-lifecycle-main-cleanup.ts | 2 + .../startup-lifecycle-composer.test.ts | 1 + .../internal-subtitle-extraction.test.ts | 60 ++++++++ .../runtime/internal-subtitle-extraction.ts | 79 +++++++++- src/main/runtime/network-media-path.test.ts | 64 ++++++++ src/main/runtime/network-media-path.ts | 142 ++++++++++++++++++ .../runtime/subtitle-prefetch-runtime.test.ts | 54 +++++++ src/main/runtime/subtitle-prefetch-runtime.ts | 12 +- src/renderer/style.css | 4 - src/renderer/subtitle-render.test.ts | 28 ++-- src/renderer/subtitle-render.ts | 15 +- 20 files changed, 503 insertions(+), 37 deletions(-) create mode 100644 changes/dedupe-embedded-subtitle-extraction.md create mode 100644 changes/remove-secondary-subtitle-display-limit.md create mode 100644 src/main/runtime/network-media-path.test.ts create mode 100644 src/main/runtime/network-media-path.ts diff --git a/changes/dedupe-embedded-subtitle-extraction.md b/changes/dedupe-embedded-subtitle-extraction.md new file mode 100644 index 00000000..413e489c --- /dev/null +++ b/changes/dedupe-embedded-subtitle-extraction.md @@ -0,0 +1,4 @@ +type: fixed +area: subtitles + +- Prevented embedded subtitle parsing from starving network playback: mounted SMB/NFS media now uses deduplicated mpv live text, while duplicate extraction requests for local media share one ffmpeg process. diff --git a/changes/remove-secondary-subtitle-display-limit.md b/changes/remove-secondary-subtitle-display-limit.md new file mode 100644 index 00000000..38d8c7f8 --- /dev/null +++ b/changes/remove-secondary-subtitle-display-limit.md @@ -0,0 +1,4 @@ +type: fixed +area: overlay + +- Secondary subtitle overlays now show every rendered line instead of clipping text after roughly four lines. diff --git a/docs-site/mining-workflow.md b/docs-site/mining-workflow.md index 8dd0c304..ea0d9d42 100644 --- a/docs-site/mining-workflow.md +++ b/docs-site/mining-workflow.md @@ -108,9 +108,11 @@ The secondary bar is a compact top-strip region in the same overlay window. It s - Quick comprehension checks without leaving the mining flow. - Auto-populating the translation field on mined cards - when a card is created, SubMiner uses the secondary subtitle text as the translation field value (unless AI translation is configured to override it). +For local media, SubMiner can parse supported embedded secondary tracks into timed cues. For remote URLs and files on network mounts, it uses mpv's live secondary subtitle text instead of scanning the media with ffmpeg. + It is controlled by `secondarySub` configuration and shares its lifecycle with the main overlay window. Cycle which track feeds it with `Shift+J`. -SubMiner collapses duplicate ASS layers in parsed secondary tracks. Long lines repeated as dialogue and positioned signs are treated as the same line when they differ only in whitespace or terminal punctuation. Dense multi-row sign layouts, such as translated timetables, are excluded instead of being concatenated into the secondary bar. When SubMiner must use mpv's live text as a fallback, it still filters full-line duplicates while preserving short repeated dialogue. +SubMiner collapses duplicate ASS layers in parsed secondary tracks. Exact repeated lines collapse at any length, while distinct simultaneous short lines remain separate. Long dialogue and positioned-sign copies also collapse when they differ only in whitespace or terminal punctuation. Dense multi-row sign layouts, such as translated timetables, are excluded instead of being concatenated into the secondary bar. ### Display Modes diff --git a/docs/architecture/subtitle-overlay-priming.md b/docs/architecture/subtitle-overlay-priming.md index 111ba67e..48be3fe4 100644 --- a/docs/architecture/subtitle-overlay-priming.md +++ b/docs/architecture/subtitle-overlay-priming.md @@ -97,11 +97,12 @@ coming and prefetching would otherwise idle for the rest of the cue. ## Secondary Subtitle Flow -- `secondary-sub-text` remains the immediate fallback, so unreadable and remote subtitle sources - still appear without waiting for file resolution. -- Parsed secondary text and the live fallback share a flattened-line identity for long lines. This - removes dialogue/sign repetitions that differ only in whitespace or terminal punctuation while - retaining short repeated lines that can represent authored dialogue without source metadata. +- `secondary-sub-text` remains the immediate fallback, so unreadable subtitle sources, remote URLs, + and files on network mounts still appear without waiting for file resolution. Embedded-track + extraction is skipped for those sources to avoid competing with playback for network bandwidth. +- Parsed secondary text and the live fallback remove exact repeated lines at any length. A + flattened-line identity also removes long dialogue/sign repetitions that differ only in + whitespace or terminal punctuation, while distinct simultaneous short lines remain separate. - `secondary-subtitle-track.ts` resolves `secondary-sid` against mpv's track list. External tracks are read directly; supported embedded text tracks are extracted through the same ffmpeg-backed source resolver used by primary subtitle prefetching. diff --git a/src/main.ts b/src/main.ts index 934e0dec..2fb555a3 100644 --- a/src/main.ts +++ b/src/main.ts @@ -588,9 +588,10 @@ import { import { buildSubtitleSidebarSourceKey } from './main/runtime/subtitle-prefetch-source'; import { createSubtitlePrefetchInitController } from './main/runtime/subtitle-prefetch-init'; import { + createCachedInternalSubtitleTrackExtractor, loadSubtitleSourceText, - extractInternalSubtitleTrackToTempFile, } from './main/runtime/internal-subtitle-extraction'; +import { createRemoteMediaPathDetector } from './main/runtime/network-media-path'; import { applyCharacterDictionarySelection } from './main/character-dictionary-selection'; import { getSubsyncConfig } from './subsync/utils'; @@ -2054,10 +2055,13 @@ const subtitlePrefetchInitController = createSubtitlePrefetchInitController({ } }, }); +const cachedInternalSubtitleTrackExtractor = createCachedInternalSubtitleTrackExtractor(); +const detectRemoteMediaPath = createRemoteMediaPathDetector(); const resolveActiveSubtitleSidebarSourceHandler = createResolveActiveSubtitleSidebarSourceHandler({ getFfmpegPath: () => configService.getConfig().subsync.ffmpeg_path.trim() || 'ffmpeg', + isRemoteMediaPath: detectRemoteMediaPath, extractInternalSubtitleTrack: (ffmpegPath, videoPath, track) => - extractInternalSubtitleTrackToTempFile(ffmpegPath, videoPath, track), + cachedInternalSubtitleTrackExtractor.extract(ffmpegPath, videoPath, track), logDebug: (message) => logger.debug(message), }); @@ -2086,8 +2090,8 @@ const refreshSubtitlePrefetchFromActiveTrackHandler = // Remote media has no extractable on-disk track to fall back to, so a transient // resolve miss (sid briefly 'no', a cycle onto an embedded stream track) would // otherwise drop a working cue list for the rest of the episode. - shouldKeepExistingCuesOnMissingSource: (videoPath) => - isYoutubeMediaPath(videoPath) || isRemoteMediaPath(videoPath), + shouldKeepExistingCuesOnMissingSource: async (videoPath) => + isYoutubeMediaPath(videoPath) || (await detectRemoteMediaPath(videoPath)), subtitlePrefetchInitController, resolveActiveSubtitleSidebarSource: (input) => resolveActiveSubtitleSidebarSourceHandler(input), logDebug: (message) => logger.debug(message), @@ -3962,6 +3966,7 @@ const { appState.yomitanSettingsWindow = null; }, stopJellyfinRemoteSession: () => stopJellyfinRemoteSession(), + cleanupInternalSubtitleTrackCache: () => cachedInternalSubtitleTrackExtractor.clear(), cleanupYoutubeSubtitleTempDirs: () => youtubeFlowRuntime.cleanupSubtitleTempDirs(), cleanupYoutubeMediaCache: () => youtubeMediaCache.cleanup(), cleanupJellyfinSubtitleCache: () => cleanupJellyfinSubtitleCache(), @@ -4522,6 +4527,7 @@ const { appState.activeParsedSubtitleMediaPath, ); if ((normalizedPath || null) !== previousPath) { + cachedInternalSubtitleTrackExtractor.clear(); secondarySubtitleTrackController.reset(); const resetSubtitlePayload = { text: '', tokens: null }; const frequencyDictionary = configService.getConfig().subtitleStyle.frequencyDictionary; diff --git a/src/main/main-wiring.test.ts b/src/main/main-wiring.test.ts index e536442e..9c9dbdd9 100644 --- a/src/main/main-wiring.test.ts +++ b/src/main/main-wiring.test.ts @@ -183,7 +183,10 @@ test('remote media keeps parsed cues when the active subtitle source cannot be r )?.groups?.body; assert.ok(actionBlock); - assert.match(actionBlock, /isYoutubeMediaPath\(videoPath\) \|\| isRemoteMediaPath\(videoPath\)/); + assert.match( + actionBlock, + /isYoutubeMediaPath\(videoPath\) \|\| \(await detectRemoteMediaPath\(videoPath\)\)/, + ); }); test('jellyfin subtitle preload seeds the tokenization prefetch directly', () => { @@ -860,3 +863,18 @@ test('subtitle sidebar snapshot prefers cached YouTube parsed cues before active snapshotBlock.indexOf('resolveActiveSubtitleSidebarSourceHandler'), ); }); + +test('main process guards internal subtitle extraction with the remote media detector', () => { + const source = readMainSource(); + const resolverWiring = source.match( + /const resolveActiveSubtitleSidebarSourceHandler = createResolveActiveSubtitleSidebarSourceHandler\(\{(?[\s\S]*?)\n\}\);/, + )?.groups?.body; + + assert.ok(resolverWiring); + assert.match(source, /const detectRemoteMediaPath = createRemoteMediaPathDetector\(\);/); + assert.match(resolverWiring, /isRemoteMediaPath:\s*detectRemoteMediaPath/); + assert.match( + resolverWiring, + /extractInternalSubtitleTrack:[\s\S]*cachedInternalSubtitleTrackExtractor\.extract/, + ); +}); diff --git a/src/main/runtime/app-lifecycle-actions.test.ts b/src/main/runtime/app-lifecycle-actions.test.ts index 11e75df0..5ab6227c 100644 --- a/src/main/runtime/app-lifecycle-actions.test.ts +++ b/src/main/runtime/app-lifecycle-actions.test.ts @@ -43,6 +43,7 @@ test('on will quit cleanup handler runs all cleanup steps', () => { destroyYomitanSettingsWindow: () => calls.push('destroy-yomitan-settings-window'), clearYomitanSettingsWindow: () => calls.push('clear-yomitan-settings-window'), stopJellyfinRemoteSession: () => calls.push('stop-jellyfin-remote'), + cleanupInternalSubtitleTrackCache: () => calls.push('cleanup-internal-subtitles'), cleanupYoutubeSubtitleTempDirs: () => calls.push('cleanup-youtube-subtitles'), cleanupYoutubeMediaCache: () => calls.push('cleanup-youtube-media'), cleanupJellyfinSubtitleCache: () => calls.push('cleanup-jellyfin-subtitles'), @@ -50,10 +51,11 @@ test('on will quit cleanup handler runs all cleanup steps', () => { }); cleanup(); - assert.equal(calls.length, 34); + assert.equal(calls.length, 35); assert.equal(calls[0], 'destroy-tray'); assert.equal(calls[calls.length - 1], 'stop-discord-presence'); assert.ok(calls.includes('cleanup-jellyfin-subtitles')); + assert.ok(calls.includes('cleanup-internal-subtitles')); assert.ok(calls.includes('clear-windows-visible-overlay-poll')); assert.ok(calls.includes('clear-linux-mpv-fullscreen-overlay-refresh-timeouts')); assert.ok(calls.includes('cleanup-youtube-subtitles')); @@ -97,6 +99,7 @@ test('on will quit cleanup handler cleans jellyfin subtitle cache when stopping calls.push('stop-jellyfin-remote'); throw new Error('stop failed'); }, + cleanupInternalSubtitleTrackCache: () => calls.push('cleanup-internal-subtitles'), cleanupYoutubeSubtitleTempDirs: () => calls.push('cleanup-youtube-subtitles'), cleanupYoutubeMediaCache: () => calls.push('cleanup-youtube-media'), cleanupJellyfinSubtitleCache: () => calls.push('cleanup-jellyfin-subtitles'), @@ -104,7 +107,11 @@ test('on will quit cleanup handler cleans jellyfin subtitle cache when stopping }); assert.throws(() => cleanup(), /stop failed/); - assert.deepEqual(calls, ['stop-jellyfin-remote', 'cleanup-jellyfin-subtitles']); + assert.deepEqual(calls, [ + 'stop-jellyfin-remote', + 'cleanup-jellyfin-subtitles', + 'cleanup-internal-subtitles', + ]); }); test('should restore windows on activate requires initialized runtime and no windows', () => { diff --git a/src/main/runtime/app-lifecycle-actions.ts b/src/main/runtime/app-lifecycle-actions.ts index 16f4130a..c0b77a0a 100644 --- a/src/main/runtime/app-lifecycle-actions.ts +++ b/src/main/runtime/app-lifecycle-actions.ts @@ -29,6 +29,7 @@ export function createOnWillQuitCleanupHandler(deps: { destroyYomitanSettingsWindow: () => void; clearYomitanSettingsWindow: () => void; stopJellyfinRemoteSession: () => void; + cleanupInternalSubtitleTrackCache: () => void; cleanupYoutubeSubtitleTempDirs: () => void; cleanupYoutubeMediaCache: () => void; cleanupJellyfinSubtitleCache: () => void; @@ -67,7 +68,11 @@ export function createOnWillQuitCleanupHandler(deps: { try { deps.stopJellyfinRemoteSession(); } finally { - deps.cleanupJellyfinSubtitleCache(); + try { + deps.cleanupJellyfinSubtitleCache(); + } finally { + deps.cleanupInternalSubtitleTrackCache(); + } } deps.cleanupYoutubeSubtitleTempDirs(); deps.cleanupYoutubeMediaCache(); diff --git a/src/main/runtime/app-lifecycle-main-cleanup.test.ts b/src/main/runtime/app-lifecycle-main-cleanup.test.ts index 373df2f9..f12df1e7 100644 --- a/src/main/runtime/app-lifecycle-main-cleanup.test.ts +++ b/src/main/runtime/app-lifecycle-main-cleanup.test.ts @@ -72,6 +72,7 @@ test('cleanup deps builder returns handlers that guard optional runtime objects' clearYomitanSettingsWindow: () => calls.push('clear-yomitan-settings-window'), stopJellyfinRemoteSession: () => calls.push('stop-jellyfin-remote'), + cleanupInternalSubtitleTrackCache: () => calls.push('cleanup-internal-subtitles'), cleanupYoutubeSubtitleTempDirs: () => calls.push('cleanup-youtube-subtitles'), cleanupYoutubeMediaCache: () => calls.push('cleanup-youtube-media'), cleanupJellyfinSubtitleCache: () => calls.push('cleanup-jellyfin-subtitles'), @@ -95,6 +96,7 @@ test('cleanup deps builder returns handlers that guard optional runtime objects' assert.ok(calls.includes('destroy-first-run-window')); assert.ok(calls.includes('destroy-yomitan-settings-window')); assert.ok(calls.includes('stop-jellyfin-remote')); + assert.ok(calls.includes('cleanup-internal-subtitles')); assert.ok(calls.includes('cleanup-youtube-subtitles')); assert.ok(calls.includes('cleanup-youtube-media')); assert.ok(calls.includes('cleanup-jellyfin-subtitles')); @@ -152,6 +154,7 @@ test('cleanup deps builder skips destroyed yomitan window', () => { getYomitanSettingsWindow: () => null, clearYomitanSettingsWindow: () => {}, stopJellyfinRemoteSession: () => {}, + cleanupInternalSubtitleTrackCache: () => {}, cleanupYoutubeSubtitleTempDirs: () => {}, cleanupYoutubeMediaCache: () => {}, cleanupJellyfinSubtitleCache: () => {}, @@ -204,6 +207,7 @@ test('cleanup deps builder skips global shortcut cleanup before app ready', () = getYomitanSettingsWindow: () => null, clearYomitanSettingsWindow: () => {}, stopJellyfinRemoteSession: () => {}, + cleanupInternalSubtitleTrackCache: () => {}, cleanupYoutubeSubtitleTempDirs: () => {}, cleanupYoutubeMediaCache: () => {}, cleanupJellyfinSubtitleCache: () => {}, diff --git a/src/main/runtime/app-lifecycle-main-cleanup.ts b/src/main/runtime/app-lifecycle-main-cleanup.ts index 54b751f7..249d1618 100644 --- a/src/main/runtime/app-lifecycle-main-cleanup.ts +++ b/src/main/runtime/app-lifecycle-main-cleanup.ts @@ -58,6 +58,7 @@ export function createBuildOnWillQuitCleanupDepsHandler(deps: { clearYomitanSettingsWindow: () => void; stopJellyfinRemoteSession: () => void; + cleanupInternalSubtitleTrackCache: () => void; cleanupYoutubeSubtitleTempDirs: () => void; cleanupYoutubeMediaCache: () => void; cleanupJellyfinSubtitleCache: () => void; @@ -144,6 +145,7 @@ export function createBuildOnWillQuitCleanupDepsHandler(deps: { }, clearYomitanSettingsWindow: () => deps.clearYomitanSettingsWindow(), stopJellyfinRemoteSession: () => deps.stopJellyfinRemoteSession(), + cleanupInternalSubtitleTrackCache: () => deps.cleanupInternalSubtitleTrackCache(), cleanupYoutubeSubtitleTempDirs: () => deps.cleanupYoutubeSubtitleTempDirs(), cleanupYoutubeMediaCache: () => deps.cleanupYoutubeMediaCache(), cleanupJellyfinSubtitleCache: () => deps.cleanupJellyfinSubtitleCache(), diff --git a/src/main/runtime/composers/startup-lifecycle-composer.test.ts b/src/main/runtime/composers/startup-lifecycle-composer.test.ts index b4db3e1c..fbe18443 100644 --- a/src/main/runtime/composers/startup-lifecycle-composer.test.ts +++ b/src/main/runtime/composers/startup-lifecycle-composer.test.ts @@ -49,6 +49,7 @@ test('composeStartupLifecycleHandlers returns callable startup lifecycle handler getYomitanSettingsWindow: () => null, clearYomitanSettingsWindow: () => {}, stopJellyfinRemoteSession: async () => {}, + cleanupInternalSubtitleTrackCache: () => {}, cleanupYoutubeSubtitleTempDirs: () => {}, cleanupYoutubeMediaCache: () => {}, cleanupJellyfinSubtitleCache: () => {}, diff --git a/src/main/runtime/internal-subtitle-extraction.test.ts b/src/main/runtime/internal-subtitle-extraction.test.ts index 658d4ea8..1aee8b87 100644 --- a/src/main/runtime/internal-subtitle-extraction.test.ts +++ b/src/main/runtime/internal-subtitle-extraction.test.ts @@ -6,6 +6,7 @@ import process from 'node:process'; import test from 'node:test'; import { buildFfmpegSubtitleExtractionArgs, + createCachedInternalSubtitleTrackExtractor, extractInternalSubtitleTrackToTempFile, parseTrackId, } from './internal-subtitle-extraction'; @@ -22,6 +23,65 @@ test('parseTrackId rejects negative track ids', () => { assert.equal(parseTrackId(' -2 '), null); }); +test('cached internal subtitle extraction shares concurrent and repeated track requests', async () => { + let extractionCalls = 0; + let cleanupCalls = 0; + let resolveExtraction: + | ((result: { path: string; cleanup: () => Promise }) => void) + | undefined; + const firstExtraction = new Promise<{ path: string; cleanup: () => Promise }>((resolve) => { + resolveExtraction = resolve; + }); + const extractor = createCachedInternalSubtitleTrackExtractor({ + extract: async () => { + extractionCalls += 1; + if (extractionCalls === 1) { + return firstExtraction; + } + return { + path: `/tmp/subtitle-${extractionCalls}.ass`, + cleanup: async () => { + cleanupCalls += 1; + }, + }; + }, + }); + const request = () => + extractor.extract('ffmpeg', '/Volumes/media/episode.mkv', { + 'ff-index': 3, + codec: 'ass', + }); + + const concurrent = Array.from({ length: 6 }, request); + assert.equal(extractionCalls, 1); + if (!resolveExtraction) { + throw new Error('extraction did not start'); + } + resolveExtraction({ + path: '/tmp/subtitle-1.ass', + cleanup: async () => { + cleanupCalls += 1; + }, + }); + + const results = await Promise.all(concurrent); + assert.deepEqual( + results.map((result) => result?.path), + Array.from({ length: 6 }, () => '/tmp/subtitle-1.ass'), + ); + await Promise.all(results.map((result) => result?.cleanup())); + assert.equal(cleanupCalls, 0); + + assert.equal((await request())?.path, '/tmp/subtitle-1.ass'); + assert.equal(extractionCalls, 1); + + extractor.clear(); + await new Promise((resolve) => setImmediate(resolve)); + assert.equal(cleanupCalls, 1); + assert.equal((await request())?.path, '/tmp/subtitle-2.ass'); + assert.equal(extractionCalls, 2); +}); + test('extractInternalSubtitleTrackToTempFile times out stalled ffmpeg process', async () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'subminer-ffmpeg-timeout-')); const videoPath = path.join(root, 'video.mkv'); diff --git a/src/main/runtime/internal-subtitle-extraction.ts b/src/main/runtime/internal-subtitle-extraction.ts index d303466d..a19556ec 100644 --- a/src/main/runtime/internal-subtitle-extraction.ts +++ b/src/main/runtime/internal-subtitle-extraction.ts @@ -35,6 +35,17 @@ export type MpvSubtitleTrackLike = { 'external-filename'?: unknown; }; +export type ExtractedInternalSubtitleTrack = { + path: string; + cleanup: () => Promise; +}; + +export type InternalSubtitleTrackExtractor = ( + ffmpegPath: string, + videoPath: string, + track: MpvSubtitleTrackLike, +) => Promise; + const DEFAULT_EXTRACTION_TIMEOUT_MS = 30_000; export function parseTrackId(value: unknown): number | null { @@ -80,7 +91,7 @@ export async function extractInternalSubtitleTrackToTempFile( videoPath: string, track: MpvSubtitleTrackLike, options: { extractionTimeoutMs?: number; spawnArgsOverride?: string[] } = {}, -): Promise<{ path: string; cleanup: () => Promise } | null> { +): Promise { const ffIndex = parseTrackId(track['ff-index']); const codec = typeof track.codec === 'string' ? track.codec : null; const extension = codecToExtension(codec ?? undefined); @@ -145,3 +156,69 @@ export async function extractInternalSubtitleTrackToTempFile( }, }; } + +type CachedExtraction = { + promise: Promise; +}; + +function buildCachedExtractionKey( + ffmpegPath: string, + videoPath: string, + track: MpvSubtitleTrackLike, +): string { + const codec = typeof track.codec === 'string' ? track.codec : null; + return JSON.stringify([ffmpegPath, videoPath, parseTrackId(track['ff-index']), codec]); +} + +const releaseCachedExtraction = async (): Promise => {}; + +/** + * Owns extracted subtitle files for the active media and shares one extraction between callers. + * Caller cleanup releases only its view; clear removes the owned files on media changes or quit. + */ +export function createCachedInternalSubtitleTrackExtractor( + deps: { extract?: InternalSubtitleTrackExtractor } = {}, +): { + extract: InternalSubtitleTrackExtractor; + clear: () => void; +} { + const extractTrack = deps.extract ?? extractInternalSubtitleTrackToTempFile; + const extractions = new Map(); + + const extract: InternalSubtitleTrackExtractor = async (ffmpegPath, videoPath, track) => { + const key = buildCachedExtractionKey(ffmpegPath, videoPath, track); + let cached = extractions.get(key); + if (!cached) { + const next: CachedExtraction = { + promise: extractTrack(ffmpegPath, videoPath, track), + }; + cached = next; + extractions.set(key, next); + void next.promise.catch(() => { + if (extractions.get(key) === next) { + extractions.delete(key); + } + }); + } + + const result = await cached.promise; + if (extractions.get(key) !== cached || !result) { + return null; + } + + return { + path: result.path, + cleanup: releaseCachedExtraction, + }; + }; + + const clear = (): void => { + const staleExtractions = [...extractions.values()]; + extractions.clear(); + for (const extraction of staleExtractions) { + void extraction.promise.then((result) => result?.cleanup()).catch(() => undefined); + } + }; + + return { extract, clear }; +} diff --git a/src/main/runtime/network-media-path.test.ts b/src/main/runtime/network-media-path.test.ts new file mode 100644 index 00000000..002667f1 --- /dev/null +++ b/src/main/runtime/network-media-path.test.ts @@ -0,0 +1,64 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { createRemoteMediaPathDetector } from './network-media-path'; + +test('remote media detector recognizes mounted network filesystems', async () => { + const detectRemoteMedia = createRemoteMediaPathDetector({ + platform: 'darwin', + readMountOutput: async () => + [ + '/dev/disk3s5 on /System/Volumes/Data (apfs, local, journaled)', + '//viewer@media/jellyfin on /Volumes/jellyfin (smbfs, nodev, nosuid)', + ].join('\n'), + }); + + assert.equal(await detectRemoteMedia('/Volumes/jellyfin/movie.mkv'), true); + assert.equal(await detectRemoteMedia('/Volumes/jellyfin-another/movie.mkv'), false); + assert.equal(await detectRemoteMedia('/Users/viewer/movie.mkv'), false); +}); + +test('remote media detector recognizes Linux network mount output', async () => { + const detectRemoteMedia = createRemoteMediaPathDetector({ + platform: 'linux', + readMountOutput: async () => + '//media/jellyfin on /mnt/Jellyfin\\040Media type cifs (rw,relatime)', + }); + + assert.equal(await detectRemoteMedia('/mnt/Jellyfin Media/movie.mkv'), true); +}); + +test('remote media detector shares its mount lookup between concurrent callers', async () => { + let mountReads = 0; + const detectRemoteMedia = createRemoteMediaPathDetector({ + platform: 'darwin', + readMountOutput: async () => { + mountReads += 1; + return '//viewer@media/jellyfin on /Volumes/jellyfin (smbfs, nodev, nosuid)'; + }, + }); + + const results = await Promise.all( + Array.from({ length: 6 }, () => detectRemoteMedia('/Volumes/jellyfin/movie.mkv')), + ); + + assert.deepEqual( + results, + Array.from({ length: 6 }, () => true), + ); + assert.equal(mountReads, 1); +}); + +test('remote media detector recognizes URLs and Windows UNC paths without reading mounts', async () => { + let mountReads = 0; + const detectRemoteMedia = createRemoteMediaPathDetector({ + platform: 'win32', + readMountOutput: async () => { + mountReads += 1; + return ''; + }, + }); + + assert.equal(await detectRemoteMedia('https://media.example/movie.mkv'), true); + assert.equal(await detectRemoteMedia('\\\\media-server\\jellyfin\\movie.mkv'), true); + assert.equal(mountReads, 0); +}); diff --git a/src/main/runtime/network-media-path.ts b/src/main/runtime/network-media-path.ts new file mode 100644 index 00000000..9f282821 --- /dev/null +++ b/src/main/runtime/network-media-path.ts @@ -0,0 +1,142 @@ +import { execFile } from 'node:child_process'; +import path from 'node:path'; +import process from 'node:process'; +import { resolveSubtitleSourcePath } from './subtitle-prefetch-source'; + +const DEFAULT_MOUNT_CACHE_TTL_MS = 5_000; +const NETWORK_FILESYSTEM_TYPES = new Set([ + '9p', + 'afpfs', + 'cifs', + 'davfs', + 'davfs2', + 'fuse.sshfs', + 'nfs', + 'nfs4', + 'smbfs', + 'sshfs', + 'webdav', +]); + +function isRemoteUrl(value: string): boolean { + try { + const url = new URL(value); + return url.protocol === 'http:' || url.protocol === 'https:'; + } catch { + return false; + } +} + +function decodeMountPath(value: string): string { + return value.replace(/\\([0-7]{3})/g, (_match, digits: string) => + String.fromCharCode(Number.parseInt(digits, 8)), + ); +} + +function parseNetworkMountPaths(output: string): string[] { + const networkMountPaths: string[] = []; + for (const line of output.split('\n')) { + const optionsStart = line.lastIndexOf(' ('); + if (optionsStart < 0) continue; + + let mountDescription = line.slice(0, optionsStart); + const options = line.slice(optionsStart + 2, line.indexOf(')', optionsStart)); + const linuxTypeSeparator = mountDescription.lastIndexOf(' type '); + const filesystemType = ( + linuxTypeSeparator >= 0 + ? mountDescription.slice(linuxTypeSeparator + ' type '.length) + : (options.split(',').at(0) ?? '') + ) + .trim() + .toLowerCase(); + if (!NETWORK_FILESYSTEM_TYPES.has(filesystemType)) continue; + + if (linuxTypeSeparator >= 0) { + mountDescription = mountDescription.slice(0, linuxTypeSeparator); + } + const mountSeparator = mountDescription.indexOf(' on '); + if (mountSeparator < 0) continue; + networkMountPaths.push( + path.posix.normalize(decodeMountPath(mountDescription.slice(mountSeparator + 4).trim())), + ); + } + return networkMountPaths; +} + +function readMountOutput(platform: NodeJS.Platform): Promise { + if (platform === 'win32') return Promise.resolve(''); + const command = platform === 'darwin' ? '/sbin/mount' : 'mount'; + return new Promise((resolve, reject) => { + execFile( + command, + [], + { encoding: 'utf8', timeout: 1_000, maxBuffer: 1024 * 1024 }, + (error, stdout) => { + if (error) { + reject(error); + return; + } + resolve(stdout); + }, + ); + }); +} + +function isPathWithinMount(filePath: string, mountPath: string): boolean { + const relativePath = path.posix.relative(mountPath, filePath); + return ( + relativePath === '' || + (relativePath !== '..' && + !relativePath.startsWith(`..${path.posix.sep}`) && + !path.posix.isAbsolute(relativePath)) + ); +} + +export type RemoteMediaPathDetector = (mediaPath: string) => Promise; + +export function createRemoteMediaPathDetector( + deps: { + platform?: NodeJS.Platform; + readMountOutput?: () => Promise; + now?: () => number; + mountCacheTtlMs?: number; + } = {}, +): RemoteMediaPathDetector { + const platform = deps.platform ?? process.platform; + const getMountOutput = deps.readMountOutput ?? (() => readMountOutput(platform)); + const now = deps.now ?? Date.now; + const mountCacheTtlMs = deps.mountCacheTtlMs ?? DEFAULT_MOUNT_CACHE_TTL_MS; + let mountCache: { expiresAt: number; networkMountPaths: Promise } | undefined; + + const getNetworkMountPaths = (): Promise => { + const currentTime = now(); + if (mountCache && currentTime < mountCache.expiresAt) { + return mountCache.networkMountPaths; + } + + const networkMountPaths = getMountOutput() + .then(parseNetworkMountPaths) + .catch(() => []); + mountCache = { + expiresAt: currentTime + mountCacheTtlMs, + networkMountPaths, + }; + return networkMountPaths; + }; + + return async (mediaPath): Promise => { + const source = mediaPath.trim(); + if (!source) return false; + if (isRemoteUrl(source)) return true; + + const filePath = resolveSubtitleSourcePath(source); + if (platform === 'win32') { + return filePath.startsWith('\\\\'); + } + if (!path.posix.isAbsolute(filePath)) return false; + + const networkMountPaths = await getNetworkMountPaths(); + const normalizedPath = path.posix.normalize(filePath); + return networkMountPaths.some((mountPath) => isPathWithinMount(normalizedPath, mountPath)); + }; +} diff --git a/src/main/runtime/subtitle-prefetch-runtime.test.ts b/src/main/runtime/subtitle-prefetch-runtime.test.ts index a5780680..0fbc68e7 100644 --- a/src/main/runtime/subtitle-prefetch-runtime.test.ts +++ b/src/main/runtime/subtitle-prefetch-runtime.test.ts @@ -101,6 +101,32 @@ test('subtitle prefetch runtime preserves parsed cues when YouTube active track assert.deepEqual(calls, []); }); +test('subtitle prefetch runtime preserves parsed cues when a network mount source is unresolved', async () => { + const calls: string[] = []; + const refresh = createRefreshSubtitlePrefetchFromActiveTrackHandler({ + getMpvClient: () => ({ + connected: true, + requestProperty: async (name) => (name === 'path' ? '/Volumes/jellyfin/movie.mkv' : null), + }), + getLastObservedTimePos: () => 12, + subtitlePrefetchInitController: { + cancelPendingInit: () => { + calls.push('cancel'); + }, + initSubtitlePrefetch: async () => { + calls.push('init'); + }, + }, + resolveActiveSubtitleSidebarSource: async () => null, + shouldKeepExistingCuesOnMissingSource: async (videoPath) => + videoPath.startsWith('/Volumes/jellyfin/'), + }); + + await refresh(); + + assert.deepEqual(calls, []); +}); + test('subtitle prefetch runtime does not extract internal subtitle tracks from remote media urls', async () => { let extracted = false; const resolveSource = createResolveActiveSubtitleSidebarSourceHandler({ @@ -131,6 +157,34 @@ test('subtitle prefetch runtime does not extract internal subtitle tracks from r assert.equal(extracted, false); }); +test('subtitle prefetch runtime does not extract internal subtitle tracks from network mounts', async () => { + let extracted = false; + const resolveSource = createResolveActiveSubtitleSidebarSourceHandler({ + getFfmpegPath: () => 'ffmpeg-custom', + isRemoteMediaPath: async (videoPath) => videoPath.startsWith('/Volumes/jellyfin/'), + extractInternalSubtitleTrack: async () => { + extracted = true; + return null; + }, + }); + + const resolved = await resolveSource({ + currentExternalFilenameRaw: null, + currentTrackRaw: { + type: 'sub', + id: 3, + 'ff-index': 7, + codec: 'ass', + }, + trackListRaw: [], + sidRaw: 3, + videoPath: '/Volumes/jellyfin/movie.mkv', + }); + + assert.equal(resolved, null); + assert.equal(extracted, false); +}); + test('subtitle prefetch refresh logs a warning when source resolution throws', async () => { const warnings: string[] = []; const refresh = createRefreshSubtitlePrefetchFromActiveTrackHandler({ diff --git a/src/main/runtime/subtitle-prefetch-runtime.ts b/src/main/runtime/subtitle-prefetch-runtime.ts index 098697a9..e82eedba 100644 --- a/src/main/runtime/subtitle-prefetch-runtime.ts +++ b/src/main/runtime/subtitle-prefetch-runtime.ts @@ -17,6 +17,8 @@ type ActiveSubtitleSidebarSource = { cleanup?: () => Promise; }; +type RemoteMediaPathDetector = (mediaPath: string) => boolean | Promise; + function parseTrackId(value: unknown): number | null { if (typeof value === 'number' && Number.isInteger(value)) { return value; @@ -28,7 +30,7 @@ function parseTrackId(value: unknown): number | null { return null; } -function isRemoteMediaPath(value: string): boolean { +function isRemoteMediaUrl(value: string): boolean { try { const url = new URL(value); return url.protocol === 'http:' || url.protocol === 'https:'; @@ -86,6 +88,7 @@ function getActiveSubtitleTrack( export function createResolveActiveSubtitleSidebarSourceHandler(deps: { getFfmpegPath: () => string; + isRemoteMediaPath?: RemoteMediaPathDetector; extractInternalSubtitleTrack: ( ffmpegPath: string, videoPath: string, @@ -126,7 +129,8 @@ export function createResolveActiveSubtitleSidebarSourceHandler(deps: { return { path: externalFilename, sourceKey: externalFilename }; } - if (isRemoteMediaPath(input.videoPath)) { + const isRemoteMediaPath = deps.isRemoteMediaPath ?? isRemoteMediaUrl; + if (await isRemoteMediaPath(input.videoPath)) { deps.logDebug?.('[subtitle-prefetch] skipping internal subtitle extraction for remote media'); return null; } @@ -156,7 +160,7 @@ export function createRefreshSubtitlePrefetchFromActiveTrackHandler(deps: { requestProperty: (name: string) => Promise; } | null; getLastObservedTimePos: () => number; - shouldKeepExistingCuesOnMissingSource?: (videoPath: string) => boolean; + shouldKeepExistingCuesOnMissingSource?: (videoPath: string) => boolean | Promise; subtitlePrefetchInitController: SubtitlePrefetchInitController; resolveActiveSubtitleSidebarSource: ( input: Parameters>[0], @@ -195,7 +199,7 @@ export function createRefreshSubtitlePrefetchFromActiveTrackHandler(deps: { videoPath, }); if (!resolvedSource) { - if (deps.shouldKeepExistingCuesOnMissingSource?.(videoPath) === true) { + if ((await deps.shouldKeepExistingCuesOnMissingSource?.(videoPath)) === true) { deps.logDebug?.( '[subtitle-prefetch] no active subtitle source resolved; keeping existing cues', ); diff --git a/src/renderer/style.css b/src/renderer/style.css index f1aa7704..e4977cdc 100644 --- a/src/renderer/style.css +++ b/src/renderer/style.css @@ -1928,10 +1928,6 @@ body.layer-modal #overlay { text-align: center; font-size: 24px; line-height: 1.5; - /* Backstop: pathological tracks (karaoke typesetting, sign spam) must never grow - the hover-pause band beyond a top strip. ~4 lines at line-height 1.5. */ - max-height: 6em; - overflow: hidden; color: #ffffff; -webkit-text-stroke: 0.45px rgba(0, 0, 0, 0.7); paint-order: stroke fill; diff --git a/src/renderer/subtitle-render.test.ts b/src/renderer/subtitle-render.test.ts index 64a645e8..fe798778 100644 --- a/src/renderer/subtitle-render.test.ts +++ b/src/renderer/subtitle-render.test.ts @@ -1424,11 +1424,8 @@ test('subtitle annotation CSS underlines JLPT tokens without changing token colo ); }); -test('prepareSecondarySubtitleLines preserves short stacks without layer metadata', () => { +test('prepareSecondarySubtitleLines collapses exact short copies in stacks', () => { assert.deepEqual(prepareSecondarySubtitleLines('Your\\NYour\\NYour\\NYour\\Nmosaic'), [ - 'Your', - 'Your', - 'Your', 'Your', 'mosaic', ]); @@ -1438,6 +1435,15 @@ test('prepareSecondarySubtitleLines preserves short stacks without layer metadat ]); }); +test('prepareSecondarySubtitleLines collapses exact short sign copies beside dialogue', () => { + const liveText = "And for today's sports festival...\nEntrance\nEntrance"; + + assert.deepEqual(prepareSecondarySubtitleLines(liveText), [ + "And for today's sports festival...", + 'Entrance', + ]); +}); + test('prepareSecondarySubtitleLines collapses karaoke syllable spam into one deduped line', () => { // Karaoke-typeset OP/ED: one ASS event per syllable, duplicated across layers, // joined with \N by mpv's secondary-sub-text. @@ -1448,10 +1454,10 @@ test('prepareSecondarySubtitleLines collapses karaoke syllable spam into one ded assert.deepEqual(prepareSecondarySubtitleLines(karaoke), ['ya This no ma ups']); }); -test('prepareSecondarySubtitleLines preserves repeated short dialogue without layer metadata', () => { +test('prepareSecondarySubtitleLines collapses exact repeated short lines', () => { const dialogue = ['Wait', 'Wait', 'Wait']; - assert.deepEqual(prepareSecondarySubtitleLines(dialogue.join('\\N')), dialogue); + assert.deepEqual(prepareSecondarySubtitleLines(dialogue.join('\\N')), ['Wait']); }); test('prepareSecondarySubtitleLines collapses punctuation variants of a full-sentence fallback', () => { @@ -1469,6 +1475,10 @@ test('prepareSecondarySubtitleLines preserves short simultaneous dialogue withou assert.deepEqual(prepareSecondarySubtitleLines(dialogue.join('\\N')), dialogue); }); +test('prepareSecondarySubtitleLines preserves distinct short lines with internal whitespace', () => { + assert.deepEqual(prepareSecondarySubtitleLines('AB\\NA B'), ['AB', 'A B']); +}); + test('prepareSecondarySubtitleLines keeps normal dialogue lines intact', () => { const dialogue = ' I never expected this. \\N\\N But here we are. '; @@ -1490,13 +1500,13 @@ test('prepareSecondarySubtitleLines strips ASS override tags and handles empty i assert.deepEqual(prepareSecondarySubtitleLines('{\\an8}'), []); }); -test('secondary subtitle root CSS caps height so hover-pause band stays a top strip', () => { +test('secondary subtitle root CSS does not clip long subtitle stacks', () => { const srcCssPath = path.join(process.cwd(), 'src', 'renderer', 'style.css'); const cssText = fs.readFileSync(srcCssPath, 'utf-8'); const secondaryRootBlock = extractClassBlock(cssText, '#secondarySubRoot'); - assert.match(secondaryRootBlock, /max-height:\s*6em;/); - assert.match(secondaryRootBlock, /overflow:\s*hidden;/); + assert.doesNotMatch(secondaryRootBlock, /max-height\s*:/); + assert.doesNotMatch(secondaryRootBlock, /overflow\s*:\s*hidden/); }); test('applySubtitleStyle sets known-word maturity color variables', () => { diff --git a/src/renderer/subtitle-render.ts b/src/renderer/subtitle-render.ts index 12e9bc04..9e722deb 100644 --- a/src/renderer/subtitle-render.ts +++ b/src/renderer/subtitle-render.ts @@ -667,12 +667,17 @@ function isKaraokeLikeLineSet(lines: string[]): boolean { } function collapseFullLineFallbackCopies(lines: string[]): string[] { - const seen = new Set(); + const seenExact = new Set(); + const seenFlattened = new Set(); return lines.filter((line) => { - const identity = flattenedSecondarySubtitleLineIdentity(line); - if (!identity) return true; - if (seen.has(identity)) return false; - seen.add(identity); + const exactIdentity = line.normalize('NFKC'); + if (seenExact.has(exactIdentity)) return false; + seenExact.add(exactIdentity); + + const flattenedIdentity = flattenedSecondarySubtitleLineIdentity(line); + if (!flattenedIdentity) return true; + if (seenFlattened.has(flattenedIdentity)) return false; + seenFlattened.add(flattenedIdentity); return true; }); }