Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OUR-CVFH seg-faults when applied to certain PCLPointCloud2 conversions from certain clouds #1449

Closed
barryridge opened this issue Nov 26, 2015 · 10 comments

Comments

@barryridge
Copy link

Hi all,

Firstly, FYI, I am running Ubuntu 14.04, cmake 2.8.12.2, g++ 4.8.4, and pcl-trunk on a Core i7.

I have been having trouble applying OUR-CVFH to a data set of segmented objects in PCD files. It was throwing segmentation faults on numerous files that other feature descriptors, e.g. CVFH, were having no trouble with. After some investigation, I believe I have narrowed things down. I have constructed a minimal example below to demonstrate the issue:

#include <pcl/io/pcd_io.h>
#include <pcl/features/normal_3d.h>
#include <pcl/features/our_cvfh.h>
#include <pcl/point_types.h>

// 
// (1) THIS WILL CAUSE SEG-FAULTS WHEN COMBINED WITH (3):
//
typedef pcl::PointXYZ PointT;

//
// (2) BUT THIS WILL NOT WHEN COMBINED WITH (3) OR (4):
//
// typedef pcl::PointXYZRGB PointT;

int
main(int argc, char** argv)
{
  // Input cloud (PointT):
  pcl::PointCloud<PointT>::Ptr cloud_pointt(new pcl::PointCloud<PointT>);
  // Normals:
  pcl::PointCloud<pcl::Normal>::Ptr normals(new pcl::PointCloud<pcl::Normal>);
  // OUR-CVFH features:
  pcl::PointCloud<pcl::VFHSignature308>::Ptr features(
    new pcl::PointCloud<pcl::VFHSignature308>);

  //
  // (3) THIS WILL CAUSE SEG-FAULTS WHEN COMBINED WITH (1), BUT NOT WITH (2):
  //
  // Input cloud (PCLPointCloud2):
  pcl::PCLPointCloud2::Ptr cloud_pclpc2(new pcl::PCLPointCloud2);

  // Read a XYZRGBA PCD file into cloud_xyzrgba
  if (pcl::io::loadPCDFile(argv[1], *cloud_pclpc2) != 0)
    return -1;

  // Convert from PCLPointCloud2 to a PointT cloud
  pcl::fromPCLPointCloud2(*cloud_pclpc2, *cloud_pointt);


  //
  // (4) THIS WILL NOT CAUSE SEG-FAULTS WITH EITHER (1) OR (2):
  //
  // Input cloud (PointXYZRGB):
  // pcl::PointCloud<pcl::PointXYZRGBA>::Ptr cloud_xyzrgba(new pcl::PointCloud<pcl::PointXYZRGBA>);
  //
  // // Read a XYZRGBA PCD file into cloud_xyzrgba
  // if (pcl::io::loadPCDFile<pcl::PointXYZRGBA>(argv[1], *cloud_xyzrgba) != 0)
  //    return -1;
  //
  // Convert from a PointXYZRGBA cloud to a PointT cloud
  // pcl::copyPointCloud(*cloud_xyzrgba, *cloud_pointt);


  // Estimate the normals.
  pcl::NormalEstimation<PointT, pcl::Normal> normalEstimation;
  normalEstimation.setInputCloud(cloud_pointt);
  normalEstimation.setRadiusSearch(0.03);
  pcl::search::KdTree<PointT>::Ptr kdtree(new pcl::search::KdTree<PointT>);
  normalEstimation.setSearchMethod(kdtree);
  normalEstimation.compute(*normals);

  // Compute OUR-CVFH features
  pcl::OURCVFHEstimation<PointT, pcl::Normal, pcl::VFHSignature308> ourcvfh;
  ourcvfh.setInputCloud(cloud_pointt);
  ourcvfh.setInputNormals(normals);
  ourcvfh.setSearchMethod(kdtree);
  ourcvfh.setEPSAngleThreshold(5.0 / 180.0 * M_PI);  // 5 degrees.
  ourcvfh.setCurvatureThreshold(1.0);
  ourcvfh.setNormalizeBins(false);
  ourcvfh.setAxisRatio(1.0);

  std::cout << "I will get this far..." << std::endl;
  ourcvfh.compute(*features);
  std::cout << "...but not this far." << std::endl;
}

