From d5f7c5d7acdf8e657e0e5f94b103ac0b7c5181d6 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Mon, 27 Jul 2026 17:51:07 +0200 Subject: [PATCH] fix(player): skip the deferred track pass when its switch is superseded Persisting the deferred choice suspends, so the source switch can be superseded before the pass is armed. Return early when the continuation is stale, and refuse to arm a disposed or inactive TrackManager at all. The per-callback generation checks only stopped the work; the subscription and the five-second timer were still allocated on a manager whose dispose had already run, so nothing would ever cancel them. --- .../parts/episode_navigation.dart | 6 +- lib/services/track_manager.dart | 6 + test/services/track_manager_test.dart | 111 ++++++++++++++++++ 3 files changed, 122 insertions(+), 1 deletion(-) diff --git a/lib/screens/video_player/parts/episode_navigation.dart b/lib/screens/video_player/parts/episode_navigation.dart index 2435b244..8a1af2e6 100644 --- a/lib/screens/video_player/parts/episode_navigation.dart +++ b/lib/screens/video_player/parts/episode_navigation.dart @@ -19,8 +19,12 @@ Future deferTranscodeSubtitleSelection({ // which invalidates the pending selection. Arming before that would retire the // deferred pass we depend on to apply this choice once mpv discovers the sidecar. await onSubtitleTrackChanged(deferredTrack, sourceStreamId: sourceStreamId); + // Persisting suspends, so the switch may have been superseded meanwhile. + // Arming then would attach a listener belonging to an operation nobody is + // waiting on any more. + if (!shouldContinue()) return false; trackManager.applyTrackSelectionWhenReady(); - return shouldContinue(); + return true; } extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { diff --git a/lib/services/track_manager.dart b/lib/services/track_manager.dart index 9b75c1b7..8fcc3d8f 100644 --- a/lib/services/track_manager.dart +++ b/lib/services/track_manager.dart @@ -206,7 +206,13 @@ class TrackManager { /// source that advertises subtitles keeps listening for their late native /// track-list update. The listener has a separate hard deadline and every /// callback is scoped to the current media generation. + /// + /// Callers may arm this after an `await`, so a manager disposed or + /// deactivated in the meantime must not subscribe or start a timer: nothing + /// would ever cancel them. The generation checks inside each callback only + /// stop the work, not the allocation. void applyTrackSelectionWhenReady() { + if (!_managerIsActive) return; final selectionGeneration = _selectionGeneration; bool selectionIsCurrent() => _isSelectionCurrent(selectionGeneration); final currentTracks = player.state.tracks; diff --git a/test/services/track_manager_test.dart b/test/services/track_manager_test.dart index 15ee16a2..d8902f56 100644 --- a/test/services/track_manager_test.dart +++ b/test/services/track_manager_test.dart @@ -758,6 +758,117 @@ void main() { expect(player.selectedSubtitle.map((track) => track.id), ['native-late']); }); + test('a transcode switch superseded while persisting never arms the late-track pass', () async { + await SettingsService.getInstance(); + final sourceTrack = MediaSubtitleTrack( + id: 33, + index: 0, + languageCode: 'fra', + title: 'French', + codec: 'srt', + selected: false, + forced: false, + ); + final player = _FakePlayer( + tracks: const Tracks( + audio: [AudioTrack(id: 'audio', language: 'eng')], + ), + ); + final mgr = _make( + player: player, + mediaInfo: MediaSourceInfo( + videoUrl: 'https://example.com/transcode.m3u8', + partId: 101, + audioTracks: [MediaAudioTrack(id: 1, languageCode: 'eng', selected: true)], + subtitleTracks: [sourceTrack], + chapters: const [], + ), + ); + addTearDown(mgr.dispose); + + final persistGate = Completer(); + var switchIsCurrent = true; + + final pending = deferTranscodeSubtitleSelection( + trackManager: mgr, + sourceTrack: sourceTrack, + sourceSidecar: const PlaybackSubtitleSidecar( + sourceStreamId: 33, + preload: true, + track: SubtitleTrack( + id: 'container:33', + language: 'fra', + title: 'French', + codec: 'srt', + isExternal: true, + isContainer: true, + uri: 'https://example.com/video.mkv', + ), + ), + sourceStreamId: 33, + onSubtitleTrackChanged: (track, {sourceStreamId}) async { + await persistGate.future; + await mgr.onSubtitleTrackSelectedByUser(track, sourceStreamId: sourceStreamId); + }, + shouldContinue: () => switchIsCurrent, + ); + + // The source switch is superseded while the persist is still suspended. + switchIsCurrent = false; + persistGate.complete(); + expect(await pending, isFalse); + expect(player.tracksController.hasListener, isFalse); + + player.emitTracks( + const Tracks( + audio: [AudioTrack(id: 'audio', language: 'eng')], + subtitle: [ + SubtitleTrack( + id: 'native-superseded', + language: 'fra', + title: 'French', + codec: 'srt', + isExternal: true, + isContainer: true, + uri: 'https://example.com/video.mkv', + ), + ], + ), + ); + await _drainAsync(); + + expect(player.selectedSubtitle, isEmpty); + }); + + test('arming a disposed manager subscribes nothing and starts no timer', () async { + await SettingsService.getInstance(); + + fakeAsync((async) { + final player = _FakePlayer( + tracks: const Tracks( + audio: [AudioTrack(id: '1', language: 'eng')], + ), + ); + final mgr = _make(player: player, mediaInfo: _mediaInfoWithSubtitles(selected: true)); + mgr.dispose(); + + mgr.applyTrackSelectionWhenReady(); + + expect(player.tracksController.hasListener, isFalse); + expect(async.nonPeriodicTimerCount, 0); + + player.emitTracks( + const Tracks( + audio: [AudioTrack(id: '1', language: 'eng')], + subtitle: [SubtitleTrack(id: '10', language: 'eng')], + ), + ); + async.flushMicrotasks(); + + expect(player.selectedSubtitle, isEmpty); + }); + }); + test('five-second fallback keeps listening and applies a late advertised subtitle', () async { await SettingsService.getInstance();