From d07d849fe40006673488d17a3a42fd91164f6b5a Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Fri, 10 Jul 2026 19:06:51 +0200 Subject: [PATCH] refactor(player): return an outcome enum from _reloadMediaInPlace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old bool meant "attempt was current and handled", so callers could not distinguish a real pre-open failure (old stream still loaded) from success — the distinction the #1520 recovery needs. --- .../parts/episode_navigation.dart | 43 +++++++++++-------- lib/screens/video_player/parts/lifecycle.dart | 6 ++- .../video_player/parts/watch_together.dart | 8 ++-- lib/screens/video_player_screen.dart | 21 +++++++++ 4 files changed, 54 insertions(+), 24 deletions(-) diff --git a/lib/screens/video_player/parts/episode_navigation.dart b/lib/screens/video_player/parts/episode_navigation.dart index 5a84a844..ced57f2c 100644 --- a/lib/screens/video_player/parts/episode_navigation.dart +++ b/lib/screens/video_player/parts/episode_navigation.dart @@ -221,7 +221,12 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { /// every post-open resume point (subtitle-load resume, frame-rate gate /// release) arms track selection without playing, the same way a Watch /// Together-owned start does. The caller owns starting playback. - Future _reloadMediaInPlace({ + /// + /// The returned [_MediaReloadOutcome] tells the caller what actually + /// happened: only [_MediaReloadOutcome.failed] means the previous session + /// is still on screen with its (possibly dead) stream; user feedback for + /// failures is shown here unless [showErrorUi] is false. + Future<_MediaReloadOutcome> _reloadMediaInPlace({ required MediaItem metadata, int? selectedMediaIndex, String? selectedMediaSourceId, @@ -240,12 +245,12 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { }) async { if (widget.isLive) { _clearEpisodeLoadingFlags(); - return false; + return _MediaReloadOutcome.rejected; } final existingPlayer = player; if (!mounted || existingPlayer == null || _playbackTransition != _PlaybackTransition.idle) { if (mounted) _clearEpisodeLoadingFlags(); - return false; + return _MediaReloadOutcome.rejected; } _playbackTransition = _PlaybackTransition.reloadingMedia; @@ -294,7 +299,7 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { final cycleWatchTogetherAttachment = watchTogetherWasAttached; final wtOwnsStart = _watchTogetherOwnsPlaybackStart(); - if (!isCurrentReload()) return true; + if (!isCurrentReload()) return _MediaReloadOutcome.superseded; final targetMediaIndex = selectedMediaIndex ?? _effectiveSelectedMediaIndex; final targetQualityPreset = qualityPreset ?? _selectedQualityPreset; @@ -323,7 +328,7 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { } catch (e) { appLogger.w('Failed to pause before $reason', error: e); } - if (!isCurrentReload()) return true; + if (!isCurrentReload()) return _MediaReloadOutcome.superseded; // Overlap the old item's stop report with the resolve round-trip; it // is awaited again right before the open below. @@ -341,7 +346,7 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { sessionIdentifier: _playbackSessionIdentifier, transcodeSessionId: _playbackTranscodeSessionId, ); - if (!isCurrentReload()) return true; + if (!isCurrentReload()) return _MediaReloadOutcome.superseded; final result = playbackContext.result; final mediaClient = playbackContext.reportingClient; final plexClient = mediaClient is PlexClient ? mediaClient : null; @@ -369,11 +374,11 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { offlineWatchService: offlineWatchService, requested: resumePosition, ); - if (!isCurrentReload()) return true; + if (!isCurrentReload()) return _MediaReloadOutcome.superseded; final displayCriteria = result.mediaInfo?.displayCriteria; final settingsService = await SettingsService.getInstance(); - if (!isCurrentReload()) return true; + if (!isCurrentReload()) return _MediaReloadOutcome.superseded; // Same pre-open frame-rate orchestration as the initial start flow — // including the Android MPV startup decoder refresh, whose gate is @@ -387,7 +392,7 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { hasVideoUrl: true, ensureAudioFocus: () => currentPlayer.requestAudioFocus(), ); - if (frameRatePlan == null || !isCurrentReload()) return true; + if (frameRatePlan == null || !isCurrentReload()) return _MediaReloadOutcome.superseded; _frameRate.resetForNewItem(); if (frameRatePlan.countsAsApplied) _frameRate.applied = true; @@ -397,7 +402,7 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { displayCriteria: displayCriteria, isTranscoding: result.isTranscoding, ); - if (!isCurrentReload()) return true; + if (!isCurrentReload()) return _MediaReloadOutcome.superseded; final openTiming = _playbackOpenTiming( backend: metadata.backend, isTranscoding: result.isTranscoding, @@ -411,7 +416,7 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { unawaited(DiscordRPCService.instance.stopPlayback()); unawaited(TraktScrobbleService.instance.stopPlayback()); unawaited(TrackerCoordinator.instance.stopPlayback()); - if (!isCurrentReload()) return true; + if (!isCurrentReload()) return _MediaReloadOutcome.superseded; frameRatePlan.armStartupRefreshGate(currentPlayer); final externalSubtitlePlan = _prepareExternalSubtitleOpenPlan( @@ -443,7 +448,9 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { _commitPlaybackSession(session); }, ); - if (!didOpen || !isCurrentReload()) return true; + // A false didOpen means shouldContinue stopped the sequence pre-open + // (open failures throw into the catch below) — superseded either way. + if (!didOpen || !isCurrentReload()) return _MediaReloadOutcome.superseded; _completionLatch.reset(); // Versions/mediaInfo come from the committed session; rebuild so the @@ -492,7 +499,7 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { player == currentPlayer, applySelectionWhenResumeSkipped: (wtOwnsStart || startPaused) && !frameRatePlan.holdPlaybackStart, ); - if (!isCurrentReload()) return true; + if (!isCurrentReload()) return _MediaReloadOutcome.superseded; await _releaseFrameRateStartupGate( currentPlayer: currentPlayer, @@ -508,7 +515,7 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { ), playbackResumedForStartupFrame: resumeForStartupFrame, ); - if (!isCurrentReload()) return true; + if (!isCurrentReload()) return _MediaReloadOutcome.superseded; // Same helper as the initial start flow, so any future change lands in // both paths together. @@ -528,14 +535,14 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { } unawaited(_loadAdjacentEpisodes(metadata: metadata, attempt: attempt)); - if (!isCurrentReload()) return true; + if (!isCurrentReload()) return _MediaReloadOutcome.superseded; if (_autoPipEnabled) { unawaited(_videoPIPManager?.updateAutoPipState(isPlaying: currentPlayer.state.playing)); } - return true; + return _MediaReloadOutcome.opened; } catch (e) { - if (!isCurrentReload()) return true; + if (!isCurrentReload()) return _MediaReloadOutcome.superseded; _completionLatch.reset(); if (!didOpenReplacement) { // Nothing was opened: the previous session is still committed, so @@ -576,7 +583,7 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { if (mounted && showErrorUi) { showErrorSnackBar(context, t.messages.errorLoading(error: e.toString())); } - return true; + return didOpenReplacement ? _MediaReloadOutcome.opened : _MediaReloadOutcome.failed; } finally { // Release the reload transition unless a newer flow already took // ownership (a non-reload attempt force-idles it; a newer reload can diff --git a/lib/screens/video_player/parts/lifecycle.dart b/lib/screens/video_player/parts/lifecycle.dart index 658daea0..302e9db0 100644 --- a/lib/screens/video_player/parts/lifecycle.dart +++ b/lib/screens/video_player/parts/lifecycle.dart @@ -268,7 +268,7 @@ extension _VideoPlayerLifecycleMethods on VideoPlayerScreenState { if (!mounted || currentPlayer == null || !_isPlayerInitialized) return; _recordLifecycleState('resumed', action: 'tv_background_suspend_reload'); - final reloaded = await _reloadMediaInPlace( + final outcome = await _reloadMediaInPlace( metadata: _currentMetadata, resumePosition: resumePosition, preserveCurrentTrackSelection: true, @@ -278,8 +278,10 @@ extension _VideoPlayerLifecycleMethods on VideoPlayerScreenState { startPaused: true, reason: 'TV background suspend restore', ); - if (!reloaded) { + if (outcome == _MediaReloadOutcome.rejected) { appLogger.w('TV background suspend restore: in-place reload rejected'); + } else if (outcome == _MediaReloadOutcome.failed) { + appLogger.w('TV background suspend restore: in-place reload failed'); } } } diff --git a/lib/screens/video_player/parts/watch_together.dart b/lib/screens/video_player/parts/watch_together.dart index 4759c905..edbe3379 100644 --- a/lib/screens/video_player/parts/watch_together.dart +++ b/lib/screens/video_player/parts/watch_together.dart @@ -175,7 +175,7 @@ extension _VideoPlayerWatchTogetherMethods on VideoPlayerScreenState { // fetchItem populates mediaVersions, so the saved preference resolves to // a verified index/id here rather than a raw stored index. final savedVersion = await resolveSavedMediaVersionFor(metadata); - final handled = await _reloadMediaInPlace( + final outcome = await _reloadMediaInPlace( metadata: metadata, selectedMediaIndex: savedVersion?.index ?? 0, selectedMediaSourceId: savedVersion?.sourceId, @@ -187,7 +187,7 @@ extension _VideoPlayerWatchTogetherMethods on VideoPlayerScreenState { reason: 'watch together media switch', ); if (!mounted) return false; - if (!handled) { + if (outcome == _MediaReloadOutcome.rejected) { if (player == null) { unawaited(_replaceScreenWithPlayer(metadata)); return true; @@ -196,8 +196,8 @@ extension _VideoPlayerWatchTogetherMethods on VideoPlayerScreenState { // error; the next heartbeat re-dispatches and converges once idle. return false; } - // handled==true also covers "reload failed after rollback" and - // "superseded by a newer attempt" — trust only the committed identity. + // failed (after rollback) and superseded land here too — trust only the + // committed identity. final onTarget = _currentMetadata.id == ratingKey && _currentMetadata.serverId == serverId; if (onTarget) { // A success ends the failure episode for this key; a later failure to diff --git a/lib/screens/video_player_screen.dart b/lib/screens/video_player_screen.dart index 4060c136..37049a24 100644 --- a/lib/screens/video_player_screen.dart +++ b/lib/screens/video_player_screen.dart @@ -134,6 +134,27 @@ Future _setWakelock(bool enabled) async { /// transition is in flight. enum _PlaybackTransition { idle, reloadingMedia, restartingTranscode, switchingChannel } +/// Outcome of [VideoPlayerScreenState._reloadMediaInPlace]. +enum _MediaReloadOutcome { + /// An entry guard refused the attempt (live screen, unmounted, another + /// transition in flight). Nothing was touched; safe to retry later. + rejected, + + /// A newer playback attempt took ownership mid-reload; its outcome + /// governs what is on screen now. + superseded, + + /// The replacement media opened and its session committed. A post-open + /// step may still have failed (tracks/services were rewired in the + /// catch), but the network stream is fresh. + opened, + + /// The reload failed before the replacement opened: the previous session + /// is still committed, the eagerly-set identity was rolled back, and the + /// old (possibly dead — #1520) stream is still loaded. + failed, +} + /// Handle for one playback attempt (initial start, in-place reload, /// transcode restart). Async continuations check [isCurrent] after every /// await: it holds while the screen is mounted, the captured player is