search

ros-drivers / video_stream_opencv

A package to open video streams and publish them in ROS using the opencv videocapture mechanism
227 stars 159 forks source link

Allow setting of pixel format (bgr8 and rgb8 are supported) #47

Closed J-Rojas closed 5 years ago

J-Rojas commented 5 years ago

Partial Fix for #46. Support bgr8 and rgb8 formats. The format can be selecting using the "output_pixel_encoding" parameter. The capture node will convert from BGR to RGB format before enqueuing the frame.

awesomebytes commented 5 years ago

Looks good to me, what do you reckon @furushchev

furushchev commented 5 years ago

@awesomebytes I will test it tomorrow :+1:

furushchev commented 5 years ago

@J-Rojas Thank you for sending the PR. I understood what you want to do. However, it limits the format to only bgr8 and rgb8 regardless of many formats supported in OpenCV. I basically suggest to convert color space in subscriber nodes since OpenCV natively treat image data as bgr8 by default and there is no difference in performance whether color space conversion is in this node or subscribers.

Converting to RGB8 color space in subscriber nodes can be done very easily by using cv_bridge package, where you only need to set encoding to "bgr8" when you get cv::Mat from image message via toCvCopy or toCvShare. Please see the tutorial: http://wiki.ros.org/cv_bridge/Tutorials/UsingCvBridgeToConvertBetweenROSImagesAndOpenCVImages#cv_bridge.2BAC8-Tutorials.2BAC8-UsingCvBridgeCppHydro.CA-51364fcf5e26ba1de20c6a29f5814a956784abf9_39

Hope it helps you with solving your issue.

J-Rojas commented 5 years ago

Thanks for reviewing. This library is meant to assist people who are converting movie files to image streams. I decided it would be very simple to run this node and convert the images. I didn't want to create yet another node to handle conversions. Ideally the output format is configurable to add this convenience to users of the library so they do not need to do additional work for conversion.

furushchev commented 5 years ago

@J-Rojas OK, I created another PR to support other than "bgr8" and "rgb8" #50. Please check if it is ok.

J-Rojas commented 5 years ago

Closing, this is addressed with PR#50.

https://github.com/ros-drivers/video_stream_opencv/pull/50