Merge pull request #536 from carbon/gif-cleanup

Improve Gif CQ, Part 2

Author: JimBobSquarePants

src/ImageSharp/Formats/Gif/DisposalMethod.cs

@@ -11,23 +11,26 @@ namespace SixLabors.ImageSharp.Formats.Gif
     public enum DisposalMethod
     {
         /// <summary>
-        /// No disposal specified. The decoder is not required to take any action.
+        /// No disposal specified.
+        /// The decoder is not required to take any action.
         /// </summary>
         Unspecified = 0,
 
         /// <summary>
-        /// Do not dispose. The graphic is to be left in place.
+        /// Do not dispose.
+        /// The graphic is to be left in place.
         /// </summary>
         NotDispose = 1,
 
         /// <summary>
-        /// Restore to background color. The area used by the graphic must be restored to
-        /// the background color.
+        /// Restore to background color.
+        /// The area used by the graphic must be restored to the background color.
         /// </summary>
         RestoreToBackground = 2,
 
         /// <summary>
-        /// Restore to previous. The decoder is required to restore the area overwritten by the
+        /// Restore to previous.
+        /// The decoder is required to restore the area overwritten by the
         /// graphic with what was there prior to rendering the graphic.
         /// </summary>
         RestoreToPrevious = 3

src/ImageSharp/Formats/Gif/GifConstants.cs

@@ -54,7 +54,7 @@ internal static class GifConstants
         /// <summary>
         /// The application block size.
         /// </summary>
-        public const byte ApplicationBlockSize = 0x0b;
+        public const byte ApplicationBlockSize = 11;
 
         /// <summary>
         /// The comment label.

src/ImageSharp/Formats/Gif/GifDecoderCore.cs

@@ -32,7 +32,7 @@ internal sealed class GifDecoderCore
         /// <summary>
         /// The currently loaded stream.
         /// </summary>
-        private Stream currentStream;
+        private Stream stream;
 
         /// <summary>
         /// The global color table.
@@ -57,7 +57,7 @@ internal sealed class GifDecoderCore
         /// <summary>
         /// The graphics control extension.
         /// </summary>
-        private GifGraphicsControlExtension graphicsControlExtension;
+        private GifGraphicControlExtension graphicsControlExtension;
 
         /// <summary>
         /// The metadata
@@ -236,9 +236,9 @@ public IImageInfo Identify(Stream stream)
         /// </summary>
         private void ReadGraphicalControlExtension()
         {
-            this.currentStream.Read(this.buffer, 0, 6);
+            this.stream.Read(this.buffer, 0, 6);
 
-            this.graphicsControlExtension = GifGraphicsControlExtension.Parse(this.buffer);
+            this.graphicsControlExtension = GifGraphicControlExtension.Parse(this.buffer);
         }
 
         /// <summary>
@@ -247,7 +247,7 @@ private void ReadGraphicalControlExtension()
         /// <returns><see cref="GifImageDescriptor"/></returns>
         private GifImageDescriptor ReadImageDescriptor()
         {
-            this.currentStream.Read(this.buffer, 0, 9);
+            this.stream.Read(this.buffer, 0, 9);
 
             return GifImageDescriptor.Parse(this.buffer);
         }
@@ -257,7 +257,7 @@ private GifImageDescriptor ReadImageDescriptor()
         /// </summary>
         private void ReadLogicalScreenDescriptor()
         {
-            this.currentStream.Read(this.buffer, 0, 7);
+            this.stream.Read(this.buffer, 0, 7);
 
             this.logicalScreenDescriptor = GifLogicalScreenDescriptor.Parse(this.buffer);
         }
@@ -268,13 +268,13 @@ private void ReadLogicalScreenDescriptor()
         /// <param name="length">The number of bytes to skip.</param>
         private void Skip(int length)
         {
-            this.currentStream.Skip(length);
+            this.stream.Skip(length);
 
             int flag;
 
-            while ((flag = this.currentStream.ReadByte()) != 0)
+            while ((flag = this.stream.ReadByte()) != 0)
             {
-                this.currentStream.Skip(flag);
+                this.stream.Skip(flag);
             }
         }
 
@@ -285,7 +285,7 @@ private void ReadComments()
         {
             int length;
 
-            while ((length = this.currentStream.ReadByte()) != 0)
+            while ((length = this.stream.ReadByte()) != 0)
             {
                 if (length > GifConstants.MaxCommentLength)
                 {
@@ -294,13 +294,13 @@ private void ReadComments()
 
                 if (this.IgnoreMetadata)
                 {
-                    this.currentStream.Seek(length, SeekOrigin.Current);
+                    this.stream.Seek(length, SeekOrigin.Current);
                     continue;
                 }
 
                 using (IManagedByteBuffer commentsBuffer = this.MemoryManager.AllocateManagedByteBuffer(length))
                 {
-                    this.currentStream.Read(commentsBuffer.Array, 0, length);
+                    this.stream.Read(commentsBuffer.Array, 0, length);
                     string comments = this.TextEncoding.GetString(commentsBuffer.Array, 0, length);
                     this.metaData.Properties.Add(new ImageProperty(GifConstants.Comments, comments));
                 }
@@ -327,7 +327,7 @@ private void ReadFrame<TPixel>(ref Image<TPixel> image, ref ImageFrame<TPixel> p
                 {
                     int length = imageDescriptor.LocalColorTableSize * 3;
                     localColorTable = this.configuration.MemoryManager.AllocateManagedByteBuffer(length, true);
-                    this.currentStream.Read(localColorTable.Array, 0, length);
+                    this.stream.Read(localColorTable.Array, 0, length);
                 }
 
                 indices = this.configuration.MemoryManager.AllocateManagedByteBuffer(imageDescriptor.Width * imageDescriptor.Height, true);
@@ -354,8 +354,8 @@ private void ReadFrame<TPixel>(ref Image<TPixel> image, ref ImageFrame<TPixel> p
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private void ReadFrameIndices(in GifImageDescriptor imageDescriptor, Span<byte> indices)
         {
-            int dataSize = this.currentStream.ReadByte();
-            using (var lzwDecoder = new LzwDecoder(this.configuration.MemoryManager, this.currentStream))
+            int dataSize = this.stream.ReadByte();
+            using (var lzwDecoder = new LzwDecoder(this.configuration.MemoryManager, this.stream))
             {
                 lzwDecoder.DecodePixels(imageDescriptor.Width, imageDescriptor.Height, dataSize, indices);
             }
@@ -526,10 +526,10 @@ private void ReadLogicalScreenDescriptorAndGlobalColorTable(Stream stream)
         {
             this.metaData = new ImageMetaData();
 
-            this.currentStream = stream;
+            this.stream = stream;
 
             // Skip the identifier
-            this.currentStream.Skip(6);
+            this.stream.Skip(6);
             this.ReadLogicalScreenDescriptor();
 
             if (this.logicalScreenDescriptor.GlobalColorTableFlag)

src/ImageSharp/Formats/Gif/GifEncoderCore.cs

@@ -4,50 +4,45 @@
 using System;
 using System.Buffers.Binary;
 using System.IO;
-using System.Linq;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using System.Text;
-using SixLabors.ImageSharp.IO;
 using SixLabors.ImageSharp.Memory;
 using SixLabors.ImageSharp.MetaData;
 using SixLabors.ImageSharp.PixelFormats;
 using SixLabors.ImageSharp.Processing.Quantization;
 
-// TODO: This is causing more GC collections than I'm happy with.
-// This is likely due to the number of short writes to the stream we are doing.
-// We should investigate reducing them since we know the length of the byte array we require for multiple parts.
 namespace SixLabors.ImageSharp.Formats.Gif
 {
     /// <summary>
-    /// Performs the gif encoding operation.
+    /// Implements the GIF encoding protocol.
     /// </summary>
     internal sealed class GifEncoderCore
     {
         private readonly MemoryManager memoryManager;
 
         /// <summary>
-        /// The temp buffer used to reduce allocations.
+        /// A reusable buffer used to reduce allocations.
         /// </summary>
-        private readonly byte[] buffer = new byte[16];
+        private readonly byte[] buffer = new byte[20];
 
         /// <summary>
-        /// Gets the TextEncoding
+        /// Gets the text encoding used to write comments.
         /// </summary>
         private readonly Encoding textEncoding;
 
         /// <summary>
-        /// Gets or sets the quantizer for reducing the color count.
+        /// Gets or sets the quantizer used to generate the color palette.
         /// </summary>
         private readonly IQuantizer quantizer;
 
         /// <summary>
-        /// Gets or sets a value indicating whether the metadata should be ignored when the image is being decoded.
+        /// A flag indicating whether to ingore the metadata when writing the image.
         /// </summary>
         private readonly bool ignoreMetadata;
 
         /// <summary>
-        /// The number of bits requires to store the image palette.
+        /// The number of bits requires to store the color palette.
         /// </summary>
         private int bitDepth;
 
@@ -91,7 +86,7 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream)
             this.WriteLogicalScreenDescriptor(image, stream, index);
 
             // Write the first frame.
-            this.WriteComments(image, stream);
+            this.WriteComments(image.MetaData, stream);
 
             // Write additional frames.
             if (image.Frames.Count > 1)
@@ -133,7 +128,8 @@ private int GetTransparentIndex<TPixel>(QuantizedFrame<TPixel> quantized)
         {
             // Transparent pixels are much more likely to be found at the end of a palette
             int index = -1;
-            var trans = default(Rgba32);
+            Rgba32 trans = default;
+
             ref TPixel paletteRef = ref MemoryMarshal.GetReference(quantized.Palette.AsSpan());
             for (int i = quantized.Palette.Length - 1; i >= 0; i--)
             {
@@ -168,13 +164,12 @@ private void WriteHeader(Stream stream)
         private void WriteLogicalScreenDescriptor<TPixel>(Image<TPixel> image, Stream stream, int transparencyIndex)
             where TPixel : struct, IPixel<TPixel>
         {
+            byte packedValue = GifLogicalScreenDescriptor.GetPackedValue(false, this.bitDepth - 1, false, this.bitDepth - 1);
+
             var descriptor = new GifLogicalScreenDescriptor(
                 width: (ushort)image.Width,
                 height: (ushort)image.Height,
-                bitsPerPixel: 0,
-                pixelAspectRatio: 0,
-                globalColorTableFlag: false, // TODO: Always false for now.
-                globalColorTableSize: this.bitDepth - 1,
+                packed: packedValue,
                 backgroundColorIndex: unchecked((byte)transparencyIndex));
 
             descriptor.WriteTo(this.buffer);
@@ -196,40 +191,37 @@ private void WriteApplicationExtension(Stream stream, ushort repeatCount)
                 this.buffer[1] = GifConstants.ApplicationExtensionLabel;
                 this.buffer[2] = GifConstants.ApplicationBlockSize;
 
-                stream.Write(this.buffer, 0, 3);
-
-                stream.Write(GifConstants.ApplicationIdentificationBytes, 0, 11); // NETSCAPE2.0
+                // Write NETSCAPE2.0
+                GifConstants.ApplicationIdentificationBytes.AsSpan().CopyTo(this.buffer.AsSpan(3, 11));
 
-                this.buffer[0] = 3; // Application block length
-                this.buffer[1] = 1; // Data sub-block index (always 1)
+                // Application Data ----
+                this.buffer[14] = 3; // Application block length
+                this.buffer[15] = 1; // Data sub-block index (always 1)
 
                 // 0 means loop indefinitely. Count is set as play n + 1 times.
                 repeatCount = (ushort)Math.Max(0, repeatCount - 1);
 
-                BinaryPrimitives.WriteUInt16LittleEndian(this.buffer.AsSpan(2, 2), repeatCount); // Repeat count for images.
+                BinaryPrimitives.WriteUInt16LittleEndian(this.buffer.AsSpan(16, 2), repeatCount); // Repeat count for images.
 
-                this.buffer[4] = GifConstants.Terminator; // Terminator
+                this.buffer[18] = GifConstants.Terminator; // Terminator
 
-                stream.Write(this.buffer, 0, 5);
+                stream.Write(this.buffer, 0, 19);
             }
         }
 
         /// <summary>
         /// Writes the image comments to the stream.
         /// </summary>
-        /// <typeparam name="TPixel">The pixel format.</typeparam>
-        /// <param name="image">The <see cref="ImageFrame{TPixel}"/> to be encoded.</param>
+        /// <param name="metadata">The metadata to be extract the comment data.</param>
         /// <param name="stream">The stream to write to.</param>
-        private void WriteComments<TPixel>(Image<TPixel> image, Stream stream)
-            where TPixel : struct, IPixel<TPixel>
+        private void WriteComments(ImageMetaData metadata, Stream stream)
         {
             if (this.ignoreMetadata)
             {
                 return;
             }
 
-            ImageProperty property = image.MetaData.Properties.FirstOrDefault(p => p.Name == GifConstants.Comments);
-            if (property == null || string.IsNullOrEmpty(property.Value))
+            if (!metadata.TryGetProperty(GifConstants.Comments, out ImageProperty property) || string.IsNullOrEmpty(property.Value))
             {
                 return;
             }
@@ -255,15 +247,33 @@ private void WriteComments<TPixel>(Image<TPixel> image, Stream stream)
         /// <param name="transparencyIndex">The index of the color in the color palette to make transparent.</param>
         private void WriteGraphicalControlExtension(ImageFrameMetaData metaData, Stream stream, int transparencyIndex)
         {
-            var extension = new GifGraphicsControlExtension(
+            byte packedValue = GifGraphicControlExtension.GetPackedValue(
                 disposalMethod: metaData.DisposalMethod,
-                transparencyFlag: transparencyIndex > -1,
+                transparencyFlag: transparencyIndex > -1);
+
+            var extension = new GifGraphicControlExtension(
+                packed: packedValue,
                 transparencyIndex: unchecked((byte)transparencyIndex),
                 delayTime: (ushort)metaData.FrameDelay);
 
-            extension.WriteTo(this.buffer);
+            this.WriteExtension(extension, stream);
+        }
+
+        /// <summary>
+        /// Writes the provided extension to the stream.
+        /// </summary>
+        /// <param name="extension">The extension to write to the stream.</param>
+        /// <param name="stream">The stream to write to.</param>
+        public void WriteExtension(IGifExtension extension, Stream stream)
+        {
+            this.buffer[0] = GifConstants.ExtensionIntroducer;
+            this.buffer[1] = extension.Label;
 
-            stream.Write(this.buffer, 0, GifGraphicsControlExtension.Size);
+            int extensionSize = extension.WriteTo(this.buffer.AsSpan(2));
+
+            this.buffer[extensionSize + 2] = GifConstants.Terminator;
+
+            stream.Write(this.buffer, 0, extensionSize + 3);
         }
 
         /// <summary>
@@ -279,7 +289,7 @@ private void WriteImageDescriptor<TPixel>(ImageFrame<TPixel> image, Stream strea
                 localColorTableFlag: true,
                 interfaceFlag: false,
                 sortFlag: false,
-                localColorTableSize: this.bitDepth); // Note: we subtract 1 from the colorTableSize writing
+                localColorTableSize: (byte)this.bitDepth); // Note: we subtract 1 from the colorTableSize writing
 
             var descriptor = new GifImageDescriptor(
                 left: 0,
@@ -302,24 +312,23 @@ private void WriteImageDescriptor<TPixel>(ImageFrame<TPixel> image, Stream strea
         private void WriteColorTable<TPixel>(QuantizedFrame<TPixel> image, Stream stream)
             where TPixel : struct, IPixel<TPixel>
         {
-            // Grab the palette and write it to the stream.
             int pixelCount = image.Palette.Length;
 
-            // Get max colors for bit depth.
-            int colorTableLength = (int)Math.Pow(2, this.bitDepth) * 3;
-            var rgb = default(Rgb24);
+            int colorTableLength = (int)Math.Pow(2, this.bitDepth) * 3; // The maximium number of colors for the bit depth
+            Rgb24 rgb = default;
+
             using (IManagedByteBuffer colorTable = this.memoryManager.AllocateManagedByteBuffer(colorTableLength))
             {
                 ref TPixel paletteRef = ref MemoryMarshal.GetReference(image.Palette.AsSpan());
                 ref Rgb24 rgb24Ref = ref Unsafe.As<byte, Rgb24>(ref MemoryMarshal.GetReference(colorTable.Span));
-
                 for (int i = 0; i < pixelCount; i++)
                 {
                     ref TPixel entry = ref Unsafe.Add(ref paletteRef, i);
                     entry.ToRgb24(ref rgb);
                     Unsafe.Add(ref rgb24Ref, i) = rgb;
                 }
 
+                // Write the palette to the stream
                 stream.Write(colorTable.Array, 0, colorTableLength);
             }
         }