Skip to content

Conversation

@terrence2
Copy link

@terrence2 terrence2 commented Jun 2, 2023

Pull Request Overview

This pull request adds a delta compression filter and fixes multi-filter decompression; this appears to be the first test case making use of multiple filters.

Terrain height data tends to have lots of periodic repeating patterns, so GeoTiffs compressed with GDAL's liblzma support tend to have lots of tiles that make use of delta filtering. I need a native rust implementation of lzma decompression for use in OpenFA, so that I can port it to run on the web.

Testing Strategy

This pull request was tested by...

  • Added relevant end-to-end tests (an .xz file with a delta filter).
  • Integrated into OpenFA's GeoTiff parser and demonstrated equivalent function to liblzma.
  • Ran cargo bench to verify there were no performance regressions.

Supporting Documentation and References

Code for the delta filter was adapted from the C code in xz-file-format.txt.

Copy link
Author

@terrence2 terrence2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding some notes inline to aid review:

Copy link
Owner

@gendx gendx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution, and apologies for the delay as I was caught up in many other things this year. Overall this looks good, I left a few comments.

@gendx gendx added the waiting on author Waiting for pull request author to address review comments label Nov 8, 2023
@terrence2
Copy link
Author

Great feedback! I think the new diff is much cleaner and clearer than the previous.

Since I was touching decomp_big_file anyway, I took the liberty of replacing read_all_file with the new fs::read builtin while I was there.

Please give it another look, at your convenience.

@terrence2 terrence2 requested a review from gendx November 11, 2023 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting on author Waiting for pull request author to address review comments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants