Establish production realtime architecture foundations - #419
Merged
Merged
Conversation
Owner
Author
|
Split 2026-09-10: the beta-stability commits on this branch are now a standalone PR against main ( |
This was referenced Sep 10, 2026
(cherry picked from commit 257ff26)
(cherry picked from commit 9bfd2d9)
(cherry picked from commit 678c024)
(cherry picked from commit f6089d3)
(cherry picked from commit f5b9f53)
(cherry picked from commit 0c0c90d)
(cherry picked from commit 4c342fd)
(cherry picked from commit 723eb0d)
(cherry picked from commit 8fdd94e)
(cherry picked from commit f578670)
(cherry picked from commit da3b033)
(cherry picked from commit 68a5e4d)
(cherry picked from commit b7211f0)
(cherry picked from commit 9c9acbc)
(cherry picked from commit e971698)
(cherry picked from commit d5ad67f)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YASnsbX3oQRZRxUNroqjY5 (cherry picked from commit cfd8d85)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YASnsbX3oQRZRxUNroqjY5 (cherry picked from commit 9fa899e)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YASnsbX3oQRZRxUNroqjY5 (cherry picked from commit bd26bc7)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YASnsbX3oQRZRxUNroqjY5 (cherry picked from commit 9335166)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YASnsbX3oQRZRxUNroqjY5 (cherry picked from commit 2774400)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YASnsbX3oQRZRxUNroqjY5 (cherry picked from commit 0484a9a)
The beta split annotated three CLAUDE.md/handoff links as "(on the PR #419 branch)" and dropped the churn test's explanatory banner. On this branch the plans exist, so the links are plain again and the banner is restored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SAB8v62BEV8ihmaeGhhtY9
iamfatness
force-pushed
the
codex/production-realtime-architecture
branch
from
September 10, 2026 19:03
1d164b7 to
0b2a91b
Compare
Owner
Author
Rebased onto
|
Owner
Author
|
State check, 2026-09-12. Recording this so the branch's real position is on the
The failing check is stale. This branch predates #500 ( Conflict cost today is small but compounding — four files, all hot core:
Everything else auto-merges, including Why it is not simply merged. Two standing rulings, both still in force:
So this branch is not blocked on code. It is waiting for a consumer, and the The decision, owner's call:
Recommendation: (1), and soon. Four conflicts after 68 commits is cheap; the |
68 commits of `main` (two days, including the whole 2026-09-12 batch) against a merge base of 2026-09-10. Four conflicts, all in the hot core, plus two semantic breakages that only the build and the test run exposed. RouteSourcePolicy.h -- additive both ways, kept both. This branch's exactSource/frameIdentity answer "which immutable frame is this route pinned to"; main's directedSpeakerParticipantId answers "which participant does a follow-speaker route resolve to". The new field is APPENDED, never inserted. MediaCore.cpp -- the route policy input is now built by NAMED FIELD, not positionally. Both sides constructed it positionally and DISAGREED on slot 6 (this branch: a positional fallback participant; main: the directed speaker), so a positional resolution would have bound the speaker into a dead field and silently broken every follow-speaker route. main had also deleted the variable this branch passed there, so the old call site no longer compiled. positionalFallbackParticipantId is deliberately left unset: #480 retired the positional fallback outright, and the newer owner ruling wins. The two fill rules are NOT in conflict and both now stand, with explicit precedence: #480 "no source is configured" -> blank, transparent, opacity 0 here "an exact source was DEMANDED and refused" -> opaque black, and it wins Transparent would let whatever else is composited show through, and the point of an exact-source refusal is that the WRONG guest must not appear in that slot. Missing/Rejected can only arise when exactSource is non-null, so this changes nothing for routes as they exist on main. Two positional-initializer breakages the compiler and the suite caught: - ZoomEngineParticipant gained persistentId as the THIRD field on main, so seven positional initializers in ZoomEngineRuntimeTest were assigning a bool to a std::string. The empty persistentId is deliberate -- those tests assert that identical display names never collapse into one durable person, which only means anything when the engine supplied no persistent id. - main's own follow-speaker test passed the speaker positionally into slot 6 and silently bound the wrong member. Converted to named fields. Also clears the stale CI red: this branch predated #500, which stopped mac-show-drill being permanently red on a runner that cannot hold 60Hz. Tests: 1126 native (main alone: 931), 2317 MediaCore, 1469 WinUI -- all green; escape scanner clean. WinUI is 1469 rather than 1477 because PR #510's eight new tests are not on main yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
This was referenced Sep 13, 2026
fix(#448): one animator per Tiles wall — and the first real consumer of the #419 SourceRegistry
#511
Merged
…64 round-trip (#521 slice1 T3 complete) Completes Task 3: the compositor exports a DEDICATED keyed-mutex BGRA shared texture for hardware encode (exportEncoderSharedTexture / ensureEncoderSharedTexture), mirroring the vcam tap's producer keying (AcquireSync(0,0)/CopyResource/ReleaseSync(1)) so encode never contends with WinUI's ProgramFrame::sharedTexture consumer. Carried on ProgramFrame::encoderSharedTexture. Uses legacy GetSharedHandle to match the encoder's legacy OpenSharedResource. Encoder consumer keying is AcquireSync(1)/ReleaseSync(0). Proven end to end on the dev rig (RTX 4090): compositor renders solid mid-gray -> dedicated keyed-mutex texture -> MF hardware H.264 MFT (VideoProcessor BGRA->NV12) -> Annex-B bitstream -> ffmpeg decode. Test asserts a keyframe, non-empty bitstream, and the decoded coded Y plane within 16 of the encoded gray (measured 128.0 exactly over 12 frames). Reads the yuv420p Y plane, not -pix_fmt gray, to avoid limited->full range expansion. Self-skips without a hardware MFT or ffmpeg. Full native suite: 939 passed, 0 failed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…m to muxer; env fallback (#521 slice1 T4) The RTMP/SRT sender now decides its encode path once at process start via the pure chooseStreamEncodePath policy (folded into GpuVideoEncoder.h): GPU-direct only when the platform has an MF encoder, a hardware session is (probably) available, the resolved codec is H.264, the compositor is exporting the dedicated encoder texture on the starting frame, and COREVIDEO_GPU_ENCODE is not "0". Any base blocker keeps precedence in the logged reason. On the GPU path the sender starts the hardware encoder BEFORE FFmpeg (so encoder->start() is the real capability gate; failure downgrades to raw and FFmpeg is never launched in bitstream mode with nothing feeding it), builds FFmpeg args with videoBitstreamInput=true (-c:v copy muxer, T2), submits each frame's compositor encoder texture to the encoder, and the encoder's sink writes the ~6 Mbps bitstream to FFmpeg stdin. stopFfmpegProcess stops and joins the encoder before closing the pipe (no write-after-close). The raw NV12/BGRA pipe stays the fallback for every non-capable machine, non-H.264 codec, and the env toggle. Every path selection is logged ([gpu-encode] path=...). Availability never REFUSES on a pending probe (the TESTER rule); start() is the gate. Tests: chooseStreamEncodePath (5 cases: gpu-direct, base-blocker precedence, non-H.264, missing encoder texture, env toggle). The runtime submit-vs-raw routing is proven by the Task 6 localhost acceptance gate (real core + real FFmpeg), not a mocked-process unit test. Full native suite: 944 passed, 0 failed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…estarts (#521 slice1 T5) The encode loop now distinguishes a genuine device loss from a transient encode miss: on a convert/ProcessInput failure it reads ID3D11Device::GetDeviceRemovedReason() and, when the device is removed/reset/hung, sets healthy_=false and breaks with a [gpu-encode] device lost line. submit() already returns false once unhealthy, so the sender reports a video-write failure and the existing OutputDestinationSupervisor restarts it, which re-decides the encode path (Task 4) against the recreated device. Capacity is already priority-1 for slice 1: the stream is the only GPU encoder, and the sender gates GPU-direct on the EncoderCapacityProbe session ceiling (sessionAvailable -> "no-free-encoder-session" fallback, covered by the existing policy tests) without ever refusing on a pending probe. Test: the submit-fails-when-not-running contract (the supervisor's restart trigger) on the real impl. A real TDR is not injected (matching the shell device-loss recovery's documented posture); the classification is verified by inspection and the round-trip test proves healthy() stays true on the happy path. Full native suite: 945 passed, 0 failed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…-direct stream at realtime (#521 slice1 T6) Adds scripts/validate-gpu-encode.mjs (streams the fake-engine program to a localhost SRT sink; FAILS unless the GPU path is taken and the received stream is >=58fps and the sink's -stats speed >=0.97x; --force-raw confirms the fallback) and the CLAUDE.md section. Gate result on the dev rig: path=gpu-direct, received 60.0fps of 60, realtime. Writing the gate surfaced four real defects, each fixed here (the raw-pipe path capped at ~0.76x; this is what makes GPU-direct actually keep up): - Compositor exports the dedicated encoder texture whenever streaming, BUFFERED or not (it was gated !buffered, so with the default 3-frame program buffer the sender saw no encoder texture and fell back to raw). The stable handle rides the program buffer to the sender; the copy is the latest composed frame, so the stream taps live pixels. - Encoder consumer keyed-mutex timeout 4ms -> 34ms (~2 frames): 4ms missed the 16ms production cadence on almost every frame, wasted the MFT input slot and starved the encoder to ~2fps. - Encode loop no longer waits for a fresh submit per NeedInput (that serialized with the mutex wait to ~30fps); it reads the latest handle and lets the keyed mutex pace it to the producer's 60fps. - FFmpeg bitstream input: -use_wallclock_as_timestamps 1 -r <fps>. A live Annex-B stream on a pipe has no container timestamps; -r alone left stream 0's PTS unset once audio was mapped, and -c:v copy then muxed a stream the endpoint read at 0x/stalled. Wallclock stamps each arriving access unit at realtime (monotonic for a 60fps feed). Verified: validate-gpu-encode.mjs PASS (gpu-direct, 60.0fps realtime) and --force-raw PASS (fallback still streams); mac-show-drill --load 8 PASS (p50 27.8ms, 100% delivered, no regression); full native suite 945 passed, 0 failed. Live RTMP to real YouTube remains the final manual acceptance step. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
…dable (#521) Adds -stats -stats_period 1 to both the bitstream and raw ffmpeg arg branches. This is what makes the sender's realtime keep-up (speed=x, media-time/wall-time) readable from ffmpeg's own stderr — used to confirm GPU-direct streams to real YouTube at ~1.02x vs the raw path's ~0.76x. Matches the deploy branch's intent; the raw path lacked it on this branch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ
iamfatness
added a commit
that referenced
this pull request
Sep 14, 2026
…of the #419 SourceRegistry (#511) * docs(spec): the Tiles wall as a persistent source (slice 2, #448) Answers #447 (T5.1) by naming exactly which #419 foundations slice 2 consumes, and makes #448 the first real consumer so those foundations can land on main wired rather than as an unwired island. The residual defect: adoptSettledFrom only hands a wall across when every tile is atRest, so a wall taken MID-ANIMATION still re-animates. With two per-bus animators there is no correct answer to that, which is why the fix is structural -- one animator per wall, one texture, buses sample it. Owner decisions recorded in the spec: the wall registers in SourceRegistry under a composed kind (not a capture kind, whose subscription/availability fields are meaningless for a wall); the plan carries ONE tiles-wall layer rather than collapsing expanded tile layers inside the D3D adapter, which pulls some Metal and CPU-preview parity forward deliberately; and the multiview PVW cell moves to sampling the Preview bus texture the way the PGM cell already samples Program. Identity was verified rather than assumed: the shell already emits tiles:<sceneId> (TilesLayerPayloadBuilder.cs:50) and the core takes it verbatim (MediaCore.cpp:1808), so dropping today's redundant sceneId prefix preserves the exact equivalence classes. Gates are recorded with their honest state: the iGPU budget number CANNOT be produced here (needs #425) and Metal will compile and pass CI but not be executed. Neither is quietly skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * docs(spec): composed sources use nullopt, not a struct split (owner ruling) The first draft said the composed kind "carries only Token, displayName, format and publication state", which implied splitting SourceRegistry::Source into a common core plus a capture payload. Investigating the cost changed the answer. SourceRegistry::Source is one struct with a kind discriminator, and about five of its fields are meaningless for a wall: no personId, no externalId (there is no SDK handle), it cannot be Departed (a wall is released when unreferenced), and it is never subscribed. Two findings made this more than cosmetic: - The defaults are ASSERTIONS. subscriptionObserved{false} is initialised engaged, and its own comment says nullopt is what means unknown -- so a registered wall would assert "subscription observed = false". That is the shape of #468 and of "absent lifecycle means UNKNOWN, never healthy". - This is a PUBLISHED, golden-tested contract, not an internal struct: test/data/wave1-authority.json is authority-goldens-v2 with ~1,700 lines of scenarios, and a serialized source carries personId, videoAvailable, videoFresh, audioAvailable, audioFresh, audioMuted. Registering a wall forces an answer to "is this wall's audio muted?". Splitting the struct was therefore rejected on cost: it touches every existing #419 consumer and its tests and reshapes those goldens -- #419 surgery smuggled in under a Tiles-wall fix, when the owner asked for a minimal carve. Leaving the fields as-is was rejected too: a field that serializes a false claim is how #468 happened. The fields become std::optional and stay nullopt for composed sources, the serializer omits them, and two new tests pin it: a wall never claims a subscription state, and the existing capture goldens stay byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * docs(spec): correct two wrong facts the spec was approved on Found while gathering detail for the implementation plan. Both errors were mine, and one of them was the argument AGAINST the option I talked the owner out of. 1. The spec claimed SourceRegistry::Snapshot is a published, golden-tested wire contract. IT IS NOT. That snapshot is consumed in-process by ShowPlanGenerator and SceneVersionShadow and never reaches the wire or the session state. The authority-goldens-v2 scenarios belong to ZoomSourceAuthorityAdapter::Observation::Source -- the Zoom ROSTER OBSERVATION synced INTO the registry, carrying videoFresh/audioMuted/personId. A wall registered directly in SourceRegistry never appears there. 2. Because of (1), the stated cost of splitting the struct ("reshapes ~1,700 lines of goldens") was fabricated. Measured: about seven files in the Zoom authority path plus tests. Real but modest. The conclusion still stands after re-deciding with correct facts (owner: keep the nullopt approach), but on NARROWER grounds: subscriptionObserved{false} is initialised engaged while its own comment says nullopt means unknown, so a registered wall asserts "subscription observed = false" -- and that feeds PLAN GENERATION. The split is deferred to slice 3, where more than one composed kind would justify the shape. Also records a finding that applies whichever option is chosen: validRegistration requires a non-empty externalId, so add() returns Invalid for a wall today, and externalConflict matches kind+processEpoch+externalId, so two walls with empty external ids would collide. Both now have named tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * docs(plan): plan 1 of 2 for slice 2 — one animator per Tiles wall (#448) Four tasks, each ending in an independently testable deliverable: 1. SourceRegistry admits a composed source and never claims a subscription state for it (both current rules reject a wall today: validRegistration demands a non-empty externalId, and externalConflict would merge two walls that both have none). 2. TilesWallSource owns one wall's animation, keyed by wall id. 3. MediaCore uses one animator per wall and adoptSettledFrom is DELETED. This is the task that fixes #448. 4. The wall registers as a composed source and the take record proves its continuity with a generation. Split from the spec deliberately. Plan 1 changes NO drawing, so it can land while beta-2026-09-12-c425e3c is still being live-checked; plan 2 (wall texture, one tiles-wall layer, Metal/CPU parity, PVW cell, AtomicTake) changes the render path and should wait for that beta to be shaken out. Each plan produces working, testable software on its own. Self-review caught a type inconsistency worth recording: the first draft had TilesPlanAnimation calling a std::function installed by TilesWallSource, which Task 2 never installed AND which would allocate on the render tick. Replaced by advance() returning bool "did I reset", which the wrapper turns into a generation bump -- so the generation cannot move without the animator moving, and vice versa. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * chore(registry): carve SourceRegistry out of #419 onto main's line, unchanged Prerequisite for plan 1 of persistent-sources slice 2. No behaviour change: the files are taken verbatim from origin/codex/production-realtime-architecture and wired into the existing targets. The carve is clean because SourceRegistry.cpp includes only its own header plus <stdexcept> and <utility> -- zero project dependencies -- which is the condition the parent spec set ("SourceRegistry.h is reused only if it fits cleanly"). Nothing else from #419 is needed: SceneVersionShadow and ShowPlanGenerator consume the registry but are not part of this plan. Verified: its own 22 tests pass, and the full suite is 953 green (931 + 22). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * feat(registry): admit composed sources, and never claim a subscription state for them Also fixes a dangling-reference bug in the ACaptureSourceStillCarriesItsCaptureFields test: `registry.snapshot()->sources.front()` as one expression drops the temporary shared_ptr (and its Snapshot) at the semicolon, leaving `source` a dangling reference. Bind the shared_ptr first, same pattern already used by the other new test in this file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * fix(registry): skip composed sources in retirement/availability, and narrow the composed conflict bypass to externalId Fix round 1 review findings: 1. retireProcessEpoch and setAvailability reached a composed source's fields unconditionally, reintroducing the false claim install() was fixed to remove (nullopt flipping to concrete Departed/false). Both now SKIP a composed source instead of marking/mutating it: retirement has nothing to fence (no provider process, no callbacks) and availability does not apply to a wall at all. Not erased, not marked Departed - a wall's lifetime is scene-reference, released by a separate sweep in a later task. 2. externalConflict's composed bypass disabled the instanceId collision check too, not just the externalId one. Restructured so only the externalId clause is skipped for a composed registration; two composed registrations sharing an explicit instanceId still conflict. Adds 3 covering tests, two confirmed red (via a temporary guard revert) before the fix and green after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * feat(tiles): a wall owns its own animation, keyed by wall id TilesWallSource wraps TilesPlanAnimation and bumps a generation whenever the wrapped animation reports it reset, giving the take record's proof that nothing restarted (spec section 5) as a number instead of an opinion. TilesWallSources maps wall id -> source so both buses asking for the same wall get the same object; lifetime follows "referenced by a scene" via releaseAllExcept, and a recreated wall starts at generation 0 rather than continuing a retired one's count. TilesPlanAnimation::advance now returns bool (true when it reset the animator) instead of void, so TilesWallSource::advance can detect a reset with no std::function/allocation on the render tick. Every existing caller (MediaCore, TilesAnimatorTest) already discards the return value as a statement, so this is a source-compatible widening. Not wired into MediaCore yet - that is the next task. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * fix(tiles): cover TilesWallSource::advance's reset-detection glue Review round 1 finding 1: the original 5 tests never called TilesWallSource::advance - they only exercised noteReset()/generation bookkeeping directly, leaving the actual glue (bump the generation iff the wrapped TilesPlanAnimation::advance reports a reset) completely untested. Add two tests driving advance() directly: a different wall key arriving moves the generation by exactly one (the reset path), and the same wall key arriving twice does not move it. Verified both are load-bearing by temporarily breaking the glue two ways (unconditional noteReset(), and an inverted condition) and confirming each break turns at least one test red before restoring the fix. Finding 2: add the missing <iterator> include for std::next, previously compiling only because MSVC's <map> transitively pulls it in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * fix(tiles): one animator per wall, so a wall taken mid-animation is continuous MediaCore held programTilesAnimation_ and previewTilesAnimation_, and handed settled state from one to the other on a Take via TilesPlanAnimation::adoptSettledFrom. That hand-off refused a wall whose tiles were still flying (mid-flight state had no correct owner with two animators), so a wall taken mid-animation re-animated its entrance from alpha 0 on Program. MediaCore now keeps ONE core::TilesWallSource per wall id (tilesWallSources_, from Task 1/2), shared by whichever bus is looking at it, so there is nothing to hand over. adoptSettledFrom is deleted. TilesPlanAnimation::advance already reports whether it reset (bool return); TilesWallSource turns that into a generation bump. TakeRecordPolicy's wallAdoptedSettled is renamed wallContinuous: it no longer means "state was handed over" but "the wall's generation did not move across this take", computed in MediaCore by comparing tilesWallGeneration() before and after the take-tick's advance. The verdict string "adopted-settled" becomes "continuous". Rewrote the three TilesAnimatorTest.cpp hand-off tests that pinned the old per-bus behaviour: a wall settled on one bus is now trivially settled on the other (same object); the refused-hand-off case becomes "a wall never cued starts cold, at generation 0, in its own object, contaminated by nothing else"; the all-stale-beat guard test is kept as-is (it already tested the shared animator directly, and matters more now that a wipe hits every bus at once). Added the headline regression test (TilesRenderPlanTest.cpp, AWallTakenMidAnimationIsContinuous). A wall's own first tick with content pops in fully opaque by design (TilesAnimator treats a fresh animator's first non-empty sample as pre-existing, not entering — see DifferentWallCannotReuseAnotherWallsCachedGeometry), so the test instead joins a member onto an ALREADY-ESTABLISHED wall and takes it before the join settles. Verified red-before-green by temporarily restoring the base (aa35f4f) per-bus mechanism: the same scenario failed with the new tile snapping straight to full opacity instead of continuing its entrance, confirming the test exercises the real defect. Closes #448 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * fix(tiles): review round 1 — idempotent reset, read-path phantom-wall, freeze fix, verdict/rect assertions Fixes five review findings on the #448 one-animator-per-wall change: - Finding 0: TilesPlanAnimation::advance's not-present/disabled early return now reports whether it actually reset (key_ was non-empty), not "true" unconditionally — a repeated disabled tick is a harmless no-op, not a generation tick-counter. - Finding A (Critical): the preview GPU-composite applyLatest call used forWall(), inserting a phantom ""-keyed wall for any preview scene with no wall, under coreMutex, every render tick. Switched to find() + null-check, matching the two multiview sites. - Finding B: the double-advance guard keyed on wall-id equality alone, so a wall with animateLayout=false on Program and true on Preview at the SAME id was advanced by neither branch (frozen). Hoisted `programAdvanced = present && animateLayout` and gated preview's advance on `!programAdvanced || previewWallId != programWallId`. - Finding C: advance() was only called when animateLayout was true, so a disabled wall's sampled_ was retained forever and kept feeding stale geometry to the multiview PGM cell and the preview composite. Restored the unconditional call whenever the wall is present, letting `enabled` decide reset-vs-sample inside advance() (safe now under Finding 0's guard). - Finding D: added an end-to-end assertion in the headline test that the take record itself reads wall=="continuous", verdict=="cut" — not just the raw generation number. - Finding E: added rect-continuity assertions for tiles whose opacity was already 1 at the mid-flight tick (only the newly-joined tile had a ramping alpha; the pre-existing two tiles' RECTS were mid-spring from the layout change, unasserted before this). Self-caught while implementing Finding C: widening the advance gate to "present" alone broke ATakeIsRecordedWithBothSidesOfTheWall — a wall id seen for the first time starts at generation 0, and with Finding 0's idempotence a disabled first call also reports "did not reset", so raw generation equality misread a brand-new non-animated wall as continuous. Fixed by requiring wall EXISTENCE (find() before forWall()), not just generation equality. Self-caught while implementing Finding E: the rect-continuity check first failed against the CORRECT implementation too, because the shared wall's 100ms clamped-floor duration converges audibly close to target within a few ms of real test-process overhead. Added wallSceneWithDuration() and drove the test at the animator's 2000ms clamp ceiling for a reliable margin. Re-verified red against the reverted base mechanism with both new teeth (Finding D's take-record assertion and Finding E's rect check both fail); re-verified green on the fixed tree, 968/968, three consecutive runs with no flakiness. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * fix(tiles): review round 2 — one advance per wall per tick, not two with contradictory enabled Re-review confirmed round 1's wallExistedBefore term and the 2000ms tolerance retune were both sound. It found one new Important finding introduced by Findings B and C combining: a wall present on both buses with DISAGREEING animateLayout (Program false, Preview true, or vice versa) was advanced TWICE in one tick on the SAME shared object with contradictory `enabled` values — Program's call (enabled=false) reset it, then Preview's separate call (enabled=true) saw an empty key and reset it AGAIN. Net per tick: generation +2, no animation ever completing, the object re-adopted from scratch every tick. Not a verdict inversion (it read the conservative "rebuilt"), but incoherent under the one-animator-per-wall model this task exists to establish. Restructured the take-tick block so exactly one advance() call happens per wall per tick, true by construction: - `sameWall` decides whether Program and Preview name one object or two. - When they share an id, `enabled` is the OR of both buses' animateLayout — a wall animates if EITHER bus wants it to — and there is exactly one advance() call to carry that decision. - Preview only gets its own separate advance() call when it names a DIFFERENT wall id (a genuinely different object). Kept everything the round-1 re-review confirmed: the `had` idempotence guard in TilesPlanAnimation::advance, find() on every read path, the wallExistedBefore existence check, and the unconditional-whenever-present call shape that fixed Finding C (now applied symmetrically to both branches). Added TilesRenderPlan.ASharedWallWithDisagreeingAnimateFlagsAdvancesAtMostOncePerTick — no prior coverage exercised this configuration, which is why it took two review rounds to surface. Verified red against c11862d (generation climbed every tick) and green after this restructure, across three consecutive full suite runs (969/969, 0 failed). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * fix(tiles): review round 3 — no wasted preview plan build, Program wins for a shared wall, drop empty-id exemption Three fixes from re-review of round 2's restructure: Finding 1 (performance): the preview branch's `!sameWall` gate lost its `animateLayout` check, so buildPreviewCompositorRenderPlan(videoFrames) — a full buildRenderPlanForScene deep build, ~13 strings per layer, plus the paused-clip-cue pass — ran every render tick under coreMutex for any disabled preview wall (the DEFAULT) with a different id from Program, then was thrown away: advance() returns at its very first line when !enabled and never touches the plan. Same class of waste the "Task 4 review fix (I6)" comment a few lines below already exists to prevent. Now the deep plan is only built when previewEnabled is true; the disabled path passes the already-built program `renderPlan` instead (advance() never reads it there). Finding 2 (behaviour, RULING): `enabled = programEnabled || previewEnabled` let a PREVIEW-side animateLayout toggle start motion on PROGRAM for a shared wall, with no take involved, because the shared advance samples straight into the program renderPlan. This codebase's bedrock rule is that an off-air Preview look never changes what is on air (CLAUDE.md: "an off-air Preview look can never take video ... from a Program source") — the preview scene is an operator-editable draft (S2b), so a draft edit reaching Program is a live-show hazard, and Program is inherited by the vcam, every recording and every stream. Changed to `enabled = programEnabled` — Program always wins for a shared wall. Cost-free thanks to round 1's Finding C: a disabled shared wall still resets (idempotently), sampled_ clears, and applyLatest no-ops on the mismatched key_, so Preview simply falls back to raw (non-animated) geometry rather than stale rects — nothing on air moves because of an off-air edit. Finding 3 (minor): dropped the `!programWallId.empty()` term from `sameWall` — two empty ids are the SAME map entry (TilesWallSources keys on the string), so excluding them re-opened the exact two-advance-on-one-object shape round 2 closed. No generation effect, but the wrong shape regardless. Rewrote the round-2 test to assert the NEW property directly: with Program's animateLayout=false, a wall shared with an animateLayout=true Preview never leaves generation 0 — not "settles after climbing once" (round 2's OR behaviour), which the rewritten test now catches if ever restored (verified by temporarily reintroducing the OR: the test fails on tick 0, as expected). Verified: covering suites green (TilesRenderPlan 29, TilesAnimator 11, TakeRecord 11, RenderedSceneAttribution 3), full binary 969/969 across three consecutive runs, check-string-escapes.py clean. Deferred per coordinator instruction, NOT fixed: forWall() admitting an empty wall id on the write path; the rect-tolerance/duration headroom. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * fix(tiles): review round 4 — remove the foreign-plan foot-gun, add a real precondition Finding 1 (Important, coordinator's own prescribed shape): the disabled preview-wall branch passed the PROGRAM render plan into a PREVIEW wall's advance() call. Correct today only because advance() returns at its very first statement when !enabled — an invariant living in a different file from the call site depending on it. If that early return is ever reordered or gains code above it, applyLatest would silently rewrite rect/opacity on every tile:* layer of the PROGRAM plan (the same object handed to compositor->render() and cached into lastRenderPlan_): on-air geometry corruption inherited by the virtual camera, every recording and every stream. Removed the hazard rather than documenting it further: - TilesPlanAnimation::releaseIfIdle() — plan-free by construction, cannot ever read a plan because it takes none. Same idempotent generation contract as advance()'s early return (had = !key_.empty() before reset). - advance()'s early return now just calls releaseIfIdle(). - TilesWallSource::releaseIfIdle() wraps it and calls noteReset() on a real release, matching advance()'s generation semantics exactly. - MediaCore's disabled-preview branch now calls tilesWallSources_.forWall(previewWallId).releaseIfIdle() — no plan argument exists to misuse. - Documented advance()'s `plan` parameter as read ONLY on the present-and-enabled path, so the next person knows releaseIfIdle() is the only correct call when there is no real plan for a wall. - Left the all-stale early return (`targets.empty() && !sampled_.empty()`) untouched — it runs after the key_ assignment and was never part of this hazard. Finding 2 (Minor): the round-3 test was entirely negative assertions (generation == 0), which pass vacuously if the shared configuration itself stopped existing. Added preconditions: Program's wall must actually render "tile:zoom:1" (forced via the existing setTilesMemberFrameAgesForTest seam, which only rebuilds lastRenderPlan_ and cannot disturb tilesWallSources_/generation), and Preview's scene must have landed on the SAME "tiles:s" wall id with animateLayout true — before the zero-generation assertions run. Verified: covering suites green (TilesRenderPlan 29, TilesAnimator 11, TakeRecord 11, RenderedSceneAttribution 3), full binary 969/969 across two consecutive runs, check-string-escapes.py clean. Generation semantics are unchanged by the refactor by construction (releaseIfIdle IS the old early return, extracted) — the round-2/3 tests pass for the same reason as before. Still deferred per coordinator instruction: forWall() admitting an empty wall id on the write path; the rect-tolerance/duration headroom. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * feat(tiles): register the wall as a composed source, releasing it when unreferenced The wall is the first real consumer of SourceRegistry's Kind::Composed (#448 slice 2, task 4). Registers as Composed the tick a wall becomes live (sourceId = its layerId, externalId empty, a fixed core-process epoch) and releases it the tick nothing on either bus names it any longer, in lockstep with tilesWallSources_.releaseAllExcept — the same "named by a live scene" lifetime, one level up. registeredWallIds_ is the idempotence guard so a live wall's steady-state tick never touches the registry mutex. SourceRegistry had no way to actually remove a composed source (real sources are tombstoned via retireProcessEpoch/setAvailability, which explicitly SKIP composed sources with a comment deferring removal to "a separate sweep (later task)"). Added SourceRegistry::removeComposed, which erases outright (never tombstones) and refuses non-Composed kinds - the wall has no provider process to fence, so a tombstone would be a permanent, meaningless entry that also makes add() answer Conflict forever for a freed wall id. Per explicit ruling, this task does NOT touch TakeRecordPolicy or the take record's sources[] - that continuity fact already exists via wallContinuous, and a second representation of it is exactly what this codebase forbids elsewhere (one status machine per fact). Tests: SourceRegistryComposedTest.cpp (removeComposed: erases, frees the id for reuse, refuses non-composed, NotFound on unknown) and TilesRenderPlanTest.cpp (a live wall registers as Composed with all five capture-only fields nullopt; an unreferenced wall is gone from the registry; a released-then-re-cued wall gets a new instanceId; a wall staying live across many ticks holds exactly one registry entry). Full suite: 977 passed, 0 failed (was 969). Red-before-green verified by reverting MediaCore.cpp/.h to HEAD with the new tests in place (compile failure), then restoring. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * fix(tiles): review round 1 - falsifiable idempotence test, retry-on-failure, alloc-free by construction Fix round 1 of the Task 4 wall-registers-as-composed-source review. Finding 1 (Important): ALiveWallHoldsExactlyOneRegistryEntryAcrossManyTicks asserted a count std::map makes structurally impossible to violate, and was near-unfalsifiable - dropping the idempotence guard entirely (add() answering Conflict on every tick, taking the registry mutex 60x/s on the render path) still passed it. Replaced with ALiveWallKeepsTheSameRegistryIdentityAcrossManyTicks, asserting the wall's instanceId is identical across 25 ticks - the realistic regression (remove-and-re-add every tick) mints a fresh instanceId, which this catches and a count cannot. Verified genuinely falsifiable by simulating that exact regression and confirming red, then restoring and confirming green. Finding 2 (Minor): a registration that failed (Invalid is reachable, not just Conflict/Exhausted) was remembered as registered anyway, so it was never retried while the wall stayed live. Now the guard id is only added on Result::Applied, so the next tick retries a failed wall. Finding 3 (Minor): registeredWallIds_.insert(id).second is not guaranteed allocation-free on an already-present key on every implementation (MSVC's unordered_set has historically built the node before detecting the duplicate). Switched to contains()-then-insert so the render-path no-allocation rule holds by construction. Finding 4 (Minor): three SourceRegistry comment fixes - removeComposed takes a bare SourceId deliberately (sole-owner lifetime, no replace()- style fence); retireProcessEpoch's composed-skip comment names removeComposed instead of "later task" and explains why a tombstone was never actually reachable (setAvailability refuses Composed, and a permanently-nullopt availability makes externalConflict's Departed check permanently true); Snapshot::sources notes composed sources vanish rather than tombstone. Finding 5 (Minor, merge-blocking): added a CLAUDE.md section documenting the composed-source erase-vs-tombstone asymmetry and both reasons for it, per this repo's docs-updated-is-part-of-done rule. Full suite: 977 passed, 0 failed (x2 consecutive). Escape scanner clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * test(rig): make the fake engine's motion measurable by the soak's luma gate `live-meeting-soak.mjs` gates "did the picture move" on frame-to-frame YAVG exceeding 0.05, because 8995 frames of flat luma once passed every validator that counted frames. The fake engine's motion was a ONE-ROW band that brightens row N and restores row N-1, which changes the frame mean by ~0.0007 — so a rig delivering a measured 60.0/s scored 0% of frames moving, and the gate could never pass on the only engine that can drive this soak headlessly. A gate no harness can satisfy is unfalsifiable, which is worse than no gate. Pulse a BLOCK instead: h/8 rows stepping 7 luma per frame is ~0.9 YAVG at the frame level, 18x the threshold, at 1/8 the cost of the full repaint this function deliberately avoids (that repaint made the PRODUCER the bottleneck at ~22fps/source). Offset by participant id so a wall's tiles carry different values while all advancing together. Measured after: 5 minutes, 20 takes, 8 sources — movingFrameRatio 0.0 -> 0.998, LIVE-MEETING SOAK PASS, with the take property intact (20 cut, 0 rebuilt) and 60.0/s render, 0 deadline misses, 0 dropped video. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * fix(#448): refuse setSubscription on a composed source, bound the wall-registration retry, correct the defect narrative Final-review findings on the #448 slice, plus the documentation defect the review surfaced. setSubscription refused Kind::Composed nowhere, while CLAUDE.md asserted it did. An observed:true call was already refused as a side effect (a nullopt availability is not Available), but requested:true with observed nullopt applied cleanly and turned a NOT-APPLICABLE field into a concrete claim — the exact lie the nullopt fields exist to prevent. Nothing in the tree calls it for a wall, so only the docs were wrong, which is how an invariant rots. Red-verified first. The wall registration loop retried a PERMANENTLY invalid id forever. A failed add is deliberately not remembered as registered so transient failures retry on the next liveness transition — but an id refused on its SPELLING can never become valid, so that retry became a registry-mutex acquisition plus an unbounded log line on every render tick. SourceRegistry::kMaxIdBytes is now public (one declared bound, used by validRegistration) so a caller can tell the two cases apart; unregisterableWallIds_ skips the permanent ones once and loudly, and warnedWallRegistrationIds_ bounds the retryable failures' line to once per id while keeping the retry. Both are pruned on the same liveness rule as registeredWallIds_. THE DEFECT NARRATIVE WAS BACKWARDS in CLAUDE.md, the spec, the plan and the headline test's own comment: a reset does not "replay from alpha 0". TilesAnimator treats a reset animator's next non-empty sample() as an ADOPTION, so the wall SNAPS TO ITS FINAL STATE — alpha pops to 1, mid-spring rects jump to settled. The consequence is not cosmetic: EXPECT_GE(alpha_after, alpha_before) is satisfied by a snap just as well as by continuity and catches NOTHING (the first draft of that test passed against the unfixed code). The falsifying assertions are the ones bounding the other side — post-take alpha below 0.9, and a tile already at opacity 1 keeping its mid-spring rect — and those are what was verified red. Documented at all four sites, with the honest limit that the generation-equality check is not independently falsified by that revert. Also: CLAUDE.md claimed build-dev is a single-config generator. It is Visual Studio 18 2026 with CMAKE_RUNTIME_OUTPUT_DIRECTORY pinned for every config, so Debug and Release write the same exe path and a missing --config Release silently installs a Debug core. That cost a false regression report on 2026-09-12 (coreMutex 1% -> 81%); the tell was uniform inflation across trivial stages and an 8,322,560-byte binary against Release's 2,168,832. Documented with the check. Plus: the registry section now says it has a production writer and no production reader yet, and the shared-wall applyLatest coupling is commented at both multiview cell sites and filed to plan 2. Tests: 978 passed, 0 failed on the Release core. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * docs: close the #448 slice-2 ledger, file the residual minors, update the backlog The final gates re-run on the post-fix binary (978 tests, SHOW DRILL PASSED, LIVE-MEETING SOAK PASS at 20 takes / 0 rebuilt), the soak motion gate's unfalsifiability and its ruling, the environmental cause of the first soak failure (we join as host, so killing the app ended the meeting), and the record that the defect narrative was backwards in four places. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ * docs: record the finalizer-thread XAML release crash (#513) — mechanism confirmed, trigger not reproduced Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014yuH7EMvWCWdkhvevrtMyJ --------- Co-authored-by: Claude <noreply@anthropic.com>
iamfatness
marked this pull request as ready for review
September 14, 2026 10:58
iamfatness
added a commit
that referenced
this pull request
Sep 14, 2026
This was referenced Sep 14, 2026
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
2 participants
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.
Problem
The first production soak exposed a shared-state and shared-execution architecture where roster refreshes can substitute sources, Preview and Program can disagree, Take acknowledgement can be mistaken for rendered pixels, and UI, monitor, output, or writer stalls can interfere with the show.
What this branch now contains
Two things, and they are worth separating.
1. Pre-cutover architecture foundations (the original scope). Generated identity, lifecycle, media evidence and strict Take contracts; authoritative
ShowStateandSourceRegistryprimitives; deterministic plan preparation and an idempotent atomic Take coordinator; bounded authority shadow and a rationalShowClock; strict native Take JSON and managed source-authority admission; immutable exact-source references with fail-closed command behaviour; bounded immutable scene-version storage; exact Zoom producer identity bound to the same immutable pixel allocation; and the Wave 2 execution substrate (SourceFrameLease,ProgramRenderWorker,ProgramPacketPlayout).The live product still uses the legacy render/Take route. These remain unregistered: they are constructed by their own tests and nothing else.
MediaCore.cppandJsonRpcServer.cppcontain no reference to any of them. See §1 of the completion plan for the precise state, which differs from how the execution record reads.2. Beta stability and quality-of-life work (added 2026-09-09). Windows-first, aimed at external operators running real shows on machines we cannot see:
requested → preparing → producing → stopping → finalizing → completed/failed/interrupted,producingrequires fresh observed progress, and senders have a lifecycle for the first time.A finding this work surfaced
Program is not isolated from monitor rendering. Program, Preview and Multiview share one render thread and one D3D immediate context. Measured on an RTX 4090: a sustained Preview overrun of ~1.5 frame periods costs Program half its produced frames (121 → 65 per 2 s window). The program buffer does not help, because it protects delivery timing for frames that were produced, and these were never rendered. The proper fix is the monitor compositor split; a cheap mitigation (the multiview tick divisor, currently at full rate) exists.
Validation
Local, Windows: native 967, MediaCore 2231, WinUI 1292, vitest 3878, scripts 163, contract check and typecheck clean.
Loaded 8×1080p60 drill: sustained 60.0 fps, 0 dropped, worst frame 17.2 ms, core-lock over budget 2%, operator command p50 4.5 ms / p99 13.9 ms.
CI green on every job except
mac-show-drill, which fails onmainas well and is a real recording defect belonging to Wave 3. macOS is not the first-release platform.Plans
docs/production-realtime-execution-plan.mddocs/production-realtime-completion-plan.md🤖 Generated with Claude Code
https://claude.ai/code/session_01YASnsbX3oQRZRxUNroqjY5