From 14ced6f6d2594cba4a54711b134301c96407e841 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Tue, 12 Mar 2024 12:02:45 +0100 Subject: [PATCH 1/5] Set YcbcrSubSampling to 1, 1 with jpeg compression and PhotometricInterpretation YCbCr --- .../Formats/Tiff/TiffDecoderOptionsParser.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs index 5a5c2b3e51..7172bd6fe6 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -628,6 +628,12 @@ private static void ParseCompression(this TiffDecoderCore options, TiffCompressi options.ColorType = TiffColorType.Rgb; } + if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr) + { + options.YcbcrSubSampling[0] = 1; + options.YcbcrSubSampling[1] = 1; + } + break; } @@ -642,6 +648,12 @@ private static void ParseCompression(this TiffDecoderCore options, TiffCompressi options.ColorType = TiffColorType.Rgb; } + if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr) + { + options.YcbcrSubSampling[0] = 1; + options.YcbcrSubSampling[1] = 1; + } + break; } From 363769d28220ec8117ecee63f7c0a034b17aedbb Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Tue, 12 Mar 2024 12:21:58 +0100 Subject: [PATCH 2/5] Avoid code duplication --- .../Formats/Tiff/TiffDecoderOptionsParser.cs | 46 ++++++++----------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs index 7172bd6fe6..7b0f439b1c 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -620,19 +620,7 @@ private static void ParseCompression(this TiffDecoderCore options, TiffCompressi } options.CompressionType = TiffDecoderCompressionType.OldJpeg; - - if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr) - { - // 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; - } - - if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr) - { - options.YcbcrSubSampling[0] = 1; - options.YcbcrSubSampling[1] = 1; - } + AdjustOptionsYCbCrJpegCompression(options); break; } @@ -640,19 +628,7 @@ 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; - } - - if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr) - { - options.YcbcrSubSampling[0] = 1; - options.YcbcrSubSampling[1] = 1; - } + AdjustOptionsYCbCrJpegCompression(options); break; } @@ -671,6 +647,24 @@ private static void ParseCompression(this TiffDecoderCore options, TiffCompressi } } + private static void AdjustOptionsYCbCrJpegCompression(TiffDecoderCore options) + { + if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr) + { + // 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; + } + + if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr) + { + // Some tiff encoder set this to values different from [1, 1]. The jpeg decoder already handles this, + // so we set this always to [1, 1], see: https://github.com/SixLabors/ImageSharp/issues/2679 + options.YcbcrSubSampling[0] = 1; + options.YcbcrSubSampling[1] = 1; + } + } + private static bool IsBiColorCompression(TiffCompression? compression) { if (compression is TiffCompression.Ccitt1D or TiffCompression.CcittGroup3Fax or From e8bbdf703ab55acb98f39320b22093f0f20558e1 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 4 Aug 2024 15:29:06 +0200 Subject: [PATCH 3/5] Always set YcbcrSubSampling to 1:1 for TiffCompression.Jpeg, fixes issue #2679 --- .../Formats/Tiff/TiffDecoderOptionsParser.cs | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs index 7b0f439b1c..864452fd26 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -620,7 +620,12 @@ private static void ParseCompression(this TiffDecoderCore options, TiffCompressi } options.CompressionType = TiffDecoderCompressionType.OldJpeg; - AdjustOptionsYCbCrJpegCompression(options); + if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr) + { + // 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; } @@ -628,7 +633,20 @@ private static void ParseCompression(this TiffDecoderCore options, TiffCompressi case TiffCompression.Jpeg: { options.CompressionType = TiffDecoderCompressionType.Jpeg; - AdjustOptionsYCbCrJpegCompression(options); + 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; + } + + // Some tiff encoder set this to values different from [1, 1]. The jpeg decoder already handles this, + // so we set this always to [1, 1], see: https://github.com/SixLabors/ImageSharp/issues/2679 + if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr) + { + options.YcbcrSubSampling[0] = 1; + options.YcbcrSubSampling[1] = 1; + } break; } @@ -647,24 +665,6 @@ private static void ParseCompression(this TiffDecoderCore options, TiffCompressi } } - private static void AdjustOptionsYCbCrJpegCompression(TiffDecoderCore options) - { - if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr) - { - // 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; - } - - if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr) - { - // Some tiff encoder set this to values different from [1, 1]. The jpeg decoder already handles this, - // so we set this always to [1, 1], see: https://github.com/SixLabors/ImageSharp/issues/2679 - options.YcbcrSubSampling[0] = 1; - options.YcbcrSubSampling[1] = 1; - } - } - private static bool IsBiColorCompression(TiffCompression? compression) { if (compression is TiffCompression.Ccitt1D or TiffCompression.CcittGroup3Fax or From 978aff8208bd904b1bbd497f664e1389a7184d3b Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 4 Aug 2024 17:22:57 +0200 Subject: [PATCH 4/5] Add test case for issue 2679 --- .../Formats/Tiff/TiffDecoderTests.cs | 17 +++++++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + ...de_JpegCompressedWithIssue2679_Issue2679.png | 3 +++ tests/Images/Input/Tiff/Issues/Issue2679.tiff | 3 +++ 4 files changed, 24 insertions(+) create mode 100644 tests/Images/External/ReferenceOutput/TiffDecoderTests/TiffDecoder_CanDecode_JpegCompressedWithIssue2679_Issue2679.png create mode 100644 tests/Images/Input/Tiff/Issues/Issue2679.tiff diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index ab49805a35..97f02f3684 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -671,6 +671,23 @@ public void TiffDecoder_CanDecode_TiledWithNonEqualWidthAndHeight(TestIm public void TiffDecoder_CanDecode_BiColorWithMissingBitsPerSample(TestImageProvider provider) where TPixel : unmanaged, IPixel => TestTiffDecoder(provider); + // https://github.com/SixLabors/ImageSharp/issues/2679 + [Theory] + [WithFile(Issues2679, PixelTypes.Rgba32)] + public void TiffDecoder_CanDecode_JpegCompressedWithIssue2679(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(TiffDecoder.Instance); + + // The image is handcrafted to simulate issue 2679. ImageMagick will throw an expection here and wont decode, + // so we compare to rererence output instead. + image.DebugSave(provider); + image.CompareToReferenceOutput( + ImageComparer.Exact, + provider, + appendPixelTypeToFileName: false); + } + [Theory] [WithFile(JpegCompressedGray0000539558, PixelTypes.Rgba32)] public void TiffDecoder_ThrowsException_WithCircular_IFD_Offsets(TestImageProvider provider) diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index ac70792681..772437c4b0 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -1032,6 +1032,7 @@ public static class Tiff public const string Issues2255 = "Tiff/Issues/Issue2255.png"; public const string Issues2435 = "Tiff/Issues/Issue2435.tiff"; public const string Issues2587 = "Tiff/Issues/Issue2587.tiff"; + public const string Issues2679 = "Tiff/Issues/Issue2679.tiff"; public const string JpegCompressedGray0000539558 = "Tiff/Issues/JpegCompressedGray-0000539558.tiff"; public const string Tiled0000023664 = "Tiff/Issues/tiled-0000023664.tiff"; diff --git a/tests/Images/External/ReferenceOutput/TiffDecoderTests/TiffDecoder_CanDecode_JpegCompressedWithIssue2679_Issue2679.png b/tests/Images/External/ReferenceOutput/TiffDecoderTests/TiffDecoder_CanDecode_JpegCompressedWithIssue2679_Issue2679.png new file mode 100644 index 0000000000..6150aacb37 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/TiffDecoderTests/TiffDecoder_CanDecode_JpegCompressedWithIssue2679_Issue2679.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:6cd36c7e07a08e22cceecd28a056c80e5553a8c092bfc091e902d13bd5c46f4d +size 120054 diff --git a/tests/Images/Input/Tiff/Issues/Issue2679.tiff b/tests/Images/Input/Tiff/Issues/Issue2679.tiff new file mode 100644 index 0000000000..1bc49f079c --- /dev/null +++ b/tests/Images/Input/Tiff/Issues/Issue2679.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:feb938396b9d5b4c258244197ba382937a52c93f72cc91081c7e6810e4a3b94c +size 6136 From 5c60126377aefbf089ccf365df0880d3006b3c63 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 4 Aug 2024 17:35:33 +0200 Subject: [PATCH 5/5] Add check, if YcbcrSubSampling has a value --- .../Formats/Tiff/TiffDecoderOptionsParser.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs index 864452fd26..c3ff758ac1 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -633,21 +633,22 @@ 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; - } // Some tiff encoder set this to values different from [1, 1]. The jpeg decoder already handles this, // so we set this always to [1, 1], see: https://github.com/SixLabors/ImageSharp/issues/2679 - if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr) + if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr && options.YcbcrSubSampling != null) { options.YcbcrSubSampling[0] = 1; options.YcbcrSubSampling[1] = 1; } + 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; }