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

Added progress callback when save_all is used #7435

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
64 changes: 20 additions & 44 deletions Tests/test_file_apng.py
Expand Up @@ -691,50 +691,26 @@ def callback(state):
out, "PNG", save_all=True, append_images=[im, im2], progress=callback
)

assert progress == [
{
"image_index": 0,
"image_filename": "Tests/images/apng/single_frame.png",
"completed_frames": 1,
"total_frames": 7,
},
{
"image_index": 1,
"image_filename": "Tests/images/apng/single_frame.png",
"completed_frames": 2,
"total_frames": 7,
},
{
"image_index": 2,
"image_filename": "Tests/images/apng/delay.png",
"completed_frames": 3,
"total_frames": 7,
},
{
"image_index": 2,
"image_filename": "Tests/images/apng/delay.png",
"completed_frames": 4,
"total_frames": 7,
},
{
"image_index": 2,
"image_filename": "Tests/images/apng/delay.png",
"completed_frames": 5,
"total_frames": 7,
},
{
"image_index": 2,
"image_filename": "Tests/images/apng/delay.png",
"completed_frames": 6,
"total_frames": 7,
},
{
"image_index": 2,
"image_filename": "Tests/images/apng/delay.png",
"completed_frames": 7,
"total_frames": 7,
},
]
expected = []
for i in range(2):
expected.append(
{
"image_index": i,
"image_filename": "Tests/images/apng/single_frame.png",
"completed_frames": i + 1,
"total_frames": 7,
}
)
for i in range(5):
expected.append(
{
"image_index": 2,
"image_filename": "Tests/images/apng/delay.png",
"completed_frames": i + 3,
"total_frames": 7,
}
)
assert progress == expected


def test_seek_after_close():
Expand Down
40 changes: 14 additions & 26 deletions Tests/test_file_mpo.py
Expand Up @@ -304,29 +304,17 @@ def callback(state):
with Image.open("Tests/images/frozenpond.mpo") as im2:
im.save(out, "MPO", save_all=True, append_images=[im2], progress=callback)

assert progress == [
{
"image_index": 0,
"image_filename": "Tests/images/sugarshack.mpo",
"completed_frames": 1,
"total_frames": 4,
},
{
"image_index": 0,
"image_filename": "Tests/images/sugarshack.mpo",
"completed_frames": 2,
"total_frames": 4,
},
{
"image_index": 1,
"image_filename": "Tests/images/frozenpond.mpo",
"completed_frames": 3,
"total_frames": 4,
},
{
"image_index": 1,
"image_filename": "Tests/images/frozenpond.mpo",
"completed_frames": 4,
"total_frames": 4,
},
]
expected = []
for i, filename in enumerate(
["Tests/images/sugarshack.mpo", "Tests/images/frozenpond.mpo"]
):
for j in range(2):
expected.append(
{
"image_index": i,
"image_filename": filename,
"completed_frames": i * 2 + j + 1,
"total_frames": 4,
}
)
assert progress == expected
40 changes: 14 additions & 26 deletions Tests/test_file_pdf.py
Expand Up @@ -193,32 +193,20 @@ def callback(state):
with Image.open("Tests/images/frozenpond.mpo") as im2:
im.save(out, "PDF", save_all=True, append_images=[im2], progress=callback)

assert progress == [
{
"image_index": 0,
"image_filename": "Tests/images/sugarshack.mpo",
"completed_frames": 1,
"total_frames": 4,
},
{
"image_index": 0,
"image_filename": "Tests/images/sugarshack.mpo",
"completed_frames": 2,
"total_frames": 4,
},
{
"image_index": 1,
"image_filename": "Tests/images/frozenpond.mpo",
"completed_frames": 3,
"total_frames": 4,
},
{
"image_index": 1,
"image_filename": "Tests/images/frozenpond.mpo",
"completed_frames": 4,
"total_frames": 4,
},
]
expected = []
for i, filename in enumerate(
["Tests/images/sugarshack.mpo", "Tests/images/frozenpond.mpo"]
):
for j in range(2):
expected.append(
{
"image_index": i,
"image_filename": filename,
"completed_frames": i * 2 + j + 1,
"total_frames": 4,
}
)
assert progress == expected


def test_multiframe_normal_save(tmp_path):
Expand Down
32 changes: 12 additions & 20 deletions Tests/test_file_tiff.py
Expand Up @@ -714,32 +714,24 @@ def callback(state):
out, "TIFF", save_all=True, append_images=[im2], progress=callback
)

assert progress == [
expected = [
{
"image_index": 0,
"image_filename": "Tests/images/hopper.tif",
"completed_frames": 1,
"total_frames": 4,
},
{
"image_index": 1,
"image_filename": "Tests/images/multipage.tiff",
"completed_frames": 2,
"total_frames": 4,
},
{
"image_index": 1,
"image_filename": "Tests/images/multipage.tiff",
"completed_frames": 3,
"total_frames": 4,
},
{
"image_index": 1,
"image_filename": "Tests/images/multipage.tiff",
"completed_frames": 4,
"total_frames": 4,
},
}
]
for i in range(3):
expected.append(
{
"image_index": 1,
"image_filename": "Tests/images/multipage.tiff",
"completed_frames": i + 2,
"total_frames": 4,
}
)
assert progress == expected

