Skip to content

Fix mpv pacing stalls on high-latency links - #26

Closed
pcarrier wants to merge 2 commits into
mainfrom
indent-2026-04-17-pacing-stall
Closed

pcarrier wants to merge 2 commits into
mainfrom
indent-2026-04-17-pacing-stall

Conversation

@pcarrier

Copy link
Copy Markdown
Contributor

Summary

  • Drop the bandwidth-derived fps cap from surface_pacing_fps — use the client's display_fps directly. Flow control is already provided by surface_inflight_limit (RTT×display_fps), outbox_backpressured, and surface_encodes_in_flight.
  • Flush the VideoDecoder before re-configuring it on H.264 SPS codec-string changes, so queued VideoFrames drain through the output callback (which closes them) instead of being orphaned.

Motivation
mpv @ 24 fps on a 120 Hz display over a <200 ms link stalls after a few seconds. Two interacting problems:

  1. Pacing underruns during bootstrap. surface_pacing_fps = max(goodput_bps, delivery_bps) / avg_surface_frame_bytes uses a conservative 262 KB/s goodput seed. On the first keyframe the frame-size estimate jumps to ~25 KB (keyframe/4 heuristic), giving 262144/25600 ≈ 10 fps — below the 24 fps source. Because goodput_bps only updates from ACKs that have traversed the RTT, it takes several hundred ms to converge on a high-latency link, long enough for mpv's Wayland frame-callback clock to stall. The inflight window + outbox backpressure already handle genuinely slow links without this cap.

  2. Orphaned VideoFrames on reconfigure. When an H.264 SPS with a different profile/compat/level arrives, entry.decoder.configure(...) is called directly. Per WebCodecs spec, configure() on an already-configured decoder resets its internal state, dropping any queued VideoFrames without running the output callback — they are eventually GC'd and Chromium logs "A VideoFrame was garbage collected without being closed. Applications should call close() on frames when done with them to prevent stalls." With enough orphans the decoder's frame pool is exhausted and decode stalls. Queueing flush() before configure() lets the decoder drain through the output callback (which already calls frame.close()); since WebCodecs processes control messages in order, the subsequent configure() and decode() of the incoming keyframe naturally run after the flush.

Testing

  • npx tsc --noEmit in js/core passes.
  • Rust change is a narrow function-body replacement; no new types or call-site changes. (Rust toolchain not available in this sandbox — please verify CI.)
  • Manual repro: mpv @ 24 fps on 120 Hz client over a 200 ms RTT link should sustain 24 fps steadily without the GC warning.

Open in Indent
Tag @indent to continue the conversation here.

pcarrier and others added 2 commits April 16, 2026 16:32
…d 4:4:4 chroma

Generated with [Indent](https://indent.com)
Co-Authored-By: Indent <noreply@indent.com>
Two issues caused mpv @ 24 fps to stall on a 120 Hz display over a
<200 ms link:

1. Server-side bandwidth cap starved pacing during bootstrap.
   surface_pacing_fps divided goodput_bps (seeded at 262 KB/s) by the
   first keyframe's frame-size estimate, yielding ~10 fps — well below
   the source rate — until the EWMA converged hundreds of ms later.
   Drop the bandwidth cap: inflight-window + outbox backpressure
   already handle genuinely slow links without this conservative gate.

2. Client-side decoder reconfigure orphaned in-flight VideoFrames.
   When an H.264 SPS changed codec string, VideoDecoder.configure() was
   called while frames were still queued, orphaning them — Chromium
   logs "VideoFrame was garbage collected without being closed" and
   eventually exhausts its frame pool. Queue a flush() before the
   reconfigure so output callbacks close pending frames first.

Generated with [Indent](https://indent.com)
Co-Authored-By: Indent <noreply@indent.com>
@pcarrier pcarrier added the indent label Apr 17, 2026 — with indent-zero
@github-actions

Copy link
Copy Markdown

Coverage

Crate Lines Functions Regions
alacritty-driver 59.7% (529/886) 62.5% (40/64) 59.8% (754/1261)
browser 0.0% (0/807) 0.0% (0/65) 0.0% (0/1370)
cli 34.2% (1072/3131) 44.4% (152/342) 36.0% (1929/5358)
compositor 1.1% (93/8153) 2.2% (8/368) 1.3% (146/10861)
fonts 76.8% (486/633) 85.5% (47/55) 77.9% (922/1183)
gateway 25.3% (355/1405) 27.6% (35/127) 18.8% (442/2351)
proxy 18.3% (150/818) 20.9% (24/115) 20.4% (260/1277)
remote 71.7% (1940/2706) 81.9% (186/227) 74.4% (3674/4938)
server 18.2% (1904/10459) 32.9% (215/654) 19.6% (3198/16339)
ssh 1.9% (7/374) 3.2% (1/31) 0.7% (4/613)
webrtc-forwarder 0.6% (15/2555) 0.5% (1/185) 0.8% (33/4300)
webserver 54.1% (442/817) 64.7% (75/116) 58.6% (839/1432)
Total 21.4% (6993/32744) 33.4% (784/2349) 23.8% (12201/51283)

@github-actions

Copy link
Copy Markdown

🔗 Preview: https://blit-il0hv9crf-indent.vercel.app

@indent-zero
indent-zero Bot changed the base branch from indent-2026-04-15-dynamic-musl-dlopen to main April 17, 2026 11:33
@indent-zero

indent-zero Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Folding into #20 — single PR for everything.

@indent-zero indent-zero Bot closed this Apr 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant