Add support for read and write tEXt, iTXt and zTXt chunks (#951)

* Add support for writing tEXt chunks

* Add support for reading zTXt chunks

* Add check, if keyword is valid

* Add support for reading iTXt chunks

* Add support for writing iTXt chunks

* Remove Test Decode_TextEncodingSetToUnicode_TextIsReadWithCorrectEncoding: Assertion is wrong, the correct keyword name is "Software"

* Add support for writing zTXt chunk

* Add an encoder Option to enable compression when the string is larger than a given threshold

* Moved uncompressing text into separate method

* Remove textEncoding option from png decoder options: the encoding is determined by the specification: https://www.w3.org/TR/PNG/#11zTXt

* Removed invalid compressed zTXt chunk from test image

* Revert accidentally committed changes to Sandbox Program.cs

* Review adjustments

* Using 1024 bytes as a limit when to compress text as recommended by the spec

* Fix inconsistent line endings

* Trim leading and trailing whitespace on png keywords

* Move some metadata related tests into GifMetaDataTests.cs

* Add test case for gif with large text

* Gif text metadata is now a list of strings

* Encoder writes each comment as a separate block

* Adjustment of the Tests to the recent changes

* Move comments to GifMetadata

* Move Png TextData to format PngMetaData

Author: brianpopow

src/ImageSharp/Formats/Bmp/BmpCompression.cs

@@ -69,7 +69,7 @@ internal enum BmpCompression : int
         /// rather than four or eight bits in size.
         ///
         /// Note: Because compression value of 4 is ambiguous for BI_RGB for windows and RLE24 for OS/2, the enum value is remapped
-        /// to a different value.
+        /// to a different value, to be clearly separate from valid windows values.
         /// </summary>
         RLE24 = 100,
     }

src/ImageSharp/Formats/Gif/GifConstants.cs

@@ -1,4 +1,4 @@
-// Copyright (c) Six Labors and contributors.
+// Copyright (c) Six Labors and contributors.
 // Licensed under the Apache License, Version 2.0.
 
 using System.Collections.Generic;
@@ -7,7 +7,7 @@
 namespace SixLabors.ImageSharp.Formats.Gif
 {
     /// <summary>
-    /// Constants that define specific points within a gif.
+    /// Constants that define specific points within a Gif.
     /// </summary>
     internal static class GifConstants
     {
@@ -67,14 +67,9 @@ internal static class GifConstants
         public const byte CommentLabel = 0xFE;
 
         /// <summary>
-        /// The name of the property inside the image properties for the comments.
+        /// The maximum length of a comment data sub-block is 255.
         /// </summary>
-        public const string Comments = "Comments";
-
-        /// <summary>
-        /// The maximum comment length.
-        /// </summary>
-        public const int MaxCommentLength = 1024 * 8;
+        public const int MaxCommentSubBlockLength = 255;
 
         /// <summary>
         /// The image descriptor label <value>,</value>.
@@ -102,18 +97,18 @@ internal static class GifConstants
         public const byte EndIntroducer = 0x3B;
 
         /// <summary>
-        /// Gets the default encoding to use when reading comments.
+        /// The character encoding to use when reading and writing comments - (ASCII 7bit).
         /// </summary>
-        public static readonly Encoding DefaultEncoding = Encoding.ASCII;
+        public static readonly Encoding Encoding = Encoding.ASCII;
 
         /// <summary>
-        /// The list of mimetypes that equate to a gif.
+        /// The collection of mimetypes that equate to a Gif.
         /// </summary>
         public static readonly IEnumerable<string> MimeTypes = new[] { "image/gif" };
 
         /// <summary>
-        /// The list of file extensions that equate to a gif.
+        /// The collection of file extensions that equate to a Gif.
         /// </summary>
         public static readonly IEnumerable<string> FileExtensions = new[] { "gif" };
     }
-}
+}

src/ImageSharp/Formats/Gif/GifDecoder.cs

@@ -1,8 +1,7 @@
-// Copyright (c) Six Labors and contributors.
+// Copyright (c) Six Labors and contributors.
 // Licensed under the Apache License, Version 2.0.
 
 using System.IO;
-using System.Text;
 using SixLabors.ImageSharp.Metadata;
 using SixLabors.ImageSharp.PixelFormats;
 
