On Fri, 21 Feb 2025 02:31:42 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 four additional > commits since the last revision: > > - 8160327: alphabetize imports > > This is in response to: > https://github.com/openjdk/jdk/pull/22898#discussion_r1956718373 > - 8160327: fallback to using MarkerSegment if ExifMarkerSegment fails > > This is in response to: > https://github.com/openjdk/jdk/pull/22898#discussion_r1955194737 > > "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." > - 8160327: include support for JFIF *and* EXIF thumbnails together > > This is in response to: > https://github.com/openjdk/jdk/pull/22898#issuecomment-2666324177 > > As mentioned here ( > https://github.com/openjdk/jdk/pull/22898#issuecomment-2666949882 ) we'll > need to update some documentation about thumbnails. > - 8160327: moving test + resources to separate directory > > This is in response to: > https://github.com/openjdk/jdk/pull/22898#issuecomment-2657721887
The code changes now look good. I've not looked at the test yet but I see that it's been put in its own directory along with the images which is good. I don't think that the images need the `jdk_8160327` prefix however. Has it been determined which of these images can legitimately be included, aside from SV650? Also, as I mentioned before, I think that the JPEG metadata specification will need to slightly updated to cover the modified thumbnail behavior. This change will require a CSR. ------------- PR Comment: https://git.openjdk.org/jdk/pull/22898#issuecomment-2675396159
