On Fri, 14 Feb 2025 19:37:35 GMT, Brian Burkhalter <[email protected]> wrote:
>> 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.
Hmm. I think it better to address it now. Who knows when we'd get back to it.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22898#discussion_r1956750727
