Hello Prahalad,
Please find my inputs.
As per PNG Specification for text chunks
http://libpng.org/pub/png/spec/1.2/PNG-Chunks.html#C.Anc-text . There can be
multiple chunks with same keyword.
1) In PNGMetadata. extractCreationTimeFromText() you are updating the
"creation_time_present" to false when it doesn't follow RFC1123. So if there
are multiple text chunks and if the latest chunk doesn't follow RFC1123 while
decoding it will result in "creation_time_present" to be false. In case where
we are not able to decode the provided text, we should not update
"creation_time_present" to false.
2) From the PNGMetadata. extractCreationTimeFromText() implementation I see
that, if there are multiple text chunks following RFC1123 text we update our
standard metadata node with last parsed chunk. This information should be
covered properly in comments as it is specific to our implementation and it is
not part of PNG specification.
3) Since we are maintaining standard metadata "ImageCreationTime" info from the
last RFC1123 compliant text chunk while decoding. When an user adds new
"ImageCreationTime" standard node which follows RFC1123 text to already present
metadata, we should update the last text chunk from which we have captured
creation time information while decoding. In the present iteration of code in
PNGMetadata.encodeCreationTimeToText() we are updating the first available text
chunk with the new information provided in stanrdardmetadata node.
4) Also when user adds new native metadata creation time node with RFC1123
compliance to already present list of creation time nodes. We should make sure
that the newly added native time chunk is the one that should be updated when
one more addition of standard metadata "ImageCreationTime" node happens.
Basically for 3 & 4 points we have to maintain what is the active native text
chunk and what should be its corresponding creation time information in
standard metadata.
5) One more question is about how are we planning to maintain IIOMetadata
consistency between multiple decoding & encoding sequences. We decode the
metadata of PNG file as text chunks appear but we encode multiple same text
chunks collectively. So when an user gets IIOMetadata of an image and updates
it with multiple native and standard creation time chunks and later encode it.
How we will maintain that what is the primary native text chunk from where we
have to update standard metadata creation time.
6) It's better to maintain /* */ format for multiple line comments then using
multiple //.
Thanks,
Jay
-----Original Message-----
From: Prahalad Kumar Narayanan
Sent: Saturday, April 22, 2017 4:33 PM
To: 2d-dev@openjdk.java.net
Subject: [OpenJDK 2D-Dev] [10] RFR: JDK-8164971: PNG metadata does not handle
ImageCreationTime
Hello Everyone
Good day to you.
Request your time in reviewing a fix for the bug
. JDK-8164971 PNG metadata does not handle ImageCreationTime
Root Cause:
. First, the PNGImageReader 's logic skips parsing of 'any' chunk once IDAT is
found.
. Hence, if the ancilliary text chunks appear after IDAT chunk, they are not
processed at all.
. Second, the parsing of text chunks does not check for 'Creation Time' keyword.
. As a result the image creation time is never retrieved from .png image.
. The converse is true as well- While writing a png image, the creation time is
not written to text chunk
Details on the Fix:
. PNGImageReader
. Logic has been fixed to continue parsing chunks until IEND chunk is found
. Methods that process text chunks now check for the presence of 'Creation
Time' and retrieve the time information.
. PNGMetadata:
. New methods and variables to support image creation time in metadata.
. Proper use of DateTimeFormatter.RFC_1123_DATE_TIME to decode time from
text chunk and encode time to text as well.
. Changes to existing methods (that allow node retrieval & merge) for
reading and updating creation time.
. Test Case & Test Image
. The test case- PngCreationTimeTest, checks the following use-cases
. Decoding creation time from duke.png 's text chunk.
. I created the image using gimp and added text chunk
programmatically with Creation Time in it
. Updating the image creation time using- mergeTree (nativeTree) and
inspecting same value in standard Document node
. Updating the image creation time using- mergeTree (standardTree) and
inspecting same value in native tree's text entry.
Other Details:
. The changes were tested with Jtreg and JCK suites . No regressions were seen.
Kindly review the changes at your convenience and provide your suggestions.
Review Link:
http://cr.openjdk.java.net/~pnarayanan/8164971/webrev.00/
Thank you for your time in review
Have a good day
