[Muguett-DBY]

Saving a CMYK MPO, or an RGB MPO with keep_rgb=True, can place the MP Index at the wrong offset. Pillow then reopens it as a single JPEG instead of exposing the other frames. This was found through a Codex-assisted source investigation, rather than an affected user's file.

The writer buffers the encoded first frame, finds its MP Index placeholder and writes the index there. This is @radarhere's suggested approach. It supports write-only destinations and holds the compressed first frame in memory. The current branch includes a CMYK round-trip regression. Related: #10326.

Following review, the constructed EXIF collision case and its extra handling were removed. @radarhere subsequently removed the extra write-only test and release-note entry in f1b49543; those have not been restored.

Validation:

• At 58f4307b0, the exact Python writer passed the CMYK regression and three write-only regression cases, plus 72 combinations of mode, keep_rgb, ordinary EXIF metadata and DPI. Decoded frames matched separately encoded JPEGs. The writer's file is byte-for-byte unchanged at f1b49543; the write-only test is no longer part of the submitted test suite.
• These checks used the existing Pillow 13.0 codec and reader, not a full branch build or pytest suite. The existing runtime also passed the 59-example selftest after its three required tracked fixtures were included in the sparse checkout; the initial attempt without those fixtures failed.
• At f1b49543, all 51 GitHub check contexts passed. Earlier 58f4307b0 runs had a macOS action-download DNS failure and a Windows Python 3.15 job that reached its 45-minute limit while installing Ghostscript, before Pillow built or tested. Those failures remain in the history and were not treated as successful tests.

Investigation, implementation and checks assisted by Codex.

[radarhere]

libjpeg writes a 16-byte Adobe APP14 header for these encodings, compared with the 18-byte JFIF APP0 header assumed by the writer

Can you link to the part of the libjpeg-turbo source code where it determines which header to use?

[Muguett-DBY]

Yes — the default selection is in libjpeg-turbo's jpeg_default_colorspace: normal RGB input selects YCbCr, while CMYK stays CMYK. jpeg_set_colorspace enables JFIF for grayscale/YCbCr and Adobe for RGB/CMYK. The marker writer emits those flags in that order.

On Pillow's side, JpegEncode.c selects the input color space and keep_rgb calls jpeg_set_colorspace(..., JCS_RGB).

While checking this I found an omission in my patch: Pillow also enables JFIF when both rounded DPI values are positive, so CMYK and keep_rgb can have both markers. The old PR head corrupted the index with dpi=(72, 72). I've pushed 3627a43 to account for that and extended the tests; the 12 affected parameter combinations fail with the previous head and pass with the update. All 144 amended combinations pass locally with the existing Pillow 12.3.0 codec. New-head CI is pending.

[radarhere]

I think you'll agree that the conditions to determine the position are getting a little complicated.

How about just searching through the output instead? I've pushed a commit. See what you think.

[Muguett-DBY]

Thanks, the search is much simpler. I found one compatibility issue with reading the output, though: callers can pass a write-only binary file to Image.save.

from PIL import Image

first = Image.new("RGB", (16, 8))
second = Image.new("RGB", (24, 16))
with open("out.mpo", "wb") as f:
    first.save(f, "MPO", save_all=True, append_images=[second])

With the new writer this raises io.UnsupportedOperation: read. The Image.save contract requires seek, tell and write, but not read. I exercised the exact writers with the existing Pillow 13.0 codec: 3627a43 saves this successfully, while 7fc5ff8 raises. Could we locate the marker while writing, or handle write-only outputs separately?

[Muguett-DBY]

I pushed ab7ad7e to capture the 20-byte search window as the first JPEG is written. That keeps the marker search and lets write-only files work. I added three cases that fail on the previous head and pass with the change.

One trade-off: the first frame now uses the Python file-object encoding path instead of the direct file-descriptor path. I checked 288 mode/EXIF/DPI/stream combinations and 60 multi-frame/metadata cases with the exact writer and the existing Pillow 13.0 codec; the full branch CI is running.

[radarhere]

I've created #10328 with my next suggestion.

[Muguett-DBY]

Thanks, buffering the first frame makes the write-only case simpler. I found one edge case in the proposed commit: index(extra) can match bytes inside an earlier EXIF value instead of the actual MPF marker. For example:

from io import BytesIO
import struct
from PIL import Image

ifd_length = 66 + 16 * 2
placeholder = b"\xff\xe2" + struct.pack(">H", 6 + ifd_length) + b"MPF\0" + b" " * ifd_length
exif = Image.Exif()
exif[0x9286] = b"ASCII\0\0\0" + placeholder
out = BytesIO()
Image.new("RGB", (16, 8)).save(
    out, "MPO", save_all=True, append_images=[Image.new("RGB", (24, 16))],
    exif=exif.tobytes(),
)
out.seek(0)
with Image.open(out) as im:
    print(im.format, getattr(im, "n_frames", 1))  # JPEG 1, expected MPO 2

This EXIF value is deliberately constructed; ordinary EXIF worked in my checks. Could the search start after the EXIF segment, or walk the JPEG markers instead?

I checked the proposed _save_all with the existing Pillow 13.0 codec and reader, not a full branch build. Checked with Codex.

[Muguett-DBY]

Thanks for the buffering suggestion. I merged your commit and pushed 16eca93, with the search starting past the EXIF payload and the write-only tests retained. The new EXIF regression fails on the suggested head and passes with the update; three write-only cases and 288 input combinations also passed locally with the existing Pillow 13.0 codec. GitHub checks are running.

The first compressed frame is now held in memory, as in your suggestion.

[radarhere]

I found one edge case in Muguett-DBY#1: index(extra) can match bytes inside an earlier EXIF value instead of the actual MPF marker

I think this has crossed the line from 'edge case' into 'shooting yourself in the foot'. I don't like leaving a comment in the code that 'The EXIF payload can itself contain the placeholder bytes' because it gives the impression that this is a realistic concern.

This PR is only becoming more obscure over time. I'm not giving up, but I would like to ask - what motivated you to create this? Will it help you in any way? Or did AI just find a bug and you created the PR?

[Muguett-DBY]

Codex found this during a contribution search; it wasn't prompted by a failure in an application or an affected user's file. The CMYK/keep_rgb round-trip failure was reproducible, but I should have made that context clearer when opening the PR.

You're right about the EXIF example. I constructed the payload specifically to match the placeholder, then gave it too much weight in the implementation. I've removed the guard, comment and that test in 58f4307. Your buffering approach remains, along with the CMYK and write-only regression tests.

Thanks for the time you've put into reviewing and simplifying this.