Merge pull request #2124 from SixLabors/bp/Issue2123

Jpeg compressed tiff: jpeg decoder should handle the conversion from YCbCr to RGB

Author: JimBobSquarePants

src/ImageSharp/Common/Helpers/Numerics.cs

@@ -968,7 +968,7 @@ public static uint RotateRightSoftwareFallback(uint value, int offset)
         /// Tells whether input value is outside of the given range.
         /// </summary>
         /// <param name="value">Value.</param>
-        /// <param name="min">Mininum value, inclusive.</param>
+        /// <param name="min">Minimum value, inclusive.</param>
         /// <param name="max">Maximum value, inclusive.</param>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static bool IsOutOfRange(int value, int min, int max)

src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs

@@ -90,6 +90,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>
@@ -514,27 +519,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);
@@ -701,6 +754,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/Compression/Decompressors/JpegTiffCompression.cs

@@ -65,13 +65,11 @@ protected override void Decompress(BufferedReadStream stream, int byteCount, int
                         jpegDecoder.ParseStream(stream, spectralConverterGray, CancellationToken.None);
 
                         // TODO: Should we pass through the CancellationToken from the tiff decoder?
-                        using var decompressedBuffer = spectralConverterGray.GetPixelBuffer(CancellationToken.None);
+                        using Buffer2D<L8> decompressedBuffer = spectralConverterGray.GetPixelBuffer(CancellationToken.None);
                         CopyImageBytesToBuffer(buffer, decompressedBuffer);
                         break;
                     }
 
-                    // If the PhotometricInterpretation is YCbCr we explicitly assume the JPEG data is in RGB color space.
-                    // There seems no other way to determine that the JPEG data is RGB colorspace (no APP14 marker, componentId's are not RGB).
                     case TiffPhotometricInterpretation.YCbCr:
                     case TiffPhotometricInterpretation.Rgb:
                     {
@@ -82,7 +80,7 @@ protected override void Decompress(BufferedReadStream stream, int byteCount, int
                         jpegDecoder.ParseStream(stream, spectralConverter, CancellationToken.None);
 
                         // TODO: Should we pass through the CancellationToken from the tiff decoder?
-                        using var decompressedBuffer = spectralConverter.GetPixelBuffer(CancellationToken.None);
+                        using Buffer2D<Rgb24> decompressedBuffer = spectralConverter.GetPixelBuffer(CancellationToken.None);
                         CopyImageBytesToBuffer(buffer, decompressedBuffer);
                         break;
                     }

src/ImageSharp/Formats/Tiff/PhotometricInterpretation/YCbCrConverter.cs

@@ -19,16 +19,16 @@ internal class YCbCrConverter
 
         private static readonly Rational[] DefaultLuma =
         {
-            new Rational(299, 1000),
-            new Rational(587, 1000),
-            new Rational(114, 1000)
+            new(299, 1000),
+            new(587, 1000),
+            new(114, 1000)
         };
 
         private static readonly Rational[] DefaultReferenceBlackWhite =
         {
-            new Rational(0, 1), new Rational(255, 1),
-            new Rational(128, 1), new Rational(255, 1),
-            new Rational(128, 1), new Rational(255, 1)
+            new(0, 1), new(255, 1),
+            new(128, 1), new(255, 1),
+            new(128, 1), new(255, 1)
         };
 
         public YCbCrConverter(Rational[] referenceBlackAndWhite, Rational[] coefficients)

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/TiffDecoderCore.cs

@@ -228,7 +228,7 @@ public IImageInfo Identify(BufferedReadStream stream, CancellationToken cancella
         /// <typeparam name="TPixel">The pixel format.</typeparam>
         /// <param name="tags">The IFD tags.</param>
         /// <param name="cancellationToken">The token to monitor cancellation.</param>
-        /// <returns> The tiff frame. </returns>
+        /// <returns>The tiff frame.</returns>
         private ImageFrame<TPixel> DecodeFrame<TPixel>(ExifProfile tags, CancellationToken cancellationToken)
             where TPixel : unmanaged, IPixel<TPixel>
         {

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,7 @@ 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);
+            image.CompareToReferenceOutput(ImageComparer.Exact, provider);
         }
 
         [Theory]
@@ -642,10 +641,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);