Skip to content

Fixes to GeoTiff writing with original LZW compression. - #2180

Merged
lossyrob merged 3 commits into
locationtech:masterfrom
lossyrob:fix/geotiff-lzw-streaming
May 8, 2017
Merged

lossyrob merged 3 commits into
locationtech:masterfrom
lossyrob:fix/geotiff-lzw-streaming

Conversation

@lossyrob

@lossyrob lossyrob commented May 7, 2017

Copy link
Copy Markdown
Member

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 requested a review from echeipesh May 7, 2017 05:45
arr(i) = imageData.segmentBytes.getSegmentByteCount(i)
}
arr
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@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

Copy link
Copy Markdown
Contributor

👍

@lossyrob
lossyrob removed the request for review from echeipesh May 8, 2017 18:53
@lossyrob
lossyrob merged commit ad4bd78 into locationtech:master May 8, 2017
@lossyrob lossyrob added this to the 1.1 milestone May 8, 2017
@kthompson

Copy link
Copy Markdown
Contributor

Thanks! :D

@lossyrob

lossyrob commented May 8, 2017

Copy link
Copy Markdown
Member Author

@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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Savings a streaming MultibandGeoTiff creates a corrupted geotiff

3 participants