Skip to content

GIF extents size change #8218

Description

@Yay295

The GIF file test_extents() test

Pillow/Tests/test_file_gif.py

Lines 1381 to 1390 in 6a9acfa

def test_extents() -> None:
with Image.open("Tests/images/test_extents.gif") as im:
assert im.size == (100, 100)
# Check that n_frames does not change the size
assert im.n_frames == 2
assert im.size == (100, 100)
im.seek(1)
assert im.size == (150, 150)

tests that an image with two frames, where the second frame is offset from the first, will change the size of the image. The actual frame data is that each frame is 100x100, and the second frame is offset by 50 pixels both right and down. Here's an equivalent image with different colored frames (to make it easier to see them):

grow

created with ImageMagick magick -delay 50 -size 100x100 xc:#FF0000 ( xc:#0000FF -set page +50+50 ) grow.gif

Logically, when overlaying one image onto another with an offset, the size of the image will increase by that offset. However, I have not found any application that actually does this when displaying this GIF. Every application I have tried has kept the size of the first frame, cutting off any pixels from the second frame that extend beyond that. ffplay even gives a warning message: "Image too wide by 50, truncating."

Based on this real-world testing, I think Pillow should probably do the same as everyone else and not change the image size when the GIF frames are different sizes.

It also seems that behind the scenes, the image is not actually 150x150. len(im.getdata()) at the end of that test is 10000, not 22500, and im.getpixel((123,123)) raises an IndexError.

Activity

  1. Yay295 commented on Jul 9, 2024

    @Yay295
    ContributorAuthor

    Related: #3822 #2383

  2. radarhere commented on Jul 16, 2024

    @radarhere
    Member

    https://www.w3.org/Graphics/GIF/spec-gif89a.txt states

    Each image must fit within the boundaries of the Logical Screen, as defined in the Logical Screen Descriptor.

    I take the word 'must' to mean that images that don't are invalid. Pillow often tries to be flexible though, so we aim to process this image anyway. Given that the images are going against the spec, I don't think there can be an 'official' answer as to what we should do.

    The current solution was suggested in #2383 (comment)

    I'd propose that if the extents are outside the existing image, we should increase the canvas size to cover the union of the existing and next frame.

    My feeling is that if the choice is between 'passing along more image content to the user' and 'cutting off unexpected content', then showing more image content is better. In the face of ambiguity, I also have a general reluctance to reverse earlier decisions.

    If we're talking about images that go against the specification, then having a consistent experience across all decoders sounds like an improbable goal, because, well, it's an image with undefined behaviour.

  3. Yay295 commented on Jul 16, 2024

    @Yay295
    ContributorAuthor

    I'd propose that if the extents are outside the existing image, we should increase the canvas size to cover the union of the existing and next frame.

    It doesn't seem like this is actually happening though, because as I mentioned at the end

    len(im.getdata()) at the end of that test is 10000, not 22500, and im.getpixel((123,123)) raises an IndexError.

    The only thing that seems to actually be changed is the reported size.

  4. radarhere commented on Jul 16, 2024

    @radarhere
    Member

    I've created #8237

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions