diff --git a/lib/screens/video_player/parts/episode_navigation.dart b/lib/screens/video_player/parts/episode_navigation.dart index 2ec0cbde..923dd35a 100644 --- a/lib/screens/video_player/parts/episode_navigation.dart +++ b/lib/screens/video_player/parts/episode_navigation.dart @@ -159,6 +159,7 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { hasCommittedSelection: committedSubtitleSelection != null, committedTrack: committedSubtitleSelection?.primaryTrack, nativeTrack: currentPlayer.state.track.subtitle, + declinedPreference: committedSubtitleSelection?.declinedPreference, ); final secondarySubtitlePreference = followServerSelections ? null @@ -771,7 +772,11 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { plexClient: plexClient, getProfileSettings: () => userProfileProvider.profileSettings, preferredAudioTrack: currentAudioTrack, - preferredSubtitleTrack: SubtitlePreference.trackOrNull(subtitleSelection.primaryTrack), + // A declined carry stays alive for the native passes: freezing the + // resolver's off verdict here would turn a metadata mismatch into a + // navigation-priority off that no late track can undo (#1785). + preferredSubtitleTrack: + subtitleSelection.declinedPreference ?? SubtitlePreference.trackOrNull(subtitleSelection.primaryTrack), preferredSecondarySubtitleTrack: SubtitlePreference.trackOrNull(subtitleSelection.secondaryTrack), ); _trackManager = trackManager; diff --git a/lib/screens/video_player/parts/playback_services.dart b/lib/screens/video_player/parts/playback_services.dart index 8e10b546..f7f85c2b 100644 --- a/lib/screens/video_player/parts/playback_services.dart +++ b/lib/screens/video_player/parts/playback_services.dart @@ -414,6 +414,7 @@ extension _VideoPlayerPlaybackServiceMethods on VideoPlayerScreenState { mediaInfo: mediaInfo, canReportPlayback: () => _hasRenderedFirstFrame && !_hasFatalPlaybackError, hasRenderedPlayback: () => _hasRenderedFirstFrame, + subtitleOffIsDeliberate: () => _playbackSession?.subtitleSelection.declinedPreference == null, onPausedKeepalive: mediaClient is PlexClient && effectivePlayMethod == 'Transcode' ? () => mediaClient.pingTranscodeSession(_playbackTranscodeSessionId) : null, diff --git a/lib/screens/video_player/parts/playback_start.dart b/lib/screens/video_player/parts/playback_start.dart index 89fd99f6..f48562af 100644 --- a/lib/screens/video_player/parts/playback_start.dart +++ b/lib/screens/video_player/parts/playback_start.dart @@ -346,7 +346,10 @@ extension _VideoPlayerPlaybackStartMethods on VideoPlayerScreenState { plexClient: mediaClient is PlexClient ? mediaClient : null, getProfileSettings: () => context.read().profileSettings, preferredAudioTrack: _preferredAudioTrack, - preferredSubtitleTrack: SubtitlePreference.trackOrNull(subtitleSelection.primaryTrack), + // Same rule as the reload flow: a declined preference is retried by + // the native passes instead of being frozen into off (#1785). + preferredSubtitleTrack: + subtitleSelection.declinedPreference ?? SubtitlePreference.trackOrNull(subtitleSelection.primaryTrack), preferredSecondarySubtitleTrack: SubtitlePreference.trackOrNull(subtitleSelection.secondaryTrack), ); diff --git a/lib/screens/video_player_screen.dart b/lib/screens/video_player_screen.dart index ec50f194..e8777154 100644 --- a/lib/screens/video_player_screen.dart +++ b/lib/screens/video_player_screen.dart @@ -152,10 +152,15 @@ bool playerChromeStartsVisible({required bool isTv}) => !isTv; /// committed semantic choice — a [SubtitleIntent] — may cross the item /// boundary; native state is a fallback for sessions created before /// source-backed selection was recorded. +/// +/// [declinedPreference] is the committed selection's unserved carry: its off +/// is fallout from a metadata mismatch, not a viewer choice, so it must not +/// harden into an explicit off on the next item (#1785). SubtitlePreference? subtitlePreferenceForItemChange({ required bool hasCommittedSelection, required SubtitleTrack? committedTrack, required SubtitleTrack? nativeTrack, + SubtitlePreference? declinedPreference, }) { SubtitlePreference? normalize(SubtitleTrack? track, {required bool preserveOff}) { if (track == null) return null; @@ -169,6 +174,15 @@ SubtitlePreference? subtitlePreferenceForItemChange({ return normalize(nativeTrack, preserveOff: true); } + final committedIsOff = committedTrack == null || committedTrack.id == SubtitleTrack.off.id; + if (declinedPreference != null && committedIsOff) { + // Live native state wins — a late native pass may have served the + // declined carry — otherwise the declined preference itself keeps + // crossing item boundaries until the viewer or a richer catalog + // settles it. + return normalize(nativeTrack, preserveOff: false) ?? SubtitlePreference.demoteToIntent(declinedPreference); + } + if (committedTrack == null) return const SubtitlePreference.off(); final committedPreference = normalize(committedTrack, preserveOff: true); @@ -176,6 +190,39 @@ SubtitlePreference? subtitlePreferenceForItemChange({ return normalize(nativeTrack, preserveOff: false); } +/// Builds the committed [PlaybackSubtitleSelection] for a user's subtitle +/// pick in one slot, leaving the other slot untouched. +/// +/// [sourceTrack] and [sidecar] are null when the pick has no source-catalog +/// identity — items without subtitle rows, or a native track the identity +/// matcher cannot map. The raw native [track] is committed then (without +/// source ids), so the session still reflects what is on screen and the next +/// item boundary demotes it to a semantic intent instead of hardening the +/// stale previous selection into an explicit off (#1785). +PlaybackSubtitleSelection subtitleSelectionForUserPick({ + required PlaybackSubtitleSelection currentSelection, + required bool isPrimarySlot, + required SubtitleTrack track, + MediaSubtitleTrack? sourceTrack, + PlaybackSubtitleSidecar? sidecar, +}) { + final resolvedTrack = sourceTrack == null + ? track + : PlaybackSubtitleResolver.subtitleTrackForSource(sourceTrack, sidecar: sidecar); + return PlaybackSubtitleSelection( + primaryTrack: isPrimarySlot ? resolvedTrack : currentSelection.primaryTrack, + primarySourceStreamId: isPrimarySlot ? sourceTrack?.id : currentSelection.primarySourceStreamId, + primarySidecar: isPrimarySlot ? sidecar : currentSelection.primarySidecar, + secondaryTrack: isPrimarySlot ? currentSelection.secondaryTrack : resolvedTrack, + secondarySourceStreamId: isPrimarySlot ? currentSelection.secondarySourceStreamId : sourceTrack?.id, + secondarySidecar: isPrimarySlot ? currentSelection.secondarySidecar : sidecar, + // A primary pick is a decision that retires any unresolved carry; a + // secondary-only change must not relabel the primary's declined off as + // deliberate (that would persist -1 and harden the next boundary). + declinedPreference: isPrimarySlot ? null : currentSelection.declinedPreference, + ); +} + /// The in-place media-source transitions a [VideoPlayerScreenState] can run. /// They are mutually exclusive by construction — entry points bail while a /// transition is in flight. @@ -1878,10 +1925,13 @@ class VideoPlayerScreenState extends State with WidgetsBindin if (track.id == SubtitleTrack.off.id) { _updatePlaybackSessionSubtitleSelection(session, switch (slot) { _SubtitleSelectionSlot.primary => const PlaybackSubtitleSelection.off(), + // Dropping only the secondary must not relabel the primary's + // declined off as deliberate. _SubtitleSelectionSlot.secondary => PlaybackSubtitleSelection( primaryTrack: currentSelection.primaryTrack, primarySourceStreamId: currentSelection.primarySourceStreamId, primarySidecar: currentSelection.primarySidecar, + declinedPreference: currentSelection.declinedPreference, ), }); if (mounted) _setPlayerState(() {}); @@ -1890,7 +1940,6 @@ class VideoPlayerScreenState extends State with WidgetsBindin final info = _currentMediaInfo; final currentPlayer = player; - if (info == null || currentPlayer == null) return; MediaSubtitleTrack? sourceTrack; final currentSourceId = switch (slot) { @@ -1901,41 +1950,44 @@ class VideoPlayerScreenState extends State with WidgetsBindin _SubtitleSelectionSlot.primary => currentSelection.primarySidecar, _SubtitleSelectionSlot.secondary => currentSelection.secondarySidecar, }; - if (sourceStreamId != null) { - for (final candidate in info.subtitleTracks) { - if (candidate.id == sourceStreamId) { - sourceTrack = candidate; - break; + if (info != null) { + if (sourceStreamId != null) { + for (final candidate in info.subtitleTracks) { + if (candidate.id == sourceStreamId) { + sourceTrack = candidate; + break; + } + } + } else if (track.isExternal && currentSourceId != null && currentSidecar?.track.uri == track.uri) { + for (final candidate in info.subtitleTracks) { + if (candidate.id == currentSourceId) { + sourceTrack = candidate; + break; + } } } - } else if (track.isExternal && currentSourceId != null && currentSidecar?.track.uri == track.uri) { - for (final candidate in info.subtitleTracks) { - if (candidate.id == currentSourceId) { - sourceTrack = candidate; - break; - } + if (sourceTrack == null && currentPlayer != null) { + sourceTrack = findPlexTrackForMpvSubtitle( + track, + info.subtitleTracks, + allMpvTracks: currentPlayer.state.tracks.subtitle, + ); } } - sourceTrack ??= findPlexTrackForMpvSubtitle( - track, - info.subtitleTracks, - allMpvTracks: currentPlayer.state.tracks.subtitle, - ); - if (sourceTrack == null) return; - final sidecar = _sidecarForSourceStreamId(session, sourceTrack.id); - final resolvedTrack = PlaybackSubtitleResolver.subtitleTrackForSource(sourceTrack, sidecar: sidecar); - final selection = PlaybackSubtitleSelection( - primaryTrack: slot == _SubtitleSelectionSlot.primary ? resolvedTrack : currentSelection.primaryTrack, - primarySourceStreamId: slot == _SubtitleSelectionSlot.primary - ? sourceTrack.id - : currentSelection.primarySourceStreamId, - primarySidecar: slot == _SubtitleSelectionSlot.primary ? sidecar : currentSelection.primarySidecar, - secondaryTrack: slot == _SubtitleSelectionSlot.secondary ? resolvedTrack : currentSelection.secondaryTrack, - secondarySourceStreamId: slot == _SubtitleSelectionSlot.secondary - ? sourceTrack.id - : currentSelection.secondarySourceStreamId, - secondarySidecar: slot == _SubtitleSelectionSlot.secondary ? sidecar : currentSelection.secondarySidecar, + // No source identity (no catalog, or a native track the identity matcher + // cannot map) still commits the raw pick: an uncommitted selection reads + // as the session's previous choice — usually the initial off — and the + // next episode boundary would harden that into an explicit off while the + // viewer visibly watches with subtitles on (#1785). The raw track carries + // no source ids, so it demotes to a semantic intent at the boundary. + final sidecar = sourceTrack == null ? null : _sidecarForSourceStreamId(session, sourceTrack.id); + final selection = subtitleSelectionForUserPick( + currentSelection: currentSelection, + isPrimarySlot: slot == _SubtitleSelectionSlot.primary, + track: track, + sourceTrack: sourceTrack, + sidecar: sidecar, ); _updatePlaybackSessionSubtitleSelection(session, selection); if (mounted) _setPlayerState(() {}); diff --git a/lib/services/playback_progress_tracker.dart b/lib/services/playback_progress_tracker.dart index 4748cc2a..24f948be 100644 --- a/lib/services/playback_progress_tracker.dart +++ b/lib/services/playback_progress_tracker.dart @@ -87,6 +87,14 @@ class PlaybackProgressTracker { /// must not turn an unrendered native clock position into watched progress. final bool Function()? hasRenderedPlayback; + /// Whether an off subtitle state is a real decision (viewer or server) + /// rather than the fallout of a declined cross-item carry. When false, the + /// off state is not reported as an explicit `-1` stream index — persisting + /// it would make Jellyfin hand the off back as this item's default on every + /// later open, latching a metadata mismatch into a server-side choice + /// (#1785). Callers default to deliberate. + final bool Function()? subtitleOffIsDeliberate; + /// Timer for periodic progress updates Timer? _progressTimer; @@ -166,6 +174,7 @@ class PlaybackProgressTracker { this.onPausedKeepalive, this.canReportPlayback, this.hasRenderedPlayback, + this.subtitleOffIsDeliberate, this.updateInterval = const Duration(seconds: 10), }) : assert(!isOffline || offlineWatchService != null, 'offlineWatchService is required when isOffline is true'), assert(isOffline || client != null, 'client is required when isOffline is false') { @@ -605,7 +614,11 @@ class PlaybackProgressTracker { int? _currentSubtitleStreamIndex(MediaSourceInfo info) { final track = player.state.track.subtitle; - if (track == null || track.id == 'no') return -1; + if (track == null || track.id == 'no') { + // An off that merely fell out of a declined carry is withheld rather + // than persisted as an explicit -1 (see [subtitleOffIsDeliberate]). + return (subtitleOffIsDeliberate?.call() ?? true) ? -1 : null; + } if (track.isExternal && track.uri != null) { for (final mediaTrack in info.subtitleTracks) { diff --git a/lib/services/playback_subtitle_resolver.dart b/lib/services/playback_subtitle_resolver.dart index 82c5f0fc..3509d0ab 100644 --- a/lib/services/playback_subtitle_resolver.dart +++ b/lib/services/playback_subtitle_resolver.dart @@ -45,6 +45,14 @@ class PlaybackSubtitleSelection { final PlaybackSubtitleSidecar? secondarySidecar; final List preloadedSidecars; + /// The primary preference the resolver could not serve when this selection + /// is off. Distinguishes "the carried choice declined and the ladder fell + /// through" from a deliberate off (#1785): the open flow hands the declined + /// preference back to the track manager so late-arriving native tracks may + /// still serve it, and progress reports must not persist the fallout as an + /// explicit server-side off. A user or server decision leaves this null. + final SubtitlePreference? declinedPreference; + const PlaybackSubtitleSelection({ required this.primaryTrack, this.primarySourceStreamId, @@ -53,9 +61,10 @@ class PlaybackSubtitleSelection { this.secondarySourceStreamId, this.secondarySidecar, this.preloadedSidecars = const [], + this.declinedPreference, }); - const PlaybackSubtitleSelection.off({this.preloadedSidecars = const []}) + const PlaybackSubtitleSelection.off({this.preloadedSidecars = const [], this.declinedPreference}) : primaryTrack = SubtitleTrack.off, primarySourceStreamId = null, primarySidecar = null, @@ -192,13 +201,25 @@ class PlaybackSubtitleResolver { ); final primaryResult = service.selectSubtitleTrack(availableTracks, primaryPreference, selectedAudio); final primary = primaryResult?.track; + // A non-off preference that still lands on off (or resolves to a track + // this catalog cannot back) was declined, not chosen — keep it on the + // selection so the open flow can retry it against native tracks (#1785). + final declinedPreference = primaryPreference != null && primaryPreference is! SubtitleOffPreference + ? primaryPreference + : null; if (primary == null || primary.id == SubtitleTrack.off.id) { - return PlaybackSubtitleSelection.off(preloadedSidecars: preloadedSidecars); + return PlaybackSubtitleSelection.off( + preloadedSidecars: preloadedSidecars, + declinedPreference: declinedPreference, + ); } final primaryCandidate = candidates.where((candidate) => candidate.track.id == primary.id).firstOrNull; if (primaryCandidate == null) { - return PlaybackSubtitleSelection.off(preloadedSidecars: preloadedSidecars); + return PlaybackSubtitleSelection.off( + preloadedSidecars: preloadedSidecars, + declinedPreference: declinedPreference, + ); } _SubtitleCandidate? secondaryCandidate; diff --git a/lib/services/track_selection_service.dart b/lib/services/track_selection_service.dart index 586f7cc4..d56fd00a 100644 --- a/lib/services/track_selection_service.dart +++ b/lib/services/track_selection_service.dart @@ -347,47 +347,100 @@ MediaSubtitleTrack? findPlexTrackForMpvSubtitle( /// /// Identity matching ([findPlexTrackForMpvSubtitle]) answers "which row IS /// this track"; this answers "which row of a DIFFERENT item serves the same -/// intent". Language and effective forced-ness are hard requirements: the -/// intent's class is preserved or the match declines, so the selection ladder -/// can fall back to the server's own per-item choice (#1716/#1717). +/// intent". Effective forced-ness is a hard requirement, and so is language +/// when both sides declare one: the intent's class is preserved or the match +/// declines, so the selection ladder can fall back to the server's own +/// per-item choice (#1716/#1717). +/// +/// When language metadata is missing on either side, a unique real title +/// match may vouch for the row instead (#1785) — untagged tracks whose only +/// signal is a title like "Swedish" are common, and declining them turned the +/// viewer's subtitles off on every episode advance. Codec/external parity is +/// never sufficient evidence on its own (an arbitrary untagged row would +/// reintroduce the #1716 wrong-track class); it only breaks ties WITHIN the +/// title-matched set, and a residual tie declines rather than guesses. MediaSubtitleTrack? findSourceTrackForIntent(SubtitleIntent intent, List sourceTracks) { - MediaSubtitleTrack? bestMatch; - var bestScore = -1; - for (final row in sourceTracks) { - if (!_languagesMatch(intent.language, row.languageCode ?? row.language)) continue; - if (row.effectiveForced != intent.forced) continue; - - var score = 0; - if (_subtitleCodecsMatch(intent.codec, row.codec)) score += 5; - score += _titleScore(intent.title, row.title, row.displayTitle); - if (intent.isExternal == row.isExternal) score += 1; - if (score > bestScore) { - bestScore = score; - bestMatch = row; - } - } - return bestMatch; + return _findTrackForIntent( + intent, + sourceTracks, + isSelectable: (_) => true, + language: (row) => row.languageCode ?? row.language, + effectiveForced: (row) => row.effectiveForced, + titleScore: (row) => _titleScore(intent.title, row.title, row.displayTitle), + codec: (row) => row.codec, + isExternal: (row) => row.isExternal, + ); } /// Native-track twin of [findSourceTrackForIntent], for catalogs the source /// side cannot describe (legacy offline sidecars) and late-arriving tracks. SubtitleTrack? findNativeTrackForIntent(SubtitleIntent intent, List tracks) { - SubtitleTrack? bestMatch; - var bestScore = -1; - for (final track in tracks) { - if (track.id == SubtitleTrack.auto.id || track.id == SubtitleTrack.off.id) continue; - if (!_languagesMatch(intent.language, track.language)) continue; - if (track.effectiveForced != intent.forced) continue; + return _findTrackForIntent( + intent, + tracks, + isSelectable: (track) => track.id != SubtitleTrack.auto.id && track.id != SubtitleTrack.off.id, + language: (track) => track.language, + effectiveForced: (track) => track.effectiveForced, + titleScore: (track) => _titleScore(intent.title, track.title, null), + codec: (track) => track.codec, + isExternal: (track) => track.isExternal, + ); +} +/// Shared scorer behind [findSourceTrackForIntent]/[findNativeTrackForIntent]. +/// +/// Score bands keep the evidence classes strictly ordered: a language-parity +/// match starts at 10 while a title-evidence match tops out at 9 +/// (codec 5 + title 3 + external 1), so the two can never tie and language +/// always outranks a title coincidence. +T? _findTrackForIntent( + SubtitleIntent intent, + List candidates, { + required bool Function(T) isSelectable, + required String? Function(T) language, + required bool Function(T) effectiveForced, + required int Function(T) titleScore, + required String? Function(T) codec, + required bool Function(T) isExternal, +}) { + T? bestMatch; + var bestScore = -1; + var bestIsTitleEvidence = false; + var bestIsAmbiguous = false; + for (final candidate in candidates) { + if (!isSelectable(candidate)) continue; + if (effectiveForced(candidate) != intent.forced) continue; + + final candidateLanguage = language(candidate); + final candidateTitleScore = titleScore(candidate); + final hasLanguageParity = intent.language != null && candidateLanguage != null; var score = 0; - if (_subtitleCodecsMatch(intent.codec, track.codec)) score += 5; - score += _titleScore(intent.title, track.title, null); - if (intent.isExternal == track.isExternal) score += 1; + if (hasLanguageParity) { + // A declared language on both sides stays authoritative: a + // contradiction declines no matter what the title says. + if (!_languagesMatch(intent.language, candidateLanguage)) continue; + score += 10; + } else if (candidateTitleScore < 3) { + // Language evidence is missing on at least one side; only a real + // title match may serve the intent then. + continue; + } + if (_subtitleCodecsMatch(intent.codec, codec(candidate))) score += 5; + score += candidateTitleScore; + if (intent.isExternal == isExternal(candidate)) score += 1; if (score > bestScore) { bestScore = score; - bestMatch = track; + bestMatch = candidate; + bestIsTitleEvidence = !hasLanguageParity; + bestIsAmbiguous = false; + } else if (score == bestScore && bestIsTitleEvidence) { + // Only title-evidence matches can tie (their band never reaches a + // language-parity score). Two rows the tiebreakers cannot separate + // mean the catalog cannot say which one the viewer meant. + bestIsAmbiguous = true; } } + if (bestIsTitleEvidence && bestIsAmbiguous) return null; return bestMatch; } diff --git a/test/screens/video_player/player_initialization_lifecycle_test.dart b/test/screens/video_player/player_initialization_lifecycle_test.dart index 0f419c6c..ca42d95b 100644 --- a/test/screens/video_player/player_initialization_lifecycle_test.dart +++ b/test/screens/video_player/player_initialization_lifecycle_test.dart @@ -6,6 +6,8 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:plezy/focus/focusable_button.dart'; import 'package:plezy/providers/playback_state_provider.dart'; import 'package:plezy/mpv/mpv.dart'; +import 'package:plezy/media/media_source_info.dart'; +import 'package:plezy/services/playback_subtitle_resolver.dart'; import 'package:plezy/screens/video_player_screen.dart'; import 'package:plezy/services/settings_service.dart'; import 'package:plezy/services/subtitle_preference.dart'; @@ -117,6 +119,146 @@ void main() { expect((result! as SubtitleIntentPreference).intent.language, 'eng'); }); + test('a declined committed off re-carries the declined preference (#1785)', () { + const declined = SubtitlePreference.intent( + SubtitleIntent(language: 'swe', forced: false, title: 'Swedish', codec: 'srt'), + ); + + // The committed off is fallout from a declined carry: with the player + // also off, the declined preference itself keeps crossing boundaries. + expect( + subtitlePreferenceForItemChange( + hasCommittedSelection: true, + committedTrack: SubtitleTrack.off, + nativeTrack: SubtitleTrack.off, + declinedPreference: declined, + ), + declined, + ); + }); + + test('live native state outranks a declined carry after a late rescue (#1785)', () { + const declined = SubtitlePreference.intent( + SubtitleIntent(language: 'swe', forced: false, title: 'Swedish', codec: 'srt'), + ); + + final result = subtitlePreferenceForItemChange( + hasCommittedSelection: true, + committedTrack: SubtitleTrack.off, + nativeTrack: const SubtitleTrack(id: '5', language: 'swe', title: 'Swedish', codec: 'srt'), + declinedPreference: declined, + ); + + expect(result, isA()); + expect((result! as SubtitleIntentPreference).intent.language, 'swe'); + }); + + test('a declined stale source reference crosses the boundary as its intent (#1785)', () { + const declined = SubtitlePreference.track( + SubtitleTrack(id: 'source:9', title: 'Swedish', language: 'swe', codec: 'srt'), + ); + + final result = subtitlePreferenceForItemChange( + hasCommittedSelection: true, + committedTrack: SubtitleTrack.off, + nativeTrack: SubtitleTrack.off, + declinedPreference: declined, + ); + + expect(result, isA()); + expect((result! as SubtitleIntentPreference).intent.language, 'swe'); + }); + + test('a pick without source identity is committed raw and carries as an intent (#1785)', () { + // The identity matcher failed (or the item has no subtitle rows): the + // committed selection must still reflect the pick on screen, not the + // session's previous choice. + const picked = SubtitleTrack(id: '3', title: 'Swedish', language: 'swe', codec: 'srt'); + + final selection = subtitleSelectionForUserPick( + currentSelection: const PlaybackSubtitleSelection.off(), + isPrimarySlot: true, + track: picked, + ); + + expect(selection.primaryTrack, picked); + expect(selection.primarySourceStreamId, isNull); + expect(selection.primarySidecar, isNull); + + // Next episode boundary: the raw commit demotes to a semantic intent + // instead of hardening the stale off. + final carried = subtitlePreferenceForItemChange( + hasCommittedSelection: true, + committedTrack: selection.primaryTrack, + nativeTrack: picked, + ); + expect(carried, isA()); + expect((carried! as SubtitleIntentPreference).intent.language, 'swe'); + }); + + test('a source-backed pick keeps its source identity in the committed selection', () { + final sourceTrack = MediaSubtitleTrack( + id: 7, + languageCode: 'swe', + title: 'Swedish', + codec: 'srt', + selected: false, + forced: false, + ); + + final selection = subtitleSelectionForUserPick( + currentSelection: const PlaybackSubtitleSelection.off(), + isPrimarySlot: true, + track: const SubtitleTrack(id: '3', title: 'Swedish', language: 'swe', codec: 'srt'), + sourceTrack: sourceTrack, + ); + + expect(selection.primaryTrack.id, 'source:7'); + expect(selection.primarySourceStreamId, 7); + }); + + test('a secondary-slot pick leaves the committed primary untouched', () { + const primary = SubtitleTrack(id: 'source:4', title: 'Swedish', language: 'swe', codec: 'srt'); + const secondaryPick = SubtitleTrack(id: '5', title: 'English', language: 'eng', codec: 'srt'); + + final selection = subtitleSelectionForUserPick( + currentSelection: const PlaybackSubtitleSelection(primaryTrack: primary, primarySourceStreamId: 4), + isPrimarySlot: false, + track: secondaryPick, + ); + + expect(selection.primaryTrack, primary); + expect(selection.primarySourceStreamId, 4); + expect(selection.secondaryTrack, secondaryPick); + expect(selection.secondarySourceStreamId, isNull); + }); + + test('a secondary-only change keeps the primary declined carry alive (#1785)', () { + const declined = SubtitlePreference.intent( + SubtitleIntent(language: 'swe', forced: false, title: 'Swedish', codec: 'srt'), + ); + const current = PlaybackSubtitleSelection.off(declinedPreference: declined); + + final selection = subtitleSelectionForUserPick( + currentSelection: current, + isPrimarySlot: false, + track: const SubtitleTrack(id: '5', title: 'English', language: 'eng', codec: 'srt'), + ); + + // The primary off stays fallout, not a decision: -1 must remain withheld + // and the next boundary must still re-carry the intent. + expect(selection.primaryTrack.id, SubtitleTrack.off.id); + expect(selection.declinedPreference, declined); + + // A primary decision retires the carry. + final decided = subtitleSelectionForUserPick( + currentSelection: selection, + isPrimarySlot: true, + track: const SubtitleTrack(id: '3', title: 'Swedish', language: 'swe', codec: 'srt'), + ); + expect(decided.declinedPreference, isNull); + }); + testWidgets('initialization ownership serializes rollback, retry, and route removal', (tester) async { final failedDispose = Completer(); final replacementInitialize = Completer(); diff --git a/test/services/playback_progress_tracker_test.dart b/test/services/playback_progress_tracker_test.dart index d5a1b8a0..8947da5c 100644 --- a/test/services/playback_progress_tracker_test.dart +++ b/test/services/playback_progress_tracker_test.dart @@ -703,6 +703,55 @@ void main() { expect(progressSelection.subtitleStreamIndex, -1); }); + test('an off that fell out of a declined carry is not persisted as -1 (#1785)', () async { + final client = _FakePlexClient(); + const selectedAudio = AudioTrack(id: 'audio_1', language: 'jpn'); + const subtitlesOff = SubtitleTrack(id: 'no'); + final player = _FakePlayer( + position: const Duration(seconds: 5), + duration: const Duration(seconds: 100), + tracks: const Tracks( + audio: [ + AudioTrack(id: 'audio_0', language: 'eng'), + selectedAudio, + ], + subtitle: [SubtitleTrack(id: 'text_0', language: 'eng')], + ), + track: const TrackSelection(audio: selectedAudio, subtitle: subtitlesOff), + ); + final mediaInfo = MediaSourceInfo( + videoUrl: '', + audioTracks: [ + MediaAudioTrack(id: 1, languageCode: 'eng', selected: false), + MediaAudioTrack(id: 2, languageCode: 'jpn', selected: true), + ], + subtitleTracks: [MediaSubtitleTrack(id: 3, languageCode: 'eng', selected: false, forced: false)], + chapters: const [], + mediaSourceId: 'source-1', + ); + final tracker = PlaybackProgressTracker( + client: client, + metadata: _meta(ratingKey: '42'), + player: player, + isOffline: false, + mediaInfo: mediaInfo, + subtitleOffIsDeliberate: () => false, + ); + addTearDown(tracker.dispose); + + await tracker.sendProgress('playing'); + await Future.delayed(Duration.zero); + await tracker.sendProgress('playing'); + await Future.delayed(Duration.zero); + + final progressSelection = client.playbackStreamSelections[1]; + // Withheld, so the server keeps whatever it knew — an explicit -1 + // would come back as this item's default and latch the fallout. + expect(progressSelection.subtitleStreamIndex, isNull); + // The audio selection is still reported normally. + expect(progressSelection.audioStreamIndex, 2); + }); + test('stopped reports only resolve media source and do not include selected streams', () async { final client = _FakePlexClient(); const selectedAudio = AudioTrack(id: 'audio_1', language: 'jpn'); diff --git a/test/services/playback_subtitle_resolver_test.dart b/test/services/playback_subtitle_resolver_test.dart index ff0244a1..35845c17 100644 --- a/test/services/playback_subtitle_resolver_test.dart +++ b/test/services/playback_subtitle_resolver_test.dart @@ -548,6 +548,65 @@ void main() { }); }); + group('issue #1785 declined-carry surfacing', () { + const swedishIntent = SubtitlePreference.intent( + SubtitleIntent(language: 'swe', forced: false, title: 'Swedish', codec: 'srt'), + ); + + MediaSubtitleTrack untaggedRow(int id, {String? title, String codec = 'srt'}) => + MediaSubtitleTrack(id: id, title: title, codec: codec, selected: false, forced: false); + + test('an untagged catalog row with the matching title still serves the carry', () { + final result = PlaybackSubtitleResolver.resolve( + metadata: metadata, + mediaInfo: _mediaInfo([untaggedRow(3, title: 'English'), untaggedRow(4, title: 'Swedish')]), + sidecars: const [], + preferredSubtitleTrack: swedishIntent, + preserveSourceIdentity: false, + ); + + expect(result.primarySourceStreamId, 4); + expect(result.declinedPreference, isNull); + }); + + test('a carry no row can serve is kept as declined, not converted to off', () { + final result = PlaybackSubtitleResolver.resolve( + metadata: metadata, + mediaInfo: _mediaInfo([_sourceSubtitle(3, language: 'eng')]), + sidecars: const [], + preferredSubtitleTrack: swedishIntent, + preserveSourceIdentity: false, + ); + + expect(result.isOff, isTrue); + expect(result.declinedPreference, swedishIntent); + }); + + test('an explicit off carry is a decision, never a decline', () { + final result = PlaybackSubtitleResolver.resolve( + metadata: metadata, + mediaInfo: _mediaInfo([_sourceSubtitle(3, language: 'eng')]), + sidecars: const [], + preferredSubtitleTrack: const SubtitlePreference.off(), + preserveSourceIdentity: false, + ); + + expect(result.isOff, isTrue); + expect(result.declinedPreference, isNull); + }); + + test('a ladder off with no carry at all is a decision, never a decline', () { + final result = PlaybackSubtitleResolver.resolve( + metadata: metadata, + mediaInfo: _mediaInfo([_sourceSubtitle(3, language: 'eng')]), + sidecars: const [], + ); + + expect(result.isOff, isTrue); + expect(result.declinedPreference, isNull); + }); + }); + test('selected embedded subtitle keeps sidecars out of the open', () { final result = PlaybackSubtitleResolver.resolve( metadata: metadata, diff --git a/test/services/track_selection_service_test.dart b/test/services/track_selection_service_test.dart index 1d8e29d2..95408fe8 100644 --- a/test/services/track_selection_service_test.dart +++ b/test/services/track_selection_service_test.dart @@ -1137,10 +1137,74 @@ void main() { expect(findSourceTrackForIntent(fullIntent, rows)?.id, 2); }); - test('language-less intents decline', () { + test('language-less intent declines when no row carries a matching title', () { const intent = SubtitleIntent(forced: false, title: 'French', codec: 'srt'); expect(findSourceTrackForIntent(intent, [_plexSub(1, languageCode: 'fre')]), isNull); }); + + // ============================================================ + // #1785 — missing language tags must not turn the carry off when a + // unique real title identifies the row; codec parity alone is never + // evidence, and ambiguity declines rather than guesses. + // ============================================================ + + test('title-only intent matches the row with the same title when tags are missing (#1785)', () { + const intent = SubtitleIntent(forced: false, title: 'Swedish', codec: 'subrip'); + final rows = [_plexSub(1, title: 'English', codec: 'subrip'), _plexSub(2, title: 'Swedish', codec: 'subrip')]; + expect(findSourceTrackForIntent(intent, rows)?.id, 2); + }); + + test('tagged intent reaches an untagged row through its title (#1785)', () { + const intent = SubtitleIntent(language: 'swe', forced: false, title: 'Swedish', codec: 'subrip'); + expect(findSourceTrackForIntent(intent, [_plexSub(1, title: 'Swedish', codec: 'subrip')])?.id, 1); + }); + + test('title-only intent reaches a tagged row through its title (#1785)', () { + const intent = SubtitleIntent(forced: false, title: 'Swedish', codec: 'subrip'); + final rows = [ + _plexSub(1, languageCode: 'eng', title: 'English'), + _plexSub(2, languageCode: 'swe', title: 'Swedish'), + ]; + expect(findSourceTrackForIntent(intent, rows)?.id, 2); + }); + + test('declared languages stay authoritative over a coincidental title', () { + const intent = SubtitleIntent(language: 'swe', forced: false, title: 'Swedish', codec: 'subrip'); + expect(findSourceTrackForIntent(intent, [_plexSub(1, languageCode: 'eng', title: 'Swedish')]), isNull); + }); + + test('codec parity alone never vouches for an untagged row', () { + const intent = SubtitleIntent(forced: false, title: 'Swedish', codec: 'subrip'); + expect(findSourceTrackForIntent(intent, [_plexSub(1, codec: 'subrip')]), isNull); + }); + + test('an ambiguous same-title untagged pair declines rather than guesses', () { + const intent = SubtitleIntent(forced: false, title: 'Swedish', codec: 'subrip'); + final rows = [_plexSub(1, title: 'Swedish', codec: 'subrip'), _plexSub(2, title: 'Swedish', codec: 'subrip')]; + expect(findSourceTrackForIntent(intent, rows), isNull); + }); + + test('codec separates same-titled untagged rows before declining', () { + const intent = SubtitleIntent(forced: false, title: 'Swedish', codec: 'ass'); + final rows = [_plexSub(1, title: 'Swedish', codec: 'subrip'), _plexSub(2, title: 'Swedish', codec: 'ass')]; + expect(findSourceTrackForIntent(intent, rows)?.id, 2); + }); + + test('forced parity still gates title-evidence matches', () { + const intent = SubtitleIntent(forced: false, title: 'Swedish', codec: 'subrip'); + expect(findSourceTrackForIntent(intent, [_plexSub(1, title: 'Swedish Forced', codec: 'subrip')]), isNull); + }); + + test('a language-parity match outranks a title-evidence match', () { + const intent = SubtitleIntent(language: 'swe', forced: false, title: 'Swedish', codec: 'subrip'); + final rows = [ + // Title-evidence candidate (untagged, matching title + codec). + _plexSub(1, title: 'Swedish', codec: 'subrip'), + // Language-parity candidate with a non-matching title. + _plexSub(2, languageCode: 'swe', title: 'Svenska full'), + ]; + expect(findSourceTrackForIntent(intent, rows)?.id, 2); + }); }); group('findNativeTrackForIntent', () { @@ -1167,6 +1231,21 @@ void main() { final forcedOnly = [_sub('1', lang: 'fre', title: 'FR Forced', codec: 'ass')]; expect(findNativeTrackForIntent(fullIntent, forcedOnly), isNull); }); + + test('an untagged native track with the matching title serves the intent (#1785)', () { + // The server catalog may lack tags while mpv reads them from the + // container — and vice versa: a tagged intent must still reach an + // untagged native track through its title. + const intent = SubtitleIntent(language: 'swe', forced: false, title: 'Swedish', codec: 'subrip'); + final tracks = [SubtitleTrack.auto, SubtitleTrack.off, _sub('3', title: 'Swedish', codec: 'subrip')]; + expect(findNativeTrackForIntent(intent, tracks)?.id, '3'); + }); + + test('an ambiguous untagged native pair declines rather than guesses', () { + const intent = SubtitleIntent(forced: false, title: 'Swedish', codec: 'subrip'); + final tracks = [_sub('1', title: 'Swedish', codec: 'subrip'), _sub('2', title: 'Swedish', codec: 'subrip')]; + expect(findNativeTrackForIntent(intent, tracks), isNull); + }); }); group('selectSubtitleTrack - intent preferences (#1716/#1717)', () {