Added setting for converting GIF P frames to RGB

[radarhere]

Alternative to #5974

Each frame in a GIF may contain up to 256 colours, and so after the first frame of a GIF, there might be more colours than can be fit into a P mode image. To handle this, #5857 caused subsequent GIF frames to be converted to RGB or RGBA. This resolved a number of open issues, and didn't change the behaviour of Pillow for users who just look at the first frame of a GIF.

Even so, there has been feedback.

• In Reading GIF frame-by-frame changes mode during read in Pillow 9 #5929 and Inconsistent GIF result after resizing #6085, two users would like the first frame of GIFs to be RGB as well.
• On the other hand, WIP: only convert GIF frames if palette actually changes #5974 would like GIFs to stay in P mode if the palette stays the same, rather than Pillow always upgrading to RGB after the first P frame.

To handle both of these cases, this PR suggests adding an enum to control the behaviour.

from PIL import GifImagePlugin
GifImagePlugin.PALETTE_TO_RGB = GifImagePlugin.ModeStrategy.AFTER_FIRST # default
GifImagePlugin.PALETTE_TO_RGB = GifImagePlugin.ModeStrategy.ALWAYS # first frame is RGB
GifImagePlugin.PALETTE_TO_RGB = GifImagePlugin.ModeStrategy.DIFFERENT_PALETTE_ONLY # keep P mode when the palette is the same

Also, I wasn't paying much consideration to L mode GIFs in #5857. This has also received feedback, and #6086 tried to address this by upgrading L to LA when transparency was present. Instead, here I'm keeping L mode GIFs as L, always.

[mara004]

Why not make DIFFERENT_PALETTE_ONLY the default?

[radarhere]

1. When changing the mode for GIFs, I think it should be predictable. If there is a P mode GIF and you seek forward, I think it is simpler to know that it will always become RGB(A), rather than wondering how this particular file will act. If you look at Inconsistent GIF result after resizing #6085, that user was confused why the first frame was different. If some subsequent frames were P and some were RGB(A), and that behaviour changed based on the particular GIF, I think that would introduce more confusion for users.

2. Pillow values backwards compatibility. I would be reluctant to change this behaviour after it was established in Pillow 9.0.

3. There is not a consensus that DIFFERENT_PALETTE_ONLY is the preferred behaviour. As I said, Reading GIF frame-by-frame changes mode during read in Pillow 9 #5929 and Inconsistent GIF result after resizing #6085 are reports from the users who would like GIF frames to never be P.

[radarhere]

And in the news this week, we say farewell to one of the creators of GIF, one of the people who helped image formats as we know them, and really, much of how people communicate through images on the Internet.

[FirefoxMetzger]

I like this idea and have three comments/suggestions:

1. Looking at the list, I think that a GifImagePlugin.ModeStrategy.NEVER option would be a nice complement to the existing ones. This would also allow "reverting" to pillow8 behavior for users who want backwards compatibility or who have a usecase that requires it.
2. The name GifImagePlugin.ModeStrategy.DIFFERENT_PALETTE_ONLY is a bit confusing when considering how the docs describe it. Perhaps GifImagePlugin.ModeStrategy.LOCAL_PALETTES_ONLY would be more fitting (since it triggers on the presence of local color palettes not on them being the same/different).
3. I'm also a bit doubtful if GifImagePlugin.ModeStrategy.DIFFERENT_PALETTE_ONLY will always do the desired thing. For example, I can construct at least one weird edge case where each frame sets a different value for the transparent color index and - when combined with a suitable background color - I can end up with 257 colors in a frame. Then again, I don't know why one would want to do that, so I guess it's okay to ignore.

[radarhere]

1. Wouldn't that result in incorrect images?
2. I was trying to leave room for a possible future where someone creates an issue requesting that the setting not only check for local vs global, but actually compares the palettes. If we went with LOCAL_PALETTES_ONLY now, then we would have to add DIFFERENT_PALETTE_ONLY then, and LOCAL_PALETTES_ONLY would become redundant.
3. Feel free to post that example code for investigation. I'm not going to pretend that this PR makes GifImagePlugin perfect, just that it does fundamentally address the request to minimise data in each frame by sticking to P mode where that wouldn't corrupt the image.

[FirefoxMetzger]

Wouldn't that result in incorrect images?

