Repository navigation
Conversation
…ons in true image pixels Build step 2c (processing.js, on the viewer's hooks; writes through the one shared helper, moved from overlay.js into window.GeecsCameraView.write): - Background: grab from the next frame (PROC1:SaveBackground, press-only), subtract on/off (offered once a background is held); said to live in the IOC's memory only. - Rotate/flip: TRANS1:Type (no readback record: the output is monitored), with a warning that positions and statistics are then in transformed pixels. - Saturation: STAT0's raw max against the pixel format's full scale (99%, which also catches a Blackfly's left-aligned 12-bit Mono16), judged only on a fresh raw max; a chip in the image header, optional sound. - Found live: ffmpegServer scales a frame to fit MAXW x MAXH (a Rot90 1280x1024 frame arrives as 960x1200), which put the overlay ~6% off. The page now stretches the JPEG to MJPG1:ArraySize0/1_RBV, so every position is in image pixels (#1133 depends on this). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ubtraction, the true-size check is per axis, read-only disables the transform F1: rotated frames minus a background grabbed in the old orientation was silently wrong (NDProcess checks only the size): the service puts EnableBackground 0 before a TRANS1:Type write, refusing an invalid transform before changing anything; the page says to grab again. F2: the JPEG is accepted as the true frame scaled when each side is within 1.5 px of trueW*s, trueH*s; the ratio tolerance failed on very wide frames. F3: read-only disables the rotate/flip select too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…true-size check tries each axis's scale Review round 2 on #1134: the 'grab a new background' state lasted one update, after which 'subtract' was offered again on the misaligned background; now any transform change seen in the stream marks the background stale (subtract disabled) until a grab. The size check used the short side's scale, which misses wide frames (2448x200 -> 1600x130); it now accepts either side's factor (cases in the comment, checked in node). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adversarial review (fresh-context reviewer, two rounds)The reviewer found no write outside the allow-list. The chain is confirmed: TRANS1 → PROC1 → ROI1/MJPG1 is a side branch, and HDF1, PVA and ARR take frames from cam1, so no write here reaches scan data.
Round-2 items fixed without a third round (review stopping rule). After the fixes: GeecsPanels 119 passed, lint OK. |
skbarber
left a comment
There was a problem hiding this comment.
Reviewed this PR in the #1130–#1135 stack, including the existing review fixes. 1 remaining P2 finding(s), attached inline. Validation at stack head dc8ac07: panel suite plus Core EPICS bootstrap/light-import tests, 143 passed (including JavaScript syntax checks). Targeted offline probes reproduced the findings on #1134/#1135. No hardware writes or live camera testing performed.
| const p = snap.processing || {}; | ||
| if (lastTransform !== null && p.transform && p.transform !== lastTransform) stale = true; | ||
| if (p.transform) lastTransform = p.transform; | ||
| const valid = p.background_valid === 1 && !stale; |
There was a problem hiding this comment.
[P2] Keep background validity outside the lifetime of a browser page
After grabbing a background at None and changing to Rot180, this page correctly disables Subtract. Reloading it (or opening another viewer after the rotation) resets lastTransform and stale, however, so the unchanged ValidBackground_RBV == 1 immediately enables Subtract again. The server accepts PROC1:EnableBackground = 1 without checking the orientation of the saved background, restoring subtraction with the wrong image. I reproduced the enabled → disabled → enabled sequence by running this script against identical IOC snapshots before and after recreating the page. Track the background/transform relationship in shared state and enforce it on the enable route, so every viewer retains the guard until a new background is actually captured.
There was a problem hiding this comment.
Fixed in 67f2d4f9b. The staleness now lives in the service, shared by every viewer:
CameraFeedmarks the background stale on anyTRANS1:Typechange seen on the IOC, from this panel or any other client.- It clears only when a grab completes (
PROC1:SaveBackground_RBV1 → 0). /putrefusesPROC1:EnableBackground 1while stale (409). Switching it off is always allowed.- The page shows the service's flag (
processing.background_stale), so a reload or a second viewer sees the same.
The test (test_a_rotation_makes_the_background_stale_for_every_viewer_until_a_grab) fails without the server check.
Known limit: a service restarted after a rotation can't know the held background's orientation. The flag is per service process, like the history.
…iewer and refuses subtraction until a new grab GPT review P2 on #1134: the 'background predates a rotation' guard lived in one page; a reload or a second viewer re-enabled subtraction with the wrong-orientation background. The feed now marks the background stale on a TRANS1:Type change seen on the IOC (from any client) and clears it when a grab completes (PROC1:SaveBackground_RBV 1 -> 0); the put route refuses EnableBackground 1 while stale (409), and the page shows the service's flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stacked PR, step 2c of the panels arc. Base:
feat/panels-crosshairs(#1133) ← #1132 ← #1131 ← #1130. The stack merges in order at the end of the arc. #1133 depends on this one for correct positions on a scaled stream.What
processing.jsadds a third layer on the viewer's hooks. The write helpers move fromoverlay.jsinto one sharedwindow.GeecsCameraView.write.SaveBackground, which the allow-list makes press-only:(1,).ValidBackgroundsays one is held.TRANS1:Type, monitored on the output record itself because it has no readback. A warning shows while it's set: positions and statistics are then in transformed pixels.MJPG1:MAXW/MAXH(1600×1200), so a Rot90 1280×1024 frame arrives as 960×1200 and the overlay was about 6% off. The page now stretches the JPEG toMJPG1:ArraySize0/1_RBV(accepted only when the aspect agrees), so every position stays in image pixels.329 added, 63 removed.
Tests
Hardware verification (done on ADTEST_CAM1, then restored)
ValidBackground= Valid.Found, not fixed here (IOC chain): background subtraction wraps around on 8-bit frames. Noise just below the background goes negative and wraps to 255: subtracting on a dark frame gave STAT max 255, mean 0.63. Measured on the IOC:
PROC1:EnableLowClip 1at 0 gives max 2, mean 0.003. A geecs-epics PR will pin it at boot on both cameras.🤖 Generated with Claude Code