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

Reading Big GeoTiffs #1753

Merged
merged 21 commits into from Nov 2, 2016

Conversation

Projects
None yet
3 participants
@jbouffard
Contributor

jbouffard commented Oct 31, 2016

This PR adds the ability for GeoTrellis to read Big, GeoTiff files. It is important to note that this will only work with GeoTiffs. Regular BigTiff files are currently not supported even though their smaller counterparts are. This is an issue that will be addressed and fixed in the near future.

@jbouffard

This comment has been minimized.

Show comment
Hide comment
@jbouffard

jbouffard Oct 31, 2016

Contributor

I should also mention that I added three new files to the geotiff-test-files.zip link. Because of that, it is now just over the max file size of GitHub, though, it still uploaded fine. It's 50.8 MB while the max is 50 MB.

Contributor

jbouffard commented Oct 31, 2016

I should also mention that I added three new files to the geotiff-test-files.zip link. Because of that, it is now just over the max file size of GitHub, though, it still uploaded fine. It's 50.8 MB while the max is 50 MB.

@jamesmcclain

jamesmcclain requested changes Oct 31, 2016 edited

All of the changes look pretty conservative and logical to me.

I would just want to double check that the BigTiff code works when the BigTiff is generated from a known-good copy of all-ones.tif (that is, a BigGeoTIFF gdal_translated from a normal GeoTiff which has been gdal_translated from all-ones.tif).

@jbouffard

This comment has been minimized.

Show comment
Hide comment
@jbouffard

jbouffard Oct 31, 2016

Contributor

@jamesmcclain Unfortuantely, even when I created a correct version of all-ones.tif and then made a BigTiff version of that with gdal_translate it still failed.

Also, I just added the comment to the TiffFieldType code.

Contributor

jbouffard commented Oct 31, 2016

@jamesmcclain Unfortuantely, even when I created a correct version of all-ones.tif and then made a BigTiff version of that with gdal_translate it still failed.

Also, I just added the comment to the TiffFieldType code.

@jamesmcclain

This comment has been minimized.

Show comment
Hide comment
@jamesmcclain

jamesmcclain Oct 31, 2016

Member

@jamesmcclain Unfortuantely, even when I created a correct version of all-ones.tif and then made a BigTiff version of that with gdal_translate it still failed.

Ah, okay. If we can just confirm that the actual bytes in the offset field of entry 34737 match those of the too-large value, that would be good enough for me.

Member

jamesmcclain commented Oct 31, 2016

@jamesmcclain Unfortuantely, even when I created a correct version of all-ones.tif and then made a BigTiff version of that with gdal_translate it still failed.

Ah, okay. If we can just confirm that the actual bytes in the offset field of entry 34737 match those of the too-large value, that would be good enough for me.

@jbouffard jbouffard changed the title from [WIP] Reading Big GeoTiffs to Reading Big GeoTiffs Oct 31, 2016

@jbouffard

This comment has been minimized.

Show comment
Hide comment
@jbouffard

jbouffard Oct 31, 2016

Contributor

@jamesmcclain I'm going to take this PR out of WIP, since it seems to work except for this one file; but I'll compare the bytes of the two GeoTiffs in a bit.

Contributor

jbouffard commented Oct 31, 2016

@jamesmcclain I'm going to take this PR out of WIP, since it seems to work except for this one file; but I'll compare the bytes of the two GeoTiffs in a bit.

@jbouffard

This comment has been minimized.

Show comment
Hide comment
@jbouffard

jbouffard Oct 31, 2016

Contributor

@lossyrob This is ready for review.

Contributor

jbouffard commented Oct 31, 2016

@lossyrob This is ready for review.

@jamesmcclain

This comment has been minimized.

Show comment
Hide comment
@jamesmcclain

jamesmcclain Oct 31, 2016

Member

@jamesmcclain I'm going to take this PR out of WIP, since it seems to work except for this one file; but I'll compare the bytes of the two GeoTiffs in a bit.

Okay, sounds like a winner.

Member

jamesmcclain commented Oct 31, 2016

@jamesmcclain I'm going to take this PR out of WIP, since it seems to work except for this one file; but I'll compare the bytes of the two GeoTiffs in a bit.

Okay, sounds like a winner.

@lossyrob lossyrob added this to the 1.0 milestone Nov 1, 2016

Made PR review changes
Fixed formatting issue in TiffTagsReader
@jbouffard

This comment has been minimized.

Show comment
Hide comment
@jbouffard

jbouffard Nov 1, 2016

Contributor

@lossyrob Format change has been made.

Contributor

jbouffard commented Nov 1, 2016

@lossyrob Format change has been made.

@lossyrob lossyrob merged commit 91ddc02 into locationtech:master Nov 2, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment