diff --git a/Tests/test_file_apng.py b/Tests/test_file_apng.py index 15689476c91..93e2c894e5b 100644 --- a/Tests/test_file_apng.py +++ b/Tests/test_file_apng.py @@ -559,8 +559,63 @@ def test_apng_save_large_duration(tmp_path: Path) -> None: test_file = tmp_path / "temp.png" im = Image.new("1", (1, 1)) im2 = Image.new("1", (1, 1), 1) - with pytest.raises(ValueError, match="cannot write duration"): - im.save(test_file, save_all=True, append_images=[im2], duration=65536000) + # A duration too large for a single fcTL delay is split across + # consecutive identical frames (#10063) + im.save(test_file, save_all=True, append_images=[im2], duration=65536000) + with Image.open(test_file) as reloaded: + assert isinstance(reloaded, PngImagePlugin.PngImageFile) + durations = [] + for i in range(reloaded.n_frames): + reloaded.seek(i) + durations.append(reloaded.info["duration"]) + assert sum(durations) == 2 * 65536000 + + +def test_apng_save_split_duration_from_merged_frames(tmp_path: Path) -> None: + # From the issue: two identical frames merge into one, and the merged + # duration (32768 + 32769 = 65537 ms) no longer fits a single fcTL delay + test_file = tmp_path / "temp.png" + frame_a = Image.new("RGB", (1, 1), "green") + frame_b = Image.new("RGB", (1, 1), "red") + frame_a.save( + test_file, + save_all=True, + append_images=[frame_b, frame_b], + duration=[100, 32768, 32769], + ) + with Image.open(test_file) as reloaded: + assert isinstance(reloaded, PngImagePlugin.PngImageFile) + durations = [] + for i in range(reloaded.n_frames): + reloaded.seek(i) + durations.append(reloaded.info["duration"]) + assert durations == [100.0, 65535.0, 2.0] + + +def test_apng_save_split_duration_exact_remainder(tmp_path: Path) -> None: + test_file = tmp_path / "temp.png" + im = Image.new("1", (1, 1)) + im2 = Image.new("1", (1, 1), 1) + im.save(test_file, save_all=True, append_images=[im2], duration=[100, 65537]) + with Image.open(test_file) as reloaded: + assert isinstance(reloaded, PngImagePlugin.PngImageFile) + durations = [] + for i in range(reloaded.n_frames): + reloaded.seek(i) + durations.append(reloaded.info["duration"]) + assert durations == [100.0, 65535.0, 2.0] + + +def test_apng_save_duration_fits_without_split(tmp_path: Path) -> None: + test_file = tmp_path / "temp.png" + im = Image.new("1", (1, 1)) + im2 = Image.new("1", (1, 1), 1) + im.save(test_file, save_all=True, append_images=[im2], duration=[100, 70000]) + with Image.open(test_file) as reloaded: + assert isinstance(reloaded, PngImagePlugin.PngImageFile) + assert reloaded.n_frames == 2 + reloaded.seek(1) + assert reloaded.info["duration"] == 70000.0 def test_apng_save_disposal(tmp_path: Path) -> None: diff --git a/docs/releasenotes/13.0.0.rst b/docs/releasenotes/13.0.0.rst index 5ec4c3d1485..5186e6e4444 100644 --- a/docs/releasenotes/13.0.0.rst +++ b/docs/releasenotes/13.0.0.rst @@ -134,6 +134,15 @@ escape sequences. Other changes ============= +Split APNG frame durations that are too large for a single fcTL chunk +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +When saving an APNG, a frame duration whose fractional delay does not fit the +16-bit numerator of a fcTL chunk used to raise ``ValueError: cannot write +duration``. The duration is now split across consecutive identical frames, +each with a delay that fits, so that the animation still replays the full +duration (#10063). + Clean up TIFF encoders before closing their output files ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/PIL/PngImagePlugin.py b/src/PIL/PngImagePlugin.py index 56f64f927e4..4077520251e 100644 --- a/src/PIL/PngImagePlugin.py +++ b/src/PIL/PngImagePlugin.py @@ -1193,6 +1193,25 @@ class _Frame(NamedTuple): encoderinfo: dict[str, Any] +def _apng_frame_delays(frame_duration: float) -> list[Fraction]: + """Split a frame duration into delays that fit a fcTL chunk. + + The delay numerator of a fcTL chunk is 16 bits, so a duration whose + fractional delay has a numerator over 65535 cannot be written as a + single chunk. Such a duration is split into consecutive delays, each + fitting the limit, so that consecutive identical frames replay the + full duration (#10063). + """ + delay = Fraction(frame_duration / 1000).limit_denominator(65535) + if delay.numerator <= 65535: + return [delay] + whole, remainder = divmod(delay.numerator, 65535) + delays = [Fraction(65535, delay.denominator)] * whole + if remainder: + delays.append(Fraction(remainder, delay.denominator)) + return delays + + def _write_multiple_frames( im: Image.Image, fp: IO[bytes], @@ -1274,8 +1293,16 @@ def _write_multiple_frames( chunk( fp, b"acTL", - o32(len(im_frames)), # 0: num_frames - o32(loop), # 4: num_plays + # 0: num_frames. A duration too large for a single fcTL delay is + # written as several consecutive identical frames, so count the + # pieces, not just the collected frames. + o32( + sum( + len(_apng_frame_delays(frame_data.encoderinfo.get("duration", 0))) + for frame_data in im_frames + ) + ), # 4: num_plays + o32(loop), ) # default image IDAT (if it exists) @@ -1299,44 +1326,42 @@ def _write_multiple_frames( size = im_frame.size encoderinfo = frame_data.encoderinfo frame_duration = encoderinfo.get("duration", 0) - delay = Fraction(frame_duration / 1000).limit_denominator(65535) - if delay.numerator > 65535: - msg = "cannot write duration" - raise ValueError(msg) + delays = _apng_frame_delays(frame_duration) frame_disposal = encoderinfo.get("disposal", disposal) frame_blend = encoderinfo.get("blend", blend) - # frame control - chunk( - fp, - b"fcTL", - o32(seq_num), # sequence_number - o32(size[0]), # width - o32(size[1]), # height - o32(bbox[0]), # x_offset - o32(bbox[1]), # y_offset - o16(delay.numerator), # delay_numerator - o16(delay.denominator), # delay_denominator - o8(frame_disposal), # dispose_op - o8(frame_blend), # blend_op - ) - seq_num += 1 - # frame data - _apply_encoderinfo(im_frame, im.encoderinfo) - if frame == 0 and not default_image: - # first frame must be in IDAT chunks for backwards compatibility - ImageFile._save( - im_frame, - cast("IO[bytes]", _idat(fp, chunk)), - [ImageFile._Tile("zip", (0, 0, *im_frame.size), 0, rawmode)], - ) - else: - fdat_chunks = _fdat(fp, chunk, seq_num) - ImageFile._save( - im_frame, - cast("IO[bytes]", fdat_chunks), - [ImageFile._Tile("zip", (0, 0, *im_frame.size), 0, rawmode)], + for delay_index, delay in enumerate(delays): + # frame control + chunk( + fp, + b"fcTL", + o32(seq_num), # sequence_number + o32(size[0]), # width + o32(size[1]), # height + o32(bbox[0]), # x_offset + o32(bbox[1]), # y_offset + o16(delay.numerator), # delay_numerator + o16(delay.denominator), # delay_denominator + o8(frame_disposal), # dispose_op + o8(frame_blend), # blend_op ) - seq_num = fdat_chunks.seq_num + seq_num += 1 + # frame data + _apply_encoderinfo(im_frame, im.encoderinfo) + if frame == 0 and delay_index == 0 and not default_image: + # first frame must be in IDAT chunks for backwards compatibility + ImageFile._save( + im_frame, + cast("IO[bytes]", _idat(fp, chunk)), + [ImageFile._Tile("zip", (0, 0, *im_frame.size), 0, rawmode)], + ) + else: + fdat_chunks = _fdat(fp, chunk, seq_num) + ImageFile._save( + im_frame, + cast("IO[bytes]", fdat_chunks), + [ImageFile._Tile("zip", (0, 0, *im_frame.size), 0, rawmode)], + ) + seq_num = fdat_chunks.seq_num return None