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

[Phil Race]

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 at 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 at openjdk.java.net
>     <mailto:2d-dev at 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 at openjdk.java.net
>         <mailto:2d-dev at 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
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20151124/7012f10e/attachment.html>