Pillow icon indicating copy to clipboard operation
Pillow copied to clipboard

GIF extents size change

Open Yay295 opened this issue 1 year ago • 4 comments

The GIF file test_extents() test

https://github.com/python-pillow/Pillow/blob/6a9acfa5ca2b3ba462b086e4af776993bbd94a72/Tests/test_file_gif.py#L1381-L1390

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.

Yay295 avatar Jul 09 '24 01:07 Yay295

Related: #3822 #2383

Yay295 avatar Jul 09 '24 01:07 Yay295

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 https://github.com/python-pillow/Pillow/issues/2383#issuecomment-278113341

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.

radarhere avatar Jul 16 '24 00:07 radarhere

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.

Yay295 avatar Jul 16 '24 00:07 Yay295

I've created #8237

radarhere avatar Jul 16 '24 11:07 radarhere