Add a generic `savePNGFile()` function #204

taketwo · 2013-07-28T19:28:03Z

This pull request introduces a new savePNGFile() function as discussed in this thread: http://www.pcl-developers.org/A-generic-savePNGFile-function-td5707976.html.

The function accepts the name of the field which should be used to extract image data. Currently "rgb", "normal", "label", "z", "curvature", and "intensity" fields are supported. The underlying implementation is driven by a set of PointCloudImageExtractorFrom...Field classes which know how to extract data from a particular field (applying additional scaling as necessary).

There are tests for all Image Extractors.

Additionally there is a new application "pcd2png" in the "tools" module which uses the new functionality.

There are three things that should be discussed before merging this pull request:

The new tool "pcd2png" obsoletes "organized_pcd_to_png". Should it be removed?
There exist a version of savePNGFile() with the following signature:

void savePNGFile (const std::string& file_name, const pcl::PointCloud<pcl::PointXYZL>& cloud);

Should it be removed in favor of using savePNGFile("blah.png", cloud, "label")?
3) There exist a templated version of savePNGFile() with the following signature:

template <typename T> void
savePNGFile (const std::string& file_name, const pcl::PointCloud<T>& cloud)

It uses "r", "g", and "b" field of a point cloud (if exist, otherwise it fails to compile, of course). Should it be removed in favor of using savePNGFile("blah.png", cloud, "rgb")?

The new function takes sensor_msgs::Image and saves it as a PNG file. Currently supported are "rgb8", "mono8", and "mono16" image encodings.

* PointCloudImageExtractorFromRGBField produced a color image based on the data in "rgb" field of a point cloud. * PointCloudImageExtractorFromNormalField produced a color image based on the data in "normal" field of a point cloud. * PointCloudImageExtractorFromLabelField produced a mono image based on the data in "label" field of a point cloud. * PointCloudImageExtractorFromZField produces a mono image based on the "z" field of a point cloud. * PointCloudImageExtractorFromCurvatureField produces a mono image based on the "curvature" field of a point cloud. * PointCloudImageExtractorFromIntensityField produces a mono image based on the "intensity" field of a point cloud.

The new function takes a field name and uses appropriate PointCloudImageExtractor subclass to extract an image from the given field. Currently supported are "normal", rgb", "label", "z", "curvature", and "intensity" fields.

chriskilding · 2013-07-31T10:18:29Z

Does this support the full resolution of an OpenNI camera's depth map (IIRC the Kinect has 16 bits of depth resolution in terms of image files) or is it downsampled / truncated to fit in the PNG?

taketwo · 2013-07-31T10:52:43Z

