feat: Improve EXIF dimension tags handling#96
feat: Improve EXIF dimension tags handling#96justinmayer merged 2 commits intopelican-plugins:mainfrom
Conversation
If a processed image has the ExifImageWidth or ExifImageHeight tags and the IMAGE_PROCESS_COPY_EXIF_TAGS setting is True, update the tags to the same value as ImageWidth or ImageHeight, respectively. Add tests to verify that: - an image without ExifImageWidth or ExifImageHeight is left untouched - in an image with ExifImageWidth or ExifImageHeight, their values are the same as ImageWidth or ImageHeight.
justinmayer
left a comment
There was a problem hiding this comment.
Seems sensible to me. Any thoughts, @minchinweb?
|
I'm just able to do a "desktop" (ok, phone) review at the moment, but it looks good. Thanks for the PR, and thanks for including tests! I was vaguely aware that Exif dimensions could mis-match the "physical" dimensions, as it came up on Immich's list of "Cursed Knowledge", bit hadn't drawn the connection it could affect us here. (For future reference,) Is there a reason not to add these dimensions to images that don't already have them? Also, should add a note to our list of "known issues" that Exif and "physical" dimensions will drift of ExifTool isn't installed? |
|
I do not add the EXIF dimension tags because I don't want to "pollute" the image with tags that were not there originally. If ExifTool is not installed, the tags (including the EXIF dimension tags) will not be copied from the original image, so there is no possibility for them to drift from the correct value. |
|
Everything looks good to me! @justinmayer will you merge this? |
|
Yes, indeed, I’d be happy to. Thank you for taking a look — I appreciate the extra set of eyes. @patrickfournier: Many thanks for your work on this! ✨ |
This pull request improves how EXIF tags are handled when processing images, especially regarding image dimensions, and enhances the related test coverage. The most important changes can be grouped as follows:
EXIF Tag Copying and Image Dimension Handling:
ExifImageWidthandExifImageHeighttags to match the dimensions of the transformed image when copying EXIF tags. This ensures that metadata accurately reflects the new image size after processing.Testing Enhancements:
Process and Logging Improvements:
Documentation:
Minor Edits: