-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Very minor changes to allow multiband GeoTiff reading #214
Comments
I'm in favor improving the crate to support a broader range of TIFF files. No need to gate changes on the file specifically being a geotiff. The only point of returning "unsupported" errors is because the rest of the code doesn't handle them properly. The goal is to either return an error or correctly decode the image. If any checks rejects TIFFs that we'd otherwise decode fine, then we should fix that.
I'm not quite sure what you mean here, but it might be easiest just to see the PR you have in mind.
Sounds reasonable.
Yep, this is why we have the option to disable decoding limits! Doesn't sound like further changes are required here. |
…lowing BlackIsZero and WhiteIsZero to have up to 64 bits per sample. Also includes decode_geotiff_images unit test.
I submitted the PR that I described (minus the decoding limit changed, which you pointed out isn't neeeded).
More than happy to do more on this if needed, so pls push back on anything that needs work here. |
Hi, small ping here to say that I've drafted a Pull Request to enable multi-band reads at #224, building on @tromper's work. Have tested the multi-band patch in my downstream library at weiji14/cog3pio#13, and it seems to be working nicely for a sample file, but would appreciate someone having a look to review it properly if possible! |
Sorry for the delay! The PR is on my list to review, but I've been rather busy with the image |
Thanks @fintelia! |
I realize that this has been proposed to various degrees before. In the GIS space, geotiffs with multiple bands are relatively common. It would be nice to be able to just read all of the band data into a vector. The coordinate projections can easily be handled manually since the relevant tags are already exposed. There are only a couple of minor things blocking the read functionality:
One way to implement the above would be to detect if a tiff is a geotiff (e.g. it has the coordinate project tag info present probably works) and then just bypass the above restrictions just for geotiffs w/o impacting any of the current functionality for non-geotiffs I was able to do this quickly and the existing ColorType::Gray ends up working fine for the different geotiff band data types. This isn't going work for all possible geotiffs (e.g bands with different data types won't work currently), but that's a less common use case (and commented as a todo in the code already, so maybe it happens someday). There are a couple of existing geotiff crates, but they were built for a single use case and appear to be largely unused/unsupported. I am happy to code up the changes (all 5 lines) and submit, but wanted to check first if a line had already been drawn on this topic and it isn't going to happen.
The text was updated successfully, but these errors were encountered: