feat: LatestFrameWriter/LatestFrameReader (producer-owned latest-frame, seqlock)

[DanielKow]

What

Adds the additive latest-frame shared-memory primitive from Epic 020 / feature-002 (ADR-1 + ADR-11): a producer-owned, consumer-optional, newest-frame-wins, tear-free zero-copy display buffer built on zerobuffer's platform layer only. It does not touch the SPSC Reader/Writer, so the AI path's SPSC contract is unchanged (FR-6).

This is Wave 1 (foundation). streamer (GstDisplaySink → LatestFrameWriter) and native-player (ShmFrameSource → LatestFrameReader) build on it in Wave 2.

Public API — link against this (for Wave-2 engineers)

Header: #include <zerobuffer/latest_frame.h> — one definition of the on-segment layout, shared by both sides.

namespace zerobuffer {

class LatestFrameWriter {           // producer — streamer GstDisplaySink
  LatestFrameWriter(std::string name, size_t slot_size, uint32_t slot_count = 3);
  void     set_metadata(const void* caps, size_t len);   // writes [u32 LE len][JSON]
  uint8_t* get_slot();                                    // reserve next slot (no wait)
  void     publish(uint64_t sequence, size_t size);       // seqlock publish
  size_t   slot_size() const;  uint32_t slot_count() const;  const std::string& name() const;
};

class LatestFrameReader {           // consumer — native-player ShmFrameSource
  explicit LatestFrameReader(std::string name);           // tolerates absent segment

  // ZERO-COPY, seqlock-validated read of the newest frame. `consume` is invoked
  // with a pointer DIRECTLY into the published slot (no copy, no per-frame alloc).
  // Returns false if the segment is absent, no NEW frame within timeout, or all
  // retries tore. Caller commits its work only when this returns true.
  template <class Fn>  // void consume(const uint8_t* src, size_t size, uint64_t sequence)
  bool read_latest_into(Fn&& consume, std::chrono::milliseconds timeout);

  const void* get_metadata_raw();   size_t get_metadata_size();   // [u32 LE len][JSON]
  bool        is_writer_alive();                          // false = producer gone (FR-9)
  bool        is_attached() const;  const std::string& name() const;
};
}

Zero-copy read (ADR-11 / design §4–5 "no extra copy"): the reader hands consume a pointer straight into the slot; the seqlock is validated around the consume — on a mid-read lap the consume is re-invoked on the newest slot (bounded retries), so the caller treats its work as committed only when read_latest_into returns true. This eliminates the N×-per-reader full-frame copy (a 12 MB HDR frame × N readers), critical for many-reader HDR-FullHD on RPi. The primitive stays generic — it never copies and knows nothing about PixelRepack/DisplayBuffer.

⚠️ consume contract for Wave-2 (native-player ShmFrameSource) — differs from a copy API: because the pointer is live shared memory validated after consume runs, consume

1. may be handed torn bytes (a mid-write lap) — the tests prove this happens; do not trust pixel values until read_latest_into returns true;
2. may be invoked more than once per call (each retry) — make the repack restartable/idempotent (repack into the same DisplayBuffer slot; do not accumulate);
3. present() only on a true return — never present-on-false and never assume a single invocation.

The intended shape: consume runs PixelRepack slot→its DisplayBuffer slot; on a tear the repack is simply redone into the same slot, and it is present()ed only after true, so a torn frame is never presented. (Documented in the header too.)

Metadata wire contract (verified against native-player)

get_metadata_raw() returns a pointer to a 4-byte little-endian length prefix followed by the JSON, and get_metadata_size() returns 4 + json_len — exactly what nativeplayer::ShmCaps::parse(metadata, size) consumes (shm_caps.cpp:142-159). set_metadata(caps, len) takes the bare JSON and the writer owns the length framing. Before the first set_metadata, the reader reports "no caps yet" (get_metadata_raw()==nullptr, size 0), not an empty-caps block.

