Skip to content

Commit 1e3195b

Browse files
fix: close the gaps the widened mask union and open producer opened
Widening `DetectionMask` and opening the live producer contract both moved a boundary, and each left a consumer on the far side that had never been asked to handle what now crosses it. **A dense mask crashed the editable annotation session.** Routing every reader through `decodeDetectionMask()` was necessary and not sufficient: `createEditableAnnotationFrameSession()` never decodes, it deep freezes, and `Object.freeze()` throws on an array buffer view with elements. Every mask had been a `counts` string, so nothing in core had met a typed array nested in a `Detection`. The freeze now steps over views; the buffer is documented immutable and shared, so there was nothing there to protect. Rebasing onto main surfaced the same class of bug from the other direction — #90's `regionCoverageMask` is typed `DetectionMask` and assigned straight from `detection.mask`, so it decodes through the union now too. **The producer contract accepted more than it drew.** Only dense masks reach the fill loops, which is the right call — decoding RLE per detection per frame is the cost the dense encoding exists to avoid — but a producer publishing cold storage got a blank overlay and no signal. Skipped masks are now counted and reported through the readout. Keypoint edge indices are bounds-checked rather than asserted, since the skeleton is no longer a constant this package owns, and keypoint color resolves per detection from `className` the way boxes already did; one frame-wide color was an artifact of pose having had exactly one class. `createExecutorchPoseKeypointInstructions` returns as a deprecated forwarding alias, so the rename breaks no import. **The media clock paid twice for held frames.** It re-reported identical detections to React on every presented frame, turning a ~1.4Hz cross-thread hop under `analysis` into ~30Hz under `media`, and rebuilt the ID-mask artifact from scratch each time — ~15ms per frame with the JS builder on a Pixel 10 Pro. Those milliseconds come out of the wait time the pacing budget banks, so refilling an identical artifact was lowering the inference cadence it existed to protect. `skia.ts` now splits the fill from the upload; the pump caches the build keyed on the detections array identity held frames already share. Uniforms and the `SkImage` stay per packet, so `PreparedFrameStore` ownership is unchanged and a reused artifact reports `fillMs: 0` rather than a cost it did not pay. Under `analysis`, `shouldInfer` is always true, so that lane is bit-identical. Also corrects the record on the spin loop. The pump runtime does have `setTimeout` — `createWorkletRuntimeForThread()` delegates to worklets' `createWorkletRuntime()`, whose `enableEventLoop` defaults to true. What it cannot do is fire one from inside `runPump`, a synchronous loop that never returns to the run loop. That makes the spin a structural choice with a known exit at Phase 3, not a missing primitive to go hunting for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent e098ecf commit 1e3195b

18 files changed

Lines changed: 728 additions & 110 deletions

docs/internal/react-native-architecture.md

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,16 @@ serialization/preparation worklets, and shared file/live defaults.
157157
controls, state subscription, and capability surface while its strict-sync
158158
native-pointer pump and Skia presentation lanes remain package-private.
159159
`createReactNativeVideoSession()` is retained only as a deprecated forwarding
160-
alias. File sessions are analysis-paced, support pause/play/stop, and report
161-
seeking as unsupported.
160+
alias. File sessions support pause/play/stop and report seeking as unsupported.
161+
162+
Pacing is a session option, not a fixed property. `clock: "analysis"` remains
163+
the default and infers on every decoded frame, so playback runs at inference
164+
speed — correct for producing a fully annotated video. `clock: "media"`
165+
presents frames on their own timeline, so a ten second clip takes ten seconds,
166+
inferring on whatever subset the measured model cost affords and holding the
167+
most recent detections across the frames in between. See
168+
[`react-native-unified-session-plan.md`](react-native-unified-session-plan.md)
169+
for the pacing policy and what remains.
162170

163171
Android saved-video decoding is implemented (experimental) as a Nitro C++
164172
`VideoFrameSource` under `packages/react-native/android/`: `AMediaExtractor`

docs/internal/react-native-live-rendering.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,10 @@ Same-day iPhone 15 comparison (debug build, same code, same Basketball
208208
sample, 6 masks, CoreML INT8, native mask prep): SEG 72ms, FILL 1ms, TICK
209209
88ms (~7.8 fps) — consistent with the pre-extraction 46-58ms baseline plus
210210
dev-mode overhead. The cross-platform gap is entirely the detection
211-
producer's backend (ANE INT8 vs CPU fp32); both platforms are strict-sync
212-
analysis-paced by design, so file playback intentionally runs at inference
213-
speed rather than media speed.
211+
producer's backend (ANE INT8 vs CPU fp32).
212+
213+
Both figures above were measured under the analysis clock, where playback runs
214+
at inference speed by design. Saved-video sessions now also accept
215+
`clock: "media"`, which holds the clip to its real duration and infers on the
216+
subset that fits; the presented-frame ratio under that clock is the number to
217+
record here next, per device.

