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

pcl::people::GroundBasedPeopleDetectionApp has missing documentation/small bugs #2120

Open
Cwiiis opened this issue Dec 5, 2017 · 2 comments
Labels
needs: code review Specify why not closed/merged yet

Comments

@Cwiiis
Copy link

Cwiiis commented Dec 5, 2017

As these are all relatively minor/easily solved issues, and to save myself some time, I've chosen to file this as a single issue - let me know if this should be broken out though.

There are a few things that make GroundBasedPeopleDetectionApp unusable in some circumstances, and also just very hard to discover how to use in others:

  • No documentation on what format it expects the point cloud in. Indeed, it expects x to increase rightwards, y to increase downwards (this was the confusing part) and z to increase outwards (I think - it might not matter).
  • It also expects the cloud points to be arranged in a width x height arrangement so that it can later extract a coloured image.
  • It would be useful to also document the expected arrangement when 'vertical' is true
  • setPersonClusterLimits takes variables called min/max width/height to calculate min/max points for Euclidean clustering, but the algorithm it uses to determine how many points this is isn't documented. It doesn't really correspond to min/max width/height of a human though, despite what the documentation says. This ought to be fleshed out, or perhaps just less overloaded in use (the min/max height does get used as person height later, though it doesn't correspond to height unless that person is standing straight).
  • setDimensionLimits is never called on the HeadBasedSubclustering object, so if your cloud happens to be of a configuration where people have more than 5000 points (or fewer than 30), you're out of luck.
  • Cluster tolerance is 2 * voxel size, rather than a configurable tolerance or ratio. For the default 6cm voxel size, this is a very high tolerance. It would seem to me that this variable isn't really linked with voxel size.
@SergioRAgostinho SergioRAgostinho added the needs: code review Specify why not closed/merged yet label Dec 5, 2017
@taketwo
Copy link
Member

taketwo commented Dec 9, 2017

Hi, thanks for bringing this up. For the parts where you are certain, it would be great if you submit a pull request with documentation clarifications. As for the parts that are not so clear, perhaps the author @mmunaro can clarify. (I did not see him for a while, but let's hope he is still reachable.)

@mmunaro
Copy link
Contributor

mmunaro commented Dec 13, 2017

Hi @Cwiiis , thanks for your comments.

  • the input point cloud the GroundBasedPeopleDetectionApp expects is a standard organized point cloud most algorithms in PCL expects. The convention of y increasing downwards is also shared with images (e.g. in OpenCV). Anyway, it would be great if you could improve the documentation so that no other people get stuck there. In particular, it is better to specify it expects an "organized point cloud" (see here: http://pointclouds.org/documentation/tutorials/basic_structures.php).

  • the input variables in setPersonClusterLimits actually represent min/max height and width, in meters, of clusters containing people (one or more). Thus, the max width is usually bigger than the width of a person. Min/max width were actually added by @moritzblume , so he could maybe comment more on this. However, the basic idea is that the voxel size (and the distance from the camera) determines the number of points a cluster containing a person should have.

  • for the above reasons, the tolerance on the max distance used for clustering does depend on the voxel size (in general, the cluster tolerance cannot be lower than the voxel size, otherwise no clusters are created).

  • I think that code was mainly used on data coming from Kinect or stereo cameras. If you think there are applications for which it is important to set also other parameters, it would be great if you could propose a pull request that adds those functionalities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: code review Specify why not closed/merged yet
Projects
None yet
Development

No branches or pull requests

4 participants