Hey there,

during investigating the test failures of Set default alpha value 255 I was mainly wondering why there is so much code which appears to be duplicate and stumbled over the following:
Point clouds saved and loaded with different functions lead to different internal representations.

If creating a PointCloud<PointT> cloud, converting it to a PCLPointCloud2 object, saving that to a file, loading it back into another object, then the data vector of the two don't have the same length.
Code for reproducing this behavior can be found here.
Actually one of them loads the data with and the other without padding bytes (like the w value of the position). This makes sense to me in order to save disk space, but after loading the cloud again the data should be equal to the original, shouldn't it?
So I don't think this behavior is intended.

Also I have the impression that there is some duplicate code.
For example there are 4 implementations of generateHeader which basically all do the same and could be merged into one.
There are also 8 versions of writing pcd data compared to effectivly 1 read method (read(PCLPointCloud2)). The read method reads a PCLPointCloud2 and the others just use that along with from PCLPointCloud2 to read the templated types. Of course this read function contains reading ASCII, binary and compressed binary files which is split for the writing methods.
in addition there are methods for saving subsets of PointCloud<PointT> types but none PCLPointCloud2 types. Probably it doesn't make much sense for the latter ones. But if they existed one could reduce the implementation to

• writeASCII(PCLPointCloud2, vector<int>[, bool useIndices])
• writeBinary(PCLPointCloud2, vector<int>[, bool useIndices])
• writeBinaryCompressed(PCLPointCloud2, vector<int>[, bool useIndices])

What do you think?

Cheers,
Stefan