diff --git a/lib/services/track_manager.dart b/lib/services/track_manager.dart index c1bd219e..d24db2ba 100644 --- a/lib/services/track_manager.dart +++ b/lib/services/track_manager.dart @@ -8,7 +8,6 @@ import '../media/media_source_info.dart'; import '../services/settings_service.dart'; import '../services/track_selection_service.dart'; import '../utils/app_logger.dart'; -import '../utils/language_codes.dart'; import '../utils/track_label_builder.dart'; /// Persists a track choice for the current part to the server. @@ -313,26 +312,12 @@ class TrackManager { final partId = await _guardTrackChange(info); if (partId == null || info == null) return; - int? streamID = _matchTrackByAttributes( - mpvLanguage: track.language, - mpvTitle: track.title, - plexTracks: info.audioTracks, - getLanguageCode: (t) => t.languageCode, - getDisplayTitle: (t) => t.displayTitle, - getTitle: (t) => t.title, - getId: (t) => t.id, - ); - + final matchedPlex = findPlexTrackForMpvAudio(track, info.audioTracks, allMpvTracks: player.state.tracks.audio); + final streamID = matchedPlex?.id; if (streamID != null) { - appLogger.d('Matched audio by lang/title: streamID $streamID'); + appLogger.d('Matched audio to streamID $streamID'); } else { - final matchedPlex = findPlexTrackForMpvAudio(track, info.audioTracks, allMpvTracks: player.state.tracks.audio); - streamID = matchedPlex?.id; - if (streamID != null) { - appLogger.d('Matched audio by properties: streamID $streamID'); - } else { - appLogger.e('Could not match audio track to any Plex track'); - } + appLogger.e('Could not match audio track to any Plex track'); } await _saveTrackPreferences(partId: partId, trackType: 'audio', streamID: streamID); @@ -350,30 +335,16 @@ class TrackManager { streamID = 0; appLogger.i('User turned subtitles off, saving preference'); } else if (info != null) { - streamID = _matchTrackByAttributes( - mpvLanguage: track.language, - mpvTitle: track.title, - plexTracks: info.subtitleTracks, - getLanguageCode: (t) => t.languageCode, - getDisplayTitle: (t) => t.displayTitle, - getTitle: (t) => t.title, - getId: (t) => t.id, + final matchedPlex = findPlexTrackForMpvSubtitle( + track, + info.subtitleTracks, + allMpvTracks: player.state.tracks.subtitle, ); - + streamID = matchedPlex?.id; if (streamID != null) { - appLogger.d('Matched subtitle by lang/title: streamID $streamID'); + appLogger.d('Matched subtitle to streamID $streamID'); } else { - final matchedPlex = findPlexTrackForMpvSubtitle( - track, - info.subtitleTracks, - allMpvTracks: player.state.tracks.subtitle, - ); - streamID = matchedPlex?.id; - if (streamID != null) { - appLogger.d('Matched subtitle by properties: streamID $streamID'); - } else { - appLogger.e('Could not match subtitle track to any Plex track'); - } + appLogger.e('Could not match subtitle track to any Plex track'); } } @@ -422,50 +393,6 @@ class TrackManager { } } - /// Match an mpv track against Plex tracks by language and title. - int? _matchTrackByAttributes({ - required String? mpvLanguage, - required String? mpvTitle, - required List plexTracks, - required String? Function(T) getLanguageCode, - required String? Function(T) getDisplayTitle, - required String? Function(T) getTitle, - required int Function(T) getId, - }) { - final normalizedLang = _iso6391To6392(mpvLanguage); - - for (final plexTrack in plexTracks) { - final matchLang = getLanguageCode(plexTrack) == normalizedLang; - final matchTitle = (mpvTitle == null || mpvTitle.isEmpty) - ? true - : (getDisplayTitle(plexTrack) == mpvTitle || getTitle(plexTrack) == mpvTitle); - - if (matchLang && matchTitle) { - return getId(plexTrack); - } - } - return null; - } - - /// Convert ISO 639-1 code (e.g. "fr") to ISO 639-2/B (e.g. "fre"). Plex - /// streams use the 3-letter form. - static String? _iso6391To6392(String? code) { - if (code == null || code.isEmpty) return null; - final lang = code.split('-').first.toLowerCase(); - - try { - final variations = LanguageCodes.getVariations(lang); - for (final variation in variations) { - if (variation.length == 3) { - return variation; - } - } - return null; - } catch (e) { - return null; - } - } - /// Clean up subscriptions. void dispose() { _externalSubtitleAddsInFlight = false; diff --git a/test/services/track_manager_test.dart b/test/services/track_manager_test.dart index cf413479..b6049ec4 100644 --- a/test/services/track_manager_test.dart +++ b/test/services/track_manager_test.dart @@ -111,11 +111,12 @@ TrackManager _make({ MediaSourceInfo? mediaInfo, bool active = true, void Function(String, {Duration? duration})? showMessage, + TrackPreferencePersister? persister, }) { return TrackManager( player: player, isActive: () => active, - persistTrackPreference: _noopPersister, + persistTrackPreference: persister ?? _noopPersister, getProfileSettings: () => null, waitForProfileSettings: () async {}, metadata: metadata ?? _meta(), @@ -513,6 +514,73 @@ void main() { }); }); + // ============================================================ + // onSubtitleTrackChanged — same-language stream mapping (#1443) + // ============================================================ + + group('onSubtitleTrackChanged', () { + // Reproduces the #1443 MKVToolNix screenshot: the "forced" French subtitle + // is NOT flagged forced in the container — it only carries the name + // "Forced" — and the regular French sub has an empty name. So both sides + // report forced=false, and the saved streamID must come from the title + // (forced sub) and ordinal position (the empty-title regular sub), not from + // "first language match wins". + MediaSourceInfo info() => MediaSourceInfo( + videoUrl: 'https://example.com/video.mkv', + partId: 1, + audioTracks: [MediaAudioTrack(id: 1, languageCode: 'fre', selected: true)], + subtitleTracks: [ + MediaSubtitleTrack(id: 30, languageCode: 'fre', title: 'Forced', codec: 'ass', selected: false, forced: false), + MediaSubtitleTrack(id: 31, languageCode: 'fre', codec: 'ass', selected: false, forced: false), + MediaSubtitleTrack(id: 32, languageCode: 'eng', title: 'SDH', codec: 'ass', selected: false, forced: false), + ], + chapters: const [], + ); + + const playerSubs = [ + SubtitleTrack(id: '2_0', language: 'fre', title: 'Forced', codec: 'ass'), + SubtitleTrack(id: '2_1', language: 'fre', codec: 'ass'), + SubtitleTrack(id: '2_2', language: 'eng', title: 'SDH', codec: 'ass'), + ]; + + test('persists distinct streamIDs for title-only-forced vs regular same-language subs', () async { + await SettingsService.getInstance(); + final player = _FakePlayer(tracks: const Tracks(subtitle: playerSubs)); + int? captured; + final mgr = _make( + player: player, + mediaInfo: info(), + persister: ({required int partId, required String trackType, int? streamID}) async { + captured = streamID; + }, + ); + addTearDown(mgr.dispose); + + await mgr.onSubtitleTrackChanged(playerSubs[0]); // "Forced"-named track + expect(captured, 30); + + await mgr.onSubtitleTrackChanged(playerSubs[1]); // regular (empty title) + expect(captured, 31); + }); + + test('persists stream 0 when subtitles are turned off', () async { + await SettingsService.getInstance(); + final player = _FakePlayer(tracks: const Tracks(subtitle: playerSubs)); + int? captured = -1; + final mgr = _make( + player: player, + mediaInfo: info(), + persister: ({required int partId, required String trackType, int? streamID}) async { + captured = streamID; + }, + ); + addTearDown(mgr.dispose); + + await mgr.onSubtitleTrackChanged(SubtitleTrack.off); + expect(captured, 0); + }); + }); + // ============================================================ // Lifecycle // ============================================================ diff --git a/test/services/track_selection_service_test.dart b/test/services/track_selection_service_test.dart index e9577e18..ad5e2660 100644 --- a/test/services/track_selection_service_test.dart +++ b/test/services/track_selection_service_test.dart @@ -645,4 +645,69 @@ void main() { expect(result.track.id, 'no'); }); }); + + // ============================================================ + // findPlexTrackForMpvSubtitle / findPlexTrackForMpvAudio — same-language + // disambiguation (regression for #1443). The player reports null titles for + // MKV tracks that carry only a forced flag, so the forced flag (+2) and the + // ordinal tiebreaker (+1) must separate two tracks that share a language. + // ============================================================ + + group('findPlexTrackForMpvSubtitle - forced disambiguation', () { + // Disposition-flagged forced track: forced is set in the container, so both + // Plex and the player carry forced=true on the forced track. + test('disposition-flagged forced track maps via the forced flag', () { + final plexTracks = [ + _plexSub(10, index: 0, languageCode: 'fre', codec: 'ass', forced: false), + _plexSub(11, index: 1, languageCode: 'fre', codec: 'ass', forced: true), + ]; + final mpvNonForced = _sub('2_0', lang: 'fre', codec: 'ass'); + final mpvForced = _sub('2_1', lang: 'fre', codec: 'ass', isForced: true); + final allMpv = [mpvNonForced, mpvForced]; + + expect(findPlexTrackForMpvSubtitle(mpvForced, plexTracks, allMpvTracks: allMpv)?.id, 11); + expect(findPlexTrackForMpvSubtitle(mpvNonForced, plexTracks, allMpvTracks: allMpv)?.id, 10); + }); + + // Title-only "forced" track — the exact #1443 file (MKVToolNix screenshot): + // the forced sub is NOT flagged forced in the container, it only carries the + // name "Forced"; the regular French sub has an empty name. Both sides report + // forced=false, so disambiguation rides on title (forced sub) and ordinal + // position (the empty-title regular sub). + test('title-only forced track and empty-title regular track stay distinct (#1443)', () { + final plexTracks = [ + _plexSub(30, index: 0, languageCode: 'fre', title: 'Forced', codec: 'ass', forced: false), + _plexSub(31, index: 1, languageCode: 'fre', codec: 'ass', forced: false), + _plexSub(32, index: 2, languageCode: 'eng', title: 'SDH', codec: 'ass', forced: false), + ]; + final mpvForcedByName = _sub('2_0', lang: 'fre', title: 'Forced', codec: 'ass'); + final mpvRegular = _sub('2_1', lang: 'fre', codec: 'ass'); + final mpvSdh = _sub('2_2', lang: 'eng', title: 'SDH', codec: 'ass'); + final allMpv = [mpvForcedByName, mpvRegular, mpvSdh]; + + expect(findPlexTrackForMpvSubtitle(mpvForcedByName, plexTracks, allMpvTracks: allMpv)?.id, 30); + expect(findPlexTrackForMpvSubtitle(mpvRegular, plexTracks, allMpvTracks: allMpv)?.id, 31); + }); + }); + + group('findPlexTrackForMpvAudio - same-language disambiguation', () { + // Two French audio tracks differing only by channel count, titles null. + final plexTracks = [ + _plexAudio(20, index: 0, languageCode: 'fre', codec: 'ac3', channels: 2), + _plexAudio(21, index: 1, languageCode: 'fre', codec: 'ac3', channels: 6), + ]; + final mpvStereo = _audio('1_0', lang: 'fre', codec: 'ac3', channels: 2); + final mpvSurround = _audio('1_1', lang: 'fre', codec: 'ac3', channels: 6); + final allMpv = [mpvStereo, mpvSurround]; + + test('surround player track maps to the 6-channel Plex stream', () { + final match = findPlexTrackForMpvAudio(mpvSurround, plexTracks, allMpvTracks: allMpv); + expect(match?.id, 21); + }); + + test('stereo player track maps to the 2-channel Plex stream', () { + final match = findPlexTrackForMpvAudio(mpvStereo, plexTracks, allMpvTracks: allMpv); + expect(match?.id, 20); + }); + }); }