[feat] Epipolar-guided L2R tracking for multicamera mode - #71
Open
slepichev wants to merge 7 commits into
Open
Conversation
Replace the projection-based initial guess for left-to-right (L2R) LK
tracking with per-pixel epipolar curves. Both MultiSOFCPU and MultiSOFGPU
now seed LK from precomputed candidate positions sampled along each
feature's epipolar curve, tried in first-successful-wins order.
Mechanics:
- EpipolarCurves precomputes a corner-grid of per-pixel candidate lists
(log-uniform depth sampling, dedup at >= 1 top-level pixel spacing).
Candidates() bilinearly interpolates from the four surrounding corners.
- Depth range flows from Odometry::Config::{min_depth,max_depth}. Any
negative value auto-detects from the pair baseline: ~7 cm baseline
-> [0.1 m, 20 m], KITTI-scale (~0.5 m) -> [7 m, 1000 m].
- Multi-launch scan stops advancing the candidate index once
kL2REarlyExitFraction of observations are tracked; identical semantics
on CPU and GPU.
- Search radius capped at CrossCamSearchRadius(top_l) to prevent LK from
drifting onto decoy features (previously scaled with baseline * focal
and could reach hundreds of pixels).
- GPU batching sentinels ({0, 0} offset for exhausted / already-won
points) are discarded host-side; only real candidate outcomes can win.
Hygiene:
- Per-pair scratch (uvL, cands, winners) lives on the pair struct so
allocations persist across frames.
- AutoDetectDepthRange throws std::invalid_argument on non-positive or
non-monotonic ranges after auto-detection.
- .at() on pair lookups fails loudly on unknown ids instead of silently
inserting a default-constructed entry.
Public API:
- Odometry::Config::{min_depth,max_depth} (default -1.f = auto).
- Python binding, YAML loader (python/utils.py) and debug_dump.cpp
updated.
- TROUBLESHOOTING.md Step 8 covers when and how to override.
slepichev
requested review from
hrabeti-nvidia and
vikuznetsov-nvidia
as code owners
July 28, 2026 01:10
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMulticamera L2R tracking now accepts configurable depth bounds, generates cached epipolar candidates, and uses candidate-driven matching on CPU and GPU. Configuration is exposed through C++, Python, JSON dumps, and troubleshooting documentation, with new stereo regression tests. ChangesMulticamera L2R tracking
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant MultiSOFCPU
participant MultiSOFBase
participant EpipolarCurves
participant IFeatureTracker
MultiSOFCPU->>MultiSOFBase: GetOrBuildEpipolarCurves(camera pair, pyramid level)
MultiSOFBase->>EpipolarCurves: Candidates(left observation)
EpipolarCurves-->>MultiSOFCPU: Right-image candidate positions
MultiSOFCPU->>IFeatureTracker: Track candidate UV pair
IFeatureTracker-->>MultiSOFCPU: Tracking result
MultiSOFCPU-->>MultiSOFCPU: Publish first successful winner
sequenceDiagram
participant MultiSOFGPU
participant MultiSOFBase
participant EpipolarCurves
participant track_points
MultiSOFGPU->>MultiSOFBase: GetOrBuildEpipolarCurves(camera pair, pyramid level)
MultiSOFBase->>EpipolarCurves: Candidates(left observation)
EpipolarCurves-->>MultiSOFGPU: Candidate offsets
MultiSOFGPU->>track_points: Launch candidate index
track_points-->>MultiSOFGPU: Track statuses and information
MultiSOFGPU-->>MultiSOFGPU: Store winners and publish tracks
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/sof/epipolar_curves.cpp`:
- Line 29: Replace the fixed kNumDepthSamples value used by
GetOrBuildEpipolarCurves with an adaptive, bounded sample count based on each
epipolar curve’s projected pixel extent, reducing projections for short curves
while preserving sufficient sampling for long curves. Update the sampling logic
across the curve-building path, including the code spanning the referenced
range, so it no longer performs 10,000 projections per corner.
- Around line 42-57: Update AutoDetectDepthRange to trigger
baseline-interpolated defaults only when min_depth or max_depth is strictly
negative, preserving zero as an explicit value subject to validation. Extend its
final validation to reject non-finite bounds, including positive infinity, while
retaining the required 0 < min_depth < max_depth relationship and
std::invalid_argument behavior.
In `@libs/sof/epipolar_curves.h`:
- Around line 43-45: Update the EpipolarCurves constructor and its cache-builder
path to accept Pose for the right_from_left SE(3) transform, following the
cuvslam2.h API type. Convert Pose internally only at points requiring Eigen
operations, and update all related declarations, definitions, and call sites
consistently.
In `@libs/sof/sof_multicamera_gpu.cpp`:
- Line 73: Update the tracker construction flow around CreateGPUTracker so a
nullptr result is handled immediately and cannot reach
pair.tracker->track_points(...). Replace reliance on the assert with a
release-safe fail-fast check while preserving the existing valid-tracker path.
- Around line 113-165: Refactor the candidate loop around tracks_data and the
winner-update logic to compact only active point indices into a dense
per-iteration sub-batch. Launch copy and tracking operations with active_count,
then map returned results back to their original points through an index vector,
preserving candidate ordering, winner handling, early exit, and tracked_count
behavior without processing sentinel entries.
- Around line 90-105: Guard PrimaryToSecondaryGPUTracker::tracks_data before the
loops that index it with primary_obs.size(). Ensure the buffer has capacity for
n elements, preferably resizing it to n, or clamp n to its fixed capacity and
consistently use the clamped count in iteration and GetTrackingResults paths;
prevent any unchecked tracks_data[i] access when the observation count exceeds
1000.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: e7f8f189-c228-473d-b0ee-03ecdbda1762
📒 Files selected for processing (15)
TROUBLESHOOTING.mdlibs/cuvslam/cuvslam2.cpplibs/cuvslam/cuvslam2.hlibs/cuvslam/debug_dump.cpplibs/sof/CMakeLists.txtlibs/sof/epipolar_curves.cpplibs/sof/epipolar_curves.hlibs/sof/internal/sof_multicamera_base.hlibs/sof/internal/sof_multicamera_cpu.hlibs/sof/internal/sof_multicamera_gpu.hlibs/sof/sof_config.hlibs/sof/sof_multicamera_base.cpplibs/sof/sof_multicamera_cpu.cpplibs/sof/sof_multicamera_gpu.cpppython/cuvslam2.cpp
slepichev
changed the title
Test Results
cuVSLAM Evaluation KPIs
Artifacts |
The epipolar-guided left-to-right tracker added in de8f3de shipped without unit coverage on either the CPU or the GPU path. sof_l2r_test drives MultiSOF through its public trackNextFrame on a single frame and asserts the number of published secondary-camera observations. One test per implementation; the GPU case compiles only under USE_CUDA. The frame comes from the new test_data/sof/lr_test dataset (640x400, 7.5 cm baseline, polynomial distortion). Images are committed next to stereo.edex, so the test is self-contained and needs no dataset mount. The 7.5 cm baseline also exercises the small-stereo depth-range auto-detect anchor. Under USE_RERUN the two frames are logged side by side with one line per match, through the shared RerunVisualizer. Off by default like every other unit test; run with RERUN=1 to view. libs/sof/test gains a pnp link (TrackPerFrameSettings pulls in PNPSettings::InertialSettings) and, under USE_RERUN, visualizer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/sof/test/sof_l2r_test.cpp`:
- Line 90: In TrackL2R, replace the non-fatal expectation for edex_rig.start()
with failure reporting that immediately returns 0 when startup fails; likewise,
handle getFrame() failure by reporting it and returning 0 before accessing frame
data. Apply these changes at libs/sof/test/sof_l2r_test.cpp lines 90-90 and
114-114.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 5f4136f7-edbf-4044-918d-14b5b739fb01
⛔ Files ignored due to path filters (2)
test_data/sof/lr_test/left/000000.pngis excluded by!**/*.pngtest_data/sof/lr_test/right/000000.pngis excluded by!**/*.png
📒 Files selected for processing (3)
libs/sof/test/CMakeLists.txtlibs/sof/test/sof_l2r_test.cpptest_data/sof/lr_test/stereo.edex
EpipolarCurves sampled 10000 log-uniform depths per pixel corner and then threw nearly all of them away: the dedup pass keeps a sample only if it lands at least one top-level pixel from the last kept one, which leaves a handful of candidates at KITTI-scale baselines and a few dozen at short ones. Over 99% of the projections were computed and discarded. 50 samples cover the same log-uniform range densely enough to reach every candidate the dedup would have kept, so the curves come out effectively unchanged while the per-pair build cost drops by ~200x. Verified: no metric degradation on KITTI, and SOFL2R tracks the same number of points on test_data/sof/lr_test for both CPU and GPU. Also replaces the repeated type names on three adjacent casts with const auto. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TrackL2R used non-fatal expectations for CameraRigEdex::start and getFrame, then kept going. EXPECT_* cannot abort a function that returns a value, so a missing or unreadable test_data/sof/lr_test carried on into getCamerasNum() == 0, and fig.primary_cameras().front() dereferenced an empty vector. getFrame failing left metas empty and metas[0] read out of bounds. Either way a dataset problem surfaced as a crash instead of a test failure. Report the ErrorCode with ADD_FAILURE and return 0 immediately, so the test fails with the reason instead of taking the process down. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/sof/epipolar_curves.cpp (1)
63-74: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winBounds-check before converting to
size_t.!(x >= 0.f)still lets+infand oversized inputs reachstatic_cast<size_t>, which is undefined behavior. Reject non-finite values and coordinates at or beyond the last interpolable cell before the cast.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/sof/epipolar_curves.cpp` around lines 63 - 74, Update the bounds validation in the epipolar-curve interpolation path before the v0/u0 conversions: reject non-finite v_f or u_f values and reject coordinates at or beyond the last interpolable cell using the available curve dimensions. Ensure these checks occur before static_cast<size_t>, while preserving interpolation for valid coordinates whose v1/u1 corners are in range.
♻️ Duplicate comments (1)
libs/sof/epipolar_curves.cpp (1)
45-57: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject zero and non-finite depth bounds.
The public contract in
libs/sof/epipolar_curves.hspecifies negative values for auto-detection, but<= 0.fsilently converts an explicit zero.max_depth = +infalso passes validation and later produces non-finite sampling values. Use< 0.ffor auto-detection and require both bounds to be finite.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/sof/epipolar_curves.cpp` around lines 45 - 57, Update AutoDetectDepthRange so only negative min_depth or max_depth values trigger auto-detection, preserving explicit zero values for validation. Extend the final validation to require both bounds are finite while retaining the 0 < min_depth < max_depth constraint.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@libs/sof/epipolar_curves.cpp`:
- Around line 63-74: Update the bounds validation in the epipolar-curve
interpolation path before the v0/u0 conversions: reject non-finite v_f or u_f
values and reject coordinates at or beyond the last interpolable cell using the
available curve dimensions. Ensure these checks occur before
static_cast<size_t>, while preserving interpolation for valid coordinates whose
v1/u1 corners are in range.
---
Duplicate comments:
In `@libs/sof/epipolar_curves.cpp`:
- Around line 45-57: Update AutoDetectDepthRange so only negative min_depth or
max_depth values trigger auto-detection, preserving explicit zero values for
validation. Extend the final validation to require both bounds are finite while
retaining the 0 < min_depth < max_depth constraint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 51667ab1-5a4e-4fa1-833b-3fd2043f2a6c
📒 Files selected for processing (1)
libs/sof/epipolar_curves.cpp
Two gaps in AutoDetectDepthRange: min_depth / max_depth of exactly 0 triggered auto-detection. The public API and the header both document a *negative* value as the auto trigger, so a 0 is an explicit input and almost certainly a mistake — silently replacing it with a baseline-interpolated default hid that. Only negative values auto-detect now; 0 falls through to validation and throws. +inf passed validation. min_depth > 0 and min_depth < max_depth both hold for max_depth = +inf, so log_max was infinite, log_step was infinite, and log_d went NaN on the first decrement — every depth sample after that projected to NaN and the curves came out silently empty. Validation now requires both bounds to be finite. NaN was already rejected by the !(x > 0) / !(x < y) form and still is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/sof/epipolar_curves.cpp`:
- Around line 49-55: Update the min_depth and max_depth auto-detection guards in
the surrounding epipolar-curve setup so only finite negative values are replaced
with computed defaults. Ensure ±infinity and other non-finite bounds remain
unchanged and reach the existing validation condition, while preserving the
current interpolation for valid finite negative inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 1659f099-65fd-4874-a998-28dc2789b170
📒 Files selected for processing (1)
libs/sof/epipolar_curves.cpp
AutoDetectDepthRange silently replaced inputs it should have rejected: - 0 triggered auto-detection. The header and public API both document a negative value as the auto trigger, so 0 is an explicit input and almost certainly a mistake; substituting a baseline default hid it. - -inf triggered auto-detection, while NaN did not. Non-finite inputs now behave uniformly: none are replaced, all reach validation. - +inf passed validation, since min_depth > 0 and min_depth < +inf both hold. log_max was then infinite, log_step infinite, and log_d went NaN on the first decrement, so every depth sample projected to NaN and the curves came out silently empty. Only finite negative values auto-detect now, and validation requires both bounds to be finite on top of 0 < min_depth < max_depth. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CreateGPUTracker handles only TrackerType::LK and LKHorizontal; KLT and KLTHorizontal fall to the default branch and return nullptr. That null was stored and only checked by assert(pair.tracker != nullptr) in LaunchTrackingPrimaryToSecondary, which is compiled out under NDEBUG. Release builds configured with a KLT lr_tracker therefore dereferenced null inside track_points instead of reporting the bad setting. Throw std::invalid_argument from the MultiSOFGPU constructor when the tracker cannot be created, so the failure names the cause and the existing assert becomes a genuine invariant rather than the only check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Summary by CodeRabbit
min_depth/max_depth) with auto-detection when set negative.min_depth/max_depthin the PythonOdometry.Config.min_depth/max_depth.