[unvisible789]

Fixes #10331.

Changes proposed in this pull request:

• When a JPEG has no JFIF density and its DPI comes from EXIF, read YResolution (0x011B) for the vertical axis instead of reusing XResolution. If YResolution is absent, the previous behaviour (X for both axes) is kept, as are the fallbacks to (72, 72) for invalid values.
• Add test_dpi_from_exif_different_axes (the reproduction from the issue: XResolution 300, YResolution 150 -> (300, 150)).

Testing: I couldn't build the C extensions locally (no system image-library headers), so I applied the change to the 13.0.0 tag and ran it with the official 13.0.0 compiled extensions. JpegImagePlugin.py is the same there as in main apart from this change. Tests/test_file_jpeg.py, test_file_tiff.py and test_file_libtiff.py all pass, and the new test fails without the fix ((300.0, 300.0)). ruff and black pass on the changed files.

Disclosure: this change was prepared by an AI assistant on behalf of @unvisible789.

[Muguett-DBY]

The separate X/Y values work in my checks, including the cm-to-inch conversion. I noticed one compatibility case worth deciding on: with a valid XResolution of 300 and an invalid YResolution of IFDRational(0, 0), the existing code returns (300, 300), while this change returns (72, 72). The new Y lookup sends both axes through the fallback, so a broken Y tag also discards the usable X value.

Would it make sense to fall back to X for an invalid Y, as for a missing Y? Either way, a test for that case would make the behavior explicit.

I checked the exact base and PR versions of _read_dpi_from_exif using generated JPEGs and the existing Pillow 12.3 codec; I did not run the full branch test suite.

[unvisible789]

Good catch, thanks. An invalid YResolution now falls back to the X value, the same as a missing one, so your example gives (300, 300) again. I added a test for that case.

[OKAN-SOFTWARE]

I independently reproduced #10331 against Pillow 13.1.0.dev0 and ran additional regression cases.

Two cases worth including in the test coverage:

• EXIF XResolution=300, YResolution=150, ResolutionUnit=3 (centimeters): expected DPI (762.0, 381.0).
• EXIF XResolution=300, missing YResolution, ResolutionUnit=2: expected DPI (300.0, 300.0) for backward compatibility.

My local JPEG test suite passed (109 passed, 2 skipped). I also noticed your latest revision preserves a valid XResolution when YResolution is invalid, which is a useful compatibility safeguard.

These extra cases may help protect against future regressions.

[unvisible789]

Thanks for testing this and for the extra cases. I'll leave it to the maintainers whether they want more tests here, and I'm happy to add those two if so.

[Muguett-DBY]

Since #10353 landed, this branch now conflicts with the reader change. For the reconciliation, I tried keeping your X/Y logic and changing the unit lookup to:

resolution_unit = exif.get(0x0128)

That preserves main's new behaviour when the unit is missing: EXIF X=96 and Y=150 reads as (96, 150), rather than falling back to (72, 72). I used Codex to check 36 serialized EXIF combinations with missing, inch and centimetre units and valid, missing or invalid X/Y values. The check used the exact reader with that one-line adjustment and the existing Pillow 13.0 codec; it wasn't a full branch build or test suite. Sharing in case it's useful when updating the branch.

[unvisible789]

Thanks @Muguett-DBY, that's the right reconciliation. @radarhere has since merged main into this branch, and the resolved reader already uses exif.get(0x0128) for the unit, so a missing ResolutionUnit now keeps the X/Y values instead of falling back to 72. All checks are passing on the merged branch. Thanks @radarhere for updating it.

[radarhere]

Investigating the history of this, I found that the Y Resolution was deliberately ignored. @hugovk said

http://www.sno.phy.queensu.ca/~phil/exiftool/TagNames/EXIF.html says:

• Exif.Image.XResolution: "The number of pixels per in the direction. When the image resolution is unknown, 72 [dpi] is designated."
• Exif.Image.YResolution: "The number of pixels per in the direction. The same value as is designated."
  ...

we don't need to check for YResolution, it's the same as XResolution

[OKAN-SOFTWARE]

Glad to see the upstream compatibility issue resolved and the checks passing.

I also wanted to follow up on the two additional regression cases I independently verified and shared earlier:

• Centimetre units with different X/Y resolutions: expected (762, 381).
• Missing YResolution with inch units: expected (300, 300).

My local JPEG suite passed with 109 tests and 2 skips. These cases aren't currently included in the PR tests, and I think they're worth covering to protect the compatibility behavior.

Happy to share the test implementations if useful.

[radarhere]

@OKAN-SOFTWARE good work on checking this, but with the number of PRs coming in at the moment generated by AI, I'm concerned about the test suite becoming bloated. I would rather restrict new tests to individual scenarios that fail without the change, or tests that increase coverage.

[radarhere]

I initially thought that different values should be allowed, because the TIFF specification said so, but I'm backtracking, as I think the EXIF "tags were inherited from the TIFF 6.0 standard", and EXIF has more specific rules.

https://www.cipa.jp/std/documents/e/DC-X008-Translation-2019-E.pdf describes YResolution as

The number of pixels per ResolutionUnit in the ImageLength direction. The same value as XResolution shall be designated.

I'm having a hard time find anything that clearly says otherwise. If anyone here can find something, or a real image that demonstrates different settings, please let me know.