fix(video): close recovery gaps found in deep player review
- Expire _isQualitySwitching after a 5s grace window: a resolution-only switch never interrupts playback so no event clears the flag, which disabled stall classification (and the 8s recovery) for the rest of the session — including from session start for saved qualities - Cap latency-driven recovery: route full refreshes through the existing attempt cap + exponential backoff, and stop intervening entirely after 3 consecutive high readings (device clock skew would otherwise loop seek/refresh forever, every 3s in PiP) - Respect user pause in the 4xx and media-services-reset auto-refresh paths: defer the refresh until resume instead of force-resuming audio - Guard the _onPlayerError liveness check with _initGeneration so a slow stream-info fetch can't pin a stale error overlay on recovered playback - Confirm offline with a second /streams check before tearing down playback state — Helix transiently drops live streams from results - Capture the controller in _setStreamQualityIndex and re-check after awaits instead of _controller! (dispose race during quality switch) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
6060bbd41b
commit
949b5cb73f
3 changed files with 103 additions and 13 deletions
|
|
@ -76,6 +76,12 @@ abstract class NativeVideoStoreBase
|
|||
DateTime? _lastPlayingStart;
|
||||
int _shortPlayBounceCount = 0;
|
||||
var _isQualitySwitching = false;
|
||||
Timer? _qualitySwitchGraceTimer;
|
||||
|
||||
/// Set when an automatic refresh was suppressed because the user had
|
||||
/// paused (e.g. the playback session expired mid-pause). Consumed by
|
||||
/// [handlePausePlay] to refresh instead of playing a dead session.
|
||||
var _pendingRefreshOnResume = false;
|
||||
var _initGeneration = 0;
|
||||
static final _bareResolutionRe = RegExp(r'^(\d+)p$');
|
||||
|
||||
|
|
@ -536,6 +542,12 @@ abstract class NativeVideoStoreBase
|
|||
statusCode >= 400 &&
|
||||
statusCode < 500 &&
|
||||
!_isWithinRecoveryCap()) {
|
||||
if (_userPaused) {
|
||||
// Don't yank a user-paused stream back to life — the refresh would
|
||||
// reload with autoplay. Defer until they resume.
|
||||
_pendingRefreshOnResume = true;
|
||||
return;
|
||||
}
|
||||
debugPrint(
|
||||
'NativeVideoStore: $statusCode on HLS fetch, refreshing with fresh '
|
||||
'token (attempt ${_totalRefreshAttempts + 1}/${VideoTimingConstants.maxRefreshAttempts})',
|
||||
|
|
@ -547,8 +559,9 @@ abstract class NativeVideoStoreBase
|
|||
|
||||
// Check stream liveness before showing the error — an offline channel
|
||||
// is expected to 404 and the overlay handles it.
|
||||
final generation = _initGeneration;
|
||||
updateStreamInfo(forceUpdate: true).then((_) {
|
||||
if (_disposed) return;
|
||||
if (_disposed || generation != _initGeneration) return;
|
||||
runInAction(() {
|
||||
if (_streamInfo != null) {
|
||||
_recordNativePlaybackError(
|
||||
|
|
@ -603,6 +616,14 @@ abstract class NativeVideoStoreBase
|
|||
return;
|
||||
}
|
||||
|
||||
if (_userPaused) {
|
||||
// The player is orphaned either way, but don't auto-resume a stream
|
||||
// the user paused — rebuild when they tap play.
|
||||
_disposeController();
|
||||
_pendingRefreshOnResume = true;
|
||||
return;
|
||||
}
|
||||
|
||||
_totalRefreshAttempts++;
|
||||
debugPrint(
|
||||
'NativeVideoStore: media services reset, rebuilding player '
|
||||
|
|
@ -795,6 +816,15 @@ abstract class NativeVideoStoreBase
|
|||
!_loading &&
|
||||
!_userPaused) {
|
||||
_highLatencyCount++;
|
||||
// Still high after several interventions — the reading can't be
|
||||
// trusted (device clock skew) or the device genuinely can't keep up.
|
||||
// Either way, looping seeks/refreshes forever makes playback worse.
|
||||
// The counter survives refreshes and only resets on a healthy
|
||||
// reading, so a persistent condition stops recovery permanently.
|
||||
if (_highLatencyCount >
|
||||
VideoTimingConstants.maxHighLatencyRecoveries) {
|
||||
return;
|
||||
}
|
||||
if (_isInPipMode || _highLatencyCount == 1) {
|
||||
// During PiP, only seek to live edge — handleRefresh() disposes
|
||||
// the controller which kills PiP.
|
||||
|
|
@ -803,13 +833,16 @@ abstract class NativeVideoStoreBase
|
|||
);
|
||||
_controller?.seekToLiveEdge();
|
||||
_controller?.play();
|
||||
} else {
|
||||
} else if (!_isWithinRecoveryCap() &&
|
||||
_backoffDelayRemaining() == Duration.zero) {
|
||||
debugPrint(
|
||||
'NativeVideoStore: latency still high (${rounded}s), refreshing',
|
||||
'NativeVideoStore: latency still high (${rounded}s), refreshing '
|
||||
'(attempt ${_totalRefreshAttempts + 1}/${VideoTimingConstants.maxRefreshAttempts})',
|
||||
);
|
||||
_highLatencyCount = 0;
|
||||
_totalRefreshAttempts++;
|
||||
_handleRefresh(userInitiated: false);
|
||||
}
|
||||
// Within the refresh cap/backoff window — wait for the next poll.
|
||||
return;
|
||||
}
|
||||
_highLatencyCount = 0;
|
||||
|
|
@ -867,6 +900,13 @@ abstract class NativeVideoStoreBase
|
|||
|
||||
@override
|
||||
void handlePausePlay() {
|
||||
if (_pendingRefreshOnResume) {
|
||||
// The playback session died while user-paused (token expiry, media
|
||||
// services reset) — playing the dead session would no-op or error.
|
||||
_pendingRefreshOnResume = false;
|
||||
_handleRefresh(userInitiated: false);
|
||||
return;
|
||||
}
|
||||
if (_controller == null) return;
|
||||
if (_paused) {
|
||||
_userPaused = false;
|
||||
|
|
@ -913,8 +953,11 @@ abstract class NativeVideoStoreBase
|
|||
_isStalled = false;
|
||||
_hasPlayedOnce = false;
|
||||
_userPaused = false;
|
||||
_pendingRefreshOnResume = false;
|
||||
_stallRecoveryAttempt = 0;
|
||||
_highLatencyCount = 0;
|
||||
// _highLatencyCount intentionally survives refreshes so the
|
||||
// latency-recovery cap can stop a persistent loop; it resets on the
|
||||
// next healthy latency reading.
|
||||
_firstTimeSettingQuality = true;
|
||||
_pendingQualityIndex = null;
|
||||
_isQualitySwitching = false;
|
||||
|
|
@ -1009,20 +1052,36 @@ abstract class NativeVideoStoreBase
|
|||
@action
|
||||
Future<void> _setStreamQualityIndex(int index) async {
|
||||
_streamQualityIndex = index;
|
||||
if (_controller == null) return;
|
||||
final controller = _controller;
|
||||
if (controller == null) return;
|
||||
|
||||
_isQualitySwitching = true;
|
||||
// A resolution-only switch never interrupts playback, so no play/pause
|
||||
// event arrives to clear the flag — expire it after a grace window or
|
||||
// it sticks for the session and disables stall classification.
|
||||
_qualitySwitchGraceTimer?.cancel();
|
||||
_qualitySwitchGraceTimer = Timer(
|
||||
VideoTimingConstants.qualitySwitchGrace,
|
||||
() {
|
||||
if (_disposed) return;
|
||||
runInAction(() => _isQualitySwitching = false);
|
||||
},
|
||||
);
|
||||
|
||||
if (index == 0) {
|
||||
// 'Auto' — reset to adaptive quality
|
||||
await _controller!.setQuality(NativeVideoPlayerQuality.auto());
|
||||
await controller.setQuality(NativeVideoPlayerQuality.auto());
|
||||
} else {
|
||||
final qualityIndex = index - 1;
|
||||
if (qualityIndex >= 0 && qualityIndex < _qualityObjects.length) {
|
||||
await _controller!.setQuality(_qualityObjects[qualityIndex]);
|
||||
await controller.setQuality(_qualityObjects[qualityIndex]);
|
||||
}
|
||||
}
|
||||
|
||||
// The controller can be disposed or replaced during the platform call
|
||||
// (user backs out, chat-only toggle, media services reset).
|
||||
if (_disposed || !identical(controller, _controller)) return;
|
||||
|
||||
// Toggle background PiP based on audio-only mode.
|
||||
// Only call when the mode actually changes to avoid unnecessary
|
||||
// method channel round-trips on every quality switch.
|
||||
|
|
@ -1035,9 +1094,9 @@ abstract class NativeVideoStoreBase
|
|||
_isAudioOnlyMode = isAudioOnly;
|
||||
if (Platform.isIOS) {
|
||||
if (isAudioOnly) {
|
||||
_controller!.disableAutomaticInlinePip();
|
||||
controller.disableAutomaticInlinePip();
|
||||
} else {
|
||||
_controller!.enableAutomaticInlinePip();
|
||||
controller.enableAutomaticInlinePip();
|
||||
}
|
||||
} else if (Platform.isAndroid && _autoPipAvailable == true) {
|
||||
if (isAudioOnly) {
|
||||
|
|
@ -1139,6 +1198,7 @@ abstract class NativeVideoStoreBase
|
|||
_stallRecoveryTimer?.cancel();
|
||||
_initRetryTimer?.cancel();
|
||||
_healthyPlaybackTimer?.cancel();
|
||||
_qualitySwitchGraceTimer?.cancel();
|
||||
_pipSub?.cancel();
|
||||
_pipSub = null;
|
||||
_qualitiesSub?.cancel();
|
||||
|
|
@ -1157,6 +1217,7 @@ abstract class NativeVideoStoreBase
|
|||
void _resetPlayerStateCommon() {
|
||||
_hasPlayedOnce = false;
|
||||
_userPaused = false;
|
||||
_pendingRefreshOnResume = false;
|
||||
_paused = true;
|
||||
_isStalled = false;
|
||||
_isQualitySwitching = false;
|
||||
|
|
|
|||
|
|
@ -63,11 +63,23 @@ class StreamInfoPoller {
|
|||
final info = await twitchApi.getStream(userLogin: userLogin);
|
||||
return StreamInfoResult(stream: info);
|
||||
} catch (_) {
|
||||
// Helix can transiently drop a live stream from /streams (or 500 on
|
||||
// one endpoint while others succeed). A false "confirmed offline"
|
||||
// tears down latency polling and chat delay on healthy playback, so
|
||||
// require a second failed check before falling back to /channels.
|
||||
try {
|
||||
final channel = await twitchApi.getChannel(userId: userId);
|
||||
return StreamInfoResult(offlineChannel: channel);
|
||||
await Future<void>.delayed(
|
||||
VideoTimingConstants.offlineConfirmationDelay,
|
||||
);
|
||||
final info = await twitchApi.getStream(userLogin: userLogin);
|
||||
return StreamInfoResult(stream: info);
|
||||
} catch (_) {
|
||||
return const StreamInfoResult();
|
||||
try {
|
||||
final channel = await twitchApi.getChannel(userId: userId);
|
||||
return StreamInfoResult(offlineChannel: channel);
|
||||
} catch (_) {
|
||||
return const StreamInfoResult();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -28,6 +28,23 @@ class VideoTimingConstants {
|
|||
static const int highLatencyThresholdSeconds = 30;
|
||||
static const int chatSyncDriftToleranceSeconds = 2;
|
||||
|
||||
/// Maximum consecutive high-latency interventions (seek/refresh) before
|
||||
/// giving up on latency-driven recovery. Persistent high readings usually
|
||||
/// mean device clock skew or a device that can't keep up — looping
|
||||
/// recoveries forever makes playback worse, not better.
|
||||
static const int maxHighLatencyRecoveries = 3;
|
||||
|
||||
/// How long a quality switch may suppress stall classification before the
|
||||
/// flag expires. A resolution-only switch never interrupts playback, so no
|
||||
/// play/pause event arrives to clear it — without this backstop the flag
|
||||
/// sticks for the whole session and disables stall recovery.
|
||||
static const qualitySwitchGrace = Duration(seconds: 5);
|
||||
|
||||
/// Delay before re-checking /streams when the first check reports offline.
|
||||
/// Helix transiently drops live streams from results; a single failed
|
||||
/// check isn't enough to tear down healthy playback.
|
||||
static const offlineConfirmationDelay = Duration(seconds: 2);
|
||||
|
||||
// Overlay
|
||||
static const overlayAutoHide = Duration(seconds: 5);
|
||||
static const overlayQuickHide = Duration(seconds: 3);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue