PointCloudLibrary / PointCloudLibrary/pcl

[io] savePCDFileBinary doesn't check if (height*width) equals no. of points

Open
#3,772 6 comments 0 reactions 1 assignee View on GitHub

@haritha-j is already working on this.

Since Mar 22, 2020.

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

Description

Describe the bug

pcl::io::savePCDFileBinary doesn't perform a check to see if
(height * width ) == no_of_points
before saving the file. Consequently, attempting to open the resultant PCD file in most applications will fail.

In contrast, pcl::io::savePCDFileASCII does perform this check, and throws EXCEPTION : [pcl::PCDWriter::writeASCII] Number of points different than width * height!

Context

Issue occurs when attempting to save in binary format, if the width*height value is not equal to the number of points in the cloud.

The following code snippet from pcd_io.hpp demonstrates how the ASCII variant handles this.
if (cloud.width * cloud.height != cloud.points.size ()) { throw pcl::IOException ("[pcl::PCDWriter::writeASCII] Number of points different than width * height!"); return (-1); }

Expected behavior

Throw the above mentioned exception, same as the ASCII variant.

As a side note, would it make more sense for the PCDWriter to actually attempt to rectify this issue by changing the width, and perhaps inform the user with a warning, rather than throwing an exception?
To a general user, I feel like that may be more convenient, but of course I'm not well-versed enough with the library to think of any situation why an exception would be more useful.

Current Behavior

No error thrown, output file is corrupt. (at least in the sense that most viewers won't open it due to the mismatch)

To Reproduce

  • Create a new point cloud and add some points
  • Keep the width and height values at 1
  • Save the cloud using the savePCDFileBinary method

Screenshots/Code snippets

Your Environment (please complete the following information):

  • OS: Ubuntu 18.04
  • Compiler: GCC 7.5
  • PCL Version: 1.9

Possible Solution

Simply adding the above check to the PCDWriter::writeBinary method should fix the issue. I'd be happy to put in a PR.

Additional context

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.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.