Merge pull request #2126 from SixLabors/backport/issue-2123

Backport - Issue 2123

Author: JimBobSquarePants

.github/workflows/build-and-test.yml

@@ -4,11 +4,13 @@ on:
   push:
     branches:
       - main
+      - "release/*"
     tags:
       - "v*"
   pull_request:
     branches:
       - main
+      - "release/*"
 jobs:
   Build:
     strategy:

src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs

@@ -89,6 +89,11 @@ internal sealed class JpegDecoderCore : IRawJpegData, IImageDecoderInternals
         /// </summary>
         private JFifMarker jFif;
 
+        /// <summary>
+        /// Whether the image has a JFIF marker. This is needed to determine, if the colorspace is YCbCr.
+        /// </summary>
+        private bool hasJFif;
+
         /// <summary>
         /// Contains information about the Adobe marker.
         /// </summary>
@@ -488,27 +493,75 @@ private JpegColorSpace DeduceJpegColorSpace(byte componentCount)
 
             if (componentCount == 3)
             {
-                if (!this.adobe.Equals(default) && this.adobe.ColorTransform == JpegConstants.Adobe.ColorTransformUnknown)
+                // We prioritize adobe marker over jfif marker, if somebody really encoded this image with redundant adobe marker,
+                // then it's most likely an adobe jfif image.
+                if (!this.adobe.Equals(default))
                 {
-                    return JpegColorSpace.RGB;
+                    if (this.adobe.ColorTransform == JpegConstants.Adobe.ColorTransformYCbCr)
+                    {
+                        return JpegColorSpace.YCbCr;
+                    }
+
+                    if (this.adobe.ColorTransform == JpegConstants.Adobe.ColorTransformUnknown)
+                    {
+                        return JpegColorSpace.RGB;
+                    }
+
+                    // Fallback to the id color deduction: If these values are 1-3 for a 3-channel image, then the image is assumed to be YCbCr.
+                    if (this.Components[2].Id == 3 && this.Components[1].Id == 2 && this.Components[0].Id == 1)
+                    {
+                        return JpegColorSpace.YCbCr;
+                    }
+
+                    JpegThrowHelper.ThrowNotSupportedColorSpace();
+                }
+
+                if (this.hasJFif)
+                {
+                    // JFIF implies YCbCr.
+                    return JpegColorSpace.YCbCr;
                 }
 
+                // Fallback to the id color deduction.
                 // If the component Id's are R, G, B in ASCII the colorspace is RGB and not YCbCr.
+                // See: https://docs.oracle.com/javase/7/docs/api/javax/imageio/metadata/doc-files/jpeg_metadata.html#color
                 if (this.Components[2].Id == 66 && this.Components[1].Id == 71 && this.Components[0].Id == 82)
                 {
                     return JpegColorSpace.RGB;
                 }
 
+                // 3-channel non-subsampled images are assumed to be RGB.
+                if (this.Components[2].VerticalSamplingFactor == 1 && this.Components[1].VerticalSamplingFactor == 1 && this.Components[0].VerticalSamplingFactor == 1 &&
+                    this.Components[2].HorizontalSamplingFactor == 1 && this.Components[1].HorizontalSamplingFactor == 1 && this.Components[0].HorizontalSamplingFactor == 1)
+                {
+                    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;
             }
 
             if (componentCount == 4)
             {
-                return this.adobe.ColorTransform == JpegConstants.Adobe.ColorTransformYcck
-                    ? JpegColorSpace.Ycck
-                    : JpegColorSpace.Cmyk;
+                // jfif images doesn't not support 4 component images, so we only check adobe.
+                if (!this.adobe.Equals(default))
+                {
+                    if (this.adobe.ColorTransform == JpegConstants.Adobe.ColorTransformYcck)
+                    {
+                        return JpegColorSpace.Ycck;
+                    }
+
+                    if (this.adobe.ColorTransform == JpegConstants.Adobe.ColorTransformUnknown)
+                    {
+                        return JpegColorSpace.Cmyk;
+                    }
+
+                    JpegThrowHelper.ThrowNotSupportedColorSpace();
+                }
+
+                // Fallback to cmyk as neither of cmyk nor ycck have 'special' component ids.
+                return JpegColorSpace.Cmyk;
             }
 
             JpegThrowHelper.ThrowNotSupportedComponentCount(componentCount);
@@ -675,6 +728,8 @@ private void ExtendProfile(ref byte[] profile, byte[] extension)
         /// <param name="remaining">The remaining bytes in the segment block.</param>
         private void ProcessApplicationHeaderMarker(BufferedReadStream stream, int remaining)
         {
+            this.hasJFif = true;
+
             // We can only decode JFif identifiers.
             // Some images contain multiple JFIF markers (Issue 1932) so we check to see
             // if it's already been read.

src/ImageSharp/Formats/Jpeg/JpegThrowHelper.cs

@@ -51,5 +51,8 @@ internal static class JpegThrowHelper
 
         [MethodImpl(InliningOptions.ColdPath)]
         public static void ThrowNotSupportedComponentCount(int componentCount) => throw new NotSupportedException($"Images with {componentCount} components are not supported.");
+
+        [MethodImpl(InliningOptions.ColdPath)]
+        public static void ThrowNotSupportedColorSpace() => throw new NotSupportedException("Image color space could not be deduced.");
     }
 }

