search

ros-industrial / industrial_reconstruction

Tools for surface reconstruction from 2D depth images
Apache License 2.0
48 stars 17 forks source link

Should CameraInfo msg be synced with depth and RGB? #3

Open gavanderhoorn opened 1 year ago

gavanderhoorn commented 1 year ago

The current implementation appears to use a separate subscriber for CameraInfo msgs:

https://github.com/ros-industrial/industrial_reconstruction/blob/2cf137d2941be6425bc4908e70c0cc1a2fc34c63/industrial_reconstruction/industrial_reconstruction/industrial_reconstruction.py#L131

incoming msgs are stored in a member variable in the callback, with a keep-last policy:

https://github.com/ros-industrial/industrial_reconstruction/blob/2cf137d2941be6425bc4908e70c0cc1a2fc34c63/industrial_reconstruction/industrial_reconstruction/industrial_reconstruction.py#L350-L351

If the assumption is those msgs are all identical, this can work, but technically, it's legal for ROS camera drivers to publish new CameraInfo msgs for every image they publish. Synchronisation would happen based on the header.stamp in both. This allows for settings to change between captures, and consuming nodes to be aware of such changes.

It looks like this should not be too difficult to implement, as message_filters is already used to sync depth and RGB streams. Adding CameraInfo would just require an additional message_filters.Subscriber instance.

gavanderhoorn commented 1 year ago

technically, it's legal for ROS camera drivers to publish new CameraInfo msgs for every image they publish

actually, it'd also be legal for drivers to publish different CameraInfo messages for depth, RGB and other images they publish. A separate CameraInfo topic for each topic carrying Images. That would complicate things a bit more.

It'd be OK if that would not be supported, but I just wanted to point it out.

marrts commented 1 year ago

I like the idea of syncing CameraInfo with the Images based off what you said and also this could also lead to supporting multiple cameras simultaneously.

I also think the TSDF algorithm relies on both depth and color images having the same intrinsic parameters, so a single CameraInfo per camera would be necessary.