Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Image Transport (Idea) #21

Closed
emrekuru97 opened this issue Feb 6, 2024 · 9 comments
Closed

Support for Image Transport (Idea) #21

emrekuru97 opened this issue Feb 6, 2024 · 9 comments

Comments

@emrekuru97
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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:

  • compressed_image_transport is mostly covered by reusing compressed data when available and doing the rest on the fly
  • compressed_depth_image_transport would only apply when a raw float format would become available since 16bit discrete values would already be covered by the standard compression plugin
  • theora_image_transport: the Theora format isn't widely used in the ROS ecosystem as far as I can see and using the raw MJPEG stream from the camera would be a better choice anyway to remove the decode/encode overhead

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
Copy link
Contributor Author

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

@emrekuru97
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Owner

Fixed via #23.

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

No branches or pull requests

2 participants