From 22d9095e5c74217331f6552d06ac3fef3dea0bae Mon Sep 17 00:00:00 2001 From: Ray Gardner Date: Fri, 13 May 2022 12:45:01 -0600 Subject: [PATCH 1/5] Correct placement of GIF comment Place GIF comment after Global Color table. Should go after "NETSCAPE" looping extension after pull #6211. --- Tests/test_file_gif.py | 23 +++++++++++++++++++++++ src/PIL/GifImagePlugin.py | 23 +++++++++++++---------- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/Tests/test_file_gif.py b/Tests/test_file_gif.py index 3c2fab722..1bd772dc6 100644 --- a/Tests/test_file_gif.py +++ b/Tests/test_file_gif.py @@ -813,6 +813,29 @@ def test_zero_comment_subblocks(): assert_image_equal_tofile(im, TEST_GIF) +def test_write_comment(tmp_path): + out = str(tmp_path / "temp.gif") + with Image.open("Tests/images/multiple_comments.gif") as im: + im.save(out, save_all=True, comment="Test") + with Image.open(out) as reread: + # Comments written should appear only in first frame + assert reread.info["comment"] == b"Test" + for i, frame in enumerate(ImageSequence.Iterator(reread)): + assert (i == 0 and frame.info["comment"] == b"Test" or + i != 0 and "comment" not in frame.info) + + +def test_write_no_comment(tmp_path): + out = str(tmp_path / "temp.gif") + with Image.open("Tests/images/multiple_comments.gif") as im: + # Empty comment="" arg should suppress all comments + im.save(out, save_all=True, comment="") + with Image.open(out) as reread: + assert "comment" not in reread.info + for frame in ImageSequence.Iterator(reread): + assert "comment" not in frame.info + + def test_version(tmp_path): out = str(tmp_path / "temp.gif") diff --git a/src/PIL/GifImagePlugin.py b/src/PIL/GifImagePlugin.py index 9b34a3b0e..4d785d834 100644 --- a/src/PIL/GifImagePlugin.py +++ b/src/PIL/GifImagePlugin.py @@ -715,15 +715,6 @@ def _write_local_header(fp, im, offset, flags): + o8(0) ) - if "comment" in im.encoderinfo and 1 <= len(im.encoderinfo["comment"]): - fp.write(b"!" + o8(254)) # extension intro - comment = im.encoderinfo["comment"] - if isinstance(comment, str): - comment = comment.encode() - for i in range(0, len(comment), 255): - subblock = comment[i : i + 255] - fp.write(o8(len(subblock)) + subblock) - fp.write(o8(0)) if "loop" in im.encoderinfo: number_of_loops = im.encoderinfo["loop"] fp.write( @@ -929,7 +920,7 @@ def _get_global_header(im, info): palette_bytes = _get_palette_bytes(im) color_table_size = _get_color_table_size(palette_bytes) - return [ + header = [ b"GIF" # signature + version # version + o16(im.size[0]) # canvas width @@ -943,6 +934,18 @@ def _get_global_header(im, info): _get_header_palette(palette_bytes), ] + if "comment" in info and len(info["comment"]): + comment = info["comment"] + if isinstance(comment, str): + comment = comment.encode() + header.append(b"!" + o8(254)) # extension intro + for i in range(0, len(comment), 255): + subblock = comment[i : i + 255] + header.append(o8(len(subblock)) + subblock) + header.append(o8(0)) + + return header + def _write_frame_data(fp, im_frame, offset, params): try: From 416de882e418d64ff3e058903cd4fafabcff0d52 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 14 May 2022 15:36:51 +0000 Subject: [PATCH 2/5] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- Tests/test_file_gif.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Tests/test_file_gif.py b/Tests/test_file_gif.py index 1bd772dc6..eee432116 100644 --- a/Tests/test_file_gif.py +++ b/Tests/test_file_gif.py @@ -821,8 +821,12 @@ def test_write_comment(tmp_path): # Comments written should appear only in first frame assert reread.info["comment"] == b"Test" for i, frame in enumerate(ImageSequence.Iterator(reread)): - assert (i == 0 and frame.info["comment"] == b"Test" or - i != 0 and "comment" not in frame.info) + assert ( + i == 0 + and frame.info["comment"] == b"Test" + or i != 0 + and "comment" not in frame.info + ) def test_write_no_comment(tmp_path): From 6257e788adac28f004313816b7a15b677754947b Mon Sep 17 00:00:00 2001 From: Ray Gardner Date: Sat, 14 May 2022 11:57:41 -0600 Subject: [PATCH 3/5] Update test_file_gif.py Changed to use a test image already in Images folder --- Tests/test_file_gif.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/test_file_gif.py b/Tests/test_file_gif.py index eee432116..762dab8df 100644 --- a/Tests/test_file_gif.py +++ b/Tests/test_file_gif.py @@ -815,7 +815,7 @@ def test_zero_comment_subblocks(): def test_write_comment(tmp_path): out = str(tmp_path / "temp.gif") - with Image.open("Tests/images/multiple_comments.gif") as im: + with Image.open("Tests/images/dispose_prev.gif") as im: im.save(out, save_all=True, comment="Test") with Image.open(out) as reread: # Comments written should appear only in first frame @@ -831,7 +831,7 @@ def test_write_comment(tmp_path): def test_write_no_comment(tmp_path): out = str(tmp_path / "temp.gif") - with Image.open("Tests/images/multiple_comments.gif") as im: + with Image.open("Tests/images/dispose_prev.gif") as im: # Empty comment="" arg should suppress all comments im.save(out, save_all=True, comment="") with Image.open(out) as reread: From 67f5e5d272c95e6e8f61547dba022fb2d3c4daa8 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sat, 21 May 2022 23:29:55 +1000 Subject: [PATCH 4/5] Test an empty string comment in arguments removes existing comment --- Tests/test_file_gif.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/Tests/test_file_gif.py b/Tests/test_file_gif.py index 3f7b7aeb9..268a2349d 100644 --- a/Tests/test_file_gif.py +++ b/Tests/test_file_gif.py @@ -853,15 +853,17 @@ def test_write_comment(tmp_path): ) -def test_write_no_comment(tmp_path): +def test_empty_string_comment(tmp_path): out = str(tmp_path / "temp.gif") - with Image.open("Tests/images/dispose_prev.gif") as im: - # Empty comment="" arg should suppress all comments + with Image.open("Tests/images/chi.gif") as im: + assert "comment" in im.info + + # Empty string comment should suppress existing comment im.save(out, save_all=True, comment="") - with Image.open(out) as reread: - assert "comment" not in reread.info - for frame in ImageSequence.Iterator(reread): - assert "comment" not in frame.info + + with Image.open(out) as reread: + for frame in ImageSequence.Iterator(reread): + assert "comment" not in frame.info def test_version(tmp_path): From 62d0f0e38d8a4395b33627b24454da384b78ca77 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sun, 22 May 2022 15:30:16 +1000 Subject: [PATCH 5/5] Once comment is loaded, keep it for subsequent frames --- Tests/images/second_frame_comment.gif | Bin 0 -> 2400 bytes Tests/test_file_gif.py | 47 +++++++++++++++++--------- src/PIL/GifImagePlugin.py | 17 ++++++---- 3 files changed, 42 insertions(+), 22 deletions(-) create mode 100644 Tests/images/second_frame_comment.gif diff --git a/Tests/images/second_frame_comment.gif b/Tests/images/second_frame_comment.gif new file mode 100644 index 0000000000000000000000000000000000000000..c8fc957911e79693933aa77329561f557f0a6cee GIT binary patch literal 2400 zcmZ?wbh9u|WMp7uXlED&qaiS&LqG@Qdr)3r;9y~3WMN@Y{3q?4pPQSSSE7)ar%;lS zs!*JooS&DXkXDqKo5}!J56nA)BRb?pJwLKT0GeNrGlbH}4v|sckLVD9W(aEl_3RI! literal 0 HcmV?d00001 diff --git a/Tests/test_file_gif.py b/Tests/test_file_gif.py index 268a2349d..c4f634fae 100644 --- a/Tests/test_file_gif.py +++ b/Tests/test_file_gif.py @@ -837,22 +837,6 @@ def test_read_multiple_comment_blocks(): assert im.info["comment"] == b"Test comment 1\nTest comment 2" -def test_write_comment(tmp_path): - out = str(tmp_path / "temp.gif") - with Image.open("Tests/images/dispose_prev.gif") as im: - im.save(out, save_all=True, comment="Test") - with Image.open(out) as reread: - # Comments written should appear only in first frame - assert reread.info["comment"] == b"Test" - for i, frame in enumerate(ImageSequence.Iterator(reread)): - assert ( - i == 0 - and frame.info["comment"] == b"Test" - or i != 0 - and "comment" not in frame.info - ) - - def test_empty_string_comment(tmp_path): out = str(tmp_path / "temp.gif") with Image.open("Tests/images/chi.gif") as im: @@ -866,6 +850,37 @@ def test_empty_string_comment(tmp_path): assert "comment" not in frame.info +def test_retain_comment_in_subsequent_frames(tmp_path): + # Test that a comment block at the beginning is kept + with Image.open("Tests/images/chi.gif") as im: + for frame in ImageSequence.Iterator(im): + assert frame.info["comment"] == b"Created with GIMP" + + with Image.open("Tests/images/second_frame_comment.gif") as im: + assert "comment" not in im.info + + # Test that a comment in the middle is read + im.seek(1) + assert im.info["comment"] == b"Comment in the second frame" + + # Test that it is still present in a later frame + im.seek(2) + assert im.info["comment"] == b"Comment in the second frame" + + # Test that rewinding removes the comment + im.seek(0) + assert "comment" not in im.info + + # Test that a saved image keeps the comment + out = str(tmp_path / "temp.gif") + with Image.open("Tests/images/dispose_prev.gif") as im: + im.save(out, save_all=True, comment="Test") + + with Image.open(out) as reread: + for frame in ImageSequence.Iterator(reread): + assert frame.info["comment"] == b"Test" + + def test_version(tmp_path): out = str(tmp_path / "temp.gif") diff --git a/src/PIL/GifImagePlugin.py b/src/PIL/GifImagePlugin.py index f5ec610cb..c91c1fbff 100644 --- a/src/PIL/GifImagePlugin.py +++ b/src/PIL/GifImagePlugin.py @@ -163,6 +163,8 @@ class GifImageFile(ImageFile.ImageFile): self.__frame = -1 self._fp.seek(self.__rewind) self.disposal_method = 0 + if "comment" in self.info: + del self.info["comment"] else: # ensure that the previous frame was loaded if self.tile and update_image: @@ -230,7 +232,7 @@ class GifImageFile(ImageFile.ImageFile): # comment = b"" - # Collect one comment block + # Read this comment block while block: comment += block block = self.data() @@ -395,7 +397,9 @@ class GifImageFile(ImageFile.ImageFile): ) ] - for k in ["duration", "comment", "extension", "loop"]: + if info.get("comment"): + self.info["comment"] = info["comment"] + for k in ["duration", "extension", "loop"]: if k in info: self.info[k] = info[k] elif k in self.info: @@ -929,17 +933,18 @@ def _get_global_header(im, info): # Global Color Table _get_header_palette(palette_bytes), ] + if info.get("comment"): + comment_block = b"!" + o8(254) # extension intro - if "comment" in info and len(info["comment"]): comment = info["comment"] if isinstance(comment, str): comment = comment.encode() - header.append(b"!" + o8(254)) # extension intro for i in range(0, len(comment), 255): subblock = comment[i : i + 255] - header.append(o8(len(subblock)) + subblock) - header.append(o8(0)) + comment_block += o8(len(subblock)) + subblock + comment_block += o8(0) + header.append(comment_block) return header