fix(tiff): Correctly read TIFF EXIF fields for ExifVersion and FlashPixVersion - #5045
Conversation
This allows us to correctly read the ExifVersion and FlashPixVersion metadata in an EXIF block of a TIFF file. Signed-off-by: Larry Gritz <lg@larrygritz.com>
There was a problem hiding this comment.
Pull request overview
This pull request fixes the reading of EXIF ExifVersion and FlashPixVersion metadata fields from TIFF files. Previously, these fields were being skipped despite having handler functions defined. The fix adds special-case handling in the TIFF input code to properly read these 4-byte undefined-type fields as strings.
Changes:
- Added special handling in tiffinput.cpp to read ExifVersion and FlashPixVersion as 4-character strings
- Enhanced safe_tiffgetfield to support fields that require passing a count parameter
- Fixed a syntax error and improved type consistency in debug code in exif.cpp
- Updated test reference outputs to reflect the newly-readable metadata fields
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/tiff.imageio/tiffinput.cpp | Added special case handling for EXIF_EXIFVERSION and EXIF_FLASHPIXVERSION in find_tag(), and extended safe_tiffgetfield() to handle passcount fields |
| src/tiff.imageio/tiffoutput.cpp | Changed debug output conditionals from #ifndef NDEBUG to #if 0 to unconditionally disable verbose debug messages |
| src/libOpenImageIO/exif.cpp | Fixed syntax error in debug print statement, changed 200L to 200UL for type consistency, added #ifdef guard for EXIF_TIFF_UTF8, and removed misleading comments about skipping version tags |
| testsuite/tiff-suite/ref/*.txt | Updated test reference outputs to include the newly-readable Exif:ExifVersion and Exif:FlashPixVersion fields for test images |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # if 0 | ||
| print("Unhandled EXIF {} ({}) / tag {} tifftype {} count {}\n", | ||
| p.name(), p.type(), tag, tifftype, count); | ||
| # endif |
There was a problem hiding this comment.
The debug output has been unconditionally disabled by changing from #ifndef NDEBUG to #if 0. This means these debug messages will never be printed, even in debug builds. If this is intentional to reduce noise, consider using a more specific debug flag (like #if DEBUG_TIFF_EXIF) to allow selective enabling when needed for debugging EXIF-related issues.
| # if 0 | ||
| print( | ||
| "Error handling EXIF {} ({}) / tag {} tifftype {} count {}\n", | ||
| p.name(), p.type(), tag, tifftype, count); | ||
| # endif |
There was a problem hiding this comment.
Similar to above, this debug output has been unconditionally disabled by changing from commented code to #if 0. Consider using a named debug flag for more granular control if these messages would be useful for debugging EXIF write operations.
0457774
into
AcademySoftwareFoundation:main
…ixVersion (AcademySoftwareFoundation#5045) This allows us to correctly read the ExifVersion and FlashPixVersion metadata in an EXIF block of a TIFF file. Signed-off-by: Larry Gritz <lg@larrygritz.com>
…ixVersion (AcademySoftwareFoundation#5045) This allows us to correctly read the ExifVersion and FlashPixVersion metadata in an EXIF block of a TIFF file. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
…ixVersion (AcademySoftwareFoundation#5045) This allows us to correctly read the ExifVersion and FlashPixVersion metadata in an EXIF block of a TIFF file. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Vlad <shaamaan@gmail.com>
…ixVersion (AcademySoftwareFoundation#5045) This allows us to correctly read the ExifVersion and FlashPixVersion metadata in an EXIF block of a TIFF file. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
…ixVersion (AcademySoftwareFoundation#5045) This allows us to correctly read the ExifVersion and FlashPixVersion metadata in an EXIF block of a TIFF file. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>
…ixVersion (AcademySoftwareFoundation#5045) This allows us to correctly read the ExifVersion and FlashPixVersion metadata in an EXIF block of a TIFF file. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com> Signed-off-by: Vlad <shaamaan@gmail.com>
…ixVersion (AcademySoftwareFoundation#5045) This allows us to correctly read the ExifVersion and FlashPixVersion metadata in an EXIF block of a TIFF file. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com> Signed-off-by: Vlad <shaamaan@gmail.com>
This allows us to correctly read the ExifVersion and FlashPixVersion metadata in an EXIF block of a TIFF file.