Share the length-prefixed record reader across the stdin demos - #346
Conversation
stream_stdin.h centralised set_stdin_binary() and read_exact() so there would be one source of truth instead of one copy per demo, but it stopped one level too low. The thing every caller actually wanted -- read a <u32_le len><body> record -- stayed replicated in four demos, and the copies had drifted: the little-endian assembly re-typed each time, three different range checks, and four different answers to what an EOF part-way through a record means. Add read_length(), read_body() and the composed read_record(). The result states are deliberately finer than "it failed", because that distinction is exactly what the callers differ on: a producer that closed cleanly between records is an ordinary end of run, one that closed with a length word already written has lost a record, and a length out of range is a producer bug rather than a stream end. Each demo maps those states onto its own policy -- streamtx warns on one and exits 2 on another, txdemo counts the lost shard and stops, svctx treats any incomplete record as the end of the clip -- and none of them re-derives them. duplex keeps composing read_length() and read_body() rather than taking read_record(): its top length bit escapes to a control TLV with its own much smaller bound, so it has to see the length before the body may be read. That is the case the primitives exist for. The selftest now drives read_record() -- the path the demos run -- and pins what each state means, through tmpfile() for the cases a well-formed stream never produces: a truncated prefix, a length with no body, a body cut short, a zero length, a length past the bound. Validated on air, not just headlessly: the txdemo reader through the adaptive FEC harness (0.869 -> 0.935 FEC delivery across an exclusion, in line with the four runs before this change) and the streamtx reader through the jammer-resilience matrix (6132 bodies aired, 0.917 clean / 0.975 jammed). The duplex and svctx readers are covered by the shared selftest and the build only -- neither has a bench harness here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The duplex demo escapes to a control TLV on the length's top bit and bounds that body at 256 rather than at its PSDU maximum, so it reads the length, masks it, and only then reads the body. That composition is the reason read_length and read_body are public at all, and nothing covered it -- before this change either. A record whose length has the top bit set must arrive verbatim and must not be mistaken for an oversize PSDU. Mutation-checked: breaking the length assembly's top-byte shift fails the test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The stdin demos other than streamtx had no way to be judged on air here. duplex
is exercised by tests/adaptive_onair.sh, but adaptive_link.py consumes its
event stream on a pipe and drops what it is not looking for, so nothing
observes whether a control TLV was read or acted on; svctx has
tests/svc_uep_onair.sh, which needs an 8814AU in kernel monitor mode.
Two scripts, each a scripted stream plus a devourer receiver as witness:
duplex_ctl_onair.sh PSDUs -> SET_RATE -> PSDUs -> SET_PWR -> PSDUs, and
asserts duplex logged a stream.ctl per TLV, aired every
PSDU (the record stream stayed in sync across the
escapes), and that the witness decoded the rate change
svctx_uep_witness.sh synthetic NALs with controlled temporal_ids, asserting
the witnessed rate histogram spans more than one rung —
the ladder reached the air
Measured: duplex reads both TLVs, airs 1500/1500 PSDUs and switches 452 frames
at MCS1 to 980 at 6M; svctx flies MCS0/MCS1/MCS4 across 1837 frames.
One trap is worth the comment it carries. The TLV must switch DOWN to a more
robust rate, never up: at this bench's ~13 dB margin an MCS1 -> MCS5 switch
reads as "SET_RATE never took effect", because the frames fly at the new rate
and the witness cannot decode them — one rate in the histogram and a quietly
lower frame count. The first version of this test asserted exactly that and
called a working demo broken.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
End-to-end bench coverage added for the two demos that had none here ( duplex — 452 frames at MCS1 (segment 1) then 980 at 6M (segments 2–3) — the TLVs were read, SET_RATE reached the air, and every PSDU aired, so the record stream stayed in sync across both escapes. svctx — Three ladder rungs on air (MCS0 critical / MCS1 T0 / MCS4 T1), which also proves the NAL records were read with their temporal_ids intact. Refactor equivalence, measured rather than argued: I rebuilt duplex from the pre-refactor source against the same library and ran the same script — identical outcome (2 TLVs, 1500/1500 aired). So the reader change is behaviour-preserving on hardware, not just by inspection. One trap is now documented in the script, because it cost me a wrong conclusion first: the TLV must switch down to a more robust rate. At this bench's ~13 dB margin an MCS1 → MCS5 switch is indistinguishable from no switch at all — the frames fly at the new rate, the witness can't decode them, and you get one rate in the histogram plus a quietly lower frame count. I initially reported that as a pre-existing SET_RATE defect; it wasn't. The frame counts gave it away (each run decoded exactly the segments flown at the robust rate). |
1 participant
Follow-up to #345, which noted this as a known duplication rather than folding it in.
stream_stdin.hcentralisedset_stdin_binary()andread_exact()so there would be one source of truth instead of one copy per demo — but it stopped one level too low. The thing every caller actually wanted, read one<u32_le len><body>record, stayed replicated in four demos, and the copies had already drifted: the little-endian assembly re-typed each time, three different range checks, and four different answers to what an EOF part-way through a record means.What's shared now
read_length(),read_body(), and the composedread_record().The result states are deliberately finer than "it failed", because that distinction is precisely what the callers differ on:
OkEofShortEofMidBodyBadLengthEach demo maps those onto its own policy and none re-derives them:
streamtxwarns on one and exits 2 on another,txdemocounts the lost shard and stops,svctxtreats any incomplete record as the end of the clip,duplexstops its TX thread and lets RX run on.duplexkeeps composingread_length()+read_body()rather than takingread_record(): its top length bit escapes to a control TLV with its own much smaller bound, so it must see the length before the body may be read. That's the case the primitives exist for.Testing
The selftest now drives
read_record()— the path the demos actually run — and pins what each state means, throughtmpfile()for the cases a well-formed stream never produces: truncated prefix, length with no body, body cut short, zero length, length past the bound.Validated on air, not only headlessly:
47/47 ctest green.
🤖 Generated with Claude Code