Add com character limit, comment as IList, remove unnecessary extension methods

Author: RobertMut

src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs

@@ -177,39 +177,46 @@ private void WriteJfifApplicationHeader(ImageMetadata meta, Span<byte> buffer)
     /// <param name="metadata">The image metadata.</param>
     private void WriteComment(JpegMetadata metadata)
     {
-        if (metadata.Comments is { Count: 0 })
+        int maxCommentLength = 65533;
+
+        if (metadata.Comments.Count == 0)
         {
             return;
         }
 
-        // Length (comment strings lengths) + (comments markers with payload sizes)
-        int commentsBytes = metadata.Comments.Sum(x => x.Length) + (metadata.Comments.Count * 4);
-        int commentStart = 0;
-        Span<byte> commentBuffer = new byte[commentsBytes];
-
-        foreach (Memory<char> comment in metadata.Comments)
+        for (int i = 0; i < metadata.Comments.Count; i++)
         {
-            int totalComLength = comment.Length + 4;
+            Memory<char> chars = metadata.Comments[i];
+
+            if (chars.Length > maxCommentLength)
+            {
+                Memory<char> splitComment = chars.Slice(maxCommentLength, chars.Length - maxCommentLength);
+                metadata.Comments.Insert(i + 1, splitComment);
 
-            Span<byte> commentData = commentBuffer.Slice(commentStart, totalComLength);
-            Span<byte> markers = commentData.Slice(0, 2);
-            Span<byte> payloadSize = commentData.Slice(2, 2);
-            Span<byte> payload = commentData.Slice(4, comment.Length);
+                // We don't want to keep the extra bytes
+                chars = chars.Slice(0, maxCommentLength);
+            }
+
+            int commentLength = chars.Length + 4;
+
+            Span<byte> comment = new byte[commentLength];
+            Span<byte> markers = comment.Slice(0, 2);
+            Span<byte> payloadSize = comment.Slice(2, 2);
+            Span<byte> payload = comment.Slice(4, chars.Length);
 
             // Beginning of comment ff fe
             markers[0] = JpegConstants.Markers.XFF;
             markers[1] = JpegConstants.Markers.COM;
 
             // Write payload size
-            BinaryPrimitives.WriteInt16BigEndian(payloadSize, (short)(commentData.Length - 2));
+            int comWithoutMarker = commentLength - 2;
+            payloadSize[0] = (byte)((comWithoutMarker >> 8) & 0xFF);
+            payloadSize[1] = (byte)(comWithoutMarker & 0xFF);
 
-            Encoding.ASCII.GetBytes(comment.Span, payload);
+            Encoding.ASCII.GetBytes(chars.Span, payload);
 
-            // Indicate begin of next comment in buffer
-            commentStart += totalComLength;
+            this.outputStream.Write(comment, 0, comment.Length);
         }
-
-        this.outputStream.Write(commentBuffer, 0, commentBuffer.Length);
     }
 
     /// <summary>

src/ImageSharp/Formats/Jpeg/JpegMetadata.cs

@@ -106,7 +106,7 @@ public int Quality
     /// <summary>
     /// Gets the comments.
     /// </summary>
-    public ICollection<Memory<char>>? Comments { get; }
+    public IList<Memory<char>> Comments { get; }
 
     /// <inheritdoc/>
     public IDeepCloneable DeepClone() => new JpegMetadata(this);

src/ImageSharp/Formats/Jpeg/MetadataExtensions.cs

@@ -18,37 +18,4 @@ public static partial class MetadataExtensions
     /// <param name="metadata">The metadata this method extends.</param>
     /// <returns>The <see cref="JpegMetadata"/>.</returns>
     public static JpegMetadata GetJpegMetadata(this ImageMetadata metadata) => metadata.GetFormatMetadata(JpegFormat.Instance);
