Fix ImImagePlugin.seek() frame stride for non-8-bit modes - #9800
Open
chuenchen309 wants to merge 1 commit into
Open
Fix ImImagePlugin.seek() frame stride for non-8-bit modes#9800chuenchen309 wants to merge 1 commit into
chuenchen309 wants to merge 1 commit into
Conversation
seek() computed the per-frame byte stride as 8 * len(self.mode), using the length of the mode *name* string as bits-per-pixel. That is only correct when len(mode) equals the band count and the bands are 8-bit (L, RGB, RGBA, CMYK, LA, P). For I;16 (16-bit), I / F (32-bit) and YCbCr (3-byte) the stride was wrong, so seeking to frame >= 1 in a multi-frame IM image either raised 'image file is truncated' or returned pixels from the wrong offset. Use 8 * bands * itemsize, which matches Image.new(mode).tobytes() for every IM-supported mode and leaves the working modes unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrew Chen <48723787+chuenchen309@users.noreply.github.com>
akx
reviewed
Jul 20, 2026
| else: | ||
| bits = 8 * len(self.mode) | ||
| mode = ImageMode.getmode(self.mode) | ||
| bits = 8 * len(mode.bands) * int(mode.typestr[-1]) |
Contributor
There was a problem hiding this comment.
It's really not clear to a reader why we multiply things with an integer from the last character of a type string :)
Member
Do you have such a file available? Or did you find an IM specification that you were able to refer to for this? If not, you're saying that this part of our seeking implementation is incorrect... but only this part, and presuming that the rest of it is correct, and creating a new test based on our seeking implementation. It just seems a bit snake-eating-its-own-tail. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes proposed in this pull request:
ImImagePlugin.seek()computing the per-frame byte stride as8 * len(self.mode)— the length of the mode name string used as bits-per-pixel. That only equals the real value whenlen(mode)matches the band count and the bands are 8-bit (L,RGB,RGBA,CMYK,LA,P). ForI;16(16-bit),I/F(32-bit) andYCbCr(3-byte), the stride was wrong, soseek(frame >= 1)on a multi-frame IM image raisedOSError: image file is truncated(I;16,YCbCr) or returned pixels from the wrong offset (I,F).8 * bands * itemsize(viaImageMode), which matchesImage.new(mode).tobytes()for every IM-supported mode and leaves the six already-correct modes byte-for-byte unchanged.Reachability, stated honestly: this is a low-frequency bug. Pillow's own
_saveonly writes one frame's data, so Pillow cannot itself produce a valid multi-frame IM file — the trigger is a multi-frame IM/IFUNC file from an external tool in one of these less-common modes. The IM reader does advertise multi-frame support (n_frames/seekare wired), so reading such a file with a wrong stride is a real correctness bug, but it is not a hot path. Happy to close if you'd rather not carry it.