Skip to content

Conversation

@mvieth
Copy link

@mvieth mvieth commented Sep 23, 2019

This PR adds functionality to compress images of type 16UC1 and 32FC1, which are common formats for depth images. These types cannot be compressed with jpeg, so png respectively tiff are used.
This PR also adds some debug output.

Copy link
Contributor

@calderpg calderpg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor changes, mostly to keep style of spacing and brackets consistent.

Please consider if other image encodings should handled the same way, for example, mono16 is equivalent to 16UC1, so perhaps both should be handled

sensor_msgs::Image decompressed;
cv::Mat decoded = cv::imdecode(compressed.data, CV_LOAD_IMAGE_ANYCOLOR);
cv::Mat decoded = cv::imdecode(compressed.data, CV_LOAD_IMAGE_UNCHANGED);
if(decoded.data==NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For style consistency, change to:

if (decoded.data == NULL)
{

cv::Mat decoded = cv::imdecode(compressed.data, CV_LOAD_IMAGE_UNCHANGED);
if(decoded.data==NULL) {
ROS_WARN("Decoding unsuccessful");
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
else
{

cv::Mat decoded = cv::imdecode(compressed.data, CV_LOAD_IMAGE_ANYCOLOR);
cv::Mat decoded = cv::imdecode(compressed.data, CV_LOAD_IMAGE_UNCHANGED);
if(decoded.data==NULL) {
ROS_WARN("Decoding unsuccessful");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since CV->ROS conversion of an image that couldn't be decoded isn't a good idea, this should throw, such as:

throw std::runtime_error("OpenCV image decoding failed");

encoding_params.push_back(CV_IMWRITE_JPEG_QUALITY);
encoding_params.push_back(quality);
bool ret = cv::imencode(".jpg", cv_image, compressed.data, encoding_params);
bool ret;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should initialize the value, i.e.

bool ret = true;

ROS_DEBUG("Encoding in tiff format");
ret = cv::imencode(".tiff", cv_image, compressed.data, encoding_params);
// jpeg and png are the only acceptable values for the format field, so it is not set here
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
else
{

encoding_params.push_back(CV_IMWRITE_JPEG_QUALITY);
encoding_params.push_back(quality);
ret = cv::imencode(".jpg", cv_image, compressed.data, encoding_params);
compressed.format="jpeg";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compressed.format = "jpeg";

if(image.encoding=="16UC1") {
ROS_DEBUG("Encoding in png format");
ret = cv::imencode(".png", cv_image, compressed.data, encoding_params);
compressed.format="png";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compressed.format = "png";

bool ret = cv::imencode(".jpg", cv_image, compressed.data, encoding_params);
bool ret;
ROS_DEBUG_STREAM("Image encoding: " << image.encoding);
if(image.encoding=="16UC1") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (image.encoding == "16UC1")
{

ROS_DEBUG("Encoding in png format");
ret = cv::imencode(".png", cv_image, compressed.data, encoding_params);
compressed.format="png";
} else if(image.encoding=="32FC1") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
else if (image.encoding == "32FC1")
{

} else if(image.encoding=="32FC1") {
ROS_DEBUG("Encoding in tiff format");
ret = cv::imencode(".tiff", cv_image, compressed.data, encoding_params);
// jpeg and png are the only acceptable values for the format field, so it is not set here
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would set compressed.format = "tiff"; here, so that a user looking at raw messages knows what encoding is being used. TIFF-encoded images aren't handled by image_transport already, so setting the format type won't cause any new issues.

@mvieth
Copy link
Author

mvieth commented Sep 24, 2019

I made the requested changes. I initialized ret with false, since that makes IMO more sense.
Regarding the other image encodings: yes, probably other encodings should be handled by png and tiff (mono16, and 16U and 32F with more than one channel), but I could not test them, so I left them to the default case jpeg.

@mvieth mvieth requested a review from calderpg September 24, 2019 08:08
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

Successfully merging this pull request may close these issues.

2 participants