Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not use transparency when saving GIF if it has been removed when normalizing mode #7284

Merged
merged 1 commit into from Jul 13, 2023

Conversation

radarhere
Copy link
Member

Resolves #7279

def _write_single_frame(im, fp, palette):
im_out = _normalize_mode(im)
for k, v in im_out.info.items():
im.encoderinfo.setdefault(k, v)

When _normalize_mode converts the image, transparency may be removed from im.info by convert(). However, that change is not detected at the moment because the transparency is only removed for the new im_out image - im still has transparency, and when _write_local_header gets passed im a few lines later, it is detected.

_write_local_header(fp, im, (0, 0), flags)

def _write_local_header(fp, im, offset, flags):
transparent_color_exists = False
try:
if "transparency" in im.encoderinfo:
transparency = im.encoderinfo["transparency"]
else:
transparency = im.info["transparency"]
transparency = int(transparency)

This PR simplifies matters by no longer checking im.info in _write_local_header, only im.encoderinfo. Any transparency that needs to be known has been copied into encoderinfo. See

for k, v in im_out.info.items():
im.encoderinfo.setdefault(k, v)
and
im_frame.encoderinfo = params

@homm
Copy link
Member

homm commented Jul 13, 2023

Do we have clearly policy how im.info interacts with im.encoderinfo? Why in some cases we want to account it while in other we don't?

@radarhere
Copy link
Member Author

Just to start by stating the obvious

  • im.info is something that is attached to an image for its lifetime.
  • im.encoderinfo are additional arguments passed as keywords when saving.

im.encoderinfo has a higher priority because the user is explicitly passing those values. Values in im.info might have been set by the user, but might also just have been set when opening the image.

I'm not trying to ignore im.info in this case. I'm using the transparency after the image has been converted to a GIF-appropriate mode, rather than using the original transparency implicit in the image. If pixel values can change, then it makes sense to me for the implicit transparency to change with them.

The code you've highlighted appears different because "duration" and "disposal" aren't things that are affected when changing mode. "transparency" is.

In short, I think im.info values apply to the image in the current state, whereas im.save("out.gif", ...) arguments apply to the GIF in its final form.

@homm
Copy link
Member

homm commented Jul 13, 2023

Thanks. I realize that we have the original im in _write_local_header, while in fact we are saving im_out and that's why we should ignore im.info in this place.

@hugovk hugovk merged commit 7a2510c into python-pillow:main Jul 13, 2023
53 checks passed
@radarhere radarhere deleted the gif branch July 13, 2023 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants