sync: bring upstream to fork parity v0.65.2 -> v0.67.1 - #114
Merged
Conversation
sensor.index and sensor.mode parsed on CV610 but were read by nothing and advertised by nothing, so a write was rejected. video0.fps was the selector instead, through an exact-match cv610_mode_for_fps() — a request for 45 fps was a hard config error where SigmaStar substitutes a mode and says so. cv610_mode_select() gives the two knobs the semantics sensor_select() gives them: a forced index wins over the requested rate and must exist, otherwise the rate is a target. One case diverges deliberately. SigmaStar's sensor_mode_cost() tie-break scores fps_excess at 0 for every mode BELOW the target, so a target above them all falls back to table order — the slowest mode. That rarely fires there because its modes carry min-max fps ranges; on this point-fps table it would fire constantly, so a target above every mode clamps to the fastest instead. Two things follow from a rate that can now differ from video0.fps: - the GOP limit is checked against the SELECTED mode's rate. The encoder has always derived its GOP from pipeline.fps (cv610_runtime.c), and that is no longer video0.fps once a mode can be forced or a target substituted. - /api/v1/modes reports what bring-up actually selected, published through the same venc_api_set_sensor_info() star6e_pipeline.c uses, instead of recomputing the configured answer per request. It could previously report a mode that was not running. -1 now means bring-up has not completed, which is reachable only on failure: backend_execute() runs prepare before init, and init is what starts the HTTP server. Two tests asserted the old exact-match intent (fps 120 and fps 45 rejected). They are replaced by the substitution they became, not deleted, and fps 0 keeps a rejection case so the check itself is still proven live. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
Three HTTP behaviours changed and the contract has to say so: sensor.index and sensor.mode become supported on CV610 (restart_required, matching the shared table), video0.fps becomes a target rather than a command, and /api/v1/modes reports the achieved selection rather than recomputing the configured one per request. The gopSize check moving to the selected mode's rate is a fourth, reachable as a 409 on a config that used to be accepted. The 0.18.2 change-log entry says "video0.fps alone selects the sensor mode". That was true when it shipped and stays as written — it is a dated record, not a live statement. The /api/v1/modes section above it now carries the current rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
isp.keepAspect sat in the CV610 config defaulting true, but the backend advertised no isp.* field and read none — a knob that did nothing, which is worse than one that is absent. video0.size=1440x1080 validated (multiple of 8, under the capture size) and then VPSS stretched the whole 1920x1080 capture into it, so 4:3 out of a 16:9 sensor came out squashed while Star6E and Maruko framed it correctly. vpss_setup() now sets a VPSS group crop from pipeline_common_compute_precrop() — the same shared function star6e_pipeline.c and maruko_pipeline.c call, so the rule cannot drift between three backends and tests/test_pipeline_common.c already covers it (it asserts this exact 1920x1080 -> 1440x1080 case). A matching aspect leaves the crop disabled rather than setting a no-op rect. Cropping cannot introduce an upscale: the crop matches the request's aspect exactly, and cv610_mode_check_output() already bounds the request by the capture, so the cropped rect is >= the request in both dimensions. Also: the Opus banner claimed "10.0 ms frames" for a release after CV610_AUDIO_POINT_NUM went 480 -> 960 (#227). The encoder was right at 20 ms — 50 packets/s on the wire — but the single line a reader could check that against was wrong. Derived from the constants now, alongside the bitrate, which is the same literal that went stale. Bench: aspect crop 1440x1080+240+0 of 1920x1080 -> 1440x1080 with keepAspect true, no crop line with it false, none at 1280x720 either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
…nditionally Adversarial review of this branch, two findings. 1. CV610 never ran the shared validate_field_cfg() sweep at config load. venc_api_validate_loaded_config() dispatched to cv610_validate_config() INSTEAD of the shared key sweep, so sixteen shared rules did not apply on that backend — and CV610 re-implemented two of them (video0.size's >=128 and multiple-of-8 gates) inside cv610_mode_check_output(), which is what made the gap visible: two copies of one HEVC constraint that can drift. The asymmetry was internal too. /api/v1/set has always run validate_field_cfg() before validate_backend_config() (venc_api.c:2311), so the same value was accepted from /etc/waybeam.json at boot and rejected with 409 over HTTP. The sweep now runs first, backend rules after, so all three backends share one rule set and one set of error strings. CV610 keeps its own size gates as a pipeline-boundary guard, because cv610_prepare() calls cv610_mode_check_output() directly against a config edited behind the daemon's back. Device-proven on .181: isp.awbMode="bogus" now refuses to start with "awb_mode must be 'auto' or 'ct_manual'" — it loaded before. Note most of the numeric shared rules are unreachable at load because the parser normalises first (sceneHoldoff has a floor of 1, zoom clamps to [0,1]), so a numeric field proves nothing here; a string enum is what discriminates. 2. The VPSS group crop is now written unconditionally, disable included. Skipping the call when no crop is needed would inherit the previous run's rectangle if the group ever outlived a teardown, and MPP objects are kernel state on this SoC. vpss_teardown() destroys the group precisely so that cannot happen today (af53ef6) — this stops depending on it. Not a proven live bug; the cost is one MPI call. Also confirmed and left alone: every encoder/VUI/GOP consumer already reads pipeline.fps rather than the requested video0.fps, and query_live_fps returns it, so /api/v1/fps/live reports the achieved rate; g_cfg_mutex is statically initialised, so publishing sensor info from prepare() before venc_api_register() is safe; and the aspect crop cannot introduce an upscale, because the crop matches the request's aspect, cv610_mode_check_output() bounds the request by the capture, and compute_precrop's & ~1u cannot round below an even request — which makes CV610's multiple-of-8 gate load-bearing for the crop, not pedantry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
waybeam-link is the single rate controller and actuates within seconds of the link coming up, so video0.bitrate in the shipped config is a BOOT SEED, not an operating point. What a seed has to survive is the worst rung rather than the expected one: landing at the §9.8 MCS0 no-feedback floor while offering 8 Mbps floods the air, and on this fleet an over-offered craft has already been measured demoting a SECOND craft's link on the same channel within a minute. Pinning it low costs nothing once the controller takes over. The test asserted 8000; it now asserts 2600 and carries the reason, so the next person to "fix" it low has to argue with the rung rather than the number. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
…tor-parity feat(cv610): resolution/fps handling reaches SigmaStar parity
The plugin wrote 26 registers and left a TODO naming the range it was
missing (0x301C..0x4549). Sony's sequence for that range is 131 entries,
so the sensor ran on power-on defaults for the reserved analog/ADC trim,
the MIPI TX timing, and the HDR context that linear mode needs parked.
imx662_cfg.h carries the sequence, transcribed from imx662_global_settings[]
in pauliustumas/imx662 -- the source the TODO pointed at. Two entries are
dropped: the master-start write, because imx662_default_reg_init() releases
STANDBY and XMSTA itself once the mode registers are in, and the HMAX pair,
which is commented out upstream and is per-mode state this driver owns.
The table runs first, in standby, so the per-mode writes that follow win
where the two overlap. That ordering is load-bearing for 0x3A50/51/52:
the table holds the 12-bit AD timing and the RAW10 modes overwrite it.
Measured on the 192.168.2.181 bench with the new tools/cv610_i2c_dump.c,
which does the 16-bit-address read the sensor needs from a second process
while the pipeline runs (imx662_read_register() is a stub -- the ISP only
ever writes, so nothing could confirm an init landed):
- 94 of the entries differ from this module's power-on state.
- The readout-window group (0x303C-0x303F, 0x3044-0x3047) already
matched bit for bit, so writing it is a confirmed no-op here.
- A RAW10-vs-RAW12 dump of the whole range differs in only 8 registers,
all of them mode or bit-depth state the driver already owns. None of
the reserved blocks are bit-depth dependent.
Cold-boot verified on all four modes, each with a 1222-register readback
at zero I2C errors, every reference entry matching silicon except the
excluded HMAX pair, and the AD-timing triplet correct for the mode's
bit depth:
1080p30 RAW12 30.03 fps
1080p60 RAW12 60.03 fps
1080p90 RAW10 90.03 fps
1080p100 RAW10 100.06 fps (pre-change baseline: 100.06)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
cmos_get_isp_default() handed the ISP a zeroed ot_isp_cmos_default. Every algorithm key defaults to 0, so demosaic, gamma, CLUT, LDCI, CAC, anti-false-colour, dehaze and CA were all switched off and the pipeline ran with no tone curve at all. That is what the flat, hazy, low-contrast image was -- not a sensor problem. The tables come from the in-tree Hi3516CV610 driver smart_sc450ai, which targets this same ISP silicon, ported verbatim into imx662_cmos_param.h. This group describes how the ISP behaves rather than how the sensor behaves, which is why it transfers: gamma is a tone curve, CLUT a colour lookup, demosaic/CAC/anti-false-colour are interpolation artefact controls. The blocks that are fitted to sensor noise -- bayer_nr, sharpen and the noise_calibration polynomial -- are deliberately left off; sc450ai is 4 MP and IMX662 is a 2 MP STARVIS, so those need their own measurement. Measured on the 192.168.2.181 bench, three cold-boot arms at identical pipeline config (1080p100, 6000 kbps, hub stopped so nothing could rewrite venc's config between arms), 30 frames each, only the plugin differing: arm sharpness stdY meanY flatNoise kB original 207 39.6 97.0 1.13 616 + register sequence 185 38.4 95.7 1.14 618 + register sequence + ISP 497 42.4 79.2 1.39 771 Sharpness up 169% with sharpen still disabled, so that is detail recovered by demosaic and CAC rather than edge enhancement; contrast up, haze gone, and larger frames at the same CBR because there is more real detail to code. Operator confirmed the result on the live stream. Two honest costs: flat-field noise rises 1.14 -> 1.39, which is what bayer_nr exists to absorb, and the pre-existing blue cast is amplified. The AWB statics are left alone -- the operator has already judged AWB auto the most balanced on this bench, and frame-mean RGB is not a white reference. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
The plugin's rule was a bare `%.o: %.c`, so nothing tracked the headers. Both the register sequence (imx662_cfg.h) and the ISP tuning tables (imx662_cmos_param.h) live in headers, which means a header-only edit relinked the existing objects and produced a binary that did not contain the change -- while make still exited 0. This is not hypothetical. Two sensor plugins built from different saturation tables came out byte-identical, and the difference was only caught by md5-ing the two artifacts rather than trusting the build. An A/B run on those binaries would have compared a change against itself and reported "no effect". -MMD -MP, -include the .d files, and clean them in the clean target. Verified by mutation: editing one value in imx662_cmos_param.h now changes the linked .so, where before it did not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
Three additions on top of the ISP algorithm blocks, each A/B'd on the 192.168.2.181 bench as its own cold-boot arm at identical pipeline config (1080p100, 6000 kbps, hub stopped), 30 frames per arm. sharpen + drc + bayer_nr. Objective metrics argued against these: on a static-frame comparison sharpness read 718 -> 241 and I called it detail loss. The operator watched the live stream and judged it clearly the best arm, which is the better instrument -- Laplacian variance rewards grain as much as detail, so the drop was largely noise being removed. Same-scene arms isolate the effect to sharpen+drc; bayer_nr alone measured as a no-op on a bright scene. It is kept anyway because sharpen reads the noise model that bayer_nr and noise_calibration provide, so it is not independently droppable at the value that was judged. AWB saturation. cmos_get_awb_default() memset the whole ot_isp_awb_sensor_default and refilled only wb_ref_temp, the static gains and wb_para, leaving agc_tbl zeroed -- valid = 0 and every saturation entry 0. The ISP was never told what saturation to run, which is the washed-out colour the operator reported. sc450ai's curve at its nominal 128 measured slightly *less* chroma than no table at all, so the shipped curve is scaled up at the low-ISO end and keeps the reference taper at high gain, where chroma noise scales: 128 128 128 128 124 120 116 112 108 104 100 94 90 90 90 90 reference 168 168 166 162 156 148 140 132 124 116 108 100 92 90 90 90 shipped Measured G -> H: mean chroma magnitude 20.19 -> 28.46 (+41%) with sharpness flat at 396 -> 406, so it moves colour and nothing else. Operator picked the boosted curve. Also corrects sns_mode: sns_id was 0, disagreeing with cv610_pipeline.c's IMX662_SNS_ID, which that file already documents as having to match -- and which would misbind a future PQ .bin, since those are chip- and sensor-locked. sns_mode was never set at all. The noise-fitted tables are sc450ai's, from a 4 MP sensor. They are kept on a hardware verdict, not a calibration; a measured IMX662 fit is still the right thing to replace them with. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
The README still said HDR and broad ISP tuning were deferred, which was true of the scaffold and is no longer true of the plugin. Records the init ordering that 0x3A50/51/52 depends on, which of the sc450ai-derived tables transfer on principle and which are borrowed noise tuning, and that these are compile-time seeds because CV610 has no apply_iq_param and therefore no runtime IQ layer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
PR #229 landed the ISP seeds as compile-time constants, which is the right place for defaults but leaves no way to tune. /api/v1/iq/{get,set} return not_implemented on CV610 because g_cv610_apply_callbacks supplies neither query_iq_info nor apply_iq_param. Records the callback contract, the registration site, the reference implementation already on disk (cv610_streamer/isp_control.c, proven against this silicon via the cv610_isp_ctl CLI on .181), a verification plan whose central check is that a set actually moves mean chroma rather than merely returning ok, and the bench traps this session paid for -- above all that stop/start hard-hangs the SoC and the daemon must be cycled by reboot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
0.65.4 seeded this ISP through the IMX662 plugin's cmos_get_isp_default(), which the ISP reads once at pipe start. Correcting any of those values meant a cross-compile, a plugin deploy and a reboot. This makes them live. /api/v1/iq and /api/v1/iq/set are existing generic routes; they answered 501 on cv610 only because g_cv610_apply_callbacks supplied neither callback. 11 groups, 59 fields: saturation, color_tone, csc, ccm, wb, sharpen, nr, drc, ldci, dehaze, ca, plus a read-only decode of ss_mpi_isp_get_module_ctrl() so a seeded-but-bypassed block is distinguishable from one that was never seeded. Deliberately unlike star6e_iq.c, which dlopens libmi_isp.so and addresses opaque structs by hand-computed byte offsets. This backend links ss_mpi directly, so every field is an offsetof() into the SDK's own type and every range is the header's documented one — a mistyped member is a compile error rather than a silent write into the neighbouring field. Two behaviours worth stating because neither is guessable: - Writing a manual.* field also forces that group's op_type to manual. A manual value is inert while the block runs its auto curve, which from the outside is indistinguishable from a broken setter. The emitted schema marks which fields do this. - query() reads back from the ISP rather than returning a cache, so a clamped or rejected set is visible instead of assumed. cv610_pipeline_isp_ready() gates both entry points: firing MPI ioctls at an ISP that was never inited is not a thing to discover in the field.
The tab was hidden by a hardcoded `backendName === 'cv610'` test. It now probes /api/v1/iq once at load and hides the tab only when that route does not answer, so a backend gaining or losing the surface needs no edit here. Un-hiding it alone would have been wrong: IQ_CATEGORIES and IQ_FIELDS are the SigmaStar parameter set, so a cv610 would have rendered a list of knobs it does not have. Backends that describe themselves now drive the list from the `_schema` block in the response, keeping the field table in exactly one place — the C table that also enforces the ranges. Copying it into the page is the same trap as the modes store's third copy of the mode list. Backends with no schema keep the existing hardcoded categories unchanged. Also: a chip prefills the value the ISP currently holds instead of an empty box, and a successful set re-queries and reports what the ISP kept. Those differ whenever a value is clamped, and the response echo of the request is not evidence of what was applied. The intro text claimed the IQ system was SigmaStar-specific and dlopen-based, which was never true of every backend and is not true of this one.
…IQ spec Measured through the new IQ surface on .181: drc.enable and dehaze.enable both read 0, and ss_mpi_isp_get_module_ctrl() reports the hardware bypassing both. Toggling drc.enable clears and restores that bypass bit, so the two readouts agree and the mechanism is not in doubt. The cause is in the ported tables themselves — g_cmos_drc and g_cmos_dehaze carry enable = 0, inherited faithfully from sc450ai's linear-mode defaults. So this is not a porting error, but it does mean DRC has never contributed to a CV610 image and has never been evaluated on this sensor. Worth knowing before anyone tunes against it. The spec is marked implemented, with what changed from the plan and why, and the one question it deliberately left open: nothing persists across a reboot.
Also corrects the 0.65.4 entry. It said the ISP algorithm blocks were "enabled", which over-states it for DRC and dehaze: those two are registered with the ISP but ship enable = 0 and are bypassed in hardware. The claim was only checkable once this release built the instrument that checks it.
…hat 501s Self-review of #230 found three things worth fixing and three worth deleting. Bug: an array set accepted MORE values than the field holds. The parse loop exited on `n < count` without checking for a trailing comma, so `saturation.auto.sat=<16 values>,999` wrote the 16 and dropped the 999 — while /api/v1/iq/set echoed all 17 back as applied. Silent truncation plus a response that agrees with the request is invisible from the outside. Both directions are now rejected: exactly `count` values or nothing is written. Short lists were already rejected; that path is unchanged and still tested. Bug: the IQ tab now shows on cv610, and it carries an "Import JSON" button. handle_iq_import() is compiled behind `#if HAVE_BACKEND_STAR6E || HAVE_BACKEND_MARUKO`, so on cv610 that button always 501s. The query response now carries `_caps.import`, and the page hides the control when a backend says it has no importer — the same reasoning that already disables the recording actuators here, so the buttons cannot lie. Hardening: JSON_PUT clamps at the buffer end, so an overflow would have served syntactically broken JSON and the page would have failed to parse it with no clue why. query() now fails the request instead (500). Current response is 6.7 KB of a 24 KB buffer, so this is a guard, not a fix. The tab was also un-hidden in markup and hidden by the probe, which flashed a tab that then vanished on backends without the surface. Hidden by default, revealed when the probe answers. Deleted, all of it unused: Cv610IqGroup.size was written by the GROUP macro and never read; IQ_ACCESSORS took a struct-type argument its body ignored; and FT_S8/FT_S16 had no field of either type. No exposed field is signed. Note `make build` does not pass -Werror — only `make lint` does, and `verify` does not run it. `make lint` is clean for all three backends.
Four independent reviewers went over this branch. Their findings, fixed here. **star6e/maruko regression — boolean prefill.** Those backends emit a real JSON boolean for bool-only params (star6e_iq.c:449-453), so the new prefill put "true" in the value box — which handle_iq_set's numeric validator rejects. 14 chips were reachable in that state, and the clamp-detector then labelled it "(clamped)". Booleans normalise to 1/0. Before this branch the box was empty, so this was introduced here. **star6e/maruko regression — an ISP sweep on every page load.** The capability probe hit /api/v1/iq at init on every backend; on star6e that is 62 dlopen'd ISP getters serialized on the single httpd thread, to decide whether to draw a tab. Route availability now comes from /api/v1/capabilities, which init() already fetches, and the IQ payload loads on entry to the tab instead. That also drops cv610's bespoke `_caps` block — one fact, one place — and fixes prefill staleness, since the payload is re-read each time the tab is opened. **Fail-closed tab.** Hiding by default (my own earlier fix for a tab that flashed) meant any init failure, or a star6e whose libmi_isp.so is missing, latched the tab off with no message and no retry. Capability drives visibility; a failed load now reports the reason in the tab body. **Out-of-range values were clamped, not rejected**, while the HTTP layer echoes the request back — so both the response and the log recorded a value that was never applied. Rejecting instead makes the echoed value true by construction, without touching the shared handler's contract. **parse_i32's overflow guard was dead code.** `v < INT32_MIN || v > INT32_MAX` on ARM EABI, where long is 32-bit, is `long < LONG_MIN || long > LONG_MAX` — a tautology the compiler folds away. A saturated strtol result passed straight through. Now checks errno == ERANGE, with the range test kept only where long is actually wider. **forces_manual was one-directional.** Writing a manual field selects manual mode, but nothing selected auto — so after any manual write, restoring an ISO curve stored the numbers, read them back, and changed nothing: the same "looks like a broken setter" trap the flag existed to prevent, in the other direction. Two reviewers found this independently. Replaced by a domain (direct/manual/auto); a write selects the domain it belongs to. The schema reports the domain and the WebUI marks both kinds. Also: URL-encode the value (an unencoded '&' silently truncated the request and was then mislabelled as a clamp), guard a malformed schema group instead of blanking the tab, catch the async rejection, and correct a mutex comment that implied the lock serialized MPI calls when it guards only the scratch buffers. lint clean on cv610/star6e/maruko; verify green.
Both found by an adversarial review against Sony's register map, and both verified against IMX662_Register.xlsx before changing anything. **0x44C0 was never written.** It is absent from pauliustumas/imx662's imx662_global_settings[], which this table was transcribed from — so the transcription was faithful and the upstream list is wrong. Sony's map gives 44C0h "Set to 7Fh" (reset 00h) in all 18 All-pixel modes, on every mode sheet in the workbook. The shape makes the omission obvious once seen: 0x44C0..0x44CF is an 8-entry 16-bit little-endian table mirrored verbatim at 0x44D0..0x44DF, and our table already carried the mirror's 7F/01. Bank 0 entry 0 was latching 0x0100 instead of 0x017F while its twin was correct — the two banks disagreeing at exactly one index. Reflection timing for the 0x44xx group is S (latched on standby release), so it was wrong for the life of every stream in every mode. **0x30DD cleared a bit Sony documents as "Fixed to 1h".** BLKLEVEL is [11:0] across 30DC/30DD, so 30DD's low nibble carries BLKLEVEL[11:8] and its top nibble is reserved with an initial value of 40h. We wrote 0x00. Writing 0x40 keeps BLKLEVEL at 50 — the black level itself does not move — and stops zeroing the fixed bit. The register list's own changelog calls this out: "Revised : Initial value of fixed register (30DDh)". Pre-existing line, but it now feeds bayer_nr, DRC and LDCI, which this PR enabled. Table integrity re-checked after the insert: 129 entries, strictly ascending, no duplicates, range 0x301A..0x4549. Also documents two things the header previously got wrong or left silent: the range is 0x301A..0x4549 (not 0x301C), 0x301A is written again by the per-mode block, and 0x30A6 is a third deliberate departure — it keeps upstream's Hi-Z where Sony drives XVS/XHS, because nothing on this module consumes those pins and driving pads of unknown wiring is a hardware risk with no benefit today. And narrows a comment that implied the color-sector copy was live: it is seeded but inert, since g_imx662_color_sector.valid is 0.
…ings Measured, not guessed: with the lights off a 9.26 Mbps CBR target produced 29-65 Mbps. A/B with interleaved controls priced every knob, and sharpening was 88% of it -- 65.4 -> 7.8 Mbps with it off, which is under target. The sc450ai-derived sharpen auto tables rise along the ISO axis (edge_strength 195 -> 500), sharpening harder the darker it gets. On a high-pass filter fed a noise-dominated frame that is a bitrate multiplier. Both tables are tapered along ISO only; daylight columns are byte-identical, verified by invariant. QP bounds are wired but are NOT the fix, and the log says why: the driver default was already min=10 max=51, the H.265 maximum. The rate controller had full authority the whole time. Worth having; cannot rescue this scene. AE gain ceilings are exposed as an `exposure` IQ group -- the bench runs a 12739x (~82 dB) system gain ceiling, previously a compile-time constant marked VERIFY and unreachable at runtime. Operator-confirmed on the live stream: dark room now holds target. Daylight unchanged at 10.09 Mbps / 100.20 fps.
FDG_SEL0 (0x3030) was defined in the header and never written, so the sensor sat at its 0h reset -- low conversion gain -- and every dark frame was amplified through the noisier path. HCG is IMX662's actual low-light feature and it was simply unused. Sony's map gives 0h: LCG, 1h: HCG with V (per-frame) reflection timing, so this rides along with the existing gain update rather than being a static init value: FDG_SEL0 becomes a ninth fast-update slot next to SHR0/GAIN/VMAX. Threshold with hysteresis, entering at code 40 (12.0 dB) and dropping back at 34 (10.2 dB, Sony's floor while HCG is selected). Bright scenes stay in LCG because HCG trades saturation headroom for read noise -- the SRM's "saturation signal gets smaller according to Vsat". No gain compensation is applied, deliberately. AE is a closed loop and absorbs the conversion-gain step on its own; measured on the .181 bench in a dark room, FDG_SEL0 reads 0x01 and the gain code holds 0x47 (21.3 dB) rock steady with no hunting. Adding an offset correction would be guessing at a figure neither the SRM nor the datasheet states, to remove a transient that does not show up in practice. reg_addr[] is now sized from its own initializer and reg_num derived from it: the fixed [8] caught this addition as an overflow at -Werror, and the next slot should not depend on someone remembering to bump a literal.
… value Two related gain-chain fixes, both from reading what the 82 dB ceiling was actually made of: 398x sensor analog (52 dB) times 32x ISP digital (30 dB). IMX662 has no digital gain register of its own. **ISP digital gain 32x -> 4x.** It is the worst gain in the chain -- it amplifies noise and quantisation equally and buys no SNR, where analog gain and HCG at least act ahead of the ADC. The old ceiling let the total reach 12739x, and a dark room at that ceiling was judged "super grainy" on the .181 bench while 1-2 steps down looked better. The new total is 398 * 4 = 1592x (64.0 dB), inside the preferred range, and AE now exhausts analog before it reaches for digital. Verified on hardware: sys_gain_max reads 1592x, and with the room dark AE sits at gain code 141 (42.3 dB analog) with HCG engaged -- spending analog, digital headroom still unused. It stays tunable by eye at /api/v1/iq/set?exposure.auto.ispd_gain_max=<n>, because darker-but-cleaner is an operator trade, not a constant. **IMX662_AGAIN_MAX's comment claimed ~72 dB; the value gives 52.** The `/10` makes it a factor of ten -- 20 dB -- smaller than 10^(72/20), while IMX662_GAIN_REG_MAX (240 codes x 0.3 dB) really does allow 72. The value is left alone and the comment corrected: the measured complaint here is too much gain, not too little, and raising the analog ceiling only makes sense together with the digital cap above, as one A/B.
The remaining grain lever was meant to be a re-fit of g_cmos_noise_calibration, which is sc450ai's photon-transfer fit -- 16 doubles measured from different silicon with a different pixel. It is the wrong model for IMX662 and it is what decides how hard NR filters at each ISO. I am not writing that table. A real fit needs flat-field captures across the gain range and a noise-variance measurement per Bayer channel; inventing 16 coefficients would be strictly worse than keeping a real fit from real silicon on the same ISP, and it would look identical in review. What is defensible is exposing the knobs that fit drives, so the borrowed model can be corrected by eye against the live image until someone measures the real one: - `nr.coring_ratio` -- 33-entry LUT, "strength of reserving the random noise according to luma", so LOWER filters harder. 33 values fits the 255-byte query-param cap with room to spare; a wrong length is rejected rather than partly applied. - `nr.md_en` -- CV610's motion-detect temporal NR, the tnr_en slot's variant on this chip. Temporal filtering is the strongest tool against high-gain grain because noise is uncorrelated frame to frame and the scene is not. It reads back 1, so it was already on -- worth knowing before anyone tunes against it. Verified on .181: both readable, the LUT writable end-to-end at 33 values and rejected at 32.
… contract Four independent reviewers went over the stack. The findings that changed code: **HCG shipped an uncompensated 15.3 dB step (HIGH).** The premise in my comments — "the gain code is total dB in both modes" — was wrong. The GAIN register is PGA gain applied AFTER the conversion gain, so the two multiply. The datasheet says so directly: Rcg (HCG/LCG) typ 5.8, corroborated by G sensitivity 18383 vs 3166 Digit/lx/s. And Sony's 34-code floor is a saturation constraint, not the conversion gain — Vsat 3895 (LCG) / 1204 (HCG) = 3.235, and 34 codes x 0.3 dB = 3.236x. My reading required that floor to be 51 codes. Uncompensated there is no fixed point between 12 dB and 25.5 dB: AE brightens 5.8x, slams the code past the floor, HCG drops out, the frame goes dark, and it limit-cycles. Daylight never enters that band and a dark room sits above it, which is exactly why both regimes I tested measured clean. The written code now has the offset subtracted and the thresholds moved to 91/85 so the compensated code clears the floor. The floor guard tests before subtracting — reg is unsigned, and a post-subtraction check cannot catch a wrap. Also: the latch is reset in sensor_global_init (a stale latch is now a 5.8x error on frame 1 after reinit), slot 8 is gated on linear mode (ClearHDR needs 3030h=02h and an FDG_SEL1 slot that does not exist here), and cmos_gains_update null-checks its state pointer. **The IQ tab was fail-closed again (HIGH).** My own fix for the flashing tab added `hidden` to the markup, so visibility depended on init() completing — and only /api/v1/modes had a .catch. One transient failure of config, capabilities or version and the tab was gone for good on star6e and maruko, where it was previously always present. That is the failure 8f4abba claims to have closed; I moved it rather than removed it. The tab is visible by default, hidden only on a positive routes.iq === false, and all four init fetches catch. **Two more star6e/maruko regressions from the same commit.** The post-set re-query is a whole-ISP sweep (46 dlopen'd getters on the single httpd thread) — my own commit message cited that cost as the reason for removing the load-time probe, then reintroduced it per set. And the prefill wrote the ISP's value into the actuator and selected it, so inspecting a parameter and pressing Set forced enable=1/op_type=manual on star6e's legacy path. Both removed; iqCurrentValue and IQ_LAST are now dead and deleted with them. **Contract.** data.routes was added to a contracted response with contract_version frozen and HISTORY asserting "no HTTP behaviour changes". Bumped to 0.18.4, documented data.routes and the CV610 IQ response shape as an explicit second form, and corrected two Backend Support Matrix rows that this stack made false. **QP bounds.** min_i_qp is no longer written: this craft ships qpDelta = -4, which lowers I-frame QP, and an I-frame floor silently cancels it. Star6E touches only the P bounds for the same reason. The resolved pair is now rejected when min > max (the API validator only compares them when both are non-zero), and a failed cold-boot apply is logged instead of (void)-ed. **parse_first_query_param rejects over-long keys and values** instead of truncating. A silently shortened value can still satisfy a caller's own validation — an array setter counting elements sees the right count with its last element cut mid-token — and then writes something never asked for. Record corrections: 0.65.5's entry described clamping, forces_manual, the /api/v1/iq probe and _caps.import, all of which this stack replaced; 0.65.6 omitted three of its four commits and quoted the pre-change gain chain as current. The sensor_ctl.c header still called the register block a TODO. The spec still named forces_manual. The plugin README said nothing about the ISO taper, HCG or the gain ceiling.
…#229) Verified on the .181 bench: 129-entry register table read back from silicon, all ISP groups reporting ret=0, four fps modes nominal. Two independent adversarial reviewers re-verified the table mechanically (strictly ascending, no duplicates, the 0x44C0/0x44D0 banks byte-identical) and the 0x30DD fixed-bit correction against Sony's register map.
A reviewer noted data.routes shipped with zero test coverage, and that the two existing capabilities tests could not have caught a regression in it: both memset the callbacks struct before venc_api_register(), so query_iq_info is always NULL and "iq" is always false. A test that can only observe one value of a boolean is not covering it. Two arms differing only in the pointer: without query_iq_info the response must carry "iq":false, with it "iq":true. That is the actual contract -- routes tracks the registered callback, not the backend name. Note `make test` currently reports 64 pre-existing failures in the star6e recorder suites, identical on master (2414 passed / 64 failed there, 2424 / 64 here). They are environmental: those tests write to /tmp/venc_recorder_test_* and this host's /tmp is full. Not caused by, and not fixed by, this branch.
11 ISP groups / 59 fields over /api/v1/iq, offsetof-based against the SDK types. Includes the star6e/maruko regression fixes from 8f4abba. WebUI paths re-verified against real star6e payloads captured from .232 (routes absent -> tab stays visible; legacy 62-param render; no prefill; no per-set sweep).
The first cut guarded only S95waybeam stop(). There are two call sites for "$LOADER" stop -- stop(), and start()'s failure rollback -- so a failed start on a live craft would still rmmod under the hub and wedge the SoC. Same gate-held-at-one-of-two-call-sites shape as before. Put the check in load-cv610-online stop_modules() instead, where the rmmod actually is: it now covers stop(), the start-failure rollback, and a hand-run "load-cv610-online stop", with no exclusions -- unloading while venc itself holds the devices is equally fatal. S95waybeam stop() keeps a pre-kill check, but delegates it to the new "load-cv610-online holders <ignore-pids>" subcommand so there is one implementation. It needs the early answer to refuse before destroying state: refusing after the kill leaves venc dead with modules loaded, which "$LOADER" start rejects as dirty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
Pre-merge review on hardware, three defects in my own guard. The device list was derived from module names instead of the target's /dev, so it missed three real nodes: ot_mipi_rx (the node is not "mipi_rx"), logmpp and venc_msg. Worse, the list was a multi-line string literal while the lookup matches on " name ". Names landing on a line boundary carried a newline instead of a space and never matched at all -- isp_dev and mipi_rx were dead entries, and venc holds four /dev/isp_dev fds. The hub case still worked only because /dev/rgn happens to sit mid-line. Built by concatenation now, so every entry is space-delimited by construction. S95waybeam stop also read a failed query as "no holders": a loader predating the "holders" subcommand exits non-zero with empty stdout, and stop() would then proceed into exactly the unload this guards. A partially deployed pair -- new init script, old loader -- is how that gets hit, and it is what I did by hand on the bench twice today. It now fails closed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
star6e and maruko never touch insmod/rmmod from S95waybeam -- the modules are loaded once at boot and venc re-inits the graph against them. cv610 was the outlier, and both of its init scripts have been unchanged since the initial backend commit, so the reload looks like bring-up scaffolding rather than a requirement. It is not a requirement for the graceful path: mpp_cleanup() already tears the graph down completely and calls ss_mpi_vb_exit() from the process that created VB, which is the only process allowed to. What the reload actually covered is the crash path -- a venc that dies without mpp_cleanup() leaves VB held by a dead owner, vb_set_cfg returns BUSY, and that was tolerated blindly. Harmless while the modules are reloaded every time; a stale pool layout otherwise. sys_setup() now reads the live config back with ss_mpi_vb_get_cfg() and compares the pools it was about to request. An identical layout is adopted and logged as such; a different one fails hard, naming the recovery command, instead of silently handing out wrong-sized blocks. load-cv610-online start is idempotent once the full set is up (open_user is loaded last, so its presence means the sequence completed); a partial set is still refused. S95waybeam stop leaves the modules loaded, and a new S95waybeam reload is the recovery path -- it unloads, so the holder guard from the previous commit applies and the hub must be stopped first. scripts/cv610_reload_free_verify.sh restarts across a mode change rather than only in place: a same-mode restart passes even when a stale VB layout was inherited, so it cannot tell the fix from the bug. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
Device verification on .181 turned up three things.
ISP, not VB, is what a hard kill leaves behind. After kill -9 the VB config is
released and vb_set_cfg succeeds, but ISP[0] stays inited and the next run dies
at ss_mpi_isp_mem_init with 0xa01c800c "already inited". sys_setup() now
pre-cleans with ss_mpi_isp_exit(VI_PIPE) next to the existing sys_exit/vb_exit,
before anything of ours is registered, so a crash recovers without a reload.
reload() dropped CV610_SENSOR_PROFILE/CV610_AUDIO on the way to the loader, so
it reloaded video-only with the imx662 clock and start() then correctly refused
the audio mismatch -- leaving the craft with no daemon. Pass the same env
start() does.
The suite had two flaws of its own, both the same shape: verifying that a
stimulus was applied rather than that it had an effect.
- T3 set video0.width/height, which do not exist -- the real key is the
video0.size string. It created keys nothing reads and then "passed" a
same-mode restart labelled as a mode change. It now sets video0.size and
asserts the ENCODED geometry moved, read from venc's own VPSS line.
- T5 grepped a VB marker and recorded a cheerful NOTE while venc was dead at
isp_mem_init. It now asserts the daemon is running and streaming.
- The hard-kill case also had to move last: a SIGKILL poisons module
reloading for the rest of the boot, so running it before the reload test
was testing the reload against state the kill had already broken.
20 passed, 0 failed on .181.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
T6 leaves video0.size at its test value and only T5 restored it, so the suite printed "Config restored" while walking away from a bench encoding 640x360 -- found exactly that way on .181. Restore and restart into the configured mode, then report the encoded size so the claim is checkable rather than asserted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
The message said "run '$0 restart'", and $0 is the loader. Run bare it reloads with the loader's own defaults -- video-only, imx662 clock -- which is the exact state that produced the message, so the advice loops. Point at S95waybeam reload, which passes the profile through. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
…h-live-consumer cv610: refuse to unload the MPP stack under a live consumer
…itor T2 left "does the OSD still render" to a human. Assert it: read the VGS job counters for the hub's region out of /proc/umap/rgn and require them to advance with zero failures. Those counters, not the hub's perf line, are the effect side. The hub counts a publish it issued, which is the stimulus; a publish into a region nothing composites would still count up there. Verified to fail when pointed at a handle that does not exist, so the assertion can fail. Baseline is taken in the preconditions, so a hub built without CV610_OSD records an explicit UNVERIFIED note instead of passing by absence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
Adversarial re-read of my own assertion found both. A counter RESET is not a stall. The VGS job counters belong to the region, so if the hub recreates it between the two samples the count starts from zero and b < a -- read as FAIL. That flakes exactly when the thing under test is working. Retry the sample pair instead of failing. The baseline was racy. The region is created a few seconds after the HUB's start, not ours, so a baseline sampled 8 s in can be empty on a craft where the suite started the hub -- which silently downgrades the T2 assertion to an UNVERIFIED note. Wait for the region when the hub is up. Also assert the OSD in T3, the harder case: the hub rebinds within ~1 s but at the old window size, so for a few seconds an oversized region is attached to the smaller channel. Measured benign on .181 -- 30 s with job_fail 0 and venc streaming -- but it is the arm that would catch it turning nasty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
…rement Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
…estart cv610: stop reloading the MPP modules on a venc restart
The "aenc MMB leak" from 0.65.8 is not a leak. The kernel reclaims both blocks -- measured, the pool returns to its exact baseline (34 blocks / 38332 KB) across five hard-kill cycles, and the blocks vanish from /proc/umap/media-mem immediately. Those MMB LEAK lines are the reclaim reporting itself. What they were masking is real: after a hard kill venc restarts SILENTLY WITHOUT AUDIO. MPP objects are kernel state, so the AI device stays claimed by the dead pid and every AI call from the successor is refused with OT_ERR_AI_NOT_PERM (0xa015800d) -- including ss_mpi_ai_disable(), so it cannot even clean up. ss_mpi_ai_set_pub_attr() then fails and venc blames "needs CV610_AUDIO=1 at module load", which is not what happened. Video streams normally, so nothing looks wrong. Deterministic, 2 of 2. Masked until 0.65.8, whose module reload also wiped the claim -- so this is that release's blind spot, and reload is no way out either: a hard kill poisons module reloading for the rest of the boot. Fix: cycle the audio module (ss_mpi_audio_exit + ss_mpi_audio_init) before bring-up, then release the stale AI/AENC objects. Releasing them alone is refused; the module cycle is what clears the claim, which is exactly why a SECOND graceful restart used to recover audio where the first did not. This folds that cycle into the first start. Same shape as sys_setup()'s isp_exit/sys_exit/vb_exit pre-clean. T6 asserted VIDEO streaming, which is why this went unseen -- it passed with audio dead. It now asserts the aenc block count and the absence of an audio-start failure, and skips explicitly on a video-only craft. T5 now refuses to run when a hard kill already happened this boot, instead of reporting a FAIL that points at the code rather than the contaminated boot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
Review of my own pre-clean. The caller sets state->audio_initialized before calling it, so if the cycle's exit succeeded and its init failed, the flag stayed set while the module was down -- and cv610_audio_stop() would then call ss_mpi_audio_exit() on a de-initialised module. An unbalanced exit is the same class of stale state this function exists to clear, which makes it the wrong bug to leave in it. The flag now follows what each call actually did, in both directions, so it means what it says at every point rather than being inferred from the happy path. Also corrected the function comment, which still described every call as best-effort after the module cycle became load-bearing, and now records that the cycle runs after the caller's own audio_init deliberately -- that is the ordering verified on hardware. Re-verified on .181: graceful control and kill-then-start both clean (aenc blocks 2, zero audio failures), suite 25 passed / 0 failed on a clean boot, and 22/0 with T5 correctly SKIPPED on a boot already contaminated by a hard kill -- which exercised that new precondition for real. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
An independent review disassembled the shipped vendor libraries instead of reading the headers, and found a regression I had introduced plus four calls that cannot do what their comment claimed. The fatal predicate was on the wrong call. ss_mpi_audio_exit() is not atomic: it tears down the userspace half unconditionally and only then issues the AUDIO_EXIT ioctl, so a non-zero return means "half exited". Returning -1 whenever the RE-INIT failed therefore turned a start that previously worked into a dead one whenever the EXIT failed -- on a call that was not on the start path before this branch, and that the hardware runs never exercised because they only ever saw exit=0x0. Fatal now only when the exit succeeded and the re-init did not, which is the single combination where this function took the module down and failed to bring it back. audio_initialized is no longer touched here. The two error directions are not symmetric: a second ss_mpi_audio_exit() is safe because the module's sub-exits are flag-guarded, while a SKIPPED exit leaks the kernel AUDIO_INIT claim and the /dev fd -- which is opened without O_CLOEXEC and survives the respawn execv. A partially-succeeded re-init leaves exactly that to clean up, so the flag must stay set. My previous commit had it backwards for that case. Four of the five object releases are gone. The SDK tracks those objects process-locally, so a successor cannot release a dead predecessor's copies: sys_unbind dispatches through a table audio_init had just memset, aenc_destroy_chn returns SUCCESS off a zero "created" flag (so its 0x0 in the log read as "released the predecessor's channel" when it had done nothing), ai_disable_chn gates the same way, and opus_deinit was worse than inert -- it printed "illegal handle(-1)!" to stderr on EVERY clean start, which the device log confirms and which is now gone. Only ai_disable reaches kernel state, and it is the only code that ever differed between the broken and healthy cases. Removal was re-verified on hardware rather than assumed. Also stopped the audio warning asserting one guessed cause. It claimed "needs CV610_AUDIO=1 at module load" for every failure -- the misreport this branch exists to fix -- and the new failure path would have routed through it too. It now points at the logged call and qualifies the module hint. And aenc_blocks() in the suite used "grep -c || echo 0", which prints 0 twice when there are no matches. Re-verified on .181: graceful control and kill-then-start both clean (aenc 2, zero audio failures, no vendor noise), suite 25 passed / 0 failed on a clean boot, make test 2489/0, -Werror lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
Second review pass. My replacement warning was wrong for the case it most needed to get right, and HISTORY still described the deleted calls. The module-absent case does NOT fail at the /dev/acodec open. ss_mpi_audio_init opens /dev/ab first, and open_aio -- the module that creates it -- is staged only under CV610_AUDIO=1 (load-cv610-online:147), a switch independent of audio.enabled in waybeam.json. So the message telling the operator that a /dev/acodec error means absent modules "and anything else does not" pointed away from the truth in exactly the configuration an operator can reach by enabling audio in the API without touching the platform conf. Measured on .181 with CV610_AUDIO=0 and audio.enabled=true: /dev/ab absent, the vendor lib prints "open err", and the failure lands on ss_mpi_audio_init. The hint now lives there and names both switches; the generic warning claims no cause at all and defers to what each failure point already prints. Video was unaffected throughout, which is the behaviour that comment block promises. Also logged the two failure exits that returned NULL silently (calloc, pthread_mutex_init) -- the warning says the cause is reported above, and those two made that untrue. HISTORY now describes what merges: ai_disable only, why the other four cannot work from a successor process, the opus_deinit stderr noise, and the warning change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
cv610: recover audio after an abnormal venc exit (the aenc "leak" is not a leak)
I was about to add a dirty-flag file that venc would write and the loader would read, so a reload could refuse on a boot where module reloading is poisoned. That was the wrong layer. Checking what reload is actually for answered it: nothing. It has no caller -- not the hub (mod_venc uses restart), not the verify suite's crash test, not venc. Its premise was falsified in the release that added it. The comment claims VB is left held by a dead owner after a crash and reload is "the way out"; 0.65.8's own notes record the opposite -- ISP, not VB, is what a crash leaves behind, VB is released -- and the suite prints "VB was released when the process died" every run. What a crash really leaves (ISP, and since 0.65.9 the AI claim) venc pre-cleans itself, so a hard-killed venc recovers from a plain start, video and audio both. And unloading is not free: a hard kill poisons module reloading for the rest of the boot, so a reload issued afterwards leaves the craft with no modules at all -- or, measured on a video-only craft, resets the SoC. Deleting the action removes the only shipped path that reaches that through normal operation. The vendor defect underneath is unchanged and still wants its own issue. Knock-ons: the loader's audio-mismatch hint pointed at "S95waybeam reload" and now says reboot, which is what actually applies a CV610_AUDIO or sensor-profile change. The suite loses T5 and, with it, the contaminated-boot precondition T5 needed -- nothing in the suite unloads now, so a hard kill cannot poison anything it does. Net -35 lines, no new file, no new venc responsibility, no venc<->script protocol. Device-verified on .181 (0.65.10 deployed, all four artifacts md5-matched): "S95waybeam reload" now prints usage and exits 1; a normal restart brings venc back with 25 modules and audio up. The full suite run is still outstanding -- the bench moved to other work before I could take it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
My changelog claimed reload was the only shipped path that unloads
modules on a running craft. It was not, and the counterexample sits on
the recovery this release prescribes.
start() rolled back with "$LOADER" stop when the daemon failed to come up
within 2 s. Since 0.65.8 made the loader's start idempotent -- it returns
success having done nothing when the set is already up -- that rollback
tears down a set the invocation never created. The reachable case: a venc
hard-killed earlier, operator runs a plain start per the new doctrine,
bring-up dies on leftover state, rollback fires, and on a boot whose
module reloading is already poisoned the modules do not come back. Zero
modules, or an SoC reset on a video-only craft.
The rollback is removed rather than made conditional. Leaving modules up
after a failed start costs nothing -- the next start re-inits against
them -- and a conditional would be one more thing to get wrong on the
path that matters most.
Same review pass, smaller things it caught:
- The endorsed expert path 'load-cv610-online restart' was recommended
bare, but bare it reloads with the loader's defaults (video-only,
imx662) rather than the craft's profile. That caveat lived inside
reload() and died with it; it is now stated where it survives, together
with the safer 'load-cv610-online stop' + 'S95waybeam start' form.
- cv610_pipeline.c told the operator to "reload the MPP stack with
'load-cv610-online restart'" -- bare, and only ever after a crash,
which is precisely the boot where an unload cannot be undone. It now
says reboot. The "held by a dead owner" substring the verify suite
greps for is unchanged.
- Suite: the T6 header blamed the kill for the SoC reset that an unload
after the kill causes; two strings still named the deleted action; and
the cp left where T5 was could never fire, since T4 restores and
nothing between writes the config.
- HISTORY contradicted itself ("nothing called it" vs "the suite loses
T5") and claimed nothing in the suite unloads, when T1 attempts one
deliberately to prove the loader refuses it.
make test 2489/0, -Werror lint clean, shellcheck/dash/busybox clean.
Device verification of the rollback removal is outstanding: the bench is
in use for other work.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HqJeX8rr7TyxQ6Qu2LtmGV
cv610: delete the reload action instead of guarding it
…tH265SliceSplit (0.66.0) video0.sliceCount binds MI_VENC_SetH265SliceSplit so the encoder emits independent H.265 slices, which is what lets the waybeam-link receiver conceal RF loss spatially instead of dropping the whole frame. Default is unchanged: slice_count=1 (split off), and the split call is gated on >1, so a fresh device behaves exactly as before. Two ring-path changes are unconditional and intentional: the ring-full recovery IDR gets its own 1 s holdoff, honored-aware so a request swallowed by the shared 100 ms limiter rolls the anchor back (A/B'd on device, 53 -> 7 IDR AUs per 6 s); and a pack whose NAL count exceeds the SDK's 8-entry packetInfo table now aborts the frame instead of shipping it truncated, warning once per pipeline start. The sliceCount ceiling was raised from 8 to VENC_SLICE_COUNT_MAX (32), replacing three drifting literal 8s. The old cap rested on a misreading — packetInfo[8] bounds NALs per PACK and the output walker iterates packs — and it capped the fleet at 6 delivered slices since 7 and 8 both quantize to 6. The GDR refresh AU has always been a 17-slice picture through that same walker. Device-verified on .232: sliceCount 17 delivers 17 slices of 1 CTU-64 row, 247/249 recorded AUs at CTU addresses 0,30,...,480, fps and CBR held, zero truncation warnings at 60 frames/s. make verify green on both backends; 2539 host tests, 0 failed.
Three defects introduced by the 0.65.3..0.67.0 range, one long-standing sensor-register error, and the documentation that range changed but did not update. No wire format, config schema or frame-SHM layout change. Star6E dual-VENC crashed at startup: star6e_pipeline_start_venc() accepts vcfg == NULL (start_dual passes it for ch1) but 0.66.0's slice block dereferenced it unguarded, so any craft with record.mode=dual segfaulted during startup controls regardless of sliceCount. star6e_pipeline.c is not in the host test library, so the suite could not see it. Also on that path: the slice-split struct went to the vendor with uninitialised padding (both siblings in the same function are memset), and a libmi_venc.so lacking the optional MI_VENC_SetH265SliceSplit export failed startup with a message that read as a driver rejection. The failure stays fatal -- one slice while the API advertises N would lie to the receiver's spatial concealment -- but now names the missing symbol. The CV610 partial-load rollback could still wedge the SoC. stop_modules() refuses to unload under a live consumer and its comment claimed every unload path was covered; cleanup_partial(), armed as an EXIT trap across the whole load sequence, was not. It now carries the same guard and leaves modules loaded, which is recoverable; a wedged SoC is not. The IMX662 analog gain register was never written. Sony's register list puts GAIN[10:0] at 3070h/3071h and jumps 3069h -> 3070h; 306Ch/306Dh are not registers. Analog gain never actuated, all gain was ISP digital, and the HCG compensation never landed, making each HCG engage an uncompensated ~5.8x brightness step. The vendor reference driver carries the same wrong pair. This changes low-light behaviour materially and the 0.65.6 gain/ISO tuning was measured against the dead register -- that path needs device re-verification. The HCG band's compile-time guard could not fire in the dangerous direction (unsigned wrap); now signed, plus an assert rejecting an inverted ON/OFF pair. Both mutation-tested. make stage SOC_BUILD=cv610 returned 0 when the sensor plugin failed to build, because the recipe chains with ';'. data.routes.iq claimed both /api/v1/iq and /api/v1/iq/set were serviced while testing only query_iq_info; /iq/set gates on apply_iq_param. Now requires both -- every shipped backend registers both, so no response changes. The test that asserted the old pairing gained the missing arm, mutation-verified. Docs: docs/CV610_BACKEND.md described unloading behaviour deleted in 0.65.8/0.65.10 and recorded none of the operator rules those releases established; /api/v1/intra/status and /api/v1/resilience/status are documented for the first time; an orphaned code fence rendered ~25 lines of the capabilities section as a code block. Gates: test-werror/asan/tsan 2580/0, verify, pre-pr, and CI-pinned cv610 lint+stage all green.
This was referenced Aug 22, 2026
PR Summary by QodoSync upstream parity through v0.67.1
AI Description
Diagram
High-Level Assessment
Files changed (79)
|
Code Review by Qodo
1. Multiple HISTORY.md versions added
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
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
1 participant
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.
Parity sync of
snokvist/waybeam_venconto upstream: 54 commits, 80 files,+9036/−1749, v0.65.2 → v0.67.1. The fork is a strict descendant of
upstream/master(git rev-list --left-right --count upstream/master...masterreads
0 54), so this is a plain fast-forward — no cherry-pick, no rebase, nohistory rewriting.
Important
Please merge this with a merge commit, not a squash. A squash rewrites the
SHA, which drops the merge-base back and forces the fork to be re-healed with a
force-push; that has already happened twice (v0.60.2 and v0.65.0). Merge commits
on #105/#108/#109 kept the topology intact and made every subsequent sync a
clean fast-forward. It also preserves the granularity of the 11 releases below.
What is in it
Eleven releases, one HISTORY entry each —
git log --first-parentand theHISTORY.mddiff account for exactly the same set./api/v1/iq, addressed byoffsetofreloadaction andstart()'s module rollback (#235)video0.sliceCountbindsMI_VENC_SetH265SliceSplit(#236)The load-bearing one for OpenIPC users is 0.65.7–0.65.10.
S95waybeam stopused to
rmmodthe MPP modules while a live consumer still held/dev/rgn. TheHiSilicon drivers take no module reference for an open fd, so the
rmmodsucceeded and wedged the SoC hard enough to need a physical power cycle. Unloading
is now guarded and no longer happens on stop.
Protocol impact: none. No RTP/CRSF/SSE change. Frame-SHM v1 was checked
explicitly against
upstream/master— no new flags, no struct field changes;CV610 reaches parity on the existing wire rather than extending it.
0.67.1 — the pre-merge review pass
Before sending this up I ran an adversarial review over the whole range. It found
three defects introduced by 0.65.3..0.67.0, all fixed here:
src/star6e_pipeline.c— SIGSEGV at startup.star6e_pipeline_start_venc()accepts
vcfg == NULL(start_dual()passes it for ch1), but 0.66.0's sliceblock dereferenced it unguarded. Any craft with
record.mode=dualcrashed,independent of
sliceCount. Invisible to the host suite —star6e_pipeline.cis not in
TEST_LIB_SRCS.src/star6e_pipeline.c— slice-split struct handed to the vendor withuninitialised padding; both siblings in the same function are memset.
init.d/load-cv610-online— the unload guard had a hole.cleanup_partial(),armed as an
EXITtrap across the whole load sequence, unloaded without theholder check, so a mid-sequence insmod failure could still reproduce the wedge
0.65.7 exists to prevent.
Plus a long-standing sensor bug: the IMX662 analog gain register was never
written. Sony's register list puts
GAIN[10:0]at3070h/3071hand jumpsfrom
3069hstraight to3070h—306Ch/306Dhare not registers. Analog gainnever actuated, all gain was ISP digital, and the HCG compensation never landed.
Note the OpenHisilicon reference driver
(
libraries/sensor/hi3516cv6xx/sony_imx662/imx662_cmos.h) carries the same wrongpair, so the vendor tree has this bug too — worth fixing there as well.
Also:
make stage SOC_BUILD=cv610returned 0 when the sensor plugin failed tobuild (the recipe chains with
;) — CI caught it only via its explicittest -f.Validation
Host gates, clean tree (
VERSIONis not an object dependency, so a staleout/hides a version bump):
make test-werror/test-asan/test-tsan— 2580 passed, 0 failedmake verify(Maruko + Star6E, incl.webui-check),make pre-prlint-build-cv610CI job reproduced locally against its own pinned deps(
OpenIPC/openhisilicon@ff20187,OpenIPC/firmware@d8fbff3) — lint + stageclean, all six required artifacts present including
libsns_imx662.soDevice verification (binaries md5-matched to this tree):
record.mode=dual,sliceCount=9): v0.66.0 → exit 139 = SIGSEGV; this tree → clean start,dual ch1 active, ch0 still 9 slices, 60 fps, stable.
0x3070=0x00always while0x306Ccarried the AE code; after,0x3070live-tracks AE. Driving AE bysqueezing
exposure.auto.exp_time_max: 27 → 33 → 53 → 73 → 88 codes in LCG,HCG engages between 88 and 99 codes (threshold 91), and the compensation is
exact on every HCG sample (
written + 51 = total: 48/99, 61/112, 77/128,94/145), never below the 34-code Vsat floor. Pipeline healthy at 1080p100.
/api/v1/intra/statusand/api/v1/resilience/statusresponsesmatch the new contract sections field for field; loader refuses to unload while
consumers hold devices;
cleanup_partial()verified to refuse under a liveholder and still unload when none is present.
Not verified: the low-light image itself. Analog gain now actuates, so the
picture changes and the ISO-indexed sharpen/NR tables land on real buckets. The
0.65.6 gain/ISO tuning was measured against the dead register and should be
revisited on hardware.
HTTP contract
contract_version0.18.2 → 0.18.6, additive throughout — all four sitesbumped (document header, example body, divergence note,
venc_api.c). New in thisrange: CV610 advertises
video0.resilience/video0.slice_count;video0.sliceCount1..32 on all three backends;data.routes; the CV610 IQresponse shape; and
/api/v1/intra/status+/api/v1/resilience/statusdocumented for the first time (both have been served for several releases).
Note on the open PRs
#110, #111, #112 and #113 all touch
VERSIONandHISTORY.md, and three touchsrc/venc_api.cand the contract doc, so they will conflict with this regardlessof merge order. Concretely, after this lands they will each need a re-cut:
0.65.3, feat(venc): expose video0.maxIpProp, the RC I-to-P size cap the firmware honours #111 claims0.66.0— all three arealready used by this range, so those bumps need renumbering (next free is
0.67.2) along with theirHISTORYand contract entries.VERSIONbump at all, and it editsweb/dashboard.html—it will also need
src/venc_webui.cregenerated (make webui-checkverifiesthe gzip blob against the HTML), which conflicts with the regeneration in this
range.
Happy to help rebase any of them, and sorry for the churn — this bundle had been
accumulating on the fork for a while.