Depends on whether you view a multi-frame GIF as a video or as a sequence of images. If you see it as a video then yes; returning a P frame seems incorrect because it is unexpected to not get a reconstructed frame. (Which is I guess why the default changed in pillow9.) However, if you see GIF as a sequence of individual images then no; getting a P frame (which is exactly what is stored in the file) seems correct, because it is unexpected that GIF can return RGB/RGBA considering that it is a palette only format (only stores P or L). (Which is I guess the reason why the first frame remains as P by default.)

So from my perspective, the flag gives you the option to either ALWAYS reconstruct (GIF is video) or to turn off the reconstruction for the first frame and only do it AFTER_FIRST (mainly for the use-case of reading only the first frame) or to turn it off for some files and do it for DIFFERENT_PALETTE_ONLY. Thinking that thought to its end it seems kind of natural to have the option to turn reconstruction off completely and to NEVER reconstruct. I wouldn't make it the default (of course), but it doesn't seem to add much maintenance burden while also completing the pattern so I think it could be nice to have.

I was trying to leave room for a possible future where someone creates an issue requesting that the setting not only check for local vs global [...]

Fair enough, then the name makes perfect sense.

[...] I'm not going to pretend that this PR makes GifImagePlugin perfect, just that it does fundamentally address the request to minimise data in each frame by sticking to P mode [...]

Yep, and that it does achieve. As I said before, it's probably too much of an edge case to be of practical concern; mainly brought it up because I wasn't sure if you are worried about such niche cases or if you would prefer to address it once it actually becomes a problem.

[radarhere]

You know this, but I just want to give some context to anyone reading this in the future.

#6075 (comment)

So, an animated GIF does not have to be like a... PowerPoint slideshow, where independent images display one after another.
It can be like a series of Photoshop layers. Some layers are mostly transparent, and by applying one layer after another, the image is transformed.

Ok, so your idea for NEVER would provide the individual layers without combining them. These images may easily look terrible, as there was no expectation from the program creating the GIF that they would ever be seen. You were previously interested in this in #5307.

This isn't what DIFFERENT_PALETTE_ONLY does. It still combines the frames. Just because we're not converting to RGB doesn't mean that we're not pasting each layer onto the previous ones and using disposal methods to remove parts of previous images. A GIF might be very simple and have full-sized opaque images as layers, like PowerPoint, but there is no guarantee of that at all.

This also isn't what Pillow previously did. It still pasted layers onto each other and used disposal, it just didn't realise that more than a single palette might need to be used to determine colours.

#5307 (comment)

Without modifying Pillow, I wouldn't think that getting the raw frame with transparent pixels is even possible at the moment. If you'd like that as a new feature, please create a new issue.

If you look through the issues tagged with GIF, you will find that there is already a problem with combining the palette between GIF frames. Looking at the code, when loading each frame, it defaults to the global palette, which can be overridden by the local palette. The matter of keeping colors from previous frames doesn't just apply to transparency, but also to each new tile that may not take up the entire width and height of the image.

So I really don't think that setting an option called PALETTE_TO_RGB to NEVER correctly communicates what you are describing.

If we were to add this feature, I would want to use a different name. Instead of PALETTE_TO_RGB, I would suggest LOADING_STRATEGY, with RGB_AFTER_FIRST, RGB_AFTER_DIFFERENT_PALETTE, RGB_ALWAYS and... P_SPLIT.

Just for the record, could you make your case as to why this is a helpful feature? In #5307 (comment), you were interested in this as a debugging tool, essentially.

At the same time, it may still be useful to be able to just get the current frame without applying information from previous frames, e.g., for inspecting that writing worked as intended.

[FirefoxMetzger]

This isn't what DIFFERENT_PALETTE_ONLY does. It still combines the frames. [...]

Oh, then I've misunderstood what DIFFERENT_PALETTE_ONLY does. What do you think of changing the part of the docs that threw me?

Suggested change

	GIF frames do not always contain individual palettes however. If there is only
	a global palette, then all of the colors can fit within ``P`` mode. If you would
	prefer the frames to be kept as ``P`` in that case, there is also a setting
	available::
	GIF frames do not always contain individual palettes, however. If there is only
	a global palette, then frames may still use values of previous frames, but all
	of the colors can fit within ``P`` mode. If you would prefer the frames to be
	served as ``P`` in that case, there is also a setting available::

[radarhere]

