[Muguett-DBY]

Fixes #10344. Draft: the non-square DPI case needs a decision before this is ready.

Saving an RGB JPEG with keep_rgb=True and DPI currently writes both JFIF and Adobe markers; libjpeg then decodes the RGB components as YCbCr. This was found through a Codex-assisted source investigation.

The branch builds on @radarhere's writer-only approach, using EXIF for RGB density and retaining JFIF for other modes. It copies supplied EXIF before updating it and keeps the requested inch unit. The reader change in #10353 is included.

There is an unresolved issue in the current head: it writes different EXIF X/Y resolutions when the requested DPI differs. As discussed in #10334, the CIPA EXIF specification calls for those values to match, and Pillow's current reader reports X for both. The change therefore cannot yet preserve non-square DPI correctly. The PR is a draft while that trade-off is resolved.

At 9a20ff03, all 49 GitHub check runs and both external statuses passed. The focused local tests used the exact Python writer and reader with an existing Pillow 13.0 codec; they do not establish correctness for the unresolved case above. Investigation, implementation and checks assisted by Codex.

[radarhere]

I find the conditions for using RGB very complex.

What if instead, when keep_rgb and DPI are used together, we abandon the JFIF header, and store the DPI in EXIF? Muguett-DBY#2

[Muguett-DBY]

Thanks, avoiding the conflicting JFIF marker when writing is a much simpler approach. I tried the proposed commit and found a DPI case the 72-DPI test seems to miss:

from io import BytesIO
from PIL import Image

out = BytesIO()
Image.new("RGB", (20, 20), "red").save(out, "JPEG", keep_rgb=True, dpi=(96, 96))
out.seek(0)
with Image.open(out) as im:
    print(im.getpixel((0, 0)), im.info["dpi"])  # (255, 0, 0), (72, 72)

_read_dpi_from_exif() falls back to 72 because the new EXIF data has no ResolutionUnit. The proposal sets only XResolution; for non-square DPI, YResolution would also need to be saved and read. Passing an Image.Exif object also mutates the caller's object. I checked the proposed Python writer with the existing Pillow 13.0 codec and reader, using an adapter for _Tile; this was not a full branch build.

One trade-off to consider: a writer-only change would prevent new files from having both markers, but files already saved by older Pillow versions would still decode with the wrong colors. What do you think about handling those existing files?

Checked with Codex.

[radarhere]

_read_dpi_from_exif() falls back to 72 because the new EXIF data has no ResolutionUnit

I've created #10353

for non-square DPI, YResolution would also need to be saved and read

I think this would be better addressed in #10334

Passing an Image.Exif object also mutates the caller's object

I've pushed a commit for this.

a writer-only change would prevent new files from having both markers, but files already saved by older Pillow versions would still decode with the wrong colors. What do you think about handling those existing files?

If Pillow saved those images incorrectly, then they have incorrect colours. keep_rgb was added in Pillow 10.2.0 without any reports until now. I don't think this is in heavy use. Legitimate images created by other software might also have those same settings, yes? We shouldn't interpret their data incorrectly because of this.

[Muguett-DBY]

Thanks, the writer-only approach makes sense. Avoiding this marker combination when saving is preferable to changing how other software's files are decoded. The current branch now reflects your replacement, and I've updated the PR description accordingly.

I checked 94de18b8 with the reader change from #10353: a new RGB JPEG saved at 96 DPI reopens with the original colors and (96, 96). The supplied Image.Exif object also stays unchanged and its description survives. With the current reader alone, the no-existing-EXIF case still falls back to (72, 72), so this depends on #10353. These were exact Python-function checks with the existing Pillow 13.0 codec, not a full branch build.

I'll leave non-square DPI to #10334 as you suggest.

[Muguett-DBY]

Thanks for merging #10353 and updating the branch. While checking the new head, I found a case the square-DPI test misses:

from io import BytesIO
from PIL import Image

exif = Image.Exif()
exif[0x011A] = 300
exif[0x011B] = 200
exif[0x0128] = 3  # resolution in centimetres
out = BytesIO()
Image.new("RGB", (12, 12)).save(out, "JPEG", keep_rgb=True, dpi=(96, 96), exif=exif)
with Image.open(BytesIO(out.getvalue())) as im:
    print(im.info["dpi"])  # (243.84, 243.84), not (96, 96)

The new XResolution is in inches, but the copied ResolutionUnit remains centimetres. Assigning the entire DPI tuple to XResolution also emits a “too many entries” warning and discards its second value. For a new file, that leaves no YResolution for #10334 to read; with supplied EXIF it can leave an older YResolution in place.

There is a related change for non-RGB modes: with Image.new("L", ...), keep_rgb=True and dpi=(96, 150), main reopens at (96, 150) but this head returns (96, 96). The encoder ignores keep_rgb for L, YCbCr and CMYK, so those modes can retain their JFIF density.

I checked the exact writer and reader at a4f2c0d against the existing Pillow 13.0 codec, with an _Tile compatibility adapter; this was not a full branch build. I'll check a change that stores both axes with an inch unit for RGB and keeps the existing path for the other modes.

[Muguett-DBY]

Thanks for checking the EXIF specification in #10334. I missed that requirement when I added separate X/Y tags here: the current head writes unequal EXIF values for non-square DPI, and Pillow still reads X for both axes. I've moved this PR back to draft.

For RGB JPEGs, JFIF density preserves two axes but reintroduces the conflicting colour markers; EXIF avoids that conflict but specifies matching axes. Would rejecting unequal DPI for keep_rgb=True be acceptable, or is there another representation you would prefer? I'll keep the change scoped to the original colour issue and trim the extra tests accordingly.

[radarhere]

Why not leave the ResolutionUnit unset?

[radarhere]

Would rejecting unequal DPI for keep_rgb=True be acceptable

That sounds fine to me.