diff --git a/ImageSharp.sln b/ImageSharp.sln index b8204c47d1..162de84168 100644 --- a/ImageSharp.sln +++ b/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}" diff --git a/src/ImageSharp/Formats/Jpeg/JpegComData.cs b/src/ImageSharp/Formats/Jpeg/JpegComData.cs new file mode 100644 index 0000000000..4e832d9030 --- /dev/null +++ b/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; + +/// +/// Represents a JPEG comment +/// +public readonly struct JpegComData +{ + /// + /// Initializes a new instance of the struct. + /// + /// The comment buffer. + public JpegComData(ReadOnlyMemory value) + => this.Value = value; + + /// + /// Gets the value. + /// + public ReadOnlyMemory Value { get; } + + /// + /// Converts string to + /// + /// The comment string. + /// The + public static JpegComData FromString(string value) => new(value.AsMemory()); + + /// + public override string ToString() => this.Value.ToString(); +} diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index ccace190f9..906505b76a 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -480,9 +480,11 @@ internal void ParseStream(BufferedReadStream stream, SpectralConverter spectralC 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 @@ public void Dispose() this.scanDecoder = null; } + /// + /// Assigns COM marker bytes to comment property + /// + /// The input stream. + /// The remaining bytes in the segment block. + 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)); + } + /// /// Returns encoded colorspace based on the adobe APP14 marker. /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs index 7fc2a1f45e..243bbe051d 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs +++ b/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 /// private static readonly JpegFrameConfig[] FrameConfigs = CreateFrameConfigs(); + /// + /// The current calling encoder. + /// private readonly JpegEncoder encoder; /// @@ -89,6 +93,9 @@ public void Encode(Image image, Stream stream, CancellationToken // 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 @@ private void WriteJfifApplicationHeader(ImageMetadata meta, Span buffer) this.outputStream.Write(buffer, 0, 18); } + /// + /// Writes the COM tags. + /// + /// The configuration. + /// The image metadata. + private void WriteComments(Configuration configuration, JpegMetadata metadata) + { + if (metadata.Comments.Count == 0) + { + return; + } + + const int maxCommentLength = 65533; + using IMemoryOwner bufferOwner = configuration.MemoryAllocator.Allocate(maxCommentLength); + Span 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 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; + } + } + } + /// /// Writes the Define Huffman Table marker and tables. /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs b/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs index 59fc2f9cba..fe1324a862 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegMetadata.cs @@ -15,6 +15,7 @@ public class JpegMetadata : IDeepCloneable /// public JpegMetadata() { + this.Comments = new List(); } /// @@ -25,6 +26,7 @@ private JpegMetadata(JpegMetadata other) { this.ColorType = other.ColorType; + this.Comments = other.Comments; this.LuminanceQuality = other.LuminanceQuality; this.ChrominanceQuality = other.ChrominanceQuality; } @@ -101,6 +103,11 @@ public int Quality /// public bool? Progressive { get; internal set; } + /// + /// Gets the comments. + /// + public IList Comments { get; } + /// public IDeepCloneable DeepClone() => new JpegMetadata(this); } diff --git a/src/ImageSharp/Formats/Jpeg/MetadataExtensions.cs b/src/ImageSharp/Formats/Jpeg/MetadataExtensions.cs index 753dfdb60e..7330e74b79 100644 --- a/src/ImageSharp/Formats/Jpeg/MetadataExtensions.cs +++ b/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; diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs index 1c203e7342..cbb2befcd4 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs @@ -425,6 +425,21 @@ public void EncodedStringTags_Read() VerifyEncodedStrings(exif); } + [Theory] + [WithFile(TestImages.Jpeg.Issues.Issue2067_CommentMarker, PixelTypes.Rgba32)] + public void JpegDecoder_DecodeMetadataComment(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + string expectedComment = "TEST COMMENT"; + using Image 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); diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs index 2b721b9b51..f06fbe9635 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs +++ b/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(1, 1); - var expectedProfile = new IptcProfile(); + using Image 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(memStream); + using Image output = Image.Load(memStream); IptcProfile actual = output.Metadata.IptcProfile; Assert.NotNull(actual); IEnumerable values = expectedProfile.Values; @@ -55,17 +55,17 @@ public void Encode_PreservesIptcProfile() public void Encode_PreservesExifProfile() { // arrange - using var input = new Image(1, 1); + using Image 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(memStream); + using Image output = Image.Load(memStream); ExifProfile actual = output.Metadata.ExifProfile; Assert.NotNull(actual); IReadOnlyList values = input.Metadata.ExifProfile.Values; @@ -76,16 +76,16 @@ public void Encode_PreservesExifProfile() public void Encode_PreservesIccProfile() { // arrange - using var input = new Image(1, 1); + using Image 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(memStream); + using Image output = Image.Load(memStream); IccProfile actual = output.Metadata.IccProfile; Assert.NotNull(actual); IccProfile values = input.Metadata.IccProfile; @@ -99,12 +99,10 @@ public void Encode_WithValidExifProfile_DoesNotThrowException(TestImageP { Exception ex = Record.Exception(() => { - var encoder = new JpegEncoder(); - using (var stream = new MemoryStream()) - { - using Image image = provider.GetImage(JpegDecoder.Instance); - image.Save(stream, encoder); - } + JpegEncoder encoder = new(); + using MemoryStream stream = new(); + using Image image = provider.GetImage(JpegDecoder.Instance); + image.Save(stream, encoder); }); Assert.Null(ex); @@ -114,44 +112,99 @@ public void Encode_WithValidExifProfile_DoesNotThrowException(TestImageP [MemberData(nameof(RatioFiles))] public void Encode_PreserveRatio(string imagePath, int xResolution, int yResolution, PixelResolutionUnit resolutionUnit) { - var testFile = TestFile.Create(imagePath); - using (Image input = testFile.CreateRgba32Image()) - { - using (var memStream = new MemoryStream()) - { - input.Save(memStream, JpegEncoder); - - memStream.Position = 0; - using (var output = Image.Load(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 input = testFile.CreateRgba32Image(); + using MemoryStream memStream = new(); + input.Save(memStream, JpegEncoder); + + memStream.Position = 0; + using Image output = Image.Load(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 input = testFile.CreateRgba32Image()) - { - using (var memStream = new MemoryStream()) - { - input.Save(memStream, JpegEncoder); - - memStream.Position = 0; - using (var output = Image.Load(memStream)) - { - JpegMetadata meta = output.Metadata.GetJpegMetadata(); - Assert.Equal(quality, meta.Quality); - } - } - } + TestFile testFile = TestFile.Create(imagePath); + using Image input = testFile.CreateRgba32Image(); + using MemoryStream memStream = new(); + input.Save(memStream, JpegEncoder); + + memStream.Position = 0; + using Image output = Image.Load(memStream); + JpegMetadata meta = output.Metadata.GetJpegMetadata(); + Assert.Equal(quality, meta.Quality); + } + + [Theory] + [WithFile(TestImages.Jpeg.Issues.Issue2067_CommentMarker, PixelTypes.Rgba32)] + public void Encode_PreservesComments(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + // arrange + using Image input = provider.GetImage(JpegDecoder.Instance); + using MemoryStream memStream = new(); + + // act + input.Save(memStream, JpegEncoder); + + // assert + memStream.Position = 0; + using Image output = Image.Load(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 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 output = Image.Load(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 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 output = Image.Load(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 void Encode_PreservesColorType(TestImageProvider provider { // arrange using Image 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(memoryStream); + using Image output = Image.Load(memoryStream); JpegMetadata meta = output.Metadata.GetJpegMetadata(); Assert.Equal(expectedColorType, meta.ColorType); } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs index 05f22667dc..e07c42f898 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegMetadataTests.cs +++ b/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 void Quality_ReturnsMaxQuality() Assert.Equal(meta.Quality, qualityLuma); } + + [Fact] + public void Comment_EmptyComment() + { + var meta = new JpegMetadata(); + + Assert.True(Array.Empty().SequenceEqual(meta.Comments)); + } + + [Fact] + public void Comment_OnlyComment() + { + string comment = "test comment"; + var expectedCollection = new Collection { 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()); + } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index ac70792681..6be8ff6a68 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -315,6 +315,7 @@ public static class Issues 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 { diff --git a/tests/Images/Input/Jpg/issues/issue-2067-comment.jpg b/tests/Images/Input/Jpg/issues/issue-2067-comment.jpg new file mode 100644 index 0000000000..18dc6f2e32 --- /dev/null +++ b/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