Skip to content

Fix MPO index placement after Adobe JPEG headers - #10328

Open
Muguett-DBY wants to merge 8 commits into
python-pillow:mainfrom
Muguett-DBY:fix/mpo-adobe-header-offset
Open

Muguett-DBY wants to merge 8 commits into
python-pillow:mainfrom
Muguett-DBY:fix/mpo-adobe-header-offset

Conversation

@Muguett-DBY

@Muguett-DBY Muguett-DBY commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

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.

The writer now locates the MP Index marker in the encoded JPEG rather than calculating its position from marker lengths. It captures the 20-byte search window as the first frame is written, so the output does not need to be readable. The change adds CMYK and write-only-file regression tests and a release note. Related: #10326.

The capture forwards encoded data through Python's file-object path instead of the direct file-descriptor path for the first frame. In a small Windows disk check using the existing Pillow 13.0 codec, a 3000×2000 two-frame save took a median of 73.5 ms before and 73.3 ms after (six paired samples); this is only an indicative local measurement.

Validation:

  • The three new write-only regression cases fail with UnsupportedOperation: read on 7fc5ff8 and pass on 7b456b0. With the exact changed writer and Pillow 13.0's existing codec and reader, 288 mode, EXIF, DPI and stream combinations passed independent JPEG-marker, index and pixel checks; another 60 multi-frame/metadata cases passed.
  • These local checks exercised the writer and regression test body, not a full branch build or pytest suite. The bundled Pillow 13.0 selftest passed 59 examples.
  • At the current head, all 52 GitHub check runs and both external statuses passed, including lint, tests, coverage, docs and benchmark. Earlier CI runs found two test-style errors and then a test type-narrowing error; both were addressed in follow-up commits.

Investigation, implementation and checks assisted by Codex.

@radarhere radarhere added the 🤖-assisted AI-assisted label Oct 27, 2026
@radarhere

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor Author

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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖-assisted AI-assisted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants