Merge pull request #2641 from RobertMut/main

Add JPEG COM marker support

ImageSharp.sln

@@ -238,6 +238,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "issues", "issues", "{5C9B68
 		tests\Images\Input\Jpg\issues\issue750-exif-tranform.jpg = tests\Images\Input\Jpg\issues\issue750-exif-tranform.jpg
 		tests\Images\Input\Jpg\issues\Issue845-Incorrect-Quality99.jpg = tests\Images\Input\Jpg\issues\Issue845-Incorrect-Quality99.jpg
 		tests\Images\Input\Jpg\issues\issue855-incorrect-colorspace.jpg = tests\Images\Input\Jpg\issues\issue855-incorrect-colorspace.jpg
+		tests\Images\Input\Jpg\issues\issue-2067-comment.jpg = tests\Images\Input\Jpg\issues\issue-2067-comment.jpg
 	EndProjectSection
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "fuzz", "fuzz", "{516A3532-6AC2-417B-AD79-9BD5D0D378A0}"

src/ImageSharp/Formats/Jpeg/JpegComData.cs

@@ -0,0 +1,32 @@
+// Copyright (c) Six Labors.
+// Licensed under the Six Labors Split License.
+
+namespace SixLabors.ImageSharp.Formats.Jpeg;
+
+/// <summary>
+/// Represents a JPEG comment
+/// </summary>
+public readonly struct JpegComData
+{
+    /// <summary>
+    /// Initializes a new instance of the <see cref="JpegComData"/> struct.
+    /// </summary>
+    /// <param name="value">The comment buffer.</param>
+    public JpegComData(ReadOnlyMemory<char> value)
+        => this.Value = value;
+
+    /// <summary>
+    /// Gets the value.
+    /// </summary>
+    public ReadOnlyMemory<char> Value { get; }
+
+    /// <summary>
+    /// Converts string to <see cref="JpegComData"/>
+    /// </summary>
+    /// <param name="value">The comment string.</param>
+    /// <returns>The <see cref="JpegComData"/></returns>
+    public static JpegComData FromString(string value) => new(value.AsMemory());
+
+    /// <inheritdoc/>
+    public override string ToString() => this.Value.ToString();
+}

src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs

@@ -480,9 +480,11 @@ internal sealed class JpegDecoderCore : IRawJpegData, IImageDecoderInternals
                         break;
 
                     case JpegConstants.Markers.APP15:
-                    case JpegConstants.Markers.COM:
                         stream.Skip(markerContentByteSize);
                         break;
+                    case JpegConstants.Markers.COM:
+                        this.ProcessComMarker(stream, markerContentByteSize);
+                        break;
 
                     case JpegConstants.Markers.DAC:
                         if (metadataOnly)
@@ -515,6 +517,25 @@ internal sealed class JpegDecoderCore : IRawJpegData, IImageDecoderInternals
         this.scanDecoder = null;
     }
 
+    /// <summary>
+    /// Assigns COM marker bytes to comment property
+    /// </summary>
+    /// <param name="stream">The input stream.</param>
+    /// <param name="markerContentByteSize">The remaining bytes in the segment block.</param>
+    private void ProcessComMarker(BufferedReadStream stream, int markerContentByteSize)
+    {
+        char[] chars = new char[markerContentByteSize];
+        JpegMetadata metadata = this.Metadata.GetFormatMetadata(JpegFormat.Instance);
+
+        for (int i = 0; i < markerContentByteSize; i++)
+        {
+            int read = stream.ReadByte();
+            chars[i] = (char)read;
+        }
+
+        metadata.Comments.Add(new JpegComData(chars));
+    }
+
     /// <summary>
     /// Returns encoded colorspace based on the adobe APP14 marker.
     /// </summary>

src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs

@@ -2,6 +2,7 @@
 // Licensed under the Six Labors Split License.
 #nullable disable
 