⚠️ Note for streamer: today's gstzerosink writes bare JSON with no prefix; the new GstDisplaySink must pass JSON to set_metadata (which adds the prefix) — do not hand-roll the prefix.

⚠️ API deviations from displaysink-design.md (please sync downstream)

• The design sketched publish(uint64_t seq). I added the frame byte-count: publish(uint64_t sequence, size_t size) — the reader reports size and ShmFrameSource drops when it disagrees with ShmCaps::frame_bytes(), so the real count must travel with the frame. size > slot_size is clamped (no OOB).
• The design sketched a copy-returning read_latest() + a LatestFrame value class. Replaced by the zero-copy read_latest_into(consume, timeout) functor above (single path; data()/size()/sequence()/valid() become the consume params + the bool return). Team lead is syncing displaysink-design.md Reader API + design.md §4/§5.

Design notes

• Seqlock over a triple- (or wider) slot ring, zero-copy. Per-slot seqlock: writer makes it odd before touching the payload, even after; reader reads publish_index (acquire) → seqlock (acquire) → invokes consume on the slot in place → acquire fence → re-checks seqlock; a mid-read lap → re-invoke consume on the newest. Cross-process atomicity via std::atomic_ref over plain POD fields; frame_number/size are relaxed atomic_ref, the payload is read in place by the caller (validated by the seqlock re-check). No per-frame copy, no per-frame allocation.
• Never waits on a reader (FR-7). Publishing continues with 0..N readers; attach/detach/pause/crash never perturb the producer.
• Crash cleanup / stale reclaim. Header carries writer pid + start-time. On create, a stale segment left by a dead writer is reclaimed (process_exists); a segment owned by a live writer is refused. On clean release the writer invalidates the header magic before unlinking → readers detect "producer closed" and re-attach to a fresh segment (works even in-process). Liveness is pid + start-time via platform::process_exists.

Tests (gtest, tests/test_latest_frame.cpp — 17, all green)

Writer create+publish with no reader · reader-attaches-after-writer gets latest · newest-wins skips intermediates · zero-copy pointer rotates through the ring (proves no per-frame alloc: 1 < distinct ≤ slot_count) · absent segment tolerated (consume never invoked) · detach/re-attach (writer unaffected) · writer restart → reader recovers · writer-gone reported, no crash/block · oversized frame clamped, no OOB · metadata set + rewrite (exact prefix bytes) · caps not-ready before first set_metadata · tear-free under concurrent write (256 KB frames, committed_torn==0 AND retry path exercised) · multi-reader · second-writer-on-live-segment rejected · stale-segment reclaimed.

Sanitizers: full suite TSan verified clean (ThreadSanitizer, ASLR off via setarch -R), and clean under ASan + UBSan (lib instrumented — no leaks/OOB/UB). The in-place payload read is a benign, standard seqlock pattern the seqlock re-check makes correct. (TSan prints a benign "atomic_thread_fence not supported with -fsanitize=thread" note — a TSan instrumentation limitation, not a race; TSan reports zero races.)

Build-config fixes (pre-existing, unrelated to the primitive — flagged to team lead)

BUILD_TESTS=ON did not configure on a clean checkout, independent of this change:

1. tests/CMakeLists.txt referenced test_frame_raii.cpp, which is absent from the repo → now guarded with if(EXISTS ...).
2. tests/generated/CMakeLists.txt links nlohmann_json but the main CMake only fetches it after add_subdirectory(tests) → hoisted a find/fetch to the top of tests/CMakeLists.txt.
3. .gitignore rule /cpp/tests/test_* was silently ignoring test sources → added !/cpp/tests/test_*.cpp so test sources are tracked while built binaries stay ignored.

🤖 Generated with Claude Code

[DanielKow]

Review — C++ Reviewer (Epic 020 / feature-002, Wave-1 foundation)

Verdict: no blockers. Approve pending the MINORs below. This is a correct, well-structured seqlock primitive. I re-derived the publish/read ordering against the C++ memory model and it holds; I built and ran everything under three sanitizers.

Verification I ran (not just trusted)

