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

Only write GIF comments at the beginning of the file #6300

Merged
merged 7 commits into from May 23, 2022

Conversation

raygard
Copy link
Contributor

@raygard raygard commented May 14, 2022

Helps #6207

Place GIF comment after Global Color table. Should go after "NETSCAPE" looping extension after pull #6211.

Fixes #6299.

Changes proposed in this pull request:

  • Move writing of comment block from _write_local_header() to _get_global_header()
  • Add a test test_write_comment() to validate that a comment is written to the first frame only
  • Add a test test_write_no_comment() to validate that an empty comment="" arg to .save leaves no comments in saved file
  • Tests use image file multiple_comments.gif added in PR Separate multiple GIF comment blocks with newlines #6294 but could use any animated GIF instead — CHANGED to dispose_prev.gif’

The change here to _get_global_header() overlaps the change in PR #6211. Please be sure the merge is done so that the if "comment" in info and len(info["comment"]): code in this PR is placed after the if "loop" in info: code in PR #6211. The comment must come after the "NETSCAPE2.0" block.

Place GIF comment after Global Color table. Should go after "NETSCAPE" looping extension after pull #6211.
pre-commit-ci bot and others added 2 commits May 14, 2022 15:36
Changed to use a test image already in Images folder
@raygard
Copy link
Contributor Author

raygard commented May 14, 2022

I updated the tests to use dispose_prev.gif so that the automated tests would complete.

@raygard
Copy link
Contributor Author

raygard commented May 23, 2022

@radarhere I am fine with your changes to this. Thanks.

@radarhere radarhere changed the title Correct placement of GIF comment Only write GIF comments at the beginning of the file May 23, 2022
@radarhere
Copy link
Member

https://github.com/raygard/Pillow/pull/2 keeps the last comment read in info. The thinking is that if GIF comments are not intended to exist on a per-frame basis, then it would seem more correct to keep them when seeking to the the next frame - this way, if there is only a comment at the start of a GIF file, it is present for throughout subsequent seek operations.

With that change, the following code also works before and after this PR, helping backwards compatibility.

from PIL import Image
im = Image.new("RGB", (1, 1))
im2 = Image.new("RGB", (1, 1), "#f00")
im.save("out.gif", save_all=True, append_images=[im2], comment=b"comment")

reloaded = Image.open("out.gif")
reloaded.seek(1)
assert reloaded.info["comment"] == b"comment"

@radarhere radarhere merged commit 06b0e64 into python-pillow:main May 23, 2022
@raygard raygard deleted the comment_correct_placement branch May 24, 2022 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GIF comment blocks should be placed near front of file, once
2 participants