My hope is that a previous part of the documentation already conveys the idea that new frames are layered on top of old modes.

P mode images are changed to RGB because each frame of a GIF may introduce up to 256 colors. Because P can only have up to 256 colors, the image is converted to handle all of the colors.

[FirefoxMetzger]

Yes, the part that you referenced provides the motivation for why the image is converted to RGB. What isn't clear yet (or at least wasn't clear to me when I read it) is that this behavior also applies if we don't convert to RGB. Maybe we can rephrase the sentence you referenced to better convey that decoding happens in all cases:

Transparency allows a GIF frame to use more than 256 colors, 256 from a local palette + other colors from previous frames. To handle this correctly, images will - by default - be promoted to RGB. While this recombination (decoding) will always happen, you can control how the result will be returned.

[radarhere]

I've pushed a commit. What do you think?

[FirefoxMetzger]

This also isn't what Pillow previously did. It still pasted layers onto each other and used disposal, it just didn't realise that more than a single palette might need to be used to determine colours.

Right. You mentioned this before, but I forgot that pillow is implemented like this; my bad. In that case, it does indeed seem disproportional to add this behavior here since it would essentially be a new feature instead of just "turning off a new feature".

Just for the record, could you make your case as to why this is a helpful feature?

I would still like it as a debugging tool 😅.

Another reason is that, yes, a raw frame may easily look terrible, but there are also cases where it contains useful information. For example when creating cartoon/anime animations like this one: https://i.imgur.com/uNdDoPT.gif . In its raw form, the file is fairly well-behaved; at least before any further optimization takes place. It uses disposal to clear each frame and because the background is transparent you can extract the individual steps of the animation [keyframes, but I don't want to alias that word so I will stick to steps]. However, if I now add a complex background things get complicated. Each raw frame still only contains one step of the animation but pillow will only let me access that step overlayed on the complex background. How would I get an animation step back? I could do some complex CV (background removal/extraction), write a small parser myself (it's a fairly simple use-case), or I may have the raw animation steps stored elsewhere. None of these seem particularly ideal. That being said, this is about as niche a use-case as me writing L-mode GIF and since supporting it would mean writing a sophisticated new feature, I agree that it is cleaner not to do it until there is more demand.

A third reason - though this is apparently just me and my completionism - is that it seems odd that it is only possible to get the decoded frames, but not the demuxed (raw) frames. We are essentially reading GIF like a video (raw bytes -[decompression]-> demuxed frame -[reconstruct]-> decoded frame). We can view demuxed frames as an intermediate implementation detail, but they are still frames so it seems odd (to me) that there is no easy way to access them.

[hugovk]

Can we add a versionadded for the docs?

https://pillow--6150.org.readthedocs.build/en/6150/reference/plugins.html#PIL.GifImagePlugin.ModeStrategy

[radarhere]

Ok, that now appears in the docs.

[nulano]

The setting name PALETTE_TO_RGB seems confusing to me.

In the code

GifImagePlugin.PALETTE_TO_RGB = GifImagePlugin.ModeStrategy.XXX

it isn't clear to my why a ModeStrategy should be used for a PALETTE_TO_RGB setting. I would prefer to have both of them share a base name for easier discoverability, either renaming the setting to e.g. LOADING_STRATEGY like suggested above in #6150 (comment) (and perhaps changing the enum to LoadingStrategy to match it), or renaming the enum to e.g. PaletteToRgbStrategy. Of the two options, I would prefer the former simply because it allows adding P_SPLIT in the future if that is determined to be a common request.

[radarhere]

Fair enough. I've pushed a commit to rename the setting.

[nulano]

It may be worth noting the name of the default option:

Suggested change

	GifImagePlugin.LOADING_STRATEGY = GifImagePlugin.LoadingStrategy.RGB_AFTER_DIFFERENT_PALETTE_ONLY
	GifImagePlugin.LOADING_STRATEGY = GifImagePlugin.LoadingStrategy.RGB_AFTER_DIFFERENT_PALETTE_ONLY
	To restore the default behavior of reading the first frame in ``P`` mode and converting to ``RGB`` for subsequent frames::
	from PIL import GifImagePlugin
	GifImagePlugin.LOADING_STRATEGY = GifImagePlugin.LoadingStrategy.RGB_AFTER_FIRST

[radarhere]

Ok, I've pushed a commit with a variation of this.

[hugovk]

Thanks all!