Fix raw_mjpeg function and shrink the compressed image msg size #336
Conversation
| inline void reset_image_size_in_bytes() | ||
| { | ||
| m_image.set_size_in_bytes(); | ||
| } | ||
|
|
There was a problem hiding this comment.
@hilookas this method should be added to the image_t structure, where inside it should set either the height or bytes_per_line variable to 0 and call set_size_in_bytes().
| @@ -171,7 +171,7 @@ void UsbCamNode::init() | |||
|
|
|||
| // if pixel format is equal to 'mjpeg', i.e. raw mjpeg stream, initialize compressed image message | |||
| // and publisher | |||
| if (m_parameters.pixel_format_name == "mjpeg") { | |||
| if (m_parameters.pixel_format_name == "raw_mjpeg") { | |||
There was a problem hiding this comment.
@hilookas We should support both mjpeg and raw_mjpeg for compressed image streams I think, no?
| if (m_parameters.pixel_format_name == "raw_mjpeg") { | |
| auto pixel_fmt = m_parameters.pixel_format_name; | |
| if (pixel_fmt == "mjpeg" || pixel_fmt == "raw_mjpeg" ) { |
| @@ -391,6 +391,8 @@ bool UsbCamNode::take_and_send_image_mjpeg() | |||
|
|
|||
| // grab the image, pass image msg buffer to fill | |||
| m_camera->get_image(reinterpret_cast<char *>(&m_compressed_img_msg->data[0])); | |||
| m_compressed_img_msg->data.resize(m_camera->get_image_size_in_bytes()); | |||
| m_camera->reset_image_size_in_bytes(); // reset for next grab | |||
There was a problem hiding this comment.
@hilookas I dont think this does what you think it does 😅 the set_size_in_bytes() method (called inside the reset_image_size_in_bytes method you added above, just changes the image_t structure size to whatever the height and bytes_per_line values are set to. So if those two values don't change at all, this function wouldn't change anything / reset it to anything.
| @@ -423,7 +425,7 @@ void UsbCamNode::update() | |||
| // If the camera exposure longer higher than the framerate period | |||
| // then that caps the framerate. | |||
| // auto t0 = now(); | |||
| bool isSuccessful = (m_parameters.pixel_format_name == "mjpeg") ? | |||
| bool isSuccessful = (m_parameters.pixel_format_name == "raw_mjpeg") ? | |||
There was a problem hiding this comment.
@hilookas should probably support both mjpeg and raw_mjpeg, right?
There was a problem hiding this comment.
To my understand, mode mjpeg and mode raw_mjpeg is different.
mjpeg mode should decode the mjpeg image from driver, and send it to m_image topic, which will be compressed again to /compressed by image_transport
raw_mjpeg should send jpeg encoded image from driver and send it directly to /compressed, m_image topic should keep empty preventing cpu-consuming jpeg decoding. This mode should be optimized to send compressed image only.
There was a problem hiding this comment.
mjpeg mode should decode the mjpeg image from driver, and send it to m_image topic, which will be compressed again to /compressed by image_transport
And infact there is no "mjpeg" mode right now for ros2
| @@ -82,7 +82,8 @@ void UsbCam::process_image(const char * src, char * & dest, const int & bytes_us | |||
| // TODO(flynneva): could we skip the copy here somehow? | |||
| // If no conversion required, just copy the image from V4L2 buffer | |||
| if (m_image.pixel_format->requires_conversion() == false) { | |||
| memcpy(dest, src, m_image.size_in_bytes); | |||
| m_image.size_in_bytes = bytes_used; | |||
There was a problem hiding this comment.
@hilookas why are you setting size_in_bytes directly here? that value should be set well before we get to this function. Also the memcpy line just below this one doesnt "know" about the size_in_bytes setting here at all for the image_t struct.
There was a problem hiding this comment.
That's because the length of the mjpeg encoded image fetched from v4l2 driver is changed between different frames.
We need record current length of the frame, and during the topic sending process, we need use the corrent length of the frame. Incorrect length will cause the memory leaking issue.
Hence, memcpy should use bytes_used (which is from v4l2 driver) rather than m_image.size_in_bytes
raw_mjpegmode is broken.This pr fix this mode, and reduce the compressed image msg size by truncating the tailing zeros.