mapillary / OpenSfM

Open source Structure-from-Motion pipeline
https://www.opensfm.org/
BSD 2-Clause "Simplified" License
3.34k stars 852 forks source link

depthmaps/merged.ply inconsistent (broken?) format [PCL] #122

Closed sfuerte closed 7 years ago

sfuerte commented 7 years ago

ISSUE

Running opensfm/bin/opensfm compute_depthmaps to generate a point cloud in opensfm/depthmaps/merged.ply.

When trying to read the PLY file with PCL, receiving the following error:

[pcl::PLYReader] opensfm/depthmaps/merged.ply:1465336: parse error
[pcl::PLYReader::read] problem parsing header!
[pcl::PLYReader::read] problem parsing header!

The file itself looks normal:

ply
format ascii 1.0
element vertex 1465323
property float x
property float y
property float z
property float nx
property float ny
property float nz
property uchar diffuse_red
property uchar diffuse_green
property uchar diffuse_blue
end_header
25.5590 58.7498 -43.8184 0.029 0.102 0.994 112 102 100
...
merged.ply lines 1-44/1465336 byte 1982/81468783 0%  (press RETURN)

"QUICK FIX"

After some Internet search, the issue is in newline formatting: dos vs unix style. Apparently, PCL is quite sensitive to that and recognizes only the former.
Easy fix can be done by vi +":w ++ff=unix" +":q" merged.ply.

Looks like when OpenSfM writes the PLY file, it misses one character on the last line.

The size difference between the generated and converted files is just one byte:

81468783  merged.ply           (converted with vi)
81468782  merged.ply-orig      (generated by OpenSfM)

PS

Though it's still questionable what shall be fixed:

IMHO both.

paulinus commented 7 years ago

If there is only one byte difference, i would guess that only a final \n has been added by vi. In this case replacing this line https://github.com/mapillary/OpenSfM/blob/f80abbcb09a0ec6b016b967f291aa09f6567c39f/opensfm/dense.py#L314 with

    return '\n'.join(header + vertices + [''])

should fix the problem.

Could confirm that this fixes the PCL reading problem? thanks!

sfuerte commented 7 years ago

Tested and confirmed. Your change to dense.py#L314 fixes the PCL reading issue.

Thank you!