PointCloudLibrary / PointCloudLibrary/pcl

[io] Add option to strip padding bytes while dumping and add padding while reading `PCLPointCLoud2`

Open
#1,566 5 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

kind: todo module: io
Dominant language
C++
Stars
11.1k
Forks
4.7k
Avg merge
4d 10h
Merged PRs (30d)
6

Description

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

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the linked reproduction, then inspect the PCLPointCloud2 read(PCLPointCloud2) path, the four generateHeader implementations, and the ASCII, binary, and compressed-binary writing methods mentioned in the issue. Compare the serialized and deserialized data sizes, and define the intended padding behavior before considering the proposed consolidation of writing paths.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
computer-vision
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.