docs/internal/react-native-unified-session-plan.md

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,17 @@ or `SyncMediaRendererAdapter` directly.
104104
processor and renderer are both sync is proven to run its whole frame path
105105
with no microtask in it, and the async default is proven to still yield.
106106

107+
**Widening a union means auditing its consumers, not just its decoders.**
108+
Routing every reader through `decodeDetectionMask()` was necessary and not
109+
sufficient: `createEditableAnnotationFrameSession()` never decodes, it deep
110+
freezes, and `Object.freeze()` throws on an array buffer view with elements. A
111+
dense mask crashed it. Every mask had been a `counts` string, so nothing in
112+
core had ever met a typed array nested in a `Detection`. The freeze now steps
113+
over views, and the same question — what does this do with bytes rather than a
114+
string? — is worth asking of any new `DetectionMask` consumer. Main added one
115+
during this branch's life (`regionCoverageMask`, in the web mask compositor)
116+
and it needed the same fix.
117+
107118
## Phase 1 — Open the producer contract (live camera first)
108119

109120
Today `UseReactNativeLiveInferenceOptions` requires
@@ -129,6 +140,18 @@ containing "Executorch" appears in the live hook's public options.
129140
published geometry, so `ReactNativeLiveInferenceMode` is gone. Keypoint drawing
130141
moved out of the adapter as `createReactNativeKeypointDrawInstructions`, which
131142
was the last vendor name the hook imported.
143+
`createExecutorchPoseKeypointInstructions` stays as a deprecated forwarding
144+
alias, so the rename does not break an import.
145+
146+
**What an open contract had to admit.** Opening the door meant being honest
147+
about what is behind it. Only `DenseBitmapDetectionMask` reaches the fill
148+
loops, so an RLE mask is skipped rather than decoded per frame — but that is now
149+
counted and reported through the readout's `skippedRleMaskCount`, because a
150+
blank overlay with no signal is not an open contract. Keypoint edge indices are
151+
bounds-checked instead of asserted, since the skeleton is no longer a constant
152+
this package owns. And keypoint color resolves per detection from `className`,
153+
the same way boxes already did; a single frame-wide color was an artifact of
154+
pose having had exactly one class.
132155

133156
Two notes for whoever picks this up. Detections still cross into the ID-mask
134157
fill through `serializeReactNativeLiveDetectionFrame`, a shallow bridge to the
@@ -171,9 +194,29 @@ until the schedule recovers, and another inference becomes affordable. The
171194
policy lives in `sessions/media-clock-policy.ts` as pure functions, because
172195
nothing inside the pump worklet is reachable from a test.
173196

174-
Two limits carried forward. The wait spins — the pump runtime has no sleep
175-
primitive — which costs less than `analysis` (never waits, never stops
176-
inferring) but is worse than pacing presentation off vsync. And held
197+
**Two costs the first cut paid, since removed.** Held frames re-reported
198+
identical detections to React on every presented frame, turning a ~1.4 Hz
199+
cross-thread hop under `analysis` into a ~30 Hz one under `media`; reporting is
200+
now gated on `shouldInfer`, which is the only rate at which the payload
201+
changes. And every held frame rebuilt the ID-mask artifact from scratch — ~15 ms
202+
per frame with the JS builder on a Pixel 10 Pro, taken directly out of the wait
203+
time the budget banks, so refilling an identical artifact was lowering the
204+
inference cadence it was supposed to protect. `skia.ts` now splits into
205+
`buildReactNativeSkiaMaskArtifact()` and
206+
`createReactNativeSkiaMaskFrameFromArtifact()`; the pump caches the build keyed
207+
on the detections array identity that held frames already share. Uniforms and
208+
the `SkImage` are still per packet, so `PreparedFrameStore` ownership is
209+
unchanged, and a reused artifact reports `fillMs: 0` rather than repeating a
210+
cost it did not pay.
211+
212+
**Two limits carried forward.** The wait spins. That is a structural choice,
213+
not a missing primitive: `createWorkletRuntimeForThread()` delegates to
214+
worklets' `createWorkletRuntime()`, whose `enableEventLoop` defaults to true
215+
and installs `setTimeout` on the runtime. What cannot happen is firing one from
216+
inside `runPump`, a single synchronous loop that never returns to the run loop.
217+
Removing the spin means restructuring the pump into a per-frame continuation,
218+
which also moves pause, resume, teardown, and source close off the guarantee
219+
that loop exit currently provides — so it belongs with Phase 3. And held
177220
detections lag their frame; propagating with a tracker is the next step, worth
178221
judging against device numbers rather than in the abstract.
179222

