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

Explicitly enable strip chopping for large uncompressed TIFFs #5517

Merged
merged 1 commit into from Dec 30, 2021

Conversation

kmilos
Copy link
Contributor

@kmilos kmilos commented May 31, 2021

Helps #5370 (only works for uncompressed images)

@radarhere
Copy link
Member

@radarhere radarhere commented May 31, 2021

Would you be able to add a test, that fails without your change and passes with it?

@kmilos kmilos changed the title Enable strip chopping for large TIFFs Enable strip chopping for large uncompressed TIFFs Jun 2, 2021
@kmilos
Copy link
Contributor Author

@kmilos kmilos commented Jun 3, 2021

Ah, only internal TiffImagePlugin.ImageFileDirectory_v2 is ever used to parse the TIFF IFDs (in both Image.open() and TiffImagePlugin._load_libtiff(), there's no way to actually see how libtiff is re-interpreting the IFD as far as I can tell, so not really sure how to test this...

Tests/test_file_libtiff.py Outdated Show resolved Hide resolved
@radarhere
Copy link
Member

@radarhere radarhere commented Nov 16, 2021

https://linux.die.net/man/3/tifffdopen

The default behaviour, for backwards compatibility, is to enable strip chopping.

Is it possible that this PR doesn't actually do anything?

@kmilos
Copy link
Contributor Author

@kmilos kmilos commented Nov 17, 2021

I only know this: https://gitlab.com/libtiff/libtiff/-/blob/29219a5bd14040d60e376324a52ef91b68808143/libtiff/tif_open.c#L254-257

But it does seem the flags are initialized by default with the intention to do the chopping: https://gitlab.com/libtiff/libtiff/-/blob/29219a5bd14040d60e376324a52ef91b68808143/libtiff/tif_open.c#L161-164

However, there was also recent (not yet released) PR that seems to indicate the default wasn't working on libtiff CMake builds (which is what Pillow uses lately AFAIK): https://gitlab.com/libtiff/libtiff/-/merge_requests/271

If this is the behaviour Pillow was expecting, doesn't hurt to be explicit ;)

@radarhere radarhere changed the title Enable strip chopping for large uncompressed TIFFs Explicitly enable strip chopping for large uncompressed TIFFs Dec 30, 2021
@radarhere
Copy link
Member

@radarhere radarhere commented Dec 30, 2021

Ok, if the default has been wrong in previous libtiff versions, then it seems like this would be good, as it will fix that.

@radarhere radarhere merged commit 6565d5b into python-pillow:main Dec 30, 2021
51 checks passed
@kmilos kmilos deleted the patch-1 branch Feb 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants