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

Fixes to GeoTiff writing with original LZW compression. #2180

Merged
merged 3 commits into from May 8, 2017

Conversation

Projects
None yet
3 participants
@lossyrob
Member

lossyrob commented May 7, 2017

This PR fixes several issues with GeoTiff writing, in the case where you write a GeoTiff tile that has LZW compression with a predictor. We don't have support for writing LZW from uncompressed values, but we allow LZW to be used if the original compressed GeoTiff segments exist. This case had several issues:

  • Decompression with a predictor that had little endian byte order did not write the correct byte order.
  • The predictor GeoTiff tag was not being written

Writing bytes from a streaming source (using LazySegmentBytes) was also broken, because changes were made to not guarantee the ordering of the sequence of segments, which the writer relied on. A docstring mentions not to assume that the segments are iterated over in segment index order, and the writer code changed to gather segment byte counts in a way that provides the correct order.

Fixes #2177

@lossyrob lossyrob added the in progress label May 7, 2017

@lossyrob lossyrob requested a review from echeipesh May 7, 2017

arr(i) = imageData.segmentBytes.getSegmentByteCount(i)
}
arr
}

This comment has been minimized.

@lossyrob

lossyrob May 7, 2017

Member

@echeipesh @jbouffard this is the offending line for the streaming problem. Notice the doc string I made for SegmentBytes. The fixed line's reliance on the ordering of the segments must have been missed when SegmentBytes moved from always being segment order (which was clear since it used to only be Array[Array[Byte]] to an arbitrary order based on the getSegments(0 until length) call.

@lossyrob

lossyrob May 7, 2017

Member

@echeipesh @jbouffard this is the offending line for the streaming problem. Notice the doc string I made for SegmentBytes. The fixed line's reliance on the ordering of the segments must have been missed when SegmentBytes moved from always being segment order (which was clear since it used to only be Array[Array[Byte]] to an arbitrary order based on the getSegments(0 until length) call.

@jbouffard

This comment has been minimized.

Show comment
Hide comment
@jbouffard

jbouffard May 8, 2017

Contributor

👍

Contributor

jbouffard commented May 8, 2017

👍

@lossyrob lossyrob removed the request for review from echeipesh May 8, 2017

@lossyrob lossyrob merged commit ad4bd78 into locationtech:master May 8, 2017

1 of 2 checks passed

ip-validation The pull request did not pass Eclipse validation. The following users have invalid Signed-off-by footers: rdemanuele@gmail.com
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@lossyrob lossyrob removed the in progress label May 8, 2017

@lossyrob lossyrob added this to the 1.1 milestone May 8, 2017

@kthompson

This comment has been minimized.

Show comment
Hide comment
@kthompson

kthompson May 8, 2017

Contributor

Thanks! :D

Contributor

kthompson commented May 8, 2017

Thanks! :D

@lossyrob

This comment has been minimized.

Show comment
Hide comment
@lossyrob

lossyrob May 8, 2017

Member

@kthompson no problem. Can you test it out just to make sure it works on your end as well?

Member

lossyrob commented May 8, 2017

@kthompson no problem. Can you test it out just to make sure it works on your end as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment