From b2ca85172cf17f45d57f212bb085d718288e1488 Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:36:54 +0200 Subject: [PATCH] fix(playback): seek Embedded MPV steps relative to mpv's own position (#1518) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(playback): seek Embedded MPV steps relative to mpv's own position Arrow keys and the ±10 s buttons in the Embedded MPV player advanced only about a second per press when pressed repeatedly or held. The shortcuts already asked for 5 s steps, but `EmbeddedMpvCommandRunner.seekBy` turned each step into an absolute `seek` computed from `session.positionSeconds`, which is floored to whole seconds, polled every 500 ms (helper snapshots at most every 250 ms) and not refreshed by the seek reply. Every press inside that window therefore landed on the same target. Steps now go through a new `EMBEDDED_MPV_SEEK_BY` IPC / `seekEmbeddedMpvBy` bridge method that every backend forwards as mpv `seek relative+exact`: `seekBy` exports in the macOS addon and the Windows/Linux `wid` addon (Linux over its JSON IPC socket), and a `seek-by` stdin command in the frame-copy helper. mpv resolves the delta against its own position and merges queued relative seeks, so presses accumulate as in mpv itself. The absolute form survives only as a fallback for a preload without the method or an addon binary without `seekBy`; the timeline scrub still commits an absolute target. Validated with a real mpv 0.39 IPC probe: three relative seeks in a burst advance +15 s, three absolute seeks from one stale base advance +5 s. Co-Authored-By: Claude Fable 5.1 * fix(playback): drop speculative position update from relative Embedded MPV seeks Review follow-up for the relative seek path. The macOS and Windows/Linux `seekBy` exports advanced `snapshot.positionSeconds` by the delta after dispatching the mpv command. That is not idempotent the way the absolute seek's optimistic write is: the observer (mpv event thread, or the Linux IPC poll) can already have stored the post-seek `time-pos` under the same mutex, so adding the delta on top counted the step twice, and while paused nothing corrected it. On Linux it also advertised a position that a failed socket delivery never reached. Relative steps now leave the snapshot alone; only the observed `time-pos` updates the position. The packaged Linux frame-copy smoke now drives `seekEmbeddedMpvBy` through the built app: a burst of three +2 s steps issued without waiting for snapshots has to land on 6 s, and a -60 s step has to clamp at 0. The generated Y4M fixture grows from 2 s to 12 s (about 415 KB) so the burst and the playing section that follows stay inside the clip. Replayed against a local mpv 0.39 with the same fixture and media server: burst -> 6.0, -60 -> 0. Co-Authored-By: Claude Fable 5.1 * docs(agents): mirror the Embedded MPV relative-seek contract into AGENTS.md Review follow-up: the Shared Player Controls section documents the frame-copy commands and shortcuts, so the relative seekEmbeddedMpvBy invariant lives there too, next to the CLAUDE.md note. Co-Authored-By: Claude Fable 5.1 * fix(playback): reject a Linux relative seek the mpv IPC socket did not accept Review follow-up: the Linux branch of SeekBy discarded the socket transaction result and returned normally, so a step that never reached mpv looked like a seek still awaiting observation. It now throws like a failed mpv_command_async on the in-process engines; the renderer swallows the rejection and resyncs from the next snapshot, and the main process logs it. Co-Authored-By: Claude Fable 5.1 --------- Co-authored-by: Claude Fable 5.1 --- .changes/playback-embedded-mpv-seek-steps.md | 10 +++ AGENTS.md | 10 +++ CLAUDE.md | 2 +- ...bedded-mpv-frame-copy-packaged-fixtures.ts | 14 +++- .../embedded-mpv-frame-copy-packaged.e2e.ts | 55 ++++++++++++++- .../native/helper/mpv_frame_helper.cpp | 11 +++ .../native/src/embedded_mpv.mm | 45 +++++++++++++ .../native/src/embedded_mpv_wid_common.h | 67 +++++++++++++++++++ .../src/app/api/main.preload.ts | 5 ++ .../src/app/events/embedded-mpv.events.ts | 7 ++ .../embedded-mpv-frame-copy.adapter.spec.ts | 13 ++++ .../embedded-mpv-frame-copy.adapter.ts | 4 ++ .../embedded-mpv-native.service.spec.ts | 41 ++++++++++++ .../services/embedded-mpv-native.service.ts | 31 +++++++++ docs/architecture/embedded-mpv-native.md | 4 +- docs/architecture/player-controls-contract.md | 4 +- .../src/lib/electron-api.interface.ts | 13 ++++ .../shared/interfaces/src/lib/ipc-commands.ts | 1 + .../embedded-mpv-command-runner.spec.ts | 33 ++++++++- .../embedded-mpv-command-runner.ts | 21 +++++- .../embedded-mpv-session-controller.spec.ts | 12 +++- 21 files changed, 392 insertions(+), 11 deletions(-) create mode 100644 .changes/playback-embedded-mpv-seek-steps.md diff --git a/.changes/playback-embedded-mpv-seek-steps.md b/.changes/playback-embedded-mpv-seek-steps.md new file mode 100644 index 000000000..d8dd4bb7e --- /dev/null +++ b/.changes/playback-embedded-mpv-seek-steps.md @@ -0,0 +1,10 @@ +--- +type: fix +area: playback +--- + +Arrow keys and the ±10 s buttons in the Embedded MPV player now move by their +full step every time. Pressing an arrow repeatedly, or holding it, used to +advance only about a second per press because each step was computed from a +stale position; steps are now relative seeks executed by mpv itself, so rapid +presses add up. diff --git a/AGENTS.md b/AGENTS.md index ba683bc87..dc8d0321c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -289,6 +289,16 @@ Key files: commands are cancelled. Same-session IPC replies also yield to a broadcast snapshot received while the command was pending, preventing a successful recording acknowledgement from being rolled back by a stale reply. +- Embedded MPV seek steps (arrow keys, ±10 s buttons, `PlayerController.seekBy`) + go through the relative `seekEmbeddedMpvBy` IPC: every backend forwards the + delta as mpv `seek relative+exact` (addon export `seekBy`, helper + stdin command `seek-by`, Linux JSON IPC) and never advances the snapshot + position itself. Do not derive an absolute target from the renderer's + `positionSeconds`: it is floored to whole seconds, polled every 500 ms, and + a seek reply does not carry the new position, so rapid presses computed from + it collapse onto one target. Only the timeline scrub commits an absolute + `seek`. Contract: `docs/architecture/embedded-mpv-native.md` ("Resume And + Track Handling"). - DASH (`.mpd`) sources play through a lazily imported Shaka Player source engine (`libs/ui/playback/src/lib/shaka-engine/`) inside the HTML5 and ArtPlayer components; ClearKey keys come from KODIPROP-derived diff --git a/CLAUDE.md b/CLAUDE.md index 8b41b312b..02661c4aa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1008,7 +1008,7 @@ app as a real argument, so it is not an option. API. Radio's `