#12 Correct LZW decoder for Tiff

src/ImageSharp/Formats/Tiff/Compression/LzwTiffCompression.cs

@@ -21,10 +21,8 @@ public LzwTiffCompression(MemoryAllocator allocator)
         public override void Decompress(Stream stream, int byteCount, Span<byte> buffer)
         {
             var subStream = new SubStream(stream, byteCount);
-            using (var decoder = new TiffLzwDecoder(subStream))
-            {
-                decoder.DecodePixels(buffer.Length, 8, buffer);
-            }
+            var decoder = new TiffLzwDecoder(subStream, this.Allocator);
+            decoder.DecodePixels(buffer.Length, 8, buffer);
         }
     }
 }

src/ImageSharp/Formats/Tiff/Utils/TiffLzwDecoder.cs

@@ -5,6 +5,7 @@
 using System.Buffers;
 using System.IO;
 using SixLabors.ImageSharp.Formats.Gif;
+using SixLabors.ImageSharp.Memory;
 
 namespace SixLabors.ImageSharp.Formats.Tiff
 {
@@ -19,7 +20,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff
     /// byte indicating the length of the sub-block. In TIFF the data is written as a single block
     /// with no length indicator (this can be determined from the 'StripByteCounts' entry).
     /// </remarks>
-    internal sealed class TiffLzwDecoder : IDisposable
+    internal sealed class TiffLzwDecoder
     {
         /// <summary>
         /// The max decoder pixel stack size.
@@ -37,52 +38,23 @@ internal sealed class TiffLzwDecoder : IDisposable
         private readonly Stream stream;
 
         /// <summary>
-        /// The prefix buffer.
+        /// The memory allocator.
         /// </summary>
-        private readonly int[] prefix;
+        private readonly MemoryAllocator allocator;
 
         /// <summary>
-        /// The suffix buffer.
-        /// </summary>
-        private readonly int[] suffix;
-
-        /// <summary>
-        /// The pixel stack buffer.
-        /// </summary>
-        private readonly int[] pixelStack;
-
-        /// <summary>
-        /// A value indicating whether this instance of the given entity has been disposed.
-        /// </summary>
-        /// <value><see langword="true"/> if this instance has been disposed; otherwise, <see langword="false"/>.</value>
-        /// <remarks>
-        /// If the entity is disposed, it must not be disposed a second
-        /// time. The isDisposed field is set the first time the entity
-        /// is disposed. If the isDisposed field is true, then the Dispose()
-        /// method will not dispose again. This help not to prolong the entity's
-        /// life in the Garbage Collector.
-        /// </remarks>
-        private bool isDisposed;
-
-        /// <summary>
-        /// Initializes a new instance of the <see cref="TiffLzwDecoder"/> class
+        /// Initializes a new instance of the <see cref="TiffLzwDecoder" /> class
         /// and sets the stream, where the compressed data should be read from.
         /// </summary>
         /// <param name="stream">The stream to read from.</param>
-        /// <exception cref="System.ArgumentNullException"><paramref name="stream"/> is null.</exception>
-        public TiffLzwDecoder(Stream stream)
+        /// <param name="allocator">The memory allocator.</param>
+        /// <exception cref="System.ArgumentNullException"><paramref name="stream" /> is null.</exception>
+        public TiffLzwDecoder(Stream stream, MemoryAllocator allocator)
         {
             Guard.NotNull(stream, nameof(stream));
 
             this.stream = stream;
-
-            this.prefix = ArrayPool<int>.Shared.Rent(MaxStackSize);
-            this.suffix = ArrayPool<int>.Shared.Rent(MaxStackSize);
-            this.pixelStack = ArrayPool<int>.Shared.Rent(MaxStackSize + 1);
-
-            Array.Clear(this.prefix, 0, MaxStackSize);
-            Array.Clear(this.suffix, 0, MaxStackSize);
-            Array.Clear(this.pixelStack, 0, MaxStackSize + 1);
+            this.allocator = allocator;
         }
 
         /// <summary>
@@ -95,6 +67,15 @@ public void DecodePixels(int length, int dataSize, Span<byte> pixels)
         {
             Guard.MustBeLessThan(dataSize, int.MaxValue, nameof(dataSize));
 
+            // Initialize buffers
+            using IMemoryOwner<int> prefixMemory = this.allocator.Allocate<int>(MaxStackSize, AllocationOptions.Clean);
+            using IMemoryOwner<int> suffixMemory = this.allocator.Allocate<int>(MaxStackSize, AllocationOptions.Clean);
+            using IMemoryOwner<int> pixelStackMemory = this.allocator.Allocate<int>(MaxStackSize + 1, AllocationOptions.Clean);
+
+            Span<int> prefix = prefixMemory.GetSpan();
+            Span<int> suffix = suffixMemory.GetSpan();
+            Span<int> pixelStack = pixelStackMemory.GetSpan();
+
             // Calculate the clear code. The value of the clear code is 2 ^ dataSize
             int clearCode = 1 << dataSize;
 
@@ -111,54 +92,39 @@ public void DecodePixels(int length, int dataSize, Span<byte> pixels)
             int code;
             int oldCode = NullCode;
             int codeMask = (1 << codeSize) - 1;
+
+            int inputByte = 0;
             int bits = 0;
 
             int top = 0;
-            int count = 0;
-            int bi = 0;
             int xyz = 0;
 
-            int data = 0;
             int first = 0;
 
             for (code = 0; code < clearCode; code++)
             {
-                this.prefix[code] = 0;
-                this.suffix[code] = (byte)code;
+                prefix[code] = 0;
+                suffix[code] = (byte)code;
             }
 
-            byte[] buffer = new byte[255];
+            // Decoding process
             while (xyz < length)
             {
                 if (top == 0)
                 {
-                    if (bits < codeSize)
-                    {
-                        // Load bytes until there are enough bits for a code.
-                        if (count == 0)
-                        {
-                            // Read a new data block.
-                            count = this.ReadBlock(buffer);
-                            if (count == 0)
-                            {
-                                break;
-                            }
-
-                            bi = 0;
-                        }
-
-                        data += buffer[bi] << bits;
+                    // Get the next code
+                    int data = inputByte & ((1 << bits) - 1);
 
+                    while (bits < codeSize)
+                    {
+                        inputByte = this.stream.ReadByte();
+                        data = (data << 8) | inputByte;
                         bits += 8;
-                        bi++;
-                        count--;
-                        continue;
                     }
 
-                    // Get the next code
-                    code = data & codeMask;
-                    data >>= codeSize;
+                    data >>= bits - codeSize;
                     bits -= codeSize;
+                    code = data & codeMask;
 
                     // Interpret the code
                     if (code > availableCode || code == endCode)
@@ -178,7 +144,7 @@ public void DecodePixels(int length, int dataSize, Span<byte> pixels)
 
                     if (oldCode == NullCode)
                     {
-                        this.pixelStack[top++] = this.suffix[code];
+                        pixelStack[top++] = suffix[code];
                         oldCode = code;
                         first = code;
                         continue;
@@ -187,27 +153,27 @@ public void DecodePixels(int length, int dataSize, Span<byte> pixels)
                     int inCode = code;
                     if (code == availableCode)
                     {
-                        this.pixelStack[top++] = (byte)first;
+                        pixelStack[top++] = (byte)first;
 
                         code = oldCode;
                     }
 
                     while (code > clearCode)
                     {
-                        this.pixelStack[top++] = this.suffix[code];
-                        code = this.prefix[code];
+                        pixelStack[top++] = suffix[code];
+                        code = prefix[code];
                     }
 
-                    first = this.suffix[code];
+                    first = suffix[code];
 
-                    this.pixelStack[top++] = this.suffix[code];
+                    pixelStack[top++] = suffix[code];
 
                     // Fix for Gifs that have "deferred clear code" as per here :
                     // https://bugzilla.mozilla.org/show_bug.cgi?id=55918
                     if (availableCode < MaxStackSize)
                     {
-                        this.prefix[availableCode] = oldCode;
-                        this.suffix[availableCode] = first;
+                        prefix[availableCode] = oldCode;
+                        suffix[availableCode] = first;
                         availableCode++;
                         if (availableCode == codeMask + 1 && availableCode < MaxStackSize)
                         {
@@ -223,49 +189,8 @@ public void DecodePixels(int length, int dataSize, Span<byte> pixels)
                 top--;
 
                 // Clear missing pixels
-                pixels[xyz++] = (byte)this.pixelStack[top];
+                pixels[xyz++] = (byte)pixelStack[top];
             }
         }
-
-        /// <inheritdoc />
-        public void Dispose()
-        {
-            // Do not change this code. Put cleanup code in Dispose(bool disposing) above.
-            this.Dispose(true);
-        }
-
-        /// <summary>
-        /// Reads the next data block from the stream. For consistency with the GIF decoder,
-        /// the image is read in blocks - For TIFF this is always a maximum of 255
-        /// </summary>
-        /// <param name="buffer">The buffer to store the block in.</param>
-        /// <returns>
-        /// The <see cref="T:byte[]"/>.
-        /// </returns>
-        private int ReadBlock(byte[] buffer)
-        {
-            return this.stream.Read(buffer, 0, 255);
-        }
-
-        /// <summary>
-        /// Disposes the object and frees resources for the Garbage Collector.
-        /// </summary>
-        /// <param name="disposing">If true, the object gets disposed.</param>
-        private void Dispose(bool disposing)
-        {
-            if (this.isDisposed)
-            {
-                return;
-            }
-
-            if (disposing)
-            {
-                ArrayPool<int>.Shared.Return(this.prefix);
-                ArrayPool<int>.Shared.Return(this.suffix);
-                ArrayPool<int>.Shared.Return(this.pixelStack);
-            }
-
-            this.isDisposed = true;
-        }
     }
 }

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

@@ -53,6 +53,27 @@ public void Identify(string imagePath, int expectedPixelSize, int expectedWidth,
             }
         }
 
+        [Theory]
+        [InlineData(TestImages.Tiff.RgbLzw_NoPredictor_Multistrip, TiffByteOrder.LittleEndian)]
+        [InlineData(TestImages.Tiff.RgbLzw_NoPredictor_Multistrip_Motorola, TiffByteOrder.BigEndian)]
+        public void ByteOrder(string imagePath, TiffByteOrder expectedByteOrder)
+        {
+            var testFile = TestFile.Create(imagePath);
+            using (var stream = new MemoryStream(testFile.Bytes, false))
+            {
+                IImageInfo info = Image.Identify(stream);
+
+                Assert.NotNull(info.Metadata);
+                Assert.Equal(expectedByteOrder, info.Metadata.GetTiffMetadata().ByteOrder);
+
+                // todo: it's not a mistake?
+                stream.Seek(0, SeekOrigin.Begin);
+
+                using var img = Image.Load(stream);
+                Assert.Equal(expectedByteOrder, img.Metadata.GetTiffMetadata().ByteOrder);
+            }
+        }
+
         [Theory]
         [WithFileCollection(nameof(SingleTestImages), PixelTypes.Rgba32)]
         public void Decode<TPixel>(TestImageProvider<TPixel> provider)

tests/ImageSharp.Tests/TestImages.cs

@@ -504,7 +504,7 @@ public static class Tiff
             public const string Calliphora_PaletteUncompressed = "Tiff/Calliphora_palette_uncompressed.tiff";
             public const string Calliphora_RgbDeflate_Predictor = "Tiff/Calliphora_rgb_deflate.tiff";
             public const string Calliphora_RgbJpeg = "Tiff/Calliphora_rgb_jpeg.tiff";
-            public const string Calliphora_RgbLzwe_Predictor = "Tiff/Calliphora_rgb_lzw.tiff";
+            public const string Calliphora_RgbLzw_Predictor = "Tiff/Calliphora_rgb_lzw.tiff";
             public const string Calliphora_RgbPackbits = "Tiff/Calliphora_rgb_packbits.tiff";
             public const string Calliphora_RgbUncompressed = "Tiff/Calliphora_rgb_uncompressed.tiff";
 
@@ -516,6 +516,9 @@ public static class Tiff
             public const string RgbDeflateMultistrip = "Tiff/rgb_deflate_multistrip.tiff";
             public const string RgbJpeg = "Tiff/rgb_jpeg.tiff";
             public const string RgbLzw_Predictor = "Tiff/rgb_lzw.tiff";
+            public const string RgbLzw_NoPredictor_Multistrip = "Tiff/rgb_lzw_noPredictor_multistrip.tiff";
+            public const string RgbLzw_NoPredictor_Multistrip_Motorola = "Tiff/rgb_lzw_noPredictor_multistrip_Motorola.tiff";
+            public const string RgbLzw_NoPredictor_Singlestrip_Motorola = "Tiff/rgb_lzw_noPredictor_singlestrip_Motorola.tiff";
             public const string RgbLzwMultistrip_Predictor = "Tiff/rgb_lzw_multistrip.tiff";
             public const string RgbPackbits = "Tiff/rgb_packbits.tiff";
             public const string RgbPackbitsMultistrip = "Tiff/rgb_packbits_multistrip.tiff";
@@ -534,13 +537,13 @@ public static class Tiff
 
             public const string SampleMetadata = "Tiff/metadata_sample.tiff";
 
-            public static readonly string[] All = { Calliphora_GrayscaleUncompressed, Calliphora_PaletteUncompressed, /*Calliphora_RgbDeflate_Predictor, Calliphora_RgbLzwe_Predictor, */ Calliphora_RgbPackbits, Calliphora_RgbUncompressed, GrayscaleDeflateMultistrip, GrayscaleUncompressed, PaletteDeflateMultistrip, PaletteUncompressed, /*RgbDeflate_Predictor,*/ RgbDeflateMultistrip, /*RgbJpeg,*/ /*RgbLzw_Predictor, RgbLzwMultistrip_Predictor,*/ RgbPackbits, RgbPackbitsMultistrip, RgbUncompressed, /* MultiframeLzw_Predictor, MultiFrameDifferentVariants, SampleMetadata,*/ SmallRgbDeflate, SmallRgbLzw };
+            public static readonly string[] All = { Calliphora_GrayscaleUncompressed, Calliphora_PaletteUncompressed, /*Calliphora_RgbDeflate_Predictor, Calliphora_RgbLzwe_Predictor, */ Calliphora_RgbPackbits, Calliphora_RgbUncompressed, GrayscaleDeflateMultistrip, GrayscaleUncompressed, PaletteDeflateMultistrip, PaletteUncompressed, /*RgbDeflate_Predictor,*/ RgbDeflateMultistrip, /*RgbJpeg,*/ /*RgbLzw_Predictor, RgbLzwMultistrip_Predictor,*/ RgbLzw_NoPredictor_Multistrip, RgbLzw_NoPredictor_Multistrip_Motorola, RgbLzw_NoPredictor_Singlestrip_Motorola, RgbPackbits, RgbPackbitsMultistrip, RgbUncompressed, /* MultiframeLzw_Predictor, MultiFrameDifferentVariants, SampleMetadata,*/ SmallRgbDeflate, SmallRgbLzw, };
 
             public static readonly string[] Multiframes = { MultiframeDeflateWithPreview /*MultiframeLzw_Predictor, MultiFrameDifferentSize, MultiframeDifferentSizeTiled, MultiFrameDifferentVariants,*/ };
 
             public static readonly string[] Metadata = { SampleMetadata };
 
-            public static readonly string[] NotSupported = { Calliphora_RgbJpeg, Calliphora_RgbDeflate_Predictor, Calliphora_RgbLzwe_Predictor, RgbDeflate_Predictor, RgbLzw_Predictor, RgbLzwMultistrip_Predictor, RgbJpeg, RgbUncompressedTiled, MultiframeLzw_Predictor, MultiframeDifferentSize, MultiframeDifferentVariants };
+            public static readonly string[] NotSupported = { Calliphora_RgbJpeg, Calliphora_RgbDeflate_Predictor, Calliphora_RgbLzw_Predictor, RgbDeflate_Predictor, RgbLzw_Predictor, RgbLzwMultistrip_Predictor, RgbJpeg, RgbUncompressedTiled, MultiframeLzw_Predictor, MultiframeDifferentSize, MultiframeDifferentVariants };
         }
     }
 }