[Liblas-devel] Thoughts on a messy bug
mateusz at loskot.net
Wed Jul 29 20:52:06 EDT 2009
Howard Butler wrote:
> The interpretation resided in the length of the LAS header. We read the
> spec to mean that a LAS header was the required bits (227 bytes) + the
> VLRs + (2 pad bytes if the minor version was 1.0). The spec is unclear
> if you want to write extra space in the header (accounting for the pad
> bytes as necessary) and always depending upon the DataOffset to
> determine where to start reading/writing points. Because of this
> assumption, my lazy ass used the dataoffset as the bit to do accounting
> for writing VLRs. Additionally, I was using this shorthand accounting
> to pass information into the reader/writer about where to start
> reading/writing. This meant that things could get out of sync and it
> was impossible to account for header pad space.
I'd blame ASPRS for not-specifying details in LAS specification.
It's not for the first time.
> At any rate, there are many files out in the wild that *do* have extra
> space at the end of the VLRs before the beginning of the points. We
> need to be able to read them no matter what, and it would be nice to be
> able to write in such a fashion as well. I have a patch that implements
> a fix for #136 and supports writing extra padding when the dataoffset is
> greater than the space the VLRs would require (and accounting for 2 pad
> bytes for minor version 1.0). I wrote this against the 1.2 branch, and
> all tests pass.
Yes, makes sense.
> The hesitation involves:
> 1) the patch adds a new method to the C/Python API because it is now
> necessary to be able to manually set the dataoffset. No C++ API has
> changed, however. Do we release a minor release with a C API addition?
Why not? Both APIs are part of the same library.
> 2) the patch changes nearly 800 lines of code and feels quite
> substantial (but much simpler, frankly)
Looks like no other option than to apply it.
> 3) the patch represents a significant behavioral change, especially when
> writing files. The user can now set the dataoffset to whatever they
> wish, and when VLR writing happens, it checks if it has enough space.
> If it doesn't, the writer expands the header for them to fit. If the
> user sets the dataoffset so large that there's no problem fitting the
> VLRs in, \0's are written in between the end of the VLRs to the
> dataoffset byte.
What's the default behavior if a user does not set the offset?
> My questions are:
> a) should we release this fix plus the rest of the 1.2 stuff as 1.2.1
> late this week <http://liblas.org/milestone/1.2.1>
I'd go for it.
> b) should this constitute a major release (I would propose to call it
> 1.4 so as to not confuse with the LAS spec due to no 1.3 support in
> trunk currently).
Good point. The fix is substantial as you've pointed.
I have no problem with new major release.
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org
More information about the Liblas-devel