-
Notifications
You must be signed in to change notification settings - Fork 7
Fix: Passing sensor_t into sensor_from_rig #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
lamaria/structs/trajectory.py
Outdated
| # left camera poses are provided | ||
| # sensor_from_rig == cam0_from_imu | ||
| transform = rig.sensor_from_rig(sensor_id=2) | ||
| left_camera = reconstruction.cameras[2] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 2? Try to avoid magic numbers or at least document them properly using a variable with unambiguous name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updating the naming for better clarity, the 2 for left camera comes from this function here where we build an initial reconstruction from a device calibration (for evaluation of each sequence).
Lines 31 to 74 in 6e45892
| def initialize_reconstruction_from_calibration_file( | |
| calibration_file: Path, | |
| ) -> InitReconstruction: | |
| """Initialize a COLMAP reconstruction from Aria calibration | |
| json file found on website: https://lamaria.ethz.ch/slam_datasets | |
| Adds a dummy camera as an IMU along with two cameras. | |
| Args: | |
| calibration_file (Path): | |
| Path to the Aria calibration json file | |
| Returns: | |
| InitReconstruction: The initialized COLMAP reconstruction | |
| """ | |
| reconstruction = pycolmap.Reconstruction() | |
| imu = pycolmap.Camera(camera_id=1, model="SIMPLE_PINHOLE", params=[0, 0, 0]) | |
| reconstruction.add_camera(imu) | |
| rig = pycolmap.Rig(rig_id=1) | |
| ref_sensor = pycolmap.sensor_t( | |
| id=1, # imu is the rig | |
| type=pycolmap.SensorType.CAMERA, | |
| ) | |
| rig.add_ref_sensor(ref_sensor) | |
| for i, (key, _) in enumerate(ARIA_CAMERAS): | |
| cam = camera_colmap_from_json( | |
| calibration_file=calibration_file, | |
| camera_label=key, | |
| ) | |
| cam.camera_id = i + 2 # start from 2 since 1 is imu | |
| reconstruction.add_camera(cam) | |
| sensor = pycolmap.sensor_t(id=i + 2, type=pycolmap.SensorType.CAMERA) | |
| rig_from_sensor = get_t_imu_camera_from_calibration_file( | |
| calibration_file=calibration_file, | |
| camera_label=key, | |
| ) | |
| sensor_from_rig = rig_from_sensor.inverse() | |
| rig.add_sensor(sensor, sensor_from_rig) | |
| reconstruction.add_rig(rig) | |
| return reconstruction |
lamaria/structs/trajectory.py
Outdated
| # left camera poses are provided | ||
| # sensor_from_rig == cam0_from_imu | ||
| left_camera = reconstruction.cameras[2] | ||
| left_camera_id = 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This information is duplicated with a few lines below
[
(LEFT_CAMERA_STREAM_LABEL, 2),
(RIGHT_CAMERA_STREAM_LABEL, 3),
]| ) -> pycolmap.Reconstruction: | ||
| pose_data = self.as_tuples() | ||
|
|
||
| image_id = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't work if the reconstruction already has images, cameras, or rigs, should we assert this? or make this code more generic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a hidden function that we don't want to expose. We only use it to add estimate poses to an already initialized reconstruction:
lamaria/lamaria/structs/trajectory.py
Lines 78 to 91 in c3d467b
| def add_estimate_poses_to_reconstruction( | |
| self, | |
| reconstruction: pycolmap.Reconstruction, | |
| timestamp_to_images: dict, | |
| ) -> pycolmap.Reconstruction: | |
| """ | |
| Adds estimate poses as frames to input reconstruction. | |
| """ | |
| self._ensure_loaded() | |
| reconstruction = self._add_images_to_reconstruction( | |
| reconstruction, timestamp_to_images | |
| ) | |
| return reconstruction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whereas the more general function is to initialize the reconstruction with a calibration file (pinhole/Aria camera):
Line 31 in c3d467b
| def initialize_reconstruction_from_calibration_file( |
No description provided.