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

[Debug] fix pcd io small probability bug #6122

Merged

Conversation

GCaptainNemo
Copy link
Contributor

I encountered a bug in parsing the PCD header. The reason is that the first point and first field x (FLOAT32) of my PCD is 13.265933990478516, which corresponds exactly to "DATA" after decoding, causing the code to not decode PCD correctly.

#Here is an python example to explain
import struct

float_number = 13.265933990478516
binary_data = struct.pack('f', float_number)
hex_string = binary_data.hex()
ascii_string = bytes.fromhex(hex_string).decode('ascii')
print(ascii_string)  # output : DATA

So I submitted this PR, in addition to DATA, the code also needs to use the following string ("binary", "binary_compressed") to determine if it is HEADER.

@mvieth
Copy link
Member

mvieth commented Sep 8, 2024

@GCaptainNemo Thanks, this looks like an interesting case. Unfortunately, your current code change makes many tests fail. I have a slightly different suggestion that will hopefully work:
Change

pcl/io/src/pcd_io.cpp

Lines 326 to 335 in af7b2d5

if (line_type.substr (0, 4) == "DATA")
{
data_idx = static_cast<int> (fs.tellg ());
if (st.at (1).substr (0, 17) == "binary_compressed")
data_type = 2;
else
if (st.at (1).substr (0, 6) == "binary")
data_type = 1;
continue;
}

To:

      if (line_type.substr (0, 4) == "DATA")
      {
        if (st.at (1).substr (0, 17) == "binary_compressed")
          data_type = 2;
        else if (st.at (1).substr (0, 6) == "binary")
          data_type = 1;
        else if (st.at (1).substr (0, 5) == "ascii")
          data_type = 0;
        else {
          PCL_WARN("[pcl::PCDReader::readHeader] Unknown DATA format: %s\n", line.c_str());
          continue;
        }
        data_idx = static_cast<int> (fs.tellg ());
        break; // DATA is the last header entry, everything after it will be interpreted as point cloud data
      }

For reference: https://pcl.readthedocs.io/projects/tutorials/en/master/pcd_file_format.html

@mvieth mvieth added module: io changelog: fix Meta-information for changelog generation labels Sep 8, 2024
@GCaptainNemo
Copy link
Contributor Author

@GCaptainNemo Thanks, this looks like an interesting case. Unfortunately, your current code change makes many tests fail. I have a slightly different suggestion that will hopefully work: Change

pcl/io/src/pcd_io.cpp

Lines 326 to 335 in af7b2d5

if (line_type.substr (0, 4) == "DATA")
{
data_idx = static_cast<int> (fs.tellg ());
if (st.at (1).substr (0, 17) == "binary_compressed")
data_type = 2;
else
if (st.at (1).substr (0, 6) == "binary")
data_type = 1;
continue;
}

To:

      if (line_type.substr (0, 4) == "DATA")
      {
        if (st.at (1).substr (0, 17) == "binary_compressed")
          data_type = 2;
        else if (st.at (1).substr (0, 6) == "binary")
          data_type = 1;
        else if (st.at (1).substr (0, 5) == "ascii")
          data_type = 0;
        else {
          PCL_WARN("[pcl::PCDReader::readHeader] Unknown DATA format: %s\n", line.c_str());
          continue;
        }
        data_idx = static_cast<int> (fs.tellg ());
        break; // DATA is the last header entry, everything after it will be interpreted as point cloud data
      }

For reference: https://pcl.readthedocs.io/projects/tutorials/en/master/pcd_file_format.html

@mvieth Thank you for your suggestion! I have changed it to your version.

Copy link
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@larshg larshg merged commit c02905a into PointCloudLibrary:master Sep 10, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: fix Meta-information for changelog generation module: io
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants