From 3b4bc1de2384239add75ae5ad33b2ee068ba79f1 Mon Sep 17 00:00:00 2001 From: Ildar Khayrutdinov Date: Sat, 13 Feb 2021 12:47:29 +0300 Subject: [PATCH] Remove TiffEncoderPixelStorageMethod, add CRC writing for deflate. Correct tests. --- .../Compressors/DeflateCompressor.cs | 2 +- .../Formats/Tiff/ITiffEncoderOptions.cs | 5 - src/ImageSharp/Formats/Tiff/TiffEncoder.cs | 5 +- .../Formats/Tiff/TiffEncoderCore.cs | 35 +++-- .../Tiff/TiffEncoderPixelStorageMethod.cs | 26 ---- .../Formats/Tiff/TiffEncoderTests.cs | 122 ++++++++++-------- 6 files changed, 86 insertions(+), 109 deletions(-) delete mode 100644 src/ImageSharp/Formats/Tiff/TiffEncoderPixelStorageMethod.cs diff --git a/src/ImageSharp/Formats/Tiff/Compression/Compressors/DeflateCompressor.cs b/src/ImageSharp/Formats/Tiff/Compression/Compressors/DeflateCompressor.cs index f4b6c6ad7f..64d3b1ea31 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Compressors/DeflateCompressor.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Compressors/DeflateCompressor.cs @@ -38,7 +38,7 @@ namespace SixLabors.ImageSharp.Formats.Experimental.Tiff.Compression.Compressors stream.Write(rows); stream.Flush(); - //// stream.Dispose(); // todo: dispose write crc + stream.Dispose(); int size = (int)this.memoryStream.Position; diff --git a/src/ImageSharp/Formats/Tiff/ITiffEncoderOptions.cs b/src/ImageSharp/Formats/Tiff/ITiffEncoderOptions.cs index 8d0a15ffe6..03a8328eca 100644 --- a/src/ImageSharp/Formats/Tiff/ITiffEncoderOptions.cs +++ b/src/ImageSharp/Formats/Tiff/ITiffEncoderOptions.cs @@ -38,11 +38,6 @@ namespace SixLabors.ImageSharp.Formats.Experimental.Tiff /// IQuantizer Quantizer { get; } - /// - /// Gets the pixel storage method. - /// - TiffEncoderPixelStorageMethod PixelStorageMethod { get; } - /// /// Gets the maximum size of strip (bytes). /// diff --git a/src/ImageSharp/Formats/Tiff/TiffEncoder.cs b/src/ImageSharp/Formats/Tiff/TiffEncoder.cs index 86091a5c48..5f747a6851 100644 --- a/src/ImageSharp/Formats/Tiff/TiffEncoder.cs +++ b/src/ImageSharp/Formats/Tiff/TiffEncoder.cs @@ -33,10 +33,7 @@ namespace SixLabors.ImageSharp.Formats.Experimental.Tiff public IQuantizer Quantizer { get; set; } /// - public TiffEncoderPixelStorageMethod PixelStorageMethod { get; set; } - - /// - public int MaxStripBytes { get; set; } + public int MaxStripBytes { get; set; } = TiffEncoderCore.DefaultStripSize; /// public void Encode(Image image, Stream stream) diff --git a/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs b/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs index f378e2fe8e..ec9c761aa4 100644 --- a/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs +++ b/src/ImageSharp/Formats/Tiff/TiffEncoderCore.cs @@ -25,14 +25,14 @@ namespace SixLabors.ImageSharp.Formats.Experimental.Tiff /// internal sealed class TiffEncoderCore : IImageEncoderInternals { + public const int DefaultStripSize = 8 * 1024; + public static readonly ByteOrder ByteOrder = BitConverter.IsLittleEndian ? ByteOrder.LittleEndian : ByteOrder.BigEndian; private static readonly ushort ByteOrderMarker = BitConverter.IsLittleEndian ? TiffConstants.ByteOrderLittleEndianShort : TiffConstants.ByteOrderBigEndianShort; - private const int DefaultStripSize = 8 * 1024; - /// /// Used for allocating memory during processing operations. /// @@ -58,8 +58,6 @@ namespace SixLabors.ImageSharp.Formats.Experimental.Tiff /// private readonly DeflateCompressionLevel compressionLevel; - private readonly TiffEncoderPixelStorageMethod storageMode; - private readonly int maxStripBytes; /// @@ -75,7 +73,6 @@ namespace SixLabors.ImageSharp.Formats.Experimental.Tiff this.quantizer = options.Quantizer ?? KnownQuantizers.Octree; this.UseHorizontalPredictor = options.UseHorizontalPredictor; this.compressionLevel = options.CompressionLevel; - this.storageMode = options.PixelStorageMethod; this.maxStripBytes = options.MaxStripBytes; } @@ -182,19 +179,10 @@ namespace SixLabors.ImageSharp.Formats.Experimental.Tiff private int CalcRowsPerStrip(ImageFrame image, int bytesPerRow) { - switch (this.storageMode) - { - default: - case TiffEncoderPixelStorageMethod.Auto: - case TiffEncoderPixelStorageMethod.MultiStrip: - int sz = this.maxStripBytes > 0 ? this.maxStripBytes : DefaultStripSize; - int height = sz / bytesPerRow; - - return height > 0 ? (height < image.Height ? height : image.Height) : 1; + int sz = this.maxStripBytes > 0 ? this.maxStripBytes : DefaultStripSize; + int height = sz / bytesPerRow; - case TiffEncoderPixelStorageMethod.SingleStrip: - return image.Height; - } + return height > 0 ? (height < image.Height ? height : image.Height) : 1; } /// @@ -258,9 +246,16 @@ namespace SixLabors.ImageSharp.Formats.Experimental.Tiff { if (this.CompressionType == TiffEncoderCompression.CcittGroup3Fax || this.CompressionType == TiffEncoderCompression.ModifiedHuffman) { - this.Mode = TiffEncodingMode.BiColor; - this.bitsPerPixel = TiffBitsPerPixel.Pixel1; - return; + if (this.Mode == TiffEncodingMode.Default) + { + this.Mode = TiffEncodingMode.BiColor; + this.bitsPerPixel = TiffBitsPerPixel.Pixel1; + return; + } + else if (this.Mode != TiffEncodingMode.BiColor) + { + TiffThrowHelper.ThrowImageFormatException($"The {this.CompressionType} compression and {this.Mode} aren't compatible. Please use {this.CompressionType} only with {TiffEncodingMode.BiColor} or {TiffEncodingMode.Default} mode."); + } } if (this.Mode == TiffEncodingMode.Default) diff --git a/src/ImageSharp/Formats/Tiff/TiffEncoderPixelStorageMethod.cs b/src/ImageSharp/Formats/Tiff/TiffEncoderPixelStorageMethod.cs deleted file mode 100644 index e1e12c08d0..0000000000 --- a/src/ImageSharp/Formats/Tiff/TiffEncoderPixelStorageMethod.cs +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright (c) Six Labors. -// Licensed under the Apache License, Version 2.0. - -namespace SixLabors.ImageSharp.Formats.Experimental.Tiff -{ - /// - /// The tiff encoder pixel storage method. - /// - public enum TiffEncoderPixelStorageMethod - { - /// - /// The auto mode. - /// - Auto, - - /// - /// The single strip mode. - /// - SingleStrip, - - /// - /// The multi strip mode. - /// - MultiStrip, - } -} diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs index 774454259d..b9368e4ca6 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. +using System; using System.IO; using SixLabors.ImageSharp.Formats; @@ -21,12 +22,12 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff { private static readonly IImageDecoder ReferenceDecoder = new MagickReferenceDecoder(); - private readonly Configuration configuration; + private static readonly Configuration Configuration; - public TiffEncoderTests() + static TiffEncoderTests() { - this.configuration = new Configuration(); - this.configuration.AddTiff(); + Configuration = new Configuration(); + Configuration.AddTiff(); } [Theory] @@ -62,7 +63,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff // assert memStream.Position = 0; - using var output = Image.Load(this.configuration, memStream); + using var output = Image.Load(Configuration, memStream); TiffMetadata meta = output.Metadata.GetTiffMetadata(); Assert.Equal(expectedBitsPerPixel, meta.BitsPerPixel); Assert.Equal(expectedCompression, meta.Compression); @@ -85,7 +86,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff // assert memStream.Position = 0; - using var output = Image.Load(this.configuration, memStream); + using var output = Image.Load(Configuration, memStream); TiffMetadata meta = output.Metadata.GetTiffMetadata(); Assert.Equal(expectedBitsPerPixel, meta.BitsPerPixel); } @@ -108,13 +109,30 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff // assert memStream.Position = 0; - using var output = Image.Load(this.configuration, memStream); + using var output = Image.Load(Configuration, memStream); TiffMetadata meta = output.Metadata.GetTiffMetadata(); Assert.Equal(TiffBitsPerPixel.Pixel1, meta.BitsPerPixel); Assert.Equal(expectedCompression, meta.Compression); } + [Theory] + [InlineData(TiffEncodingMode.ColorPalette, TiffEncoderCompression.CcittGroup3Fax)] + [InlineData(TiffEncodingMode.ColorPalette, TiffEncoderCompression.ModifiedHuffman)] + [InlineData(TiffEncodingMode.Gray, TiffEncoderCompression.ModifiedHuffman)] + [InlineData(TiffEncodingMode.Rgb, TiffEncoderCompression.ModifiedHuffman)] + public void TiffEncoder_IncompatibilityOptions(TiffEncodingMode mode, TiffEncoderCompression compression) + where TPixel : unmanaged, IPixel + { + // arrange + using var input = new Image(10, 10); + var encoder = new TiffEncoder() { Mode = mode, Compression = compression }; + using var memStream = new MemoryStream(); + + // act + Assert.Throws(() => input.Save(memStream, encoder)); + } + [Theory] [WithFile(Calliphora_RgbUncompressed, PixelTypes.Rgba32)] public void TiffEncoder_EncodeRgb_Works(TestImageProvider provider) @@ -207,30 +225,20 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff [Theory] [WithFile(Calliphora_PaletteUncompressed, PixelTypes.Rgba32)] - public void TiffEncoder_EncodeColorPalette_WithLzwCompression_Works(TestImageProvider provider) - where TPixel : unmanaged, IPixel - { - var encoder = new TiffEncoder { Mode = TiffEncodingMode.ColorPalette, Compression = TiffEncoderCompression.Lzw, PixelStorageMethod = TiffEncoderPixelStorageMethod.SingleStrip }; - - this.TiffEncoderPaletteTest(provider, encoder); - } - - [Theory] - [WithFile(Calliphora_PaletteUncompressed, PixelTypes.Rgba32)] - public void TiffEncoder_EncodeColorPalette_WithLzwCompressionAndPredictor_Works(TestImageProvider provider) + public void TiffEncoder_EncodeColorPalette_WithLzwCompression_Works_Bug(TestImageProvider provider) where TPixel : unmanaged, IPixel { - var encoder = new TiffEncoder { Mode = TiffEncodingMode.ColorPalette, Compression = TiffEncoderCompression.Lzw, UseHorizontalPredictor = true, PixelStorageMethod = TiffEncoderPixelStorageMethod.SingleStrip }; + var encoder = new TiffEncoder { Mode = TiffEncodingMode.ColorPalette, Compression = TiffEncoderCompression.Lzw }; this.TiffEncoderPaletteTest(provider, encoder); } [Theory] [WithFile(Calliphora_PaletteUncompressed, PixelTypes.Rgba32)] - public void TiffEncoder_EncodeColorPalette_WithLzwCompressionAndPredictor_Bug(TestImageProvider provider) + public void TiffEncoder_EncodeColorPalette_WithLzwCompressionAndPredictor_Works_Bug(TestImageProvider provider) where TPixel : unmanaged, IPixel { - var encoder = new TiffEncoder { Mode = TiffEncodingMode.ColorPalette, Compression = TiffEncoderCompression.Lzw, PixelStorageMethod = TiffEncoderPixelStorageMethod.MultiStrip, UseHorizontalPredictor = true }; + var encoder = new TiffEncoder { Mode = TiffEncodingMode.ColorPalette, Compression = TiffEncoderCompression.Lzw, UseHorizontalPredictor = true }; Assert.Throws(() => this.TiffEncoderPaletteTest(provider, encoder)); } @@ -257,7 +265,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff image.Save(memStream, encoder); memStream.Position = 0; - using var encodedImage = (Image)Image.Load(this.configuration, memStream); + using var encodedImage = (Image)Image.Load(Configuration, memStream); var encodedImagePath = provider.Utility.SaveTestOutputFile(encodedImage, "tiff", encoder); TiffTestUtils.CompareWithReferenceDecoder(encodedImagePath, encodedImage); } @@ -288,22 +296,30 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff where TPixel : unmanaged, IPixel => TestTiffEncoderCore(provider, TiffBitsPerPixel.Pixel1, TiffEncodingMode.BiColor, TiffEncoderCompression.ModifiedHuffman); [Theory] - [WithFile(Calliphora_BiColorUncompressed, PixelTypes.L8, TiffEncodingMode.BiColor, TiffEncoderPixelStorageMethod.SingleStrip)] - [WithFile(Calliphora_BiColorUncompressed, PixelTypes.L8, TiffEncodingMode.BiColor, TiffEncoderPixelStorageMethod.MultiStrip, 9 * 1024)] - [WithFile(GrayscaleUncompressed, PixelTypes.L8, TiffEncodingMode.Gray, TiffEncoderPixelStorageMethod.SingleStrip)] - [WithFile(GrayscaleUncompressed, PixelTypes.L8, TiffEncodingMode.Gray, TiffEncoderPixelStorageMethod.MultiStrip, 16 * 1024)] - [WithFile(PaletteDeflateMultistrip, PixelTypes.L8, TiffEncodingMode.ColorPalette, TiffEncoderPixelStorageMethod.SingleStrip)] - [WithFile(PaletteDeflateMultistrip, PixelTypes.L8, TiffEncodingMode.ColorPalette, TiffEncoderPixelStorageMethod.MultiStrip, 32 * 1024)] - [WithFile(RgbUncompressed, PixelTypes.Rgba32, TiffEncodingMode.Rgb, TiffEncoderPixelStorageMethod.SingleStrip)] - [WithFile(RgbUncompressed, PixelTypes.Rgba32, TiffEncodingMode.Rgb, TiffEncoderPixelStorageMethod.MultiStrip, 64 * 1024)] - public void TiffEncoder_StorageMethods(TestImageProvider provider, TiffEncodingMode mode, TiffEncoderPixelStorageMethod storageMethod, int maxSize = 0) + [WithFile(GrayscaleUncompressed, PixelTypes.L8, TiffEncodingMode.Gray, TiffEncoderCompression.PackBits, 16 * 1024)] + [WithFile(PaletteDeflateMultistrip, PixelTypes.L8, TiffEncodingMode.ColorPalette, TiffEncoderCompression.Lzw, 32 * 1024)] + [WithFile(RgbUncompressed, PixelTypes.Rgba32, TiffEncodingMode.Rgb, TiffEncoderCompression.Deflate, 64 * 1024)] + [WithFile(RgbUncompressed, PixelTypes.Rgb24, TiffEncodingMode.Rgb, TiffEncoderCompression.None, 10 * 1024)] + [WithFile(RgbUncompressed, PixelTypes.Rgba32, TiffEncodingMode.Rgb, TiffEncoderCompression.None, 30 * 1024)] + [WithFile(RgbUncompressed, PixelTypes.Rgb48, TiffEncodingMode.Rgb, TiffEncoderCompression.None, 70 * 1024)] + public void TiffEncoder_StripLength(TestImageProvider provider, TiffEncodingMode mode, TiffEncoderCompression compression, int maxSize) + where TPixel : unmanaged, IPixel => + TestStripLength(provider, mode, compression, maxSize); + + [Theory] + [WithFile(Calliphora_BiColorUncompressed, PixelTypes.L8, TiffEncodingMode.BiColor, TiffEncoderCompression.CcittGroup3Fax, 9 * 1024)] + public void TiffEncoder_StripLength_OutOfBounds(TestImageProvider provider, TiffEncodingMode mode, TiffEncoderCompression compression, int maxSize) + where TPixel : unmanaged, IPixel => + //// CcittGroup3Fax compressed data length can be larger than the original length + Assert.Throws(() => TestStripLength(provider, mode, compression, maxSize)); + + private static void TestStripLength(TestImageProvider provider, TiffEncodingMode mode, TiffEncoderCompression compression, int maxSize) where TPixel : unmanaged, IPixel { // arrange - var tiffEncoder = new TiffEncoder() { PixelStorageMethod = storageMethod, MaxStripBytes = maxSize }; - using Image input = provider.GetImage(); + var tiffEncoder = new TiffEncoder() { Mode = mode, Compression = compression, MaxStripBytes = maxSize }; + Image input = provider.GetImage(); using var memStream = new MemoryStream(); - TiffFrameMetadata inputMeta = input.Frames.RootFrame.Metadata.GetTiffMetadata(); // act @@ -311,24 +327,28 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff // assert memStream.Position = 0; - using var output = Image.Load(this.configuration, memStream); + var output = Image.Load(Configuration, memStream); TiffFrameMetadata meta = output.Frames.RootFrame.Metadata.GetTiffMetadata(); - if (storageMethod == TiffEncoderPixelStorageMethod.SingleStrip) + Assert.True(output.Height > (int)meta.RowsPerStrip); + Assert.True(meta.StripOffsets.Length > 1); + Assert.True(meta.StripByteCounts.Length > 1); + + foreach (Number sz in meta.StripByteCounts) { - Assert.Equal(output.Height, (int)meta.RowsPerStrip); - Assert.Equal(1, meta.StripOffsets.Length); - Assert.Equal(1, meta.StripByteCounts.Length); + Assert.True((uint)sz <= maxSize); } - else - { - Assert.True(output.Height > (int)meta.RowsPerStrip); - Assert.True(meta.StripOffsets.Length > 1); - Assert.True(meta.StripByteCounts.Length > 1); - foreach (Number sz in meta.StripByteCounts) + // for uncompressed more accurate test + if (compression == TiffEncoderCompression.None) + { + for (int i = 0; i < meta.StripByteCounts.Length - 1; i++) { - Assert.True((int)sz <= maxSize); + // the difference must be less than one row + int stripBytes = (int)meta.StripByteCounts[i]; + var widthBytes = (meta.BitsPerPixel + 7) / 8 * (int)meta.Width; + + Assert.True((maxSize - stripBytes) < widthBytes); } } @@ -338,9 +358,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff (TiffBitsPerPixel)inputMeta.BitsPerPixel, mode, Convert(inputMeta.Compression), - maxStripSize: maxSize, - storageMethod: storageMethod - ); + maxStripSize: maxSize); } private static void TestTiffEncoderCore( @@ -351,7 +369,6 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff bool usePredictor = false, bool useExactComparer = true, int maxStripSize = 0, - TiffEncoderPixelStorageMethod? storageMethod = null, float compareTolerance = 0.01f) where TPixel : unmanaged, IPixel { @@ -361,7 +378,6 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff Mode = mode, Compression = compression, UseHorizontalPredictor = usePredictor, - PixelStorageMethod = storageMethod ?? TiffEncoderPixelStorageMethod.Auto, MaxStripBytes = maxStripSize }; @@ -382,10 +398,10 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff return TiffEncoderCompression.Lzw; case TiffCompression.PackBits: return TiffEncoderCompression.PackBits; + case TiffCompression.Ccitt1D: + return TiffEncoderCompression.ModifiedHuffman; case TiffCompression.CcittGroup3Fax: return TiffEncoderCompression.CcittGroup3Fax; - case TiffCompression.CcittGroup4Fax: - return TiffEncoderCompression.ModifiedHuffman; } } }