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