So, assuming others can replicate this with their own data, as you can see from the above code, there is a workaround, but it unfortunately involves jettisoning PCLPointCloud2, which could be a problem if you don't know in advance what kind of PCD files you're working with.

I would be interested to hear if others can replicate this problem. Either way, I can provide sample PCD files showing breakage and non-breakage cases if needed. I have been using partial views of household objects recorded from a Kinect 2 sensor.

Cheers,
Barry Ridge

@SergioRAgostinho
Copy link
Member

It would indeed be good if you could provide a PCD in which your example segfaults. It would be even best it you could replicate the problem with one of the files in this folder.

@barryridge
Copy link
Author

Hi Sergio,

Here are two sample PCD files from my data set showing segmented partial views of both a box-shaped object and a ball-shaped object. The code I included above, in its original form, causes a seg-fault for the first case, but not for the latter case.

I will try testing some of the PCD files you linked to and get back to you. I will also try to test this on a different computer.

Cheers,
Barry

@barryridge
Copy link
Author

Testing on test files from the test folder

Ok, so I tested the above code on many of the PCD files in the folder you linked to @SergioRAgostinho (at least the ones that seemed like reasonable candidates for testing with OUR-CVFH, i.e. well-segmented objects), and got the following results:

milk_color.pcd: "*** Error in `ourcvfhfeatureextractionsimple': free(): invalid next size (fast): 0x00000000008df2b0 ***"
sac_plane_test.pcd: "No clusters were found in the surface... using VFH..."
noisy_slice_displaced.pcd: No errors.
ism_test.pcd: "No clusters were found in the surface... using VFH..."
ism_train.pcd: "No clusters were found in the surface... using VFH..."
lamppost.pcd: "No clusters were found in the surface... using VFH..."
milk.pcd: No errors.
cturtle.pcd: No errors.
car6.pcd: "No clusters were found in the surface... using VFH..."
bun01.pcd: "No clusters were found in the surface... using VFH..."
bun02.pcd: "No clusters were found in the surface... using VFH..."
bun03.pcd: "No clusters were found in the surface... using VFH..."
bun0.pcd: "No clusters were found in the surface... using VFH..."
bun0.pcd: "No clusters were found in the surface... using VFH..."
bun4.pcd: "No clusters were found in the surface... using VFH..."
bunny.pcd: "No clusters were found in the surface... using VFH..."

What's interesting here is that it throws a memory error for milk_color.pcd (which is XYZRGBA), but not for milk.pcd (which is XYZ). It's as though the conversion performed using fromPCLPointCloud2() is causing OUR-CVFH to fail, unless the PCLPointCloud2 was read from a file with a point type that matches the target point type for the conversion. I.e., it's ok to go from XYZ PCD file -> PCLPointCloud2 cloud -> XYZ cloud, or XYZRGBA PCD file -> PCLPointCloud2 -> XYZRGBA cloud, but it's not ok to go from XYZRGBA PCD file -> PCLPointCloud2 cloud -> XYZ cloud.

Am I using fromPCLPointCloud2() as intended here?

The documentation appears to suggest that I am:

Alternatively, you can read a PCLPointCloud2 blob (available only in PCL 1.x). Due to the dynamic nature of point clouds, we prefer to read them as binary blobs, and then convert to the actual representation that we want to use.

pcl::PCLPointCloud2 cloud_blob;
pcl::io::loadPCDFile ("test_pcd.pcd", cloud_blob);
pcl::fromPCLPointCloud2 (cloud_blob, *cloud); //* convert from pcl/PCLPointCloud2 to pcl::PointCloud<T>

Testing on a different computer

