diff --git a/lib/i18n/en.i18n.json b/lib/i18n/en.i18n.json index 3d0021fd..23836ed2 100644 --- a/lib/i18n/en.i18n.json +++ b/lib/i18n/en.i18n.json @@ -544,6 +544,7 @@ "autoRemovedWatchedDownload": "Auto-removed: ${title}", "removedFromContinueWatching": "Removed from Continue Watching", "errorLoading": "Error: ${error}", + "streamInterrupted": "The stream was interrupted. Press play or seek to retry.", "fileInfoNotAvailable": "File information not available", "errorLoadingFileInfo": "Error loading file info: ${error}", "errorLoadingSeries": "Error loading series", diff --git a/lib/i18n/strings.g.dart b/lib/i18n/strings.g.dart index cad77199..5dbfb92d 100644 --- a/lib/i18n/strings.g.dart +++ b/lib/i18n/strings.g.dart @@ -4,7 +4,7 @@ /// To regenerate, run: `dart run slang` /// /// Locales: 16 -/// Strings: 22465 (1404 per locale) +/// Strings: 22466 (1404 per locale) // coverage:ignore-file // ignore_for_file: type=lint, unused_import diff --git a/lib/i18n/strings_en.g.dart b/lib/i18n/strings_en.g.dart index a0f29818..05cf3110 100644 --- a/lib/i18n/strings_en.g.dart +++ b/lib/i18n/strings_en.g.dart @@ -1672,6 +1672,9 @@ class TranslationsMessagesEn { /// en: 'Error: ${error}' String errorLoading({required Object error}) => 'Error: ${error}'; + /// en: 'The stream was interrupted. Press play or seek to retry.' + String get streamInterrupted => 'The stream was interrupted. Press play or seek to retry.'; + /// en: 'File information not available' String get fileInfoNotAvailable => 'File information not available'; @@ -5470,10 +5473,11 @@ extension on Translations { 'messages.autoRemovedWatchedDownload' => ({required Object title}) => 'Auto-removed: ${title}', 'messages.removedFromContinueWatching' => 'Removed from Continue Watching', 'messages.errorLoading' => ({required Object error}) => 'Error: ${error}', + 'messages.streamInterrupted' => 'The stream was interrupted. Press play or seek to retry.', 'messages.fileInfoNotAvailable' => 'File information not available', - 'messages.errorLoadingFileInfo' => ({required Object error}) => 'Error loading file info: ${error}', _ => null, } ?? switch (path) { + 'messages.errorLoadingFileInfo' => ({required Object error}) => 'Error loading file info: ${error}', 'messages.errorLoadingSeries' => 'Error loading series', 'messages.musicNotSupported' => 'Music playback is not yet supported', 'messages.noDescriptionAvailable' => 'No description available', @@ -5985,9 +5989,9 @@ extension on Translations { 'downloads.tvShows' => 'TV Shows', 'downloads.movies' => 'Movies', 'downloads.music' => 'Music', - 'downloads.tracksQueued' => ({required Object count}) => '${count} tracks queued for download', _ => null, } ?? switch (path) { + 'downloads.tracksQueued' => ({required Object count}) => '${count} tracks queued for download', 'downloads.noDownloads' => 'No downloads yet', 'downloads.noDownloadsDescription' => 'Downloaded content will appear here for offline viewing', 'downloads.downloadNow' => 'Download', diff --git a/lib/screens/video_player/completion_latch.dart b/lib/screens/video_player/completion_latch.dart index 55ae6b22..9eb3b435 100644 --- a/lib/screens/video_player/completion_latch.dart +++ b/lib/screens/video_player/completion_latch.dart @@ -1,3 +1,41 @@ +import 'dart:math' as math; + +/// Position must be within this many ms of the best-known duration for a +/// player EOF signal to count as the real end of the media. +/// +/// Wide enough that a transcode container ending a couple of seconds short +/// of the server metadata duration still classifies as genuine, yet a +/// spurious EOF that slips through inside the window lands where servers +/// already mark the item watched (~90%), so the user outcome is unchanged. +/// The failure this guards against (#1520) parks playback minutes short. +const int spuriousEofToleranceMs = 10000; + +/// How a player EOF signal should be interpreted. +enum EofSignalClass { genuine, spurious, unknown } + +/// Classify a player EOF signal against the best-known media duration. +/// +/// mpv reports a clean EOF when a network stream dies mid-file (a reaped +/// transcode session or an idle connection closed during a long pause), so +/// the signal alone cannot be trusted — position is the only discriminator. +/// +/// [playerDurationMs] alone is not trustworthy either: on chunked transcode +/// streams the player's duration can be unknown or track the growing demuxer +/// cache (i.e. equal the parked position), making every spurious EOF look +/// genuine. [metadataDurationMs] (the server's item duration) anchors the +/// comparison; max() of the two also covers the opposite failure — server +/// metadata understating the real file length. +EofSignalClass classifyEofSignal({ + required int positionMs, + required int playerDurationMs, + required int? metadataDurationMs, + int toleranceMs = spuriousEofToleranceMs, +}) { + final effectiveDurationMs = math.max(playerDurationMs, metadataDurationMs ?? 0); + if (effectiveDurationMs <= 0) return EofSignalClass.unknown; + return positionMs >= effectiveDurationMs - toleranceMs ? EofSignalClass.genuine : EofSignalClass.spurious; +} + /// What a position tick means for the end-of-video prompt flow. enum CompletionLatchSignal { /// Nothing to do. diff --git a/lib/screens/video_player/parts/episode_navigation.dart b/lib/screens/video_player/parts/episode_navigation.dart index ced57f2c..946c4f17 100644 --- a/lib/screens/video_player/parts/episode_navigation.dart +++ b/lib/screens/video_player/parts/episode_navigation.dart @@ -452,6 +452,12 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { // (open failures throw into the catch below) — superseded either way. if (!didOpen || !isCurrentReload()) return _MediaReloadOutcome.superseded; _completionLatch.reset(); + if (isItemChange) { + // Same-item reloads (including the spurious-EOF recovery itself and + // quality switches) keep the spent budget — that is the loop guard. + _spuriousEofRecoveryAttempts = 0; + _spuriousEofRecoveryBaselineMs = null; + } // Versions/mediaInfo come from the committed session; rebuild so the // controls pick them up. Same-part switches (quality/audio/subtitle) diff --git a/lib/screens/video_player/parts/playback_services.dart b/lib/screens/video_player/parts/playback_services.dart index 2393b3e2..47cb84c2 100644 --- a/lib/screens/video_player/parts/playback_services.dart +++ b/lib/screens/video_player/parts/playback_services.dart @@ -84,6 +84,12 @@ extension _VideoPlayerPlaybackServiceMethods on VideoPlayerScreenState { _rearmCompletionLatch(); } } + // A mid-file EOF is the stream dying under us (#1520), not the media + // ending: it must never mark the item watched, prompt Play Next, or + // exit a movie. Intercepted here and not inside _onVideoCompleted + // because the credits-marker auto-skip legitimately calls + // _onVideoCompleted from mid-credits positions. + if (done && _interceptSpuriousEof(currentPlayer)) return; _onVideoCompleted(done); }); @@ -114,9 +120,15 @@ extension _VideoPlayerPlaybackServiceMethods on VideoPlayerScreenState { final isOnline = statusMap[serverId] == true; if (!isOnline) { wasOffline = true; - } else if (wasOffline && _isBuffering.value) { + } else if (wasOffline && (_isBuffering.value || _spuriousEofRecoveryParked)) { wasOffline = false; - _forceStreamReconnect(); + if (_spuriousEofRecoveryParked) { + // A parked stream is dead server-side; a seek-in-place would + // land in the drained cache — only a fresh resolve replaces it. + unawaited(_retrySpuriousEofRecovery(reason: 'server back online')); + } else { + _forceStreamReconnect(); + } } }); } @@ -150,6 +162,16 @@ extension _VideoPlayerPlaybackServiceMethods on VideoPlayerScreenState { lastObservedPositionMs = position.inMilliseconds; } + // A recovered stream that progressed well past the recovery point + // proves the reload worked — restore the full spurious-EOF retry + // budget for the next stream death. + final recoveryBaselineMs = _spuriousEofRecoveryBaselineMs; + if (recoveryBaselineMs != null && + position.inMilliseconds >= recoveryBaselineMs + VideoPlayerScreenState._spuriousEofProgressResetMs) { + _spuriousEofRecoveryAttempts = 0; + _spuriousEofRecoveryBaselineMs = null; + } + final duration = activePlayer.state.duration; _completionLatch.classifyPosition( positionMs: position.inMilliseconds, @@ -546,4 +568,98 @@ extension _VideoPlayerPlaybackServiceMethods on VideoPlayerScreenState { _rearmCompletionLatch(); unawaited(_seekPlayback(pos)); } + + /// Intercept an EOF signal that fired far from the end of the media. + /// + /// The player reports a clean EOF when a network stream dies mid-file + /// (#1520) — no libmpv signal distinguishes it from the real end, so + /// position vs best-known duration is the only discriminator (see + /// [classifyEofSignal]). Returns true when the signal was spurious and + /// handled here (recovery started, or playback stays parked); false lets + /// the caller run the normal completion flow. + bool _interceptSpuriousEof(Player currentPlayer) { + // Live EOFs have their own handling, an offline file can't lose its + // stream, and in-flight transitions already produce expected EOFs that + // _onVideoCompleted ignores — all fall through untouched. + if (widget.isLive || _isOfflinePlayback) return false; + if (_playbackTransition != _PlaybackTransition.idle) return false; + // Already parked: swallow duplicate EOF signals without burning budget + // or re-toasting. + if (_spuriousEofRecoveryParked) return true; + + final positionMs = currentPlayer.state.position.inMilliseconds; + final playerDurationMs = currentPlayer.state.duration.inMilliseconds; + final metadataDurationMs = _currentMetadata.durationMs; + final signal = classifyEofSignal( + positionMs: positionMs, + playerDurationMs: playerDurationMs, + metadataDurationMs: metadataDurationMs, + ); + if (signal != EofSignalClass.spurious) return false; + + appLogger.w( + 'Spurious EOF at ${positionMs}ms (playerDuration=${playerDurationMs}ms, ' + 'metadataDuration=${metadataDurationMs}ms, ' + 'cacheEnd=${currentPlayer.state.buffer.inMilliseconds}ms), ' + 'recovery attempt ${_spuriousEofRecoveryAttempts + 1}/' + '${VideoPlayerScreenState._maxSpuriousEofRecoveryAttempts}', + ); + + if (_spuriousEofRecoveryAttempts >= VideoPlayerScreenState._maxSpuriousEofRecoveryAttempts) { + _parkAfterFailedRecovery(); + return true; + } + _spuriousEofRecoveryAttempts++; + _spuriousEofRecoveryBaselineMs = positionMs; + unawaited(_recoverFromSpuriousEof(currentPlayer)); + return true; + } + + /// Leave playback parked on the dead stream: no auto-exit — the user keeps + /// their place and the snackbar names the actions that actually rebuild the + /// stream (play/seek route to [_retrySpuriousEofRecovery] while parked). + void _parkAfterFailedRecovery() { + _spuriousEofRecoveryParked = true; + unawaited(_setWakelock(false)); + showGlobalErrorSnackBar(t.messages.streamInterrupted); + } + + /// Recover from a spurious EOF by re-running the full playback decision in + /// place — the same path as the TV background suspend restore, because the + /// failure is the same: the server-side stream is gone and only a fresh + /// resolve replaces it (a seek-in-place lands inside the dead cache, and a + /// same-session transcode seek can hit the reaped session). + Future _recoverFromSpuriousEof(Player currentPlayer) async { + final outcome = await _reloadMediaInPlace( + metadata: _currentMetadata, + resumePosition: currentPlayer.state.position, + preserveCurrentTrackSelection: true, + startPaused: !_playbackIntentShouldPlay, + showErrorUi: false, + reason: 'spurious EOF recovery', + ); + if (outcome == _MediaReloadOutcome.failed) _parkAfterFailedRecovery(); + // rejected/superseded: another flow owns the player and will commit + // fresh media (clearing any park). opened: recovered — the budget + // resets via 30s of progress or an item change. + } + + /// Rebuild the dead stream after playback parked on a spurious EOF. + /// User actions and the server-online monitor land here; these retries are + /// always allowed and never consume the automatic budget. + Future _retrySpuriousEofRecovery({required String reason, Duration? resumePosition}) async { + final currentPlayer = player; + if (currentPlayer == null || _playbackTransition != _PlaybackTransition.idle) return; + appLogger.i('Retrying dead-stream recovery ($reason)'); + _spuriousEofRecoveryParked = false; + final outcome = await _reloadMediaInPlace( + metadata: _currentMetadata, + resumePosition: resumePosition ?? currentPlayer.state.position, + preserveCurrentTrackSelection: true, + startPaused: !_playbackIntentShouldPlay, + showErrorUi: false, + reason: 'stream recovery ($reason)', + ); + if (outcome == _MediaReloadOutcome.failed) _parkAfterFailedRecovery(); + } } diff --git a/lib/screens/video_player/parts/seeking.dart b/lib/screens/video_player/parts/seeking.dart index 873504dc..859444ed 100644 --- a/lib/screens/video_player/parts/seeking.dart +++ b/lib/screens/video_player/parts/seeking.dart @@ -6,6 +6,12 @@ extension _VideoPlayerSeekingMethods on VideoPlayerScreenState { if (!mounted || currentPlayer == null) return; final target = clampSeekPosition(currentPlayer, position); + // Parked on a dead stream (#1520): a native seek would land inside the + // drained cache — rebuild the stream at the target instead. + if (_spuriousEofRecoveryParked && !widget.isLive && _playbackTransition == _PlaybackTransition.idle) { + await _retrySpuriousEofRecovery(reason: 'seek', resumePosition: target); + return; + } if (_plexTranscodeSeekAction(currentPlayer, target) == PlexTranscodeSeekAction.nativeSeek) { await currentPlayer.seek(target); return; diff --git a/lib/screens/video_player_screen.dart b/lib/screens/video_player_screen.dart index 37049a24..0e4b91cb 100644 --- a/lib/screens/video_player_screen.dart +++ b/lib/screens/video_player_screen.dart @@ -375,6 +375,23 @@ class VideoPlayerScreenState extends State with WidgetsBindin // position ticks only re-arm once playback is more than 2s from the end. final CompletionLatch _completionLatch = CompletionLatch(rearmWindowMs: 2000); + // Spurious-EOF recovery (#1520): a long pause can get the server-side + // stream reaped or the idle socket killed; on resume the player drains its + // cache and signals a clean EOF mid-file. Recovery reloads in place, + // bounded so a persistently dying stream can't reload-loop. The budget + // restores once playback progresses well past the last recovery point or + // on an item change; user-initiated retries (play/seek) are always allowed + // and never consume it. + static const int _maxSpuriousEofRecoveryAttempts = 2; + static const int _spuriousEofProgressResetMs = 30000; + int _spuriousEofRecoveryAttempts = 0; + int? _spuriousEofRecoveryBaselineMs; + + /// Playback is parked mid-file on a dead stream: automatic recovery failed + /// or its budget is spent. Exits: user play/seek (always allowed) or the + /// server-status monitor seeing the server come back online. + bool _spuriousEofRecoveryParked = false; + late final FocusNode _playNextCancelFocusNode; late final FocusNode _playNextConfirmFocusNode; @@ -498,6 +515,8 @@ class VideoPlayerScreenState extends State with WidgetsBindin _requestedMediaSourceId = session.mediaSourceId; _selectedQualityPreset = session.qualityPreset; _selectedAudioStreamId = session.audioStreamId; + // Any freshly opened stream ends a dead-stream park (#1520). + _spuriousEofRecoveryParked = false; // Every successful open passes through here (never live TV), making it // the chokepoint for the local last-played history. Offline plays are // excluded — like version prefs, the history describes online intent. @@ -526,6 +545,12 @@ class VideoPlayerScreenState extends State with WidgetsBindin Future _playWithPlaybackIntent(Player currentPlayer) { _playbackIntentShouldPlay = true; + if (_spuriousEofRecoveryParked && _playbackTransition == _PlaybackTransition.idle) { + // Parked on a dead stream: play/pause on a drained cache is a no-op + // (mpv doesn't even flip `pause` on EOF), so any press means "get my + // video back" — rebuild the stream instead (#1520). + return _retrySpuriousEofRecovery(reason: 'play pressed'); + } return currentPlayer.play(); } @@ -535,6 +560,10 @@ class VideoPlayerScreenState extends State with WidgetsBindin } Future _playOrPauseWithPlaybackIntent(Player currentPlayer) { + if (_spuriousEofRecoveryParked && _playbackTransition == _PlaybackTransition.idle) { + _playbackIntentShouldPlay = true; + return _retrySpuriousEofRecovery(reason: 'play/pause pressed'); + } _playbackIntentShouldPlay = !currentPlayer.state.playing; return currentPlayer.playOrPause(); } diff --git a/test/screens/video_player/completion_latch_test.dart b/test/screens/video_player/completion_latch_test.dart index bcf9d8df..9e2864e3 100644 --- a/test/screens/video_player/completion_latch_test.dart +++ b/test/screens/video_player/completion_latch_test.dart @@ -76,4 +76,57 @@ void main() { l.rearmIfClear(promptVisible: false, countdownActive: false); expect(l.triggered, isFalse); }); + + group('classifyEofSignal', () { + EofSignalClass classify(int positionMs, {int playerDurationMs = 0, int? metadataDurationMs}) => classifyEofSignal( + positionMs: positionMs, + playerDurationMs: playerDurationMs, + metadataDurationMs: metadataDurationMs, + ); + + test('mid-file EOF is spurious (#1520)', () { + expect(classify(600000, playerDurationMs: 2520000, metadataDurationMs: 2520000), EofSignalClass.spurious); + }); + + test('metadata anchors when player duration tracks the demuxer cache', () { + // Chunked transcode: mpv's duration equals the parked position, which + // alone would make the dead stream look genuinely finished. + expect(classify(600000, playerDurationMs: 600000, metadataDurationMs: 2520000), EofSignalClass.spurious); + }); + + test('genuine at the exact end', () { + expect(classify(2520000, playerDurationMs: 2520000, metadataDurationMs: 2520000), EofSignalClass.genuine); + }); + + test('genuine when the stream ends slightly short of metadata duration', () { + expect(classify(2517000, playerDurationMs: 2517000, metadataDurationMs: 2520000), EofSignalClass.genuine); + }); + + test('player duration wins when metadata understates the file', () { + // The 3b611a1e failure mode: a short metadata duration must not turn + // the real end of a longer file into a spurious classification. + expect(classify(2520000, playerDurationMs: 2520000, metadataDurationMs: 2400000), EofSignalClass.genuine); + }); + + test('classifies from metadata alone when player duration is unknown', () { + expect(classify(600000, metadataDurationMs: 2520000), EofSignalClass.spurious); + expect(classify(2515000, metadataDurationMs: 2520000), EofSignalClass.genuine); + }); + + test('unknown when no duration is available', () { + expect(classify(600000), EofSignalClass.unknown); + expect(classify(600000, metadataDurationMs: 0), EofSignalClass.unknown); + }); + + test('tolerance boundary is inclusive', () { + expect( + classify(2520000 - spuriousEofToleranceMs, playerDurationMs: 2520000, metadataDurationMs: null), + EofSignalClass.genuine, + ); + expect( + classify(2520000 - spuriousEofToleranceMs - 1, playerDurationMs: 2520000, metadataDurationMs: null), + EofSignalClass.spurious, + ); + }); + }); }