JPEG only supports 8 bits images

[ivanpauno]

JPEG only supports 8bit images.

If you currently use a 16 bit image with jpeg compressed image transport and use rqt_image_view to try to plot it you get the following errors:

• when publishing MONO16

  ImageView.callback_image() while trying to convert image from 'mono16' to 'rgb8' an exception was thrown (Image is wrongly 
  formed: step < width * byte_depth * num_channels  or  2048 != 2048 * 2 * 1)
  

• when publishing RGB16

  ImageView.callback_image() while trying to convert image from 'rgb16' to 'rgb8' an exception was thrown (Image is wrongly 
  formed: step < width * byte_depth * num_channels  or  1920 != 640 * 2 * 3)
  

and the publisher side doesn't fail.

After this patch, the publisher side fails showing the following message:

[gzserver-1] [ERROR] [1624039438.516390848] [CompressedPublisher]: Compressed Image Transport - JPEG compression requires 8 bit color format (input format is: rgb16)

[ivanpauno]

To make the Gpr and Rpr checkers pass, #72 is needed.

[mallanmba]

Instead of failing on publisher side, what do you think about simply converting all image types to bgr8?
I attempted adding a conversion to mono8 if a mono16 image was detected, but that did nothing (I got the same error on the receiving side) but simply commenting out this line...
https://github.com/ros-perception/image_transport_plugins/blob/noetic-devel/compressed_image_transport/src/compressed_publisher.cpp#L123
...permits any image type to be sent as JPEG compressed bgr8 which seems a little more user friendly.

[jacobperron]

@mallanmba's suggestion seems reasonable to me.

[ivanpauno]

https://github.com/ros-perception/image_transport_plugins/blob/noetic-devel/compressed_image_transport/src/compressed_publisher.cpp#L123
...permits any image type to be sent as JPEG compressed bgr8 which seems a little more user friendly.

I'm pretty sure I tried that (publishing a rgb16 image that was converted to bgr8), and it was also not working.
Though, it should be possible to make that work and it sounds more reasonable than this approach, so I will give it a try.

[ivanpauno]

Depends on:

• [ros2] Fix RCLCPP_* logs, a string literal must be the first argument #72
• Fix SimpleSubscriberPlugin image_common#195 (subscribeImpl() was not being called)

Now mono16 images are being published as mono8, simular to what was suggested here.
The subscription side is decoding the compressed image and converting it back to mono16 (for consistency to what the compressed subscriber is doing in other cases, e.g. a rgb8 image is published as bgr8 and being converted back to rgb8 after decoding).

This PR is also:

• Avoiding a ParameterAlreadyDeclaredException, similar to Declare parameters in topic namespace #58.
• Overriding the subscribeImpl() override that takes a SubscriptionOptions argument as well.

There are still things that I find weird, but I didn't want to modify:

• The subscription side is converting the image back to the original format.
  I don't think that makes much sense, it would be IMO ok if the original image is rgb16 and the received image is bgr8 as sensor_msgs/msg/Image is self descriptive.
• I don't think that the information we're writing after the ";" in the format field is strictly needed, I think imdecode used correctly can figure that out already.

[ivanpauno]

Fpr and Gpr are expected to fail, as this branch isn't intended to be compatible with them (there's a galactic and a foxy branch).
We need to merge ros-perception/image_common#195 and release image_common to rolling to make the Rpr job pass.

[ivanpauno]

@jacobperron could you take another look?

[ivanpauno]

Pinging @mjcarroll as a maintainer.

[mjcarroll]

@ros-pull-request-builder retest this please

[ivanpauno]

The Fpr and Gpr failures are expected, because this is overriding methods added after ros-perception/image_common#195 and ros-perception/image_common#186.

The Rpr checker was passing before, the error in the last run seems to be random.

This PR seems to overlap with #78.

[clalancette]

@ros-pull-request-builder retest this please

[ivanpauno]

@mjcarroll Should we modify the release repository to use the foxy-devel branch for galactic?
https://github.com/ros2-gbp/image_transport_plugins-release/blob/4cb6c5dc902f85c8ab79a6b13677ae4f9935433c/tracks.yaml#L114

Same in rosdistro https://github.com/ros/rosdistro/blob/ec8dd166b57d3667c9353c002691bae9ddc7fa6e/galactic/distribution.yaml#L1053.

I have checked both foxy-devel and the ros2 branch, and the only difference between the two would be this commit.

[mjcarroll]

@mjcarroll Should we modify the release repository to use the foxy-devel branch for galactic?

Yes, I think so, I can take care of it.

[mjcarroll]

Fpr and Gpr checkers can be ignored since Fpr and Gpr should now be using the foxy-devel branch because of:

• [galactic] Update image_transport_plugins to use foxy-devel ros/rosdistro#30215 (galactic)
• Update image_transport_plugins dev branches ros/rosdistro#30185 (foxy)

tracks.yaml has also been updated accordingly.

[mjcarroll]

@ros-pull-request-builder retest this please