Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 71 additions & 1 deletion libs/odometry/multi_visual_odometry_base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,51 @@

namespace cuvslam::odom {

namespace {

bool IsAllZeroHostU8(const ImageSource& source, const ImageShape& shape) {
if (source.data == nullptr || source.memory_type != ImageSource::Host || source.type != ImageSource::U8) {
return false;
}

const size_t num_channels = source.image_encoding == ImageEncoding::RGB8 ? 3 : 1;
const auto* data = static_cast<const uint8_t*>(source.data);
const size_t num_values = static_cast<size_t>(shape.width) * static_cast<size_t>(shape.height) * num_channels;
return std::all_of(data, data + num_values, [](uint8_t value) { return value == 0; });
}

bool AreAllAvailableImagesBlack(const Sources& sources, const sof::Images& images) {
bool saw_image = false;
for (size_t cam_id = 0; cam_id < images.size(); ++cam_id) {
if (images[cam_id] == nullptr || cam_id >= sources.size() || sources[cam_id].data == nullptr) {
continue;
}
saw_image = true;
if (!IsAllZeroHostU8(sources[cam_id], images[cam_id]->get_image_meta().shape)) {
return false;
}
}
return saw_image;
}

void DropCurrentImages(sof::Images& images) {
for (auto& image : images) {
image = nullptr;
}
}

void ClearFrameStat(IVisualOdometry::VOFrameStat* stat) {
if (!stat) {
return;
}
stat->keyframe = false;
stat->heating = false;
stat->tracks2d.clear();
stat->tracks3d.clear();
}

} // namespace

MultiVisualOdometryBase::MultiVisualOdometryBase(const camera::Rig& rig, const camera::FrustumIntersectionGraph& fig,
const Settings& settings, bool use_gpu)

Expand Down Expand Up @@ -73,12 +118,38 @@ bool MultiVisualOdometryBase::track(const Sources& curr_sources, [[maybe_unused]
TRACE_EVENT ev = profiler_domain_.trace_event("MultiVisualOdometryBase::track()", profiler_color_);
const int64_t timestamp = (*first_image)->get_image_meta().timestamp; // current frame timestamp
Isometry3T predicted_world_from_rig = prev_world_from_rig_;
Isometry3T world_from_rig;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Use Pose for the new SE(3) variable.

world_from_rig stores an SE(3) transform. Replace the new Isometry3T declaration with the Pose type from cuvslam2.h.

As per coding guidelines, “Use the Pose type from cuvslam2.h for all SE(3) transforms.”

🧰 Tools
🪛 Clang (14.0.6)

[warning] 121-121: variable 'world_from_rig' is not initialized

(cppcoreguidelines-init-variables)

🤖 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/odometry/multi_visual_odometry_base.cpp` at line 121, Replace the
Isometry3T type of the world_from_rig declaration with the Pose type from
cuvslam2.h, keeping the variable name and initialization behavior unchanged.

Source: Coding guidelines

pipelines::ISFMSolver& solver = get_solver();

if (settings_.use_prediction) {
do_predict(&prediction_model_, timestamp, predicted_world_from_rig);
}

if (can_track_visual_blackout() && AreAllAvailableImagesBlack(curr_sources, curr_images)) {
for (auto& cam_observations : observations_) {
cam_observations.clear();
}

const bool have_pose = solver.solveNextFrame(
timestamp, sof::FrameState::None, observations_, world_from_rig, static_info_exp,
{per_frame_setting.sba, per_frame_setting.sm, per_frame_setting.vo_pnp, per_frame_setting.inertial_stereo_pnp,
per_frame_setting.imu_pnp, per_frame_setting.icp});

DropCurrentImages(curr_images);
if (!have_pose) {
ClearFrameStat(last_frame_stat_.get());
delta = Isometry3T::Identity();
static_info_exp.setZero();
return false;
Comment on lines +139 to +143

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Reset solver state after a failed blackout solve.

The normal PnP failure path resets state, but this path returns directly. SolverSfMInertial::solveNextFrame advances prev_pose_ts_ns at Line 616 and replaces last_frame_preint_ at Line 698 before it returns false. The next frame can therefore use a pose from before the blackout with IMU state from after it.

Call reset() before returning false.

Proposed fix
     DropCurrentImages(curr_images);
     if (!have_pose) {
+      reset();
       ClearFrameStat(last_frame_stat_.get());
       delta = Isometry3T::Identity();
       static_info_exp.setZero();
       return false;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!have_pose) {
ClearFrameStat(last_frame_stat_.get());
delta = Isometry3T::Identity();
static_info_exp.setZero();
return false;
if (!have_pose) {
reset();
ClearFrameStat(last_frame_stat_.get());
delta = Isometry3T::Identity();
static_info_exp.setZero();
return false;
🤖 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/odometry/multi_visual_odometry_base.cpp` around lines 139 - 143, Update
the !have_pose failure path to call reset() before returning false, ensuring
SolverSfMInertial state is cleared after a failed blackout solve while
preserving the existing frame-stat, delta, and static-info resets.

}

ClearFrameStat(last_frame_stat_.get());
prediction_model_.add_known_pose(world_from_rig, timestamp);
delta = prev_world_from_rig_.inverse() * world_from_rig;
prev_world_from_rig_ = world_from_rig;
return true;
}

sof::FrameState frame_type;
for (auto& cam_observations : observations_) {
cam_observations.clear();
Expand All @@ -98,7 +169,6 @@ bool MultiVisualOdometryBase::track(const Sources& curr_sources, [[maybe_unused]
IVisualOdometry::VOFrameStat* stat = last_frame_stat_.get();
std::vector<Track2D>* tracks2d = stat ? &(stat->tracks2d) : nullptr;
Tracks3DMap* tracks3d = stat ? &(stat->tracks3d) : nullptr;
Isometry3T world_from_rig;

const bool have_pose =
solver.solveNextFrame(timestamp, frame_type, observations_, world_from_rig, static_info_exp,
Expand Down
2 changes: 2 additions & 0 deletions libs/odometry/multi_visual_odometry_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ class MultiVisualOdometryBase : public IVisualOdometry {
virtual pipelines::ISFMSolver& get_solver() = 0;

protected:
virtual bool can_track_visual_blackout() const { return false; }

void reset();
camera::Rig rig_;
camera::FrustumIntersectionGraph fig_;
Expand Down
2 changes: 2 additions & 0 deletions libs/odometry/stereo_inertial_odometry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,6 @@ std::optional<pipelines::SolverSfMInertial::ImuState> StereoInertialOdometry::Ge
return solver_.GetImuState();
}

bool StereoInertialOdometry::can_track_visual_blackout() const { return solver_.get_gravity().has_value(); }

} // namespace cuvslam::odom
2 changes: 2 additions & 0 deletions libs/odometry/stereo_inertial_odometry.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class StereoInertialOdometry : public MultiVisualOdometryBase {
std::optional<pipelines::SolverSfMInertial::ImuState> GetImuState() const;

private:
bool can_track_visual_blackout() const override;

imu::ImuCalibration calib_;
pipelines::SolverSfMInertial solver_;
};
Expand Down
13 changes: 10 additions & 3 deletions libs/pipelines/track_online_inertial.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,10 @@ bool SolverSfMInertial::solveNextFrame(int64_t time_ns, const sof::FrameState& f
last_valid_pose.preintegration = sba_imu::IMUPreintegration(curr_pose.gyro_bias, curr_pose.acc_bias);
}

integrated = !pnp_result && imu_state == StateMachine::State::Ok;
const bool no_observations = obs_vector_.empty();
const bool integrated_from_blackout =
!pnp_result && no_observations && no_drops && !is_first_run && maybe_gravity.has_value();
integrated = !pnp_result && (imu_state == StateMachine::State::Ok || integrated_from_blackout);
TraceMessage(
"Frame: pnp=%d integrated=%d imu_state=%d obs=%d vel=[%.3f,%.3f,%.3f] gbias=[%.4f,%.4f,%.4f] "
"abias=[%.4f,%.4f,%.4f]",
Expand All @@ -657,10 +660,14 @@ bool SolverSfMInertial::solveNextFrame(int64_t time_ns, const sof::FrameState& f
}

if (integrated) {
integ_kf.predict_pose(*maybe_gravity, integ_kf.preintegration, curr_pose);
if (integrated_from_blackout) {
prev_pose.predict_pose(*maybe_gravity, prev_pose.preintegration, curr_pose);
} else {
integ_kf.predict_pose(*maybe_gravity, integ_kf.preintegration, curr_pose);
}
TraceDebug("Pose was integrated!");
}
if (pnp_result || imu_state == StateMachine::State::Ok) {
if (pnp_result || integrated) {
// either we successfully converged, or successfully integrated the pose
world_from_rig = curr_pose.w_from_imu * imu_from_rig;
rig_from_w = world_from_rig.inverse();
Expand Down
Loading