christianrauch / camera_ros

ROS 2 node for libcamera supported cameras (V4L2, Raspberry Pi Camera Modules)
https://libcamera.org
MIT License
69 stars 30 forks source link

Support for Image Transport (Idea) #21

Closed emrekuru97 closed 9 months ago

emrekuru97 commented 9 months ago

Hello, Why aren't the advantages provided by the image_transport used when broadcasting images to the system? Is there a special reason for this? Or would it be useful to implement this when brodcarstin images?

christianrauch commented 9 months ago

Are you referring to the compressed_image_transport package? I did not use the automatic de/-compression to have better control over the image data conversion.

For example, some cameras directly offer a MJPEG stream from which the JPEG data can directly be extracted and used in the sensor_msgs/CompressedImage. Using compressed_image_transport would require to decompress the JPEG data to raw image data and then compressing it again via the transport plugin. If you only need a CompressedImage and your camera already provides a mode for a compressed video stream, then reusing the raw data is much more efficient than converting it.

Overall, I tried to avoid image data conversion by providing the raw datastream when possible and only convert data when this is requested by a topic subscription.

emrekuru97 commented 9 months ago

Hello again,

I'm reffering to use from image_common the image_transport::CameraPublisher which will probably use compressed_image_transport and other plugins too for managing the diffenrent type (compressed, theora, raw) connections. But the image_transport::CameraPublisher needs raw data too for doing the work, so i think the reason You have mentoined is very suitable for cameras with direct MJEPEG output. But even then can we only implement image_transport::CameraPublisher when the output data from the camera is RAW i mean only in the first if clause. So we can make use of the compression parameters and other options available in image_common and transmit data if needed with a lower bandwith. Since currently cv_bridge:: has no direct option for compression rate sometimes the bw of the compressed image can be high. Thanks for yor detailed answer.

if (format_type(cfg.pixelFormat) == FormatType::RAW) 
{
  //implement image_transport::CameraPublisher 
  // without any conversions directly, image_transport::CameraPublisher will do automatic de/-compression if needed
}
else if (format_type(cfg.pixelFormat) == FormatType::COMPRESSED) 
{
  // since data id in MJPEG  format directly broadcast to compressed and do  conversion  to raw if needed same as dafult .
  // do not change since we would need a conversion to raw for using `image_transport::CameraPublisher` here.
}
christianrauch commented 9 months ago

What benefits would you see for using the CameraPublisher? Do you want to use some of the plugins from the image_transport_plugins package? Are you missing a specific functionality from the node?

emrekuru97 commented 9 months ago

I see the benefit using the default plugins, like compressed_image_transport. The missed option is compression_quality currently this is not supported by the node but compressed_image_transport supports this. We can add one of these to the node to support compression_quality:

1.CameraPublisher (Will have advantage of the default plugins like compressed_image_transport, theora_image_transport will also publish the raw image. Will do thes things only needed and without wasting computation power) 2.Direct Compressed Image Publisher Plugin (Only compressed image transport, will publish only needed too) 3.Implement compressing using cv::imencode so we can specify a parameter for compression_quality (can be thought as 2)

That was it.

christianrauch commented 9 months ago

If it's just about the interface for the compressed_image_transport, I think it would be easier to just replicate the parameters, i.e. add an option for the compression rate, to provide the same interface.

Currently, the amount of plugins in image_transport_plugins is quite limited:

When more useful transport plugins become available, e.g. for MJPEG or hardware de-/encoded AV1, then I could see the point of using the image_transport::Publisher.

As an intermediate solution until more useful plugins become available, would you be interested in replicating the interface (adding the remaining parameters etc.) and sending a PR for that?

emrekuru97 commented 9 months ago

Sure ı can do this, ı will implement and send a PR. Thank you for the detailed infotmation.

emrekuru97 commented 9 months ago

Hello again, implemented the desired functionality (only when stream is raw). The added codes are very similar to those in compressed_image_transport. This is the PR.

christianrauch commented 9 months ago

Thanks for the PR. You can add the "Fixes" tag to the PR to have this issue automatically closed when the PR is merged.

christianrauch commented 9 months ago

Fixed via https://github.com/christianrauch/camera_ros/pull/23.