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