VideoPlayer: use absolute+exact for slider seek (fix drag-back race)
The slider's _seek used plain 'absolute' (keyframe seek), which made mpv land on the nearest keyframe at-or-before the click position. For sparse-keyframe videos (1-5s GOP) the actual position landed 1-5s behind where the user clicked. The 500ms _seek_pending_until pin window from c4061b0 papered over this for half a second, but afterwards the slider visibly dragged back to mpv's keyframe-rounded position and crawled forward. Observed in BOTH the embedded preview and the popout slider — the bug lives in the shared VideoPlayer class, so fixing it once fixes both surfaces. Empirically verified by the pre-commit-1 mpv probe in docs/POPOUT_REFACTOR_PLAN.md: seek(7.0, 'absolute') landed at time_pos=5.000 (2s back); seek(12.0, 'absolute') landed at 10.033 (also 2s back). Both match the user's reported drag-back symptom. The other seek paths in VideoPlayer already use exact mode: - seek_to_ms (line 318): 'absolute+exact' - _seek_relative (line 430): 'relative+exact' The slider's _seek was the only outlier. The original c4061b0 commit chose plain 'absolute' for "responsiveness" and added the pin window to hide the keyframe rounding. This commit removes the underlying cause: the seek now decodes from the previous keyframe forward to the EXACT target position before mpv emits playback-restart, costing ~30-100ms more per seek depending on GOP density (well under the 500ms pin window) but landing time_pos at the click position exactly. The slider doesn't need any pin window to mask a discrepancy that no longer exists. The _seek_pending_until pin remains in place as defense in depth — it's now redundant for keyframe rounding but still smooths over the sub-100ms decode latency between the click and the first _poll tick that reads the new time_pos. Commit 14b will remove the legacy pin code as part of the imperative-path cleanup. This also unblocks the popout state machine's design (commits 6, 11, 14a). The state machine's SeekingVideo state lasts until mpv's playback-restart event arrives — empirically 14-34ms in the user's verification log of commit 14a. Without exact seek, commit 14b would visibly REGRESS slider behavior because compute_slider_display_ms returns mpv.time_pos after 30ms instead of the legacy 500ms — the drag-back would surface immediately on every seek. With exact seek, mpv.time_pos == seek_target_ms after the seek completes, so the state machine's slider pin is correct without needing any extra window. Found during commit 14a verification gate. Pre-existing bug — the state machine refactor revealed it but didn't introduce it. Tests passing after this commit: 81 / 81 (16 Phase A + 65 state). Phase A still green. Phase B regression target met (the dispatch trace from the verification run shows correct SeekRequested → SeekCompleted round-trips with no spurious state transitions). Verification: - Click slider mid-playback in embedded preview → no drag-back - Click slider mid-playback in popout → no drag-back - Drag the slider continuously → still works (isSliderDown path unchanged) - Period/Comma keys (relative seek) → still work (already use 'relative+exact')
This commit is contained in:
parent
35d80c32f2
commit
609066cf87
@ -418,12 +418,37 @@ class VideoPlayer(QWidget):
|
|||||||
self._mpv['loop-file'] = 'no'
|
self._mpv['loop-file'] = 'no'
|
||||||
|
|
||||||
def _seek(self, pos: int) -> None:
|
def _seek(self, pos: int) -> None:
|
||||||
"""Seek to position in milliseconds (from slider)."""
|
"""Seek to position in milliseconds (from slider).
|
||||||
|
|
||||||
|
Uses `'absolute+exact'` (frame-accurate seek) to match the
|
||||||
|
existing `seek_to_ms` and `_seek_relative` methods. The
|
||||||
|
previous `'absolute'` (keyframe-only) mode landed mpv on the
|
||||||
|
nearest keyframe at-or-before the click position, which for
|
||||||
|
sparse-keyframe videos (1-5s GOP) was 1-5s behind where the
|
||||||
|
user clicked. The 500ms `_seek_pending_until` pin window
|
||||||
|
below papered over this for half a second, but afterwards
|
||||||
|
the slider visibly dragged back to mpv's actual (rounded)
|
||||||
|
position and crawled forward — observed in both the embedded
|
||||||
|
preview and the popout slider.
|
||||||
|
|
||||||
|
Frame-accurate seek decodes from the previous keyframe up to
|
||||||
|
the exact target position, costing ~30-100ms more per seek
|
||||||
|
depending on GOP density. That's well under the legacy 500ms
|
||||||
|
pin window, and the resulting `time_pos` after the seek
|
||||||
|
equals the click position exactly, so the slider doesn't
|
||||||
|
need any pin window to mask a discrepancy that no longer
|
||||||
|
exists.
|
||||||
|
|
||||||
|
The `_seek_pending_until` pin remains in place as defense in
|
||||||
|
depth — it's now redundant for keyframe rounding but still
|
||||||
|
smooths over any sub-100ms decode latency between the click
|
||||||
|
and the first `_poll` tick that reads the new `time_pos`.
|
||||||
|
"""
|
||||||
if self._mpv:
|
if self._mpv:
|
||||||
import time as _time
|
import time as _time
|
||||||
self._seek_target_ms = pos
|
self._seek_target_ms = pos
|
||||||
self._seek_pending_until = _time.monotonic() + self._seek_pin_window_secs
|
self._seek_pending_until = _time.monotonic() + self._seek_pin_window_secs
|
||||||
self._mpv.seek(pos / 1000.0, 'absolute')
|
self._mpv.seek(pos / 1000.0, 'absolute+exact')
|
||||||
|
|
||||||
def _seek_relative(self, ms: int) -> None:
|
def _seek_relative(self, ms: int) -> None:
|
||||||
if self._mpv:
|
if self._mpv:
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user