@@ -18,11 +17,6 @@ public sealed class GifDecoder : IImageDecoder, IGifDecoderOptions, IImageInfoDe
         /// </summary>
         public bool IgnoreMetadata { get; set; } = false;
 
-        /// <summary>
-        /// Gets or sets the encoding that should be used when reading comments.
-        /// </summary>
-        public Encoding TextEncoding { get; set; } = GifConstants.DefaultEncoding;
-
         /// <summary>
         /// Gets or sets the decoding mode for multi-frame images
         /// </summary>

src/ImageSharp/Formats/Gif/GifDecoderCore.cs

@@ -1,4 +1,4 @@
-// Copyright (c) Six Labors and contributors.
+// Copyright (c) Six Labors and contributors.
 // Licensed under the Apache License, Version 2.0.
 
 using System;
@@ -77,7 +77,6 @@ internal sealed class GifDecoderCore
         /// <param name="options">The decoder options.</param>
         public GifDecoderCore(Configuration configuration, IGifDecoderOptions options)
         {
-            this.TextEncoding = options.TextEncoding ?? GifConstants.DefaultEncoding;
             this.IgnoreMetadata = options.IgnoreMetadata;
             this.DecodingMode = options.DecodingMode;
             this.configuration = configuration ?? Configuration.Default;
@@ -88,11 +87,6 @@ public GifDecoderCore(Configuration configuration, IGifDecoderOptions options)
         /// </summary>
         public bool IgnoreMetadata { get; internal set; }
 
-        /// <summary>
-        /// Gets the text encoding
-        /// </summary>
-        public Encoding TextEncoding { get; }
-
         /// <summary>
         /// Gets the decoding mode for multi-frame images
         /// </summary>
@@ -317,11 +311,12 @@ private void ReadComments()
         {
             int length;
 
+            var stringBuilder = new StringBuilder();
             while ((length = this.stream.ReadByte()) != 0)
             {
-                if (length > GifConstants.MaxCommentLength)
+                if (length > GifConstants.MaxCommentSubBlockLength)
                 {
-                    throw new ImageFormatException($"Gif comment length '{length}' exceeds max '{GifConstants.MaxCommentLength}'");
+                    throw new ImageFormatException($"Gif comment length '{length}' exceeds max '{GifConstants.MaxCommentSubBlockLength}' of a comment data block");
                 }
 
                 if (this.IgnoreMetadata)
@@ -333,10 +328,15 @@ private void ReadComments()
                 using (IManagedByteBuffer commentsBuffer = this.MemoryAllocator.AllocateManagedByteBuffer(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));
+                    string commentPart = GifConstants.Encoding.GetString(commentsBuffer.Array, 0, length);
+                    stringBuilder.Append(commentPart);
                 }
             }
+
+            if (stringBuilder.Length > 0)
+            {
+                this.gifMetadata.Comments.Add(stringBuilder.ToString());
+            }
         }
 
         /// <summary>
@@ -632,4 +632,4 @@ private void ReadLogicalScreenDescriptorAndGlobalColorTable(Stream stream)
             }
         }
     }
-}
+}

src/ImageSharp/Formats/Gif/GifEncoder.cs

@@ -1,8 +1,7 @@
-// Copyright (c) Six Labors and contributors.
+// Copyright (c) Six Labors and contributors.
 // Licensed under the Apache License, Version 2.0.
 
 using System.IO;
-using System.Text;
 using SixLabors.ImageSharp.Advanced;
 using SixLabors.ImageSharp.PixelFormats;
 using SixLabors.ImageSharp.Processing.Processors.Quantization;
@@ -14,11 +13,6 @@ namespace SixLabors.ImageSharp.Formats.Gif
     /// </summary>
     public sealed class GifEncoder : IImageEncoder, IGifEncoderOptions
     {
-        /// <summary>
-        /// Gets or sets the encoding that should be used when writing comments.
-        /// </summary>
-        public Encoding TextEncoding { get; set; } = GifConstants.DefaultEncoding;
-
         /// <summary>
         /// Gets or sets the quantizer for reducing the color count.
         /// Defaults to the <see cref="OctreeQuantizer"/>
@@ -38,4 +32,4 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream)
             encoder.Encode(image, stream);
         }
     }
