search

geotiffjs / geotiff.js

geotiff.js is a small library to parse TIFF files for visualization or analysis. It is written in pure JavaScript, and is usable in both the browser and node.js applications.
https://geotiffjs.github.io/
MIT License
845 stars 175 forks source link

Added LERC_ZSTD decompression #397

Closed TheMrCam closed 9 months ago

TheMrCam commented 9 months ago

I noticed that Planetary Computer COGs are compressed with LERC_ZSTD and could not be read by geotiff.js.

I fumbled around a bit to figure out a working implementation of the Zstandard decoder since there's a couple Javascript ports, so I figured I'd make this PR in case it can be improved or merged.

The zstddec library utilizes WebAssembly (which I'm rather unexperienced with) and getting it properly initialized was the main pain point; I'm sure it could be cleaner but this appears to work for me.

The "DOM" types in tsconfig appear to be necessary (based on https://github.com/microsoft/TypeScript-DOM-lib-generator/issues/826) but there might be better options, I just did the quickest fix to get it building.

I didn't notice until after I started on this that addDecoder could override current decoder implementations, but I'm struggling with getting that working in my own project and including this in an official release would make my job a smidge easier.

constantinius commented 9 months ago

Hey @TheMrCam

Thanks for supplying this integration! This is indeed very cool. Unfortunately, since the underlying (de-)compressor uses WASM, it seems to break testing.

Do you think there is a way around this?

TheMrCam commented 9 months ago

Unfortunately, since the underlying (de-)compressor uses WASM, it seems to break testing.

Do you think there is a way around this?

Hello @constantinius, thanks for taking a look at this! WASM is definitely not my strong suit, but I assume there should be a way around it for testing since the package seems to work when integrated into my project. Also, the failed checks in CI all pass on my local machine. I'll dig around a bit to see what I can do.

constantinius commented 9 months ago

@TheMrCam I see that node has added WASM support (not sure in which version). We are currently using node v14 in testing, which seems ancient, and has reached end end-of-life this April.

I'll try an update to Node v20, which is the current LTS version, to see if the tests pass then.

constantinius commented 9 months ago

@TheMrCam My naive update of Node did not seem to have worked, there seems to be a regression now: https://github.com/geotiffjs/geotiff.js/actions/runs/6558959613/job/17813580079?pr=398

Which node version are you using? How did you test locally?

TheMrCam commented 9 months ago

@constantinius The Node update results actually looks as I expected - I'm getting the same 6 test failures locally (on Node v18.12.1). I wasn't sure if they were actual errors or if I wasn't setting things up properly, but now that I know they come from upgrading from Node 14.x I should be able to figure out what else needs to change.

It looks like there's 3 separate issues that cause these tests to fail:

I believe these should all be fixable and am looking into it now, and I will update if my assessment changes.

TheMrCam commented 9 months ago

@constantinius I believe I found the solution, or at least it now tests OK locally & still works great integrated into my package. I did also update the ci.yml Node version to get the checks working, which absorbs your PR #398; if that's unnecessary or undesired, I can roll it back.

constantinius commented 9 months ago

Thanks @TheMrCam! This looks very good