def test_saving_icc_profile(self, tmp_path):
# Tests saving TIFF with icc_profile set.
Expand Down
24 changes: 5 additions & 19 deletions src/PIL/GifImagePlugin.py
Expand Up @@ -577,12 +577,12 @@ def _write_multiple_frames(im, fp, palette):
duration = im.encoderinfo.get("duration")
disposal = im.encoderinfo.get("disposal", im.info.get("disposal"))

progress = im.encoderinfo.get("progress")
imSequences = [im] + list(im.encoderinfo.get("append_images", []))
progress = im.encoderinfo.get("progress")
if progress:
n_frames = 0
total = 0
for imSequence in imSequences:
n_frames += getattr(imSequence, "n_frames", 1)
total += getattr(imSequence, "n_frames", 1)

im_frames = []
frame_count = 0
Expand Down Expand Up @@ -618,14 +618,7 @@ def _write_multiple_frames(im, fp, palette):
if encoderinfo.get("duration"):
previous["encoderinfo"]["duration"] += encoderinfo["duration"]
if progress:
progress(
{
"image_index": i,
"image_filename": getattr(imSequence, "filename", None),
"completed_frames": frame_count,
"total_frames": n_frames,
}
)
im._save_all_progress(imSequence, i, frame_count, total)
continue
if encoderinfo.get("disposal") == 2:
if background_im is None:
Expand All @@ -640,14 +633,7 @@ def _write_multiple_frames(im, fp, palette):
bbox = None
im_frames.append({"im": im_frame, "bbox": bbox, "encoderinfo": encoderinfo})
if progress:
progress(
{
"image_index": i,
"image_filename": getattr(imSequence, "filename", None),
"completed_frames": frame_count,
"total_frames": n_frames,
}
)
im._save_all_progress(imSequence, i, frame_count, total)

if len(im_frames) > 1:
for frame_data in im_frames:
Expand Down
17 changes: 17 additions & 0 deletions src/PIL/Image.py
Expand Up @@ -2446,6 +2446,23 @@ def save(self, fp, format=None, **params):
if open_fp:
fp.close()

def _save_all_progress(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's with "save all"? This method just passes data into another function (supplied externally).

If anything, _report_progress would make much more sense here. (Or even just _progress)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It indicates that the method is used in relation to im.save(out, save_all=True)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

…Ah. Alright, it does make sense in that context.

Though it would still make more sense to name it something like _frame_progress, I'd say; it's more generic and there's not really anything in the method itself that makes it necessarily about saving only.

self, im=None, im_index=0, completed=1, total=1, progress=None
):
if not progress:
progress = self.encoderinfo.get("progress")
if not progress:
return

progress(
{
"image_index": im_index,
"image_filename": getattr(im or self, "filename", None),
"completed_frames": completed,
"total_frames": total,
}
)

def seek(self, frame):
"""
Seeks to the given frame in this sequence file. If you seek
Expand Down
29 changes: 7 additions & 22 deletions src/PIL/MpoImagePlugin.py
Expand Up @@ -41,7 +41,6 @@ def _save(im, fp, filename):


def _save_all(im, fp, filename):
progress = im.encoderinfo.get("progress")
append_images = im.encoderinfo.get("append_images", [])
if not append_images:
try:
Expand All @@ -50,25 +49,18 @@ def _save_all(im, fp, filename):
animated = False
if not animated:
_save(im, fp, filename)
if progress:
progress(
{
"image_index": 0,
"image_filename": getattr(im, "filename", None),
"completed_frames": 1,
"total_frames": 1,
}
)
im._save_all_progress()
return

mpf_offset = 28
offsets = []
imSequences = [im] + list(append_images)
progress = im.encoderinfo.get("progress")
if progress:
frame_number = 0
n_frames = 0
completed = 0
total = 0
for imSequence in imSequences:
n_frames += getattr(imSequence, "n_frames", 1)
total += getattr(imSequence, "n_frames", 1)
for i, imSequence in enumerate(imSequences):
for im_frame in ImageSequence.Iterator(imSequence):
if not offsets:
Expand All @@ -89,15 +81,8 @@ def _save_all(im, fp, filename):
im_frame.save(fp, "JPEG")
offsets.append(fp.tell() - offsets[-1])
if progress:
frame_number += 1
progress(
{
"image_index": i,
"image_filename": getattr(imSequence, "filename", None),
"completed_frames": frame_number,
"total_frames": n_frames,
}
)
completed += 1
im._save_all_progress(imSequence, i, completed, total, progress)

ifd = TiffImagePlugin.ImageFileDirectory_v2()
ifd[0xB000] = b"0100"
Expand Down
11 changes: 1 addition & 10 deletions src/PIL/PdfImagePlugin.py
Expand Up @@ -246,7 +246,6 @@ def _save(im, fp, filename, save_all=False):
# catalog and list of pages
existing_pdf.write_catalog()

progress = im.encoderinfo.get("progress")
page_number = 0
for i, im_sequence in enumerate(ims):
im_pages = ImageSequence.Iterator(im_sequence) if save_all else [im_sequence]
Expand Down Expand Up @@ -282,15 +281,7 @@ def _save(im, fp, filename, save_all=False):
existing_pdf.write_obj(contents_refs[page_number], stream=page_contents)

page_number += 1
if progress:
progress(
{
"image_index": i,
"image_filename": getattr(im_sequence, "filename", None),
"completed_frames": page_number,
"total_frames": number_of_pages,
}
)
im._save_all_progress(im_sequence, i, page_number, number_of_pages)

#
# trailer
Expand Down