Re: [OpenJDK 2D-Dev] Review request for JDK - 8041501 : ImageIO reader is not capable of reading JPEGs without JFIF header

[prasanta sadhukhan]

+1

Regards
Prasanta
On 11/25/2015 12:08 PM, Jayathirth D V wrote:

Hi Prasanta,

Please review updated webrev :

http://cr.openjdk.java.net/~jdv/8041501/webrev.03/ <http://cr.openjdk.java.net/%7Ejdv/8041501/webrev.03/>

Thanks,

Jay

*From:*Phil Race
*Sent:* Tuesday, November 24, 2015 11:57 PM
*To:* Jayathirth D V
*Cc:* Prasanta Sadhukhan; 2d-dev@openjdk.java.net

*Subject:* Re: <OpenJDK 2D-Dev> Review request for JDK - 8041501 : ImageIO reader is not capable of reading JPEGs without JFIF header

Approved.

-phil.

On 11/24/2015 01:39 AM, Jayathirth D V wrote:

    Hi Phil,

    I have replaced already present comment block with proper comments
    , since it was contradicting Metadata spec. And removed extra
    comments added by me.

    _Please review updated webrev:_

    http://cr.openjdk.java.net/~jdv/8041501/webrev.03/
    <http://cr.openjdk.java.net/%7Ejdv/8041501/webrev.03/>

    Thanks,

    Jay

    *From:*Phil Race
    *Sent:* Tuesday, November 24, 2015 2:08 AM
    *To:* Jayathirth D V
    *Cc:* Prasanta Sadhukhan; 2d-dev@openjdk.java.net
    <mailto:2d-dev@openjdk.java.net>
    *Subject:* Re: <OpenJDK 2D-Dev> Review request for JDK - 8041501 :
    ImageIO reader is not capable of reading JPEGs without JFIF header

    Rather than adding a comment block, it seems you need to edit the
    preceding one.

      * IJG assumes all unidentified 3-channels are YCbCr.

    1715                  * We assume that only if the second two channels are

    1716                  * subsampled (either horizontally or vertically).  If 
not,

    1717                  * we assume RGB.

    The fix is invalidating that comment but that is intended since

    (a) following the comment is causing the bug

    (b) the comment is contradicting the public Metadata spec

    For the benefit of others, the text in the spec is :

    ----

    If neither marker segment is present, the following procedure is followed: 
....

    For 3- and 4-channel images, the component ids are consulted.  If these

    values are 1-3 for a 3-channel image, then the image is assumed to be YCbCr.

    ... Otherwise, 3-channel subsampled images are assumed to be YCbCr,

    3-channel non-subsampled images are assumed to be RGB

    ---

    So here is my suggested replacement text for the existing comment block :

    ---

    In the absence of certain markers, IJG has interpreted component id's of 
[1,2,3] as meaning YCbCr.

    We follow that interpretation, which is additionally described in the Image 
I/O JPEG metadata spec.

    If that condition is not met here the next step here is to examine the 
subsampling factors

    If any differ we also assume YCbCr, but if all are the same then we assume 
RGB

    This is also described in the Image I/O JPEG metadata spec.

    --

    -phil.

    On 11/23/2015 04:01 AM, Jayathirth D V wrote:

        Hi Prasanta,

        Removed repeated usage of getWidth() and getHeight(). Please
        review.

        http://cr.openjdk.java.net/~jdv/8041501/webrev.02/
        <http://cr.openjdk.java.net/%7Ejdv/8041501/webrev.02/>

        Thanks,

        Jay

        *From:*prasanta sadhukhan
        *Sent:* Monday, November 23, 2015 5:15 PM
        *To:* Jayathirth D V
        *Cc:* Philip Race; 2d-dev@openjdk.java.net
        <mailto:2d-dev@openjdk.java.net>
        *Subject:* Re: <OpenJDK 2D-Dev> Review request for JDK -
        8041501 : ImageIO reader is not capable of reading JPEGs
        without JFIF header

        On 11/23/2015 5:11 PM, Jayathirth D V wrote:

            Hi Prasanta,

            Thanks for suggestion. I have made related changes and
            updated the Webrev.

            Webrev :
            http://cr.openjdk.java.net/~jdv/8041501/webrev.01/
            <http://cr.openjdk.java.net/%7Ejdv/8041501/webrev.01/>

            Please review.

        There's no point getting the width & height repeatedly . You
        can get it once and use that info in the loop. It's not going
        to change in runtime, isn;t it :-)

        Regards
        Prasanta



            Thanks,

            Jay

            *From:*prasanta sadhukhan
            *Sent:* Monday, November 23, 2015 4:43 PM
            *To:* Jayathirth D V; 2d-dev@openjdk.java.net
            <mailto:2d-dev@openjdk.java.net>
            *Cc:* Philip Race
            *Subject:* Re: <OpenJDK 2D-Dev> Review request for JDK -
            8041501 : ImageIO reader is not capable of reading JPEGs
            without JFIF header

            Looks ok to me.
            But probably you could check for image width &height
            programmatically instead of hardcoding to 64 in testcode.

            Regards
            Prasanta

            On 11/23/2015 4:09 PM, Jayathirth D V wrote:

                Hello All,

                _Please review following fix in JDK9:_

                Bug : https://bugs.openjdk.java.net/browse/JDK-8041501

                Webrev :
                http://cr.openjdk.java.net/~jdv/8041501/webrev.00/
                <http://cr.openjdk.java.net/%7Ejdv/8041501/webrev.00/>

                Issue : Pink discoloration when we read JPEG images
                without JFIF & EXIF header and having no subsampling.

                Root cause : We are overriding JPEG color space set in
                IJG library at imageioJPEG.c without checking
                component ID’s properly when JFIF & EXIF are not
                there. Decision to change color space is solely done
                consulting sampling factors.

                Solution : Added extra check to verify component ID’s
                also before changing color space determined by IJG
                library when there is no JFIF & EXIF header.

                Thanks,

                Jay