shintadono / laszip.net

LAS/LAZ library for C#
GNU Lesser General Public License v2.1
38 stars 26 forks source link

Error Reading 1.4 Files "Compressor 3 #8

Closed twdotson closed 4 years ago

twdotson commented 6 years ago

Attempting to read some 1.4 files results in this error.

"compressor 3 not supported (LASzip v2.2r0) ... "

Here is an example LAZ (1.4) file to reproduce the problem: ftp://ftp.kymartian.ky.gov/kyaped/LAZ/N064E344.laz

rapidlasso commented 6 years ago

Those are LAS 1.4 files with point types 6 or higher that are compressed with the new "native LAS 1.4 extension" for LASzip that was paid for by NOAA. LASzip 3.X is needed to decompress those files.

ocaspi-sc commented 5 years ago

So, are there any plans to update THIS project as well, in order to support LAS 1.4?

hobu commented 5 years ago

Patches to do so would be merged.

On Nov 4, 2018, at 8:24 AM, orca456 notifications@github.com wrote:

So, are there any plans to update THIS project as well, in order to support LAS 1.4?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

shintadono commented 5 years ago

Library has been updated (see develop branch) to read compressor 3 and 4 files, and to write with compressor 4. Support for compatibility mode has also been added.

New version is very fresh, and not thoroughly tested, yet. Any feedback on bugs is extremly welcome.

jimmyfishbean commented 5 years ago

Hi Guys,

I have pulled down the develop branch and will be trying to assist with testing, reporting bugs when I find any. What is the procedure for this, for example do I create a new issue per bug, or simply add all bugs that relate to the develop branch to a single issue?

I found the following couple of issues (I will create new issues if need be once I get a reply to my question above):

1) I am using point data format 7 that uses RGB14. The code was returning "unknown LASitem type XXX" in class laszip_api method read_header() due to no RGB14 case type declaration. I added this case type and it now compresses the LAS to LAZ. If you need a copy of the original LAS file used please let me know and I will make it available somehow.

2) When decompressing the LAZ back to LAS, I get no errors but when viewing the decompressed LAS, it seems the classification has an issue. The following screens are from LASTools LasView application with the Colour By option set to classification (NOTE: There are no RGB values set in the LiDAR data). The first screen is the original LAS. The second screen is the compressed LAZ, and the third screen is the compressed LAZ decompressed back to LAS. (The last screen/image shows colouring by intensity - all three files were fine):

originallas compressedlaz decompressedlaz intensity

Hope to hear from you soon. Cheers,

Jimmy

shintadono commented 5 years ago

Hello Jimmy, first of all a big thanks for your support. For your question on how to report bugs:

On the reported issues:

  1. Yes, you are absolutely right, I messed up in read_header() and forgot a few cases in the switch block. I just fixed that.
  2. I've started to investigate that. But I'm struggling a bit, since I don't have a properly classified LAS/LAZ at hand. Could you supply me with the one you used in the screen shots?
jimmyfishbean commented 5 years ago

Hi Shintadono,

My pleasure to assist any way I can, I appreciate the work you have done to create this C# port - which I certainly take full advantage of in my projects.

Any fixes I do I will create a pull request as instructed. I probably do not feel confident enough (just now at least) to push fixes directly.

The LAS file I used in the reported bug is available for download at: http://www.routemapper.net/content/downloads/424460_511756.zip

Please could you let me know when you have downloaded it as I will need to take it offline.

I did have a look at the decompression bug but have not had a chance to try a fix - it seems the classification (which gets it's value from the classification_and_classification_flags property) does not get set correctly in class LASreadItemCompressed_POINT14_v4 method read() due to the classification_and_classification_flags property not being set (changed_flags bool is never set to true when decompressing the LAS tile above - and this value dictates if the flags/classification_and_classification_flags values are decompressed I believe). Apologies if I have this wrong.

Thanks, Jimmy

shintadono commented 5 years ago

Okay, I've got the file. Thanks a lot.

I'll have a look at LASreadItemCompressed_POINT14_v4.read() and the changed_flags, next. That could be it.

shintadono commented 5 years ago

Hi, I found the issue, the location was rather unexpected. It had nothing to do with (de-)compressors or reader/writer parts. A simple setter for the legacy classification field in laszip_point was wrong.

jimmyfishbean commented 5 years ago

Excellent. I will pull down latest code and retest. I will also be colouring the points (setting the RGB values) so will be able to test the compression/decompresssion of the RGB14 values. Thanks

jimmyfishbean commented 5 years ago

Have tested and can confirm the classification issue is now resolved, thanks.

jimmyfishbean commented 5 years ago

Hi Robert,

Is it possible to make the laszip_point rgb value not readonly or is there a good reason for this? I am building a tool for my employer that is similar to the LASTools LasColor tool. I am basically extracting the RGB values from imagery and applying them to the LiDAR data.
If unable to remove the readonly property then I suppose need to add a default constructor and a constructor that takes the ushort[] rgb value (and any other readonly properties such as the wave_packet)?

Some of the properties from laszip_point that were available in the previous version have now been removed, such as number_of_returns_of_given_pulse and flags. Are these fields/data now obsolete, even for LAS 1.2 data?

Thanks. Jimmy

shintadono commented 5 years ago

In the original library the rgb field is an array inside the point structure; meaning, you can read and write to the array (either using assignment operators or the memcpy() or similar functions), but it can't be replaced with another array, since that would be 'outside' of the point structure. (I could have done the field as 4 separate ushorts; that would make its usage quite bad.)

The readonly is doing basically the same, the array is always there and we don't need to check (before we access it), whether or not the array is still there and has the proper size. (Same goes for the wave_packet field.) Can't you use Array.Copy() function, or the .CopyTo() method, to fill the arrays?

As for the other properties: Most of them just got renamed according to the name changes in the LASzip C/C++ library (e.g. number_of_returns_of_givenpulse is just (extended/)number_of_returns, now). The previous version of this C# port used the version 2.2 API of LASzip. The RC is now version 3.2.9. with quite heavy changes compared (2.2. was ~4 year ago).

jimmyfishbean commented 5 years ago

Thanks for the explanation and apologies for the lack of insight. I should be able to proceed now

rapidlasso commented 5 years ago

There was a bug in the LASzip API for new (extended) point types when not both the extended and the legacy flags were populated. Maybe the issue you are seeing was related to this? Here the fix in LASzip: https://github.com/LASzip/LASzip/commit/2a36ee050d440c219ab5ecc0414458bad051df4b

shintadono commented 5 years ago

Hi Martin, nice of you to drop by. No, this issue had nothing to do with the flags (your fix was already ported). It was caused by an error of mine, when I tried to replicate the C structure bit fields with C# properties.

jimmyfishbean commented 5 years ago

Sorry for late reply. The DLL seems fine now, at least for LAS 1.4 point data record format 7. This is the only format I have access to so cannot test other formats for bugs.