Repository navigation
Read JPEG EXIF YResolution for the vertical DPI - #10334
unvisible789 wants to merge 2 commits into
Conversation
Muguett-DBY
left a comment
There was a problem hiding this comment.
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.
|
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. |
Fixes #10331.
Changes proposed in this pull request:
YResolution(0x011B) for the vertical axis instead of reusingXResolution. IfYResolutionis absent, the previous behaviour (X for both axes) is kept, as are the fallbacks to(72, 72)for invalid values.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.pyis the same there as inmainapart from this change.Tests/test_file_jpeg.py,test_file_tiff.pyandtest_file_libtiff.pyall pass, and the new test fails without the fix ((300.0, 300.0)).ruffandblackpass on the changed files.Disclosure: this change was prepared by an AI assistant on behalf of @unvisible789.