Added support for Kitti dataset by jeffin07 · Pull Request #20 · kornia/kornia-slam

jeffin07 · 2026-04-09T17:07:39Z

Added support for Kitti Dataset, Odometry Grascale. To Run use

cargo run --release --manifest-path examples/orb_slam/Cargo.toml -- --dataset kitti --data /path_to_kitti_dataset/

Fixes #18

cjpurackal · 2026-04-13T09:04:47Z

 }
+
+
+impl PipelineConfig {


Maybe a better way is to define the pipeline config in the dataset specific file, in this case kitti.rs and by extension, same for euroc.rs

cjpurackal · 2026-04-13T09:06:13Z

    // ── ORB detector (used externally before feeding Pipeline) ─────────────
    let detector = kornia_imgproc::features::OrbDetector {
-        n_keypoints: 1000,
+        // KITTI generally needs a denser feature set to keep tracking stable.


remove unnecessary commnets

cjpurackal · 2026-04-13T09:07:00Z

@jeffin07 which specific dataset from kitti did you run this on?

Copilot

Pull request overview

Adds KITTI (Odometry grayscale) dataset support to the examples/orb_slam runner, including dataset selection via CLI and KITTI-tuned SLAM defaults.

Changes:

Add a KITTI dataset reader and export it from the common datasets module.
Extend the ORB-SLAM example CLI to select between euroc and kitti, and tune feature density + pipeline config for KITTI.
Adjust bootstrap behavior and update dependency lockfiles for the kornia git branch used by the example.

Reviewed changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
examples/orb_slam/src/pipeline.rs	Tweaks bootstrap failure handling and triangulation call site behavior in map growth.
examples/orb_slam/src/main.rs	Adds `--dataset` flag, switches dataset loading, tunes ORB keypoints for KITTI, and uses dataset-specific pipeline config.
examples/orb_slam/src/config.rs	Introduces `PipelineConfig::for_dataset()` with KITTI-tuned parameters.
examples/common/datasets/mod.rs	Exposes the new `kitti` module and `KittiDataset` type.
examples/common/datasets/kitti.rs	Implements KITTI odometry grayscale reader (calibration, times, image listing).
examples/orb_slam/Cargo.lock	Locks example dependencies to the kornia git branch commit.
Cargo.lock	Locks workspace dependencies to the kornia git branch commit.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-13T12:08:04Z

 /// CLI arguments.
 #[derive(argh::FromArgs)]
 #[argh(description = "Monocular visual odometry on EuRoC dataset")]
 struct Args {
+
+    /// dataset type: "euroc", "kitti"
+    #[argh(option, default="String::from(\"euroc\")")]
+    dataset: String,
+
    /// path to EuRoC dataset root (e.g. V1_01_easy/)
    #[argh(option)]
    data: String,


The CLI description still says it's for the EuRoC dataset only, but this PR adds --dataset kitti support. Please update the #[argh(description = ...)] text (and ideally the --data help string) so the usage/help output matches the new behavior.

Copilot · 2026-04-13T12:08:04Z

+    let (camera, samples): (PinholeCamera, Vec<datasets::euroc::DatasetSample>) = match dataset_kind.as_str()
+    {
+        "euroc" => {
+            println!("Euroc dataset unsupported");


The message printed for the "euroc" branch says "Euroc dataset unsupported", but the code proceeds to open and run EuRoC. This is misleading for users; adjust the message to reflect what’s actually happening (or remove it).

Suggested change

println!("Euroc dataset unsupported");

println!("EuRoC dataset");

Copilot · 2026-04-13T12:08:05Z

+    let dataset_kind = &args.dataset.to_lowercase();
+    let (camera, samples): (PinholeCamera, Vec<datasets::euroc::DatasetSample>) = match dataset_kind.as_str()
+    {


samples is typed as Vec<datasets::euroc::DatasetSample> even when --dataset kitti is selected. Since KITTI reuses DatasetSample via super::euroc, this works but creates an odd cross-module coupling. Consider moving DatasetSample/DatasetError into a shared module (e.g. examples/common/datasets/types.rs re-exported by datasets/mod.rs) and using that type here instead of referencing datasets::euroc::....

Copilot · 2026-04-13T12:08:05Z

@@ -67,17 +92,18 @@
        args.start_frame + n_frames
    );

-    // ── Camera (from EuRoC cam0 sensor.yaml) ───────────────────────────────
-    let camera = dataset.camera();
+    // // ── Camera (from EuRoC cam0 sensor.yaml) ───────────────────────────────
+    // let camera = dataset.camera();



There are several commented-out lines left in place (old EuRoC-only dataset/samples and camera setup). Since this is example code, keeping dead/commented code makes it harder to read and maintain; please remove the commented-out blocks if they’re no longer needed.

Copilot · 2026-04-13T12:08:05Z

+/// EuRoC `cam0` calibration loaded from `sensor.yaml`.
+#[derive(Debug, Clone, Copy)]
+pub struct KittiCameraCalibration {
+    /// Focal length in x.
+    pub fx: f64,
+    /// Focal length in y.
+    pub fy: f64,
+    /// Principal point x.
+    pub cx: f64,
+    /// Principal point y.
+    pub cy: f64,
+    /// Radial distortion coefficient.
+    pub k1: f64,
+    /// Radial distortion coefficient.
+    pub k2: f64,
+    /// Tangential distortion coefficient.
+    pub p1: f64,
+    /// Tangential distortion coefficient.
+    pub p2: f64,
+}
+
+impl KittiCameraCalibration{
+
+    /// Converts the parsed EuRoC calibration into a `PinholeCamera`.
+    pub fn to_pinhole_camera(self) -> PinholeCamera {
+        PinholeCamera {
+            fx: self.fx,
+            fy: self.fy,
+            cx: self.cx,
+            cy: self.cy,
+            k1: self.k1,
+            k2: self.k2,
+            p1: self.p1,
+            p2: self.p2,
+        }
+    } 
+}
+
+
+/// PNG images in `<root>/mav0/cam0/data/`.
+#[derive(Debug, Clone)]
+pub struct KittiDataset {


The module-level docs appear to be copy/pasted from the EuRoC reader (e.g., mentions of sensor.yaml and <root>/mav0/cam0/data/). Please update the docstrings to reflect KITTI’s actual structure (e.g., calib.txt, times.txt, image_0/).

Copilot · 2026-04-13T12:08:05Z

+    path::{Path, PathBuf},
+};
+
+use super::euroc::{DatasetError, DatasetSample, GroundTruthPose};


GroundTruthPose is imported but not used (and the ground-truth field is commented out). This will trigger unused-import warnings; remove the import (and/or implement the ground-truth support if intended).

Suggested change

use super::euroc::{DatasetError, DatasetSample, GroundTruthPose};

use super::euroc::{DatasetError, DatasetSample};

Copilot · 2026-04-13T12:08:06Z

+    // pub fn open(root: impl AsRef<Path>) -> Result<Self, DatasetError> 
+    pub fn open(root: impl AsRef<Path>) -> Result<Self, DatasetError>{
+
+         println!("Kitti dataset open method called");


println!("Kitti dataset open method called") introduces unconditional stdout output from a dataset loader. This makes the example noisy and can interfere with tooling; prefer using eprintln! consistently with the rest of the example, or remove the message / gate it behind a verbosity flag.

Suggested change

println!("Kitti dataset open method called");

eprintln!("Kitti dataset open method called");

Copilot · 2026-04-13T12:08:06Z

+        // let samples = Self::load_image(&root, &timestamp)?;
+
+        let samples = match Self::load_image(&root, &timestamp){
+            Ok(s) => s,
+            Err(DatasetError) => return Err(DatasetError::FileNotFound(root)),
+        };
+


This match arm pattern Err(DatasetError) is not a valid way to match a Result<_, DatasetError> and will not compile. Also, converting any error from load_image into FileNotFound(root) discards useful error context. Prefer let samples = Self::load_image(&root, &timestamp)?; or Err(e) => return Err(e) to preserve the actual failure (I/O vs parse, etc.).

Suggested change

// let samples = Self::load_image(&root, &timestamp)?;

let samples = match Self::load_image(&root, &timestamp){

Ok(s) => s,

Err(DatasetError) => return Err(DatasetError::FileNotFound(root)),

};

let samples = Self::load_image(&root, &timestamp)?;

Copilot · 2026-04-13T12:08:06Z

+    fn load_times(root: &Path) -> Vec<f64>{
+        let times_file = root.join("times.txt");
+
+        let times = match fs::read_to_string(&times_file){
+            Ok(s) => s,
+            Err(_) => return Vec::new(),
+        };
+
+        return times.lines().filter_map(|t| t.trim().parse::<f64>().ok()).collect();


load_times silently returns an empty vector if times.txt can’t be read. That makes a missing/incorrect dataset root look like it worked and then falls back to synthetic timestamps. Since KITTI odometry sequences normally include times.txt, consider returning Result<Vec<f64>, DatasetError> and propagating the I/O/parse error from open() (and/or validating that times.txt exists up front).

Suggested change

fn load_times(root: &Path) -> Vec<f64>{

let times_file = root.join("times.txt");

let times = match fs::read_to_string(&times_file){

Ok(s) => s,

Err(_) => return Vec::new(),

};

return times.lines().filter_map(|t| t.trim().parse::<f64>().ok()).collect();

fn load_times(root: &Path) -> Result<Vec<f64>, DatasetError>{

let times_file = root.join("times.txt");

if !times_file.exists() {

return Err(DatasetError::FileNotFound(times_file));

}

let times = fs::read_to_string(&times_file)?;

let mut parsed_times = Vec::new();

for (line_idx, line) in times.lines().enumerate() {

let trimmed = line.trim();

if trimmed.is_empty() {

continue;

}

let timestamp = trimmed.parse::<f64>().map_err(|_| {

DatasetError::Parse(format!(

"Malformed timestamp at line {} in {}",

line_idx + 1,

times_file.display()

))

})?;

parsed_times.push(timestamp);

}

Ok(parsed_times)

Copilot · 2026-04-13T12:08:07Z

+impl KittiDataset{
+    // pub fn open(root: impl AsRef<Path>) -> Result<Self, DatasetError> 
+    pub fn open(root: impl AsRef<Path>) -> Result<Self, DatasetError>{
+
+         println!("Kitti dataset open method called");
+        let root = root.as_ref().to_path_buf();
+
+        // let seq_dir = if root.join("image_0").is_dir() {
+        //     root.clone()
+        // }
+
+
+        let image_dir = root.join("image_0");
+
+        if !image_dir.is_dir(){
+            return Err(DatasetError::FileNotFound(image_dir));
+        }
+
+
+        // load the camera
+        let camera = Self::load_camera_from_calib(&root)?;
+        // load timestamps
+        let timestamp = Self::load_times(&root);


EurocDataset has unit tests for calibration parsing and failure cases (see examples/common/datasets/euroc.rs), but KittiDataset introduces similar parsing logic (calib.txt, times.txt, image discovery) without any tests. Please add basic tests (e.g., minimal temp directory tree with calib.txt, times.txt, and a couple of image_0/*.png files) to cover success + missing/malformed calibration/times cases.

jeffin07 · 2026-04-13T16:49:17Z

@cjpurackal I used 01 Folder from kitti.

jeffin07 · 2026-05-06T17:43:36Z

@cjpurackal Fixed comments. Please review

cjpurackal · 2026-06-04T20:53:38Z

tests failing ^^ @jeffin07

cjpurackal reviewed Apr 13, 2026

View reviewed changes

cjpurackal requested a review from Copilot April 13, 2026 12:03

Copilot started reviewing on behalf of cjpurackal April 13, 2026 12:04 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

jeffin07 marked this pull request as ready for review May 12, 2026 15:02

jeffin07 force-pushed the kitti_dataset branch from 6587722 to d321797 Compare May 15, 2026 17:51

jeffin07 added 11 commits June 4, 2026 20:33

feat: support for kitti dataset

cd9d292

kitti dataset support added but tracking is failing

f70b551

tracking updated but need improvement

b8752d1

Kitti specific config for tracking improvement

96730c4

moved config to each dataset files

3aadff7

refactor: unwanted functions from config

c60f54b

review comments fix 1

7194fc1

moved DatasetError and DatasetSample into types.rs

7dd0047

Added tests and review comment fix

0103e06

rebased with develop

4a93fce

rebase and kittisource

77aefe6

jeffin07 force-pushed the kitti_dataset branch from d321797 to 77aefe6 Compare June 4, 2026 16:09

	println!("Euroc dataset unsupported");
	println!("EuRoC dataset");

	use super::euroc::{DatasetError, DatasetSample, GroundTruthPose};
	use super::euroc::{DatasetError, DatasetSample};

	println!("Kitti dataset open method called");
	eprintln!("Kitti dataset open method called");

-    fn load_times(root: &Path) -> Vec<f64>{
-        let times_file = root.join("times.txt");
-        let times = match fs::read_to_string(&times_file){
-            Ok(s) => s,
-            Err(_) => return Vec::new(),
-        };
-        return times.lines().filter_map(|t| t.trim().parse::<f64>().ok()).collect();
+    fn load_times(root: &Path) -> Result<Vec<f64>, DatasetError>{
+        let times_file = root.join("times.txt");
+        if !times_file.exists() {
+            return Err(DatasetError::FileNotFound(times_file));
+        }
+        let times = fs::read_to_string(&times_file)?;
+        let mut parsed_times = Vec::new();
+        for (line_idx, line) in times.lines().enumerate() {
+            let trimmed = line.trim();
+            if trimmed.is_empty() {
+                continue;
+            }
+            let timestamp = trimmed.parse::<f64>().map_err(|_| {
+                DatasetError::Parse(format!(
+                    "Malformed timestamp at line {} in {}",
+                    line_idx + 1,
+                    times_file.display()
+                ))
+            })?;
+            parsed_times.push(timestamp);
+        }
+        Ok(parsed_times)

Uh oh!

Conversation

jeffin07 commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjpurackal commented Apr 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

jeffin07 commented Apr 13, 2026

Uh oh!

jeffin07 commented May 6, 2026

Uh oh!

cjpurackal commented Jun 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

jeffin07 commented Apr 9, 2026 •

edited

Loading