refactor(player): return an outcome enum from _reloadMediaInPlace

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.
2026-07-10 19:06:51 +02:00
parent e6ffc54c56
commit d07d849fe4
4 changed files with 54 additions and 24 deletions
@@ -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<bool> _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
@@ -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');
    }
  }
 }
@@ -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
@@ -134,6 +134,27 @@ Future<void> _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