search

pal-robotics / realsense_gazebo_plugin

157 stars 134 forks source link

set 'png' compression format for depth images #28

Closed christian-rauch closed 3 years ago

christian-rauch commented 3 years ago

This sets lossless png compression for depth images by default, instead of the lossy jpeg compression which introduces compression artefacts.

Fixes https://github.com/pal-robotics/realsense_gazebo_plugin/issues/27

saikishor commented 3 years ago

@christian-rauch Thanks for the PR, but this seems to be a parameter of compressed_depth_image_transport package, but not of this plugin.

christian-rauch commented 3 years ago

Yes. In the end, this is just a parameter. Bu having selected 'jpeg' as the default format creates compression artefacts (since it is a lossy compression) and hence corrupts the depth data.

saikishor commented 3 years ago

I believe it should be set by the launch file that is launching the nodelet, but not the plugin that has nothing to do with the compressed_depth_image_transport

christian-rauch commented 3 years ago

The parameter can indeed be set in multiple ways. But having it set to a sensible default value inside the plugin has the advantage that it will always give correct information. If you leave this to the user, he/she has to track down the topic name first and format the parameter correctly.

I don't see a good reason why someone would want keep compression artefacts in depth data by default.

saikishor commented 3 years ago

@christian-rauch Ok I am fine to merge it. Could you add some explanation with comments about the reasons it was added, it is helpful for future references

christian-rauch commented 3 years ago

Ok, I added two paragraphs to the commit message to explain why changing the default compression format makes sense for depth images.

saikishor commented 3 years ago

@christian-rauch I couldn't see the changes in the PR. No need of such a long explanation, simply mention that these lines are added as to fix the issue you mentioned above and this helps to set the default functional parameters of the compressed_depth_image_transport

christian-rauch commented 3 years ago

I couldn't see the changes in the PR

It is in this commit: https://github.com/pal-robotics/realsense_gazebo_plugin/pull/28/commits/e42fbafc747009540f86fce1d2d539a836d5ac91. I just force-pushed the changed commit.

default functional parameters of the compressed_depth_image_transport

Technically, it is the compressed_image_transport which is usually responsible for the colour image transport, where jpeg compression makes sense. However, this transport can also be used for 16 bit depth images (16UC1), where jpeg compression makes no sense.

On top of that, there is compressed_depth_image_transport which is specifically for depth data, uses a custom compression format and publishes to compressedDepth.