src/ImageSharp/Formats/Tiff/PhotometricInterpretation/YCbCrTiffColor{TPixel}.cs

@@ -39,8 +39,15 @@ public override void Decode(ReadOnlySpan<byte> data, Buffer2D<TPixel> pixels, in
                 Span<byte> tmpBufferSpan = tmpBuffer.GetSpan();
                 ReverseChromaSubSampling(width, height, this.ycbcrSubSampling[0], this.ycbcrSubSampling[1], data, tmpBufferSpan);
                 ycbcrData = tmpBufferSpan;
+                this.DecodeYCbCrData(pixels, left, top, width, height, ycbcrData);
+                return;
             }
 
+            this.DecodeYCbCrData(pixels, left, top, width, height, ycbcrData);
+        }
+
+        private void DecodeYCbCrData(Buffer2D<TPixel> pixels, int left, int top, int width, int height, ReadOnlySpan<byte> ycbcrData)
+        {
             var color = default(TPixel);
             int offset = 0;
             int widthPadding = 0;

src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs

@@ -459,6 +459,14 @@ private static void ParseCompression(this TiffDecoderCore options, TiffCompressi
                 case TiffCompression.Jpeg:
                 {
                     options.CompressionType = TiffDecoderCompressionType.Jpeg;
+
+                    if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr && options.JpegTables is null)
+                    {
+                        // Note: Setting PhotometricInterpretation and color type to RGB here, since the jpeg decoder will handle the conversion of the pixel data.
+                        options.PhotometricInterpretation = TiffPhotometricInterpretation.Rgb;
+                        options.ColorType = TiffColorType.Rgb;
+                    }
+
                     break;
                 }
 

tests/Directory.Build.targets

@@ -21,7 +21,7 @@
     <PackageReference Update="BenchmarkDotNet" Version="0.13.0" />
     <PackageReference Update="BenchmarkDotNet.Diagnostics.Windows" Version="0.13.0" Condition="'$(IsWindows)'=='true'" />
     <PackageReference Update="Colourful" Version="3.0.0" />
-    <PackageReference Update="Magick.NET-Q16-AnyCPU" Version="8.0.1" />
+    <PackageReference Update="Magick.NET-Q16-AnyCPU" Version="11.1.2" />
     <PackageReference Update="Microsoft.DotNet.RemoteExecutor" Version="6.0.0-beta.21311.3" />
     <PackageReference Update="Microsoft.DotNet.XUnitExtensions" Version="6.0.0-beta.21311.3" />
     <PackageReference Update="Moq" Version="4.14.6" />

tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs

@@ -303,8 +303,6 @@ public void TiffDecoder_CanDecode_24Bit_Gray<TPixel>(TestImageProvider<TPixel> p
         [Theory]
         [WithFile(FlowerYCbCr888Contiguous, PixelTypes.Rgba32)]
         [WithFile(FlowerYCbCr888Planar, PixelTypes.Rgba32)]
-        [WithFile(RgbYCbCr888Contiguoush1v1, PixelTypes.Rgba32)]
-        [WithFile(RgbYCbCr888Contiguoush2v1, PixelTypes.Rgba32)]
         [WithFile(RgbYCbCr888Contiguoush2v2, PixelTypes.Rgba32)]
         [WithFile(RgbYCbCr888Contiguoush4v4, PixelTypes.Rgba32)]
         [WithFile(FlowerYCbCr888Contiguoush2v1, PixelTypes.Rgba32)]
@@ -317,6 +315,12 @@ public void TiffDecoder_CanDecode_YCbCr_24Bit<TPixel>(TestImageProvider<TPixel>
             // converting the pixel data from Magick.NET to our format with YCbCr?
             using Image<TPixel> image = provider.GetImage();
             image.DebugSave(provider);
+
+#if NETCOREAPP
+            image.CompareToReferenceOutput(ImageComparer.Exact, provider);
+#else
+            image.CompareToReferenceOutput(TolerantImageComparer.TolerantPercentage(0.0002F), provider);
+#endif
         }
 
         [Theory]
@@ -642,10 +646,13 @@ public void CanDecodeJustOneFrame<TPixel>(TestImageProvider<TPixel> provider)
 
         [Theory]
         [WithFile(RgbJpegCompressed, PixelTypes.Rgba32)]
+        [WithFile(RgbJpegCompressed2, PixelTypes.Rgba32)]
         [WithFile(RgbWithStripsJpegCompressed, PixelTypes.Rgba32)]
         [WithFile(YCbCrJpegCompressed, PixelTypes.Rgba32)]
+        [WithFile(YCbCrJpegCompressed2, PixelTypes.Rgba32)]
         [WithFile(RgbJpegCompressedNoJpegTable, PixelTypes.Rgba32)]
         [WithFile(GrayscaleJpegCompressed, PixelTypes.Rgba32)]
+        [WithFile(Issues2123, PixelTypes.Rgba32)]
         public void TiffDecoder_CanDecode_JpegCompressed<TPixel>(TestImageProvider<TPixel> provider)
             where TPixel : unmanaged, IPixel<TPixel> => TestTiffDecoder(provider, useExactComparer: false);
 

tests/ImageSharp.Tests/TestImages.cs

@@ -778,6 +778,7 @@ public static class Tiff
             public const string RgbDeflatePredictor = "Tiff/rgb_deflate_predictor.tiff";
             public const string RgbDeflateMultistrip = "Tiff/rgb_deflate_multistrip.tiff";
             public const string RgbJpegCompressed = "Tiff/rgb_jpegcompression.tiff";
+            public const string RgbJpegCompressed2 = "Tiff/twain-rgb-jpeg-with-bogus-ycbcr-subsampling.tiff";
             public const string RgbWithStripsJpegCompressed = "Tiff/rgb_jpegcompressed_stripped.tiff";
             public const string RgbJpegCompressedNoJpegTable = "Tiff/rgb_jpegcompressed_nojpegtable.tiff";
             public const string RgbLzwPredictor = "Tiff/rgb_lzw_predictor.tiff";
@@ -823,11 +824,10 @@ public static class Tiff
             public const string FlowerYCbCr888Contiguoush2v1 = "Tiff/flower-ycbcr-contig-08_h2v1.tiff";
             public const string FlowerYCbCr888Contiguoush2v2 = "Tiff/flower-ycbcr-contig-08_h2v2.tiff";
             public const string FlowerYCbCr888Contiguoush4v4 = "Tiff/flower-ycbcr-contig-08_h4v4.tiff";
-            public const string RgbYCbCr888Contiguoush1v1 = "Tiff/rgb-ycbcr-contig-08_h1v1.tiff";
-            public const string RgbYCbCr888Contiguoush2v1 = "Tiff/rgb-ycbcr-contig-08_h2v1.tiff";
             public const string RgbYCbCr888Contiguoush2v2 = "Tiff/rgb-ycbcr-contig-08_h2v2.tiff";
             public const string RgbYCbCr888Contiguoush4v4 = "Tiff/rgb-ycbcr-contig-08_h4v4.tiff";
             public const string YCbCrJpegCompressed = "Tiff/ycbcr_jpegcompressed.tiff";
+            public const string YCbCrJpegCompressed2 = "Tiff/ycbcr_jpegcompressed2.tiff";
             public const string FlowerRgb444Contiguous = "Tiff/flower-rgb-contig-04.tiff";
             public const string FlowerRgb444Planar = "Tiff/flower-rgb-planar-04.tiff";
             public const string FlowerRgb222Contiguous = "Tiff/flower-rgb-contig-02.tiff";
@@ -910,6 +910,7 @@ public static class Tiff
 
             public const string Issues1716Rgb161616BitLittleEndian = "Tiff/Issues/Issue1716.tiff";
             public const string Issues1891 = "Tiff/Issues/Issue1891.tiff";
+            public const string Issues2123 = "Tiff/Issues/Issue2123.tiff";
 
             public const string SmallRgbDeflate = "Tiff/rgb_small_deflate.tiff";
             public const string SmallRgbLzw = "Tiff/rgb_small_lzw.tiff";

tests/Images/External/ReferenceOutput/TiffDecoderTests/TiffDecoder_CanDecode_YCbCr_24Bit_Rgba32_flower-ycbcr-contig-08_h1v1.png

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:5663288720203035c454c61c529222c2170df21dcde1a89f1f30e3b668020d6f
+size 3805

tests/Images/External/ReferenceOutput/TiffDecoderTests/TiffDecoder_CanDecode_YCbCr_24Bit_Rgba32_flower-ycbcr-contig-08_h2v1.png

@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:6b81013d7b0a29ed1ac9c33e175e0c0e69494b93b2b65b692f16d9ea042b9d5d
-size 7759
+oid sha256:3e889209fc31702aaa7c966c1b5370cc0904cbfbcfd17718977045049cc1bfd9
+size 5904

tests/Images/External/ReferenceOutput/TiffDecoderTests/TiffDecoder_CanDecode_YCbCr_24Bit_Rgba32_flower-ycbcr-contig-08_h2v2.png

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:d7a559d36e3852265ab4f82e43d28cc0bfc310813a5ced08e51c1366d8e323f9
+size 146853

tests/Images/External/ReferenceOutput/TiffDecoderTests/TiffDecoder_CanDecode_YCbCr_24Bit_Rgba32_flower-ycbcr-contig-08_h4v4.png

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:533f92b6a45c2de4dc0f3cdb8debf45dcfe84790cfa652404b2f44e15f06e44f
+size 38816