-
-    /// <summary>
-    /// Sets the comment in <see cref="JpegMetadata"/>
-    /// </summary>
-    /// <param name="metadata">The metadata this method extends.</param>
-    /// <param name="index">The index of comment to be inserted to.</param>
-    /// <param name="comment">The comment string.</param>
-    public static void SetComment(this JpegMetadata metadata, int index, string comment)
-    {
-        if (metadata.Comments == null)
-        {
-            return;
-        }
-
-        ASCIIEncoding encoding = new();
-        byte[] bytes = encoding.GetBytes(comment);
-        List<Memory<char>>? comments = metadata.Comments as List<Memory<char>>;
-        comments?.Insert(index, encoding.GetChars(bytes));
-    }
-
-    /// <summary>
-    /// Gets the comments from <see cref="JpegMetadata"/>
-    /// </summary>
-    /// <param name="metadata">The metadata this method extends.</param>
-    /// <param name="index">The index of comment.</param>
-    /// <returns>The IEnumerable string of comments.</returns>
-    public static string? GetComment(this JpegMetadata metadata, int index) => metadata.Comments?.ElementAtOrDefault(index).ToString();
-
-    /// <summary>
-    /// Clears comments
-    /// </summary>
-    /// <param name="metadata">The <see cref="JpegMetadata"/>.</param>
-    public static void ClearComments(this JpegMetadata metadata) => metadata.Comments?.Clear();
 }

tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs

@@ -434,8 +434,8 @@ public void JpegDecoder_DecodeMetadataComment<TPixel>(TestImageProvider<TPixel>
         using Image<TPixel> image = provider.GetImage(JpegDecoder.Instance);
         JpegMetadata metadata = image.Metadata.GetJpegMetadata();
 
-        Assert.Equal(1, metadata.Comments?.Count);
-        Assert.Equal(expectedComment, metadata.GetComment(0));
+        Assert.Equal(1, metadata.Comments.Count);
+        Assert.Equal(expectedComment.ToCharArray(), metadata.Comments.ElementAtOrDefault(0));
         image.DebugSave(provider);
         image.CompareToOriginal(provider);
     }

tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs

@@ -172,7 +172,7 @@ public void Encode_PreservesComments<TPixel>(TestImageProvider<TPixel> provider)
         JpegMetadata actual = output.Metadata.GetJpegMetadata();
         Assert.NotEmpty(actual.Comments);
         Assert.Equal(1, actual.Comments.Count);
-        Assert.Equal("TEST COMMENT", actual.Comments.ElementAt(0).ToString());
+        Assert.Equal("TEST COMMENT", actual.Comments.ElementAtOrDefault(0).ToString());
     }
 
     [Fact]
@@ -184,18 +184,18 @@ public void Encode_SavesMultipleComments()
         using var memStream = new MemoryStream();
 
         // act
-        meta.SetComment(0, "First comment");
-        meta.SetComment(1, "Second Comment");
+        meta.Comments.Add("First comment".ToCharArray());
+        meta.Comments.Add("Second Comment".ToCharArray());
         input.Save(memStream, JpegEncoder);
 
         // assert
         memStream.Position = 0;
         using Image<Rgba32> output = Image.Load<Rgba32>(memStream);
         JpegMetadata actual = output.Metadata.GetJpegMetadata();
         Assert.NotEmpty(actual.Comments);
-        Assert.Equal(2, actual.Comments?.Count);
-        Assert.Equal(meta.Comments?.ElementAt(0).ToString(), actual.Comments?.ElementAt(0).ToString());
-        Assert.Equal(meta.Comments?.ElementAt(1).ToString(), actual.Comments?.ElementAt(1).ToString());
+        Assert.Equal(2, actual.Comments.Count);
+        Assert.Equal(meta.Comments.ElementAtOrDefault(0).ToString(), actual.Comments.ElementAtOrDefault(0).ToString());
+        Assert.Equal(meta.Comments.ElementAtOrDefault(1).ToString(), actual.Comments.ElementAtOrDefault(1).ToString());
     }
 
     [Theory]