diff --git a/src/ImageSharp/Formats/Jpeg/JpegComData.cs b/src/ImageSharp/Formats/Jpeg/JpegComData.cs index 26ade0217..4e832d903 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegComData.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegComData.cs @@ -1,32 +1,32 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Six Labors Split License. namespace SixLabors.ImageSharp.Formats.Jpeg; /// -/// Contains JPEG comment +/// Represents a JPEG comment /// public readonly struct JpegComData { - /// - /// Converts string to - /// - /// The comment string. - /// The - public static JpegComData FromString(string value) => new(value.AsMemory()); - /// /// Initializes a new instance of the struct. /// - /// The comment ReadOnlyMemory of chars. + /// The comment buffer. public JpegComData(ReadOnlyMemory value) => this.Value = value; + /// + /// Gets the value. + /// public ReadOnlyMemory Value { get; } /// - /// Converts Value to string + /// Converts string to /// - /// The comment string. + /// 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 cf5e449e7..906505b76 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -6,7 +6,6 @@ using System.Buffers; using System.Buffers.Binary; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; -using System.Text; using SixLabors.ImageSharp.Common.Helpers; using SixLabors.ImageSharp.Formats.Jpeg.Components; using SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder; @@ -481,6 +480,8 @@ internal sealed class JpegDecoderCore : IRawJpegData, IImageDecoderInternals break; case JpegConstants.Markers.APP15: + stream.Skip(markerContentByteSize); + break; case JpegConstants.Markers.COM: this.ProcessComMarker(stream, markerContentByteSize); break; @@ -523,16 +524,16 @@ internal sealed class JpegDecoderCore : IRawJpegData, IImageDecoderInternals /// The remaining bytes in the segment block. private void ProcessComMarker(BufferedReadStream stream, int markerContentByteSize) { - char[] temp = new char[markerContentByteSize]; + char[] chars = new char[markerContentByteSize]; JpegMetadata metadata = this.Metadata.GetFormatMetadata(JpegFormat.Instance); for (int i = 0; i < markerContentByteSize; i++) { int read = stream.ReadByte(); - temp[i] = (char)read; + chars[i] = (char)read; } - metadata.Comments.Add(new JpegComData(temp)); + metadata.Comments.Add(new JpegComData(chars)); } /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs index 4ef4cea2d..243bbe051 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs @@ -2,9 +2,8 @@ // Licensed under the Six Labors Split License. #nullable disable +using System.Buffers; using System.Buffers.Binary; -using System.Collections; -using System.Text; using SixLabors.ImageSharp.Common.Helpers; using SixLabors.ImageSharp.Formats.Jpeg.Components; using SixLabors.ImageSharp.Formats.Jpeg.Components.Encoder; @@ -27,6 +26,9 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals /// private static readonly JpegFrameConfig[] FrameConfigs = CreateFrameConfigs(); + /// + /// The current calling encoder. + /// private readonly JpegEncoder encoder; /// @@ -92,7 +94,7 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals this.WriteProfiles(metadata, buffer); // Write comments - this.WriteComment(jpegMetadata); + this.WriteComments(image.Configuration, jpegMetadata); // Write the image dimensions. this.WriteStartOfFrame(image.Width, image.Height, frameConfig, buffer); @@ -173,67 +175,48 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals } /// - /// Writes comment + /// Writes the COM tags. /// + /// The configuration. /// The image metadata. - private void WriteComment(JpegMetadata metadata) + private void WriteComments(Configuration configuration, JpegMetadata metadata) { - int maxCommentLength = 65533; - if (metadata.Comments.Count == 0) { return; } - // We don't want to modify original metadata - List comments = new(metadata.Comments); - - int totalPayloadLength = 0; - for (int i = 0; i < comments.Count; i++) + const int maxCommentLength = 65533; + using IMemoryOwner bufferOwner = configuration.MemoryAllocator.Allocate(maxCommentLength); + Span buffer = bufferOwner.Memory.Span; + foreach (JpegComData comment in metadata.Comments) { - JpegComData comment = comments[i]; - ReadOnlyMemory currentComment = comment.Value; - - if (comment.Value.Length > maxCommentLength) + int totalLength = comment.Value.Length; + if (totalLength == 0) { - ReadOnlyMemory splitComment = - currentComment.Slice(maxCommentLength, currentComment.Length - maxCommentLength); - comments.Insert(i + 1, new JpegComData(splitComment)); - - // We don't want to keep the extra bytes - comments[i] = new JpegComData(currentComment.Slice(0, maxCommentLength)); + continue; } - totalPayloadLength += comment.Value.Length + 4; - } - - Span payload = new byte[totalPayloadLength]; - int currentCommentStartingIndex = 0; - - for (int i = 0; i < comments.Count; i++) - { - ReadOnlyMemory comment = comments[i].Value; + // 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); - // Beginning of comment ff fe - payload[currentCommentStartingIndex] = JpegConstants.Markers.XFF; - payload[currentCommentStartingIndex + 1] = JpegConstants.Markers.COM; + // Write the marker header. + this.WriteMarkerHeader(JpegConstants.Markers.COM, currentLength + 2, buffer); - // Write payload size - int comWithoutMarker = comment.Length + 2; - payload[currentCommentStartingIndex + 2] = (byte)((comWithoutMarker >> 8) & 0xFF); - payload[currentCommentStartingIndex + 3] = (byte)(comWithoutMarker & 0xFF); + ReadOnlySpan commentValue = comment.Value.Span.Slice(comment.Value.Length - totalLength, currentLength); + for (int i = 0; i < commentValue.Length; i++) + { + buffer[i] = (byte)commentValue[i]; + } - char[] commentChars = comment.ToArray(); - for (int j = 0; j < commentChars.Length; j++) - { - // Initial 4 bytes are always reserved - payload[4 + currentCommentStartingIndex + j] = (byte)commentChars[j]; + // Write the comment. + this.outputStream.Write(buffer, 0, currentLength); + totalLength -= currentLength; } - - currentCommentStartingIndex += comment.Length + 4; } - - this.outputStream.Write(payload, 0, payload.Length); } /// diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs index f0593b462..f06fbe963 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 partial class JpegEncoderTests 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 partial class JpegEncoderTests 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 partial class JpegEncoderTests { 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,23 +112,17 @@ 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 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] @@ -138,20 +130,14 @@ public partial class JpegEncoderTests public void Encode_PreservesQuality(string imagePath, int quality) { TestFile 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); - } - } - } + 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] @@ -161,7 +147,7 @@ public partial class JpegEncoderTests { // arrange using Image input = provider.GetImage(JpegDecoder.Instance); - using var memStream = new MemoryStream(); + using MemoryStream memStream = new(); // act input.Save(memStream, JpegEncoder); @@ -172,16 +158,16 @@ public partial class JpegEncoderTests JpegMetadata actual = output.Metadata.GetJpegMetadata(); Assert.NotEmpty(actual.Comments); Assert.Equal(1, actual.Comments.Count); - Assert.Equal("TEST COMMENT", actual.Comments.ElementAtOrDefault(0).ToString()); + Assert.Equal("TEST COMMENT", actual.Comments[0].ToString()); } [Fact] public void Encode_SavesMultipleComments() { // arrange - using var input = new Image(1, 1); + using Image input = new(1, 1); JpegMetadata meta = input.Metadata.GetJpegMetadata(); - using var memStream = new MemoryStream(); + using MemoryStream memStream = new(); // act meta.Comments.Add(JpegComData.FromString("First comment")); @@ -194,8 +180,8 @@ public partial class JpegEncoderTests JpegMetadata actual = output.Metadata.GetJpegMetadata(); Assert.NotEmpty(actual.Comments); 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()); + Assert.Equal(meta.Comments[0].ToString(), actual.Comments[0].ToString()); + Assert.Equal(meta.Comments[1].ToString(), actual.Comments[1].ToString()); } [Fact] @@ -203,9 +189,9 @@ public partial class JpegEncoderTests { // arrange string longString = new('c', 65534); - using var input = new Image(1, 1); + using Image input = new(1, 1); JpegMetadata meta = input.Metadata.GetJpegMetadata(); - using var memStream = new MemoryStream(); + using MemoryStream memStream = new(); // act meta.Comments.Add(JpegComData.FromString(longString)); @@ -217,8 +203,8 @@ public partial class JpegEncoderTests JpegMetadata actual = output.Metadata.GetJpegMetadata(); Assert.NotEmpty(actual.Comments); Assert.Equal(2, actual.Comments.Count); - Assert.Equal(longString[..65533], actual.Comments.ElementAtOrDefault(0).ToString()); - Assert.Equal("c", actual.Comments.ElementAtOrDefault(1).ToString()); + Assert.Equal(longString[..65533], actual.Comments[0].ToString()); + Assert.Equal("c", actual.Comments[1].ToString()); } [Theory] @@ -231,14 +217,14 @@ public partial class JpegEncoderTests { // 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); }