+using System.Buffers;
 using System.Buffers.Binary;
 using SixLabors.ImageSharp.Common.Helpers;
 using SixLabors.ImageSharp.Formats.Jpeg.Components;
@@ -25,6 +26,9 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals
     /// </summary>
     private static readonly JpegFrameConfig[] FrameConfigs = CreateFrameConfigs();
 
+    /// <summary>
+    /// The current calling encoder.
+    /// </summary>
     private readonly JpegEncoder encoder;
 
     /// <summary>
@@ -89,6 +93,9 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals
         // Write Exif, XMP, ICC and IPTC profiles
         this.WriteProfiles(metadata, buffer);
 
+        // Write comments
+        this.WriteComments(image.Configuration, jpegMetadata);
+
         // Write the image dimensions.
         this.WriteStartOfFrame(image.Width, image.Height, frameConfig, buffer);
 
@@ -167,6 +174,51 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals
         this.outputStream.Write(buffer, 0, 18);
     }
 
+    /// <summary>
+    /// Writes the COM tags.
+    /// </summary>
+    /// <param name="configuration">The configuration.</param>
+    /// <param name="metadata">The image metadata.</param>
+    private void WriteComments(Configuration configuration, JpegMetadata metadata)
+    {
+        if (metadata.Comments.Count == 0)
+        {
+            return;
+        }
+
+        const int maxCommentLength = 65533;
+        using IMemoryOwner<byte> bufferOwner = configuration.MemoryAllocator.Allocate<byte>(maxCommentLength);
+        Span<byte> buffer = bufferOwner.Memory.Span;
+        foreach (JpegComData comment in metadata.Comments)
+        {
+            int totalLength = comment.Value.Length;
+            if (totalLength == 0)
+            {
+                continue;
+            }
+
+            // Loop through and split the comment into multiple comments if the comment length
+            // is greater than the maximum allowed length.
+            while (totalLength > 0)
+            {
+                int currentLength = Math.Min(totalLength, maxCommentLength);
+
+                // Write the marker header.
+                this.WriteMarkerHeader(JpegConstants.Markers.COM, currentLength + 2, buffer);
+
+                ReadOnlySpan<char> commentValue = comment.Value.Span.Slice(comment.Value.Length - totalLength, currentLength);
+                for (int i = 0; i < commentValue.Length; i++)
+                {
+                    buffer[i] = (byte)commentValue[i];
+                }
+
+                // Write the comment.
+                this.outputStream.Write(buffer, 0, currentLength);
+                totalLength -= currentLength;
+            }
+        }
+    }
+
     /// <summary>
     /// Writes the Define Huffman Table marker and tables.
     /// </summary>

src/ImageSharp/Formats/Jpeg/JpegMetadata.cs

@@ -15,6 +15,7 @@ public class JpegMetadata : IDeepCloneable
     /// </summary>
     public JpegMetadata()
     {
+        this.Comments = new List<JpegComData>();
     }
 
     /// <summary>
@@ -25,6 +26,7 @@ public class JpegMetadata : IDeepCloneable
     {
         this.ColorType = other.ColorType;
 
+        this.Comments = other.Comments;
         this.LuminanceQuality = other.LuminanceQuality;
         this.ChrominanceQuality = other.ChrominanceQuality;
     }
@@ -101,6 +103,11 @@ public class JpegMetadata : IDeepCloneable
     /// </remarks>
     public bool? Progressive { get; internal set; }
 
+    /// <summary>
+    /// Gets the comments.
+    /// </summary>
+    public IList<JpegComData> Comments { get; }
+
     /// <inheritdoc/>
     public IDeepCloneable DeepClone() => new JpegMetadata(this);
 }

src/ImageSharp/Formats/Jpeg/MetadataExtensions.cs

@@ -1,6 +1,7 @@
 // Copyright (c) Six Labors.
 // Licensed under the Six Labors Split License.
 