packages/core/src/detections/editable-annotation-frame-session.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
AnnotationFrameMutationKind,
55
createEditableAnnotationFrameSession,
66
} from "#detections/editable-annotation-frame-session";
7+
import { DetectionMaskEncoding } from "#types/detections";
78

89
describe("editable annotation frame session", () => {
910
const initialFrame = {
@@ -128,4 +129,36 @@ describe("editable annotation frame session", () => {
128129
"preserve the stable id",
129130
);
130131
});
132+
133+
it("snapshots a dense bitmap mask without freezing its bytes", () => {
134+
// `Object.freeze()` throws on an array buffer view with elements, so the
135+
// snapshot's deep freeze has to step over mask bytes. Every mask used to
136+
// arrive RLE-encoded, where the payload is a string, which is why this
137+
// only became reachable once the encoding widened.
138+
const data = new Uint8Array([0, 255, 0, 255]);
139+
const session = createEditableAnnotationFrameSession({
140+
detections: [
141+
{
142+
id: "dense",
143+
mask: {
144+
data,
145+
encoding: DetectionMaskEncoding.DenseBitmap,
146+
height: 2,
147+
width: 2,
148+
},
149+
},
150+
],
151+
mediaTime: 0,
152+
});
153+
154+
const snapshot = session.getSnapshot();
155+
const mask = snapshot.detections[0]?.mask;
156+
157+
expect(Object.isFrozen(snapshot)).toBe(true);
158+
expect(Object.isFrozen(mask)).toBe(true);
159+
// Shared, not deep-copied: duplicating a full-resolution mask per frame is
160+
// exactly the cost this encoding exists to avoid.
161+
expect(mask && "data" in mask ? mask.data : null).toBe(data);
162+
expect(Object.isFrozen(data)).toBe(false);
163+
});
131164
});

packages/core/src/detections/editable-annotation-frame-session.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,16 @@ function createSnapshot(frame: DetectionFrame): DetectionFrame {
229229
}
230230

