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

[Sergey Bylokhov]

+1

On 01/06/2018 13:16, Phil Race wrote:
> 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.
>>
> 


-- 
Best regards, Sergey.