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

Jeremy duke at openjdk.org
Mon Feb 24 07:11:58 UTC 2025


On Fri, 14 Feb 2025 21:08:31 GMT, Brian Burkhalter <bpb at openjdk.org> 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/ExifMarkerSegment.java line 28:
> 
>> 26: package com.sun.imageio.plugins.jpeg;
>> 27: 
>> 28: import java.io.InputStream;
> 
> I suggest putting the imports on lines 28-51 in alphabetical order.

This is updated

> 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.

This comment no longer exists after other revisions

> src/java.desktop/share/classes/com/sun/imageio/plugins/jpeg/JPEGMetadata.java line 266:
> 
>> 264:                         && (buf[ptr+7] == 0)
>> 265:                         && (buf[ptr+8] == 0)) {
>> 266:                     newGuy = new ExifMarkerSegment(buffer);
> 
> The `IllegalArgumentException` from the `ExifMarkerSegment` constructor should be handled somehow. Perhaps just fall back to setting `newGuy` to a plain `MarkerSegment`?

This is updated (see this thread https://github.com/openjdk/jdk/pull/22898#discussion_r1955194737 )

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22898#discussion_r1967118843
PR Review Comment: https://git.openjdk.org/jdk/pull/22898#discussion_r1967118674
PR Review Comment: https://git.openjdk.org/jdk/pull/22898#discussion_r1967118950


More information about the client-libs-dev mailing list