-}
+}

src/ImageSharp/Formats/Gif/GifEncoderCore.cs

@@ -1,4 +1,4 @@
-// Copyright (c) Six Labors and contributors.
+// Copyright (c) Six Labors and contributors.
 // Licensed under the Apache License, Version 2.0.
 
 using System;
@@ -37,11 +37,6 @@ internal sealed class GifEncoderCore
         /// </summary>
         private readonly byte[] buffer = new byte[20];
 
-        /// <summary>
-        /// The text encoding used to write comments.
-        /// </summary>
-        private readonly Encoding textEncoding;
-
         /// <summary>
         /// The quantizer used to generate the color palette.
         /// </summary>
@@ -57,11 +52,6 @@ internal sealed class GifEncoderCore
         /// </summary>
         private int bitDepth;
 
-        /// <summary>
-        /// Gif specific metadata.
-        /// </summary>
-        private GifMetadata gifMetadata;
-
         /// <summary>
         /// Initializes a new instance of the <see cref="GifEncoderCore"/> class.
         /// </summary>
@@ -70,7 +60,6 @@ internal sealed class GifEncoderCore
         public GifEncoderCore(MemoryAllocator memoryAllocator, IGifEncoderOptions options)
         {
             this.memoryAllocator = memoryAllocator;
-            this.textEncoding = options.TextEncoding ?? GifConstants.DefaultEncoding;
             this.quantizer = options.Quantizer;
             this.colorTableMode = options.ColorTableMode;
         }
@@ -90,8 +79,8 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream)
             this.configuration = image.GetConfiguration();
 
             ImageMetadata metadata = image.Metadata;
-            this.gifMetadata = metadata.GetFormatMetadata(GifFormat.Instance);
-            this.colorTableMode = this.colorTableMode ?? this.gifMetadata.ColorTableMode;
+            GifMetadata gifMetadata = metadata.GetFormatMetadata(GifFormat.Instance);
+            this.colorTableMode = this.colorTableMode ?? gifMetadata.ColorTableMode;
             bool useGlobalTable = this.colorTableMode == GifColorTableMode.Global;
 
             // Quantize the image returning a palette.
@@ -117,12 +106,12 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream)
             }
 
             // Write the comments.
-            this.WriteComments(metadata, stream);
+            this.WriteComments(gifMetadata, stream);
 
             // Write application extension to allow additional frames.
             if (image.Frames.Count > 1)
             {
-                this.WriteApplicationExtension(stream, this.gifMetadata.RepeatCount);
+                this.WriteApplicationExtension(stream, gifMetadata.RepeatCount);
             }
 
             if (useGlobalTable)
@@ -333,25 +322,51 @@ private void WriteApplicationExtension(Stream stream, ushort repeatCount)
         /// </summary>
         /// <param name="metadata">The metadata to be extract the comment data.</param>
         /// <param name="stream">The stream to write to.</param>
-        private void WriteComments(ImageMetadata metadata, Stream stream)
+        private void WriteComments(GifMetadata metadata, Stream stream)
         {
-            if (!metadata.TryGetProperty(GifConstants.Comments, out ImageProperty property)
-                || string.IsNullOrEmpty(property.Value))
+            if (metadata.Comments.Count == 0)
             {
                 return;
             }
 
-            byte[] comments = this.textEncoding.GetBytes(property.Value);
+            foreach (string comment in metadata.Comments)
+            {
+                this.buffer[0] = GifConstants.ExtensionIntroducer;
+                this.buffer[1] = GifConstants.CommentLabel;
+                stream.Write(this.buffer, 0, 2);
+
+                // Comment will be stored in chunks of 255 bytes, if it exceeds this size.
+                ReadOnlySpan<char> commentSpan = comment.AsSpan();
+                int idx = 0;
+                for (; idx <= comment.Length - GifConstants.MaxCommentSubBlockLength; idx += GifConstants.MaxCommentSubBlockLength)
+                {
+                    WriteCommentSubBlock(stream, commentSpan, idx, GifConstants.MaxCommentSubBlockLength);
+                }
 
-            int count = Math.Min(comments.Length, 255);
+                // Write the length bytes, if any, to another sub block.
+                if (idx < comment.Length)
+                {
+                    int remaining = comment.Length - idx;
+                    WriteCommentSubBlock(stream, commentSpan, idx, remaining);
+                }
 
-            this.buffer[0] = GifConstants.ExtensionIntroducer;
-            this.buffer[1] = GifConstants.CommentLabel;
-            this.buffer[2] = (byte)count;
+                stream.WriteByte(GifConstants.Terminator);
+            }
+        }
 