231231
function deepFreeze<T>(value: T): T {
232-
if (value && typeof value === "object" && !Object.isFrozen(value)) {
232+
// Typed arrays are skipped, not frozen: `Object.freeze()` throws on any
233+
// array buffer view that has elements. A dense mask nests its bytes in one,
234+
// and `DenseBitmapDetectionMask` already documents `data` as immutable and
235+
// shared rather than copied, so there is nothing here left to protect.
236+
if (
237+
value &&
238+
typeof value === "object" &&
239+
!ArrayBuffer.isView(value) &&
240+
!Object.isFrozen(value)
241+
) {
233242
Object.freeze(value);
234243

235244
for (const nested of Object.values(value)) {

packages/react-native/src/adapters/executorch.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
type DetectionFrame,
77
} from "supervision-js-core";
88
import type { ReactNativeLiveSerializedDetection } from "../index";
9+
import { createReactNativeKeypointDrawInstructions } from "../renderers/keypoint-draw-instructions";
910
import type { ReactNativeLiveDetectionProducer } from "../types/live-producer";
1011
import type { ReactNativeVideoFrameHandle } from "../video-frame-source";
1112

@@ -704,3 +705,16 @@ export function createDetectionFrameFromExecutorchCocoPoses(
704705
mediaTime: options.mediaTime ?? 0,
705706
};
706707
}
708+
709+
/**
710+
* @deprecated Renamed to `createReactNativeKeypointDrawInstructions` and moved
711+
* to `renderers/`. Keypoint drawing reads only core types, so nothing about it
712+
* was ever specific to ExecuTorch; the vendor name here was an artifact of
713+
* pose support having been written in this adapter first.
714+
*
715+
* Kept as a forwarding alias so the rename does not break an import. The
716+
* replacement additionally resolves color per detection instead of taking a
717+
* single color for the whole frame.
718+
*/
719+
export const createExecutorchPoseKeypointInstructions =
720+
createReactNativeKeypointDrawInstructions;

packages/react-native/src/react/use-live-inference.ts

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { useCallback, useEffect, useMemo, useRef } from "react";
22
import {
33
KeypointMarkerShape,
4-
resolveDetectionClassColorStyle,
54
type DetectionFrame,
65
type KeypointDrawInstruction,
76
type PolygonDrawInstruction,
@@ -69,6 +68,12 @@ export interface ReactNativeLiveInferenceReadout {
6968
readonly segmentationMs: number;
7069
readonly serializationMs: number;
7170
readonly shaderActive: boolean;
71+
/**
72+
* Detections whose mask the producer published RLE-encoded. Only dense masks
73+
* reach the fill loops, so a non-zero value here means masks were produced
74+
* but nothing was drawn for them.
75+
*/
76+
readonly skippedRleMaskCount: number;
7277
readonly syncMode: "synced";
7378
readonly timestamp: number;
7479
readonly visibleKeypointCount: number;
@@ -686,6 +691,7 @@ function reportLiveInference(
686691
segmentationMs: metrics.segmentationMs.value,
687692
serializationMs: metrics.serializationMs.value,
688693
shaderActive: metrics.shaderActive.value,
694+
skippedRleMaskCount: metrics.skippedRleMaskCount.value,
689695
syncMode: "synced",
690696
timestamp: frame.timestamp,
691697
visibleKeypointCount: metrics.visibleKeypointCount.value,
@@ -753,8 +759,22 @@ export function useReactNativeLiveInference(
753759
// model for a moment.
754760
//
755761
// Each closure captures its own generation and compares it against the
756-
// shared value, which React updates immediately. A superseded closure sees
757-
// the mismatch and skips the frame instead of running a stale model.
762+
// shared value, which the effect below writes on commit — ahead of the
763+
// camera's own effect reinstalling `onFrame`, which is the ordering that
764+
// matters. A superseded closure sees the mismatch and skips the frame
765+
// instead of running a stale model.
766+
//
767+
// The obvious simplification — put `producer` itself in a shared value and
768+
// compare identity — is deliberately avoided. A shared value serializes what
769+
// it holds, and a producer closes over a JSI HostFunction; that is exactly
770+
// the recursive-capture case `react-native-live-rendering.md` records as
771+
// serializing successfully and then being non-callable on the worklet
772+
// runtime. An integer carries no such risk.
773+
//
774+
// Incrementing inside `useMemo` is a render-phase write, which a StrictMode
775+
// double render inflates. That is harmless here because only equality is
776+
// ever tested, and a recomputed memo rebuilds `onFrame` and republishes the
777+
// shared value together.
758778
const producerGenerationRef = useRef(0);
759779
const producerGeneration = useMemo(() => {
760780
producerGenerationRef.current += 1;
@@ -767,7 +787,6 @@ export function useReactNativeLiveInference(
767787
activeProducerGeneration.value = producerGeneration;
768788
}, [activeProducerGeneration, producerGeneration]);
769789

770-
const poseInstructionColor = resolveDetectionClassColorStyle("person").fill;
771790
// Capture initialized worklet functions before the callback is serialized.
772791
// Worklets' Babel transform does not preserve normal function hoisting.
773792
const createPoseInstructions = createReactNativeKeypointDrawInstructions;
@@ -847,10 +866,9 @@ export function useReactNativeLiveInference(
847866
);
848867
}
849868
stage = "pose-instruction-conversion";
850-
const poseInstructions = createPoseInstructions(
851-
detectionFrame,
852-
poseInstructionColor,
853-
);
869+
// No color argument: each skeleton takes its own class color, so a
870+
// producer publishing more than one class does not draw them alike.
871+
const poseInstructions = createPoseInstructions(detectionFrame);
854872
stage = "pose-prepare-vector";
855873
const vector = presentation.prepareVector({
856874
frameHeight: frameSize.height,
@@ -882,7 +900,11 @@ export function useReactNativeLiveInference(
882900
}
883901

884902
stage = "detection-serialization";
885-
const detections = serializeDetections(detectionFrame);
903+
const serialized = serializeDetections(detectionFrame);
904+
const detections = serialized.detections;
905+
906+
metrics.skippedRleMaskCount.value = serialized.skippedRleMaskCount;
907+
886908
const segmentationMs = inferenceMs;
887909
const extensionResult = activeExtension.active
888910
? evaluateLiveInferenceObjectExtension({
@@ -970,7 +992,6 @@ export function useReactNativeLiveInference(
970992
mediaRect,
971993
metrics,
972994
mosaicCellPx,
973-
poseInstructionColor,
974995
presentation,
975996
activeProducerGeneration,
976997
privacyContourWidth,
@@ -1025,6 +1046,7 @@ function useLiveInferenceMetrics() {
10251046
const segmentationMs = useReactNativeSharedValue(0);
10261047
const serializationMs = useReactNativeSharedValue(0);
10271048
const shaderActive = useReactNativeSharedValue(false);
1049+
const skippedRleMaskCount = useReactNativeSharedValue(0);
10281050
const visibleKeypointCount = useReactNativeSharedValue(0);
10291051

10301052
return useMemo(
@@ -1045,6 +1067,7 @@ function useLiveInferenceMetrics() {
10451067
segmentationMs,
10461068
serializationMs,
10471069
shaderActive,
1070+
skippedRleMaskCount,
10481071
visibleKeypointCount,
10491072
}),
10501073
[
@@ -1064,6 +1087,7 @@ function useLiveInferenceMetrics() {
10641087
segmentationMs,
10651088
serializationMs,
10661089
shaderActive,
1090+
skippedRleMaskCount,
10671091
visibleKeypointCount,
10681092
],
10691093
);

0 commit comments

Comments
 (0)