Skip to content
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

add support for LZW compression #116

Merged
merged 21 commits into from
Oct 10, 2023
Merged

Conversation

chrstphrbrns
Copy link
Contributor

Fixes #8

Validated using images from a variety of packages, using a variety of pixel formats, and comparing with uncompressed versions of the same image

@tlnagy
Copy link
Owner

tlnagy commented Sep 27, 2023

Wow, thanks! LZW support has been requested for awhile.

I'll review this carefully later this week, but do you mind adding tests? I believe bali.tif from my tlnagy/exampletiffs repo is LZW compressed.

Also is this your own original implementation or was this adapted from somewhere? We just need to make sure the licenses are compatible if the latter.

@chrstphrbrns
Copy link
Contributor Author

This implementation is not adapted or ported from any other code

@codecov-commenter
Copy link

codecov-commenter commented Sep 28, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (560ddd8) 90.20% compared to head (a310cd5) 91.57%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #116      +/-   ##
==========================================
+ Coverage   90.20%   91.57%   +1.36%     
==========================================
  Files          13       13              
  Lines         766      902     +136     
==========================================
+ Hits          691      826     +135     
- Misses         75       76       +1     
Files Coverage Δ
src/TiffImages.jl 100.00% <ø> (ø)
src/ifds.jl 98.29% <100.00%> (+0.98%) ⬆️
src/compression.jl 97.76% <97.54%> (+1.76%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

===

I've found inconsistent behavior between Houdini and Pixelmator,
so defaulting to behavior consistent with the spec

Images will load (apparently) correctly in either case, but
we show a warning for missing EOI in the non-spec case
src/ifds.jl Show resolved Hide resolved
@tlnagy
Copy link
Owner

tlnagy commented Oct 2, 2023

Do you mind bumping the version to v0.7.0?

@tlnagy tlnagy added this to the 0.7.0 milestone Oct 2, 2023
src/ifds.jl Outdated Show resolved Hide resolved
@tlnagy
Copy link
Owner

tlnagy commented Oct 9, 2023

Okay, I think this is good to go. I'll tag v0.6.8 first to release #121 and then we'll tag and release this PR as v0.7.0

@tlnagy tlnagy merged commit 8c898d1 into tlnagy:master Oct 10, 2023
13 checks passed
@tlnagy
Copy link
Owner

tlnagy commented Oct 10, 2023

Thanks @chrstphrbrns for pushing this all the way through! I think people will be very excited for this.

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.

Add support for LZW compression
3 participants