search

ros / common_msgs

Commonly used messages in ROS. Includes messages for actions (actionlib_msgs), diagnostics (diagnostic_msgs), geometric primitives (geometry_msgs), robot navigation (nav_msgs), and common sensors (sensor_msgs), such as laser range finders, cameras, point clouds.
http://wiki.ros.org/common_msgs
179 stars 191 forks source link

[sensor_msgs] Return true in isMono for 8UC and 16UC #107

Closed wkentaro closed 7 years ago

wkentaro commented 7 years ago

This is needed in conversion from 8UC1 image msg to mono8 image. Requested by https://github.com/ros-perception/vision_opencv/issues/175

tfoote commented 7 years ago

This seems reasonable. Is there any specific meaning to mono, besides that it's a single channel? In which case I would expect all encodings with a single channel to return true. Not just the 8 and 16 bit ones.

wkentaro commented 7 years ago

I think you're right. But in that case this PR is not required by cv_bridge, because it expects isMono only returns true for 8 and 16 bit images.

tfoote commented 7 years ago

It looks like https://github.com/ros-perception/vision_opencv/pull/180 resolves https://github.com/ros-perception/vision_opencv/issues/175 without this change.

And if it's really about checking the number of channels numChannels() == 1 should suffice.

@vrabaud Do you have any background on the specific mono designation?

wkentaro commented 7 years ago

ros-perception/vision_opencv#180 needs this change implicitly because the line changed there is after isMono function. But that problem can be resolved in cv_bridge with a little change, and now I think it is better that I close this PR and fix that issue inside of cv_bridge.

wkentaro commented 7 years ago

https://github.com/ros-perception/vision_opencv/pull/180/commits/c849056ce1de2e7cabab1b82c65c276f6a1def14 will fix this issue.

vrabaud commented 7 years ago

ok, late here: mono is for grayscale images, images that contain color information and only have one channel. A depth image only has one channel but is not mono (as it does not contain intensity/color information but depth). Mono is for monochromatic, not for mono-channel (which is not English anyway), as used at the bottom of that page for example: http://wiki.ros.org/image_proc/common

That's also why MONO8 is defined in the same section as BGR8 and so on.

If we we allow this PR, it would be the same as saying that a 3-channel image (TYPE_8UC3) is the same as BGR8.

So I would reject this PR.

tfoote commented 7 years ago

Thanks @vrabaud I thought there was a semantic difference. Thanks for confirming. I'll close this. I think there can be other workarounds for the upstream issue such as providing a conversion function or updating the data source to declare mono8 to be more correct.