On Thu, 2 Jan 2025 06:49:19 GMT, Jeremy <d...@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.
This is a desirable and over-due fix but some small things to handle first.
Whilst this passed testing, I have some comments, one in-line, but testing
related ones here
There are 6 images attached, but the test uses only 5.
The images were all attached to the bug, but I don't think that means we
automatically have the rights to push them into JDK.
Brian Burkhalter attached at least 3 of them, so I need him to speak up about
where they came from. I'd also like @bplb to review this change
src/java.desktop/share/classes/com/sun/imageio/plugins/jpeg/ExifMarkerSegment.java
line 165:
> 163: ByteOrder.BIG_ENDIAN : ByteOrder.LITTLE_ENDIAN);
> 164: if (input.readUnsignedShort() != TIFF_MAGIC) {
> 165: throw new IllegalArgumentException("Bad magic number");
Where does this exception end up ? I would have supposed that if there's an
Exif segment we don't like it would be best to just act like the segment isn't
there.
-------------
PR Review: https://git.openjdk.org/jdk/pull/22898#pullrequestreview-2616189089
PR Review Comment: https://git.openjdk.org/jdk/pull/22898#discussion_r1955194737
