Measure what a frame codec costs the round trip, not only the wire - #729
Measure what a frame codec costs the round trip, not only the wire#729v-positronic wants to merge 7 commits into
Conversation
A video codec over the temporal-stack window sends a fraction of the bytes a per-frame JPEG sends. Both the encode and the decode sit on the round trip, so bytes alone do not say whether the codec is faster. This probe replays a recorded episode through the rig-side bound and reports all three per window. On real frames from a curie round, 25 frames, two cameras, 512x288: JPEG q90 919 KiB, encode 22 ms, decode 26 ms; h264 ultrafast crf20 325 KiB, encode 49 ms, decode 16 ms; h264 veryfast crf20 219 KiB, encode 78 ms, decode 22 ms. Ticket: Positronic-Robotics/internal#1168 #refs
Each codec now answers `cost(window)` and carries its own name, so the report loops one list instead of calling JPEG inline beside a list of h264 variants. Adding a codec is appending to that list. `serialization` keeps the per-frame JPEG buffers under `b'frames'`, and a second module now reads a marker, so the key becomes a public constant rather than a literal each side spells for itself. Ticket: Positronic-Robotics/internal#1168 #refs
`RestrictImageSize` is what bounds an image before it reaches the wire, and the probe re-derived its scaling instead of calling it. The two cannot then be shown to agree, which is the one property a measurement of the wire needs. Byte counts are unchanged, which says they did agree. Ticket: Positronic-Robotics/internal#1168 #refs
The per-camera rows and the summed row were one loop over the cameras plus the string 'every camera', which two lines had to spell the same way. They are now two calls to one row builder. Ticket: Positronic-Robotics/internal#1168 #refs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b0b8f3f86
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| recorded_rate = container.streams.video[0].average_rate | ||
| if recorded_rate is None: | ||
| raise ValueError(f'{mp4} declares no frame rate, so the sampled frames cannot be chosen') | ||
| step = max(1, round(float(recorded_rate) / rate_hz)) |
There was a problem hiding this comment.
Rule hidden-dependency violated:
bounded_frames treats the MP4's average_rate as the observation cadence, but episodes produced by DiskEpisodeWriter encode videos at a fixed 100 fps while storing the real frame timestamps in <camera>.frames.parquet. For ordinary 30 Hz camera data, the default 15 Hz request therefore keeps every seventh frame instead of every second frame, producing windows with substantially different motion and invalid codec-cost results. Read the sidecar timestamps and reproduce the temporal stack's time-based sampling rather than deriving sampling from the MP4 rate.
AGENTS.md reference: AGENTS.md:L7-L8
Useful? React with 👍 / 👎.
|
|
||
| def _row(label: str, group: list[Cost], windows: list[int]) -> str: | ||
| """One printed line: the median over windows of what the group cost in each window.""" | ||
| per_window = [[row for row in group if row.window == window] for window in windows] |
There was a problem hiding this comment.
Rule hidden-dependency violated:
_row silently turns a missing camera row for a global window into a zero through sum(...), so when requested camera files have different usable window counts—especially with --windows 0—per-camera medians are biased downward and the every camera row sums fewer cameras for later windows. Intersect or validate window IDs across all requested cameras before reporting, and reject cameras with no complete windows, rather than depending on every MP4 having identical length.
AGENTS.md reference: AGENTS.md:L7-L8
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3aa0af2eb2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| with av.open(buffer, 'w', format='mp4') as container: | ||
| stream = container.add_stream('libx264', rate=15) | ||
| stream.width, stream.height, stream.pix_fmt = width, height, 'yuv420p' | ||
| # One self-contained GOP with no lookahead: a request carries its own window and waits on it. |
There was a problem hiding this comment.
Rule diff-comments violated:
The comment above stream.options describes what a request carries and waits for rather than a constraint local to this encoder. Delete it or restate only the durable reason for disabling lookahead and setting the GOP length.
AGENTS.md reference: AGENTS.md:L7-L8
Useful? React with 👍 / 👎.
1 participant
Runway asked to carry the temporal-stack window as one video GOP instead of one JPEG per
frame, on the grounds that it saves both size and encode time. Bytes are easy to count and
the two times are not, and both the encode and the decode sit on the round trip, so a codec
that thirds the bytes and doubles the encode is a loss.
The probe replays a recorded episode's cameras through
RestrictImageSize, the codec thatbounds an image before it reaches the wire, then encodes each window every way it is given
and reports bytes, encode ms and decode ms per window.
On the 8 September curie round, 25 frames, two cameras, 512x288, 100 windows, run on the
rig that serves those rounds and on the box this was developed on:
The rig is a 32-thread i9-13980HX and posi-vm an 8-core EPYC-Genoa, so the absolute times
differ by about 1.5x. The bytes are identical to the KiB on both, and so is the shape: h264
ultrafast sends 2.8x fewer bytes and costs 2.2x the encode, 2.3x on the slower box. The
ratio carries between machines; the milliseconds are each box's own.
The decode column is the same machine decoding what it just encoded, so it bounds what a
decode costs on that hardware and says nothing about what the inference server spends —
that server is somebody else's and was never measured here.
JPEG runs single-threaded through
encode_jpeg, the encoder the wire uses. h264 runs withx264's own frame threading and
tune=zerolatency, so the comparison is generous to h264 onboth counts, and most generous on the rig's 32 threads.
serialization.FRAMESbecomes public in the same change. The per-frame JPEG buffers sit underthat key by a contract between whoever writes a marker and whoever reads one, and a second
module now reads one, so the key gets a single owning definition rather than a literal each
side spells for itself.
Verified over one episode of that round, 100 windows per camera on each box. The byte counts
did not move when the probe stopped re-deriving the rig-side bound and started calling
RestrictImageSize.pytest positronic/utils/tests positronic/offboard/tests positronic/policy/testspasses.Refs: Positronic-Robotics/internal#1168.