React Native: open the live producer contract and give saved video a media clock - #91
Open
lucas-fochesatto wants to merge 15 commits into
Open
React Native: open the live producer contract and give saved video a media clock#91lucas-fochesatto wants to merge 15 commits into
lucas-fochesatto wants to merge 15 commits into
Conversation
Records the investigation behind a single React Native interface that takes a model and owns everything else, for both live camera and saved video. The contracts it needs already exist and are already correct: MediaFrameSource, MediaFrameProcessor returning a DetectionFrame, and createMediaSession built on them. Three session implementations exist and the two production lanes bypass the generic one, for two concrete reasons this plan sequences around: - media-session-core.ts has zero worklet directives; its frame path awaits the processor and the renderer, which a worklet frame callback cannot do. - DetectionMask resolves only to compressed RLE, whose payload is a string. Encoding a full-resolution mask into it per frame is not affordable, which is why the native lanes grew a parallel serialized-detection shape. Phase 0 removes both. Later phases open the producer contract, make the timing policy an explicit parameter, and converge the lanes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
DetectionMask resolved only to CompressedRleDetectionMask, whose payload is a counts string. That is the right cold-storage form for the browser package's IndexedDB path and for fixture manifests, but a live producer cannot pay a full pass plus a string allocation per mask per frame. That cost is why the React Native lanes grew their own flat serialized-detection shape instead of publishing a DetectionFrame. DenseBitmapDetectionMask carries the bytes directly. Its transposed flag describes buffer layout rather than any one runtime's quirk, so a producer whose model emits a rotated buffer can publish without an upright copy. decodeDetectionMask() normalizes both encodings. The upright dense path returns the caller's buffer rather than copying it, which the tests pin: copying there would defeat the reason this encoding exists. Consumers taking the union now route through it. They previously called decodeCompressedRleMask, which throws for anything else, so the widened union would have failed at runtime in nine places. decodeCompressedRleMask stays strict, and the three helpers that literally construct RLE now say so in their return type, which lets encodeDetectionMaskPayload accept only what it can actually serialize. Widening the union also means an untyped mask literal infers the whole enum rather than a member, so several test fixtures needed annotations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
createMediaSession has zero worklet directives: its frame path awaits the processor, the renderer's prepare and present, and the packet store. A worklet frame callback has no Promise boundary to spend, so the generic session was structurally unusable by the two lanes that need it, and both grew their own pump instead. MediaFrameProcessor and MediaRendererAdapter become discriminated unions. A host declares sync: true to state that its implementation returns directly; the session then calls it inline, with no microtask between producing a result and the next cancellation check. Existing implementations satisfy the async branch unchanged. This follows PreparedFrameStore's existing presentNow and discardNow naming: the same operation, offered in a form a worklet can reach. disposePacket belongs in the renderer's sync half even though it is not obviously per-frame. The session hands packets to PreparedFrameStore, whose releaseNow throws on a disposer that returns a Promise, so a sync renderer must also get a sync disposer or every synchronous release fails at runtime. Tests assert the property rather than the shape: a fully sync session orders process, prepare, present ahead of a microtask queued during processing, while the async default lets that microtask land first. Two consequences worth knowing. A class can no longer implement MediaRendererAdapter directly, since it is now a union; class-based adapters implement one branch. And the type system cannot express the worklet directive requirement, so that stays a documented contract note. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The flag was named for a 90° clockwise rotation but decoded a plain transpose: it read stored[x * height + y] and omitted the y flip. The name and the formula disagreed, and because the test asserted the formula, it pinned the wrong convention rather than catching it. The repo already had the right answer. The React Native ID-mask fill loops sample maskRotatedCw buffers as stored[x * storedRowWidth + (storedRowWidth - 1 - y)], which is a true 90° clockwise rotation, and that path is device-proven. Decoding a transpose instead would have mirrored every mask the moment a producer published one. Renames transposed to rotatedCw so the field matches both the existing vocabulary and what it now does, and fixes the decode to include the flip. Found while writing the Phase 1 producer, which maps maskRotatedCw onto this field directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
First step toward one live interface that takes a model and owns the rest. ReactNativeLiveDetectionProducer returns a DetectionFrame rather than the package's flat serialized shape. That replaces the closed inferenceMode enum with something open: the renderer draws whatever geometry a detection carries, so a mask becomes an ID-mask fill and keypoints become vector markers. Adding a task becomes adding an adapter instead of editing the package. createExecutorchLiveSegmentationProducer is the first implementation, and it absorbs every ExecuTorch quirk that currently leaks outward. Bboxes leave "portrait screen space" and become core's center-based Rect. Masks publish as DenseBitmapDetectionMask with rotatedCw set, so the model's buffer is handed over in place rather than copied upright per frame — the reason the dense encoding exists. Class color is dropped on purpose: core detections carry no styling, and presentation already resolves color from className. One test decodes a produced rotated mask and compares it against the sampling formula transcribed from the ID-mask fill loops. That ties the core decoder to the convention the device-proven fill path uses, which is exactly the drift that produced the previous commit's bug. The live hook still consumes the old processor; switching it over and removing inferenceMode is the next step. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Second and last adapter needed before the live hook can take one producer instead of a task enum. This one is thin, and that is the point: the pose path already returned a DetectionFrame, so it only needed a named entry point under the shared contract. The distance between the two lanes was never the data shape — it was that segmentation published a flat vendor-shaped list while pose already spoke core's language. The live hook currently repeats the runner call and the frame conversion inline rather than using createExecutorchLivePoseProcessor. Naming the producer is what lets that duplication move behind the adapter in the next step. One test asserts the producer and the processor return equal frames, so the wrapper cannot drift from what the hook does today. Another pins that pose detections carry keypoints and no mask, which is what lets the renderer branch on geometry instead of on a task enum. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… shape The live ID-mask fill loops and the instant-CV rule evaluation read a flat, bbox-centric detection shape. Converting a producer's DetectionFrame into it lets the live hook switch to the vendor-neutral contract without rewriting the hot path in the same change — and that path is what currently runs on device. Two tests assert the whole point: a producer plus this bridge yields exactly what createExecutorchLiveSegmentationProcessor emits today, for upright and non-upright frames alike. If those hold, switching the hook cannot change what gets drawn. The conversion never touches mask bytes. Buffers cross by reference, so the per-frame cost is a few small objects rather than anything proportional to resolution. Three deliberate choices. An RLE mask is treated as no mask instead of being decoded, because paying a per-frame decode is the cost the dense encoding exists to avoid. A detection without a rect is dropped, since the flat shape indexes masks through the box and pose detections render through the vector lane. And the bridge stays out of the barrel: it is internal scaffolding on its way out, not new public API. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The live hook asked the host to declare a task — inferenceMode plus a segmentationProcessor slot and a pose slot, both typed with ExecuTorch names in the package's public options. Every new task meant editing the package, and a second model runtime had nowhere to enter. It now takes one ReactNativeLiveDetectionProducer and branches on the geometry that came back. Keypoints render as vector markers; a box renders through the ID-mask fill. The package no longer knows, or needs to know, which task is running. That collapses real duplication. The hook was calling the pose runner inline with ExecuTorch's exact argument shape and converting the result itself, duplicating createExecutorchLivePoseProcessor. Both lanes now make the same single producer.process(frame) call, and the vendor argument shaping lives in the adapter where a second runtime can sit beside it. The demo keeps its mode toggle, which is the point: choosing a producer is a demo concern. Nothing tells the package about it. Two limits worth stating. A frame carrying both keypoints and masks renders only the keypoints, because the two lanes still evaluate different extension rules; unifying that is separate from removing the enum. And createExecutorchPoseKeypointInstructions is still imported by name inside the hook — it is generic keypoint drawing wearing a vendor name, and renaming it belongs with the remaining orientation cleanup. ReactNativeLiveInferenceMode is removed rather than deprecated. Leaving a dead task enum exported would invite exactly the coupling this change undoes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…dapter createExecutorchPoseKeypointInstructions read only core types — DetectionFrame, KeypointDrawInstruction, KeypointVisibility. Nothing about it was specific to a model runtime. It carried a vendor name because that is where pose support was first written, and that name was the last thing tying the live hook to a particular producer. It moves to renderers/keypoint-draw-instructions.ts as createReactNativeKeypointDrawInstructions. The hook now imports zero adapter modules, so no file outside adapters/ mentions ExecuTorch. The vendor-named export is removed rather than aliased. Keeping it would preserve the impression that keypoint drawing belongs to one runtime, which is the confusion this phase set out to remove. A new test completes Phase 1's first exit criterion, which the previous commits asserted but never proved: it defines a producer for an invented runtime, imports nothing from adapters/, and drives both lanes — a masked box through the fill bridge and a keypoint skeleton through the draw instructions. If adopting a new runtime required editing package internals, that file would not compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Notes the two things a reader would otherwise have to rediscover: detections still reach the ID-mask fill through a shallow bridge to the older flat shape, and a frame carrying both keypoints and masks renders only the keypoints. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…aptures Only the pose lane survived on device after the producer switch. Segmentation went through two constructs the package's own conventions forbid inside a worklet, and both are invisible to Node tests. The segmentation producer called readExecutorchFrameTimestampSeconds directly from its worklet body while carefully capturing every other module helper into a const first. That helper is declared after the factory, and the Worklets Babel transform does not preserve function hoisting. The bridge had the same shape plus two more: it called serializeReactNativeLiveDetection before that function was declared, and it compared against the imported DetectionMaskEncoding enum object. The repo already avoids enum captures in the isolated runtime — keypoint instructions use a literal marker shape for exactly this reason. Fixes: capture the timestamp helper alongside the others, order the two converters so the callee is defined first, drop the module-scope EMPTY_MASK, and narrow structurally with "data" in mask instead of by the encoding tag. Structural narrowing also avoids comparing a string enum member against a bare literal, which does not typecheck. Every one of these passed in Node. Worklet serialization only fails on device, which is why this needed a phone to surface. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Switching modes kept inferring with the previous model for a moment. The cause is a design rule this change had broken. react-native-live-rendering records that per-render configuration reaches the package worklet through shared values precisely so useFrameOutput does not re-serialize and swap the camera callback. Selecting a task used to be a shared-value read, so a mode switch was visible on the very next frame. Making the producer a useCallback dependency turned it into a callback swap: until VisionCamera installs the new closure on the camera thread, the camera keeps invoking the old one, which captured the old producer. The models were never the delay — the demo preloads both and retains them for the session. Each closure now captures its own producer generation and compares it against a shared value that React updates as soon as the swap is known. A superseded closure skips the frame instead of running a stale model, which also matches what the old code effectively did during a transition. This makes the swap window inert rather than removing it. Removing it would mean carrying the producer itself in a shared value, and it holds a JSI HostFunction that is not safely serializable across isolated worklet runtimes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
react-native-live-rendering records a hard-won constraint: a JSI-backed runner must not be hidden inside a second processor worklet that is then captured, because the outer function can serialize successfully while the recursively captured HostFunction becomes non-callable on the isolated runtime. createExecutorchLivePoseProducer did exactly that. Its process() was a worklet capturing a processor whose own process() was a worklet capturing runOnFrame — two layers over the runner, where one is the proven depth. The segmentation producer was already at one, matching the processor design that predates this work. It now returns the processor itself. The pose path already produced a DetectionFrame, so the shared contract only ever needed a named entry point, and the pass-through worklet bought nothing while costing the constraint. Worth noting how this was missed: the wrapper was reviewed as trivially thin, and thinness read as safety. The risk was the layer itself, not its contents. It also worked on device, so device validation would not have caught it either — the documented failure mode is code that serializes and then misbehaves. The equality test now also compares function source. Node cannot observe worklet depth, so that is the closest available signal that no wrapper has been reintroduced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Saved-video playback ran at inference speed because that was the only mode: every decoded frame was inferred before the next was presented, so a ten second clip took as long as the model needed. That is the right behaviour for producing a fully annotated video and the wrong default for watching one. The session now takes a clock. `analysis` is the existing behaviour and stays the default, so nothing changes for callers that do not opt in. `media` presents frames on their own timeline: a frame past its moment is dropped, a frame not yet due is waited for, and inference runs only when there is slack to pay for it. Frames between inferred ones reuse the held detections. The inference rate self-regulates rather than following a hardcoded interval. Running a model puts the session behind, so the next frames present cheaply until the schedule recovers, at which point another inference becomes affordable. A test walks twelve frames with realistic costs and asserts what falls out: inferences never land back to back, and playback tracks the media timeline instead of running long. The policy is a pure module rather than arithmetic inline in the pump worklet, because nothing inside that worklet is reachable from a test. Two honest limits. The wait spins: the pump runtime has no sleep primitive. That still costs less than the analysis clock, which never waits and never stops inferring, but pacing presentation off vsync would remove the spin and is the better long-term shape. And holding the last detections means overlays lag their frame by up to a few frames; propagating with a tracker is the documented next step, worth doing once there are device numbers to judge it by. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The media clock is validated on device and the demo can toggle between the two on one clip. Notes the two limits it carries: the wait spins because the pump runtime has no sleep primitive, and held detections lag their frame. Also records why `realtime` was not lifted with it. The live lane does not run through createMediaSession, so making dropFramesWhileBusy a session policy today would add an option with one valid value. It belongs with the convergence work, where the clock becomes a parameter across both lanes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
|
[Jarbas Local João] — REQUEST CHANGES at Gist: https://gist.github.com/joaomarcoscrs/f54ef87e7b67fd6e646596db28e407da |
joaomarcoscrs
requested changes
Aug 21, 2026
joaomarcoscrs
left a comment
joaomarcoscrs
left a comment
Collaborator
There was a problem hiding this comment.
The new mask and media-clock paths can produce visibly incorrect output and continue doing work after playback is cancelled, so I don't think this is safe to merge yet.
| instruction: IdMaskInstruction, | ||
| ) { | ||
| const decodedMask = decodeCompressedRleMask(instruction.mask); | ||
| const decodedMask = decodeDetectionMask(instruction.mask); |
Collaborator
There was a problem hiding this comment.
When a dense mask comes from a model, foreground pixels are often 255, not 1. This generic decoder preserves those bytes. The fill path treats any non-zero byte as foreground, but the stroke helper below still checks === 1, so the same mask can render its fill while silently losing its outline. Could we make the stroke path use non-zero semantics too and cover a 0/255 mask?
| timestampMs: handle.timestampMs, | ||
| }); | ||
|
|
||
| while (Date.now() < dueAtMs && playingShared.value) { |
Collaborator
There was a problem hiding this comment.
If Pause, Stop, or Destroy flips playingShared.value while this loop is waiting, the loop exits but the current iteration falls through into serializeFrame() and presentation. From the user's perspective, pressing Pause can still start a full model run and flash one more frame afterward; Destroy can also be delayed by that inference. Could we release handle and leave the pump immediately after the wait when playback was cancelled?
| }); | ||
|
|
||
| const presentation = options.presentation ?? {}; | ||
| const clock = options.clock ?? "analysis"; |
Collaborator
There was a problem hiding this comment.
clock can now be "media", but the returned session still hard-codes playbackMode to "analysis-paced". A host that uses the session readout for a badge, telemetry, or control logic will be told the opposite of what the pump is actually doing. Could we expose the normalized clock here (or widen playbackMode) and return the selected value?
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.
Description
clock: "media"option on the file session;analysis(infer every frame) stays the default.inferenceMode+segmentationProcessor+posecollapse into oneproducerreturning aDetectionFrame; the renderer branches on geometry, not on a task.DenseBitmapDetectionMaskin core — dense masks with no per-frame RLE cost, which is what made the item above viable.MediaFrameProcessorandMediaRendererAdapter.adapters/.Full plan and what was left behind:
docs/internal/react-native-unified-session-plan.md.Type of Change
Validation
npm run verifypasses, including after rebasing ontob1c8234. Validated on a physical iPhone 15 at each step — three bugs in this branch were invisible to Node and only surfaced on device.Notes For Reviewers
Breaks types, not runtime. Nothing fails at execution; two things can fail a consumer's build:
DetectionMaskis now a union — reading.countsdirectly no longer compiles. UsedecodeDetectionMask().MediaRendererAdapterandMediaFrameProcessorare now unions —implements MediaRendererAdapter<...>no longer compiles; implementAsync...orSync...directly.Not tested on Android.
🤖 Generated with Claude Code