-            stream.Write(this.buffer, 0, 3);
-            stream.Write(comments, 0, count);
-            stream.WriteByte(GifConstants.Terminator);
+        /// <summary>
+        /// Writes a comment sub-block to the stream.
+        /// </summary>
+        /// <param name="stream">The stream to write to.</param>
+        /// <param name="commentSpan">Comment as a Span.</param>
+        /// <param name="idx">Current start index.</param>
+        /// <param name="length">The length of the string to write. Should not exceed 255 bytes.</param>
+        private static void WriteCommentSubBlock(Stream stream, ReadOnlySpan<char> commentSpan, int idx, int length)
+        {
+            string subComment = commentSpan.Slice(idx, length).ToString();
+            byte[] subCommentBytes = GifConstants.Encoding.GetBytes(subComment);
+            stream.WriteByte((byte)length);
+            stream.Write(subCommentBytes, 0, length);
         }
 
         /// <summary>
@@ -458,4 +473,4 @@ private void WriteImageData<TPixel>(IQuantizedFrame<TPixel> image, Stream stream
             }
         }
     }
-}
+}

src/ImageSharp/Formats/Gif/GifFrameMetaData.cs

@@ -1,4 +1,4 @@
-// Copyright (c) Six Labors and contributors.
+// Copyright (c) Six Labors and contributors.
 // Licensed under the Apache License, Version 2.0.
 
 namespace SixLabors.ImageSharp.Formats.Gif
@@ -51,4 +51,4 @@ private GifFrameMetadata(GifFrameMetadata other)
         /// <inheritdoc/>
         public IDeepCloneable DeepClone() => new GifFrameMetadata(this);
     }
-}
+}

src/ImageSharp/Formats/Gif/GifMetaData.cs

@@ -1,6 +1,8 @@
-// Copyright (c) Six Labors and contributors.
+// Copyright (c) Six Labors and contributors.
 // Licensed under the Apache License, Version 2.0.
 
+using System.Collections.Generic;
+
 namespace SixLabors.ImageSharp.Formats.Gif
 {
     /// <summary>
@@ -24,6 +26,11 @@ private GifMetadata(GifMetadata other)
             this.RepeatCount = other.RepeatCount;
             this.ColorTableMode = other.ColorTableMode;
             this.GlobalColorTableLength = other.GlobalColorTableLength;
+
+            for (int i = 0; i < other.Comments.Count; i++)
+            {
+                this.Comments.Add(other.Comments[i]);
+            }
         }
 
         /// <summary>
@@ -44,7 +51,13 @@ private GifMetadata(GifMetadata other)
         /// </summary>
         public int GlobalColorTableLength { get; set; }
 
+        /// <summary>
+        /// Gets or sets the the collection of comments about the graphics, credits, descriptions or any
+        /// other type of non-control and non-graphic data.
+        /// </summary>
+        public IList<string> Comments { get; set; } = new List<string>();
+
         /// <inheritdoc/>
         public IDeepCloneable DeepClone() => new GifMetadata(this);
     }
-}
+}

src/ImageSharp/Formats/Gif/IGifDecoderOptions.cs

@@ -1,7 +1,6 @@
-// Copyright (c) Six Labors and contributors.
+// Copyright (c) Six Labors and contributors.
 // Licensed under the Apache License, Version 2.0.
 
-using System.Text;
 using SixLabors.ImageSharp.Metadata;
 
 namespace SixLabors.ImageSharp.Formats.Gif
@@ -16,11 +15,6 @@ internal interface IGifDecoderOptions
         /// </summary>
         bool IgnoreMetadata { get; }
 
-        /// <summary>
-        /// Gets the text encoding that should be used when reading comments.
-        /// </summary>
-        Encoding TextEncoding { get; }
-
         /// <summary>
         /// Gets the decoding mode for multi-frame images.
         /// </summary>