GIF decoder does not correctly handle images with different palettes

[benrg]

This is the cause (or one cause) of #1692 and #1525, but I thought it would be better to open a new bug that starts with an explanation instead of "this particular image doesn't work".

GIF defines a "logical screen" that can contain pixels from an unlimited number of images, each of which can have its own local palette of up to 256 colors. This means that there is no limit (other than 16,777,216 + 1 transparent) to the number of colors that can appear simultaneously on the screen.

I think that Pillow currently represents the logical screen as an array of palette indexes, renders images onto it in palette-index space, and replaces the palette for the whole screen with the local palette of the most recently rendered image. That works for GIFs that have only one palette or completely overwrite the logical screen on every frame, but it fails in general.

The simplest solution is probably to convert the "screen" to RGBA when combining images with different palettes, unless the last image covers the whole screen and doesn't have a transparent index.

[cflyt]

The simplest solution is probably to convert the "screen" to RGBA when combining images with different palettes, unless the last image covers the whole screen and doesn't have a transparent index.

I'm sorry, I don't understand what you mean. Can you tell me how to do with "convert the 'screen' to RGBA "?

[karlcow]

@benrg

Code examples would be useful.
On a project here we are using pillow to upload and save animated GIF.
karlcow/webcompat.com@3240856

and indeed the saved animated GIF images have funky color palettes.

[karlcow]

Code for https://github.com/python-pillow/Pillow/blob/master/PIL/GifImagePlugin.py

[karlcow]

ok. First test for an animated gif.
https://github.com/python-pillow/Pillow/blame/master/PIL/GifImagePlugin.py#L349

for im_frame in ImageSequence.Iterator(im):
    im_frame = _convert_mode(im_frame)

_convert_mode is defined by default as _convert_mode(im, initial_call=False) so for the im_frame = _convert_mode(im_frame) it is called always with initial_call=False.

So I tried to modify to add dither=None

def _convert_mode(im, initial_call=False):
    # convert on the fly (EXPERIMENTAL -- I'm not sure PIL
    # should automatically convert images on save...)
    if Image.getmodebase(im.mode) == "RGB":
        if initial_call:
            palette_size = 256
            if im.palette:
                palette_size = len(im.palette.getdata()[1]) // 3
            return im.convert("P", palette=1, colors=palette_size)
        else:
            return im.convert("P", dither=None)
    return im.convert("L")

And this worked. The animated gif was perfect.

PNG and JPEG have a optimize parameter, I wonder if GIF should also have something which forbids optimization.

[karlcow]

@aclark4life should I propose a PR.
Or would it be better to have an option disabling optimization like for PNG and JPEG.

[wiredfool]

• Is the dither=none likely to be correct for all frames 2:n? I'd think that any sort of stochastic dither would play hell with differential encoding.
• The _save method does check for the optimize flag in the encoderinfo dict, but that doesn't get passed into _convert_mode.

PR would be great, good test cases are welcome as well.

[zewt]

I was working on fixing some of the GIF decoding issues (#3433) and started thinking about how to fix some of these other palette issues. I don't know that I'd work on this soon (need to get back to what I was using this for in the first place), but I figured I'd post what I was thinking.

I'd suggest scanning all frame headers when the image is opened. This will tell whether the image should decode as a paletted or RGBA image. Without an initial scan, you can't tell whether the image should be created as P or RGBA until it's too late, since it might become needed on frame 2 after you've already exposed the image.

A nice side-effect of this is you get the frame count from it, so n_frames becomes trivial. That removes the whole category of issues where reading n_frames doesn't leave things in the state they were in. For example, seek to a frame, call setpixel a few times, then reading n_frames clobbers the changes you made.

The tradeoff is that you have to scan the whole file. That's cheap, though, since it doesn't have to actually decode image data, just parse the frame headers.

The decoder would need to be able to receive the palette and output RGBA for the RGBA code path. It might also want to understand RGB, for images like the #1692 spinning earth that use multiple palettes but don't output transparency, but that's an optimization.

[fmatheis]

The problem seems to be a bug at PIL at the class GifImageFile, when the palette is normalized at _normalize_palette it is not taking into account the palette of the current frame.

See fix at https://stackoverflow.com/a/62314520/988924

[InfiniteMarcus]

The problem seems to be a bug at PIL at the class GifImageFile, when the palette is normalized at _normalize_palette it is not taking into account the palette of the current frame.

See fix at https://stackoverflow.com/a/62314520/988924

This fix really helps, but if you need to convert the gif to RGBA, it will become really weird for some reason.

EPILEPSY ALERT! THE LAST GIF MAYBE CAN TRIGGER PHOTOSENSITIVE EPILEPSY, SO PROCEED WITH CAUTION

For example, this gif:

Without the fix:

With the fix but converting to RGBA:

Did anyone know how to fix this new problem?

[radarhere]

I've created PR #5857 as a solution to this. I keep the first frame as it is, P or L, and then subsequent frames become RGB or RGBA, depending on whether the first frame had transparency or not.

With my PR, running the following code on the image from the last post, I see no problems.

from PIL import Image
im = Image.open("in.gif")
im.save("out.gif", save_all=True)