Depth ("z" field) is saved to a mono PNG file with 16 bits per pixel. By default "z" value of a point is multiplied by 10000 to get pixel value. Although the number is somewhat arbitrary (should it be different? let's discuss), it gives nice pictures for typical scenes. Of course, for points that are further away than ~6.55 m the product will overflow 16 bit and will be aliased with close points. If the user works with such scenes he could use other custom scale factor. Alternatively, he could use automatic scaling to full range.

jspricke · 2013-08-15T14:37:40Z

Hi Sergey,

that's a lot of code, thanks for contributing! On the other hand, this will take some time to review. For example I've found you are using make_shared() a number of times. Is this really needed?

Regarding the API change, would it be possible to keep the old API and make it call the new one (haven't checked the code)? We have some PCL_DEPRECATED() macros to notice users of this functions, but we can't remove stuff from the API during the 1.7.X releases.

Regarding the pcd2png, would it be possible to extend the old tool, so we don't have to introduce a new one?

Thanks!

taketwo · 2013-08-19T18:45:58Z

Jochen,

I do use make_shared in a few places for convenience reasons. I could easily remove that of course. But is there something wrong with it? Out of curiosity I grepped for make_shared in PCL sources and found many usages.
Yes, the new savePNGFile() functions that I introduced do not collide with the old ones, so they can happily live together. I will add depreciation macro to the old functions and remove the calls to them within PCL.
The old tool organized_pcd_to_png is very simple, has no options, it just loads a file and calls the savePNGFile() function. The new tool is more sophisticated, so to say. It has several command line switches to utilize the power of the new savePNGFile() functions.
I am not sure what exactly you meant by "extending the old tool". If you meant that it is preferable to keep the old name, then I would argue that the old name actually stands out from the list of other "converter" tools like obj2vtk, oni2pcd, pcd2ply, png2pcd and so on... Otherwise, if you are concerned with the command line arguments, I can set the defaults so that when called without options it behaves exactly like the old tool.

rbrusu · 2013-09-07T18:26:33Z

make_shared creates an extra copy, so it's a bit inefficient, depending on what object you apply it to.

taketwo · 2013-09-07T18:41:26Z

To my knowledge it's quite the opposite, or do I misunderstand boost documentation?

Besides convenience and style, such a function is also exception safe and considerably faster because it can use a single allocation for both the object and its corresponding control block, eliminating a significant portion of shared_ptr's construction overhead.

rbrusu · 2013-09-07T18:43:46Z

Yes, but it still calls the copy constructor afaik. The best way is to create a shared_ptr directly, and then use it throughout the code.

jspricke · 2013-09-10T08:23:31Z

Regarding the tool, I'm fine with adding the new one, could you add a warning to the old one, that it's deprecated, as well? Thanks!

taketwo · 2013-09-27T08:40:11Z

Bump. Are there any remaining issues that prevent from merging?

jspricke · 2013-10-02T08:17:37Z

io/include/pcl/io/png_io.h

 #include <string>
 #include <vector>
+#include <pcl/io/point_cloud_image_extractors.h>
+#include <boost/make_shared.hpp>


This is unneeded, as far as I can see.

jspricke · 2013-10-02T08:19:07Z

Looking fine for me, @rbrusu are you ok with it?

Old pcl::io::savePNGFile() functions are obsolete now, as well as the organized_pcd_to_png tool.

This gets rid of depreciation warning in example_supervoxels.cpp and organized_pcd_to_png.cpp.

This way the default behavior is the same as of the organized_pcd_to_png tool.

This allows to get a colorful image where different labels are painted with random colors. Unit tests and pcd2png tool are updated accordingly.

taketwo · 2013-10-02T10:30:58Z

I removed the unnecessary include and also added some documentation for PointCloudImageExtractor.

rbrusu · 2013-10-02T15:26:08Z

looks great to me!

Add a generic `savePNGFile()` function

…ibly earlier, PointCloudLibrary#204)

taketwo added 5 commits July 28, 2013 15:14

Add an overload of pcl::io::savePNGFile().

372183e

The new function takes sensor_msgs::Image and saves it as a PNG file. Currently supported are "rgb8", "mono8", and "mono16" image encodings.

Add an overload of pcl::io::savePNGFile().

b6e75bd

The new function takes a field name and uses appropriate PointCloudImageExtractor subclass to extract an image from the given field. Currently supported are "normal", rgb", "label", "z", "curvature", and "intensity" fields.

Add pcd2png application to the "tools" module.

2eb9a66

Add tests for Point Cloud Image Extractors.

4b13d85

jspricke reviewed Oct 2, 2013
View reviewed changes

taketwo added 6 commits October 2, 2013 12:28

Remove boost::make_shared invocations.

c2c6351

Add depreciation macros.

e3a9673

Old pcl::io::savePNGFile() functions are obsolete now, as well as the organized_pcd_to_png tool.

Use new pcl::io::savePNGFile() function.

a8050d2

This gets rid of depreciation warning in example_supervoxels.cpp and organized_pcd_to_png.cpp.

Make "rgb" field the default option in pcd2png.

049d173

This way the default behavior is the same as of the organized_pcd_to_png tool.

Add random color mode for "label" field.

990e601

This allows to get a colorful image where different labels are painted with random colors. Unit tests and pcd2png tool are updated accordingly.

Add more documentation for PointCloudImageExtractor.

3f1ded3

jspricke added a commit that referenced this pull request Oct 2, 2013

Merge pull request #204 from taketwo/png-io

9493da2

Add a generic `savePNGFile()` function

jspricke merged commit 9493da2 into PointCloudLibrary:master Oct 2, 2013

aPonza pushed a commit to aPonza/pcl that referenced this pull request Apr 11, 2020

remove Organized PCD to PNG tool, deprecated since pcl-1.7.1 (or poss…

ace4752

…ibly earlier, PointCloudLibrary#204)

truhoang pushed a commit to truhoang/pcl that referenced this pull request Jun 30, 2020

remove Organized PCD to PNG tool, deprecated since pcl-1.7.1 (or poss…

b04fa98

…ibly earlier, PointCloudLibrary#204)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a generic `savePNGFile()` function #204

Add a generic `savePNGFile()` function #204

taketwo commented Jul 28, 2013

chriskilding commented Jul 31, 2013

taketwo commented Jul 31, 2013

jspricke commented Aug 15, 2013

taketwo commented Aug 19, 2013

rbrusu commented Sep 7, 2013

taketwo commented Sep 7, 2013

rbrusu commented Sep 7, 2013

jspricke commented Sep 10, 2013

taketwo commented Sep 27, 2013

jspricke Oct 2, 2013

jspricke commented Oct 2, 2013

taketwo commented Oct 2, 2013

rbrusu commented Oct 2, 2013

Add a generic savePNGFile() function #204

Add a generic savePNGFile() function #204

Conversation

taketwo commented Jul 28, 2013

chriskilding commented Jul 31, 2013

taketwo commented Jul 31, 2013

jspricke commented Aug 15, 2013

taketwo commented Aug 19, 2013

rbrusu commented Sep 7, 2013

taketwo commented Sep 7, 2013

rbrusu commented Sep 7, 2013

jspricke commented Sep 10, 2013

taketwo commented Sep 27, 2013

jspricke Oct 2, 2013

Choose a reason for hiding this comment

jspricke commented Oct 2, 2013

taketwo commented Oct 2, 2013

rbrusu commented Oct 2, 2013

Add a generic `savePNGFile()` function #204

Add a generic `savePNGFile()` function #204