Repository navigation
feat(gateway): off-loop JPEG encode + rgba_raw fail-loud (gate #5 gateway half) - #10
Merged
Merged
Conversation
…ing (15d) rgba_raw exists for pixel-exact work, but offer_frame latest-wins-evicts a pending frame exactly like JPEG. At 720p an rgba_raw frame is 3.7 MB, too big to flush at sample rate, so ~2 of every 3 were dropped silently -- corrupting the one use case the encoding serves. Close the subscriber loudly with a dedicated RGBA_RAW_BACKPRESSURE reason instead (same shape as the audio-backpressure path, surfaced to the websocket as a close code + reason), so a pixel-exact consumer sees a failure rather than an invisibly holed sequence. `dropped` is not incremented -- a closed-loud frame is not a silent drop. JPEG preview subscribers are unaffected. Gate item #5 (15d) in docs/19; design in docs/20. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The JPEG encode (~8.5 ms at 720p, up to 13.7 ms) ran inline on the asyncio event loop, blocking every other subscriber, control message and audio chunk while it ran -- one such block per frame. Move it off the loop: MediaPipeline takes an injectable encode_runner defaulting to asyncio.to_thread; _relay_video becomes async and awaits it per encoding. PIL releases the GIL across the libjpeg encode, so it overlaps the loop. Measured (720p q92): longest single unbroken loop block per frame drops 13.7 ms -> 6.1 ms, and the full encode no longer blocks the loop on every frame (only PIL's GIL-held setup remains, and less than that under real I/O load). Teardown is deliberately unchanged: _finish_epoch still hard-cancels the sampler and emits EpochEnded synchronously. A cancel during the off-loop encode raises CancelledError before publish_video, so the in-flight frame is dropped rather than published after EpochEnded -- ordering is preserved in every case (graceful end, session end, reconnect, shutdown) with no new code. The cost is one frame at a real track end, immaterial on a continuous stream. This avoids the reconnect ordering hazard of the deferred-EpochEnded design (docs/20). Tests stay deterministic: to_thread composes badly with the lockstep pacer (the earlier "zero frames" failure), so the harness injects an inline runner that awaits without yielding; a guard test asserts the production default is asyncio.to_thread. Full gateway suite green; ruff + pyright clean. Gate item #5 (15b) in docs/19; design and decision in docs/20. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Closes the gateway half of gate item #5 (
docs/19-Post-Spike-Build-Plan.md), deferred out ofPR #9 which shipped the glasses H.265 publish. Two independent commits:
rgba_rawsubscriber that can't keep up now fails loud instead of silentlylatest-wins-dropping ~2 of 3 frames.
Design, and the decision that changed during implementation, in
docs/20-Gateway-Off-Loop-Encode-Design.md.15b — off-loop encode
The JPEG encode (~8.5 ms at 720p, up to 13.7 ms) ran inline on the single event loop, blocking every
other subscriber, control message and audio chunk while it ran — once per frame. It now runs on a
thread via an injectable
encode_runner(defaultasyncio.to_thread); PIL releases the GIL across thelibjpeg encode, so it overlaps the loop.
Measured (720p, quality 92):
We didn't make the encode faster; we stopped it from holding the loop hostage. The ~6 ms residual is
PIL's GIL-held setup (
frombuffer/convert) and is conservative — the benchmark's hot spin contendsfor the GIL harder than the real I/O-bound loop will.
Why teardown is untouched (the design decision)
The first design (docs/20 "Rejected") deferred
EpochEndedto the sampler so the tail frame survived.Implementation surfaced a reconnect ordering hazard: on the "new track SID = new epoch" reconnect,
EpochStarted(new)could publish while the old epoch's sampler was still draining, so a consumer wouldsee
EpochStarted(new)beforeEpochEnded(old)— reversing the boundary it resets tracker state on.The chosen design keeps
EpochEndedsynchronous, exactly as today. A cancel during the off-loopencode raises
CancelledErrorat theawaitbeforepublish_video, so the in-flight frame isdropped rather than published late — ordering is preserved by construction in every case (graceful
end, session end, reconnect, shutdown), with no teardown changes. The cost is one frame at a real track
end, immaterial on a continuous stream.
15d — rgba_raw fail-loud
rgba_rawexists for pixel-exact work but was latest-wins-dropped like JPEG. At 720p a raw frame is3.7 MB — too big to flush at sample rate — so ~2 of 3 were dropped silently, corrupting the one use
case it serves. It now closes the subscriber with a dedicated
RGBA_RAW_BACKPRESSUREreason (surfacedto the websocket as a close code + reason), so a pixel-exact consumer sees a failure, not bad data.
droppedis not incremented. JPEG preview is unaffected — for lossy preview, latest-wins is correct.Verification
uv run pytest),ruff+pyrightclean.test_rgba_raw…passes unchanged — the test harness injects an inlineencode runner so relay stays deterministic (a real thread hop composes badly with the lockstep pacer,
which is what drove the earlier "zero frames" failure).
asyncio.to_thread, andtest_rgba_raw_subscriber_is_closed_rather_than_silently_dropped.🤖 Generated with Claude Code