diff --git a/lib/screens/video_player/live_stream_retry.dart b/lib/screens/video_player/live_stream_retry.dart index b1bdd2af4..aee75214e 100644 --- a/lib/screens/video_player/live_stream_retry.dart +++ b/lib/screens/video_player/live_stream_retry.dart @@ -8,6 +8,12 @@ enum LiveStreamRetryResult { succeeded, failed, stale } /// Every asynchronous stage is followed by a current-attempt check. Failures /// from stale attempts are intentionally ignored because a newer operation /// owns the player and its error UI. +/// +/// A recovery may hand back the very session it was asked to recover +/// (Jellyfin's `recover` returns the receiver), so a failed attempt must not +/// discard — terminally stop-report — the session that is still current and +/// will be re-adopted by the next retry. [currentSession] guards the discard +/// by identity. Future runLiveStreamRetry({ required LiveStreamRecovery recover, required LiveStreamUrlLookup lookupStreamUrl, @@ -15,6 +21,7 @@ Future runLiveStreamRetry({ required Future Function(String streamUrl) open, required bool Function() isCurrent, required void Function(Session session) adoptSession, + required Session? Function() currentSession, required void Function(Object error, StackTrace stackTrace) reportFailure, required void Function(Session session) discardSession, required void Function() onFinished, @@ -44,7 +51,9 @@ Future runLiveStreamRetry({ reportFailure(error, stackTrace); return LiveStreamRetryResult.failed; } finally { - if (recovered != null && !adopted) discardSession(recovered); + if (recovered != null && !adopted && !identical(recovered, currentSession())) { + discardSession(recovered); + } onFinished(); } } diff --git a/lib/screens/video_player/parts/live_tv.dart b/lib/screens/video_player/parts/live_tv.dart index 0597f2db8..c4e685392 100644 --- a/lib/screens/video_player/parts/live_tv.dart +++ b/lib/screens/video_player/parts/live_tv.dart @@ -196,6 +196,10 @@ extension _VideoPlayerLiveTvMethods on VideoPlayerScreenState { _live.selectedSubtitle = recoveredSubtitle; _live.markStreamRestartedAtLiveEdge(); }, + // Jellyfin's recover() returns the receiver, so the recovered object can + // be the still-current session; the retry helper skips the discard by + // identity so a failed retry cannot terminally stop-report it. + currentSession: () => _live.session, discardSession: _abandonLiveSession, reportFailure: (error, stackTrace) { appLogger.e('Failed to recover live stream', error: error, stackTrace: stackTrace); diff --git a/lib/screens/video_player/parts/playback_prompts.dart b/lib/screens/video_player/parts/playback_prompts.dart index 381c4b2a8..e3be7be7e 100644 --- a/lib/screens/video_player/parts/playback_prompts.dart +++ b/lib/screens/video_player/parts/playback_prompts.dart @@ -287,6 +287,11 @@ extension _VideoPlayerPlaybackPromptMethods on VideoPlayerScreenState { _stillWatchingTimer?.cancel(); if (_showStillWatchingPrompt) { _unfocusStillWatchingPrompt(); + // Back or a next-episode action on the visible prompt is the same + // "still watching" acknowledgement as Continue: re-arm the sleep timer. + // Guarded on prompt visibility — ordinary navigation also dismisses + // here and must leave an armed timer untouched. + SleepTimerService().restartTimer(); _setPlayerState(() { _showStillWatchingPrompt = false; }); diff --git a/lib/screens/video_player/parts/playback_reload.dart b/lib/screens/video_player/parts/playback_reload.dart index 8c887640c..4153bebbf 100644 --- a/lib/screens/video_player/parts/playback_reload.dart +++ b/lib/screens/video_player/parts/playback_reload.dart @@ -741,13 +741,18 @@ extension _VideoPlayerReloadMethods on VideoPlayerScreenState { if (wasPlayingBeforeReload && mounted && player == currentPlayer) { unawaited(_playWithPlaybackIntent(currentPlayer)); } - } else if (_progressTracker == null && player == currentPlayer) { - // The new file is playing and its session is committed — keep the - // new identity and make sure progress reporting is wired to the - // item actually on screen (the failure may have hit before - // _wirePerItemPlaybackServices ran). + } + if (_progressTracker == null && player == currentPlayer) { + // Progress reporting must survive both exits: beforeArm disposed the + // tracker before the open boundary, so rebuild it for the item + // actually on screen (the failure may have hit before + // _wirePerItemPlaybackServices ran). On the rollback path + // _currentMetadata was just restored to the previous item; after a + // committed open it is the replacement. _playbackSession describes + // that same item on both paths, so its client and session fields are + // the right wiring either way. _wirePerItemPlaybackServices( - metadata: metadata, + metadata: _currentMetadata, mediaClient: _playbackSession?.reportingClient, offlineWatchService: offlineWatchService, playSessionId: _playbackPlaySessionId, diff --git a/lib/screens/video_player_screen.dart b/lib/screens/video_player_screen.dart index 30dce1c57..f05e860a2 100644 --- a/lib/screens/video_player_screen.dart +++ b/lib/screens/video_player_screen.dart @@ -863,6 +863,11 @@ class VideoPlayerScreenState extends State with WidgetsBindin @visibleForTesting PlayerChromeController get chromeController => _chromeController; + /// Lets reload-failure coverage assert the progress tracker was rebuilt and + /// which item it is bound to; the tracker itself is private screen state. + @visibleForTesting + PlaybackProgressTracker? get debugProgressTrackerForTesting => _progressTracker; + late final PlayerNavigationCoordinator _playerNavigationCoordinator; @override diff --git a/test/screens/video_player/live_stream_retry_test.dart b/test/screens/video_player/live_stream_retry_test.dart index 77058a924..c89d2df8f 100644 --- a/test/screens/video_player/live_stream_retry_test.dart +++ b/test/screens/video_player/live_stream_retry_test.dart @@ -70,6 +70,37 @@ void main() { expect(harness.adopted, isFalse); expect(harness.discarded, isTrue); }); + + test('a failed retry does not discard the recover-returned current session', () async { + // Jellyfin's recover() returns the receiver: the recovered object IS the + // session the screen still holds. Discarding it would stop-report (and + // terminally latch) the session playback keeps using and re-adopts on + // the next retry. + final harness = _RetryHarness(failAt: _Stage.open); + final session = Object(); + harness.activeSession = session; + harness.recoveredSession = session; + + expect(await harness.run(), LiveStreamRetryResult.failed); + + expect(harness.failures, hasLength(1)); + expect(harness.finished, isTrue); + expect(harness.adopted, isFalse); + expect(harness.discarded, isFalse); + }); + + test('a failed retry still discards a recovered session distinct from the current one', () async { + final harness = _RetryHarness(failAt: _Stage.open); + harness.activeSession = Object(); + harness.recoveredSession = Object(); + + expect(await harness.run(), LiveStreamRetryResult.failed); + + expect(harness.failures, hasLength(1)); + expect(harness.finished, isTrue); + expect(harness.adopted, isFalse); + expect(harness.discarded, isTrue); + }); }); } @@ -87,13 +118,19 @@ class _RetryHarness { bool adopted = false; bool discarded = false; + /// What the caller's `_live.session` currently holds; recover hands back + /// [recoveredSession] when set, otherwise a fresh object. + Object? activeSession; + Object? recoveredSession; + Future run() => runLiveStreamRetry( - recover: () => _stage(_Stage.recover, Object.new), + recover: () => _stage(_Stage.recover, () => recoveredSession ?? Object()), lookupStreamUrl: (_) => _stage(_Stage.lookup, () => 'https://example.com/live'), applyPlayerOptions: () => _stage(_Stage.options, () {}), open: (_) => _stage(_Stage.open, () {}), isCurrent: () => current, adoptSession: (_) => adopted = true, + currentSession: () => activeSession, reportFailure: (error, _) => failures.add(error), discardSession: (_) => discarded = true, onFinished: () => finished = true, diff --git a/test/screens/video_player/playback_reload_failure_test.dart b/test/screens/video_player/playback_reload_failure_test.dart new file mode 100644 index 000000000..3e2ab54ca --- /dev/null +++ b/test/screens/video_player/playback_reload_failure_test.dart @@ -0,0 +1,201 @@ +import 'dart:async'; +import 'dart:io'; + +import 'package:drift/drift.dart' show Value; +import 'package:drift/native.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:path_provider_platform_interface/path_provider_platform_interface.dart'; +import 'package:plezy/database/app_database.dart'; +import 'package:plezy/media/ids.dart'; +import 'package:plezy/media/media_display_criteria.dart'; +import 'package:plezy/models/download_models.dart'; +import 'package:plezy/mpv/mpv.dart'; +import 'package:plezy/providers/multi_server_provider.dart'; +import 'package:plezy/providers/playback_state_provider.dart'; +import 'package:plezy/providers/user_profile_provider.dart'; +import 'package:plezy/screens/video_player_screen.dart'; +import 'package:plezy/services/download_storage_service.dart'; +import 'package:plezy/services/offline_watch_sync_service.dart'; +import 'package:plezy/services/settings_service.dart'; +import 'package:provider/provider.dart'; + +import '../../test_helpers/io_fakes.dart'; +import '../../test_helpers/media_items.dart'; +import '../../test_helpers/mock_player_channels.dart'; +import '../../test_helpers/multi_server_fixtures.dart'; +import '../../test_helpers/prefs.dart'; +import '../../test_helpers/watch_together_fakes.dart'; + +/// Regression coverage for the in-place reload rollback: a reload that fails +/// AFTER `beforeArm` disposed the progress tracker but BEFORE the open +/// boundary must rebuild the tracker and bind it to the restored previous +/// item — otherwise the resumed session plays on with no progress reporting +/// and no exit flush. +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + late Directory tmpRoot; + late PathProviderPlatform previousPathProvider; + late AppDatabase db; + + setUp(() async { + resetSharedPreferencesForTest(); + SettingsService.resetForTesting(); + DownloadStorageService.resetForTesting(); + await SettingsService.getInstance(); + tmpRoot = await Directory.systemTemp.createTemp('playback_reload_failure_test_'); + previousPathProvider = PathProviderPlatform.instance; + PathProviderPlatform.instance = FakePathProvider(tmpRoot); + await DownloadStorageService.instance.initialize(SettingsService.instance); + db = AppDatabase.forTesting(NativeDatabase.memory()); + }); + + tearDown(() async { + await db.close(); + DownloadStorageService.resetForTesting(); + SettingsService.resetForTesting(); + PathProviderPlatform.instance = previousPathProvider; + if (await tmpRoot.exists()) { + await tmpRoot.delete(recursive: true); + } + }); + + testWidgets('reload failing at the open boundary rebuilds the tracker for the prior item', (tester) async { + final priorItem = testMediaItem(id: 'movie-prior', serverId: 'srv-1', title: 'Prior movie'); + final targetItem = testMediaItem(id: 'movie-next', serverId: 'srv-1', title: 'Next movie'); + + // The reload target resolves offline from this row; the content:// path is + // treated as directly playable (no filesystem check), so the flow reaches + // player.open deterministically. + await _insertCompletedDownload(db, serverId: ServerId('srv-1'), ratingKey: 'movie-next'); + + final multi = testMultiServer(); + final offlineWatch = OfflineWatchSyncService(database: db, serverManager: multi.manager); + final userProfiles = UserProfileProvider(); + addTearDown(() { + offlineWatch.dispose(); + userProfiles.dispose(); + }); + + final nativeInitialize = Completer(); + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) { + if (call.method == 'initialize') return nativeInitialize.future; + return Future.value(null); + }, + eventHandler: (_) async => null, + testBody: () async { + final key = GlobalKey(); + await tester.pumpWidget( + MultiProvider( + providers: [ + ChangeNotifierProvider(create: (_) => PlaybackStateProvider()), + ChangeNotifierProvider.value(value: multi.provider), + ChangeNotifierProvider.value(value: offlineWatch), + ChangeNotifierProvider.value(value: userProfiles), + Provider.value(value: db), + ], + child: MaterialApp( + home: VideoPlayerScreen(key: key, metadata: priorItem, isOffline: true), + ), + ), + ); + expect(key.currentState, isNotNull); + + final fakePlayer = _OpenThrowingPlayer(); + addTearDown(fakePlayer.dispose); + key.currentState!.player = fakePlayer; + expect(key.currentState!.debugProgressTrackerForTesting, isNull); + + var navDone = false; + final nav = key.currentState!.navigateToQueueItem(targetItem).whenComplete(() => navDone = true); + // Drive the reload to completion: the offline profile-settings wait + // bounds itself at 2s of test clock, and drift/database work needs + // real-event-loop yields. + for (var i = 0; i < 400 && !navDone; i++) { + await tester.pump(const Duration(milliseconds: 50)); + if (!navDone) { + await tester.runAsync(() => Future.delayed(const Duration(milliseconds: 2))); + } + } + expect(navDone, isTrue, reason: 'the in-place reload must settle'); + await nav; + + expect(fakePlayer.openCalls, 1, reason: 'the reload must fail at player.open — after beforeArm ran'); + + // The rollback must rebuild the tracker beforeArm disposed and bind it + // to the restored previous item, not the failed replacement. + final tracker = key.currentState!.debugProgressTrackerForTesting; + expect(tracker, isNotNull, reason: 'a failed pre-open reload must re-wire progress reporting'); + expect(tracker!.metadata.globalKey, priorItem.globalKey); + expect(tracker.metadata.globalKey, isNot(targetItem.globalKey)); + + // Let the rollback's failure snackbar run its display timer down so + // nothing is pending when the tree unmounts. + await tester.pump(const Duration(seconds: 5)); + await tester.pump(const Duration(seconds: 1)); + + await tester.pumpWidget(const SizedBox.shrink()); + nativeInitialize.complete(true); + await tester.pump(); + }, + ); + }); +} + +/// Minimal player for the reload flow whose `open` always throws — the exact +/// "failure before the open boundary" the rollback path recovers from. +class _OpenThrowingPlayer extends FakeSyncPlayer { + _OpenThrowingPlayer() : super(playing: true, duration: const Duration(minutes: 40)); + + int openCalls = 0; + + @override + String get playerType => 'mpv'; + + @override + bool get attachesExternalSubtitlesAtOpen => true; + + @override + bool get needsDecoderRefreshAfterDisplaySwitch => false; + + @override + Future setProperty(String name, String value) async {} + + @override + Future setDisplayCriteria(MediaDisplayCriteria? criteria, {int extraDelayMs = 0}) async {} + + @override + Future requestAudioFocus() async => true; + + @override + Future open( + Media media, { + bool play = true, + bool isLive = false, + List? externalSubtitles, + Duration? timelineDuration, + }) async { + openCalls++; + throw StateError('open failed before the open boundary'); + } +} + +Future _insertCompletedDownload(AppDatabase db, {required ServerId serverId, required String ratingKey}) async { + await db + .into(db.downloadedMedia) + .insert( + DownloadedMediaCompanion.insert( + serverId: serverId, + ratingKey: ratingKey, + globalKey: '$serverId:$ratingKey', + type: 'movie', + status: DownloadStatus.completed.index, + videoFilePath: Value('content://offline/$ratingKey'), + mediaIndex: const Value(0), + ), + ); +} diff --git a/test/screens/video_player/still_watching_sleep_timer_test.dart b/test/screens/video_player/still_watching_sleep_timer_test.dart new file mode 100644 index 000000000..d78ff1700 --- /dev/null +++ b/test/screens/video_player/still_watching_sleep_timer_test.dart @@ -0,0 +1,131 @@ +import 'dart:async'; + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:plezy/providers/playback_state_provider.dart'; +import 'package:plezy/screens/video_player_screen.dart'; +import 'package:plezy/services/settings_service.dart'; +import 'package:plezy/services/sleep_timer_service.dart'; +import 'package:provider/provider.dart'; + +import '../../test_helpers/media_items.dart'; +import '../../test_helpers/mock_player_channels.dart'; +import '../../test_helpers/prefs.dart'; + +/// Regression coverage for the sleep timer's "Still watching?" acknowledgement +/// semantics: dismissing the visible prompt via system back must re-arm the +/// timer exactly like Continue, while a back with no prompt on screen must +/// leave the timer untouched. +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + setUp(() async { + resetSharedPreferencesForTest(); + SettingsService.resetForTesting(); + await SettingsService.getInstance(); + SleepTimerService().cancelTimer(); + }); + + tearDown(() { + SleepTimerService().cancelTimer(); + }); + + testWidgets('back on the visible still-watching prompt re-arms the sleep timer with its original duration', ( + tester, + ) async { + final nativeInitialize = Completer(); + + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) { + if (call.method == 'initialize') return nativeInitialize.future; + return Future.value(null); + }, + eventHandler: (_) async => null, + testBody: () async { + final key = GlobalKey(); + await tester.pumpWidget(_screen(key)); + expect(key.currentState, isNotNull); + + final sleepTimer = SleepTimerService(); + sleepTimer.startTimer(const Duration(minutes: 30), () {}); + // Move the deadline into the past; the next 1s heartbeat fires the + // still-watching prompt while _originalDuration stays 30 minutes. + sleepTimer.extendTimer(const Duration(minutes: -31)); + await tester.pump(const Duration(seconds: 1)); + // Let the onPrompt stream event reach the screen's listener. + await tester.pump(); + + // The prompt consumed the countdown: the timer parks awaiting an answer. + expect(sleepTimer.isActive, isFalse); + expect(sleepTimer.originalDuration, const Duration(minutes: 30)); + + // System back dismisses the visible prompt — the same acknowledgement + // as Continue, so the timer must restart with its original duration. + await tester.binding.handlePopRoute(); + await tester.pump(); + + expect(sleepTimer.isActive, isTrue); + expect(sleepTimer.duration, const Duration(minutes: 30)); + expect(sleepTimer.originalDuration, const Duration(minutes: 30)); + + sleepTimer.cancelTimer(); + await tester.pumpWidget(const SizedBox.shrink()); + nativeInitialize.complete(true); + await tester.pump(); + }, + ); + }); + + testWidgets('back with no prompt visible leaves the sleep timer untouched', (tester) async { + final nativeInitialize = Completer(); + + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) { + if (call.method == 'initialize') return nativeInitialize.future; + return Future.value(null); + }, + eventHandler: (_) async => null, + testBody: () async { + final key = GlobalKey(); + await tester.pumpWidget(_screen(key)); + expect(key.currentState, isNotNull); + + final sleepTimer = SleepTimerService(); + sleepTimer.startTimer(const Duration(minutes: 30), () {}); + final endTimeBefore = sleepTimer.endTime; + expect(endTimeBefore, isNotNull); + + // No prompt is on screen: back walks the ordinary exit pipeline and + // must not reset (or restart) the armed timer. + await tester.binding.handlePopRoute(); + await tester.pump(); + + expect(sleepTimer.isActive, isTrue); + expect(sleepTimer.endTime, endTimeBefore); + expect(sleepTimer.duration, const Duration(minutes: 30)); + + sleepTimer.cancelTimer(); + await tester.pumpWidget(const SizedBox.shrink()); + nativeInitialize.complete(true); + await tester.pump(); + }, + ); + }); +} + +Widget _screen(GlobalKey key) { + return ChangeNotifierProvider( + create: (_) => PlaybackStateProvider(), + child: MaterialApp( + home: VideoPlayerScreen( + key: key, + metadata: testMediaItem(title: 'Still watching sleep timer test'), + isOffline: true, + ), + ), + ); +}