From 4816e3928f398d0c33cb83586c52f07b3ea02383 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Thu, 6 Aug 2026 23:07:58 +0200 Subject: [PATCH] fix(player): skip relative to the position a jump landed on A coalesced key-repeat skip pins its target so a slow backend cannot make the next press rebase off a position the seek has not reached yet. Nothing retired that pin when something else moved the playhead, so for the ten seconds it survived, a skip taken after a timeline tap, a chapter jump, an OS media control or a peer sync resumed from the superseded target and threw the user back across their own jump. Publish every playhead movement on the player and retire the pin whenever the announced destination is not the accumulator's own commit. Overlapping seeks and backend-chosen relocations arbitrate by which operation the backend accepted, so a request that was merely asked for cannot speak for where the playhead ended up. close #1819 --- lib/media/stepped_seek.dart | 75 +- lib/mpv/player/player.dart | 6 + lib/mpv/player/player_base.dart | 453 ++++- lib/mpv/player/player_native.dart | 50 +- lib/mpv/player/player_stream_controllers.dart | 3 + lib/mpv/player/player_streams.dart | 26 + lib/screens/music/now_playing_screen.dart | 4 + .../music/music_playback_service.dart | 7 + .../music/music_playback_service_impl.dart | 12 +- .../desktop_video_controls.dart | 8 + .../video_controls/parts/key_events.dart | 4 +- lib/widgets/video_controls/parts/markers.dart | 2 +- .../video_controls/parts/navigation.dart | 13 +- .../video_controls/parts/playback_input.dart | 60 + .../video_controls/video_controls.dart | 25 +- test/media/stepped_seek_test.dart | 207 +++ test/mpv/player_native_bridge_test.dart | 1491 +++++++++++++++++ .../music/now_playing_screen_test.dart | 50 + .../music/music_playback_service_test.dart | 58 + ...video_controls_mobile_skip_zones_test.dart | 29 + ...ideo_controls_transient_feedback_test.dart | 252 ++- 21 files changed, 2809 insertions(+), 26 deletions(-) diff --git a/lib/media/stepped_seek.dart b/lib/media/stepped_seek.dart index 93415b5f..89cb53e9 100644 --- a/lib/media/stepped_seek.dart +++ b/lib/media/stepped_seek.dart @@ -13,22 +13,55 @@ double steppedSeekMultiplier(int repeatCount) { /// The pending target remains pinned until playback reaches it (or the settle /// ceiling expires), so a slow seek cannot make the next burst rebase from a /// stale player position. +/// +/// The pin is dropped as soon as something else lays claim to the playhead — +/// feed [playheadJumps] every discontinuity the player announces, and any +/// target that is not this accumulator's own commit retires it, so the next +/// step rebases from where the user actually is rather than from a superseded +/// skip (#1819). Those announcements are request-time intent, so the pin is +/// retired as soon as another source asks, not once the backend has obeyed. class DebouncedSeekAccumulator { DebouncedSeekAccumulator({ required this.currentPosition, required this.duration, required this.seek, this.onChanged, + this.onBurstAbandoned, + Stream? playheadJumps, this.debounce = const Duration(milliseconds: 800), this.settlePoll = const Duration(seconds: 2), this.settleTolerance = const Duration(seconds: 3), this.settleCeiling = const Duration(seconds: 10), - }); + }) { + _jumpSubscription = playheadJumps?.listen(observePlayheadJump); + } + + /// How far a reported jump may land from the pinned target and still count as + /// this accumulator's own commit coming back around. Wide enough to absorb + /// re-clamping on the way to the backend, narrow enough that a real jump + /// elsewhere always retires the pin — and a foreign jump landing inside it is + /// indistinguishable from the pinned target anyway. + static const Duration ownJumpTolerance = Duration(milliseconds: 250); final Duration Function() currentPosition; final Duration Function() duration; final void Function(Duration target) seek; + + /// Fires whenever the pending target changes, including when it is retired. + /// This is a repaint ping for previews, not a seek notification. final void Function()? onChanged; + + /// Fires whenever a pending target is dropped instead of being played out — + /// a foreign playhead jump, a player swap, or the owner cancelling outright. + /// Callers use this to drop state that described that burst, such as a + /// running skip total. + /// + /// The target may already have been committed: a burst stays pinned after its + /// seek is dispatched, and something else superseding it then still leaves + /// that description stale. Only the natural settle, where playback reaches + /// the target, retires a pin without firing this. + final void Function()? onBurstAbandoned; + final Duration debounce; final Duration settlePoll; final Duration settleTolerance; @@ -36,12 +69,46 @@ class DebouncedSeekAccumulator { Duration? _pendingPosition; Duration? _lastFlushedPosition; + Duration? _ownSeekTarget; Timer? _debounceTimer; Timer? _settleTimer; + StreamSubscription? _jumpSubscription; bool _disposed = false; Duration? get pendingPosition => _pendingPosition; + /// Retire the pinned target when the playhead lands somewhere this + /// accumulator did not put it. A null [target] is a jump whose destination + /// the backend kept to itself, which is always foreign by definition. + /// + /// Position alone cannot decide this: a seek writes its target optimistically + /// and stale backend ticks then report the pre-seek position again, so an + /// unapplied slow seek looks exactly like a jump elsewhere. + void observePlayheadJump(Duration? target) { + if (_disposed) return; + final own = _ownSeekTarget; + if (target != null && own != null && (target - own).abs() <= ownJumpTolerance) { + // The commit this accumulator just issued, echoed back. Matching on the + // target rather than on arrival order keeps the pin alive even if a + // backend reports it late. + _ownSeekTarget = null; + return; + } + if (_pendingPosition == null) return; + cancel(); + } + + /// Re-point at another player's jump stream after the owner swapped players. + /// + /// The pending target described the old player's timeline and its echo will + /// never arrive now, so it is retired rather than carried across. + void attachPlayheadJumps(Stream? playheadJumps) { + if (_disposed) return; + unawaited(_jumpSubscription?.cancel()); + _jumpSubscription = playheadJumps?.listen(observePlayheadJump); + cancel(); + } + void seekBy(Duration delta) { if (_disposed) return; final maximum = duration(); @@ -69,6 +136,7 @@ class DebouncedSeekAccumulator { final target = _pendingPosition; if (target == null || target == _lastFlushedPosition) return; _lastFlushedPosition = target; + _ownSeekTarget = target; seek(target); _scheduleClear(target); } @@ -92,20 +160,25 @@ class DebouncedSeekAccumulator { _settleTimer = Timer(settlePoll, poll); } + /// Drop the pending burst without committing it. void cancel() { _debounceTimer?.cancel(); _debounceTimer = null; _settleTimer?.cancel(); _settleTimer = null; _lastFlushedPosition = null; + _ownSeekTarget = null; if (_pendingPosition != null) { _pendingPosition = null; onChanged?.call(); + onBurstAbandoned?.call(); } } void dispose() { _disposed = true; + unawaited(_jumpSubscription?.cancel()); + _jumpSubscription = null; _debounceTimer?.cancel(); _settleTimer?.cancel(); } diff --git a/lib/mpv/player/player.dart b/lib/mpv/player/player.dart index 0df5bb1c..a07fed4c 100644 --- a/lib/mpv/player/player.dart +++ b/lib/mpv/player/player.dart @@ -54,6 +54,12 @@ abstract class Player { /// ExoPlayer's native tick is itself 250ms, which bounds freshness there. Duration get currentPosition; + /// Where the source that just handed over was when it did, or null if none + /// has. A gapless advance retargets [state] and [currentPosition] at the new + /// source immediately, so anything finalising the outgoing item — progress + /// reporting, scrobbling — must read its last position from here. + Duration? get outgoingSourcePosition => null; + /// Whether audio passthrough (bitstream output) is currently active. /// /// [setRate] with a non-1.0 rate tears passthrough down, so callers that diff --git a/lib/mpv/player/player_base.dart b/lib/mpv/player/player_base.dart index a7f97c73..b4b8f6c1 100644 --- a/lib/mpv/player/player_base.dart +++ b/lib/mpv/player/player_base.dart @@ -70,6 +70,103 @@ abstract class PlayerBase with PlayerStreamControllersMixin implements Player { int _lastEmitMs = 0; int _lastCacheStateMs = 0; int _positionMs = 0; + + /// Overlapping-seek bookkeeping. Each [runSeek] optimistically writes its own + /// target, so only the last of a group to settle can tell where the backend + /// actually left the playhead — and because the backend applies commands in + /// the order they were issued, "which one landed" is decided by request + /// order, not by which reply came back first. + /// + /// A playhead move from outside [runSeek] detaches the active group: it is + /// newer information than any of that group's outcomes, and seeks starting + /// after it belong to a fresh group anchored where it left the playhead. + /// Issue-time id for anything that asks the playhead to move — a seek + /// request, a relocation claim, a source install. Monotonic, so it also + /// orders [runSeek] calls inside a group. + int _playheadOperations = 0; + + /// The newest operation the backend actually accepted. A completion may only + /// speak for the playhead while nothing newer has been accepted; asking is + /// not owning, so a rejected request never silences an accepted one. + int _acceptedOperation = 0; + + /// A relocation whose destination arrived while a newer seek was still in + /// flight. Held rather than published: that seek will define the playhead if + /// it lands, and this is still the truth if it does not. + ({int token, Duration? position})? _deferredRelocation; + + /// Which operation last wrote a position into state. Compared by identity, + /// not by value: a successor seeking to the same timestamp is still a + /// different write and must not be repaired away by its predecessor. + int _lastPositionWriter = 0; + + /// Stands in for the backend in [_lastPositionWriter]: a reported position is + /// authoritative and beats anything Dart wrote optimistically, whatever its + /// value happens to be. + static const int _backendReportedWriter = -1; + + _SeekGroup? _activeSeekGroup; + + @override + Duration? get outgoingSourcePosition => _outgoingSourcePosition; + Duration? _outgoingSourcePosition; + + /// The last position the backend reported *for the source now playing*, as + /// distinct from [_positionMs], which also carries Dart's optimistic writes. + /// Only an observation can say where a source actually got to, and an + /// observation of one source says nothing about the next — so installing a + /// source clears this rather than letting it carry over. + int _lastReportedPositionMs = 0; + + /// Set by [freezeOutgoingSourcePosition] when a source boundary is seen on + /// the same flow that carries position reports. Preferred by + /// [takeSourceOwnership], which cannot make that ordering guarantee itself. + int? _frozenOutgoingPositionMs; + + /// Wall-clock slack added to a seek's flight time before converting to media + /// time, absorbing tick granularity and clock jitter. + static const _landedProgressSlack = Duration(milliseconds: 250); + + /// Claim the playhead for a relocation whose destination the backend has not + /// reported yet, and take the token that says so. + /// + /// Claiming is what makes two overlapping relocations distinguishable: + /// reading a shared revision would let them both think they still speak for + /// the playhead. + /// + /// Claiming alone changes nothing else. A command that is then rejected never + /// moved the playhead, so an in-flight seek group stays authoritative and + /// reconciles its own outcome. A command that is accepted hands the token to + /// [commitPlayheadRelocation], which takes ownership even when the + /// destination cannot be read back, and then to [publishPlayheadRelocation] + /// when there is a destination to report. + @protected + int beginPlayheadRelocation() => ++_playheadOperations; + + /// Record that a claimed relocation actually moved the playhead, even though + /// its destination could not be read back. + /// + /// Unknown movement is still movement: an in-flight seek group settling + /// afterwards must not roll back across it, so ownership passes here while + /// the position itself waits for the backend's next tick. + @protected + void commitPlayheadRelocation(int token) { + if (_disposed || token < _acceptedOperation) return; + _takeOperationOwnership(token); + // A newer seek is still in flight: keep its group so it can arbitrate + // against this relocation when it resolves. Recorded without a destination, + // because an accepted command moved the playhead whether or not its + // position can be read — the group must not roll back across it. + if (_hasUnresolvedSeekNewerThan(token)) { + _deferredRelocation = (token: token, position: null); + return; + } + // The claim already made this token the owner; taking the playhead from the + // seek group is all that is left. The token stays valid so the same + // relocation can still publish a destination once it reads one back. + _activeSeekGroup = null; + } + Duration? _timelineDuration; int _nextPropId = 0; final Map _propIdToName = {}; @@ -246,6 +343,10 @@ abstract class PlayerBase with PlayerStreamControllersMixin implements Player { if (positionMs != null) { final pos = Duration(milliseconds: positionMs); _positionMs = positionMs; + // The backend has spoken, so no Dart-side optimistic write is on top + // any more — whatever the value happens to be. + _lastPositionWriter = _backendReportedWriter; + _lastReportedPositionMs = positionMs; // Only allocate PlayerState + emit at ~4Hz (250ms). The raw integer // remains current for synchronous position reads on every tick. final nowMs = _throttleSw.elapsedMilliseconds; @@ -669,13 +770,213 @@ abstract class PlayerBase with PlayerStreamControllersMixin implements Player { _timelineDuration = duration; } + /// Report that something is moving the playhead discontinuously, to [target] + /// — or somewhere only the backend knows, when [target] is null. Announced + /// when the move is requested, so it is intent rather than an observed + /// landing; see `PlayerStreams.playheadJump`. + /// + /// Guards disposal: several callers announce after an await, by which point + /// the controllers may already be closed. + @protected + void announcePlayheadJump(Duration? target) { + if (_disposed) return; + playheadJumpController.add(target); + } + + /// Publish a playhead position the backend chose for itself, after a command + /// that relocates it without going through [runSeek]. + /// + /// Writes state as well as announcing: `PlayerState.position` is what + /// consumers rebase relative seeks from, and its tick updates are throttled, + /// so announcing alone would leave them working off the pre-command position. + /// + /// Arbitration is by acceptance, not by request: a seek that has only been + /// asked for does not invalidate a relocation the backend already took, and a + /// destination arriving while a newer seek is still unresolved is held rather + /// than published — publishing would read as a foreign jump to whoever is + /// coalescing that seek, and discarding would lose the cue if it is rejected. + @protected + void publishPlayheadRelocation(Duration position, {int? token}) { + if (_disposed) return; + // Something newer has claimed or moved the playhead since this relocation + // started, so its answer is stale. + if (token != null && token < _acceptedOperation) return; + // A newer seek is still in flight, so it — not this answer about the past — + // will define where the playhead ends up if it lands. Publishing now would + // read as a foreign jump to whoever is coalescing that seek and cost them + // the burst; discarding would lose the cue if that seek is then rejected. + // Hold it until the group resolves. + if (token != null && _hasUnresolvedSeekNewerThan(token)) { + _deferredRelocation = (token: token, position: position); + return; + } + final writer = token ?? ++_playheadOperations; + _takeOperationOwnership(writer); + _lastPositionWriter = writer; + _activeSeekGroup = null; + _setPlaybackPosition(position); + announcePlayheadJump(position); + } + + /// Where the playhead is, given [target] was accepted [since] ago. + /// + /// A reported position at or shortly past the target is playback running on + /// from it and is fresher than the target itself; anything else is a stale + /// observation from before the seek. "Shortly" is the media time the elapsed + /// wall clock could actually cover at the current rate — a fixed window would + /// rewind real progress at 8x and preserve stale ticks in slow motion — and a + /// paused player covers none at all. + Duration _progressedFrom(Duration target, Stopwatch? since) { + // Only the backend observes playback. `_positionMs` also carries Dart's own + // optimistic writes, and a rejected request's target sitting a few + // milliseconds past an accepted one is not progress — reading it as such + // would keep the position the backend refused. + if (_lastPositionWriter != _backendReportedWriter) return target; + final observed = Duration(milliseconds: _positionMs); + final drift = observed - target; + if (drift.isNegative) return target; + final rate = _state.rate.isFinite && _state.rate > 0 ? _state.rate : 1.0; + final elapsed = since?.elapsedMicroseconds ?? 0; + final covered = _state.playing + ? Duration(microseconds: ((elapsed + _landedProgressSlack.inMicroseconds) * rate).round()) + : Duration.zero; + return drift <= covered ? observed : target; + } + + /// Mark [operation] as the newest thing the backend accepted, retiring any + /// relocation that was waiting to see whether it would land. + void _takeOperationOwnership(int operation) { + _acceptedOperation = operation; + if ((_deferredRelocation?.token ?? operation) < operation) _deferredRelocation = null; + } + + /// Settle a held relocation now that the group arbitrating it has drained. + /// + /// It wins when it is newer than anything that group landed; otherwise the + /// group's own outcome stands and the cue is history either way. + bool _resolveDeferredRelocation(_SeekGroup group) { + final deferred = _deferredRelocation; + _deferredRelocation = null; + if (deferred == null || _disposed || deferred.token <= group.landedRequest) return false; + _takeOperationOwnership(deferred.token); + // Winning without a destination still means the group must not undo itself + // across this relocation; there is simply nothing new to publish, and the + // backend's next tick supplies the position. + final position = deferred.position; + if (position != null) { + _setPlaybackPosition(position); + announcePlayheadJump(position); + return true; + } + // Destination unknown, and suppressing the group's rollback would leave a + // rejected request's optimistic target on top of state — certainly not + // where an accepted relocation put the playhead. + _repairRejectedWrite(group); + // The null this relocation sent before dispatch predates the seek it was + // waiting on, so that seek's own echo has since re-armed any coalescing + // consumer. Say again that the playhead is somewhere they did not put it. + announcePlayheadJump(null); + return true; + } + + /// Record where the playing source got to, before its successor can report + /// anything. + /// + /// [takeSourceOwnership] runs from the backend's event flow, which is not + /// ordered against the property flow carrying position reports, so by then an + /// incoming report may already have replaced the outgoing one. Callers that + /// observe the boundary *on the property flow* can close that window by + /// calling this; whoever finalises the outgoing item then gets its real last + /// position instead of its successor's first. + @protected + void freezeOutgoingSourcePosition() { + if (_disposed) return; + _frozenOutgoingPositionMs = _lastReportedPositionMs; + } + + /// The handover this froze a position for is not going to happen. Drop it, or + /// a later advance whose boundary edge is dropped would prefer this snapshot + /// over the position actually reported since. + @protected + void discardFrozenOutgoingPosition() => _frozenOutgoingPositionMs = null; + + /// The backend rolled into a different source on its own — a gapless + /// advance. Nothing that was in flight against the old one may speak for the + /// playhead any more. + @protected + void takeSourceOwnership() { + if (_disposed) return; + // Recorded before anything below overwrites it: whoever finalises the + // outgoing item needs where it got to, and every position reachable from + // here on belongs to the new source. + _outgoingSourcePosition = Duration(milliseconds: _frozenOutgoingPositionMs ?? _lastReportedPositionMs); + _frozenOutgoingPositionMs = null; + _lastReportedPositionMs = 0; + _takeOperationOwnership(++_playheadOperations); + _activeSeekGroup = null; + // A gapless advance starts the new source at its beginning, which is known + // rather than guessed. Published unconditionally: the alternative is to + // preserve whatever position was last reported, and at this boundary that + // is far more likely to be the outgoing track's last tick than an early + // one from the incoming track. If an early new-source tick really did + // arrive, its successor corrects this within one tick. + _lastPositionWriter = _playheadOperations; + _setPlaybackPosition(Duration.zero); + announcePlayheadJump(Duration.zero); + } + + /// Lift a rejected request's optimistic target off state, if it is still the + /// thing on top. + /// + /// What replaces it is the closest position actually known: what the backend + /// last reported, else the newest target this group had accepted, else where + /// it started. Nothing is announced — a rejected request's abandonment is + /// announced by whoever rejected it. + void _repairRejectedWrite(_SeekGroup group) { + if (_disposed) return; + // A later group is running: its optimistic write owns state, and even a + // backend report arriving now belongs inside its window, not this one's. + if (_activeSeekGroup != null && !identical(_activeSeekGroup, group)) return; + + if (_lastPositionWriter == _backendReportedWriter) { + // The backend has reported since, so it is authoritative whatever the + // value — this must be checked before anything that infers ownership from + // the value itself. `PlayerState.position` is throttled and can still be + // showing an abandoned target, so bring it into line. + final ticked = Duration(milliseconds: _positionMs); + if (_state.position != ticked) _setPlaybackPosition(ticked); + return; + } + + if (group.landedRequest == group.newestRequest) return; + // Someone else wrote since; their value stands even if it happens to match + // this group's target. + if (_lastPositionWriter != group.newestRequest) return; + _setPlaybackPosition(group.landedTarget ?? group.anchor); + } + + bool _hasUnresolvedSeekNewerThan(int token) { + // Asked of the requests still outstanding, not of the group's newest: that + // one may already have settled while an older sibling holds the group open. + final group = _activeSeekGroup; + return group != null && group.unsettled.any((request) => request > token); + } + @protected Duration? get configuredTimelineDuration => _timelineDuration; + /// Install a freshly opened source at [sourcePosition]. + /// + /// An in-place reload — dead-stream recovery, a quality/version switch, a + /// background-suspend resume — places the playhead here rather than through + /// [runSeek], so this is the second way it can move discontinuously. @protected void resetPlaybackProgress(Duration sourcePosition) { final position = sourcePosition; _positionMs = position.inMilliseconds; + // A source is being installed at this position; nothing has been reported + // about it yet, and its predecessor's position says nothing about it. + _lastReportedPositionMs = position.inMilliseconds; _state = _state.copyWith( completed: false, position: position, @@ -683,8 +984,12 @@ abstract class PlayerBase with PlayerStreamControllersMixin implements Player { buffer: Duration.zero, bufferRanges: const [], ); + _takeOperationOwnership(++_playheadOperations); + _lastPositionWriter = _playheadOperations; + _activeSeekGroup = null; completedController.add(false); positionController.add(position); + announcePlayheadJump(position); durationController.add(_timelineDuration ?? Duration.zero); bufferController.add(Duration.zero); bufferRangesController.add(const []); @@ -697,6 +1002,11 @@ abstract class PlayerBase with PlayerStreamControllersMixin implements Player { trackController.add(snapshot.track); } + /// Put back the state a failed open tore down. + /// + /// [resetPlaybackProgress] already announced the start position the open was + /// aiming for, so undoing it has to be announced too — otherwise a consumer + /// that pinned the abandoned resume target keeps building on it. @protected void restorePlaybackProgress(PlayerState snapshot, {Duration? position}) { final restoredPosition = position ?? snapshot.position; @@ -708,8 +1018,12 @@ abstract class PlayerBase with PlayerStreamControllersMixin implements Player { buffer: snapshot.buffer, bufferRanges: snapshot.bufferRanges, ); + _takeOperationOwnership(++_playheadOperations); + _lastPositionWriter = _playheadOperations; + _activeSeekGroup = null; completedController.add(snapshot.completed); positionController.add(restoredPosition); + announcePlayheadJump(restoredPosition); durationController.add(snapshot.duration); bufferController.add(snapshot.buffer); bufferRangesController.add(snapshot.bufferRanges); @@ -897,33 +1211,131 @@ abstract class PlayerBase with PlayerStreamControllersMixin implements Player { /// Run a backend-specific seek call, swallowing the common "not ready" errors /// the native channel throws when the engine was torn down mid-seek. + /// + /// Seeks can overlap, and each one optimistically writes its own target, so + /// no single call knows where the playhead really ended up. The last of an + /// overlapping group to settle owns the correction: if any of them landed the + /// backend is at that target, and if none did, nothing moved and the position + /// from before the group is the truth. @protected Future runSeek(Duration position, Future Function() seekFn) async { if (_disposed) return; - final previousPosition = Duration(milliseconds: _positionMs); + final request = ++_playheadOperations; + // Measures how much media time playback could legitimately have covered + // while the command was in flight; a fixed media-time window cannot tell a + // fast-rate advance from a stale pre-seek tick. + final elapsedInFlight = Stopwatch()..start(); + final group = _activeSeekGroup ??= _SeekGroup(Duration(milliseconds: _positionMs)); + group.unsettled.add(request); + group.newestRequest = request; + _lastPositionWriter = request; _setPlaybackPosition(position); + // Announce the request, not its completion: consumers coalescing their own + // seeks need to know the playhead moved out from under them while the + // backend is still working, which is exactly the window a late signal would + // miss. + announcePlayheadJump(position); - void rollbackPosition() { - // Avoid overwriting a newer native position update if one arrived while - // the platform seek was in flight. - if (_positionMs == position.inMilliseconds) { - _setPlaybackPosition(previousPosition); + void settle({required bool landed}) { + // A newer request in this group has already displaced the playhead + // optimistically, so nothing here can read the backend's position: record + // the exact target and let the group reconcile once that newer request + // resolves. + if (landed && request != group.newestRequest) { + if (request > group.landedRequest) { + group.landedRequest = request; + group.landedTarget = position; + group.landedSince = elapsedInFlight; + } + if (request > _acceptedOperation) _takeOperationOwnership(request); + } else if (landed) { + final authoritative = _progressedFrom(position, elapsedInFlight); + + if (request > group.landedRequest) { + // The backend applies commands in issue order, so the newest request + // it accepted is where it ends up — whichever reply came back first. + group.landedRequest = request; + group.landedTarget = authoritative; + group.landedSince = elapsedInFlight; + } + if (request > _acceptedOperation) { + // Newest thing the backend has accepted, so the playhead is here. + // Applied now rather than at group drain: the group may be detached + // by a relocation or still waiting on an older member, and neither + // changes the fact that nothing newer has been accepted. + _takeOperationOwnership(request); + if (_positionMs != authoritative.inMilliseconds || _state.position != authoritative) { + _setPlaybackPosition(authoritative); + } + } } + group.unsettled.remove(request); + if (group.unsettled.isNotEmpty) return; + if (!identical(_activeSeekGroup, group)) { + // Detached: something outside the group took the playhead after it + // started — a relocation, or a different source starting. A deferred + // relocation is deliberately left alone here — it is held against + // whichever group is active now, which may well be a later one than + // this, and resolving it from here would publish into that group's + // window or restore an anchor from a timeline it never saw. Its own + // rejected write is still its to clean up, though. + _repairRejectedWrite(group); + return; + } + _activeSeekGroup = null; + if (_disposed) return; + + if (_resolveDeferredRelocation(group)) return; + + final anchor = group.anchor; + final landedTarget = group.landedTarget; + final newestRequestFailed = group.landedRequest != group.newestRequest; + + if (landedTarget != null) { + // Something landed and already applied itself. Only a rejected newest + // request needs undoing here: its optimistic write is still on top, and + // the accepted target of a newest request went out as its own request. + if (newestRequestFailed) { + // The rejected newest request's optimistic write is still on top. + // Replace it with the accepted target, or with a tick that has since + // run on from it — rewinding real progress would be its own bug. + final settled = _progressedFrom(landedTarget, group.landedSince); + _setPlaybackPosition(settled); + announcePlayheadJump(settled); + } + return; + } + + // Every seek in the group was rejected, so the playhead never went where + // it was announced. Undo it in state and on the stream, or a consumer + // keeps building on a position the backend refused. + if (_lastPositionWriter == _backendReportedWriter) { + // The backend reported a position while the group was in flight, so the + // pre-group position is not what to restore — but `PlayerState.position` + // is throttled and can still be showing an abandoned target, so publish + // the reported value rather than leaving that on display. + final ticked = Duration(milliseconds: _positionMs); + _setPlaybackPosition(ticked); + announcePlayheadJump(ticked); + return; + } + _setPlaybackPosition(anchor); + announcePlayheadJump(anchor); } try { await seekFn(); + settle(landed: true); } on PlatformException catch (e) { + settle(landed: false); if (e.code == 'COMMAND_FAILED' || e.code == 'NOT_INITIALIZED') { - rollbackPosition(); appLogger.w('Seek failed (${e.code}), player not ready'); return; } - rollbackPosition(); rethrow; } catch (_) { - rollbackPosition(); + settle(landed: false); rethrow; } } @@ -1021,3 +1433,26 @@ abstract class PlayerBase with PlayerStreamControllersMixin implements Player { _textureId.dispose(); } } + +/// One run of overlapping [PlayerBase.runSeek] calls. +/// +/// Each seek writes its own target optimistically, so no single call knows +/// where the backend actually ended up; the last one to settle reconciles the +/// group. [anchor] is where the playhead was before the first of them started. +class _SeekGroup { + _SeekGroup(this.anchor); + + final Duration anchor; + + /// The source this group was issued against. + final Set unsettled = {}; + int newestRequest = 0; + int landedRequest = 0; + Duration? landedTarget; + + /// The winning request's own flight clock, still running. A drain uses it to + /// tell playback progressing from [landedTarget] apart from a stale + /// observation; restarting it at settlement would undercount media time the + /// backend covered while a slow command was still being answered. + Stopwatch? landedSince; +} diff --git a/lib/mpv/player/player_native.dart b/lib/mpv/player/player_native.dart index 81901fc8..aa47361d 100644 --- a/lib/mpv/player/player_native.dart +++ b/lib/mpv/player/player_native.dart @@ -479,6 +479,10 @@ class PlayerNative extends PlayerBase { return; } + // The handover is off: the entry is not playing and is about to be removed. + // Anything frozen for it would otherwise outlive the arm and be preferred + // by a later advance whose own boundary edge went missing. + discardFrozenOutgoingPosition(); appLogger.d('MPV-audio: clearing armed entry (playlist-remove 1)'); try { if (duringDispose) { @@ -511,6 +515,9 @@ class PlayerNative extends PlayerBase { /// arm — the fd (if any) was consumed by mpv — remove the spent entry so /// the playing entry rebases to index 0, and surface the transition. void _completeArmedAdvance(String? uri) { + // A different source is playing now, so a seek still in flight against the + // old one must not land its target on this one's timeline (#1819). + takeSourceOwnership(); _hasArmedNext = false; _armedNextUri = null; _armedNextFd = null; @@ -530,7 +537,11 @@ class PlayerNative extends PlayerBase { @override void handlePropertyChange(String name, dynamic value) { if (audioOnly && name == 'playlist-pos') { - // Debug aid only — see _handleAudioFileLoaded for the real detection. + // Detection still belongs to _handleAudioFileLoaded, but this is the last + // point ordered ahead of the new source's own position reports: they ride + // this same property flow, while `file-loaded` rides the event flow. Take + // the outgoing track's final position while it is still the current one. + if (_hasArmedNext) freezeOutgoingSourcePosition(); appLogger.d('MPV-audio: playlist-pos=$value (armed=$_hasArmedNext)'); return; } @@ -729,11 +740,48 @@ class PlayerNative extends PlayerBase { return Map.from(result ?? const {}); } + /// mpv commands that relocate the playhead while computing their own + /// destination, so Dart never learns where it landed up front (#1819). + static const _playheadRelocatingCommands = {'sub-seek'}; + @override Future command(List args) async { if (_nativeCoreUnavailable) return; await _ensureInitialized(); + // Re-checked after the await: initialization can fail, or the core can be + // torn down, while this call is suspended. Announcing a jump that no + // command will follow would retire a consumer's pending target for nothing. + if (_nativeCoreUnavailable) return; + if (args.isEmpty || !_playheadRelocatingCommands.contains(args.first)) { + await invoke('command', {'args': args}); + return; + } + + // Claimed before the announcement so two overlapping subtitle seeks, or a + // seek issued while this one runs, cannot both think they own the playhead. + final token = beginPlayheadRelocation(); + // Announced before dispatch for the same reason runSeek announces its + // request: the stale window is the round trip, not what follows it. The + // destination is unknown at this point, hence null. + announcePlayheadJump(null); await invoke('command', {'args': args}); + // The command was accepted, so the playhead has moved even if the read + // below cannot say where. Take ownership now: an in-flight seek group + // settling afterwards must not roll back across a cue that happened. + commitPlayheadRelocation(token); + // Read back where mpv actually went. `PlayerState.position` is what + // relative seeks rebase from and its tick updates are throttled, so + // without this a skip pressed straight after would start from the + // pre-command position. + // + // Nothing is published when the read fails: guessing would hand a + // fabricated position to consumers as authoritative. The null announced + // before dispatch already told consumers to drop what they were holding, + // and the backend's next tick supplies the real position. + final seconds = double.tryParse(await invoke('getProperty', {'name': 'time-pos'}) ?? ''); + if (seconds != null && seconds.isFinite && !seconds.isNegative) { + publishPlayheadRelocation(Duration(milliseconds: (seconds * 1000).round()), token: token); + } } @override diff --git a/lib/mpv/player/player_stream_controllers.dart b/lib/mpv/player/player_stream_controllers.dart index c66b5b27..c065b9e7 100644 --- a/lib/mpv/player/player_stream_controllers.dart +++ b/lib/mpv/player/player_stream_controllers.dart @@ -8,6 +8,7 @@ mixin PlayerStreamControllersMixin { final completedController = StreamController.broadcast(); final bufferingController = StreamController.broadcast(); final positionController = StreamController.broadcast(); + final playheadJumpController = StreamController.broadcast(); final durationController = StreamController.broadcast(); final seekableController = StreamController.broadcast(); final bufferController = StreamController.broadcast(); @@ -34,6 +35,7 @@ mixin PlayerStreamControllersMixin { completed: completedController.stream, buffering: bufferingController.stream, position: positionController.stream, + playheadJump: playheadJumpController.stream, duration: durationController.stream, seekable: seekableController.stream, buffer: bufferController.stream, @@ -61,6 +63,7 @@ mixin PlayerStreamControllersMixin { await completedController.close(); await bufferingController.close(); await positionController.close(); + await playheadJumpController.close(); await durationController.close(); await seekableController.close(); await bufferController.close(); diff --git a/lib/mpv/player/player_streams.dart b/lib/mpv/player/player_streams.dart index 5f378439..0885d572 100644 --- a/lib/mpv/player/player_streams.dart +++ b/lib/mpv/player/player_streams.dart @@ -17,6 +17,31 @@ class PlayerStreams { /// Stream of position updates. final Stream position; + /// Emits whenever something asks the playhead to move discontinuously — every + /// seek, whatever asked for it, and every source opened at a start position + /// by an in-place reload. + /// + /// The value is where the playhead is being put, or null when the backend + /// computes its own destination (`sub-seek`) and Dart does not know it yet. + /// A null MAY be followed by a non-null event once the destination has been + /// read back — but not always: an unreadable position publishes nothing + /// rather than guessing, and the backend's next tick supplies it instead. + /// + /// These are announced at REQUEST time, not on completion, because the window + /// a consumer has to care about is exactly while the backend is still + /// working. Treat an event as intent plus a possible correction rather than + /// as an observed landing: a seek the backend rejects is usually followed by + /// a second event carrying the position it was actually left at, though a + /// request that never reached the backend at all has nothing to correct. + /// Listeners that issue seeks themselves will also see their own requests + /// here, so they must recognise their own targets rather than assume every + /// event is foreign. + /// + /// [position] cannot stand in for this: a seek writes its target there + /// optimistically, and stale backend ticks then report the pre-seek position + /// again until the seek lands, so a listener cannot tell the two apart. + final Stream playheadJump; + /// Stream of duration changes (when media is loaded). final Stream duration; @@ -101,6 +126,7 @@ class PlayerStreams { required this.audioDevices, required this.bufferRanges, required this.playbackRestart, + this.playheadJump = const Stream.empty(), this.fileLoaded = const Stream.empty(), this.fileStarted = const Stream.empty(), this.fileLoadFailed = const Stream.empty(), diff --git a/lib/screens/music/now_playing_screen.dart b/lib/screens/music/now_playing_screen.dart index a6e1e483..e2d3c090 100644 --- a/lib/screens/music/now_playing_screen.dart +++ b/lib/screens/music/now_playing_screen.dart @@ -1036,6 +1036,10 @@ class _NowPlayingSeekBarState extends State<_NowPlayingSeekBar> { unawaited(service.seek(target)); } }, + // The scrub bar cancels the pin itself, but OS media controls, a headset + // and the lock screen all seek straight through the service, and those + // have to retire it too (#1819). + playheadJumps: context.read().playheadJumpStream, onChanged: () { if (mounted) setState(() {}); }, diff --git a/lib/services/music/music_playback_service.dart b/lib/services/music/music_playback_service.dart index c992fb9b..cee4bcc3 100644 --- a/lib/services/music/music_playback_service.dart +++ b/lib/services/music/music_playback_service.dart @@ -46,6 +46,13 @@ abstract class MusicPlaybackService extends ChangeNotifier { Duration get position; Stream get positionStream; + /// Mirrors `Player.streams.playheadJump`: something is moving the playhead + /// discontinuously, to this position, or to somewhere only the backend knows + /// when null. Request-time intent, not an observed landing. + /// Consumers coalescing their own relative seeks use it to drop a pending + /// target something else superseded (#1819). + Stream get playheadJumpStream => const Stream.empty(); + /// Full queue in playback order (shuffle already applied). List get queue; diff --git a/lib/services/music/music_playback_service_impl.dart b/lib/services/music/music_playback_service_impl.dart index b3b14f67..c486557e 100644 --- a/lib/services/music/music_playback_service_impl.dart +++ b/lib/services/music/music_playback_service_impl.dart @@ -202,6 +202,7 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO bool _sleepTimerEndOfTrack = false; final StreamController _positionController = StreamController.broadcast(); + final StreamController _playheadJumpController = StreamController.broadcast(); final StreamController _errorsController = StreamController.broadcast(); // --------------------------------------------------------------------- @@ -229,6 +230,9 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO @override Stream get positionStream => _positionController.stream; + @override + Stream get playheadJumpStream => _playheadJumpController.stream; + @override List get queue => _queue.queue; @@ -618,6 +622,7 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO _playerSubs ..clear() ..add(player.streams.position.listen(_onPosition)) + ..add(player.streams.playheadJump.listen(_playheadJumpController.add)) ..add(player.streams.playing.listen(_onPlayingChanged)) ..add(player.streams.trackTransition.listen(_onTrackTransition)) ..add(player.streams.completed.listen(_onCompleted)) @@ -686,8 +691,12 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO _invalidateArmRequests(); // The finished track played out fully — report stopped at its duration. + // Without one, the player's own record of where the outgoing source got to: + // by now its live position belongs to the track that replaced it. final finishedMs = _currentTrack?.durationMs; - _finalizeCurrentTrack(positionOverride: finishedMs != null ? Duration(milliseconds: finishedMs) : null); + _finalizeCurrentTrack( + positionOverride: finishedMs != null ? Duration(milliseconds: finishedMs) : _player?.outgoingSourcePosition, + ); // Move the cursor to the armed entry: the expected natural-next when it // still matches, otherwise wherever the armed track now sits. @@ -1571,6 +1580,7 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO // Runs to completion synchronously — see the awaitStop: false contract. unawaited(_teardownPlayerAndControls(awaitStop: false)); unawaited(_positionController.close()); + unawaited(_playheadJumpController.close()); unawaited(_errorsController.close()); _volumeNotifier.dispose(); super.dispose(); diff --git a/lib/widgets/video_controls/desktop_video_controls.dart b/lib/widgets/video_controls/desktop_video_controls.dart index 163ed362..6d8e6888 100644 --- a/lib/widgets/video_controls/desktop_video_controls.dart +++ b/lib/widgets/video_controls/desktop_video_controls.dart @@ -247,15 +247,23 @@ class DesktopVideoControlsState extends State { currentPosition: () => widget.player.state.position, duration: () => widget.player.state.duration, seek: widget.onSeekEnd, + playheadJumps: widget.player.streams.playheadJump, onChanged: () { if (mounted) setState(() {}); }, ); } + /// Drop a coalesced timeline burst that will never be committed, because what + /// it was seeking through is being replaced. + void abandonPendingSeek() => _timelineSeek.cancel(); + @override void didUpdateWidget(DesktopVideoControls oldWidget) { super.didUpdateWidget(oldWidget); + if (oldWidget.player != widget.player) { + _timelineSeek.attachPlayheadJumps(widget.player.streams.playheadJump); + } if (oldWidget.chromeController != widget.chromeController) { oldWidget.chromeController?.removeListener(_onChromeControllerChanged); widget.chromeController?.addListener(_onChromeControllerChanged); diff --git a/lib/widgets/video_controls/parts/key_events.dart b/lib/widgets/video_controls/parts/key_events.dart index 4bc1d20c..e6ea5416 100644 --- a/lib/widgets/video_controls/parts/key_events.dart +++ b/lib/widgets/video_controls/parts/key_events.dart @@ -123,8 +123,8 @@ extension _PlexVideoControlsKeyEventMethods on _PlexVideoControlsState { onPlayPause: () => unawaited(_playOrPause()), onToggleShader: _toggleShader, onSkipMarker: onSkipMarker, - onNextEpisode: widget.onNext, - onPreviousEpisode: widget.onPrevious, + onNextEpisode: _abandoningBurst(widget.onNext), + onPreviousEpisode: _abandoningBurst(widget.onPrevious), onScreenshot: _showScreenshotToast, onZoomIn: widget.onZoomIn, onZoomOut: widget.onZoomOut, diff --git a/lib/widgets/video_controls/parts/markers.dart b/lib/widgets/video_controls/parts/markers.dart index 572371ac..d82a7fff 100644 --- a/lib/widgets/video_controls/parts/markers.dart +++ b/lib/widgets/video_controls/parts/markers.dart @@ -96,7 +96,7 @@ extension _PlexVideoControlsMarkerMethods on _PlexVideoControlsState { if (marker.isCredits && isAtEnd) { if (!skipAutoPlayCountdown && widget.onNext != null) { - widget.onNext!.call(); + _abandoningBurst(widget.onNext)!.call(); } else { // Seeking to EOF is unreliable due to position stream throttling, // so pause and defer to the parent's completion flow. diff --git a/lib/widgets/video_controls/parts/navigation.dart b/lib/widgets/video_controls/parts/navigation.dart index bb6eaa07..3db52554 100644 --- a/lib/widgets/video_controls/parts/navigation.dart +++ b/lib/widgets/video_controls/parts/navigation.dart @@ -17,8 +17,8 @@ extension _PlexVideoControlsNavigationMethods on _PlexVideoControlsState { player: widget.player, volumeController: widget.volumeController, metadata: widget.metadata, - onNext: widget.onNext, - onPrevious: widget.onPrevious, + onNext: _abandoningBurst(widget.onNext), + onPrevious: _abandoningBurst(widget.onPrevious), onPlayPause: () => unawaited(_playOrPause()), chapters: _chapters, chaptersLoaded: _chaptersLoaded, @@ -46,9 +46,9 @@ extension _PlexVideoControlsNavigationMethods on _PlexVideoControlsState { isAtLiveEdge: widget.isAtLiveEdge, streamStartEpoch: widget.streamStartEpoch, currentPositionEpoch: widget.currentPositionEpoch, - onLiveSeek: widget.onLiveSeek, + onLiveSeek: _liveSeekAbandoningBurst(widget.onLiveSeek), onLiveSeekBy: widget.onLiveSeekBy, - onJumpToLive: widget.onJumpToLive, + onJumpToLive: _abandoningBurst(widget.onJumpToLive), useDpadNavigation: useDpad, serverId: widget.metadata.serverId, showQueueTab: playbackState.isQueueActive && widget.canNavigateMediaItems, @@ -65,6 +65,11 @@ extension _PlexVideoControlsNavigationMethods on _PlexVideoControlsState { } void _onQueueItemSelected(MediaItem item) { + // Same contract as next/previous: the switch is asynchronous, so a burst + // still armed here would debounce into a seek on the outgoing item. + _hiddenSeek.cancel(); + _desktopControlsKey.currentState?.abandonPendingSeek(); + _dismissSkipFeedback(); final videoPlayerState = context.findAncestorStateOfType(); videoPlayerState?.navigateToQueueItem(item); } diff --git a/lib/widgets/video_controls/parts/playback_input.dart b/lib/widgets/video_controls/parts/playback_input.dart index adb0f243..e1580697 100644 --- a/lib/widgets/video_controls/parts/playback_input.dart +++ b/lib/widgets/video_controls/parts/playback_input.dart @@ -622,10 +622,51 @@ extension _PlexVideoControlsPlaybackInputMethods on _PlexVideoControlsState { _showSkipFeedback(isForward: isForward); } + /// Wrap an absolute live action so it takes down the badge a pending live + /// skip raised. + /// + /// Live relative skips bypass [_hiddenSeek] for the parent's epoch + /// accumulator (#1253), so the jump the reopen announces finds no pending + /// target here and nothing retires the readout. The absolute action cancels + /// that queued skip, so its promised total is no longer going anywhere. + ValueChanged? _liveSeekAbandoningBurst(ValueChanged? onLiveSeek) { + if (onLiveSeek == null) return null; + return (offset) { + _dismissSkipFeedback(); + onLiveSeek(offset); + }; + } + + /// Wrap an action that replaces what is playing — a live channel switch, the + /// next or previous item — so the badge goes with the timeline it described. + /// + /// The pending skip is cancelled by the switch itself (live keeps its offset + /// in the parent accumulator, video re-keys on the new item), but neither + /// route runs through [_hiddenSeek], so nothing else takes the readout down. + VoidCallback? _abandoningBurst(VoidCallback? action) { + if (action == null) return null; + return () { + // Cancel as well as hide: a held-arrow target stays armed across the + // asynchronous switch and would otherwise debounce into a seek on the + // outgoing player before the new item re-keys the controls. The focused + // desktop timeline coalesces into its own accumulator, so it needs the + // same treatment. + _hiddenSeek.cancel(); + _desktopControlsKey.currentState?.abandonPendingSeek(); + _dismissSkipFeedback(); + action(); + }; + } + /// Handle a completed skip-zone double tap. void _handleDoubleTapSkip({required bool isForward}) { if (!widget.canControl) return; + // This tap supersedes any burst the keyboard/D-pad left pending, and it + // shares the badge with it. Retire that burst first, or its abandonment — + // triggered by this tap's own seek — would take down the readout this tap + // is about to put up. + _hiddenSeek.cancel(); _registerSkipFeedback(isForward: isForward, seconds: _seekTimeSmall); final delta = Duration(seconds: isForward ? _seekTimeSmall : -_seekTimeSmall); @@ -642,6 +683,9 @@ extension _PlexVideoControlsPlaybackInputMethods on _PlexVideoControlsState { /// Show animated visual feedback for skip gesture void _showSkipFeedback({required bool isForward}) { + // Reads `tokens(context)` below, so a caller reaching here after disposal + // would touch a defunct element rather than merely no-op. + if (!mounted) return; // Cancel BOTH timers: a skip landing during the fade-out window must not // leave the old hide timer pending, or it kills the fresh readout and zeroes // the accumulated count mid-display. @@ -675,6 +719,22 @@ extension _PlexVideoControlsPlaybackInputMethods on _PlexVideoControlsState { }); } + /// Take the skip readout down at once, because the burst it was counting will + /// never be committed. Fading it out would keep showing a total the player is + /// not going to seek to; zeroing it without hiding would flash `0s`. + void _dismissSkipFeedback() { + _feedbackTimer?.cancel(); + _feedbackTimer = null; + _feedbackHideTimer?.cancel(); + _feedbackHideTimer = null; + if (!_showDoubleTapFeedback && _accumulatedSkipSeconds == 0) return; + _setControlsState(() { + _showDoubleTapFeedback = false; + _doubleTapFeedbackOpacity = 0.0; + _accumulatedSkipSeconds = 0; + }); + } + /// Handle tap on controls overlay - route to skip zones or toggle controls void _handleControlsOverlayTap(TapUpDetails details, Size size) { final isMobile = PlatformDetector.isMobile(context); diff --git a/lib/widgets/video_controls/video_controls.dart b/lib/widgets/video_controls/video_controls.dart index 97536b43..89c856d9 100644 --- a/lib/widgets/video_controls/video_controls.dart +++ b/lib/widgets/video_controls/video_controls.dart @@ -790,6 +790,13 @@ class _PlexVideoControlsState extends State currentPosition: () => widget.player.state.position, duration: () => widget.player.state.duration, seek: (target) => unawaited(_seekToPosition(target)), + playheadJumps: widget.player.streams.playheadJump, + // The badge is a promise about the coalesced burst. Once that burst is + // abandoned — the timeline, a chapter jump, a peer, a stream rebuilt at a + // resume position, a new item, a swapped player — the promise is void, so + // take the readout down instead of leaving a total nothing will seek to + // (#1819, keeping the #1676 badge-matches-seek invariant). + onBurstAbandoned: _dismissSkipFeedback, ); // Side effects: rotation lock + focus on nav-enable. Both fire immediately // so init wiring (orientation, focus) lives in one place. @@ -876,6 +883,9 @@ class _PlexVideoControlsState extends State super.didUpdateWidget(oldWidget); if (oldWidget.player != widget.player) { ++_subtitleVisibilityWriteGeneration; + // Otherwise the accumulator keeps listening to the retired player and + // never hears the new one move. + _hiddenSeek.attachPlayheadJumps(widget.player.streams.playheadJump); } if (oldWidget.chromeController != widget.chromeController) { oldWidget.chromeController.removeListener(_onChromeChanged); @@ -888,6 +898,15 @@ class _PlexVideoControlsState extends State // the per-item chapters/markers/skip state when the item changes. // (Quality/version switches keep the same item, so no refetch churn.) if (oldWidget.metadata.globalKey != widget.metadata.globalKey) { + // A pending skip is an offset into the outgoing item's timeline; letting + // it survive would rebase the next press onto the previous episode. This + // is the lifecycle backstop for every route that replaces the item + // without going through a wrapped next/previous callback — natural + // completion, a peer, an external change — so it also has to reach the + // desktop timeline's own accumulator and the shared readout. + _hiddenSeek.cancel(); + _desktopControlsKey.currentState?.abandonPendingSeek(); + _dismissSkipFeedback(); _setControlsState(() { _chapters = []; _chaptersLoaded = false; @@ -1187,8 +1206,8 @@ class _PlexVideoControlsState extends State onCancelAutoHide: widget.chromeController.cancelAutoHide, onStartAutoHide: widget.chromeController.startAutoHide, onBack: widget.onBack, - onNext: widget.onNext, - onPrevious: widget.onPrevious, + onNext: _abandoningBurst(widget.onNext), + onPrevious: _abandoningBurst(widget.onPrevious), canControl: widget.canControl, hasFirstFrame: widget.hasFirstFrame, thumbnailDataBuilder: widget.thumbnailDataBuilder, @@ -1197,7 +1216,7 @@ class _PlexVideoControlsState extends State captureBuffer: widget.captureBuffer, isAtLiveEdge: widget.isAtLiveEdge, streamStartEpoch: widget.streamStartEpoch, - onLiveSeek: widget.onLiveSeek, + onLiveSeek: _liveSeekAbandoningBurst(widget.onLiveSeek), serverId: widget.metadata.serverId, showQueueTab: canShowQueue, onQueueItemSelected: canShowQueue ? _onQueueItemSelected : null, diff --git a/test/media/stepped_seek_test.dart b/test/media/stepped_seek_test.dart index 1e960fe3..84d7e4dc 100644 --- a/test/media/stepped_seek_test.dart +++ b/test/media/stepped_seek_test.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:fake_async/fake_async.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:plezy/media/stepped_seek.dart'; @@ -64,4 +66,209 @@ void main() { accumulator.dispose(); }); }); + + group('foreign seeks', () { + test('a seek from elsewhere retires the pin so the next step rebases', () { + fakeAsync((async) { + var position = const Duration(seconds: 20); + final seeks = []; + var abandonments = 0; + final playerJumps = StreamController.broadcast(); + final accumulator = DebouncedSeekAccumulator( + currentPosition: () => position, + duration: () => const Duration(minutes: 10), + seek: seeks.add, + playheadJumps: playerJumps.stream, + onBurstAbandoned: () => abandonments++, + ); + + accumulator.seekBy(const Duration(seconds: 10)); + accumulator.flush(); + expect(seeks, [const Duration(seconds: 30)]); + // The accumulator's own commit, echoed back by the player. + position = const Duration(seconds: 30); + playerJumps.add(const Duration(seconds: 30)); + async.flushMicrotasks(); + expect(accumulator.pendingPosition, const Duration(seconds: 30)); + expect(abandonments, 0, reason: 'the accumulator must not mistake its own seek for a foreign one'); + + // The user drops the playhead somewhere else with the timeline. + position = const Duration(minutes: 5); + playerJumps.add(const Duration(minutes: 5)); + async.flushMicrotasks(); + expect(accumulator.pendingPosition, isNull); + expect(abandonments, 1); + + accumulator.seekBy(const Duration(seconds: 10)); + accumulator.flush(); + expect( + seeks.last, + const Duration(minutes: 5, seconds: 10), + reason: 'the skip must be relative to the timeline jump, not to the superseded target', + ); + + accumulator.dispose(); + playerJumps.close(); + }); + }); + + test('a seek from elsewhere mid-burst drops the undispatched target', () { + fakeAsync((async) { + var position = const Duration(seconds: 20); + final seeks = []; + final playerJumps = StreamController.broadcast(); + final accumulator = DebouncedSeekAccumulator( + currentPosition: () => position, + duration: () => const Duration(minutes: 10), + seek: seeks.add, + playheadJumps: playerJumps.stream, + ); + + accumulator.seekBy(const Duration(seconds: 10)); + position = const Duration(minutes: 5); + playerJumps.add(const Duration(minutes: 5)); + async.flushMicrotasks(); + + async.elapse(const Duration(seconds: 2)); + expect(seeks, isEmpty, reason: 'the debounce belonged to a burst the foreign seek abandoned'); + expect(accumulator.pendingPosition, isNull); + + accumulator.dispose(); + playerJumps.close(); + }); + }); + + test('onBurstAbandoned stays out of the ordinary step and settle paths', () { + fakeAsync((async) { + var position = const Duration(seconds: 20); + var abandonments = 0; + var changes = 0; + final playerJumps = StreamController.broadcast(); + final accumulator = DebouncedSeekAccumulator( + currentPosition: () => position, + duration: () => const Duration(minutes: 10), + seek: (_) {}, + playheadJumps: playerJumps.stream, + onChanged: () => changes++, + onBurstAbandoned: () => abandonments++, + ); + + // Repeated presses advance the pending target: previews repaint, but no + // press is a foreign seek. Reporting one here would reset the caller's + // running skip total on every repeat. + accumulator.seekBy(const Duration(seconds: 10)); + accumulator.seekBy(const Duration(seconds: 10)); + accumulator.seekBy(const Duration(seconds: 10)); + expect(changes, 3); + expect(abandonments, 0); + + // Nor is the natural settle once playback reaches the target. + accumulator.flush(); + position = const Duration(seconds: 50); + async.elapse(const Duration(seconds: 2)); + expect(accumulator.pendingPosition, isNull); + expect(abandonments, 0); + + accumulator.dispose(); + playerJumps.close(); + }); + }); + + test('a retired accumulator stops listening to the player', () { + fakeAsync((async) { + final playerJumps = StreamController.broadcast(); + final accumulator = DebouncedSeekAccumulator( + currentPosition: () => const Duration(seconds: 20), + duration: () => const Duration(minutes: 10), + seek: (_) {}, + playheadJumps: playerJumps.stream, + ); + + accumulator.seekBy(const Duration(seconds: 10)); + accumulator.dispose(); + expect(playerJumps.hasListener, isFalse); + + playerJumps.add(const Duration(minutes: 5)); + async.flushMicrotasks(); + playerJumps.close(); + }); + }); + + test('a jump with no reported destination is always foreign', () { + fakeAsync((async) { + final seeks = []; + var abandonments = 0; + final playerJumps = StreamController.broadcast(); + var position = const Duration(seconds: 20); + final accumulator = DebouncedSeekAccumulator( + currentPosition: () => position, + duration: () => const Duration(minutes: 10), + seek: seeks.add, + playheadJumps: playerJumps.stream, + onBurstAbandoned: () => abandonments++, + ); + + accumulator.seekBy(const Duration(seconds: 10)); + accumulator.flush(); + expect(accumulator.pendingPosition, const Duration(seconds: 30)); + + // mpv's sub-seek picks its own cue, so the destination is unknown. + position = const Duration(minutes: 2); + playerJumps.add(null); + async.flushMicrotasks(); + + expect(accumulator.pendingPosition, isNull); + expect(abandonments, 1); + + accumulator.seekBy(const Duration(seconds: 10)); + accumulator.flush(); + expect(seeks.last, const Duration(minutes: 2, seconds: 10)); + + accumulator.dispose(); + playerJumps.close(); + }); + }); + + test('rebinding to another player drops the old pin and its stream', () { + fakeAsync((async) { + var abandonments = 0; + final oldPlayer = StreamController.broadcast(); + final newPlayer = StreamController.broadcast(); + final accumulator = DebouncedSeekAccumulator( + currentPosition: () => const Duration(seconds: 20), + duration: () => const Duration(minutes: 10), + seek: (_) {}, + playheadJumps: oldPlayer.stream, + onBurstAbandoned: () => abandonments++, + ); + + accumulator.seekBy(const Duration(seconds: 10)); + accumulator.attachPlayheadJumps(newPlayer.stream); + + expect(oldPlayer.hasListener, isFalse); + expect( + accumulator.pendingPosition, + isNull, + reason: 'the pin described the retired player, whose echo will never arrive', + ); + expect(abandonments, 1, reason: 'the burst was dropped, so its running total is stale too'); + + // A late event from the retired player must not reach the accumulator. + accumulator.seekBy(const Duration(seconds: 10)); + oldPlayer.add(const Duration(minutes: 4)); + async.flushMicrotasks(); + expect(accumulator.pendingPosition, isNotNull); + expect(abandonments, 1); + + newPlayer.add(const Duration(minutes: 6)); + async.flushMicrotasks(); + expect(accumulator.pendingPosition, isNull, reason: 'the new player is what it listens to now'); + expect(abandonments, 2); + + accumulator.dispose(); + oldPlayer.close(); + newPlayer.close(); + }); + }); + }); } diff --git a/test/mpv/player_native_bridge_test.dart b/test/mpv/player_native_bridge_test.dart index 51d992ae..608db744 100644 --- a/test/mpv/player_native_bridge_test.dart +++ b/test/mpv/player_native_bridge_test.dart @@ -12,6 +12,33 @@ import 'package:plezy/services/settings_service.dart'; import '../test_helpers/mock_player_channels.dart'; import '../test_helpers/prefs.dart'; +/// Exposes the protected playhead-arbitration seams so their interleavings can +/// be driven directly, without the method channel serialising handlers. +final class _ArbitrationPlayerNative extends PlayerNative { + int begin() => beginPlayheadRelocation(); + + void commit(int token) => commitPlayheadRelocation(token); + + void adoptNewSource() => takeSourceOwnership(); + + void publish(Duration position, int token) => publishPlayheadRelocation(position, token: token); + + Future seekVia(Duration position, Future Function() seekFn) => runSeek(position, seekFn); +} + +/// Audio-only player for driving the real gapless seam: `setNext` arms an +/// entry, and a `file-loaded` the player did not ask for means mpv rolled +/// into it. +final class _AdvancingAudioPlayerNative extends PlayerNative { + _AdvancingAudioPlayerNative() : super.audio(); + + Future seekVia(Duration position, Future Function() seekFn) => runSeek(position, seekFn); + + void markInitialized() => initialized = true; + + void installSource(Duration at) => resetPlaybackProgress(at); +} + final class _InvokingPlayerNative extends PlayerNative { Future debugInvoke(String method) => invoke(method); } @@ -1134,4 +1161,1468 @@ void main() { }); }); } + + group('playhead jump announcements', () { + test('a seek publishes its requested target alongside the optimistic position', () async { + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async => call.method == 'initialize' ? true : null, + testBody: () async { + final player = PlayerNative(); + final announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + await player.seek(const Duration(minutes: 5)); + await Future.delayed(Duration.zero); + + expect(announced, [const Duration(minutes: 5)]); + expect(player.state.position, const Duration(minutes: 5)); + } finally { + await subscription.cancel(); + await player.dispose(); + } + }, + ); + }); + + test('a seek the backend rejects announces the position it rolled back to', () async { + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async { + if (call.method == 'initialize') return true; + if (call.method == 'command') { + throw PlatformException(code: 'COMMAND_FAILED', message: 'player not ready'); + } + return null; + }, + testBody: () async { + final player = PlayerNative(); + final announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + await player.seek(const Duration(minutes: 5)); + await Future.delayed(Duration.zero); + + // The request is announced when the playhead optimistically moves, + // because a consumer coalescing its own seeks has to react inside + // that window. Undoing it has to be announced too, or that consumer + // keeps building on a target the backend rejected. + expect(announced, [const Duration(minutes: 5), Duration.zero]); + expect(player.state.position, Duration.zero); + } finally { + await subscription.cancel(); + await player.dispose(); + } + }, + ); + }); + + test('a rejected seek reports the position a native tick left behind', () async { + // `_positionMs` updates on every tick while `PlayerState.position` is + // throttled, so a tick from the still-playing old position can land mid + // seek. The rollback must not overwrite it — but staying silent would + // leave the rejected target pinned (#1819). + late PlayerNative player; + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async { + if (call.method == 'initialize') return true; + if (call.method == 'command') { + player.handlePropertyChange('time-pos', 20.0); + throw PlatformException(code: 'COMMAND_FAILED', message: 'player not ready'); + } + return null; + }, + testBody: () async { + player = PlayerNative(); + final announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + // Anchor the 250 ms emit throttle, which runs off a Stopwatch + // started at construction. Either this tick is itself throttled + // (so under 250 ms have passed) or it emits and resets the window; + // either way the racing tick a few microseconds later is throttled, + // so only the rollback can bring `PlayerState.position` back in + // line. Without this the assertion would silently stop + // discriminating on a slow machine. + player.handlePropertyChange('time-pos', 10.0); + + await player.seek(const Duration(minutes: 5)); + await Future.delayed(Duration.zero); + + expect(announced, [const Duration(minutes: 5), const Duration(seconds: 20)]); + expect(player.currentPosition, const Duration(seconds: 20), reason: 'the newer tick wins the position'); + // `PlayerState.position` is throttled, so it can still be showing + // the optimistic target. Consumers rebase off it, so the abandoned + // target must not survive there either. + expect(player.state.position, const Duration(seconds: 20)); + } finally { + await subscription.cancel(); + await player.dispose(); + } + }, + ); + }); + + test('a superseded seek failing late leaves the newer seek alone', () async { + // Otherwise the stale failure republishes the newer target, which reads + // as a foreign jump and retires the pin that target belongs to (#1819). + final firstReached = Completer(); + final releaseFirst = Completer(); + var commands = 0; + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async { + if (call.method == 'initialize') return true; + if (call.method == 'command' && commands++ == 0) { + firstReached.complete(); + await releaseFirst.future; + throw PlatformException(code: 'COMMAND_FAILED', message: 'superseded'); + } + return null; + }, + testBody: () async { + final player = PlayerNative(); + final announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + final stale = player.seek(const Duration(minutes: 5)); + await firstReached.future; + await player.seek(const Duration(minutes: 9)); + + releaseFirst.complete(); + await stale; + await Future.delayed(Duration.zero); + + expect(announced, [const Duration(minutes: 5), const Duration(minutes: 9)]); + expect(player.state.position, const Duration(minutes: 9), reason: 'the newer seek still owns the playhead'); + } finally { + if (!releaseFirst.isCompleted) releaseFirst.complete(); + await subscription.cancel(); + await player.dispose(); + } + }, + ); + }); + + /// Runs two overlapping seeks — 5m then 9m — where the FIRST one's backend + /// call is held open until the second has already settled, so replies come + /// back out of order. [firstSucceeds]/[secondSucceeds] pick each outcome. + /// + /// [tickDuringFlight] injects a native `time-pos` while both are in flight. + /// An anchor tick is sent first so it is guaranteed to fall inside the + /// 250 ms emit throttle, which is what leaves `PlayerState.position` stale. + Future<({List announced, Duration position, Duration rawPosition})> runOverlappingSeeks({ + required bool firstSucceeds, + required bool secondSucceeds, + Duration? tickDuringFlight, + }) async { + late PlayerNative player; + final firstReached = Completer(); + final releaseFirst = Completer(); + var commands = 0; + late List announced; + late Duration position; + late Duration rawPosition; + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async { + if (call.method == 'initialize') return true; + if (call.method == 'command') { + final isFirst = commands++ == 0; + if (isFirst) { + firstReached.complete(); + await releaseFirst.future; + } else if (tickDuringFlight != null) { + player.handlePropertyChange('time-pos', tickDuringFlight.inMilliseconds / 1000); + } + if (isFirst ? firstSucceeds : secondSucceeds) return null; + throw PlatformException(code: 'COMMAND_FAILED', message: 'player not ready'); + } + return null; + }, + testBody: () async { + player = PlayerNative(); + announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + player.handlePropertyChange('time-pos', 0.0); + final first = player.seek(const Duration(minutes: 5)); + await firstReached.future; + await player.seek(const Duration(minutes: 9)); + + releaseFirst.complete(); + await first; + await Future.delayed(Duration.zero); + position = player.state.position; + rawPosition = player.currentPosition; + } finally { + if (!releaseFirst.isCompleted) releaseFirst.complete(); + await subscription.cancel(); + await player.dispose(); + } + }, + ); + return (announced: announced, position: position, rawPosition: rawPosition); + } + + test('two overlapping seeks both failing put the playhead back where it started', () async { + final result = await runOverlappingSeeks(firstSucceeds: false, secondSucceeds: false); + + expect(result.announced, [const Duration(minutes: 5), const Duration(minutes: 9), Duration.zero]); + expect(result.position, Duration.zero, reason: 'neither seek landed, so nothing moved'); + }); + + test('a rejected newer seek falls back to the target its predecessor landed', () async { + final result = await runOverlappingSeeks(firstSucceeds: true, secondSucceeds: false); + + expect(result.announced, [const Duration(minutes: 5), const Duration(minutes: 9), const Duration(minutes: 5)]); + expect(result.position, const Duration(minutes: 5), reason: 'only the older seek was accepted'); + }); + + test('out-of-order replies still leave the newest accepted seek in charge', () async { + // The backend applies commands in issue order, so 9m wins even though its + // reply arrived first. Picking by completion order would rewind to 5m. + final result = await runOverlappingSeeks(firstSucceeds: true, secondSucceeds: true); + + expect(result.announced, [const Duration(minutes: 5), const Duration(minutes: 9)]); + expect(result.position, const Duration(minutes: 9)); + }); + + test('a throttled tick cannot hide the target the accepted seek reached', () async { + // The backend accepted 5m and reported it, but `PlayerState.position` is + // throttled and still shows the rejected 9m. Consumers rebase off state, + // so deciding by position instead of by which request landed would leave + // the next skip chaining from a target nothing reached (#1819). + final result = await runOverlappingSeeks( + firstSucceeds: true, + secondSucceeds: false, + tickDuringFlight: const Duration(minutes: 5), + ); + + expect(result.rawPosition, const Duration(minutes: 5)); + expect(result.position, const Duration(minutes: 5), reason: 'state must not keep the rejected 9m target'); + expect(result.announced.last, const Duration(minutes: 5)); + }); + + test('a source opened at a resume position announces where the playhead landed', () async { + // Dead-stream recovery, a quality switch and a background-suspend resume + // all rebuild the stream at a position instead of seeking, so the reload + // is the second way the playhead moves discontinuously (#1819). + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async => call.method == 'initialize' ? true : null, + testBody: () async { + final player = PlayerNative(); + final announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + await player.open(Media('https://example.test/reload.mkv', start: const Duration(minutes: 12))); + await Future.delayed(Duration.zero); + + expect(announced, [const Duration(minutes: 12)]); + expect(player.state.position, const Duration(minutes: 12)); + } finally { + await subscription.cancel(); + await player.dispose(); + } + }, + ); + }); + + test('a subtitle seek announces the jump, then the cue mpv chose', () async { + // Ctrl+Arrow is bound to sub-seek by default, right beside the plain + // arrow skip. mpv picks the cue itself, so the jump is announced without + // a destination up front and the real one is read back after, because + // `PlayerState.position` is what the next skip rebases from (#1819). + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async { + if (call.method == 'initialize') return true; + if (call.method == 'getProperty' && (call.arguments as Map)['name'] == 'time-pos') { + return '123.5'; + } + return null; + }, + testBody: () async { + final player = PlayerNative(); + final announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + await player.command(['sub-seek', '1']); + await Future.delayed(Duration.zero); + expect(announced, [isNull, const Duration(milliseconds: 123500)]); + expect( + player.state.position, + const Duration(milliseconds: 123500), + reason: 'a skip pressed straight after must start from the cue, not from before it', + ); + + // An ordinary command leaves the playhead alone and must stay quiet. + await player.command(['screenshot', 'subtitles']); + await Future.delayed(Duration.zero); + expect(announced, hasLength(2)); + } finally { + await subscription.cancel(); + await player.dispose(); + } + }, + ); + }); + + test('a relocation while seeks are in flight survives their settlement', () async { + // A subtitle seek (or an in-place reload) can land between an overlapping + // group starting and finishing. It is newer information than any of their + // outcomes, so the group must not write its own answer over it (#1819). + final firstReached = Completer(); + final releaseFirst = Completer(); + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async { + if (call.method == 'initialize') return true; + if (call.method == 'getProperty' && (call.arguments as Map)['name'] == 'time-pos') { + return '400.0'; + } + if (call.method == 'command') { + final args = List.from((call.arguments as Map)['args'] as List); + if (args.first == 'sub-seek') return null; + if (double.parse(args[1]) == 300.0) { + firstReached.complete(); + await releaseFirst.future; + return null; // the older seek is accepted, late + } + throw PlatformException(code: 'COMMAND_FAILED', message: 'newer seek rejected'); + } + return null; + }, + testBody: () async { + final player = PlayerNative(); + final announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + final first = player.seek(const Duration(minutes: 5)); + await firstReached.future; + await player.seek(const Duration(minutes: 9)); + await player.command(['sub-seek', '1']); + + releaseFirst.complete(); + await first; + await Future.delayed(Duration.zero); + + expect( + player.state.position, + const Duration(seconds: 400), + reason: 'the subtitle cue is newer than either seek outcome', + ); + expect(announced.last, const Duration(seconds: 400)); + } finally { + if (!releaseFirst.isCompleted) releaseFirst.complete(); + await subscription.cancel(); + await player.dispose(); + } + }, + ); + }); + + test('a seek started after a relocation is anchored to it, not to the held group', () async { + // The old seek is still in flight, so a naive in-flight count would fold + // the new one into its group and roll back to the pre-relocation anchor. + final firstReached = Completer(); + final releaseFirst = Completer(); + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async { + if (call.method == 'initialize') return true; + if (call.method == 'getProperty' && (call.arguments as Map)['name'] == 'time-pos') { + return '400.0'; + } + if (call.method == 'command') { + final args = List.from((call.arguments as Map)['args'] as List); + if (args.first == 'sub-seek') return null; + if (double.parse(args[1]) == 300.0) { + firstReached.complete(); + await releaseFirst.future; + return null; + } + throw PlatformException(code: 'COMMAND_FAILED', message: 'rejected'); + } + return null; + }, + testBody: () async { + final player = PlayerNative(); + final announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + final held = player.seek(const Duration(minutes: 5)); + await firstReached.future; + await player.command(['sub-seek', '1']); + await player.seek(const Duration(minutes: 9)); + + releaseFirst.complete(); + await held; + await Future.delayed(Duration.zero); + + expect( + player.state.position, + const Duration(seconds: 400), + reason: 'the rejected 9m seek must fall back to the cue it started from', + ); + expect(announced.last, const Duration(seconds: 400)); + } finally { + if (!releaseFirst.isCompleted) releaseFirst.complete(); + await subscription.cancel(); + await player.dispose(); + } + }, + ); + }); + + test('a slow subtitle seek does not overwrite a seek issued while it ran', () async { + // KeyboardShortcutsService fires sub-seek without awaiting it, so its + // read-back can complete after a newer skip has already been issued. The + // newer seek owns the playhead; the stale cue must be dropped. + final subSeekReached = Completer(); + final releaseSubSeek = Completer(); + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async { + if (call.method == 'initialize') return true; + if (call.method == 'getProperty' && (call.arguments as Map)['name'] == 'time-pos') { + return '400.0'; + } + if (call.method == 'command') { + final args = List.from((call.arguments as Map)['args'] as List); + if (args.first == 'sub-seek') { + subSeekReached.complete(); + await releaseSubSeek.future; + } + } + return null; + }, + testBody: () async { + final player = PlayerNative(); + final announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + final subSeek = player.command(['sub-seek', '1']); + await subSeekReached.future; + await player.seek(const Duration(minutes: 9)); + + releaseSubSeek.complete(); + await subSeek; + await Future.delayed(Duration.zero); + + expect(announced, [isNull, const Duration(minutes: 9)]); + expect( + player.state.position, + const Duration(minutes: 9), + reason: 'the stale subtitle cue must not win over the newer seek', + ); + } finally { + if (!releaseSubSeek.isCompleted) releaseSubSeek.complete(); + await subscription.cancel(); + await player.dispose(); + } + }, + ); + }); + + test('two overlapping subtitle seeks leave the newer cue in charge', () async { + // Each press fires without awaiting, so the older command can answer + // last. mpv applies them in order, so the newer cue is the truth and the + // older one must be discarded on arrival rather than merely overtaken. + final gates = [Completer(), Completer()]; + final reached = [Completer(), Completer()]; + var commandIndex = 0; + var readBackIndex = 0; + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async { + if (call.method == 'initialize') return true; + if (call.method == 'getProperty' && (call.arguments as Map)['name'] == 'time-pos') { + // The newer command settles first, so it reads its cue first. + return readBackIndex++ == 0 ? '200.0' : '100.0'; + } + if (call.method == 'command') { + final index = commandIndex++; + reached[index].complete(); + await gates[index].future; + } + return null; + }, + testBody: () async { + final player = PlayerNative(); + final announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + final first = player.command(['sub-seek', '1']); + await reached[0].future; + final second = player.command(['sub-seek', '1']); + await reached[1].future; + + // Newer answers first and publishes 200; the older press then + // answers with its own stale 100. + gates[1].complete(); + await second; + await Future.delayed(Duration.zero); + expect(player.state.position, const Duration(seconds: 200)); + + gates[0].complete(); + await first; + await Future.delayed(Duration.zero); + + expect(player.state.position, const Duration(seconds: 200), reason: 'the stale cue must be discarded'); + expect( + announced, + isNot(contains(const Duration(seconds: 100))), + reason: 'and never announced to a consumer', + ); + } finally { + for (final gate in gates) { + if (!gate.isCompleted) gate.complete(); + } + await subscription.cancel(); + await player.dispose(); + } + }, + ); + }); + + test('a rejected relocation leaves the seek group able to undo itself', () async { + // Claiming must not take ownership away from an in-flight group. The + // subtitle command is refused, so the playhead never moved and the + // rejected seek still has to be rolled back rather than left standing. + final seekReached = Completer(); + final releaseSeek = Completer(); + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async { + if (call.method == 'initialize') return true; + if (call.method == 'command') { + final args = List.from((call.arguments as Map)['args'] as List); + if (args.first == 'seek') { + seekReached.complete(); + await releaseSeek.future; + throw PlatformException(code: 'COMMAND_FAILED', message: 'rejected'); + } + // The subtitle command is refused too, so nothing moved. + throw PlatformException(code: 'COMMAND_FAILED', message: 'refused'); + } + return null; + }, + testBody: () async { + final player = PlayerNative(); + final announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + final seek = player.seek(const Duration(minutes: 5)); + await seekReached.future; + await expectLater(player.command(['sub-seek', '1']), throwsA(isA())); + + releaseSeek.complete(); + await seek; + await Future.delayed(Duration.zero); + + expect( + player.state.position, + Duration.zero, + reason: 'the rejected seek must still be undone, not left owning the playhead', + ); + expect(announced.last, Duration.zero); + } finally { + if (!releaseSeek.isCompleted) releaseSeek.complete(); + await subscription.cancel(); + await player.dispose(); + } + }, + ); + }); + + test('an accepted relocation with an unreadable position still takes ownership', () async { + // mpv moved the playhead; only the read-back failed. An older rejected + // seek must not roll back across a cue that actually happened, even + // though nobody can say yet where it landed. + final seekReached = Completer(); + final releaseSeek = Completer(); + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async { + if (call.method == 'initialize') return true; + // Accepted command, unreadable position. + if (call.method == 'getProperty') return null; + if (call.method == 'command') { + final args = List.from((call.arguments as Map)['args'] as List); + if (args.first == 'seek') { + seekReached.complete(); + await releaseSeek.future; + throw PlatformException(code: 'COMMAND_FAILED', message: 'rejected'); + } + } + return null; + }, + testBody: () async { + final player = PlayerNative(); + final announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + final seek = player.seek(const Duration(minutes: 5)); + await seekReached.future; + await player.command(['sub-seek', '1']); + + releaseSeek.complete(); + await seek; + await Future.delayed(Duration.zero); + + expect(announced, [ + const Duration(minutes: 5), + isNull, + ], reason: 'the rejected seek must not announce a rollback across the cue'); + } finally { + if (!releaseSeek.isCompleted) releaseSeek.complete(); + await subscription.cancel(); + await player.dispose(); + } + }, + ); + }); + + test('a rejected seek cannot invalidate the cue an accepted subtitle seek is still reading', () async { + // The seek only ever asked; it never moved the playhead. Ownership has to + // turn on operations the backend accepted, or the subtitle cue is thrown + // away and the next skip rebases from the pre-cue anchor. + final subSeekReached = Completer(); + final releaseSubSeek = Completer(); + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async { + if (call.method == 'initialize') return true; + if (call.method == 'getProperty' && (call.arguments as Map)['name'] == 'time-pos') { + return '400.0'; + } + if (call.method == 'command') { + final args = List.from((call.arguments as Map)['args'] as List); + if (args.first == 'sub-seek') { + subSeekReached.complete(); + await releaseSubSeek.future; + return null; // accepted, just slow + } + throw PlatformException(code: 'COMMAND_FAILED', message: 'rejected'); + } + return null; + }, + testBody: () async { + final player = PlayerNative(); + final announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + final subSeek = player.command(['sub-seek', '1']); + await subSeekReached.future; + await player.seek(const Duration(minutes: 9)); + + releaseSubSeek.complete(); + await subSeek; + await Future.delayed(Duration.zero); + + expect( + player.state.position, + const Duration(seconds: 400), + reason: 'the accepted cue owns the playhead; the rejected seek never touched it', + ); + expect(announced.last, const Duration(seconds: 400)); + } finally { + if (!releaseSubSeek.isCompleted) releaseSubSeek.complete(); + await subscription.cancel(); + await player.dispose(); + } + }, + ); + }); + + test('a successful seek repairs a stale tick but keeps progress past its target', () async { + // `_positionMs` moves on every tick while `PlayerState.position` is + // throttled, so settlement has to reconcile them — without rewinding + // playback that has legitimately moved on from the accepted target. What + // separates the two is the media time the command was in flight for, not + // a fixed window: at speed, real progress can outrun any constant. + const inFlight = Duration(milliseconds: 300); + Future settleWithTick(Duration tick, {required bool playing, double speed = 1.0}) async { + late Duration result; + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async { + if (call.method == 'initialize') return true; + if (call.method == 'command') await Future.delayed(inFlight); + return null; + }, + testBody: () async { + final player = PlayerNative(); + try { + if (playing) player.handlePropertyChange('pause', false); + player.handlePropertyChange('speed', speed); + // Anchor the emit throttle so the tick below cannot reach state + // on its own; only settlement can. + player.handlePropertyChange('time-pos', 0.0); + final seek = player.seek(const Duration(minutes: 5)); + player.handlePropertyChange('time-pos', tick.inMilliseconds / 1000); + await seek; + await Future.delayed(Duration.zero); + result = player.state.position; + } finally { + await player.dispose(); + } + }, + ); + return result; + } + + expect( + await settleWithTick(const Duration(seconds: 20), playing: true), + const Duration(minutes: 5), + reason: 'a tick from before the seek is stale and must not survive it', + ); + expect( + await settleWithTick(const Duration(minutes: 5, milliseconds: 400), playing: true), + const Duration(minutes: 5, milliseconds: 400), + reason: 'playback that ran on past the target during the round trip is fresher than it', + ); + expect( + await settleWithTick(const Duration(minutes: 5, seconds: 30), playing: true), + const Duration(minutes: 5), + reason: 'a jump far past the target is not progress the round trip could produce', + ); + expect( + await settleWithTick(const Duration(minutes: 5, milliseconds: 400), playing: false), + const Duration(minutes: 5), + reason: 'a paused player covers no media time, so any drift is stale', + ); + // The window scales with the rate, so the same drift flips verdict. + expect( + await settleWithTick(const Duration(minutes: 5, seconds: 2), playing: true, speed: 8.0), + const Duration(minutes: 5, seconds: 2), + reason: 'at 8x the round trip really can cover two seconds of media', + ); + expect( + await settleWithTick(const Duration(minutes: 5, seconds: 2), playing: true), + const Duration(minutes: 5), + reason: 'at 1x the same drift is far more than the round trip could cover', + ); + expect( + await settleWithTick(const Duration(minutes: 5, milliseconds: 200), playing: true, speed: 0.25), + const Duration(minutes: 5), + reason: 'slow motion covers less ground, so this is a stale tick, not progress', + ); + }); + + /// A subtitle cue whose read-back lands while a newer seek is still in + /// flight. [seekSucceeds] decides which of the two owns the playhead. + Future<({Duration position, List announced})> deferredCueRace({required bool seekSucceeds}) async { + final readReached = Completer(); + final releaseRead = Completer(); + final seekReached = Completer(); + final releaseSeek = Completer(); + late Duration position; + late List announced; + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async { + if (call.method == 'initialize') return true; + if (call.method == 'getProperty') { + readReached.complete(); + await releaseRead.future; + return '400.0'; + } + if (call.method == 'command') { + final args = List.from((call.arguments as Map)['args'] as List); + if (args.first == 'seek') { + seekReached.complete(); + await releaseSeek.future; + if (!seekSucceeds) throw PlatformException(code: 'COMMAND_FAILED', message: 'rejected'); + } + } + return null; + }, + testBody: () async { + final player = PlayerNative(); + announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + final cue = player.command(['sub-seek', '1']); + await readReached.future; + final seek = player.seek(const Duration(minutes: 9)); + await seekReached.future; + + // The cue answers while the newer seek is still unresolved. + releaseRead.complete(); + await cue; + await Future.delayed(Duration.zero); + + releaseSeek.complete(); + await seek; + await Future.delayed(Duration.zero); + position = player.state.position; + } finally { + for (final gate in [releaseRead, releaseSeek]) { + if (!gate.isCompleted) gate.complete(); + } + await subscription.cancel(); + await player.dispose(); + } + }, + ); + return (position: position, announced: announced); + } + + test('a cue held for a pending seek is published when that seek is rejected', () async { + final result = await deferredCueRace(seekSucceeds: false); + + expect(result.position, const Duration(seconds: 400), reason: 'the seek moved nothing; the cue did'); + expect(result.announced.last, const Duration(seconds: 400)); + }); + + test('a cue held for a pending seek is dropped when that seek lands', () async { + final result = await deferredCueRace(seekSucceeds: true); + + expect(result.position, const Duration(minutes: 9), reason: 'the seek landed after the cue, so it wins'); + expect( + result.announced, + isNot(contains(const Duration(seconds: 400))), + reason: 'a superseded cue must never reach a consumer as a foreign jump', + ); + }); + + test('an accepted cue with no destination yet still lifts a rejected seek target off state', () async { + // Driven through the arbitration seams directly: the method channel + // serialises mock handlers, so this interleaving — commit while a newer + // seek is unresolved, then that seek rejected before the cue's position + // is known — cannot be produced through two concurrent channel calls. + final seekReached = Completer(); + final releaseSeek = Completer(); + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async => call.method == 'initialize' ? true : null, + testBody: () async { + final player = _ArbitrationPlayerNative(); + try { + // A distinctive starting position, so the fallback below is pinned + // to the group's anchor rather than passing on a default zero. + player.handlePropertyChange('time-pos', 42.0); + const anchor = Duration(seconds: 42); + + final token = player.begin(); + final seek = player.seekVia(const Duration(minutes: 9), () async { + seekReached.complete(); + await releaseSeek.future; + throw PlatformException(code: 'COMMAND_FAILED', message: 'rejected'); + }); + await seekReached.future; + expect( + player.state.position, + const Duration(minutes: 9), + reason: 'the seek wrote its target optimistically', + ); + + // The relocation is accepted while that seek is still unresolved, + // so it is held without a destination. + player.commit(token); + + releaseSeek.complete(); + await seek; + await Future.delayed(Duration.zero); + + expect( + player.state.position, + anchor, + reason: 'the rejected target gives way to where the group started, the nearest known base', + ); + } finally { + if (!releaseSeek.isCompleted) releaseSeek.complete(); + await player.dispose(); + } + }, + ); + }); + + test('a detached group cannot consume a relocation held for the group that replaced it', () async { + // Three operations deep: an older group is detached, a newer one takes + // over, and a cue is held against that newer one. The older group + // draining must not answer for a cue it never saw — publishing would + // retire the newer group's pin mid-flight, and falling back would restore + // an anchor from the timeline it was on. + final firstReached = Completer(); + final releaseFirst = Completer(); + final secondReached = Completer(); + final releaseSecond = Completer(); + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async => call.method == 'initialize' ? true : null, + testBody: () async { + final player = _ArbitrationPlayerNative(); + final announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + final detached = player.seekVia(const Duration(minutes: 2), () async { + firstReached.complete(); + await releaseFirst.future; + }); + await firstReached.future; + // Detaches the first group without waiting for it. + player.commit(player.begin()); + + // Claimed before the replacement group starts, so that group is + // newer and this cue has to wait on it. + final heldToken = player.begin(); + final active = player.seekVia(const Duration(minutes: 9), () async { + secondReached.complete(); + await releaseSecond.future; + throw PlatformException(code: 'COMMAND_FAILED', message: 'rejected'); + }); + await secondReached.future; + player.publish(const Duration(minutes: 7), heldToken); + expect(announced, isNot(contains(const Duration(minutes: 7))), reason: 'held, not published'); + + releaseFirst.complete(); + await detached; + await Future.delayed(Duration.zero); + expect( + announced, + isNot(contains(const Duration(minutes: 7))), + reason: 'the detached group must not publish a cue held for its successor', + ); + + releaseSecond.complete(); + await active; + await Future.delayed(Duration.zero); + + expect( + player.state.position, + const Duration(minutes: 7), + reason: 'the cue outlives the seek it was waiting on, and that seek was rejected', + ); + } finally { + for (final gate in [releaseFirst, releaseSecond]) { + if (!gate.isCompleted) gate.complete(); + } + await subscription.cancel(); + await player.dispose(); + } + }, + ); + }); + + test('a detached group draining does not sync a tick over the live group\'s target', () async { + // The tick arrived inside the replacement group's window, so it is that + // group's business. The older group draining must leave state alone + // rather than treating "the backend spoke last" as licence to write. + final firstReached = Completer(); + final releaseFirst = Completer(); + final secondReached = Completer(); + final releaseSecond = Completer(); + Future? active; + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async => call.method == 'initialize' ? true : null, + testBody: () async { + final player = _ArbitrationPlayerNative(); + try { + final detached = player.seekVia(const Duration(minutes: 2), () async { + firstReached.complete(); + await releaseFirst.future; + throw PlatformException(code: 'COMMAND_FAILED', message: 'rejected'); + }); + await firstReached.future; + // Detaches the first group, then starts the replacement whose + // optimistic target is what state legitimately shows. + player.commit(player.begin()); + active = player.seekVia(const Duration(minutes: 9), () async { + secondReached.complete(); + await releaseSecond.future; + }); + await secondReached.future; + + // A stale tick lands mid-flight, as they do while a seek is running. + player.handlePropertyChange('time-pos', 120.0); + + releaseFirst.complete(); + await detached; + await Future.delayed(Duration.zero); + expect( + player.state.position, + const Duration(minutes: 9), + reason: 'the live group owns state; a drained predecessor may not hand it a tick from inside that window', + ); + } finally { + for (final gate in [releaseFirst, releaseSecond]) { + if (!gate.isCompleted) gate.complete(); + } + await active; + await player.dispose(); + } + }, + ); + }); + + test('a tick that lands exactly on a rejected target is still the backend talking', () async { + // Playback genuinely reached the position a doomed seek had asked for. + // Recognising ownership by value would read that tick as the optimistic + // write and roll the playhead back to where the seek started. + final seekReached = Completer(); + final releaseSeek = Completer(); + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async => call.method == 'initialize' ? true : null, + testBody: () async { + final player = _ArbitrationPlayerNative(); + try { + // A distinctive base, so a rollback to it would be unmistakable. + player.handlePropertyChange('time-pos', 42.0); + final seek = player.seekVia(const Duration(minutes: 9), () async { + seekReached.complete(); + await releaseSeek.future; + throw PlatformException(code: 'COMMAND_FAILED', message: 'rejected'); + }); + await seekReached.future; + // The coincidence: the backend reports the very value the seek + // wrote optimistically. + player.handlePropertyChange('time-pos', 540.0); + + releaseSeek.complete(); + await seek; + await Future.delayed(Duration.zero); + expect( + player.state.position, + const Duration(minutes: 9), + reason: 'a reported position is authoritative whatever it equals; only the writer identifies it', + ); + } finally { + if (!releaseSeek.isCompleted) releaseSeek.complete(); + await player.dispose(); + } + }, + ); + }); + + test('an incoming tick before file-loaded cannot erase where the outgoing track got to', () async { + // `time-pos` and `playlist-pos` ride the property flow; `file-loaded` + // rides the event flow, and the two are collected separately. So the new + // track can report its first position before the advance is detected. + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_audio_player', + eventChannelName: 'com.plezy/mpv_audio_player/events', + methodHandler: (call) async => call.method == 'initialize' ? true : null, + testBody: () async { + final player = _AdvancingAudioPlayerNative(); + try { + player.markInitialized(); + await player.setNext(const Media('file:///music/next.flac')); + + // The outgoing track's last report, then the boundary, then the + // incoming track's first report — all in property-flow order. + player.handlePropertyChange('time-pos', 42.0); + player.handlePropertyChange('playlist-pos', 1); + player.handlePropertyChange('time-pos', 0.5); + + // Only now does the event flow catch up. + player.handlePlayerEvent('file-loaded', null); + expect( + player.outgoingSourcePosition, + const Duration(seconds: 42), + reason: 'the outgoing track played to 42s; 0.5s is the track that replaced it', + ); + } finally { + await player.dispose(); + } + }, + ); + }); + + test('a freeze does not outlive the arm it was taken for', () async { + // The boundary was seen, then the handover was abandoned by a fresh open. + // A later advance that loses its own `playlist-pos` edge must fall back + // to what has been reported since, not to that abandoned snapshot. + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_audio_player', + eventChannelName: 'com.plezy/mpv_audio_player/events', + methodHandler: (call) async => call.method == 'initialize' ? true : null, + testBody: () async { + final player = _AdvancingAudioPlayerNative(); + try { + player.markInitialized(); + await player.setNext(const Media('file:///music/next.flac')); + player.handlePropertyChange('time-pos', 42.0); + player.handlePropertyChange('playlist-pos', 1); + + // The user picks something else instead, tearing the arm down. + await player.setNext(null); + player.handlePropertyChange('time-pos', 7.0); + + // A later advance whose boundary edge never arrived. + await player.setNext(const Media('file:///music/third.flac')); + player.handlePlayerEvent('file-loaded', null); + expect( + player.outgoingSourcePosition, + const Duration(seconds: 7), + reason: 'the abandoned arm\'s 42s belongs to a handover that never happened', + ); + } finally { + await player.dispose(); + } + }, + ); + }); + + test('a gapless advance through file-loaded starts the new track from zero', () async { + // The real seam: an armed entry plus a `file-loaded` the player did not + // ask for. A seek against the outgoing track is still in flight, and its + // target must not survive onto the track now playing. + final seekReached = Completer(); + final releaseSeek = Completer(); + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_audio_player', + eventChannelName: 'com.plezy/mpv_audio_player/events', + methodHandler: (call) async => call.method == 'initialize' ? true : null, + testBody: () async { + final player = _AdvancingAudioPlayerNative(); + try { + player.markInitialized(); + await player.setNext(const Media('file:///music/next.flac')); + + player.handlePropertyChange('time-pos', 42.0); + final seek = player.seekVia(const Duration(minutes: 9), () async { + seekReached.complete(); + await releaseSeek.future; + throw PlatformException(code: 'COMMAND_FAILED', message: 'rejected'); + }); + await seekReached.future; + expect(player.state.position, const Duration(minutes: 9), reason: 'written optimistically'); + + // mpv rolled into the armed entry on its own, announcing the + // boundary on the property flow first. + player.handlePropertyChange('playlist-pos', 1); + player.handlePlayerEvent('file-loaded', null); + expect( + player.state.position, + Duration.zero, + reason: 'the new track starts at its beginning, whatever the outgoing one was doing', + ); + expect( + player.outgoingSourcePosition, + const Duration(seconds: 42), + reason: 'what the outgoing track reported, not the target a doomed seek wrote over it', + ); + + // A second handoff with no tick in between: the new track reported + // nothing, so it got nowhere — inheriting its predecessor's + // position would report it stopped somewhere it never played. + player.markInitialized(); + await player.setNext(const Media('file:///music/third.flac')); + player.handlePlayerEvent('file-loaded', null); + expect( + player.outgoingSourcePosition, + Duration.zero, + reason: 'a source that never reported a position did not reach its predecessor\'s', + ); + + // Opening at a resume offset installs a source that is genuinely + // there, so a handoff before its first tick reports that, not zero + // and not the track before it. + player.installSource(const Duration(minutes: 5)); + player.markInitialized(); + await player.setNext(const Media('file:///music/fourth.flac')); + player.handlePlayerEvent('file-loaded', null); + expect( + player.outgoingSourcePosition, + const Duration(minutes: 5), + reason: 'a source installed at a resume offset is at that offset until told otherwise', + ); + + releaseSeek.complete(); + await seek; + await Future.delayed(Duration.zero); + expect( + player.state.position, + Duration.zero, + reason: 'the rejected seek belongs to a timeline that is gone; it may not restore onto this one', + ); + } finally { + if (!releaseSeek.isCompleted) releaseSeek.complete(); + await player.dispose(); + } + }, + ); + }); + + test('a rejected target just past an accepted one is not mistaken for progress', () async { + // Both seeks are in flight and land milliseconds apart, so the rejected + // one's optimistic write sits inside the window real playback could have + // covered. Only the backend observes playback; reading Dart's own write + // as an observation would keep the position the backend refused. + final olderReached = Completer(); + final releaseOlder = Completer(); + final newerReached = Completer(); + final releaseNewer = Completer(); + Future? newer; + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async => call.method == 'initialize' ? true : null, + testBody: () async { + final player = _ArbitrationPlayerNative(); + try { + player.handlePropertyChange('pause', false); + final older = player.seekVia(const Duration(minutes: 5), () async { + olderReached.complete(); + await releaseOlder.future; + }); + await olderReached.future; + newer = player.seekVia(const Duration(minutes: 5, milliseconds: 100), () async { + newerReached.complete(); + await releaseNewer.future; + throw PlatformException(code: 'COMMAND_FAILED', message: 'rejected'); + }); + await newerReached.future; + + releaseOlder.complete(); + await older; + releaseNewer.complete(); + await newer; + await Future.delayed(Duration.zero); + + expect( + player.state.position, + const Duration(minutes: 5), + reason: 'the accepted target stands; no backend tick ever reported the rejected one', + ); + } finally { + for (final gate in [releaseOlder, releaseNewer]) { + if (!gate.isCompleted) gate.complete(); + } + await newer; + await player.dispose(); + } + }, + ); + }); + + test('a group detached by a newer relocation still lifts its own rejected target', () async { + // The relocation is newer than the whole group, so it detaches it and the + // group is barred from rolling back across the cue. That does not license + // leaving a target the backend refused sitting on state. + final seekReached = Completer(); + final releaseSeek = Completer(); + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async => call.method == 'initialize' ? true : null, + testBody: () async { + final player = _ArbitrationPlayerNative(); + try { + player.handlePropertyChange('time-pos', 42.0); + + final seek = player.seekVia(const Duration(minutes: 9), () async { + seekReached.complete(); + await releaseSeek.future; + throw PlatformException(code: 'COMMAND_FAILED', message: 'rejected'); + }); + await seekReached.future; + + // Claimed and accepted after the seek, so it detaches the group. + player.commit(player.begin()); + + releaseSeek.complete(); + await seek; + await Future.delayed(Duration.zero); + + expect( + player.state.position, + const Duration(seconds: 42), + reason: 'a detached group must still clean up the target its own rejected request wrote', + ); + } finally { + if (!releaseSeek.isCompleted) releaseSeek.complete(); + await player.dispose(); + } + }, + ); + }); + + test('a replaced source publishes its own start instead of the old timeline', () async { + // Gapless advance rolls into a different track without publishing a + // position. A seek still in flight against the old track must not put its + // old-timeline base on the new one when it is rejected. + final seekReached = Completer(); + final releaseSeek = Completer(); + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async => call.method == 'initialize' ? true : null, + testBody: () async { + final player = _ArbitrationPlayerNative(); + try { + player.handlePropertyChange('time-pos', 42.0); + + final seek = player.seekVia(const Duration(minutes: 9), () async { + seekReached.complete(); + await releaseSeek.future; + throw PlatformException(code: 'COMMAND_FAILED', message: 'rejected'); + }); + await seekReached.future; + + // The backend rolls into the next track on its own. No position has + // been reported for it yet. + player.adoptNewSource(); + + releaseSeek.complete(); + await seek; + await Future.delayed(Duration.zero); + + expect( + player.state.position, + Duration.zero, + reason: 'the new track starts at its beginning; neither the old base nor the rejected target applies', + ); + + // Once the new track reports a position, that is authoritative. + player.handlePropertyChange('time-pos', 3.0); + expect(player.currentPosition, const Duration(seconds: 3)); + } finally { + if (!releaseSeek.isCompleted) releaseSeek.complete(); + await player.dispose(); + } + }, + ); + }); + + test('an accepted cue with no readable position still blocks a rejected newer seek rollback', () async { + // The command moved the playhead; only its position is unknown. A newer + // seek that is then rejected must not undo itself back across the cue. + final readReached = Completer(); + final releaseRead = Completer(); + final seekReached = Completer(); + final releaseSeek = Completer(); + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async { + if (call.method == 'initialize') return true; + if (call.method == 'getProperty') return null; // accepted, unreadable + if (call.method == 'command') { + final args = List.from((call.arguments as Map)['args'] as List); + if (args.first == 'seek') { + seekReached.complete(); + await releaseSeek.future; + throw PlatformException(code: 'COMMAND_FAILED', message: 'rejected'); + } + // Hold the subtitle command itself, so the seek starts while this + // relocation is still unresolved. + readReached.complete(); + await releaseRead.future; + } + return null; + }, + testBody: () async { + final player = PlayerNative(); + final announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + // A distinctive base, so the repair below is pinned to a real value + // rather than passing on a default zero. + player.handlePropertyChange('time-pos', 42.0); + + final cue = player.command(['sub-seek', '1']); + await readReached.future; + final seek = player.seek(const Duration(minutes: 9)); + await seekReached.future; + + releaseRead.complete(); + await cue; + releaseSeek.complete(); + await seek; + await Future.delayed(Duration.zero); + + expect( + announced, + [isNull, const Duration(minutes: 9), isNull], + reason: + 'no rollback may be announced across a cue the backend accepted, but the abandonment must be ' + 'said again: the first null predates the seek, whose own echo re-armed the consumer', + ); + expect( + player.state.position, + const Duration(seconds: 42), + reason: 'the rejected target must still come off state, even with nothing to replace it but the base', + ); + } finally { + for (final gate in [releaseRead, releaseSeek]) { + if (!gate.isCompleted) gate.complete(); + } + await subscription.cancel(); + await player.dispose(); + } + }, + ); + }); + + test('a subtitle seek announces before the command reaches the backend', () async { + // mpv relocates the playhead while the platform call is still in flight, + // so announcing on completion would leave a window where a skip commits + // from the superseded pin. + final commandReached = Completer(); + final releaseCommand = Completer(); + await withMockPlayerChannels( + methodChannelName: 'com.plezy/mpv_player', + eventChannelName: 'com.plezy/mpv_player/events', + methodHandler: (call) async { + if (call.method == 'initialize') return true; + if (call.method == 'command') { + commandReached.complete(); + await releaseCommand.future; + } + return null; + }, + testBody: () async { + final player = PlayerNative(); + final announced = []; + final subscription = player.streams.playheadJump.listen(announced.add); + try { + final pending = player.command(['sub-seek', '1']); + await commandReached.future; + await Future.delayed(Duration.zero); + + expect(announced, [isNull], reason: 'the jump must be known while the backend is still working'); + + releaseCommand.complete(); + await pending; + await Future.delayed(Duration.zero); + // This backend never answers the `time-pos` read, so nothing is + // published: a guessed position would be worse than none. + expect(announced, hasLength(1), reason: 'a failed read-back must not fabricate a destination'); + } finally { + if (!releaseCommand.isCompleted) releaseCommand.complete(); + await subscription.cancel(); + await player.dispose(); + } + }, + ); + }); + }); } diff --git a/test/screens/music/now_playing_screen_test.dart b/test/screens/music/now_playing_screen_test.dart index 10c5036b..2862d9a3 100644 --- a/test/screens/music/now_playing_screen_test.dart +++ b/test/screens/music/now_playing_screen_test.dart @@ -39,6 +39,7 @@ class _FakeMusicService extends StubMusicPlaybackService { MediaItem track; final MusicPlayContext context; final StreamController _positionController = StreamController.broadcast(sync: true); + final StreamController _playheadJumpController = StreamController.broadcast(sync: true); final List seeks = []; Duration _position = Duration.zero; @@ -55,6 +56,13 @@ class _FakeMusicService extends StubMusicPlaybackService { _positionController.add(position); } + /// Something outside the screen — OS media controls, a headset, the lock + /// screen — moved the playhead. + void emitPlayheadJump(Duration position) { + _position = position; + _playheadJumpController.add(position); + } + @override MediaItem get currentTrack => track; @@ -67,6 +75,9 @@ class _FakeMusicService extends StubMusicPlaybackService { @override Stream get positionStream => _positionController.stream; + @override + Stream get playheadJumpStream => _playheadJumpController.stream; + @override Duration get duration => const Duration(minutes: 3); @@ -87,6 +98,7 @@ class _FakeMusicService extends StubMusicPlaybackService { @override void dispose() { + _playheadJumpController.close(); _positionController.close(); super.dispose(); } @@ -193,6 +205,44 @@ void main() { expect(service.seeks, isEmpty); }); + testWidgets('a d-pad seek after an outside jump starts from where the jump landed', (tester) async { + // The seek bar pins its coalesced target so a slow backend cannot make the + // next press rebase off a stale position. OS media controls, a headset and + // the lock screen seek straight through the service, so that pin has to be + // retired when one of them moves the playhead (#1819). + final track = _track(id: 'one', title: 'First Track', album: 'First Album', year: 1973); + final service = _FakeMusicService( + track: track, + context: const MusicPlayContext(title: 'Queue', kind: MusicPlayContextKind.tracks), + ); + + await pumpNowPlaying(tester, service, isTv: true); + await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp); + await tester.pump(); + + await tester.sendKeyDownEvent(LogicalKeyboardKey.arrowRight); + await tester.pump(); + await tester.sendKeyUpEvent(LogicalKeyboardKey.arrowRight); + await tester.pump(); + expect(service.seeks, hasLength(1)); + final step = service.seeks.single; + expect(step, greaterThan(Duration.zero)); + + service.emitPlayheadJump(const Duration(minutes: 2)); + await tester.pump(); + + await tester.sendKeyDownEvent(LogicalKeyboardKey.arrowRight); + await tester.pump(); + await tester.sendKeyUpEvent(LogicalKeyboardKey.arrowRight); + await tester.pump(); + + expect( + service.seeks.last, + const Duration(minutes: 2) + step, + reason: 'the step must build on the outside jump, not on the superseded pin', + ); + }); + testWidgets('seek progress resets immediately when the track changes', (tester) async { final first = _track(id: 'one', title: 'First Track', album: 'First Album', year: 1973); final second = _track(id: 'two', title: 'Second Track', album: 'Second Album', year: 1999); diff --git a/test/services/music/music_playback_service_test.dart b/test/services/music/music_playback_service_test.dart index 0703854d..cf2e2269 100644 --- a/test/services/music/music_playback_service_test.dart +++ b/test/services/music/music_playback_service_test.dart @@ -61,6 +61,7 @@ class FakePlayer implements Player { final completedCtrl = StreamController.broadcast(sync: true); final bufferingCtrl = StreamController.broadcast(sync: true); final positionCtrl = StreamController.broadcast(sync: true); + final playheadJumpCtrl = StreamController.broadcast(sync: true); final durationCtrl = StreamController.broadcast(sync: true); final seekableCtrl = StreamController.broadcast(sync: true); final bufferCtrl = StreamController.broadcast(sync: true); @@ -83,6 +84,7 @@ class FakePlayer implements Player { completed: completedCtrl.stream, buffering: bufferingCtrl.stream, position: positionCtrl.stream, + playheadJump: playheadJumpCtrl.stream, duration: durationCtrl.stream, seekable: seekableCtrl.stream, buffer: bufferCtrl.stream, @@ -162,6 +164,7 @@ class FakePlayer implements Player { completedCtrl.close(); bufferingCtrl.close(); positionCtrl.close(); + playheadJumpCtrl.close(); durationCtrl.close(); seekableCtrl.close(); bufferCtrl.close(); @@ -189,6 +192,11 @@ class FakePlayer implements Player { @override Duration get currentPosition => _state.position; + /// Set by tests that drive a gapless transition; the real player records this + /// as the outgoing source hands over. + @override + Duration? outgoingSourcePosition; + @override bool get audioPassthroughActive => false; @@ -967,6 +975,38 @@ void main() { expect(h.client.reportsFor('started').map((r) => r.itemId), ['t1', 't2']); }); + test('a track with no metadata duration is reported stopped where the source actually got to', () async { + // Nothing supplies a duration to report at, so the outgoing position is the + // only truth — and by the time the transition is handled the player's live + // position already belongs to the track that replaced it. + final undated = testMediaItem( + id: 'nd', + backend: MediaBackend.plex, + kind: MediaKind.track, + title: 'Unknown length', + parentTitle: 'Album', + grandparentTitle: 'Artist', + serverId: 'srv', + ); + await h.playTracks([undated, t2]); + + // The gapless advance: the new source is at its start, and the player has + // recorded where the old one handed over. + h.player.outgoingSourcePosition = const Duration(minutes: 2, seconds: 12); + h.player.setPosition(Duration.zero); + h.player.emitTransition(_urlFor(t2)); + await pumpEventQueue(); + + final stopped = h.client.reportsFor('stopped').toList(); + expect(stopped, hasLength(1)); + expect(stopped.single.itemId, 'nd'); + expect( + stopped.single.position, + const Duration(minutes: 2, seconds: 12), + reason: 'the new source has reset the live position; the outgoing track played to 2:12', + ); + }); + test('completed with nothing armed parks paused at the end and keeps the track', () async { await h.playTracks([t1, t2]); h.player.emitTransition(_urlFor(t2)); @@ -1390,4 +1430,22 @@ void main() { expect(h.service.currentTrack?.id, 't1'); expect(h.service.sleepTimerActive, isFalse); }); + + test('the service republishes the player playhead jumps the now-playing bar listens to', () async { + // OS media controls, a headset and the lock screen seek straight through + // the service, so this stream is the only way the now-playing seek bar can + // learn that its pending keyboard target was superseded (#1819). What the + // bar then does with a jump is covered in test/media/stepped_seek_test.dart. + await h.playTracks([t1]); + + final jumps = []; + final subscription = h.service.playheadJumpStream.listen(jumps.add); + addTearDown(subscription.cancel); + + h.player.playheadJumpCtrl.add(const Duration(minutes: 2)); + h.player.playheadJumpCtrl.add(null); + await pumpEventQueue(); + + expect(jumps, [const Duration(minutes: 2), isNull]); + }); } diff --git a/test/widgets/video_controls_mobile_skip_zones_test.dart b/test/widgets/video_controls_mobile_skip_zones_test.dart index ac9ab822..e5e714ca 100644 --- a/test/widgets/video_controls_mobile_skip_zones_test.dart +++ b/test/widgets/video_controls_mobile_skip_zones_test.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'package:drift/native.dart'; import 'package:flutter/gestures.dart' show kDoubleTapTimeout; import 'package:flutter/material.dart'; @@ -299,6 +300,26 @@ void main() { await settleFeedback(tester); }); + testWidgets('a double tap keeps its own readout while a keyboard burst is pending', (tester) async { + // Both input paths share one badge. The tap's seek is foreign to the + // keyboard accumulator, so retiring that burst must not take down the + // readout the tap just raised. + await pumpControls(tester); + + await tester.sendKeyDownEvent(LogicalKeyboardKey.arrowRight); + await tester.pump(); + expect(find.byType(DoubleTapFeedback), findsOneWidget); + + await doubleTap(tester, forwardZoneOf(tester)); + + expect(find.byType(DoubleTapFeedback), findsOneWidget, reason: 'the tap that just seeked owns the readout now'); + expect(find.text('10s'), findsOneWidget, reason: 'and it counts only its own step'); + + await tester.sendKeyUpEvent(LogicalKeyboardKey.arrowRight); + await tester.pump(); + await settleFeedback(tester); + }); + testWidgets('a lone tap in the opposite zone does not skip', (tester) async { await pumpControls(tester); @@ -340,6 +361,7 @@ void main() { /// Minimal [Player] recording seek targets against a fixed 45-minute item. class _RecordingPlayer implements Player { final List seeks = []; + final StreamController _jumpController = StreamController.broadcast(); bool _playing = true; Duration _position = const Duration(minutes: 10); @@ -353,6 +375,7 @@ class _RecordingPlayer implements Player { @override PlayerStreams get streams => PlayerStreams( + playheadJump: _jumpController.stream, playing: const Stream.empty(), completed: const Stream.empty(), buffering: const Stream.empty(), @@ -377,6 +400,12 @@ class _RecordingPlayer implements Player { Future seek(Duration position) async { seeks.add(position); _position = position; + _jumpController.add(position); + } + + @override + Future dispose({bool preserveDisplayMode = false}) async { + await _jumpController.close(); } @override diff --git a/test/widgets/video_controls_transient_feedback_test.dart b/test/widgets/video_controls_transient_feedback_test.dart index c9919667..5b7ae72d 100644 --- a/test/widgets/video_controls_transient_feedback_test.dart +++ b/test/widgets/video_controls_transient_feedback_test.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:math' as math; import 'package:drift/native.dart'; import 'package:flutter/material.dart'; @@ -17,6 +18,7 @@ import 'package:plezy/utils/platform_detector.dart'; import 'package:plezy/watch_together/providers/watch_together_provider.dart'; import 'package:plezy/widgets/video_controls/player_chrome_controller.dart'; import 'package:plezy/widgets/app_icon.dart'; +import 'package:plezy/widgets/video_controls/desktop_video_controls.dart'; import 'package:plezy/widgets/video_controls/video_controls.dart'; import 'package:plezy/widgets/video_controls/widgets/double_tap_feedback.dart'; import 'package:plezy/widgets/video_controls/widgets/player_toast_indicator.dart'; @@ -75,6 +77,7 @@ void main() { chrome.dispose(); toast.dispose(); await database.close(); + await player.dispose(); }); Future pumpControls( @@ -83,6 +86,7 @@ void main() { bool wireTransportCallback = false, bool isLive = false, ValueChanged? onLiveSeekBy, + String itemId = 'transient-feedback', }) async { transportCommands = []; await tester.pumpWidget( @@ -101,7 +105,7 @@ void main() { child: PlexVideoControls( player: player, volumeController: volume, - metadata: testMediaItem(id: 'transient-feedback'), + metadata: testMediaItem(id: itemId), toastController: toast, chromeController: chrome, initialChapters: chapters, @@ -266,6 +270,101 @@ void main() { await settleFeedback(tester); }); + testWidgets('a skip after a jump elsewhere starts from where the jump landed', (tester) async { + // #1819: the timeline, a chapter jump, an OS media control and a Watch + // Together peer all land on Player.seek. Whichever of them moves the + // playhead, the coalesced target the previous skip pinned is stale, and a + // skip that resumes from it rewinds the user back across their own jump. + await pumpControls(tester); + + await tester.sendKeyDownEvent(LogicalKeyboardKey.mediaFastForward); + await tester.pump(); + await tester.sendKeyUpEvent(LogicalKeyboardKey.mediaFastForward); + await tester.pump(); + expect(player.seeks, [const Duration(minutes: 10, seconds: 10)]); + + await player.seek(const Duration(minutes: 30)); + await tester.pump(); + expect( + find.text('10s'), + findsNothing, + reason: 'the readout promised a skip the jump just cancelled, so it must come down at once', + ); + + await tester.sendKeyDownEvent(LogicalKeyboardKey.mediaFastForward); + await tester.pump(); + await tester.sendKeyUpEvent(LogicalKeyboardKey.mediaFastForward); + await tester.pump(); + + expect( + player.seeks.last, + const Duration(minutes: 30, seconds: 10), + reason: 'the skip must be relative to the new position, not to the superseded 10:10 target', + ); + expect(find.text('10s'), findsOneWidget, reason: 'the abandoned burst total must not keep climbing'); + + await settleFeedback(tester); + }); + + testWidgets('a skip after a stream rebuilt at a resume position starts from there', (tester) async { + // Dead-stream recovery answers a seek request by reopening the source at + // the target rather than seeking, so a fix that only watched Player.seek + // would leave the pin stale here. + await pumpControls(tester); + + await tester.sendKeyDownEvent(LogicalKeyboardKey.mediaFastForward); + await tester.pump(); + await tester.sendKeyUpEvent(LogicalKeyboardKey.mediaFastForward); + await tester.pump(); + final seeksBeforeReload = player.seeks.length; + + player.reopenAt(const Duration(minutes: 3)); + await tester.pump(); + expect(player.seeks, hasLength(seeksBeforeReload), reason: 'a reload is not a seek'); + + await tester.sendKeyDownEvent(LogicalKeyboardKey.mediaFastForward); + await tester.pump(); + await tester.sendKeyUpEvent(LogicalKeyboardKey.mediaFastForward); + await tester.pump(); + + expect(player.seeks.last, const Duration(minutes: 3, seconds: 10)); + + await settleFeedback(tester); + }); + + testWidgets('a new item drops the previous item\'s pending skip and its badge total', (tester) async { + // The controls survive an in-place episode swap. A pending target is an + // offset into the outgoing item's timeline, and the badge total describes + // a burst that will never be committed. + player.freezePositionOnSeek = true; + await pumpControls(tester); + + for (var i = 0; i < 2; i++) { + await tester.sendKeyDownEvent(LogicalKeyboardKey.mediaFastForward); + await tester.pump(); + await tester.sendKeyUpEvent(LogicalKeyboardKey.mediaFastForward); + await tester.pump(); + } + expect(find.text('20s'), findsOneWidget); + expect(player.seeks.last, const Duration(minutes: 10, seconds: 20)); + + await pumpControls(tester, itemId: 'next-episode'); + + await tester.sendKeyDownEvent(LogicalKeyboardKey.mediaFastForward); + await tester.pump(); + await tester.sendKeyUpEvent(LogicalKeyboardKey.mediaFastForward); + await tester.pump(); + + expect( + player.seeks.last, + const Duration(minutes: 10, seconds: 10), + reason: 'the new item restarts from the live position, not from the outgoing 10:20 target', + ); + expect(find.text('10s'), findsOneWidget, reason: 'the badge must not keep counting the abandoned burst'); + + await settleFeedback(tester); + }); + testWidgets('a media fast-forward key announces the chapter it lands on', (tester) async { await pumpControls( tester, @@ -713,9 +812,19 @@ void main() { chrome.dispose(); toast.dispose(); await database.close(); + await player.dispose(); }); - Future pumpDesktopControls(WidgetTester tester) async { + Future pumpDesktopControls( + WidgetTester tester, { + _RecordingPlayer? withPlayer, + bool isLive = false, + ValueChanged? onLiveSeekBy, + ValueChanged? onLiveSeek, + VoidCallback? onNext, + bool canNavigateMediaItems = false, + }) async { + final active = withPlayer ?? player; await tester.pumpWidget( MultiProvider( providers: [ @@ -730,12 +839,16 @@ void main() { width: 1280, height: 720, child: PlexVideoControls( - player: player, + player: active, volumeController: volume, metadata: testMediaItem(id: 'desktop-keyboard-seek'), toastController: toast, chromeController: chrome, - canNavigateMediaItems: false, + canNavigateMediaItems: canNavigateMediaItems, + isLive: isLive, + onLiveSeekBy: onLiveSeekBy, + onLiveSeek: onLiveSeek, + onNext: onNext, ), ), ), @@ -830,6 +943,119 @@ void main() { await settleFeedback(tester); }); + + testWidgets('a lagging backend still yields the pin to a jump elsewhere', (tester) async { + // The two invariants pull in opposite directions: a slow seek must not + // retire the pin (#1676), a jump from anywhere else must (#1819). Freeze + // the reported position so only the seek announcement can tell them apart. + player.freezePositionOnSeek = true; + await pumpDesktopControls(tester); + + await pressKey(tester, LogicalKeyboardKey.arrowRight); + await pressKey(tester, LogicalKeyboardKey.arrowRight); + expect(player.seeks.last, const Duration(minutes: 10, seconds: 20), reason: 'the burst is still pinned'); + + player.setPosition(const Duration(minutes: 2)); + await player.seek(const Duration(minutes: 2)); + await tester.pump(); + + await pressKey(tester, LogicalKeyboardKey.arrowRight); + + expect(player.seeks.last, const Duration(minutes: 2, seconds: 10)); + expect(find.text('10s'), findsOneWidget); + + await settleFeedback(tester); + }); + + testWidgets('swapping the player moves the pin listener with it', (tester) async { + // The controls survive a player replacement (didUpdateWidget accepts a new + // instance), so an accumulator left bound to the retired player would keep + // a pin from a timeline that no longer exists and take orders from a + // player nobody is watching. + final replacement = _RecordingPlayer() + ..setPosition(const Duration(minutes: 4)) + // Frozen, so a pinned chain stays distinguishable from a rebase. + ..freezePositionOnSeek = true; + addTearDown(replacement.dispose); + + await pumpDesktopControls(tester); + await pressKey(tester, LogicalKeyboardKey.arrowRight); + expect(player.seeks.last, const Duration(minutes: 10, seconds: 10)); + + await pumpDesktopControls(tester, withPlayer: replacement); + expect(tester.takeException(), isNull); + + await pressKey(tester, LogicalKeyboardKey.arrowRight); + expect( + replacement.seeks.last, + const Duration(minutes: 4, seconds: 10), + reason: 'the retired pin must not survive the swap and chain to 10:20', + ); + // The badge is not asserted here: pumping the replacement settles the + // feedback timer, which clears the total on its own. + + // Build a fresh pin on the new player, then let the retired one shout. + await pressKey(tester, LogicalKeyboardKey.arrowRight); + expect(replacement.seeks.last, const Duration(minutes: 4, seconds: 20)); + await player.seek(const Duration(minutes: 30)); + await tester.pump(); + + await pressKey(tester, LogicalKeyboardKey.arrowRight); + expect( + replacement.seeks.last, + const Duration(minutes: 4, seconds: 30), + reason: 'a jump from the retired player must not retire the current pin', + ); + + await settleFeedback(tester); + }); + + testWidgets('an absolute live seek takes down the badge a live skip raised', (tester) async { + // Live relative skips go to the parent epoch accumulator, not _hiddenSeek + // (#1253), so no playhead jump can retire this badge. The absolute seek + // cancels the queued skip, so its promised total is going nowhere. + final liveOffsets = []; + final absoluteSeeks = []; + await pumpDesktopControls(tester, isLive: true, onLiveSeekBy: liveOffsets.add, onLiveSeek: absoluteSeeks.add); + + await pressKey(tester, LogicalKeyboardKey.arrowRight); + expect(liveOffsets, [10]); + expect(find.text('10s'), findsOneWidget); + + chrome.show(); + await tester.pump(); + final live = tester.widget(find.byType(DesktopVideoControls)); + live.onLiveSeek!(120); + await tester.pump(); + + expect(absoluteSeeks, [120], reason: 'the wrapper still delegates to the screen'); + expect(find.byType(DoubleTapFeedback), findsNothing, reason: 'the skip it promised was cancelled with it'); + + await settleFeedback(tester); + }); + + testWidgets('switching what is playing takes the badge with the old timeline', (tester) async { + // A live channel switch cancels the queued live offset, and an item + // change re-keys the whole timeline. Neither runs through _hiddenSeek, so + // nothing else would retire a readout that now describes nothing. + final liveOffsets = []; + var nextPresses = 0; + await pumpDesktopControls(tester, isLive: true, onLiveSeekBy: liveOffsets.add, onNext: () => nextPresses++); + + await pressKey(tester, LogicalKeyboardKey.arrowRight); + expect(liveOffsets, [10]); + expect(find.text('10s'), findsOneWidget); + + chrome.show(); + await tester.pump(); + tester.widget(find.byType(DesktopVideoControls)).onNext!(); + await tester.pump(); + + expect(nextPresses, 1, reason: 'the wrapper still delegates to the screen'); + expect(find.byType(DoubleTapFeedback), findsNothing); + + await settleFeedback(tester); + }); }); group('formatSkipFeedbackLabel', () { @@ -851,6 +1077,7 @@ void main() { /// playing/position state so intent-dependent behaviour can be asserted. class _RecordingPlayer implements Player { final List seeks = []; + final StreamController _jumpController = StreamController.broadcast(); int playCalls = 0; int pauseCalls = 0; int playOrPauseCalls = 0; @@ -866,6 +1093,13 @@ class _RecordingPlayer implements Player { void setPosition(Duration value) => _position = value; + /// Mirrors [PlayerBase.resetPlaybackProgress]: an in-place reload rebuilds + /// the stream at a resume position without ever calling [seek]. + void reopenAt(Duration value) { + _position = value; + _jumpController.add(value); + } + @override String get playerType => 'mpv'; @@ -879,6 +1113,7 @@ class _RecordingPlayer implements Player { completed: const Stream.empty(), buffering: const Stream.empty(), position: const Stream.empty(), + playheadJump: _jumpController.stream, duration: const Stream.empty(), seekable: const Stream.empty(), buffer: const Stream.empty(), @@ -895,10 +1130,19 @@ class _RecordingPlayer implements Player { backendSwitched: const Stream.empty(), ); + /// Mirrors [PlayerBase.runSeek]: the requested target is announced whatever + /// asked for it, while [freezePositionOnSeek] models a backend that has not + /// moved the reported position yet. @override Future seek(Duration position) async { seeks.add(position); if (!freezePositionOnSeek) _position = position; + _jumpController.add(position); + } + + @override + Future dispose({bool preserveDisplayMode = false}) async { + await _jumpController.close(); } @override