search

r-lidar / rlas

R package to read and write las and laz files used to store LiDAR data
https://cran.r-project.org/package=rlas
GNU General Public License v3.0
34 stars 14 forks source link

extra bytes in multiple files streamlasdata #7

Closed floriandeboissieu closed 6 years ago

floriandeboissieu commented 6 years ago

The extra bytes are not read when multiple files are merged. This is caused by LASreaderMerged::open under comment special check for attributes in extra bytes. I couldn't find any reason for this resetting of attributes, maybe because the unity/uniformity of the extra bytes across files is not checked (at reading nor at writing).

Jean-Romain commented 6 years ago

Crap !

You know what? From both of us you're now the more knowledgeable about LASlib integration in R. So I'm not asking you to find the problem because you do whatever you want. But you are likely to find the problem before me :wink:

I will look at that next week but not sooner.

But I'm not sure to understand a point. When loading the data into R it doesn't work but when reading, filtering writting into a las file in a streaming way it works?

Jean-Romain commented 6 years ago

I updated lasreadermerged from the lastest commit in LAStools https://github.com/LAStools/LAStools/commit/7fb18d65bcebb2caf0332e7f44b0d9e525e72fa7 Please tell me if it solves the issue. Thanks

floriandeboissieu commented 6 years ago

The LAStools update seems to work well, nice merge! However, you forgot to include the rest of my last commit:

  1. the get_attribute should depend on the datatype
  2. if scaling/offsetting is necessary then output is converted to double, otherwise it is converted to integer or double depending on the datatype of the attribute. As it is now in devel branch, and with the extra_byte.laz I uploaded, it is loading attributes as I32 instead of I16 and it is not applying scale and offset. it could be done with if else in for loops of eb_32 and eb_64 in a similar way to LASwriterTXT::unparse_attribute (laswriter_txt.cpp), however to avoid allocating temporary memory for each point I made it with 2 new methods (see first commit on issue_7 https://github.com/Jean-Romain/rlas/pull/8/commits/17c255b482d75ff6158ee5bc9c27bad77317f666 ). To avoid bothering with merging issues with the new lasreadermerged files, I copied the lasreadermerged files (cpp and hpp) from devel to issue_7, which is the point of my ultimate commit in issue #7 pull-request. All seems to work well now, I tested on pieces of extra byte files. I'll try to make a testthat to check it automatically. Cheers Florian PS: I'll work on the writer as soon as I can, it does not seem so difficult after all :-).

On Sun, Dec 3, 2017 at 1:15 PM, Jean-Romain Roussel < notifications@github.com> wrote:

I updated lasreadermerged from the lastest commit in LAStools LAStools/LAStools@7fb18d6 https://github.com/LAStools/LAStools/commit/7fb18d65bcebb2caf0332e7f44b0d9e525e72fa7 please tell me if it solve the issue. Thanks

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Jean-Romain/rlas/issues/7#issuecomment-348760507, or mute the thread https://github.com/notifications/unsubscribe-auth/AP54IR8vGytVsDd2o4MOz_-SMTdBhQD8ks5s8pDegaJpZM4QnsWl .

Jean-Romain commented 6 years ago

I did not forget anything. I'm just a bit lost with all these commits. Are you sure I can safely merge #8 in devel? It sounds ok but maybe I should close your PR and you can make a new tidy one with only the useful stuff?

floriandeboissieu commented 6 years ago

Sorry for the delay,

Yes I am sure you can merge it, but I can make a new PR if you feel more confortable with it, so that I can explain the issue with interger/double and scale/offset. It makes sense as this issue has nothing to do with the lasreadermerge one.

To test it, try


rlas::readlasdata("inst/extdata/extra_byte.laz", eb=0)

Amplitude Pulse width 3146904 12292

Amplitude Pulse width 8.27 4.8

Finally with las2txt command line you should find the same as with the PR version:

LAStools/bin/las2txt -i ~/git/rlasfdb/inst/extdata/extra_byte.laz -parse 
txyz01 -stdout

Cheers Florian On 04/12/2017 16:15, Jean-Romain Roussel wrote:

I did not forget anything. I'm just a bit lost with all these commits. Are you sure I can safely merge #8 https://github.com/Jean-Romain/rlas/pull/8 in devel? It sounds ok but maybe I should close your PR and you can make a new tidy one with only the useful stuff?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Jean-Romain/rlas/issues/7#issuecomment-348991836, or mute the thread https://github.com/notifications/unsubscribe-auth/AP54IZpDWn8DF8qQ-D37SFNhV-JjrOOCks5s9AyggaJpZM4QnsWl.

Jean-Romain commented 6 years ago

Ok great I'm merging. Again thank you for your contributions