diff --git a/PLAN.md b/PLAN.md index 8d4e0c3..0c8f488 100644 --- a/PLAN.md +++ b/PLAN.md @@ -59,10 +59,13 @@ playback continues locally and the event is logged; clients do not reconnect. 6. Any connected client receives the event and sends the corresponding JSON IPC command to its local MPV instance. -Remote seeks use a serialized, acknowledged transaction: disable delivery of -the `seek` event for the IPC connection, apply an `absolute+exact` seek, then -re-enable the event even when the seek fails. This prevents remote seeks from -being reported as new local seeks and looping through the server. +Remote seeks use a serialized transaction: disable delivery of the `seek` +event for the IPC connection, apply an `absolute+exact` seek, wait for mpv's +`playback-restart` completion event, then re-enable the `seek` event even when +the operation fails. A successful command reply alone is not a completion +barrier because mpv can deliver seek effects afterward. Keeping delivery +disabled through completion prevents remote seeks from being reported as new +local seeks and looping through the server. **Example cli command to run MPV** diff --git a/README.md b/README.md index 56e84b8..ea75f2f 100644 --- a/README.md +++ b/README.md @@ -71,10 +71,13 @@ triggering ID to relayed events. Server and client terminals log play, pause, and seek actions with that ID; the triggering client is marked with `(you)` in its own terminal. -Remote seeks are applied using an acknowledged mpv IPC transaction: seek event -delivery is disabled for this IPC connection, an exact absolute seek is sent, -and event delivery is restored even if the seek fails. This prevents a remote -seek from being mistaken for a new local seek and sent back indefinitely. +Remote seeks are applied using an mpv IPC transaction: seek event delivery is +disabled for this IPC connection, an exact absolute seek is sent, and the +client waits for mpv's `playback-restart` completion event before restoring +seek event delivery. A command reply can arrive before the seek effects, so it +is not sufficient as the end of the suppression window. Event delivery is +still restored if the operation fails. This prevents a remote seek from being +mistaken for a new local seek and sent back indefinitely. ## Prototype limitations diff --git a/SEEK.md b/SEEK.md new file mode 100644 index 0000000..c5e6893 --- /dev/null +++ b/SEEK.md @@ -0,0 +1,104 @@ +# seek issue + +When seeking (when video is paused, or being played) the video stutters and has clearly some feedback loop between clients. +Analyze the code and find a fix. + +# client output + +```sh +client: client 1 sought to 1766.848 seconds +client: client 2 (you) sought to 1766.848 seconds +client: client 1 sought to 1766.848 seconds +client: client 2 (you) sought to 1766.848 seconds +client: client 1 sought to 1766.848 seconds +client: client 2 (you) sought to 1766.848 seconds +client: client 1 sought to 1766.848 seconds +client: client 2 (you) sought to 1766.848 seconds +client: client 1 sought to 1766.848 seconds +client: client 2 (you) sought to 1766.848 seconds +client: client 1 sought to 1766.848 seconds +client: client 2 (you) sought to 1766.848 seconds +client: client 1 sought to 1766.848 seconds +client: client 2 (you) sought to 1766.848 seconds +client: client 1 sought to 1766.848 seconds +client: client 2 (you) sought to 1766.848 seconds +client: client 1 sought to 1766.848 seconds +client: client 2 (you) sought to 1766.848 seconds +client: client 1 sought to 1766.848 seconds +client: client 2 (you) sought to 1766.848 seconds +client: client 1 sought to 1766.848 seconds +client: client 2 (you) sought to 1766.848 seconds +client: client 2 (you) played +client: client 1 sought to 1766.848 seconds +client: client 2 (you) sought to 1766.890 seconds +client: client 1 sought to 1766.932 seconds +client: client 2 (you) sought to 1766.974 seconds +client: client 1 sought to 1767.015 seconds +client: client 2 (you) sought to 1767.057 seconds +client: client 1 sought to 1767.099 seconds +client: client 2 (you) sought to 1767.140 seconds +client: client 1 sought to 1767.182 seconds +client: client 2 (you) sought to 1767.224 seconds +client: client 1 sought to 1767.266 seconds +client: client 2 (you) sought to 1767.307 seconds +client: client 1 sought to 1767.349 seconds +client: client 2 (you) paused +client: client 2 (you) played +client: client 2 (you) paused +client: client 2 (you) played +client: client 2 (you) paused +client: mpv IPC connection closed +client: server connection lost (mpv exited); playback will continue +client: mpv exited with status 15 +``` + +# server output + +```sh +server: client 2 sought to 1766.848 seconds +server: client 1 sought to 1766.848 seconds +server: client 2 sought to 1766.848 seconds +server: client 1 sought to 1766.848 seconds +server: client 2 sought to 1766.848 seconds +server: client 1 sought to 1766.848 seconds +server: client 2 sought to 1766.848 seconds +server: client 1 sought to 1766.848 seconds +server: client 2 sought to 1766.848 seconds +server: client 1 sought to 1766.848 seconds +server: client 2 sought to 1766.848 seconds +server: client 1 sought to 1766.848 seconds +server: client 2 sought to 1766.848 seconds +server: client 1 sought to 1766.848 seconds +server: client 2 sought to 1766.848 seconds +server: client 1 sought to 1766.848 seconds +server: client 2 sought to 1766.848 seconds +server: client 1 sought to 1766.848 seconds +server: client 2 sought to 1766.848 seconds +server: client 1 sought to 1766.848 seconds +server: client 2 sought to 1766.848 seconds +server: client 1 sought to 1766.848 seconds +server: client 2 sought to 1766.848 seconds +server: client 1 sought to 1766.848 seconds +server: client 2 sought to 1766.848 seconds +server: client 2 played +server: client 1 sought to 1766.848 seconds +server: client 2 sought to 1766.890 seconds +server: client 1 sought to 1766.932 seconds +server: client 2 sought to 1766.974 seconds +server: client 1 sought to 1767.015 seconds +server: client 2 sought to 1767.057 seconds +server: client 1 sought to 1767.099 seconds +server: client 2 sought to 1767.140 seconds +server: client 1 sought to 1767.182 seconds +server: client 2 sought to 1767.224 seconds +server: client 1 sought to 1767.266 seconds +server: client 2 sought to 1767.307 seconds +server: client 1 sought to 1767.349 seconds +server: client 2 paused +server: client 2 played +server: client 2 paused +server: client 2 played +server: client 2 paused +server: client 2 disconnected (connection closed) +server: client 1 disconnected (connection closed) +``` diff --git a/mpv.odin b/mpv.odin index 9f2900e..03f0a67 100644 --- a/mpv.odin +++ b/mpv.odin @@ -26,6 +26,8 @@ Mpv_Connection :: struct { pending_id: i64, pending_done: bool, pending_result: Mpv_Command_Result, + remote_seek_waiting: bool, + remote_seek_completed: bool, closed: bool, client: ^Client_State, @@ -136,10 +138,35 @@ mpv_apply_remote_seek :: proc(mpv: ^Mpv_Connection, position: f64) -> bool { seek_command := fmt.aprintf(`["seek",%.6f,"absolute+exact"]`, position) defer delete(seek_command) + sync.mutex_lock(&mpv.response_mutex) + mpv.remote_seek_waiting = true + mpv.remote_seek_completed = false + sync.mutex_unlock(&mpv.response_mutex) seek_result := mpv_command_locked(mpv, seek_command) + + seek_completed := false + sync.mutex_lock(&mpv.response_mutex) + if seek_result.success { + deadline_remaining := MPV_COMMAND_TIMEOUT + for !mpv.remote_seek_completed && !mpv.closed { + start := time.now() + if !sync.cond_wait_with_timeout(&mpv.response_cond, &mpv.response_mutex, deadline_remaining) { + break + } + elapsed := time.since(start) + if elapsed >= deadline_remaining { + break + } + deadline_remaining -= elapsed + } + seek_completed = mpv.remote_seek_completed + } + mpv.remote_seek_waiting = false + sync.mutex_unlock(&mpv.response_mutex) + enable_result := mpv_command_locked(mpv, `["enable_event","seek"]`) reenabled = enable_result.success - return seek_result.success && enable_result.success + return seek_result.success && seek_completed && enable_result.success } mpv_set_pause :: proc(mpv: ^Mpv_Connection, paused: bool) -> bool { @@ -223,6 +250,14 @@ mpv_handle_line :: proc(mpv: ^Mpv_Connection, line: string) { if !event_ok { return } + if string(event_name) == "playback-restart" { + sync.mutex_lock(&mpv.response_mutex) + if mpv.remote_seek_waiting { + mpv.remote_seek_completed = true + sync.cond_broadcast(&mpv.response_cond) + } + sync.mutex_unlock(&mpv.response_mutex) + } if mpv.client != nil { client_on_mpv_event(mpv.client, string(event_name), object) } diff --git a/mpv_test.odin b/mpv_test.odin index 973684b..2a563d1 100644 --- a/mpv_test.odin +++ b/mpv_test.odin @@ -11,6 +11,7 @@ Fake_Mpv :: struct { fd: posix.FD, received: [3]string, fail_seek: bool, + enable_before_restart: bool, } fake_mpv_read_line :: proc(fd: posix.FD, buffer: []byte) -> (line: string, ok: bool) { @@ -46,6 +47,14 @@ fake_mpv_loop :: proc(data: rawptr) { ) _ = mpv_send_all(fake.fd, response) delete(response) + if index == 1 && error_name == "success" { + poll_fd := posix.pollfd { + fd = fake.fd, + events = {.IN}, + } + fake.enable_before_restart = posix.poll(&poll_fd, 1, 50) > 0 + _ = mpv_send_all(fake.fd, "{\"event\":\"playback-restart\"}\n") + } } } @@ -83,6 +92,7 @@ remote_seek_masks_and_restores_seek_events :: proc(t: ^testing.T) { testing.expect(t, strings.contains(fake.received[0], `"disable_event","seek"`)) testing.expect(t, strings.contains(fake.received[1], `"seek",42.500000,"absolute+exact"`)) testing.expect(t, strings.contains(fake.received[2], `"enable_event","seek"`)) + testing.expect(t, !fake.enable_before_restart) for line in fake.received { delete(line) }