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

Depth image encoding is hard-coded #2

Open
gavanderhoorn opened this issue Sep 13, 2022 · 1 comment
Open

Depth image encoding is hard-coded #2

gavanderhoorn opened this issue Sep 13, 2022 · 1 comment

Comments

@gavanderhoorn
Copy link
Member

The current implementation appears to hard-code the expected image encoding of the depth image:

cv2_depth_img = self.bridge.imgmsg_to_cv2(depth_image_msg, "16UC1")

it doesn't do this for the regular RGB images:

cv2_rgb_img = self.bridge.imgmsg_to_cv2(rgb_image_msg, rgb_image_msg.encoding)

Looking at various drivers, there are two main 'types' of depth images:

  • 16-bit unsigned with depth in mm (16UC1)
  • 32-bit float with depth in m (32FC1)

At least in ROS 1, the depth_image_proc/convert_metric nodelet can be used to convert between the two types.

The hard-coded 16UC1 (and the existence of the depth_scale parameter) implies the first is expected and has been used when developing industrial_reconstruction. The code as-is is incompatible with the second type.

Would it be desirable to remove the hard-coding and rely on what is set in sensor_msgs::msgs::Image::encoding field instead?

@marrts
Copy link
Collaborator

marrts commented Sep 13, 2022

Would it be desirable to remove the hard-coding and rely on what is set in sensor_msgs::msgs::Image::encoding field instead?

Yes, this would definitely be preferable. Upon initial creation of this node I had some issues trying to do that, so I just ran with what would work for the initial application. It sounds like you and @aashish-tud have an application using different camera encodings than what I've used in the past, if you guys have a working change for that I would definitely want to merge that in.

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

2 participants