Check	Result
Build BUILD_TESTS=ON, Debug	clean, zero warnings
15 gtest cases	15/15 pass
ThreadSanitizer (full suite + 3× 20k-iter tear-free stress)	clean — 0 data races
ASan + UBSan (-fno-sanitize-recover=all)	clean — no leaks / OOB / UB
/dev/shm after run	no leaked segments
SPSC reader/writer(.cpp/.h) touched?	no — FR-6 confirmed additive

Seqlock correctness — confirmed

• Publish order (payload → release fence → seqlock even (release) → publish_index (release) → heartbeat) and read order (publish_index acquire → seqlock acquire → copy → acquire fence → seqlock re-check) are a correct seqlock. A lap is always detected and retried onto the newest slot; no path returns a torn or half-written frame.
• Triple-slot: get_slot() never targets the currently-published slot, and the skip logic gives ~2–3 publishes of separation before a reader's held slot can be reused — the seqlock backstops the rest. FR-7 (producer never blocks) holds: no wait/semaphore anywhere on the write path.
• Oversize clamped in both publish() (size→slot_size) and the reader (sz→scratch.size()) — no OOB. pid-reuse guarded via writer_start_time. In-process restart handled by invalidating magic before unlink.

MINORs

1. publish() non-atomic data reads — abstract-machine data race, benign; TSan actually passes so the PR's "TSan false-positive, can't use it" wording should become "TSan verified clean." (inline)
2. "Valid empty" metadata before first set_metadata — reader marks metadata_seq==0 as valid empty caps. (inline)
3. heartbeat_ns/heartbeat() written-but-never-read — dead surface; liveness is (better) pid-based. Delete the field/method or update displaysink-design.md, which still describes threshold staleness. (inline)
4. Reader maps RW, not read-only. SharedMemory::open only offers PROT_READ|PROT_WRITE, so the "reader" mapping is writable — the design's "maps read-only" isn't achievable without a platform-layer change. Informational: the consumer-cannot-perturb-producer guarantee holds through the defined API, but not at the memory-protection level. Track for a future platform RO-open option; not for this PR.
5. .gitignore last hunk (/csharp/ZeroBuffer/nupkg, /logs) is a pure line-ending re-emit with no content change — trim it so the diff shows only the intended !/cpp/tests/test_* negations.

Build-config fixes (team-lead question #8)

Correct and minimal. The test_frame_raii.cpp if(EXISTS) guard, the nlohmann_json hoist, and the .gitignore source-negation are the changes that make BUILD_TESTS=ON configure at all — reasonable to keep in this PR (they're prerequisites for the tests here), though they'd be defensible as a separate fix(build): commit. They do not alter SPSC behavior.

None of the above gates merge. Once 1–3 are addressed (code or doc), this is good to go for Wave 2.

[DanielKow]

Resolving the review threads — all addressed in a3ea386 / 5f52872 (+ docs on project-management master):

• MINOR-1 (TSan wording / atomic_ref): PR description corrected to "TSan verified clean" (runs clean via setarch -R). frame_number/size are now accessed through std::atomic_ref with relaxed ordering; the payload read keeps the classic seqlock-exception comment.
• MINOR-2 (metadata "valid empty" before first set_metadata): refresh_metadata now gates on metadata_seq >= METADATA_WRITTEN, so a frame published before caps are set reports caps-not-available (null), not empty-JSON. Locked by MetadataNotReadyBeforeFirstSet.
• MINOR-3 (dead heartbeat surface): deleted heartbeat_ns / heartbeat() / now_ns(); liveness is pid + start-time via process_exists. displaysink-design.md §Liveness and ADR-11 are synced to pid-based liveness, and the alive-but-hung-writer trade-off (hold last frame, FR-9) is now the documented conscious choice.
• Zero-copy re-review / consume contract: acknowledged. The three-point contract — present only on a true return; consume may run >1×/call and must be restartable; pixel values trusted only on true — is documented in the header and displaysink-design.md, and is being carried into the Wave-2 native-player (ShmFrameSource) scope.

— Claude