fix(playback): seek Embedded MPV steps relative to mpv's own position (#1518)

* 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 <delta>
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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Fable 5.1 authored and GitHub committed 2026-09-04 17:36:54 +02:00
1 parent ad81fbfc45
commit b2ca85172c
21 files changed
+392 -11

No files matched your search

@@ -27,11 +27,19 @@ export type LocalMediaServer = {
url: string;
};
function createTwoSecondY4mFixture(): Buffer {
/**
* Length of the generated Y4M clip. Long enough for the relative-seek burst
* in the packaged smoke (three +2 s steps land at 6 s) plus the playing
* section that follows it without hitting EOF; at 64x36 / 10 fps this is
* still well under half a megabyte.
*/
export const Y4M_FIXTURE_DURATION_SECONDS = 12;
function createY4mFixture(): Buffer {
const width = 64;
const height = 36;
const framesPerSecond = 10;
const frameCount = framesPerSecond * 2;
const frameCount = framesPerSecond * Y4M_FIXTURE_DURATION_SECONDS;
const yPlaneBytes = width * height;
const chromaPlaneBytes = (width / 2) * (height / 2);
const chunks: Buffer[] = [
@@ -84,7 +92,7 @@ async function closeServer(server: Server): Promise<void> {
}
export async function createLocalMediaServer(): Promise<LocalMediaServer> {
const body = createTwoSecondY4mFixture();
const body = createY4mFixture();
const resourcePath = '/embedded-mpv-frame-copy-smoke.y4m';
const server = createServer((request, response) => {
const pathname = (request.url ?? '').split('?')[0];
@@ -134,7 +134,7 @@ test.describe('Packaged Linux embedded MPV frame-copy runtime', () => {
await window.electron.setEmbeddedMpvPaused(sessionId, true);
await window.electron.loadEmbeddedMpvPlayback(sessionId, {
streamUrl,
title: 'Two-second generated Y4M fixture',
title: 'Generated Y4M fixture',
isLive: false,
});
},
@@ -180,6 +180,59 @@ test.describe('Packaged Linux embedded MPV frame-copy runtime', () => {
})
.toBeGreaterThan(0);
// Relative seek steps (arrow keys, ±10 s buttons) must reach mpv
// as `seek <delta> relative+exact` through the real preload →
// main-process → helper `seek-by` path, and a burst issued without
// waiting for snapshots has to accumulate. Absolute targets
// computed from the stale renderer snapshot collapsed such bursts
// onto one target (about one second of progress per press).
await launchedFrameCopyApp.mainWindow.evaluate(
async (sessionId) => {
const seekBy = window.electron.seekEmbeddedMpvBy;
if (!seekBy) {
throw new Error(
'seekEmbeddedMpvBy is missing from the preload bridge'
);
}
await Promise.all([
seekBy(sessionId, 2),
seekBy(sessionId, 2),
seekBy(sessionId, 2),
]);
},
created.id
);
await expect
.poll(
async () =>
(
await getLatestSession(
launchedFrameCopyApp,
created.id
)
)?.positionSeconds,
{ timeout: 15000 }
)
.toBe(6);
// Stepping back past the start clamps at zero instead of failing.
await launchedFrameCopyApp.mainWindow.evaluate(
(sessionId) =>
window.electron.seekEmbeddedMpvBy?.(sessionId, -60),
created.id
);
await expect
.poll(
async () =>
(
await getLatestSession(
launchedFrameCopyApp,
created.id
)
)?.positionSeconds,
{ timeout: 15000 }
)
.toBe(0);
await launchedFrameCopyApp.mainWindow.evaluate(
(sessionId) =>
window.electron.setEmbeddedMpvPaused(sessionId, false),