-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Fixed writing multiple StripOffsets to TIFF #8317
Conversation
cc3fdcc
to
ac3b64c
Compare
c9d1abc
to
9fb34d8
Compare
9fb34d8
to
9707f88
Compare
9707f88
to
362ffaf
Compare
raise NotImplementedError(msg) | ||
value = self._pack("L", self._unpack("L", value)[0] + offset) | ||
size, handler = self._load_dispatch[typ] | ||
values = [val + offset for val in handler(self, data, self.legacy_api)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
values = [val + offset for val in handler(self, data, self.legacy_api)] | |
values = [val + offset for val in handler(self, data)] |
Throwing this suggestion out there, see what happens - considering that this should be SHORT or LONG (or LONG8), it doesn't matter whether the legacy API is in use or not, as they're all passed to
Pillow/src/PIL/TiffImagePlugin.py
Lines 502 to 505 in b557876
def basic_handler( | |
self: ImageFileDirectory_v2, data: bytes, legacy_api: bool = True | |
) -> tuple[Any, ...]: | |
return self._unpack(f"{len(data) // size}{fmt}", data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's more correct to pass the value since we have it. The value not being used is an implementation detail that this call shouldn't be concerned with.
If libtiff is used when saving, yes. Otherwise, no. |
Co-authored-by: Andrew Murray <[email protected]>
The StripOffsets values don't really mean anything without associated image data, so I just did the same thing as when this tag only has one value and added the IFD offset to the values.
StripOffsets is overwritten with correct values when writing TIFF images, so this only matters when writing a non-TIFF image, where StripOffsets doesn't mean anything. The main benefit of this change is that it won't raise an exception now when saving a non-TIFF image with EXIF data copied from a TIFF image that has more than one strip.