[OpenJDK 2D-Dev] [10] RFR: JDK-8164971: PNG metadata does not handle ImageCreationTime

[Jayathirth D V]

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