feat(sensor-oak): stereo+IMU modality, factory calib, on-device H.264 viz, IR projector - #12
Conversation
…l frame Adds the depthai IMU node (ACCELEROMETER_RAW + GYROSCOPE_RAW) to oak_open_rgbd, gated on the new imu_hz parameter plumbed through the C ABI and OakSource::open_rgbd / open_rgbd_video. Samples are rotated into the CAM_A RGB optical frame using the device's imu-to-camera extrinsics (determinant-checked against wiped EEPROMs; identity fallback exposed via imu_aligned()). A pipeline that fails to start with an IMU retries once without it, so a broken IMU never costs the image streams. The stereo modality's raw-frame behavior is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
depthai-core's host nodes (Rectification/ImageAlign/PointCloud) require OpenCV; conda-forge libopencv makes the depthai build self-contained on machines without system OpenCV. The cargo -j2 RAM cap becomes aarch64-specific. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…enCV Two vendored-source patches applied by build_depthai.sh (idempotent): missing <deque> include in EventsManager.hpp (GCC 13), and moving the cv-free Rectification::getCalibrationData out of the OpenCV guard so DEPTHAI_OPENCV_SUPPORT=OFF links without dangling vtable symbols. DEPTHAI_CMAKE_EXTRA lets a host pass extra configure flags; sensor-oak itself needs no OpenCV, so the conda-forge libopencv dep is gone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
create<node::ToF>() instantiates Subnode<ImageFilters> (an OpenCV-only source), leaving dangling vtable symbols in a no-OpenCV build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Passive stereo collapses to single-digit valid-depth percentages on texture-poor or dim scenes (measured 1.4% valid on an indoor laptop rig). Default intensity 0.8, OAK_IR env overrides (0 = off); projector-less boards are unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extrinsics gate is now a real rotation test: isfinite over all entries, det > 0, |R·Rᵀ − I| < 1e-3, and the exact-identity not-calibrated sentinel rejected — NaN and shear/scale EEPROMs can no longer pass as aligned. The whole-device open retry is replaced by a getConnectedIMU() preflight inside add_imu_node (no more silent IMU loss on transient failures, no re-binding a different camera on multi-OAK rigs, half the no-device latency); the stereo modality now aligns samples too, against its CAM_B reference frame. IMU samples with default (zero-timestamp) reports are skipped; accuracy is deliberately NOT gated — firmware leaves it UNRELIABLE on *_RAW streams (measured: the gate silenced the stream). Batch cap back within the documented maximum (5). One readCalibration() per open feeds all EEPROM consumers. imu_hz clamps to 400. Rejection paths log distinct reasons. Duplicate OakSource constructors and the copied stereo IMU block collapsed. Docs (oak_bridge.h contract, rgbd/stereo/imu rustdoc, CLAUDE.md) updated to match reality. Verified on hardware (OAK-D USB, x86 host): streams open, IMU 98 Hz, no-IMU-extrinsics EEPROM degrades loudly to raw chip frame. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Applied all 15 review findings (see latest commit). Hardware-verified on an x86 host with an OAK-D USB: streams + IMU at 98 Hz. One review deviation worth knowing: the proposed accuracy==UNRELIABLE gate on IMU reports had to be dropped — firmware does not populate accuracy on the *_RAW streams, so the gate silenced the entire IMU (measured 0 msgs/5 s); the zero-timestamp gate covers the default-report poison case. |
…ification open_stereo delivers a RAW pair: depthai's Camera node can only undistort, never rectify — it builds its remap with an identity rectifying rotation, and only StereoDepth applies R1/R2, which forces the whole disparity block. So a stereo consumer has to rectify on the host, and until now this crate gave it nothing to rectify with. New `calib` module: per-eye intrinsics at the streamed size, per-eye distortion with its model, the metric left->right extrinsic, and a derived baseline. Two conventions are load-bearing, because depthai's defaults get both wrong for this purpose and neither error is visible in the output: * the extrinsic is the CALIBRATED one, not the board-design "spec" one; * translations are in METRES, not depthai's default centimetres. baseline_m is derived from the same extrinsic as the rotation, so the two can never disagree — unlike getBaselineDistance(), which carries that same pair of defaults independently and can silently drift from the matrix beside it. Also corrected three doc claims that were wrong rather than merely thin: open_stereo says GRAY8, not RGB888; it states the pair is unrectified and warns that pre-undistorting would be silently double-corrected downstream; and OakStereoFrame notes that left_image()/right_image() carry the frame's own retain handle, so they outlive the borrow and callers need not copy pixels out defensively. OakIntrinsics now says which camera it describes per modality, that the values are raw, and that a wiped EEPROM yields all zeros with no error — check fx > 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cargo treats branch/rev/tag on ONE git URL as three different sources, so this workspace's tag = v0.1.15-rc.5, a downstream consumer's rev = 1bc04a1 and kornia-slam's branch = main produced THREE kornia-image crates in any graph containing all three — and `Image<u8,1>` three times links nowhere. The workaround was a [patch] table pointing at local worktree paths, which meant the consuming workspace could not build on any other machine. kornia-slam is read-only and says branch = main, so that is the spec everything else has to adopt. Reproducibility comes from the committed Cargo.lock, which pins the exact commit. The cuda feature exists on main for kornia-image/tensor/imgproc, so the feature set here is unchanged.
…odality oak_open_stereo gains enable_h264: attaches the colour camera (CAM_A) to the shared hardware-encoder helper, so the stereo consumer can also serve a viewing stream — the encoder runs on the device and only the ~OAK_H264_KBPS bitstream crosses the link, costing the stereo pair nothing on the host. Same degrade rule as the IMU: a board without CAM_A skips the stream (has_video stays false) rather than failing the pair. Drained through the existing next_video()/video_q path the RGBD modality already uses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ps in add_h264_encoder Review cleanup (r5): the H.264 preflight re-walked getConnectedCameras() 55 lines after the stereo gate walked the same vector — one scan now sets all three flags. The fps floor moves inside add_h264_encoder so all three call sites get the same guard instead of one guarded and two bare. Stale comment claiming CAM_A "isn't even in this pipeline" fixed — with enable_h264 it is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… on the stereo path Adversarial review of this PR found three merge-blockers and five should-fixes. Blockers: - The crate did not compile. `open_stereo` gained `h264` and neither example followed; nothing caught it because the only job that compiles anything (`jetson`) is `if: false`. Both examples updated, and CI gains a `deps` job (`cargo metadata --locked`) — it cannot compile on a hosted runner (build.rs builds the depthai shim) but it does catch a stale lock, which is how the spec change below would otherwise have slipped through. - Two kornia graphs in one build: sensor-oak had moved to `branch = "main"` while vrt still pinned `tag = v0.1.15-rc.5`, so `Image`/`Tensor` existed twice (identical version strings — the diagnostic would read "expected Image, found Image") and Cargo.lock still recorded the old specs, so `--locked` failed outright. vrt now points at a vision-rt commit carrying the same `branch = "main"` spec. NOTE it is a spec-only commit on top of e1de223 (`chore/kornia-spec-on-xfeat`), the rev this crate already validates against: e1de223 is NOT on vision-rt's main, and main has since diverged on the vrt-xfeat API (EngineProfile.inputs -> input, Matcher::submit -> submit_match, Descriptors removed). Moving to a main-based rev needs the xfeat example migrated against hardware — separate work; kornia/vision-rt#27 carries the main-side repin. - `imu_hz` was clamped to the BNO086's 400 Hz maximum on the RGBD path only. On the stereo path an out-of-range rate makes the firmware's sensor-enable throw at pipeline start, which fails the whole open — losing the stereo pair over an IMU rate, the opposite of the header's degrade contract. Should-fixes: - The H.264 attach is now wrapped: the stream is viz-only by contract, so a board that rejects the colour node or the stereo-sized NV12 output degrades (has_video stays false) instead of taking the stereo pair down. - `oak_open_stereo` clamps `fps < 1` like `oak_open_rgbd` does. - The specific stereo-calibration failure reason is kept on the device and returned, instead of being overwritten by a generic guess (and it no longer leaves a stale message in the thread-local error slot after a good open). - The zero-timestamp IMU gate counts and reports what it drops: a skip count near half the requested rate means the gate, not the firmware, is setting the IMU rate (measured 98 Hz against a 200 Hz request). - `next_imu` reports a shim failure once instead of reading as a quiet sensor forever (rc == -1 is an exception, not "nothing queued"). - A present-but-zero baseline is rejected: it passes every read above and reaches a rectifier as NaN remap tables, which the rustdoc promises cannot happen. Header docs corrected (colour camera/encoder attach only when enable_h264 asks; fps also drives the viz stream). Verified on the Orin: `cargo test -p sensor-oak --all-targets --release` builds every target including both examples, tests green, `cargo metadata --locked` clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Pushed Blockers
Worth your call — the vrt pin Fixing (2) surfaced something structural: this crate pins vrt So this PR pins a spec-only commit on top of e1de223 ( Should-fixes: H.264 attach wrapped so it degrades instead of killing the stereo open; Verified on the Orin: |
… fix a false CI note Final review pass over the previous fixes: - The RGBD path's OPTIONAL H.264 attach was still unguarded while the stereo one had been wrapped — the same board that rejects the colour node would take depth + RGB down with it. Guarded, with a note on why this crate has two degrade idioms (the IMU can be preflighted with getConnectedIMU(); the encoder has no equivalent capability query, so it attempts and catches). The MANDATORY attach in the video-only modality stays unguarded: there the stream is the point. - clamp_imu_hz lives once in lib.rs; both open paths call it. The 400 Hz constant and its message had been copy-pasted. - The CI note I added last round was false twice: it named a job (`check`) that was renamed to `deps` before landing, and claimed compile coverage that neither enabled job provides. It now says plainly that nothing in CI compiles while `jetson` is disabled. - fps-guard comments in the two open paths agreed on the value but described different failures; header line rewrapped to the block's style. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… three H.264 sites Full-branch simplify/altitude review (the earlier pass covered only the last commit). Applied: Correctness-adjacent - oak_poll_stereo had no null-queue guard, unlike every sibling poll: calling it on an RGBD device dereferenced a null shared_ptr. ONE handle type serves all three modalities, so the compiler cannot stop that — it now errors. - The third H.264 attach site (video-only) was unguarded, and it is exactly where an RGBD request LANDS when the board cannot do depth: an encoder failure took the whole open down. All three sites now go through one try_add_h264_encoder; the video-only path, where the stream IS the output, fails with an error that says so. - The four `has_*` mirror bools were `queue != nullptr` restated by hand, and the pollers already tested the queue. Deleted; the queue IS the capability. Reuse / dedup - read_intrinsics(socket) replaces the CAM_A helper plus its inlined CAM_B twin; has_socket() replaces two getConnectedCameras() scans; attach_imu() fuses add_imu_node + read_imu_rotation, whose ORDER was load-bearing (the second early-returns unless the first started the IMU) and repeated three times; clamp_imu_hz moved next to the rest of the IMU code. - read_imu_rotation's six reject paths retyped one shared clause; a local `reject(why)` keeps the consequence in one place and the REASONS specific. - The blank-calibration reset (which must restore width/height or produce a silent rescale) is one lambda instead of three copies. - static_assert on sizeof(oak_stereo_calib): the Rust side pinned its own layout, which passes happily when the header is what changed. - The frame epoch offset is cached like the IMU drain already does — frames and IMU are promised one timeline, and recomputing per frame let the two clocks' jitter separate stamps taken at the same instant. Docs the branch had made wrong: oak_has_imu's "failed to start" path (the IMU is preflighted, so it does not exist), oak_intrinsics naming CAM_B for a modality where it is CAM_A, add_h264_encoder's caller list, a "gate below" that is above, and several references to the long-gone oak_open(). Verified on the Orin: cargo test -p sensor-oak --all-targets --release builds every target including both examples; tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This is a standalone driver crate: naming a specific downstream in its docs dates them (the consumer gets renamed, or a second one appears) and tells a reader to go look at a repo they may not have. Describe the ROLE instead — "a camera producer", "a viewer/recorder" — which is what the sentences were actually about. Also drops the build script's pointer at where the OpenCV-off depthai patches live: the contract is simply that the caller applies whatever source fixes its chosen configuration needs before invoking the script. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bcd8d17 to
7215559
Compare
The full sensor-oak arc, validated live on an Orin + laptop pair:
imu_alignedis the only record of that — reading it backwards looks exactly like an accelerometer bias).oak_open_stereo): hardware-synced left/right GRAY8 on the device capture-stamp timeline, IMU on its own queue, loud failure when CAM_B/CAM_C are missing.stereo_calib) for host rectification — raw pair + calib, the host owns the rectifier.enable_h264): CAM_A + a shared encoder helper, degrading cleanly on boards without a colour camera; only the bitstream crosses the link.DEPTHAI_CMAKE_EXTRApassthrough, a$TAG-keyed install stamp so a pin bump actually rebuilds, and kornia namedbranch = "main"the way kornia-slam does (cargo keys a git source on the exact spec string, so a differently-spelled pin yields twoImage/Tensortypes in one graph).Review rounds folded in
An adversarial correctness pass plus a full-branch simplify pass. The IMU rotation direction was verified against depthai's own source (
getImuToCameraExtrinsicsreturns T_camera←imu, applied in that direction, matching depthai's RTABMap/Basalt VIO nodes), as were the stereo extrinsic direction/units and the FFI struct layout.Fixed as a result:
open_stereogained a parameter and neither example followed. Nothing caught it because the only job that compiles anything (jetson) isif: false; CI gains adepsjob (cargo metadata --locked) that at least catches a stale lock.oak_poll_stereohad no null-queue guard, unlike every sibling poll — calling it on an RGBD device dereferenced a nullshared_ptr. One handle type serves all three modalities, so the compiler cannot prevent that; it now errors.try_add_h264_encoder; the video-only path, where the stream is the output, fails with an error that says so.imu_hzwas clamped to the BNO086's 400 Hz on the RGBD path only; out of range on the stereo path, the firmware's sensor-enable throws atpipeline.start()and the caller loses the stereo pair over an IMU rate.has_*flags werequeue != nullptrrestated by hand (and the pollers already tested the queue) — deleted; the queue is the capability.read_intrinsics(socket), onehas_socket(), oneattach_imu()(whose call order is load-bearing — the second half early-returns unless the first started the IMU), oneclamp_imu_hz, one blank-calibration builder, astatic_asserton the C-side struct size to complement the Rust-side layout test, and a cached epoch offset so frame and IMU stamps share one clock read.Verified on the Orin:
cargo test -p sensor-oak --all-targets --releasebuilds every target including both examples; tests green;cargo metadata --lockedclean.Note on the vrt pin
vrt/vrt-xfeatpoint at a vision-rt commit carrying the samebranch = "main"kornia spec, on the line this crate already validates against. vision-rt'smainhas since diverged on the vrt-xfeat API (EngineProfile.inputs→input,Matcher::submit→submit_match,Descriptorsremoved), so moving to a main-based rev needsoakd_xfeat_stereomigrated against hardware — separate work.🤖 Generated with Claude Code