[OpenJDK 2D-Dev] RFR: 8204187: Remove proprietary JPEG code from javax.imageio

[Phil Race]

Well yes, that looks like a copy/paste bug and it seems it has been 
there since
1.4 (the very beginning of image i/o !) This probably would have been a 
serious issue except
that this code is used only if there is no JFIF or adobe marker.
I'll fix that before I push .. or include it in an updated webrev if 
there are
more issues to update.

-phil.

On 06/01/2018 01:04 PM, Krishna Addepalli wrote:
> Hi Phil,
>
> The changes look fine to me. However I have one question: In SOFMarkerSegment.java file, in the function below,
>
>    int getIDencodedCSType () {
>    for (int i = 0; i < componentSpecs.length; i++) {
> 	if (componentSpecs[i].componentId < 'A') {
> 	          return JPEG.JCS_UNKNOWN;
>         }
>           }
>   switch(componentSpecs.length) {
>   case 3:
>   	if ((componentSpecs[0].componentId == 'R')
>   	          &&(componentSpecs[0].componentId == 'G')
>   	          &&(componentSpecs[0].componentId == 'B')) {
>   		          return JPEG.JCS_RGB;
>   }
>
> componentId is an integer, and it can store only one value R, G or B. However, the condition checks for the integer value being equal to all the three values simultaneously which is impossible.
> I know this code was existing before your changes, but not sure if my understanding is correct.
>
> Thanks,
> Krishna
>
> -----Original Message-----
> From: Phil Race
> Sent: Friday, June 1, 2018 10:35 PM
> To: 2d-dev <2d-dev at openjdk.java.net>
> Subject: [OpenJDK 2D-Dev] RFR: 8204187: Remove proprietary JPEG code from javax.imageio
>
> Bug : https://bugs.openjdk.java.net/browse/JDK-8204187
> CSR : https://bugs.openjdk.java.net/browse/JDK-8204189
> Webrev : http://cr.openjdk.java.net/~prr/8204187/
>
> Please review the code and the CSR.
>
> There are 4 optional color spaces for the standard JPEG plugin documented in the javax.image I/O JPEG metadata specification.
>
> https://docs.oracle.com/javase/10/docs/api/javax/imageio/metadata/doc-files/jpeg_metadata.html#color
>
> ---
> Optional ColorSpace support: Handling of PhotoYCC (YCC), PhotoYCCA (YCCA), RGBA and YCbCrA color spaces by the standard plugin, as described below, is dependent on capabilities of the libraries used to interpret the JPEG data. Thus all consequential behaviors are optional.
> ..
> ..
> [and more details]
> ---
> The necessary support for these is implemented in a modified, closed source, IJG libjpeg 6b.
>
> What this means is that OpenJDK never has supported these.
>
> However code to handle these on the Java side is implemented in the open code Oracle intends to remove the closed source native support, so we can clean up the open code.
>
> And there's a particular bug that even the supported YCCK color space would not work with OpenJDK due to the ID specified in the Java code not matching the ID specified in the standard IJG libraries.
>
> Another fix is that even though it did not have support for writing images with alpha, the writer would say it could - true only for a JDK with the closed enhancements.
> So what would happen is that even if code would check to see if it was supported when it tried it would get this exception :
>
> Exception in thread "main" javax.imageio.IIOException: Invalid argument to native writeImage
>       at
> java.desktop/com.sun.imageio.plugins.jpeg.JPEGImageWriter.writeImage(Native
> Method)
>       at
> java.desktop/com.sun.imageio.plugins.jpeg.JPEGImageWriter.writeOnThread(JPEGImageWriter.java:1075)
>       at
> java.desktop/com.sun.imageio.plugins.jpeg.JPEGImageWriter.write(JPEGImageWriter.java:371)
>       at java.desktop/javax.imageio.ImageWriter.write(ImageWriter.java:613)
>       at java.desktop/javax.imageio.ImageIO.doWrite(ImageIO.java:1628)
>       at java.desktop/javax.imageio.ImageIO.write(ImageIO.java:1594)
>       at TestWriteARGBJPEG.main(TestWriteARGBJPEG.java:15)
>
> With the removal of the Java code related to these color spaces, you will still get an exception, but a slightly different one :
>
> Exception in thread "main" javax.imageio.IIOException: Bogus input colorspace
>       at
> java.desktop/com.sun.imageio.plugins.jpeg.JPEGImageWriter.writeImage(Native
> Method)
>       at
> java.desktop/com.sun.imageio.plugins.jpeg.JPEGImageWriter.writeOnThread(JPEGImageWriter.java:1007)
>       at
> java.desktop/com.sun.imageio.plugins.jpeg.JPEGImageWriter.write(JPEGImageWriter.java:371)
>       at java.desktop/javax.imageio.ImageWriter.write(ImageWriter.java:613)
>       at TestWriteARGBJPEG.main(TestWriteARGBJPEG.java:23)
>
>
> The key to avoiding both of these is making sure the code that checks support looks to see if there is an alpha channel and gives you a correct answer. That is fixed.
> I have also included a new test which covers this and updated an existing one.
>
> I am leaving the specification alone for now at least, to leave the door open to reinstating such support somehow if it there is a need. If we do it may be just a subset.
>
> I am preparing a release note as well as a CSR for this.
>
> -phil.
>