I have tested the above out on a separate computer (Lenovo Thinkpad Core i7, Ubuntu 14.04, cmake 2.8.12.2, g++ 4.8.4, pcl-trunk) and produced the same results:

The box-shaped object causes a seg-fault, whereas the ball-shaped object does not.

milk_color.pcd produces the same error as before, whereas milk.pcd succeeds.

(I didn't test the remaining PCD files in the test folder as they seem less relevant)

@SergioRAgostinho
Copy link
Member

Thank you for your analysis @barryridge.

Am I using fromPCLPointCloud2() as intended here?

At first glance, I don't see any problem with what you're doing.

As a side note have you tried load straight into a pointcloud instead of going through PCLPointCloud2, using this pcl::io::loadPCDFile overload?

@barryridge
Copy link
Author

Yes, I have tried that- in fact, if you'll note, I included that in comments in the code I originally posted. That method works fine. The problem, as I see it, is that it defeats the purpose of using PCLPointCloud2 in the first place- i.e. you might not know what kind of file you're dealing with in advance, but maybe you just want the XYZ points anyway, so you blindly load the file into a PCLPointCloud2 and then convert it to an XYZ cloud.

@barryridge
Copy link
Author

@SergioRAgostinho, I thought a little bit more about what you suggested and since I wasn't fully sure if you meant this or not, I decided to try using the pcl::io::loadPCDFile method, templated with PointXYZ, to attempt to load the XYZ component directly from XYZRGBA files like so:

  //
  // (5) THIS HAS ALSO, PERHAPS EXPECTEDLY, PRODUCED ERRORS/SEGFAULTS ON XYZRGBA FILES:
  //
  // Input cloud (PointXYZRGB):
  pcl::PointCloud<pcl::PointXYZ>::Ptr cloud_xyz(new pcl::PointCloud<pcl::PointXYZ>);

  // Read a XYZRGBA PCD file into cloud_xyz
  if (pcl::io::loadPCDFile<pcl::PointXYZ>(argv[1], *cloud_xyz) != 0)
    return -1;

  // Copy from a PointXYZ cloud to a PointT cloud
  pcl::copyPointCloud(*cloud_xyz, *cloud_pointt);

I wasn't expecting this to work, and indeed, it does not. It produces the same results as before with the same files.

@SergioRAgostinho
Copy link
Member

@barryridge You tripped on something. I tried the both milk files you mentioned and the ones you uploaded, and was only able to get a seg-fault out of barcaffebox_seg.pcd.

The issue is being caused by an out of bound access here. The distance_normalization_factor value seems to be incorrect, because apparently there's a point with a bigger norm than distance_normalization_factor which is causing the out-of-bounds access, and I suspect the culprit to be pcl::getMaxDistance().

@SergioRAgostinho
Copy link
Member

The problem is being caused by an inconsistency in the way pcl::getMaxDistance computes the distance. If you look at its source code it is more or less clear that it is only interested in x, y and z coordinates of your points. Nevertheless, it's using Eigen::Vector4f::norm() to compute the distance.

With the particular combination you found, this is a problem because in the pointcloud passed that is passed to pcl::getMaxDistance, the 4th element in your xyz/data[4] is just trash, which is resulting on a wrong calculation of max_pt.

I'll submit a PR later today.

SergioRAgostinho added a commit to SergioRAgostinho/pcl that referenced this issue Nov 28, 2015
Fixes PointCloudLibrary#1449. getMaxDistance was using the norm of a Eigen::Vector4f
to compute the 3D distance between points. The norm was using the
4th element of xyz/data[4] which is often just bit trash. When an
index vector is provided, the function was returning the wrong
point. Added tests for both
@SergioRAgostinho
Copy link
Member

@barryridge The PR was merged. Can you test it to see if it fixed your problem? The error messages you posted before were slightly different than mine.

@barryridge
Copy link
Author

@SergioRAgostinho That appears to have resolved the issue on my side for the test files mentioned above. I will do some further testing with a larger data set and let you know if there any further issues. Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants