PointCloudLibrary / pcl

Point Cloud Library (PCL)
https://pointclouds.org/
Other
9.94k stars 4.61k forks source link

Allow datatype mismatch between pcd file and cloud type if the data can be stored inside the (larger) class field #3659

Open joelsa opened 4 years ago

joelsa commented 4 years ago

My Environment

Context

I am working with point clouds stored in .pcd files from two different laser scanners and have a pipeline that takes data from both of them. I am processing the fields x, y, z, intensity and ring in order to do a calibration. One of the laser scanners outputs has the following header:

FIELDS x y z intensity t reflectivity ring noise range
SIZE 4 4 4 4 4 2 1 2 4
TYPE F F F F U U U U U
COUNT 1 1 1 1 1 1 1 1 1

so only a single byte of information for the ring whereas the other one has the following header:

FIELDS x y z intensity timestamp ring
SIZE 4 4 4 4 8 2
TYPE F F F F F U
COUNT 1 1 1 1 1 1

so 16 bit of information for the ring data.

I have a custom point type, with the following definition:

namespace MyPoint {
  struct EIGEN_ALIGN16 PointXYZIR {
    PCL_ADD_POINT4D;
    float intensity;
    uint16_t ring;
    EIGEN_MAKE_ALIGNED_OPERATOR_NEW

    static inline PointXYZIR make(float x, float y, float z, float intensity,
                                uint16_t ring) {
      return {x, y, z, 0.0, intensity, ring};
    }
  };
}

Expected Behavior

I expected that I could load data from both scanners, since the uint8 will fit into the uint16.

Current Behavior

Ring information can only be loaded for the data that has the 16 bit ring information, since there is a type mismatch on the other scanner. There, I get a failed to find match for field 'ring' error.

Code to Reproduce

The following line is responsible for this, since the field is ignored each time the datatypes do not exactly match: https://github.com/PointCloudLibrary/pcl/blob/1e3c62e2df0c739a7f5fd13610b043408d03aebe/common/include/pcl/point_traits.h#L198

Possible Solution

For me, I added a function can_fit_into

static bool can_fit_into(pcl::uint8_t source, pcl::uint8_t target)
{
  if (source == target)
  {
    return true;
  }
  if (source == PCLPointField::INT8 && (target == PCLPointField::INT16 || target == PCLPointField::INT32))
  {
    return true;
  }
  if (source == PCLPointField::INT16 && target == PCLPointField::INT32)
  {
    return true;
  }
  if (source == PCLPointField::UINT8 && (target == PCLPointField::UINT16 || target == PCLPointField::UINT32))
  {
    return true;
  }
  if (source == PCLPointField::UINT16 && target == PCLPointField::UINT32)
  {
    return true;
  }
  if (source == PCLPointField::FLOAT32 && target == PCLPointField::FLOAT64)
  {
    return true;
  }
  return false;
}

and changed the FieldMatches struct to:

template<typename PointT, typename Tag>
  struct FieldMatches
  {
    bool operator() (const pcl::PCLPointField& field)
    {
      return (field.name == traits::name<PointT, Tag>::value &&
              can_fit_into(field.datatype, traits::datatype<PointT, Tag>::value) &&
              field.count <= traits::datatype<PointT, Tag>::size);
    }
  };

This works fine for me, now both pcd files can be correctly loaded with ring information. Of course the function is not elegant or idiomatic, but a similar solution that is maintainable should be easy to implement.

Are there any reasons that speak against matching fields if the labels are the same and the data can be stored?

kunaltyagi commented 4 years ago

Are there any reasons that speak against matching fields if the labels are the same and the data can be stored?

One that I can think of is "version incompatibility". Essentially, someone used a program to create a file, you then filtered the file and created a filtered output. Then the filtered output can't be read by the original author because you happen to have used a larger size. This is something that I've seen happen often in such round-trips.

Of course, it's not something that PCL should concern with, but PCL shouldn't make it easy for incompatibilities to arise. Though this one is easy: emitting warnings for non-exact match or adding an option can be sufficient

joelsa commented 4 years ago

One that I can think of is "version incompatibility". Essentially, someone used a program to create a file, you then filtered the file and created a filtered output. Then the filtered output can't be read by the original author because you happen to have used a larger size. This is something that I've seen happen often in such round-trips.

I think that is a valid concern, however, right now the situation is somewhat similar. If I open a point cloud, internally convert it to a different format with either a larger or a smaller field size and write it to the disk again, it cannot be opened again with the same procedure. I agree, it should be the user's responsibility to ensure the appropriate output format for the intended application, but with such a change, the incompatibility could already arise at the input stage, without the user knowing about it.

Of course, it's not something that PCL should concern with, but PCL shouldn't make it easy for incompatibilities to arise. Though this one is easy: emitting warnings for non-exact match or adding an option can be sufficient

The FieldMatching part could be more verbose in general I think, it is quite a difference for the user if a field simply does not exist in the .pcd file or if it is just a slightly wrong type, right now the output failed to find match for field doesn't really convey much information.

stale[bot] commented 4 years ago

Marking this as stale due to 30 days of inactivity. It will be closed in 7 days if no further activity occurs.