Skip to content

PCDReader: remove fields with zero count instead of throwing exception while reading#4623

Merged
mvieth merged 2 commits intoPointCloudLibrary:masterfrom
shreysamdani:master
Oct 16, 2022
Merged

PCDReader: remove fields with zero count instead of throwing exception while reading#4623
mvieth merged 2 commits intoPointCloudLibrary:masterfrom
shreysamdani:master

Conversation

@shreysamdani
Copy link
Copy Markdown
Contributor

Description

For LIDAR pcd files, there can be instances where the count for a field is < 1 if the field is "_". In such cases, we shouldn't throw an error when trying to read the file.

Bugs resolved

PDAL/PDAL#2572

@mvieth
Copy link
Copy Markdown
Member

mvieth commented Apr 1, 2021

Hi, sorry for the delay.
I am not so sure about this code change. I feel like we might introduce bugs if we allow point fields with count < 1 (even if they are only "_" fields). Do you know how these LIDAR PCD files were created? The PCDWriter from PCL won't write fields with count=0.
BTW a solution to "fix" such PCD files would be to remove the columns from the PCD header (remove the respective entries in FIELDS, SIZE, TYPE, COUNT).

@weimzh
Copy link
Copy Markdown

weimzh commented Apr 29, 2021

same issue here.
many PCD files provided by our client won't work due to this.

.PCD v0.7 - Point Cloud Data file format

VERSION 0.7
FIELDS x y z _ intensity _ timestamp _ ring _
SIZE 4 4 4 1 4 1 8 1 2 1
TYPE F F F U F U F U U U
COUNT 1 1 1 4 1 0 1 6 1 12
WIDTH 93504
HEIGHT 1
VIEWPOINT 0 0 0 1 0 0 0
POINTS 93504
DATA binary

@kunaltyagi
Copy link
Copy Markdown
Member

Instead of modifying the core lib, how about a tool to remove 0 count fields?

@mvieth
Copy link
Copy Markdown
Member

mvieth commented Apr 29, 2021

Or how about changing the PCDReader so that these fields are removed during reading, as if they had never been in the PCD file?

@kunaltyagi
Copy link
Copy Markdown
Member

😆 Didn't realize this was not an issue, but a single line PR

Comment thread io/src/pcd_io.cpp Outdated
int col_count;
sstream >> col_count;
if (col_count < 1)
if (col_count < 1 && cloud.fields[i].name != "_")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of moving ahead with a 0 count, should we be using erase+remove on cloud.fields[i]? (either here or after reading the header)

@xaedes
Copy link
Copy Markdown

xaedes commented Aug 25, 2022

This is still an issue in 2022. Will that PR be merged?

@mvieth
Copy link
Copy Markdown
Member

mvieth commented Aug 28, 2022

This is still an issue in 2022. Will that PR be merged?

Not with the changes it makes right now. Copied from my earlier comment:

I am not so sure about this code change. I feel like we might introduce bugs if we allow point fields with count < 1 (even if they are only "_" fields). Do you know how these LIDAR PCD files were created? The PCDWriter from PCL won't write fields with count=0.
BTW a solution to "fix" such PCD files would be to remove the columns from the PCD header (remove the respective entries in FIELDS, SIZE, TYPE, COUNT).

@mvieth mvieth changed the title Do not throw error for invalid count when field name is "_" (pcl::PCDReader::readHeader) PCDReader: remove fields with zero count instead of throwing exception while reading Oct 10, 2022
Copy link
Copy Markdown
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.

I will merge this in a few days if there are no objections

@mvieth mvieth merged commit 64775f8 into PointCloudLibrary:master Oct 16, 2022
@mvieth mvieth added the changelog: enhancement Meta-information for changelog generation label Nov 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: enhancement Meta-information for changelog generation module: io

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants