From 14ced6f6d2594cba4a54711b134301c96407e841 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Tue, 12 Mar 2024 12:02:45 +0100 Subject: [PATCH 1/7] 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 5a5c2b3e5..7172bd6fe 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -628,6 +628,12 @@ internal static class TiffDecoderOptionsParser options.ColorType = TiffColorType.Rgb; } + if (options.PhotometricInterpretation is TiffPhotometricInterpretation.YCbCr) + { + options.YcbcrSubSampling[0] = 1; + options.YcbcrSubSampling[1] = 1; + } + break; } @@ -642,6 +648,12 @@ internal static class TiffDecoderOptionsParser 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/7] 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 7172bd6fe..7b0f439b1 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -620,19 +620,7 @@ internal static class TiffDecoderOptionsParser } 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 @@ internal static class TiffDecoderOptionsParser 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 @@ internal static class TiffDecoderOptionsParser } } + 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 1102eb54e97ee137d19e8b0fa796f213b36f0d4c Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 1 Aug 2024 11:28:00 +1000 Subject: [PATCH 3/7] Replace PngCrcChunkHandling --- src/ImageSharp/Formats/DecoderOptions.cs | 5 ++++ src/ImageSharp/Formats/Png/PngChunk.cs | 10 +++---- .../Formats/Png/PngCrcChunkHandling.cs | 30 ------------------- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 12 ++++---- .../Formats/Png/PngDecoderOptions.cs | 5 ---- .../Formats/SegmentErrorHandling.cs | 30 +++++++++++++++++++ .../Formats/Png/PngDecoderTests.cs | 18 +++++++++-- 7 files changed, 62 insertions(+), 48 deletions(-) delete mode 100644 src/ImageSharp/Formats/Png/PngCrcChunkHandling.cs create mode 100644 src/ImageSharp/Formats/SegmentErrorHandling.cs diff --git a/src/ImageSharp/Formats/DecoderOptions.cs b/src/ImageSharp/Formats/DecoderOptions.cs index 6243a071d..be906d7d9 100644 --- a/src/ImageSharp/Formats/DecoderOptions.cs +++ b/src/ImageSharp/Formats/DecoderOptions.cs @@ -55,5 +55,10 @@ public sealed class DecoderOptions /// public uint MaxFrames { get => this.maxFrames; init => this.maxFrames = Math.Clamp(value, 1, int.MaxValue); } + /// + /// Gets the segment error handling strategy to use during decoding. + /// + public SegmentErrorHandling SegmentErrorHandling { get; init; } = SegmentErrorHandling.IgnoreNonCritical; + internal void SetConfiguration(Configuration configuration) => this.configuration = configuration; } diff --git a/src/ImageSharp/Formats/Png/PngChunk.cs b/src/ImageSharp/Formats/Png/PngChunk.cs index 6ec0df9ad..666f51daa 100644 --- a/src/ImageSharp/Formats/Png/PngChunk.cs +++ b/src/ImageSharp/Formats/Png/PngChunk.cs @@ -41,13 +41,13 @@ internal readonly struct PngChunk /// /// Gets a value indicating whether the given chunk is critical to decoding /// - /// The chunk CRC handling behavior. - public bool IsCritical(PngCrcChunkHandling handling) + /// The segment handling behavior. + public bool IsCritical(SegmentErrorHandling handling) => handling switch { - PngCrcChunkHandling.IgnoreNone => true, - PngCrcChunkHandling.IgnoreNonCritical => this.Type is PngChunkType.Header or PngChunkType.Palette or PngChunkType.Data or PngChunkType.FrameData, - PngCrcChunkHandling.IgnoreData => this.Type is PngChunkType.Header or PngChunkType.Palette, + SegmentErrorHandling.IgnoreNone => true, + SegmentErrorHandling.IgnoreNonCritical => this.Type is PngChunkType.Header or PngChunkType.Palette or PngChunkType.Data or PngChunkType.FrameData, + SegmentErrorHandling.IgnoreData => this.Type is PngChunkType.Header or PngChunkType.Palette, _ => false, }; } diff --git a/src/ImageSharp/Formats/Png/PngCrcChunkHandling.cs b/src/ImageSharp/Formats/Png/PngCrcChunkHandling.cs deleted file mode 100644 index 264d737fd..000000000 --- a/src/ImageSharp/Formats/Png/PngCrcChunkHandling.cs +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright (c) Six Labors. -// Licensed under the Six Labors Split License. - -namespace SixLabors.ImageSharp.Formats.Png; - -/// -/// Specifies how to handle validation of any CRC (Cyclic Redundancy Check) data within the encoded PNG. -/// -public enum PngCrcChunkHandling -{ - /// - /// Do not ignore any CRC chunk errors. - /// - IgnoreNone, - - /// - /// Ignore CRC errors in non critical chunks. - /// - IgnoreNonCritical, - - /// - /// Ignore CRC errors in data chunks. - /// - IgnoreData, - - /// - /// Ignore CRC errors in all chunks. - /// - IgnoreAll -} diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 01f038141..080c35c0d 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -119,7 +119,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore /// /// How to handle CRC errors. /// - private readonly PngCrcChunkHandling pngCrcChunkHandling; + private readonly SegmentErrorHandling segmentErrorHandling; /// /// A reusable Crc32 hashing instance. @@ -142,7 +142,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore this.maxFrames = options.GeneralOptions.MaxFrames; this.skipMetadata = options.GeneralOptions.SkipMetadata; this.memoryAllocator = this.configuration.MemoryAllocator; - this.pngCrcChunkHandling = options.PngCrcChunkHandling; + this.segmentErrorHandling = options.GeneralOptions.SegmentErrorHandling; this.maxUncompressedLength = options.MaxUncompressedAncillaryChunkSizeBytes; } @@ -154,7 +154,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore this.skipMetadata = true; this.configuration = options.GeneralOptions.Configuration; this.memoryAllocator = this.configuration.MemoryAllocator; - this.pngCrcChunkHandling = options.PngCrcChunkHandling; + this.segmentErrorHandling = options.GeneralOptions.SegmentErrorHandling; this.maxUncompressedLength = options.MaxUncompressedAncillaryChunkSizeBytes; } @@ -833,7 +833,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore break; default: - if (this.pngCrcChunkHandling is PngCrcChunkHandling.IgnoreData or PngCrcChunkHandling.IgnoreAll) + if (this.segmentErrorHandling is SegmentErrorHandling.IgnoreData or SegmentErrorHandling.IgnoreAll) { goto EXIT; } @@ -939,7 +939,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore break; default: - if (this.pngCrcChunkHandling is PngCrcChunkHandling.IgnoreData or PngCrcChunkHandling.IgnoreAll) + if (this.segmentErrorHandling is SegmentErrorHandling.IgnoreData or SegmentErrorHandling.IgnoreAll) { goto EXIT; } @@ -1927,7 +1927,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore private void ValidateChunk(in PngChunk chunk, Span buffer) { uint inputCrc = this.ReadChunkCrc(buffer); - if (chunk.IsCritical(this.pngCrcChunkHandling)) + if (chunk.IsCritical(this.segmentErrorHandling)) { Span chunkType = stackalloc byte[4]; BinaryPrimitives.WriteUInt32BigEndian(chunkType, (uint)chunk.Type); diff --git a/src/ImageSharp/Formats/Png/PngDecoderOptions.cs b/src/ImageSharp/Formats/Png/PngDecoderOptions.cs index abfa4b1da..a73db8777 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderOptions.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderOptions.cs @@ -11,11 +11,6 @@ public sealed class PngDecoderOptions : ISpecializedDecoderOptions /// public DecoderOptions GeneralOptions { get; init; } = new DecoderOptions(); - /// - /// Gets a value indicating how to handle validation of any CRC (Cyclic Redundancy Check) data within the encoded PNG. - /// - public PngCrcChunkHandling PngCrcChunkHandling { get; init; } = PngCrcChunkHandling.IgnoreNonCritical; - /// /// Gets the maximum memory in bytes that a zTXt, sPLT, iTXt, iCCP, or unknown chunk can occupy when decompressed. /// Defaults to 8MB diff --git a/src/ImageSharp/Formats/SegmentErrorHandling.cs b/src/ImageSharp/Formats/SegmentErrorHandling.cs new file mode 100644 index 000000000..7b28de589 --- /dev/null +++ b/src/ImageSharp/Formats/SegmentErrorHandling.cs @@ -0,0 +1,30 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +namespace SixLabors.ImageSharp.Formats; + +/// +/// Specifies how to handle validation of errors in different segments of encoded image files. +/// +public enum SegmentErrorHandling +{ + /// + /// Do not ignore any errors. + /// + IgnoreNone, + + /// + /// Ignore errors in non-critical segments of the encoded image. + /// + IgnoreNonCritical, + + /// + /// Ignore errors in data segments (e.g., image data, metadata). + /// + IgnoreData, + + /// + /// Ignore errors in all segments. + /// + IgnoreAll +} diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 8492d78f8..c3a6b11f5 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -381,6 +381,20 @@ public partial class PngDecoderTests Assert.Equal(expectedPixelSize, imageInfo.PixelType.BitsPerPixel); } + [Theory] + [InlineData(TestImages.Png.Bad.WrongCrcDataChunk, 1)] + [InlineData(TestImages.Png.Bad.Issue2589, 24)] + public void Identify_IgnoreCrcErrors(string imagePath, int expectedPixelSize) + { + TestFile testFile = TestFile.Create(imagePath); + using MemoryStream stream = new(testFile.Bytes, false); + + ImageInfo imageInfo = Image.Identify(new DecoderOptions() { SegmentErrorHandling = SegmentErrorHandling.IgnoreData }, stream); + + Assert.NotNull(imageInfo); + Assert.Equal(expectedPixelSize, imageInfo.PixelType.BitsPerPixel); + } + [Theory] [WithFile(TestImages.Png.Bad.MissingDataChunk, PixelTypes.Rgba32)] public void Decode_MissingDataChunk_ThrowsException(TestImageProvider provider) @@ -479,7 +493,7 @@ public partial class PngDecoderTests public void Decode_InvalidDataChunkCrc_IgnoreCrcErrors(TestImageProvider provider, bool compare) where TPixel : unmanaged, IPixel { - using Image image = provider.GetImage(PngDecoder.Instance, new PngDecoderOptions() { PngCrcChunkHandling = PngCrcChunkHandling.IgnoreData }); + using Image image = provider.GetImage(PngDecoder.Instance, new DecoderOptions() { SegmentErrorHandling = SegmentErrorHandling.IgnoreData }); image.DebugSave(provider); if (compare) @@ -660,7 +674,7 @@ public partial class PngDecoderTests public void Binary_PrematureEof() { PngDecoder decoder = PngDecoder.Instance; - PngDecoderOptions options = new() { PngCrcChunkHandling = PngCrcChunkHandling.IgnoreData }; + PngDecoderOptions options = new() { GeneralOptions = new() { SegmentErrorHandling = SegmentErrorHandling.IgnoreData } }; using EofHitCounter eofHitCounter = EofHitCounter.RunDecoder(TestImages.Png.Bad.FlagOfGermany0000016446, decoder, options); // TODO: Try to reduce this to 1. From 4a6b51bc288f13978adcc59f618615216c5db956 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 1 Aug 2024 15:25:41 +1000 Subject: [PATCH 4/7] Rename --- src/ImageSharp/Formats/DecoderOptions.cs | 2 +- src/ImageSharp/Formats/Png/PngChunk.cs | 8 ++++---- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 12 ++++++------ ...tErrorHandling.cs => SegmentIntegrityHandling.cs} | 2 +- .../ImageSharp.Tests/Formats/Png/PngDecoderTests.cs | 6 +++--- 5 files changed, 15 insertions(+), 15 deletions(-) rename src/ImageSharp/Formats/{SegmentErrorHandling.cs => SegmentIntegrityHandling.cs} (94%) diff --git a/src/ImageSharp/Formats/DecoderOptions.cs b/src/ImageSharp/Formats/DecoderOptions.cs index be906d7d9..3b16159b7 100644 --- a/src/ImageSharp/Formats/DecoderOptions.cs +++ b/src/ImageSharp/Formats/DecoderOptions.cs @@ -58,7 +58,7 @@ public sealed class DecoderOptions /// /// Gets the segment error handling strategy to use during decoding. /// - public SegmentErrorHandling SegmentErrorHandling { get; init; } = SegmentErrorHandling.IgnoreNonCritical; + public SegmentIntegrityHandling SegmentIntegrityHandling { get; init; } = SegmentIntegrityHandling.IgnoreNonCritical; internal void SetConfiguration(Configuration configuration) => this.configuration = configuration; } diff --git a/src/ImageSharp/Formats/Png/PngChunk.cs b/src/ImageSharp/Formats/Png/PngChunk.cs index 666f51daa..3883986d0 100644 --- a/src/ImageSharp/Formats/Png/PngChunk.cs +++ b/src/ImageSharp/Formats/Png/PngChunk.cs @@ -42,12 +42,12 @@ internal readonly struct PngChunk /// Gets a value indicating whether the given chunk is critical to decoding /// /// The segment handling behavior. - public bool IsCritical(SegmentErrorHandling handling) + public bool IsCritical(SegmentIntegrityHandling handling) => handling switch { - SegmentErrorHandling.IgnoreNone => true, - SegmentErrorHandling.IgnoreNonCritical => this.Type is PngChunkType.Header or PngChunkType.Palette or PngChunkType.Data or PngChunkType.FrameData, - SegmentErrorHandling.IgnoreData => this.Type is PngChunkType.Header or PngChunkType.Palette, + SegmentIntegrityHandling.IgnoreNone => true, + SegmentIntegrityHandling.IgnoreNonCritical => this.Type is PngChunkType.Header or PngChunkType.Palette or PngChunkType.Data or PngChunkType.FrameData, + SegmentIntegrityHandling.IgnoreData => this.Type is PngChunkType.Header or PngChunkType.Palette, _ => false, }; } diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 080c35c0d..484241d52 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -119,7 +119,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore /// /// How to handle CRC errors. /// - private readonly SegmentErrorHandling segmentErrorHandling; + private readonly SegmentIntegrityHandling segmentIntegrityHandling; /// /// A reusable Crc32 hashing instance. @@ -142,7 +142,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore this.maxFrames = options.GeneralOptions.MaxFrames; this.skipMetadata = options.GeneralOptions.SkipMetadata; this.memoryAllocator = this.configuration.MemoryAllocator; - this.segmentErrorHandling = options.GeneralOptions.SegmentErrorHandling; + this.segmentIntegrityHandling = options.GeneralOptions.SegmentIntegrityHandling; this.maxUncompressedLength = options.MaxUncompressedAncillaryChunkSizeBytes; } @@ -154,7 +154,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore this.skipMetadata = true; this.configuration = options.GeneralOptions.Configuration; this.memoryAllocator = this.configuration.MemoryAllocator; - this.segmentErrorHandling = options.GeneralOptions.SegmentErrorHandling; + this.segmentIntegrityHandling = options.GeneralOptions.SegmentIntegrityHandling; this.maxUncompressedLength = options.MaxUncompressedAncillaryChunkSizeBytes; } @@ -833,7 +833,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore break; default: - if (this.segmentErrorHandling is SegmentErrorHandling.IgnoreData or SegmentErrorHandling.IgnoreAll) + if (this.segmentIntegrityHandling is SegmentIntegrityHandling.IgnoreData or SegmentIntegrityHandling.IgnoreAll) { goto EXIT; } @@ -939,7 +939,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore break; default: - if (this.segmentErrorHandling is SegmentErrorHandling.IgnoreData or SegmentErrorHandling.IgnoreAll) + if (this.segmentIntegrityHandling is SegmentIntegrityHandling.IgnoreData or SegmentIntegrityHandling.IgnoreAll) { goto EXIT; } @@ -1927,7 +1927,7 @@ internal sealed class PngDecoderCore : ImageDecoderCore private void ValidateChunk(in PngChunk chunk, Span buffer) { uint inputCrc = this.ReadChunkCrc(buffer); - if (chunk.IsCritical(this.segmentErrorHandling)) + if (chunk.IsCritical(this.segmentIntegrityHandling)) { Span chunkType = stackalloc byte[4]; BinaryPrimitives.WriteUInt32BigEndian(chunkType, (uint)chunk.Type); diff --git a/src/ImageSharp/Formats/SegmentErrorHandling.cs b/src/ImageSharp/Formats/SegmentIntegrityHandling.cs similarity index 94% rename from src/ImageSharp/Formats/SegmentErrorHandling.cs rename to src/ImageSharp/Formats/SegmentIntegrityHandling.cs index 7b28de589..977aee4ad 100644 --- a/src/ImageSharp/Formats/SegmentErrorHandling.cs +++ b/src/ImageSharp/Formats/SegmentIntegrityHandling.cs @@ -6,7 +6,7 @@ namespace SixLabors.ImageSharp.Formats; /// /// Specifies how to handle validation of errors in different segments of encoded image files. /// -public enum SegmentErrorHandling +public enum SegmentIntegrityHandling { /// /// Do not ignore any errors. diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index c3a6b11f5..9f3c5f682 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -389,7 +389,7 @@ public partial class PngDecoderTests TestFile testFile = TestFile.Create(imagePath); using MemoryStream stream = new(testFile.Bytes, false); - ImageInfo imageInfo = Image.Identify(new DecoderOptions() { SegmentErrorHandling = SegmentErrorHandling.IgnoreData }, stream); + ImageInfo imageInfo = Image.Identify(new DecoderOptions() { SegmentIntegrityHandling = SegmentIntegrityHandling.IgnoreData }, stream); Assert.NotNull(imageInfo); Assert.Equal(expectedPixelSize, imageInfo.PixelType.BitsPerPixel); @@ -493,7 +493,7 @@ public partial class PngDecoderTests public void Decode_InvalidDataChunkCrc_IgnoreCrcErrors(TestImageProvider provider, bool compare) where TPixel : unmanaged, IPixel { - using Image image = provider.GetImage(PngDecoder.Instance, new DecoderOptions() { SegmentErrorHandling = SegmentErrorHandling.IgnoreData }); + using Image image = provider.GetImage(PngDecoder.Instance, new DecoderOptions() { SegmentIntegrityHandling = SegmentIntegrityHandling.IgnoreData }); image.DebugSave(provider); if (compare) @@ -674,7 +674,7 @@ public partial class PngDecoderTests public void Binary_PrematureEof() { PngDecoder decoder = PngDecoder.Instance; - PngDecoderOptions options = new() { GeneralOptions = new() { SegmentErrorHandling = SegmentErrorHandling.IgnoreData } }; + PngDecoderOptions options = new() { GeneralOptions = new() { SegmentIntegrityHandling = SegmentIntegrityHandling.IgnoreData } }; using EofHitCounter eofHitCounter = EofHitCounter.RunDecoder(TestImages.Png.Bad.FlagOfGermany0000016446, decoder, options); // TODO: Try to reduce this to 1. From e8bbdf703ab55acb98f39320b22093f0f20558e1 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 4 Aug 2024 15:29:06 +0200 Subject: [PATCH 5/7] 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 7b0f439b1..864452fd2 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -620,7 +620,12 @@ internal static class TiffDecoderOptionsParser } 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 @@ internal static class TiffDecoderOptionsParser 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 @@ internal static class TiffDecoderOptionsParser } } - 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 6/7] 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 ab49805a3..97f02f368 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -671,6 +671,23 @@ public class TiffDecoderTests : TiffDecoderBaseTester 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 ac7079268..772437c4b 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -1032,6 +1032,7 @@ public static class TestImages 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 000000000..6150aacb3 --- /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 000000000..1bc49f079 --- /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 7/7] 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 864452fd2..c3ff758ac 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -633,21 +633,22 @@ internal static class TiffDecoderOptionsParser 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; }