+using System.Text;
 using SixLabors.ImageSharp.Formats.Jpeg;
 using SixLabors.ImageSharp.Metadata;
 

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

@@ -425,6 +425,21 @@ public partial class JpegDecoderTests
         VerifyEncodedStrings(exif);
     }
 
+    [Theory]
+    [WithFile(TestImages.Jpeg.Issues.Issue2067_CommentMarker, PixelTypes.Rgba32)]
+    public void JpegDecoder_DecodeMetadataComment<TPixel>(TestImageProvider<TPixel> provider)
+        where TPixel : unmanaged, IPixel<TPixel>
+    {
+        string expectedComment = "TEST COMMENT";
+        using Image<TPixel> image = provider.GetImage(JpegDecoder.Instance);
+        JpegMetadata metadata = image.Metadata.GetJpegMetadata();
+
+        Assert.Equal(1, metadata.Comments.Count);
+        Assert.Equal(expectedComment, metadata.Comments.ElementAtOrDefault(0).ToString());
+        image.DebugSave(provider);
+        image.CompareToOriginal(provider);
+    }
+
     private static void VerifyEncodedStrings(ExifProfile exif)
     {
         Assert.NotNull(exif);

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

@@ -32,19 +32,19 @@ public partial class JpegEncoderTests
     public void Encode_PreservesIptcProfile()
     {
         // arrange
-        using var input = new Image<Rgba32>(1, 1);
-        var expectedProfile = new IptcProfile();
+        using Image<Rgba32> input = new(1, 1);
+        IptcProfile expectedProfile = new();
         expectedProfile.SetValue(IptcTag.Country, "ESPAÑA");
         expectedProfile.SetValue(IptcTag.City, "unit-test-city");
         input.Metadata.IptcProfile = expectedProfile;
 
         // act
-        using var memStream = new MemoryStream();
+        using MemoryStream memStream = new();
         input.Save(memStream, JpegEncoder);
 
         // assert
         memStream.Position = 0;
-        using var output = Image.Load<Rgba32>(memStream);
+        using Image<Rgba32> output = Image.Load<Rgba32>(memStream);
         IptcProfile actual = output.Metadata.IptcProfile;
         Assert.NotNull(actual);
         IEnumerable<IptcValue> values = expectedProfile.Values;
@@ -55,17 +55,17 @@ public partial class JpegEncoderTests
     public void Encode_PreservesExifProfile()
     {
         // arrange
-        using var input = new Image<Rgba32>(1, 1);
+        using Image<Rgba32> input = new(1, 1);
         input.Metadata.ExifProfile = new ExifProfile();
         input.Metadata.ExifProfile.SetValue(ExifTag.Software, "unit_test");
 
         // act
-        using var memStream = new MemoryStream();
+        using MemoryStream memStream = new();
         input.Save(memStream, JpegEncoder);
 
         // assert
         memStream.Position = 0;
-        using var output = Image.Load<Rgba32>(memStream);
+        using Image<Rgba32> output = Image.Load<Rgba32>(memStream);
         ExifProfile actual = output.Metadata.ExifProfile;
         Assert.NotNull(actual);
         IReadOnlyList<IExifValue> values = input.Metadata.ExifProfile.Values;
@@ -76,16 +76,16 @@ public partial class JpegEncoderTests
     public void Encode_PreservesIccProfile()
     {
         // arrange
-        using var input = new Image<Rgba32>(1, 1);
+        using Image<Rgba32> input = new(1, 1);
         input.Metadata.IccProfile = new IccProfile(IccTestDataProfiles.Profile_Random_Array);
 
         // act
-        using var memStream = new MemoryStream();
+        using MemoryStream memStream = new();
         input.Save(memStream, JpegEncoder);
 
         // assert
         memStream.Position = 0;
-        using var output = Image.Load<Rgba32>(memStream);
+        using Image<Rgba32> output = Image.Load<Rgba32>(memStream);
         IccProfile actual = output.Metadata.IccProfile;
         Assert.NotNull(actual);
         IccProfile values = input.Metadata.IccProfile;
@@ -99,12 +99,10 @@ public partial class JpegEncoderTests
     {
         Exception ex = Record.Exception(() =>
         {
-            var encoder = new JpegEncoder();
-            using (var stream = new MemoryStream())
-            {
-                using Image<TPixel> image = provider.GetImage(JpegDecoder.Instance);
-                image.Save(stream, encoder);
-            }
+            JpegEncoder encoder = new();
+            using MemoryStream stream = new();
+            using Image<TPixel> image = provider.GetImage(JpegDecoder.Instance);
+            image.Save(stream, encoder);
         });
 
         Assert.Null(ex);
@@ -114,44 +112,99 @@ public partial class JpegEncoderTests
     [MemberData(nameof(RatioFiles))]
     public void Encode_PreserveRatio(string imagePath, int xResolution, int yResolution, PixelResolutionUnit resolutionUnit)
     {
-        var testFile = TestFile.Create(imagePath);
-        using (Image<Rgba32> input = testFile.CreateRgba32Image())
-        {
-            using (var memStream = new MemoryStream())
-            {
-                input.Save(memStream, JpegEncoder);
-
-                memStream.Position = 0;
-                using (var output = Image.Load<Rgba32>(memStream))
-                {
-                    ImageMetadata meta = output.Metadata;
-                    Assert.Equal(xResolution, meta.HorizontalResolution);
-                    Assert.Equal(yResolution, meta.VerticalResolution);
-                    Assert.Equal(resolutionUnit, meta.ResolutionUnits);
-                }
-            }
-        }
+        TestFile testFile = TestFile.Create(imagePath);
+        using Image<Rgba32> input = testFile.CreateRgba32Image();
+        using MemoryStream memStream = new();
+        input.Save(memStream, JpegEncoder);
+
+        memStream.Position = 0;
+        using Image<Rgba32> output = Image.Load<Rgba32>(memStream);
+        ImageMetadata meta = output.Metadata;
+        Assert.Equal(xResolution, meta.HorizontalResolution);
+        Assert.Equal(yResolution, meta.VerticalResolution);
+        Assert.Equal(resolutionUnit, meta.ResolutionUnits);
     }
 
     [Theory]
     [MemberData(nameof(QualityFiles))]
     public void Encode_PreservesQuality(string imagePath, int quality)
     {
-        var testFile = TestFile.Create(imagePath);
-        using (Image<Rgba32> input = testFile.CreateRgba32Image())
-        {
-            using (var memStream = new MemoryStream())
-            {
-                input.Save(memStream, JpegEncoder);
-
-                memStream.Position = 0;
-                using (var output = Image.Load<Rgba32>(memStream))
-                {
-                    JpegMetadata meta = output.Metadata.GetJpegMetadata();
-                    Assert.Equal(quality, meta.Quality);
-                }
-            }
-        }
+        TestFile testFile = TestFile.Create(imagePath);
+        using Image<Rgba32> input = testFile.CreateRgba32Image();
+        using MemoryStream memStream = new();
+        input.Save(memStream, JpegEncoder);
+
+        memStream.Position = 0;
+        using Image<Rgba32> output = Image.Load<Rgba32>(memStream);
+        JpegMetadata meta = output.Metadata.GetJpegMetadata();
+        Assert.Equal(quality, meta.Quality);
+    }
+
+    [Theory]
+    [WithFile(TestImages.Jpeg.Issues.Issue2067_CommentMarker, PixelTypes.Rgba32)]
+    public void Encode_PreservesComments<TPixel>(TestImageProvider<TPixel> provider)
+        where TPixel : unmanaged, IPixel<TPixel>
+    {
+        // arrange
+        using Image<TPixel> input = provider.GetImage(JpegDecoder.Instance);
+        using MemoryStream memStream = new();
+
+        // act
+        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(1, actual.Comments.Count);
+        Assert.Equal("TEST COMMENT", actual.Comments[0].ToString());
+    }
+
+    [Fact]
+    public void Encode_SavesMultipleComments()
+    {
+        // arrange
+        using Image<Rgba32> input = new(1, 1);
+        JpegMetadata meta = input.Metadata.GetJpegMetadata();
+        using MemoryStream memStream = new();
+
+        // act
+        meta.Comments.Add(JpegComData.FromString("First comment"));
+        meta.Comments.Add(JpegComData.FromString("Second Comment"));
+        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[0].ToString(), actual.Comments[0].ToString());
+        Assert.Equal(meta.Comments[1].ToString(), actual.Comments[1].ToString());
+    }
+
+    [Fact]
+    public void Encode_SaveTooLongComment()
+    {
+        // arrange
+        string longString = new('c', 65534);
+        using Image<Rgba32> input = new(1, 1);
+        JpegMetadata meta = input.Metadata.GetJpegMetadata();
+        using MemoryStream memStream = new();
+
+        // act
+        meta.Comments.Add(JpegComData.FromString(longString));
+        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(longString[..65533], actual.Comments[0].ToString());
+        Assert.Equal("c", actual.Comments[1].ToString());
     }
 
     [Theory]
@@ -164,14 +217,14 @@ public partial class JpegEncoderTests
     {
         // arrange
         using Image<TPixel> input = provider.GetImage(JpegDecoder.Instance);
-        using var memoryStream = new MemoryStream();
+        using MemoryStream memoryStream = new();
 
         // act
         input.Save(memoryStream, JpegEncoder);
 
         // assert
         memoryStream.Position = 0;
-        using var output = Image.Load<Rgba32>(memoryStream);
+        using Image<Rgba32> output = Image.Load<Rgba32>(memoryStream);
         JpegMetadata meta = output.Metadata.GetJpegMetadata();
         Assert.Equal(expectedColorType, meta.ColorType);
     }

tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs

@@ -1,6 +1,7 @@
 // Copyright (c) Six Labors.
 // Licensed under the Six Labors Split License.
 
+using System.Collections.ObjectModel;
 using SixLabors.ImageSharp.Formats.Jpeg;
 
 namespace SixLabors.ImageSharp.Tests.Formats.Jpg;
@@ -57,4 +58,25 @@ public class JpegMetadataTests
 
         Assert.Equal(meta.Quality, qualityLuma);
     }
+
+    [Fact]
+    public void Comment_EmptyComment()
+    {
+        var meta = new JpegMetadata();
+
+        Assert.True(Array.Empty<JpegComData>().SequenceEqual(meta.Comments));
+    }
+
+    [Fact]
+    public void Comment_OnlyComment()
+    {
+        string comment = "test comment";
+        var expectedCollection = new Collection<string> { comment };
+
+        var meta = new JpegMetadata();
+        meta.Comments.Add(JpegComData.FromString(comment));
+
+        Assert.Equal(1, meta.Comments.Count);
+        Assert.True(expectedCollection.FirstOrDefault() == meta.Comments.FirstOrDefault().ToString());
+    }
 }

tests/ImageSharp.Tests/TestImages.cs

@@ -315,6 +315,7 @@ public static class TestImages
             public const string Issue2564 = "Jpg/issues/issue-2564.jpg";
             public const string HangBadScan = "Jpg/issues/Hang_C438A851.jpg";
             public const string Issue2517 = "Jpg/issues/issue2517-bad-d7.jpg";
+            public const string Issue2067_CommentMarker = "Jpg/issues/issue-2067-comment.jpg";
 
             public static class Fuzz
             {

tests/Images/Input/Jpg/issues/issue-2067-comment.jpg

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:d87b5429adeffcfac535aa8af2ec9801bf6c965a2e6751cfec4f8534195ba8f4
+size 21082