From bcc6e42bf82dfc962d49ed1876a946bd7be16b4f Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Tue, 27 May 2025 21:08:58 +1000 Subject: [PATCH 1/3] Fixed saving MPO with more than one appended image --- Tests/test_file_mpo.py | 18 ++++++++++-------- src/PIL/MpoImagePlugin.py | 13 ++++++++++--- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/Tests/test_file_mpo.py b/Tests/test_file_mpo.py index 73838ef44..adfa61962 100644 --- a/Tests/test_file_mpo.py +++ b/Tests/test_file_mpo.py @@ -293,16 +293,18 @@ def test_save_all() -> None: assert_image_similar(im, im_reloaded, 30) im = Image.new("RGB", (1, 1)) - im2 = Image.new("RGB", (1, 1), "#f00") - im_reloaded = roundtrip(im, save_all=True, append_images=[im2]) + for colors in (("#f00",), ("#f00", "#0f0")): + append_images = (Image.new("RGB", (1, 1), color) for color in colors) + im_reloaded = roundtrip(im, save_all=True, append_images=append_images) - assert_image_equal(im, im_reloaded) - assert isinstance(im_reloaded, MpoImagePlugin.MpoImageFile) - assert im_reloaded.mpinfo is not None - assert im_reloaded.mpinfo[45056] == b"0100" + assert_image_equal(im, im_reloaded) + assert isinstance(im_reloaded, MpoImagePlugin.MpoImageFile) + assert im_reloaded.mpinfo is not None + assert im_reloaded.mpinfo[45056] == b"0100" - im_reloaded.seek(1) - assert_image_similar(im2, im_reloaded, 1) + for im_expected in append_images: + im_reloaded.seek(im_reloaded.tell() + 1) + assert_image_similar(im_reloaded, im_expected, 1) # Test that a single frame image will not be saved as an MPO jpg = roundtrip(im, save_all=True) diff --git a/src/PIL/MpoImagePlugin.py b/src/PIL/MpoImagePlugin.py index f7393eac0..f96f658fc 100644 --- a/src/PIL/MpoImagePlugin.py +++ b/src/PIL/MpoImagePlugin.py @@ -19,7 +19,6 @@ # from __future__ import annotations -import itertools import os import struct from typing import IO, Any, cast @@ -47,12 +46,20 @@ def _save_all(im: Image.Image, fp: IO[bytes], filename: str | bytes) -> None: mpf_offset = 28 offsets: list[int] = [] - for imSequence in itertools.chain([im], append_images): + total = 0 + imSequences = [im] + list(append_images) + for imSequence in imSequences: + total += getattr(imSequence, "n_frames", 1) + for imSequence in imSequences: for im_frame in ImageSequence.Iterator(imSequence): if not offsets: # APP2 marker + ifd_length = 66 + 16 * total im_frame.encoderinfo["extra"] = ( - b"\xff\xe2" + struct.pack(">H", 6 + 82) + b"MPF\0" + b" " * 82 + b"\xff\xe2" + + struct.pack(">H", 6 + ifd_length) + + b"MPF\0" + + b" " * ifd_length ) exif = im_frame.encoderinfo.get("exif") if isinstance(exif, Image.Exif): From 41129ce1cb88ae6f7733b40f3f6a9f1a5662d2af Mon Sep 17 00:00:00 2001 From: Andrew Murray <3112309+radarhere@users.noreply.github.com> Date: Sat, 28 Jun 2025 01:20:02 +1000 Subject: [PATCH 2/3] Use list Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> --- Tests/test_file_mpo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/test_file_mpo.py b/Tests/test_file_mpo.py index adfa61962..840202214 100644 --- a/Tests/test_file_mpo.py +++ b/Tests/test_file_mpo.py @@ -294,7 +294,7 @@ def test_save_all() -> None: im = Image.new("RGB", (1, 1)) for colors in (("#f00",), ("#f00", "#0f0")): - append_images = (Image.new("RGB", (1, 1), color) for color in colors) + append_images = [Image.new("RGB", (1, 1), color) for color in colors] im_reloaded = roundtrip(im, save_all=True, append_images=append_images) assert_image_equal(im, im_reloaded) From 5732a86cc6f8aad4a28a92b7c7c78748000c8d1d Mon Sep 17 00:00:00 2001 From: Andrew Murray <3112309+radarhere@users.noreply.github.com> Date: Sat, 28 Jun 2025 10:52:25 +1000 Subject: [PATCH 3/3] Use snake case Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> --- src/PIL/MpoImagePlugin.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/PIL/MpoImagePlugin.py b/src/PIL/MpoImagePlugin.py index f96f658fc..784b6f208 100644 --- a/src/PIL/MpoImagePlugin.py +++ b/src/PIL/MpoImagePlugin.py @@ -46,12 +46,10 @@ def _save_all(im: Image.Image, fp: IO[bytes], filename: str | bytes) -> None: mpf_offset = 28 offsets: list[int] = [] - total = 0 - imSequences = [im] + list(append_images) - for imSequence in imSequences: - total += getattr(imSequence, "n_frames", 1) - for imSequence in imSequences: - for im_frame in ImageSequence.Iterator(imSequence): + im_sequences = [im, *append_images] + total = sum(getattr(seq, "n_frames", 1) for seq in im_sequences) + for im_sequence in im_sequences: + for im_frame in ImageSequence.Iterator(im_sequence): if not offsets: # APP2 marker ifd_length = 66 + 16 * total