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

loadplyfile may parse the header error #3353

Closed
wants to merge 2 commits into from

Conversation

sofu456
Copy link

@sofu456 sofu456 commented Sep 19, 2019

the iostream may be fail state with the last ingore in the end of the file in linux.

the iostream may be fail state with the last ingore in the end of the file in linux.
@taketwo
Copy link
Member

taketwo commented Sep 19, 2019

Thanks! Could you please post a short file that causes trouble on master and is parsed correctly with your patch?

some file the point is not enought for the header point count size
@sofu456
Copy link
Author

sofu456 commented Sep 23, 2019

Thanks! Could you please post a short file that causes trouble on master and is parsed correctly with your patch?

tmp.ply.txt

@taketwo
Copy link
Member

taketwo commented Sep 24, 2019

The last line of the file you posted is:

509.09079 -291.39304 -1129.5593 212 234 

The value of the blue field is missing. Therefore the line is not valid. In turn, this invalidates the entire file. The current master fails with a "parse error" message. Your patch makes it silently swallow the invalid file. I don't think this is desirable behavior.

Could you perhaps explain your use-case and why you think the PLY reader should behave in the way you proposed?

@sofu456
Copy link
Author

sofu456 commented Sep 24, 2019

The last line of the file you posted is:

509.09079 -291.39304 -1129.5593 212 234 

The value of the blue field is missing. Therefore the line is not valid. In turn, this invalidates the entire file. The current master fails with a "parse error" message. Your patch makes it silently swallow the invalid file. I don't think this is desirable behavior.

Could you perhaps explain your use-case and why you think the PLY reader should behave in the way you proposed?

maybe this cause this error, but i use the meshlab can open this file. so i think pcl can improve the fault tolerance. corlor or position loss may replace by default

@sofu456
Copy link
Author

sofu456 commented Sep 26, 2019

The last line of the file you posted is:

509.09079 -291.39304 -1129.5593 212 234 

The value of the blue field is missing. Therefore the line is not valid. In turn, this invalidates the entire file. The current master fails with a "parse error" message. Your patch makes it silently swallow the invalid file. I don't think this is desirable behavior.

Could you perhaps explain your use-case and why you think the PLY reader should behave in the way you proposed?

i just find the istream will cause a fail in the end of the file.and this file is generate by zed depth camera. and another file that do not loss the color in the end of the file . you can check it

point_cloud_PLY_18024_1242_24-09-2019-15-15-56.zip

@stale
Copy link

stale bot commented Nov 25, 2019

This pull request has been automatically marked as stale because it hasn't had
any activity in the past 60 days. Commenting or adding a new commit to the
pull request will revert this.

Come back whenever you have time. We look forward to your contribution.

@stale stale bot added the status: stale label Nov 25, 2019
@jasjuang
Copy link
Contributor

@sofu456 I just checked out your point_cloud_PLY_18024_1242_24-09-2019-15-15-56.zip and it's case 2 of the failure scenario that I filed here #3487. If you can take a look and show me how to resolve cases 3 and 4 I will greatly appreciate it.

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

Successfully merging this pull request may close these issues.

3 participants