mediad: fresh snapshots through robotctl and console HTTP - #241
Conversation
`npu-bringup.md` names this: "a call that answers with one frame. Useful for far more than perception (a snapshot in the console, a still for a bug report), and it makes capture stop fighting the daemon." A frame stays off the WebRTC control channel. At the default geometry the UYVY payload is ~1.8 MiB, so JSON/base64 would make a control request several MiB and let a slow peer tie camera data to the network. The socket answers one JSON-RPC header naming a byte count, then precisely those bytes. It asks the tee for the next frame rather than taking a cached one. `Frames` is a rendezvous, not a cache — the capture branch copies a buffer only when a reader has asked for one — so a caller waits for the capture that answers it, bounded by FRAME_TIMEOUT, and a camera that has stopped is reported as a timeout rather than answered with the frame it stopped on. `next_frame` parks on a condvar, so it runs on a blocking task rather than the runtime thread. The socket is handed to the `robot` group after binding, the same way `tof` and `padd` hand over theirs, with a missing group a warning rather than a failure: mode 0660 alone would leave it `mediad:mediad`, and the operator is only ever added to `robot`. The request read is bounded before the line is buffered rather than checked after, so a client that never sends a newline cannot make this process hold an unbounded string. `Frame` gains `captured_at`, which is what makes a snapshot joinable to a separately sampled robot state. It is observation time, never used to pace capture. Under the demand-driven tee it costs one clock read on a frame someone already asked for. `architecture.md`'s service table said `mediad` had no unix socket of its own. Giving it one is a deliberate change to the service split, so that is updated here. Five tests cover the timeout, an unknown method (which must not leave demand behind), an oversized request, a request with no newline at all, and a delivered frame whose header names exactly the bytes that follow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RP691H7VvDPD8yQvTgEW7G
pierre-rouanet
left a comment
There was a problem hiding this comment.
The four fixes are all there, and the parts I went and checked hold up:
wire_framesis wired without_width/out_height(pipeline.rs:606), so the header names post-rotation geometry rather than the sensor's.Framesalready wakes every waiter off one delivery, so concurrent callers cost one capture between them rather than one each.mainis#[cfg(target_os = "linux")], so the unconditionalframe::servespawn compiles.mediad.servicehasRuntimeDirectory=mediad, default mode 0755, so/run/mediad/is traversable byrobotand systemd clears the socket on stop. The group handoff is a faithful mirror oftof's.
On the contract question you asked: keep the timeout, don't reintroduce the cache. The argument is already written into Frames' own doc comment — a reader handed "here is a frame" cannot tell a live one from the one a stopped camera stopped on — and a cache here would contradict the module it reads from.
hello is refused, and the connection is dropped with it
This is the one I'd like fixed before merge, because it means nothing we ship can call this endpoint.
robotctl's Connection sends Call::Hello as the first thing on every command path — eighteen call sites, all client.hello()? — and hello_result turns an error response into a Failure and stops (robotctl/src/main.rs:1314). So the "snapshot in the console, a still for a bug report" this endpoint exists for cannot be built with the house client as things stand.
Note the difference from the daemon it says it mirrors: tof's subscriber answers an unknown method and keeps the connection — "so a client that spells a method wrong is told rather than dropped" — while handle returns after writing the error. Either answer hello the way robotd and configd do, or at minimum loop instead of closing.
media.frame is the first request method with no Call variant
Every other bare method:: constant is a notification — PROGRESS, TOF_FRAME, PAD_REPORT, CHORALE_BEACON. This one is a call, dispatched by a request.method != proto::method::MEDIA_FRAME string compare.
That may well be right: a Call::MediaFrame would force Service::Media into dial() and make mediad/src/route.rs::permits take a position on shipping 1.8 MiB down a datachannel. But then it should say so next to the constant, the way Call::SystemAuthenticate's return None arm explains itself. As it stands, "deliberately unroutable" and "nobody got round to it" read identically.
Fifth copy of give_to_group
configd, updater, padd, tof, now mediad — byte-identical down to the SAFETY comments. Pre-existing, and you were asked to follow the pattern, so not this PR's fault. Five is where it stops being a pattern and starts being a function.
Smaller
read_lineerrors on invalid UTF-8, and that?closes the connection with no reply — inconsistent with thePARSE_ERRORpath three lines below.read_until(b'\n')plus a lossy conversion would fold it into the same answer.- No read deadline. A client that connects and sends nothing parks a task and an fd for the life of the daemon; the byte cap bounds memory, not time. Nothing bounds concurrent connections either.
if socket.exists() { remove_file }will unlink a live socket or a regular file.robotd::claim_socketis the careful version —is_socket(), then probe forConnectionRefused;padd/tofare the crude one this copies. Fine under systemd, wrong the day twomediads coexist.- The socket path is hardcoded while every other socket
mediadtouches has a--*-socketoverride, and a bind failure is only anerror!line, so nothing inrobotctl healthwill say the frame endpoint is absent.
Merge state
Conflicts with main, but only Cargo.lock, one hunk: main added hf-robot-account and image to mediad's dep list, this adds libc. Keep all three, alphabetical. Everything else merges clean and cargo metadata --locked is happy afterwards.
|
Separately from the review: there is no way to use this yet, and I think that is worth closing before it lands rather than after. I went to test it on a board and the recipe is a scratch Python client that opens Four surfaces suggest themselves and they do not come as a set. What splits them is the 1.8 MiB.
The WebRTC datachannel — no. It is precisely what this PR's design rejects, and it is redundant besides: a peer holding a session already has the frames, as a video track. The one thing that track cannot give is raw pre-encoder pixels — it is H.264, lossy, post-RGA-conversion, bitrate-limited — and that gap is real for anything measurement-shaped, camera calibration especially. But the HTTP route serves that better than the control channel would, without putting a megabyte in front of
So: |
Support hello on the local frame socket, bound clients and reads, and claim configurable sockets without unlinking live listeners. Validate binary replies and expose uncached PNG snapshots on the console. Assisted-by: OpenAI Codex
|
Added both entry points in 291bd90:
The local endpoint now keeps the connection after The method constant and usage docs explicitly keep binary frames out of service/lane routing, the WebRTC control channel and current BLE Linux CI on the same head passed The upstream workflow is currently waiting for maintainer approval; the linked results are from the existing fork validation PR. |
A frame off the tee is the picture the sensor took, and this robot's camera is mounted a quarter turn off — so a snapshot came out sideways with nothing in the reply to say by how much. The geometry cannot recover it: a 180° mount is indistinguishable from an upright one, and a quarter turn is only a guess from the aspect ratio. `MediaFrameHeader` now carries `rotate`, degrees clockwise from upright, the same number `media.video` already tells a WebRTC peer — and zero when `--flip-in-pipeline` turned the pixels, the rule the detector's sampler and the JPEG streamer already follow. `valid_uyvy` refuses anything that is not a quarter turn, so a nonsense angle is rejected rather than silently ignored. The console's `/frame` applies it instead of reporting it. That route is the one consumer with nowhere to put an angle: a PNG opened in a browser carries no metadata a viewer will act on, so reporting it there would hand every human a sideways picture and no way to know why. It costs one rotation per request on a blocking thread, not one per frame in front of the encoder, which is what made `videoflip` expensive. The recipe in npu-bringup.md grew the `-vf transpose=1` it was missing; it produced a sideways image and said nothing about it. Assisted-by: Claude:claude-opus-5
|
Pushed 7856145: the mount angle now travels in Found it by running the documented recipe against a real camera on a dev board.
The console's
One caveat for anyone already running this branch on a dev board: a macOS: Still untested by anything, and unchanged by this commit: the socket's |
The merge that gave `handle` a `rotate` argument updated the call sites that ask for a frame but not the two that never get that far, so `mediad` failed to compile as a test target. Assisted-by: Claude:claude-opus-5
Adds fresh camera snapshots without stopping
mediad:robotctl frame --output frame.uyvysaves raw pixels locally, andGET http://<robot>:8080/framereturns a PNG from the existing console. This is themedia.frameportion of #214.Behavior
hello, retains the connection after an unknown method, then serves a JSON-RPC metadata header followed by exactly the declared raw UYVY bytes. Metadata includes post-rotation geometry and the capture timestamp.Frames' existing demand-driven rendezvous and 500 ms capture timeout. A stopped camera returns an error, never a cached image. Blocking capture waits stay off the async runtime thread.robotctl frameperforms the normal handshake, validates geometry and size, and writes the output only after receiving a complete frame. The metadata is printed to stderr./frameconverts the raw pixels to RGB and PNG, setsCache-Control: no-store, and returns 503 when capture is unavailable or capacity is exhausted. It uses the console's existing LAN access boundary; no separate authentication is added.Bounds and socket lifecycle
Local requests are capped at 4 KiB, invalid UTF-8 gets
PARSE_ERROR, and there are at most 16 connections with a five-second lifetime. HTTP capture has a three-second deadline and at most four concurrent snapshot jobs. Both clients bound response headers and reject inconsistent geometry or payloads above 16 MiB before allocating pixels.The socket defaults to
/run/mediad/media.sock, withmediad --frame-socketandrobotctl --media-socketoverrides. Startup fails if it cannot be claimed. A lifetime lock and stale-socket probe preserve regular files, symlinks and live listeners; socket permissions remain 0660 with the existingrobotgroup handoff.media.framedeliberately has noCall/service-lane route: its binary tail must not enter the WebRTC control datachannel.duckctl's current BLE transport is also outside this change. These limits and both usage recipes are documented. The pre-existing shared group-helper refactor, policy/artifact limits and LeRobot/support work remain separate.Validation
cargo test -p robotctl -p mediad -p duck-ipc-protopasses: 287 tests, five existing ignored tests.291bd90passed all three jobs (check,board,coverage); logs confirm all nine endpoint tests actually executed. macOS does not compile that module.cargo fmt --allandgit diff --checkpass. Workspace-wide Clippy and tests passed on Linux CI.The upstream workflow is still awaiting maintainer approval (
action_required, no jobs); this is distinct from the successful fork run on the same head.Merged current upstream
main, resolving the lockfile conflict while preservinghf-robot-account,imageandlibc.