Skip to content
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

Published camera_info messages not compatible with image_geometry::StereoCameraModel #872

Closed
tykurtz opened this issue Jul 31, 2019 · 4 comments
Assignees

Comments

@tykurtz
Copy link

tykurtz commented Jul 31, 2019

The frame_id of the camera_info message published on /camera/infra2/camera_info is camera_infra2_optical_frame. This is not compatible with the image_geometry::StereoCameraModel class as StereoCameraModel::fromCameraInfo() makes the assumption that the left and right camera_info messages are in the same frame. So modifying the second camera to use the first camera frame would be more in line with ROS convention.

Without digging into the realsense-ros code, my hacky fix was to subscribe, modify the frame and projection matrix, and republish the modified camera_info as below, but an upstream fix would be preferred if posible.

    def on_camera_info_received(self, camera_info):
        modified_camera_info = camera_info
        modified_camera_info.header.frame_id = 'camera_infra1_optical_frame'
        projection_matrix = list(modified_camera_info.P)
        projection_matrix[3] = projection_matrix[0] * .0499728  # fx * baseline
        modified_camera_info.P = projection_matrix

        self.right_camera_info_publisher.publish(modified_camera_info)
@doronhi
Copy link
Contributor

doronhi commented Aug 6, 2019

I was not aware of that convention. It means that there shouldn't be published a static transform for infra2 nor a topic named /camera/extrinsics/depth_to_infra2
Basically it means that infra2 should not be treated as a separate sensor.
It means an interface change.
I realize the logic in this but could you refer me to the official convention doc?

@stwirth
Copy link
Contributor

stwirth commented Aug 6, 2019

That spec can be found in the comment of the CameraInfo message definition:
http://docs.ros.org/kinetic/api/sensor_msgs/html/msg/CameraInfo.html

@tykurtz
Copy link
Author

tykurtz commented Aug 6, 2019

Additionally to @stwirth 's comment, for me the convention is also being compatible with stereo_imag_proc. The expectation I have is that with the appropriate topic remapping, one could run stereo_image_proc.launch by streaming monochrome images + camera info without any hiccups. Currently, you are going to get asserts failed in the point cloud nodelet and in the disparity nodelet due to the frame mismatch. I run into this issue in particular as I'm benchmarking different methods of disparity estimation to improve upon the on-board 3D reconstruction and wanted to reuse the point_cloud2 nodelet.

It means that there shouldn't be published a static transform for infra2 nor a topic named /camera/extrinsics/depth_to_infra2
Basically it means that infra2 should not be treated as a separate sensor.
It means an interface change.

It would still be a separate and proper sensor, just with the optical center now located in infra1's frame. But I do see your point about no longer being able to treat infra2 as an isolated monocular camera. If someone incorrectly assumed you could just use the frame_id and the intrinsic matrix from infra2's camera_info, it would be incorrect.The consistency here is that this is true for all ROS stereo cameras.

@RealSenseCustomerSupport
Copy link
Collaborator


Hi @tykurtz,

Did the fix #901 solve the issue? Please let us know if you need any further assistance with this matter.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants