RFR: 8160327: Support for thumbnails present in APP1 marker for JPEG [v2]

[Brian Burkhalter]

On Thu, 2 Jan 2025 06:49:19 GMT, Jeremy <duke at openjdk.org> wrote:

>> This adds support for parsing thumbnails in an APP1 Exif marker.
>> 
>> This builds on an unfinished proposal by Brian Burkhalter (around 2016). In that previous work the only additional meta info he parsed was the image creation time; this PR similarly includes the same property. (I can't speak to why he included that property, but it looks like he has a lot of experience with ImageIO so I trust his judgment.)
>> 
>> The test addresses the original images attached to the ticket plus a few extra images I found on my computer that include unusual properties. (Possibly those images are malformed, but if they exist in the wild and other platforms support them then I'd prefer to support them too.)
>
> Jeremy has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8160327: fixing typo so `thumbnailPos` can be zero
>   
>   The `thumbnailLength` cannot be zero, but the position can be.

src/java.desktop/share/classes/com/sun/imageio/plugins/jpeg/JPEGImageReader.java line 1653:

> 1651:             if (exifMarkerSegment != null
> 1652:                     && exifMarkerSegment.getNumThumbnails() == 1) {
> 1653:                 return 1;

I know that my original code was also like this, but I think the eventual intent was to read both JFIF (APP0) and Exif (APP1) thumbnails if both are present. In such a case, the thumbnail count would be 2, the JFIF thumbnail would be at index 0, and the Exif thumbnail at index 1.

In general I would expect that if both of these thumbnails were present, then they would be identical. If this were not the case, however, then preferring the Exif thumbnail would be a behavioral change. This is not necessarily a blocker for the current PR, but it might need to be addressed later.

src/java.desktop/share/classes/com/sun/imageio/plugins/jpeg/JPEGImageReader.java line 1758:

> 1756:             }
> 1757: 
> 1758:             // Now we know that there is a jfif segment and that iis is good

I think `iis` should be `it` in this comment.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/22898#discussion_r1956635292
PR Review Comment: https://git.openjdk.org/jdk/pull/22898#discussion_r1956634835