From c2bd1d28fdcdb455978175ad1f7e8e1c8637ad89 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Mon, 10 Aug 2026 21:23:55 +0200 Subject: [PATCH] fix(subtitles): load external subtitle files with the media whether or not selected Since a1b6a8971 only the selected sidecar attached at open, so mpv's track-list carried one external subtitle and the track sheet could only offer the rest as primary source switches - tap-and-hold on a non-selected external track selected it as primary instead of secondary. Real external files are cheap static fetches, so Jellyfin, Plex direct play, and offline discovery now mark them preload and they ride along in sub-files at open, keeping every external track selectable as a secondary subtitle without a reopen. Embedded rows extracted on a transcode stay lazy: extraction can stall behind the transcoder, which is exactly what used to trip the sidecar open guard. close #1860 --- .../jellyfin_client/parts/playback.dart | 7 ++ .../playback_initialization_service.dart | 3 + lib/services/plex_client.dart | 4 + test/services/jellyfin_client_urls_test.dart | 16 ++++ ...ack_initialization_offline_cache_test.dart | 5 ++ .../playback_subtitle_resolver_test.dart | 23 ++++++ .../plex_playback_data_request_test.dart | 75 +++++++++++++++++++ 7 files changed, 133 insertions(+) diff --git a/lib/services/jellyfin_client/parts/playback.dart b/lib/services/jellyfin_client/parts/playback.dart index bcbb8b03..ae77ebbf 100644 --- a/lib/services/jellyfin_client/parts/playback.dart +++ b/lib/services/jellyfin_client/parts/playback.dart @@ -500,6 +500,13 @@ mixin _JellyfinPlaybackMethods on _JellyfinClientInternals { externalSubtitles.add( PlaybackSubtitleSidecar( sourceStreamId: track.id, + // A real external file is a cheap static fetch, so it loads with the + // media whether or not it is selected — that is what lets the track + // sheet offer it as a secondary subtitle without a reopen (#1860). + // An embedded row extracted on a transcode stays lazy: extraction can + // stall while the transcoder spins up, which is exactly what used to + // trip the sidecar open guard (#1738). + preload: track.isExternalFile, track: SubtitleTrack.uri( url, title: diff --git a/lib/services/playback_initialization_service.dart b/lib/services/playback_initialization_service.dart index ad759788..29ddf4d9 100644 --- a/lib/services/playback_initialization_service.dart +++ b/lib/services/playback_initialization_service.dart @@ -246,6 +246,9 @@ class PlaybackInitializationService { subtitles.add( PlaybackSubtitleSidecar( sourceStreamId: trackId, + // Local files cost nothing to attach, and preloading keeps every + // downloaded sidecar selectable as a secondary subtitle (#1860). + preload: true, track: SubtitleTrack.uri( Uri.file(entity.path).toString(), title: cachedTrack?.displayTitle ?? cachedTrack?.language ?? t.videoControls.subtitleFile(name: fileName), diff --git a/lib/services/plex_client.dart b/lib/services/plex_client.dart index 1bd0fabc..a2c30dcc 100644 --- a/lib/services/plex_client.dart +++ b/lib/services/plex_client.dart @@ -3946,6 +3946,10 @@ class PlexClient externalSubtitles.add( PlaybackSubtitleSidecar( sourceStreamId: plexTrack.id, + // Every row here is a real external file: preload it with the + // media so the non-selected tracks stay selectable as secondary + // subtitles without a reopen (#1860). + preload: true, track: SubtitleTrack.uri( url, title: diff --git a/test/services/jellyfin_client_urls_test.dart b/test/services/jellyfin_client_urls_test.dart index ccf63d97..96664ff0 100644 --- a/test/services/jellyfin_client_urls_test.dart +++ b/test/services/jellyfin_client_urls_test.dart @@ -828,6 +828,11 @@ void main() { // Nothing is selected and the fixture declares no default, so the server burns nothing and // this embedded row stays fetchable as an extracted file. expect(result.subtitleSidecars.single.sourceStreamId, 2); + expect( + result.subtitleSidecars.single.preload, + isFalse, + reason: 'an extracted embedded row stays lazy: extraction can stall behind the transcoder (#1738)', + ); expect(result.externalSubtitles.single.title, 'English'); expect(result.externalSubtitles.single.language, 'eng'); final subtitleUri = Uri.parse(result.externalSubtitles.single.uri!); @@ -1367,6 +1372,11 @@ void main() { expect(sidecarRow.key, '/Videos/item-1/src-1/Subtitles/5/0/Stream.srt'); expect(sidecarRow.isExternalFile, isTrue); expect(result.subtitleSidecars.map((sidecar) => sidecar.sourceStreamId), [5]); + expect( + result.subtitleSidecars.single.preload, + isTrue, + reason: 'a real file loads with the media so it stays selectable as secondary (#1860)', + ); // The server default survives normalization so selection can honour it. expect(result.mediaInfo!.defaultSubtitleStreamIndex, 3); @@ -1465,6 +1475,7 @@ void main() { expect(result.mediaInfo!.subtitleTracks, hasLength(1)); expect(result.externalSubtitles, hasLength(1)); expect(result.subtitleSidecars.single.sourceStreamId, 3); + expect(result.subtitleSidecars.single.preload, isTrue); expect(result.externalSubtitles.single.title, 'English'); final subtitleUri = Uri.parse(result.externalSubtitles.single.uri!); expect(subtitleUri.path, '/Videos/item-1/src-1/Subtitles/3/Stream.srt'); @@ -1617,6 +1628,11 @@ void main() { // one row not fetched: a sidecar for it would paint a second copy over the burned pixels. // The other two stay fetchable, which is what keeps a secondary track renderable. expect(result.subtitleSidecars.map((sidecar) => sidecar.sourceStreamId), [3, 5]); + expect( + result.subtitleSidecars.map((sidecar) => sidecar.preload), + everyElement(isFalse), + reason: 'extraction-backed rows must not gate the open on the transcoder', + ); }); test('getPlaybackInitialization ignores TranscodingUrl for original playback static fallback', () async { diff --git a/test/services/playback_initialization_offline_cache_test.dart b/test/services/playback_initialization_offline_cache_test.dart index ba655aec..361658b0 100644 --- a/test/services/playback_initialization_offline_cache_test.dart +++ b/test/services/playback_initialization_offline_cache_test.dart @@ -356,6 +356,11 @@ void main() { expect(result.videoUrl, 'content://offline/movie-1'); expect(result.externalSubtitles, hasLength(1)); expect(result.externalSubtitles.single.uri, Uri.file(subtitlePath).toString()); + expect( + result.subtitleSidecars.single.preload, + isTrue, + reason: 'local sidecars load with the media so they stay selectable as secondary subtitles (#1860)', + ); }); test('cache-only playback extras fills missing Plex marker types from chapters', () async { diff --git a/test/services/playback_subtitle_resolver_test.dart b/test/services/playback_subtitle_resolver_test.dart index 2a6307f6..3fc84eb4 100644 --- a/test/services/playback_subtitle_resolver_test.dart +++ b/test/services/playback_subtitle_resolver_test.dart @@ -228,6 +228,29 @@ void main() { expect(result.sidecarsAtOpen.single.uri, 'https://example.test/subtitles/2.srt'); }); + test('a preloaded sidecar attaches at open even when not selected', () { + // The clients mark real external files preload, so the non-selected file + // still loads with the media and stays selectable as a secondary subtitle + // without a reopen (#1860). + final result = PlaybackSubtitleResolver.resolve( + metadata: metadata, + mediaInfo: _mediaInfo([ + _sourceSubtitle(2, selected: true, usesExternalDelivery: true), + _sourceSubtitle(3, language: 'swe', usesExternalDelivery: true), + ]), + sidecars: [ + _sidecar(2, preload: true), + _sidecar(3, language: 'swe', preload: true), + ], + ); + + expect(result.primarySourceStreamId, 2); + expect(result.sidecarsAtOpen.map((track) => track.uri), [ + 'https://example.test/subtitles/2.srt', + 'https://example.test/subtitles/3.srt', + ]); + }); + test('a transcode drops a carried secondary it cannot deliver', () { // The burn covers the primary, and an embedded secondary has neither a sidecar to fetch nor a // native track to land on. Kept selected, it made `TrackManager` wait out its thirty-second diff --git a/test/services/plex_playback_data_request_test.dart b/test/services/plex_playback_data_request_test.dart index 7c84116c..3c01111a 100644 --- a/test/services/plex_playback_data_request_test.dart +++ b/test/services/plex_playback_data_request_test.dart @@ -377,6 +377,81 @@ void main() { expect(result.subtitleSidecars.single.track.uri, contains('/library/streams/402.srt')); }); + test('direct play preloads every external subtitle file, not just the selected one', () async { + // Two sidecar files next to the video: only one is selected, but both must + // load with the media so the other stays selectable as a secondary + // subtitle without a reopen (#1860). The embedded row is the container's + // job on direct play and gets no sidecar. + final client = makeClient((request) async { + if (request.url.path == '/library/metadata/42') { + return http.Response( + jsonEncode({ + 'MediaContainer': { + 'Metadata': [ + { + 'ratingKey': '42', + 'type': 'movie', + 'title': 'Movie', + 'Media': [ + { + 'id': 7, + 'container': 'mp4', + 'Part': [ + { + 'id': 99, + 'key': '/library/parts/99/file.mp4', + 'Stream': [ + {'streamType': 1, 'id': 300, 'codec': 'h264'}, + {'streamType': 2, 'id': 301, 'index': 0, 'languageCode': 'eng', 'selected': true}, + { + 'streamType': 3, + 'id': 401, + 'index': 1, + 'codec': 'srt', + 'languageCode': 'deu', + 'key': '/library/streams/401', + 'external': true, + 'selected': true, + }, + { + 'streamType': 3, + 'id': 402, + 'index': 2, + 'codec': 'srt', + 'languageCode': 'fra', + 'key': '/library/streams/402', + 'external': true, + }, + {'streamType': 3, 'id': 403, 'index': 3, 'codec': 'ass', 'languageCode': 'eng'}, + ], + }, + ], + }, + ], + }, + ], + }, + }), + 200, + headers: {'content-type': 'application/json'}, + ); + } + return http.Response('unexpected request', 500); + }); + addTearDown(client.close); + + final result = await client.getPlaybackInitialization( + PlaybackInitializationOptions( + metadata: testMediaItem(id: '42', backend: MediaBackend.plex, kind: MediaKind.movie, serverId: 'server-id'), + selectedMediaIndex: 0, + ), + ); + + expect(result.playMethod, 'DirectPlay'); + expect(result.subtitleSidecars.map((sidecar) => sidecar.sourceStreamId), [401, 402]); + expect(result.subtitleSidecars.map((sidecar) => sidecar.preload), everyElement(isTrue)); + }); + test('playback uses metadata availability flags without probing part URLs', () async { final requests = []; final client = makeClient((request) async {