[ https://issues.apache.org/jira/browse/IMAGING-310?focusedWorklogId=767817&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-767817 ]
ASF GitHub Bot logged work on IMAGING-310: ------------------------------------------ Author: ASF GitHub Bot Created on: 09/May/22 07:44 Start Date: 09/May/22 07:44 Worklog Time Spent: 10m Work Description: kinow commented on code in PR #163: URL: https://github.com/apache/commons-imaging/pull/163#discussion_r867723828 ########## src/main/java/org/apache/commons/imaging/formats/jpeg/JpegImageParser.java: ########## @@ -824,44 +824,44 @@ public ImageInfo getImageInfo(final ByteSource byteSource, final Map<String, Obj // See http://docs.oracle.com/javase/6/docs/api/javax/imageio/metadata/doc-files/jpeg_metadata.html#color ImageInfo.ColorType colorType = ImageInfo.ColorType.UNKNOWN; - // Some images have both JFIF/APP0 and APP14. - // JFIF is meant to win but in them APP14 is clearly right, so make it win. - if (app14Segment != null && app14Segment.isAdobeJpegSegment()) { - final int colorTransform = app14Segment.getAdobeColorTransform(); - switch (colorTransform) { - case App14Segment.ADOBE_COLOR_TRANSFORM_UNKNOWN: - if (numberOfComponents == 3) { - colorType = ImageInfo.ColorType.RGB; - } else if (numberOfComponents == 4) { - colorType = ImageInfo.ColorType.CMYK; + switch (numberOfComponents) { + case 1: + colorType = ImageInfo.ColorType.GRAYSCALE; + break; + case 2: + colorType = ImageInfo.ColorType.GRAYSCALE; + transparent = true; + break; + case 3: + case 4: + // Some images have both JFIF/APP0 and APP14. + // JFIF is meant to win but in them APP14 is clearly right, so make it win. + if (app14Segment != null && app14Segment.isAdobeJpegSegment()) { + final int colorTransform = app14Segment.getAdobeColorTransform(); + switch (colorTransform) { + case App14Segment.ADOBE_COLOR_TRANSFORM_UNKNOWN: + if (numberOfComponents == 3) { + colorType = ImageInfo.ColorType.RGB; + } else if (numberOfComponents == 4) { + colorType = ImageInfo.ColorType.CMYK; + } + break; + case App14Segment.ADOBE_COLOR_TRANSFORM_YCbCr: + colorType = ImageInfo.ColorType.YCbCr; + break; + case App14Segment.ADOBE_COLOR_TRANSFORM_YCCK: + colorType = ImageInfo.ColorType.YCCK; + break; + default: + break; } - break; - case App14Segment.ADOBE_COLOR_TRANSFORM_YCbCr: - colorType = ImageInfo.ColorType.YCbCr; - break; - case App14Segment.ADOBE_COLOR_TRANSFORM_YCCK: - colorType = ImageInfo.ColorType.YCCK; - break; - default: - break; - } - } else if (jfifSegment != null) { - if (numberOfComponents == 1) { - colorType = ImageInfo.ColorType.GRAYSCALE; - } else if (numberOfComponents == 3) { - colorType = ImageInfo.ColorType.YCbCr; - } - } else { - switch (numberOfComponents) { - case 1: - colorType = ImageInfo.ColorType.GRAYSCALE; - break; - case 2: - colorType = ImageInfo.ColorType.GRAYSCALE; - transparent = true; - break; - case 3: - case 4: + } else if (jfifSegment != null) { + if (numberOfComponents == 1) { Review Comment: Ping 2. Release going out soon :steam_locomotive: Issue Time Tracking ------------------- Worklog Id: (was: 767817) Time Spent: 2h (was: 1h 50m) > JpegImageParser: Grayscale JPEG file with app14Segment returns > ColorType.UNKNOWN > -------------------------------------------------------------------------------- > > Key: IMAGING-310 > URL: https://issues.apache.org/jira/browse/IMAGING-310 > Project: Commons Imaging > Issue Type: Bug > Components: Format: JPEG > Affects Versions: 1.0-alpha2 > Reporter: John Phillips > Assignee: Bruno P. Kinoshita > Priority: Major > Fix For: 1.0-alpha3 > > Attachments: sample-grayscale-with-app14segment.jpg > > Time Spent: 2h > Remaining Estimate: 0h > > In org.apache.commons.imaging.formats.jpeg. the logic to determine colorType > (lines 825-970) does not account for a Grayscale image when there is an app14 > segment present. > When the number of components is 1 or 2, the colorType should always be > Grayscale. None of the other logic would change that. > It seems to me that the switch (numberOfComponents) statement should be moved > to the top of this bit of code, and the rest of the logic should handled > under Case 3 & 4: > {noformat} > // See > http://docs.oracle.com/javase/6/docs/api/javax/imageio/metadata/doc-files/jpeg_metadata.html#color > ImageInfo.ColorType colorType = ImageInfo.ColorType.UNKNOWN; > switch (numberOfComponents) { > case 1: > colorType = ImageInfo.ColorType.GRAYSCALE; > break; > > case 2: > colorType = ImageInfo.ColorType.GRAYSCALE; > transparent = true; > break; > > case 3: > case 4: > // Some images have both JFIF/APP0 and APP14. > // JFIF is meant to win but in them APP14 is clearly right, so make it > win. > if (app14Segment != null && app14Segment.isAdobeJpegSegment()) { > etc. > {noformat} > Attached sample-grayscale-with-app14segment.jpg, which can be used to > demonstrate the issue. > I've modified the code locally, but I'm a newbie at submitting > pull-requests.... -- This message was sent by Atlassian Jira (v8.20.7#820007)