Skip to content

Core pacing & estimator fixes: chunk-duration recompute, estimator drift, write quantum, deadline sleeps - #26

Merged
SpaceK33z merged 11 commits into
mainfrom
claude/fix-core-pacing
Jul 11, 2026
Merged

Core pacing & estimator fixes: chunk-duration recompute, estimator drift, write quantum, deadline sleeps#26
SpaceK33z merged 11 commits into
mainfrom
claude/fix-core-pacing

Conversation

@SpaceK33z

Copy link
Copy Markdown
Collaborator

Fixes the shared-core findings from the DAC output-quality analysis (docs/dac-quality-analysis.md on claude/dac-output-quality-cf26w4). All behavioral fixes come with regression tests; cargo fmt/clippy clean, full test suite green (438 lib + 11 doc tests, 9 new).

Output quality

  • UdpTimed: recompute the send period whenever pps changes, not only when the chunk size changes. Previously, in the clamped regime (chunk pinned at max_points_per_chunk, e.g. IDN's 179 points at pps ≥ 17.9k) a pps change kept a stale send period → feeding at the old rate → continuous underrun.
  • UdpTimed: bounded stall catch-up — after a scheduling stall, next_send may lag now by up to 3 chunk durations so lost time is worked off, instead of being discarded by the old next_send = now clamp (which permanently drained the device buffer on every hiccup).
  • SoftwareDecayEstimator: carry fractional consumption across rebases. record_send used to floor-truncate ~0.5 points per rebase (one-sided), inflating the estimate by ~500 phantom points/sec at 1 kHz write cadence and pinning the real device buffer at zero. New multi-rebase drift test asserts <1.5 pt drift over 1000 rebases.
  • NetworkFifo: minimum write quantum (max of 5 ms of points / 16, capped to the target buffer) — stops the ~1000 tiny-writes/sec steady state that amplified estimator drift and per-write overhead.
  • Absolute-deadline sleeps in sleep_with_control_check and the reconnect backoff — sleep loops no longer assume thread::sleep returns on time (it can overshoot 10x+ on Windows' default timer).
  • Frame-capacity clamp for authored frames: frames whose authored points alone exceed frame_capacity (Helios: 4095) are now truncated with a rate-limited warning instead of being sent oversized to the device.
  • IdlePolicy::Park honored in the armed-no-frame path (previously always blanked at origin).

Reliability / safety

  • Control messages processed inside the NetworkFifo WouldBlock spin — a Disarm (shutter close) previously waited indefinitely while a backend stalled. Also adds a 2 s staleness bound: continuous WouldBlock now escalates to disconnect so reconnect engages instead of spinning forever.
  • Reconnect attempts first, then backs off — a transiently recoverable disconnect no longer blanks output for a full backoff period before attempt feat!: streaming-first API refactor #1.
  • Scheduler thread is now named (laser-frame-scheduler); priority elevation left as documented future work (no new dependency).

Docs

  • CONTEXT.md OutputModel examples corrected (LaserCube WiFi and AVB are NetworkFifo, UdpTimed's example is IDN) and the discard_cached contract updated to match reality (on_reconnect owns cache disposal).

Notes for review:

  • Tunable constants introduced: MIN_WRITE_QUANTUM_SECS=0.005, MIN_WRITE_QUANTUM_FLOOR=16, WOULDBLOCK_STALL_TIMEOUT=2s (network_fifo.rs), CATCH_UP_CHUNKS=3 (udp_timed.rs).
  • The write quantum is capped to target_buffer_points so very low pps sessions (tiny target buffers) can't wedge — this was caught by an existing pps=30 metrics test.
  • Skipped (low severity, deliberately): rotational color delay for cyclic frame-swap slices — invasive relative to its impact.

🤖 Generated with Claude Code

https://claude.ai/code/session_01N4AZhstwBFe428pC7cJMbT


Generated by Claude Code

claude added 8 commits July 10, 2026 00:16
`recompute_chunk` only recomputed `chunk_duration` when the chunk *size*
changed, so a pps change in the clamped regime (IDN's 179-point cap at
pps >= 17.9k) kept a stale, too-slow send period and fed the device at
the wrong rate. Track the pps used for the current duration and recompute
whenever either changes.

Also replace the `next_send = now` hard clamp with bounded catch-up: after
a stall, allow `next_send` to lag `now` by up to 3 chunk durations so lost
time is worked off gradually instead of permanently draining the device
buffer, clamping only beyond that window.

Adds regression tests for both: a clamped-band pps change changes the
duration, and a simulated 1s stall retains (but bounds) the catch-up lag.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4AZhstwBFe428pC7cJMbT
`record_send` rebased the anchor to `now` from a floor-truncated read,
discarding the sub-point remainder of `elapsed x pps` on every call
(~0.5 points of upward bias per rebase -> hundreds of phantom points/sec
at ~1kHz write rates, so the estimator believes the buffer is full while
the device runs dry). Subtract only the integer points consumed and
advance the anchor by exactly the time they represent, leaving the
fraction in the residual gap so it carries forward.

Adds a 1000-rebase drift test asserting the estimator tracks a
full-precision reference to within ~1 point (the old code drifted ~500).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4AZhstwBFe428pC7cJMbT
…spin

Three fixes to the FIFO adapter:

- Add a minimum write quantum (max(5ms of points, 16), capped to the
  target buffer and max_points): when the deficit is below the quantum,
  wait for it to grow rather than dribbling out a few-point write. This
  stops the ~1000 tiny-writes/sec steady state (pure overhead and an
  amplifier for estimator rounding bias).
- Process control messages inside the inner WouldBlock spin so a Disarm
  (shutter close) takes effect promptly instead of waiting indefinitely
  for a stalled device to accept the pending write -- a safety concern.
- Bound the spin: continuous WouldBlock for >2s is treated as a
  disconnect so reconnect engages instead of spinning forever.

Tests: sub-quantum deficit is not written, above-quantum is, and a
Disarm during a perpetual-WouldBlock spin closes the shutter.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4AZhstwBFe428pC7cJMbT
`sleep_with_control_check` and the reconnect `sleep_with_stop` both
decremented a `remaining` counter by the nominal slice, so on a coarse OS
timer (Windows' 15.6ms default) the total wait could run many times long.
Loop against an absolute deadline instead.

Also attempt the first reconnect immediately and back off only between
attempts, so an instantly-recoverable disconnect no longer blanks output
for a full backoff period before the first retry.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4AZhstwBFe428pC7cJMbT
`clamp_to_capacity` only trimmed the transition prefix, so a frame whose
authored points alone exceeded frame_capacity reached the encoder
oversized (the Helios SDK refuses > 4095 points; firmware behavior is
otherwise undefined). After the transition is exhausted, truncate the
tail to capacity with a rate-limited (<=1/sec) warning.

Tests cover both the frame-change and self-loop compose paths.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4AZhstwBFe428pC7cJMbT
When armed but no frame has been submitted, the engine blanks at the
origin and the output snapped to (0,0) regardless of IdlePolicy::Park.
Fill the no-frame chunk with the configured park position instead, in the
smaller SlicePipeline-side change rather than plumbing park into the
engine. Adds a test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4AZhstwBFe428pC7cJMbT
Give the scheduler thread a stable name for profilers / thread dumps via
thread::Builder. Left a doc comment noting priority elevation as future
work (deferred to avoid a platform thread-priority dependency).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4AZhstwBFe428pC7cJMbT
- content_source.rs: `discard_cached` has no call sites; cache disposal on
  the reset path is owned by `on_reconnect` (which resets the source).
  Document that instead of claiming the driver calls `discard_cached`.
- CONTEXT.md: LaserCube WiFi and AVB are NetworkFifo in code, not
  UdpTimed; UdpTimed's example is IDN. Correct the OutputModel section
  (and the matching stale header comment in network_fifo.rs).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4AZhstwBFe428pC7cJMbT
- reconnect: add a flapping-device backoff floor. reconnect_backend_with_retry
  now takes the previous reconnect instant; a device that accepts then dies
  within one backoff no longer busy-loops disconnect→reopen→disconnect with
  zero delay (callbacks throttled to >= backoff). Genuine one-off disconnects
  still recover instantly. Threaded through both driver and stream callers.
- engine/pipeline: honor IdlePolicy::Park for empty ("clear the display")
  frames and mid-chunk empty promotions, not just the no-frame case. The engine
  now holds a configurable idle-blank point (park under Park) for all no-content
  fills instead of snapping to origin.
- tests: de-flake the two timing-sensitive tests — anchor the estimator send in
  the future in deficit_below_quantum_is_not_written so decay can't cross the
  quantum during construction; widen the bounded-catch-up upper bound.
- session: move the elevated-scheduling-priority roadmap note to issue #35.
- docs: correct the frame_capacity field doc (tail truncation) and note the
  cosmetic self-loop-seam launch point after oversized-frame tail truncation.

Adds regression tests for the flapping guard, engine idle-blank on
empty/mid-chunk-promotion, and pipeline empty-frame park.
test_estimate_buffer_reads_backend_estimator asserted exact equality
against the software-decay estimator, which sheds ~1 point per 33µs at
30k pps — the read a few µs after seeding flaked on loaded CI. Use a
tolerance band, matching the existing pattern in
test_status_reports_connected_stats_and_scheduled_ahead. The decaying
seed behavior is left intact since test_fill_result_end_drains_with_queue_depth
relies on it.
@SpaceK33z
SpaceK33z merged commit 203391c into main Jul 11, 2026
19 checks passed
SpaceK33z added a commit that referenced this pull request Jul 13, 2026
Elevate the laser-frame-scheduler thread to ThreadPriority::Max so its
pacing sleeps are less likely to be preempted under system load, reducing
output timing jitter. Best-effort: on systems that disallow raising
priority without privileges (e.g. Linux without CAP_SYS_NICE) it logs a
warning and continues at default priority rather than failing the session.

Uses the cross-platform thread-priority crate (deferred from #26). Max
maps into the current scheduling policy's range, so it needs no elevated
privileges on macOS/Windows and falls back to niceness on Linux.

Closes #35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants