Skip to content

Avoid conflicting JFIF and Adobe markers for RGB JPEGs - #10345

Open
Muguett-DBY wants to merge 7 commits into
python-pillow:mainfrom
Muguett-DBY:fix/jpeg-rgb-dpi
Open

Muguett-DBY wants to merge 7 commits into
python-pillow:mainfrom
Muguett-DBY:fix/jpeg-rgb-dpi

Conversation

@Muguett-DBY

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

Copy link
Copy Markdown
Contributor

Fixes #10344. The reader change in #10353 has been merged and included in this branch.

Saving RGB JPEGs with keep_rgb=True and DPI currently emits both JFIF and Adobe markers, which makes libjpeg decode the saved RGB components as YCbCr and changes the colors. This was found through a Codex-assisted source investigation.

The current implementation builds on @radarhere's replacement: for RGB images with keep_rgb=True, store both DPI axes and their inch unit in EXIF instead of adding JFIF density. It copies supplied EXIF data before updating it. Other image modes keep their existing JFIF handling. The reader change in #10353 is included in the branch.

Validation:

  • At 9a20ff03, 12 focused regression tests passed against the exact changed Python writer and reader using the existing Pillow 13.0 codec and an _Tile compatibility adapter. They cover supplied EXIF in centimetres, both stored axes, other image modes, zero density and invalid DPI lengths. The same tests failed on the preceding PR head. This is not a full current-source build or test suite.
  • Pillow's existing 13.0 self-test passed 59 checks. Ruff 0.17.2 and Black 26.10.1 passed on the two changed files; the repository pins slightly different versions.
  • CI for the new commit is running; not all checks have finished.

The writer stores non-square EXIF DPI correctly, but Pillow's current reader reports the X value for both axes. The reader fix in #10334 needs to land first to avoid a non-square DPI readback regression.

Investigation, implementation and checks assisted by Codex.

@radarhere

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Contributor Author

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

radarhere commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

_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 Muguett-DBY changed the title Fix RGB JPEG colors when DPI adds a JFIF marker Avoid conflicting JFIF and Adobe markers for RGB JPEGs Oct 29, 2026
@Muguett-DBY

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor Author

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.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JPEG saved with keep_rgb=True and dpi reopens with incorrect colors

2 participants