From 18edc46b0fee295151671e1e67230bf441b59827 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 11 Aug 2021 00:21:01 +0200 Subject: [PATCH 1/4] If component id's are R, G, B in ASCII the color space should be RGB --- src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 896e5f0aa..269b2fe76 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -345,10 +345,10 @@ namespace SixLabors.ImageSharp.Formats.Jpeg } /// - /// Returns the correct colorspace based on the image component count + /// Returns the correct colorspace based on the image component count and the jpeg frame components. /// /// The - private JpegColorSpace DeduceJpegColorSpace(byte componentCount) + private JpegColorSpace DeduceJpegColorSpace(byte componentCount, JpegComponent[] components) { if (componentCount == 1) { @@ -362,6 +362,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg return JpegColorSpace.RGB; } + // If the component Id's are R, G, B in ASCII the colorspace is RGB and not YCbCr. + if (components[0].Id == 82 && components[1].Id == 71 && components[2].Id == 66) + { + return JpegColorSpace.RGB; + } + // Some images are poorly encoded and contain incorrect colorspace transform metadata. // We ignore that and always fall back to the default colorspace. return JpegColorSpace.YCbCr; @@ -836,9 +842,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg // 1 byte: Number of components byte componentCount = this.temp[5]; - this.ColorSpace = this.DeduceJpegColorSpace(componentCount); - - this.Metadata.GetJpegMetadata().ColorType = this.ColorSpace == JpegColorSpace.Grayscale ? JpegColorType.Luminance : JpegColorType.YCbCr; this.Frame = new JpegFrame(frameMarker, precision, frameWidth, frameHeight, componentCount); @@ -888,6 +891,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg index += componentBytes; } + this.ColorSpace = this.DeduceJpegColorSpace(componentCount, this.Frame.Components); + this.Metadata.GetJpegMetadata().ColorType = this.ColorSpace == JpegColorSpace.Grayscale ? JpegColorType.Luminance : JpegColorType.YCbCr; + this.Frame.Init(maxH, maxV); this.scanDecoder.InjectFrameData(this.Frame, this); From 6cb871711762f9327b4deed88ab0e5a85fdfd5aa Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 11 Aug 2021 00:36:09 +0200 Subject: [PATCH 2/4] Add unit test for issue #1732 --- .../Formats/Jpg/JpegDecoderTests.cs | 13 +++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + .../Input/Jpg/issues/Issue1732-WrongColorSpace.jpg | 3 +++ 3 files changed, 17 insertions(+) create mode 100644 tests/Images/Input/Jpg/issues/Issue1732-WrongColorSpace.jpg diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 674aa6d8f..e8d307f90 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -174,6 +174,19 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg await Assert.ThrowsAsync(async () => await Image.IdentifyAsync(config, "someFakeFile", cts.Token)); } + // https://github.com/SixLabors/ImageSharp/pull/1732 + [Theory] + [WithFile(TestImages.Jpeg.Issues.WrongColorSpace, PixelTypes.Rgba32)] + public void Issue1732_DecodesWithRgbColorSpace(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using (Image image = provider.GetImage(new JpegDecoder())) + { + image.DebugSave(provider); + image.CompareToOriginal(provider); + } + } + // DEBUG ONLY! // The PDF.js output should be saved by "tests\ImageSharp.Tests\Formats\Jpg\pdfjs\jpeg-converter.htm" // into "\tests\Images\ActualOutput\JpegDecoderTests\" diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 059e51cec..8b9aa1a5f 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -237,6 +237,7 @@ namespace SixLabors.ImageSharp.Tests public const string ExifResize1049 = "Jpg/issues/issue1049-exif-resize.jpg"; public const string BadSubSampling1076 = "Jpg/issues/issue-1076-invalid-subsampling.jpg"; public const string IdentifyMultiFrame1211 = "Jpg/issues/issue-1221-identify-multi-frame.jpg"; + public const string WrongColorSpace = "Jpg/issues/Issue1732-WrongColorSpace.jpg"; public static class Fuzz { diff --git a/tests/Images/Input/Jpg/issues/Issue1732-WrongColorSpace.jpg b/tests/Images/Input/Jpg/issues/Issue1732-WrongColorSpace.jpg new file mode 100644 index 000000000..e3ba85ae8 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/Issue1732-WrongColorSpace.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:3c72235954cdfb9d0cc7f09c537704e617313dc77708b4dca27b47c94c5e67a6 +size 2852 From 68a706fb06f576bcf2c0e1dfb038710b9f732753 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 11 Aug 2021 01:05:14 +0200 Subject: [PATCH 3/4] Move reading frame ComponentIds out of only metadata block --- .../Formats/Jpeg/JpegDecoderCore.cs | 74 +++++++++---------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 269b2fe76..413c9b2bd 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -845,57 +845,57 @@ namespace SixLabors.ImageSharp.Formats.Jpeg this.Frame = new JpegFrame(frameMarker, precision, frameWidth, frameHeight, componentCount); - if (!metadataOnly) + remaining -= length; + + // Validate: remaining part must be equal to components * 3 + const int componentBytes = 3; + if (remaining != componentCount * componentBytes) { - remaining -= length; + JpegThrowHelper.ThrowBadMarker("SOFn", remaining); + } - // Validate: remaining part must be equal to components * 3 - const int componentBytes = 3; - if (remaining != componentCount * componentBytes) - { - JpegThrowHelper.ThrowBadMarker("SOFn", remaining); - } + // components*3 bytes: component data + stream.Read(this.temp, 0, remaining); - // components*3 bytes: component data - stream.Read(this.temp, 0, remaining); + // No need to pool this. They max out at 4 + this.Frame.ComponentIds = new byte[componentCount]; + this.Frame.ComponentOrder = new byte[componentCount]; + this.Frame.Components = new JpegComponent[componentCount]; - // No need to pool this. They max out at 4 - this.Frame.ComponentIds = new byte[componentCount]; - this.Frame.ComponentOrder = new byte[componentCount]; - this.Frame.Components = new JpegComponent[componentCount]; + int maxH = 0; + int maxV = 0; + int index = 0; + for (int i = 0; i < componentCount; i++) + { + byte hv = this.temp[index + 1]; + int h = (hv >> 4) & 15; + int v = hv & 15; - int maxH = 0; - int maxV = 0; - int index = 0; - for (int i = 0; i < componentCount; i++) + if (maxH < h) { - byte hv = this.temp[index + 1]; - int h = (hv >> 4) & 15; - int v = hv & 15; + maxH = h; + } - if (maxH < h) - { - maxH = h; - } + if (maxV < v) + { + maxV = v; + } - if (maxV < v) - { - maxV = v; - } + var component = new JpegComponent(this.Configuration.MemoryAllocator, this.Frame, this.temp[index], h, v, this.temp[index + 2], i); - var component = new JpegComponent(this.Configuration.MemoryAllocator, this.Frame, this.temp[index], h, v, this.temp[index + 2], i); + this.Frame.Components[i] = component; + this.Frame.ComponentIds[i] = component.Id; - this.Frame.Components[i] = component; - this.Frame.ComponentIds[i] = component.Id; + index += componentBytes; + } - index += componentBytes; - } + this.ColorSpace = this.DeduceJpegColorSpace(componentCount, this.Frame.Components); - this.ColorSpace = this.DeduceJpegColorSpace(componentCount, this.Frame.Components); - this.Metadata.GetJpegMetadata().ColorType = this.ColorSpace == JpegColorSpace.Grayscale ? JpegColorType.Luminance : JpegColorType.YCbCr; + this.Metadata.GetJpegMetadata().ColorType = this.ColorSpace == JpegColorSpace.Grayscale ? JpegColorType.Luminance : JpegColorType.YCbCr; + if (!metadataOnly) + { this.Frame.Init(maxH, maxV); - this.scanDecoder.InjectFrameData(this.Frame, this); } } From 7e7dbbb94317efa449a270de47d67879528feb72 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Fri, 13 Aug 2021 16:15:23 +0200 Subject: [PATCH 4/4] Switch order of component id check --- src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 413c9b2bd..e94b07faa 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -363,7 +363,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg } // If the component Id's are R, G, B in ASCII the colorspace is RGB and not YCbCr. - if (components[0].Id == 82 && components[1].Id == 71 && components[2].Id == 66) + if (components[2].Id == 66 && components[1].Id == 71 && components[0].Id == 82) { return JpegColorSpace.RGB; }