Skip to content

Conversation

@svepe
Copy link

@svepe svepe commented May 3, 2016

In order to use standard ROS image tools with the published images they should have the correct ROS encoding (not OpenCV). For the full list of definitions see here.

In order to use standard ROS image tools with the published images they should have the correct ROS encoding (not OpenCV). For the full list of definitions see [[here | http://docs.ros.org/jade/api/sensor_msgs/html/image__encodings_8h_source.html]]
@kohrt
Copy link

kohrt commented May 4, 2016

Hi,
thanks for the PR. I don't know why there are different types for uint16 in ROS, if there where just different names for the same type, I wouldn't care.
But changing to MONO16 isn't a good idea. Rviz works fine the way it is now, and for example image_proc checks for the CV types, see here.
The correct solution would be, to change sensor_msgs, so that both names result in the same string, for example const std::string MONO16="16UC1"; const std::string TYPE_16UC1="16UC1";.

@svepe
Copy link
Author

svepe commented May 4, 2016

The depth_image_proc checks for the OpenCV types which is fine for depth images, however the color images and the mono images should be with ROS types. Important tools such as cv_bridge assume that ROS types are used for a sensor_msgs::Image (see here).

@kohrt
Copy link

kohrt commented May 4, 2016

But it also checks the other ones, if you look further down.
Where does it result in problems, or which packages don't work, if the encoding stay like it is now?

@svepe
Copy link
Author

svepe commented May 5, 2016

I did not see those. The reason I did the pull request is that something from the package calibration_launch complains that it requires mono8.

@kohrt
Copy link

kohrt commented May 5, 2016

@mpatalberta
Copy link

How do you plan to handle intensity and rgb. The intensity IS NOT the color but is the strength of the return. PCL DOES NOT allow both intensity and rgb.

@kohrt
Copy link

kohrt commented Feb 28, 2017

@mpatalberta what do you mean? You can have multiple channels stored in a point.

@bbferka bbferka closed this Mar 20, 2018
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

Successfully merging this pull request may close these issues.

4 participants