Adds in-memory PCD serialization/deserialization; de-duplicates PCDReader::readHeader(). (take #2) - #1986
Merged
Conversation
…tream &os as a parameter, instead of using a local std::ostringstream oss and returning a std::string. Updated docstring of std::ostream & version, in order to describe return value. Added overload of PCDWriter::generateHeaderBinaryCompressed() for compatibility with legacy code. Internally, the std::ostream & version is still used to avoid needless string copies. Added an overload of PCDWriter::writeBinaryCompressed () that writes to a std::ostream. Changed existing PCDWriter::writeBinaryCompressed () implementation to simply utilize the std::ostream version, then write the result to a file.
…nd the entire body (259 lines) copied. Upon diffing the two, no meaningful differences were found. The overload was made to call the original, in order to avoid them drifting out of sync.
Updated docstring of PCDReader::readHeader() to indicate which members of the cloud parameter are modified.
…o support reading PCD-serialized point clouds from memory. This also simplifies PCDReader::read(), leaving it just to deal with opening & closing files.
…>() calls. Added TEST (PCL, LZFInMem), to exercise new functionality for reading & writing std::iostreams.
taketwo
approved these changes
Sep 4, 2017
taketwo
left a comment
taketwo
left a comment
Member
There was a problem hiding this comment.
This is an excellent PR. Thanks!
| std::vector<char> temp_buf (data_size * 3 / 2 + 2 * sizeof (unsigned int)); | ||
| // Compress the valid data | ||
| unsigned int compressed_size = pcl::lzfCompress (only_valid_data, | ||
| unsigned int compressed_size = pcl::lzfCompress (&only_valid_data.front (), |
Member
There was a problem hiding this comment.
Informative note, std::vector exposes a .data() method which does exactly this.
Contributor
Author
There was a problem hiding this comment.
Good catch.
...another C++11 improvement I seem to have missed. I've long used std::string::data(), when null-termination wasn't needed. Nice to see it in std::vector.
Feel free to change it, if you want.
Member
There was a problem hiding this comment.
Oh right, it's a C++11 feature, so we can not integrate it anyway.
| * * < 0 (-1) on error | ||
| * * == 0 on success | ||
| */ | ||
| int |
Member
There was a problem hiding this comment.
From the code this feels like it should return a Boolean instead of an integer. Was it to keep it consistent with all other methods returning integers as status?
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This supersedes #1869, in order to coalesce the changes into a smaller number of commits, each of which conforms to white space & naming conventions and avoids test failures.