From 6205e0dcd2c1f14a70cb89cf64bab2733a07f0b5 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 6 Jun 2025 22:53:38 +1000 Subject: [PATCH 1/6] Remove ExifTagValue.InteroperabilityIndex --- .../Metadata/Profiles/Exif/Tags/ExifTagValue.cs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/ImageSharp/Metadata/Profiles/Exif/Tags/ExifTagValue.cs b/src/ImageSharp/Metadata/Profiles/Exif/Tags/ExifTagValue.cs index 56e8a3ffd1..07dbc51e7d 100644 --- a/src/ImageSharp/Metadata/Profiles/Exif/Tags/ExifTagValue.cs +++ b/src/ImageSharp/Metadata/Profiles/Exif/Tags/ExifTagValue.cs @@ -23,14 +23,6 @@ internal enum ExifTagValue /// GPSIFDOffset = 0x8825, - /// - /// Indicates the identification of the Interoperability rule. - /// See https://www.awaresystems.be/imaging/tiff/tifftags/privateifd/interoperability/interoperabilityindex.html - /// - [ExifTagDescription("R98", "Indicates a file conforming to R98 file specification of Recommended Exif Interoperability Rules (ExifR98) or to DCF basic file stipulated by Design Rule for Camera File System.")] - [ExifTagDescription("THM", "Indicates a file conforming to DCF thumbnail file stipulated by Design rule for Camera File System.")] - InteroperabilityIndex = 0x0001, - /// /// A general indication of the kind of data contained in this subfile. /// See Section 8: Baseline Fields. From 5f40590c884557b24222f3dae1b8b21c97999fb2 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 11 Jun 2025 14:04:11 +1000 Subject: [PATCH 2/6] Allow additional and undefined extra samples --- .../Formats/Tiff/TiffDecoderOptionsParser.cs | 20 ++++++++++--------- .../Formats/Tiff/TiffExtraSampleType.cs | 8 +++++++- .../Formats/Tiff/TiffDecoderTests.cs | 5 +++++ tests/ImageSharp.Tests/TestImages.cs | 1 + 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs index 5a5c2b3e51..db349b8b69 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderOptionsParser.cs @@ -25,20 +25,22 @@ internal static class TiffDecoderOptionsParser /// True, if the image uses tiles. Otherwise the images has strip's. public static bool VerifyAndParse(this TiffDecoderCore options, ExifProfile exifProfile, TiffFrameMetadata frameMetadata) { - IExifValue extraSamplesExifValue = exifProfile.GetValueInternal(ExifTag.ExtraSamples); - if (extraSamplesExifValue is not null) + if (exifProfile.TryGetValue(ExifTag.ExtraSamples, out IExifValue samples)) { - short[] extraSamples = (short[])extraSamplesExifValue.GetValue(); - if (extraSamples.Length != 1) + // We only support a single sample pertaining to alpha data. + // Other information is discarded. + TiffExtraSampleType sampleType = (TiffExtraSampleType)samples.Value[0]; + if (sampleType is TiffExtraSampleType.CorelDrawUnassociatedAlphaData) { - TiffThrowHelper.ThrowNotSupported("ExtraSamples is only supported with one extra sample for alpha data."); + // According to libtiff, this CorelDRAW-specific value indicates unassociated alpha. + // Patch required for compatibility with malformed CorelDRAW-generated TIFFs. + // https://libtiff.gitlab.io/libtiff/releases/v3.9.0beta.html + sampleType = TiffExtraSampleType.UnassociatedAlphaData; } - TiffExtraSampleType extraSamplesType = (TiffExtraSampleType)extraSamples[0]; - options.ExtraSamplesType = extraSamplesType; - if (extraSamplesType is not (TiffExtraSampleType.UnassociatedAlphaData or TiffExtraSampleType.AssociatedAlphaData)) + if (sampleType is (TiffExtraSampleType.UnassociatedAlphaData or TiffExtraSampleType.AssociatedAlphaData)) { - TiffThrowHelper.ThrowNotSupported("Decoding Tiff images with ExtraSamples is not supported with UnspecifiedData."); + options.ExtraSamplesType = sampleType; } } diff --git a/src/ImageSharp/Formats/Tiff/TiffExtraSampleType.cs b/src/ImageSharp/Formats/Tiff/TiffExtraSampleType.cs index 484fc993b0..72d56e2c38 100644 --- a/src/ImageSharp/Formats/Tiff/TiffExtraSampleType.cs +++ b/src/ImageSharp/Formats/Tiff/TiffExtraSampleType.cs @@ -22,5 +22,11 @@ internal enum TiffExtraSampleType /// The extra data is unassociated alpha data is transparency information that logically exists independent of an image; /// it is commonly called a soft matte. /// - UnassociatedAlphaData = 2 + UnassociatedAlphaData = 2, + + /// + /// A CorelDRAW-specific value observed in damaged files, indicating unassociated alpha. + /// Not part of the official TIFF specification; patched in ImageSharp for compatibility. + /// + CorelDrawUnassociatedAlphaData = 999 } diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index 839334449d..d66a8def41 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -772,4 +772,9 @@ public void TiffDecoder_Decode_Resize(TestImageProvider provider testOutputDetails: details, appendPixelTypeToFileName: false); } + + [Theory] + [WithFile(ExtraSamplesUnspecified, PixelTypes.Rgba32)] + public void TiffDecoder_CanDecode_ExtraSamplesUnspecified(TestImageProvider provider) + where TPixel : unmanaged, IPixel => TestTiffDecoder(provider); } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 5a7c2e66ea..6021de15cc 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -1112,6 +1112,7 @@ public static class Tiff public const string Issues2587 = "Tiff/Issues/Issue2587.tiff"; public const string JpegCompressedGray0000539558 = "Tiff/Issues/JpegCompressedGray-0000539558.tiff"; public const string Tiled0000023664 = "Tiff/Issues/tiled-0000023664.tiff"; + public const string ExtraSamplesUnspecified = "Tiff/Issues/ExtraSamplesUnspecified.tif"; public const string SmallRgbDeflate = "Tiff/rgb_small_deflate.tiff"; public const string SmallRgbLzw = "Tiff/rgb_small_lzw.tiff"; From 379cc6289151f5f3b7f6833526a0261d568c2895 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 11 Jun 2025 14:33:35 +1000 Subject: [PATCH 3/6] Create ExtraSamplesUnspecified.tif --- tests/Images/Input/Tiff/Issues/ExtraSamplesUnspecified.tif | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 tests/Images/Input/Tiff/Issues/ExtraSamplesUnspecified.tif diff --git a/tests/Images/Input/Tiff/Issues/ExtraSamplesUnspecified.tif b/tests/Images/Input/Tiff/Issues/ExtraSamplesUnspecified.tif new file mode 100644 index 0000000000..b5835c1ed8 --- /dev/null +++ b/tests/Images/Input/Tiff/Issues/ExtraSamplesUnspecified.tif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:71d28f17d2d56481faa4d241fae882eae4f8303c70f85bc3759f6a0c2074979e +size 1426558 From 5f01d1597c884946689857d280ee3597448e12c4 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 11 Jun 2025 19:48:27 +1000 Subject: [PATCH 4/6] Use EXIF byte order for EXIF encoded strings --- .../Profiles/Exif/ExifEncodedStringHelpers.cs | 45 +++++++++++++++---- .../Metadata/Profiles/Exif/ExifReader.cs | 23 ++++++++-- .../Metadata/Profiles/Exif/Tags/ExifTag.cs | 4 +- .../Profiles/Exif/Values/ExifEncodedString.cs | 7 ++- .../Formats/WebP/WebpDecoderTests.cs | 19 ++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Webp/issues/Issue2906.webp | 3 ++ 7 files changed, 87 insertions(+), 15 deletions(-) create mode 100644 tests/Images/Input/Webp/issues/Issue2906.webp diff --git a/src/ImageSharp/Metadata/Profiles/Exif/ExifEncodedStringHelpers.cs b/src/ImageSharp/Metadata/Profiles/Exif/ExifEncodedStringHelpers.cs index e9f46731c8..9833e4e00a 100644 --- a/src/ImageSharp/Metadata/Profiles/Exif/ExifEncodedStringHelpers.cs +++ b/src/ImageSharp/Metadata/Profiles/Exif/ExifEncodedStringHelpers.cs @@ -50,22 +50,51 @@ private static Encoding JIS0208Encoding _ => UndefinedCodeBytes }; - public static Encoding GetEncoding(CharacterCode code) => code switch + public static Encoding GetEncoding(CharacterCode code, ByteOrder order) => code switch { CharacterCode.ASCII => Encoding.ASCII, CharacterCode.JIS => JIS0208Encoding, - CharacterCode.Unicode => Encoding.Unicode, + CharacterCode.Unicode => order is ByteOrder.BigEndian ? Encoding.BigEndianUnicode : Encoding.Unicode, CharacterCode.Undefined => Encoding.UTF8, _ => Encoding.UTF8 }; - public static bool TryParse(ReadOnlySpan buffer, out EncodedString encodedString) + public static bool TryParse(ReadOnlySpan buffer, ByteOrder order, out EncodedString encodedString) { if (TryDetect(buffer, out CharacterCode code)) { - string text = GetEncoding(code).GetString(buffer[CharacterCodeBytesLength..]); - encodedString = new EncodedString(code, text); - return true; + ReadOnlySpan textBuffer = buffer[CharacterCodeBytesLength..]; + if (code == CharacterCode.Unicode && textBuffer.Length >= 2) + { + // Check BOM + if (textBuffer[0] == 0xFF && textBuffer[1] == 0xFE) + { + // Little-endian BOM + string text = Encoding.Unicode.GetString(textBuffer[2..]); + encodedString = new EncodedString(code, text); + return true; + } + else if (textBuffer[0] == 0xFE && textBuffer[1] == 0xFF) + { + // Big-endian BOM + string text = Encoding.BigEndianUnicode.GetString(textBuffer[2..]); + encodedString = new EncodedString(code, text); + return true; + } + else + { + // No BOM, use EXIF byte order + string text = GetEncoding(code, order).GetString(textBuffer); + encodedString = new EncodedString(code, text); + return true; + } + } + else + { + string text = GetEncoding(code, order).GetString(textBuffer); + encodedString = new EncodedString(code, text); + return true; + } } encodedString = default; @@ -73,14 +102,14 @@ public static bool TryParse(ReadOnlySpan buffer, out EncodedString encoded } public static uint GetDataLength(EncodedString encodedString) => - (uint)GetEncoding(encodedString.Code).GetByteCount(encodedString.Text) + CharacterCodeBytesLength; + (uint)GetEncoding(encodedString.Code, ByteOrder.LittleEndian).GetByteCount(encodedString.Text) + CharacterCodeBytesLength; public static int Write(EncodedString encodedString, Span destination) { GetCodeBytes(encodedString.Code).CopyTo(destination); string text = encodedString.Text; - int count = Write(GetEncoding(encodedString.Code), text, destination[CharacterCodeBytesLength..]); + int count = Write(GetEncoding(encodedString.Code, ByteOrder.LittleEndian), text, destination[CharacterCodeBytesLength..]); return CharacterCodeBytesLength + count; } diff --git a/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs b/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs index fe510f250d..d0dcaea9d5 100644 --- a/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs +++ b/src/ImageSharp/Metadata/Profiles/Exif/ExifReader.cs @@ -90,8 +90,8 @@ internal abstract class BaseExifReader private readonly MemoryAllocator? allocator; private readonly Stream data; private List? invalidTags; - private List? subIfds; + private bool isBigEndian; protected BaseExifReader(Stream stream, MemoryAllocator? allocator) { @@ -116,7 +116,17 @@ protected BaseExifReader(Stream stream, MemoryAllocator? allocator) /// public uint ThumbnailOffset { get; protected set; } - public bool IsBigEndian { get; protected set; } + public bool IsBigEndian + { + get => this.isBigEndian; + protected set + { + this.isBigEndian = value; + this.ByteOrder = value ? ByteOrder.BigEndian : ByteOrder.LittleEndian; + } + } + + protected ByteOrder ByteOrder { get; private set; } public List<(ulong Offset, ExifDataType DataType, ulong NumberOfComponents, ExifValue Exif)> BigValues { get; } = new(); @@ -485,7 +495,14 @@ private void ReadValue64(List values, Span offsetBuffer) private void Add(IList values, IExifValue exif, object? value) { - if (!exif.TrySetValue(value)) + if (exif is ExifEncodedString encodedString) + { + if (!encodedString.TrySetValue(value, this.ByteOrder)) + { + return; + } + } + else if (!exif.TrySetValue(value)) { return; } diff --git a/src/ImageSharp/Metadata/Profiles/Exif/Tags/ExifTag.cs b/src/ImageSharp/Metadata/Profiles/Exif/Tags/ExifTag.cs index ea0b8060d5..1744241df5 100644 --- a/src/ImageSharp/Metadata/Profiles/Exif/Tags/ExifTag.cs +++ b/src/ImageSharp/Metadata/Profiles/Exif/Tags/ExifTag.cs @@ -23,14 +23,14 @@ public abstract partial class ExifTag : IEquatable /// /// The first to compare. /// The second to compare. - public static bool operator ==(ExifTag left, ExifTag right) => Equals(left, right); + public static bool operator ==(ExifTag left, ExifTag right) => left?.Equals(right) == true; /// /// Determines whether the specified instances are not considered equal. /// /// The first to compare. /// The second to compare. - public static bool operator !=(ExifTag left, ExifTag right) => !Equals(left, right); + public static bool operator !=(ExifTag left, ExifTag right) => !(left == right); /// public override bool Equals(object? obj) diff --git a/src/ImageSharp/Metadata/Profiles/Exif/Values/ExifEncodedString.cs b/src/ImageSharp/Metadata/Profiles/Exif/Values/ExifEncodedString.cs index cce7cf3e89..14b097f816 100644 --- a/src/ImageSharp/Metadata/Profiles/Exif/Values/ExifEncodedString.cs +++ b/src/ImageSharp/Metadata/Profiles/Exif/Values/ExifEncodedString.cs @@ -24,7 +24,7 @@ private ExifEncodedString(ExifEncodedString value) protected override string StringValue => this.Value.Text; - public override bool TrySetValue(object? value) + public bool TrySetValue(object? value, ByteOrder order) { if (base.TrySetValue(value)) { @@ -38,7 +38,7 @@ public override bool TrySetValue(object? value) } else if (value is byte[] buffer) { - if (ExifEncodedStringHelpers.TryParse(buffer, out EncodedString encodedString)) + if (ExifEncodedStringHelpers.TryParse(buffer, order, out EncodedString encodedString)) { this.Value = encodedString; return true; @@ -48,5 +48,8 @@ public override bool TrySetValue(object? value) return false; } + public override bool TrySetValue(object? value) + => this.TrySetValue(value, ByteOrder.LittleEndian); + public override IExifValue DeepClone() => new ExifEncodedString(this); } diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs index d81cd20849..c8d780cc20 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs @@ -5,6 +5,7 @@ using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Webp; using SixLabors.ImageSharp.Metadata; +using SixLabors.ImageSharp.Metadata.Profiles.Exif; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Tests.TestUtilities; using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; @@ -559,4 +560,22 @@ public void WebpDecoder_CanDecode_Issue2925(TestImageProvider pr image.DebugSave(provider); image.CompareToOriginal(provider, ReferenceDecoder); } + + [Theory] + [WithFile(Lossy.Issue2906, PixelTypes.Rgba32)] + public void WebpDecoder_CanDecode_Issue2906(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(WebpDecoder.Instance); + + ExifProfile exifProfile = image.Metadata.ExifProfile; + IExifValue comment = exifProfile.GetValue(ExifTag.UserComment); + + Assert.NotNull(comment); + Assert.Equal(EncodedString.CharacterCode.Unicode, comment.Value.Code); + Assert.StartsWith("1girl, pariya, ", comment.Value.Text); + + image.DebugSave(provider); + image.CompareToOriginal(provider, ReferenceDecoder); + } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 5a7c2e66ea..7f12332238 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -860,6 +860,7 @@ public static class Lossy public const string Issue2801 = "Webp/issues/Issue2801.webp"; public const string Issue2866 = "Webp/issues/Issue2866.webp"; public const string Issue2925 = "Webp/issues/Issue2925.webp"; + public const string Issue2906 = "Webp/issues/Issue2906.webp"; } } diff --git a/tests/Images/Input/Webp/issues/Issue2906.webp b/tests/Images/Input/Webp/issues/Issue2906.webp new file mode 100644 index 0000000000..0911da0472 --- /dev/null +++ b/tests/Images/Input/Webp/issues/Issue2906.webp @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:56fe6a91feb9545c0a15966e0f6bc560890b193073c96ae9e39bf387c7e0cbca +size 157092 From 5dfe7a142cd501da37b73338b01a5a6325a6718b Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 11 Jun 2025 20:09:01 +1000 Subject: [PATCH 5/6] Update ExifTag.cs --- src/ImageSharp/Metadata/Profiles/Exif/Tags/ExifTag.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Metadata/Profiles/Exif/Tags/ExifTag.cs b/src/ImageSharp/Metadata/Profiles/Exif/Tags/ExifTag.cs index 1744241df5..8ffc2bb738 100644 --- a/src/ImageSharp/Metadata/Profiles/Exif/Tags/ExifTag.cs +++ b/src/ImageSharp/Metadata/Profiles/Exif/Tags/ExifTag.cs @@ -23,14 +23,14 @@ public abstract partial class ExifTag : IEquatable /// /// The first to compare. /// The second to compare. - public static bool operator ==(ExifTag left, ExifTag right) => left?.Equals(right) == true; + public static bool operator ==(ExifTag? left, ExifTag? right) => left?.Equals(right) == true; /// /// Determines whether the specified instances are not considered equal. /// /// The first to compare. /// The second to compare. - public static bool operator !=(ExifTag left, ExifTag right) => !(left == right); + public static bool operator !=(ExifTag? left, ExifTag? right) => !(left == right); /// public override bool Equals(object? obj) From 57f58fe3cca5f6f3372065ba1c49847a52fae8aa Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 13 Jun 2025 15:04:07 +1000 Subject: [PATCH 6/6] Backport #2940 --- .../Webp/Lossless/BackwardReferenceEncoder.cs | 25 +++++------ .../Formats/Webp/Lossless/CostModel.cs | 4 +- .../Formats/Webp/Lossless/HistogramEncoder.cs | 19 ++++---- .../Formats/Webp/Lossless/PixOrCopy.cs | 45 +++++++------------ .../Formats/Webp/Lossless/Vp8LBackwardRefs.cs | 29 +++++++----- .../Formats/Webp/Lossless/Vp8LEncoder.cs | 20 ++++----- .../Formats/Webp/Lossless/Vp8LHistogram.cs | 6 +-- .../Formats/WebP/Vp8LHistogramTests.cs | 12 ++--- 8 files changed, 71 insertions(+), 89 deletions(-) diff --git a/src/ImageSharp/Formats/Webp/Lossless/BackwardReferenceEncoder.cs b/src/ImageSharp/Formats/Webp/Lossless/BackwardReferenceEncoder.cs index 2e7dd722fc..defa63ed06 100644 --- a/src/ImageSharp/Formats/Webp/Lossless/BackwardReferenceEncoder.cs +++ b/src/ImageSharp/Formats/Webp/Lossless/BackwardReferenceEncoder.cs @@ -149,9 +149,8 @@ private static int CalculateBestCacheSize( } // Find the cacheBits giving the lowest entropy. - for (int idx = 0; idx < refs.Refs.Count; idx++) + foreach (PixOrCopy v in refs) { - PixOrCopy v = refs.Refs[idx]; if (v.IsLiteral()) { uint pix = bgra[pos++]; @@ -387,7 +386,7 @@ private static void BackwardReferencesHashChainFollowChosenPath(ReadOnlySpan bgra, int cach { int pixelIndex = 0; ColorCache colorCache = new ColorCache(cacheBits); - for (int idx = 0; idx < refs.Refs.Count; idx++) + foreach (ref PixOrCopy v in refs) { - PixOrCopy v = refs.Refs[idx]; if (v.IsLiteral()) { uint bgraLiteral = v.BgraOrDistance; @@ -790,9 +788,7 @@ private static void BackwardRefsWithLocalCache(ReadOnlySpan bgra, int cach if (ix >= 0) { // Color cache contains bgraLiteral - v.Mode = PixOrCopyMode.CacheIdx; - v.BgraOrDistance = (uint)ix; - v.Len = 1; + v = PixOrCopy.CreateCacheIdx(ix); } else { @@ -814,14 +810,13 @@ private static void BackwardRefsWithLocalCache(ReadOnlySpan bgra, int cach private static void BackwardReferences2DLocality(int xSize, Vp8LBackwardRefs refs) { - using List.Enumerator c = refs.Refs.GetEnumerator(); - while (c.MoveNext()) + foreach (ref PixOrCopy v in refs) { - if (c.Current.IsCopy()) + if (v.IsCopy()) { - int dist = (int)c.Current.BgraOrDistance; + int dist = (int)v.BgraOrDistance; int transformedDist = DistanceToPlaneCode(xSize, dist); - c.Current.BgraOrDistance = (uint)transformedDist; + v = PixOrCopy.CreateCopy((uint)transformedDist, v.Len); } } } diff --git a/src/ImageSharp/Formats/Webp/Lossless/CostModel.cs b/src/ImageSharp/Formats/Webp/Lossless/CostModel.cs index beebc48abc..c6131bc2aa 100644 --- a/src/ImageSharp/Formats/Webp/Lossless/CostModel.cs +++ b/src/ImageSharp/Formats/Webp/Lossless/CostModel.cs @@ -40,9 +40,9 @@ public void Build(int xSize, int cacheBits, Vp8LBackwardRefs backwardRefs) using OwnedVp8LHistogram histogram = OwnedVp8LHistogram.Create(this.memoryAllocator, cacheBits); // The following code is similar to HistogramCreate but converts the distance to plane code. - for (int i = 0; i < backwardRefs.Refs.Count; i++) + foreach (PixOrCopy v in backwardRefs) { - histogram.AddSinglePixOrCopy(backwardRefs.Refs[i], true, xSize); + histogram.AddSinglePixOrCopy(in v, true, xSize); } ConvertPopulationCountTableToBitEstimates(histogram.NumCodes(), histogram.Literal, this.Literal); diff --git a/src/ImageSharp/Formats/Webp/Lossless/HistogramEncoder.cs b/src/ImageSharp/Formats/Webp/Lossless/HistogramEncoder.cs index 3a96362cfd..595332eeef 100644 --- a/src/ImageSharp/Formats/Webp/Lossless/HistogramEncoder.cs +++ b/src/ImageSharp/Formats/Webp/Lossless/HistogramEncoder.cs @@ -109,12 +109,11 @@ private static void HistogramBuild( { int x = 0, y = 0; int histoXSize = LosslessUtils.SubSampleSize(xSize, histoBits); - using List.Enumerator backwardRefsEnumerator = backwardRefs.Refs.GetEnumerator(); - while (backwardRefsEnumerator.MoveNext()) + + foreach (PixOrCopy v in backwardRefs) { - PixOrCopy v = backwardRefsEnumerator.Current; int ix = ((y >> histoBits) * histoXSize) + (x >> histoBits); - histograms[ix].AddSinglePixOrCopy(v, false); + histograms[ix].AddSinglePixOrCopy(in v, false); x += v.Len; while (x >= xSize) { @@ -465,7 +464,7 @@ private static bool HistogramCombineStochastic(Vp8LHistogramSet histograms, int } } - HistoListUpdateHead(histoPriorityList, p); + HistoListUpdateHead(histoPriorityList, p, j); j++; } @@ -525,7 +524,7 @@ private static void HistogramCombineGreedy(Vp8LHistogramSet histograms) } else { - HistoListUpdateHead(histoPriorityList, p); + HistoListUpdateHead(histoPriorityList, p, i); i++; } } @@ -647,7 +646,7 @@ private static double HistoPriorityListPush( histoList.Add(pair); - HistoListUpdateHead(histoList, pair); + HistoListUpdateHead(histoList, pair, histoList.Count - 1); return pair.CostDiff; } @@ -674,13 +673,11 @@ private static void HistoListUpdatePair( /// /// Check whether a pair in the list should be updated as head or not. /// - private static void HistoListUpdateHead(List histoList, HistogramPair pair) + private static void HistoListUpdateHead(List histoList, HistogramPair pair, int idx) { if (pair.CostDiff < histoList[0].CostDiff) { - // Replace the best pair. - int oldIdx = histoList.IndexOf(pair); - histoList[oldIdx] = histoList[0]; + histoList[idx] = histoList[0]; histoList[0] = pair; } } diff --git a/src/ImageSharp/Formats/Webp/Lossless/PixOrCopy.cs b/src/ImageSharp/Formats/Webp/Lossless/PixOrCopy.cs index d6b10ada55..bb8ce18aad 100644 --- a/src/ImageSharp/Formats/Webp/Lossless/PixOrCopy.cs +++ b/src/ImageSharp/Formats/Webp/Lossless/PixOrCopy.cs @@ -6,37 +6,24 @@ namespace SixLabors.ImageSharp.Formats.Webp.Lossless; [DebuggerDisplay("Mode: {Mode}, Len: {Len}, BgraOrDistance: {BgraOrDistance}")] -internal sealed class PixOrCopy +internal readonly struct PixOrCopy { - public PixOrCopyMode Mode { get; set; } - - public ushort Len { get; set; } - - public uint BgraOrDistance { get; set; } - - public static PixOrCopy CreateCacheIdx(int idx) => - new PixOrCopy - { - Mode = PixOrCopyMode.CacheIdx, - BgraOrDistance = (uint)idx, - Len = 1 - }; - - public static PixOrCopy CreateLiteral(uint bgra) => - new PixOrCopy - { - Mode = PixOrCopyMode.Literal, - BgraOrDistance = bgra, - Len = 1 - }; - - public static PixOrCopy CreateCopy(uint distance, ushort len) => - new PixOrCopy + public readonly PixOrCopyMode Mode; + public readonly ushort Len; + public readonly uint BgraOrDistance; + + private PixOrCopy(PixOrCopyMode mode, ushort len, uint bgraOrDistance) { - Mode = PixOrCopyMode.Copy, - BgraOrDistance = distance, - Len = len - }; + this.Mode = mode; + this.Len = len; + this.BgraOrDistance = bgraOrDistance; + } + + public static PixOrCopy CreateCacheIdx(int idx) => new(PixOrCopyMode.CacheIdx, 1, (uint)idx); + + public static PixOrCopy CreateLiteral(uint bgra) => new(PixOrCopyMode.Literal, 1, bgra); + + public static PixOrCopy CreateCopy(uint distance, ushort len) => new(PixOrCopyMode.Copy, len, distance); public int Literal(int component) => (int)(this.BgraOrDistance >> (component * 8)) & 0xFF; diff --git a/src/ImageSharp/Formats/Webp/Lossless/Vp8LBackwardRefs.cs b/src/ImageSharp/Formats/Webp/Lossless/Vp8LBackwardRefs.cs index ace9d62271..634fac5e82 100644 --- a/src/ImageSharp/Formats/Webp/Lossless/Vp8LBackwardRefs.cs +++ b/src/ImageSharp/Formats/Webp/Lossless/Vp8LBackwardRefs.cs @@ -1,21 +1,28 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Buffers; +using SixLabors.ImageSharp.Memory; + namespace SixLabors.ImageSharp.Formats.Webp.Lossless; -internal class Vp8LBackwardRefs +internal class Vp8LBackwardRefs : IDisposable { - public Vp8LBackwardRefs(int pixels) => this.Refs = new List(pixels); + private readonly IMemoryOwner refs; + private int count; + + public Vp8LBackwardRefs(MemoryAllocator memoryAllocator, int pixels) + { + this.refs = memoryAllocator.Allocate(pixels); + this.count = 0; + } + + public void Add(PixOrCopy pixOrCopy) => this.refs.Memory.Span[this.count++] = pixOrCopy; - /// - /// Gets or sets the common block-size. - /// - public int BlockSize { get; set; } + public void Clear() => this.count = 0; - /// - /// Gets the backward references. - /// - public List Refs { get; } + public Span.Enumerator GetEnumerator() => this.refs.Slice(0, this.count).GetEnumerator(); - public void Add(PixOrCopy pixOrCopy) => this.Refs.Add(pixOrCopy); + /// + public void Dispose() => this.refs.Dispose(); } diff --git a/src/ImageSharp/Formats/Webp/Lossless/Vp8LEncoder.cs b/src/ImageSharp/Formats/Webp/Lossless/Vp8LEncoder.cs index 0351798b5f..0ef110bb9f 100644 --- a/src/ImageSharp/Formats/Webp/Lossless/Vp8LEncoder.cs +++ b/src/ImageSharp/Formats/Webp/Lossless/Vp8LEncoder.cs @@ -137,14 +137,9 @@ public Vp8LEncoder( this.Refs = new Vp8LBackwardRefs[3]; this.HashChain = new Vp8LHashChain(memoryAllocator, pixelCount); - // We round the block size up, so we're guaranteed to have at most MaxRefsBlockPerImage blocks used: - int refsBlockSize = ((pixelCount - 1) / MaxRefsBlockPerImage) + 1; for (int i = 0; i < this.Refs.Length; i++) { - this.Refs[i] = new Vp8LBackwardRefs(pixelCount) - { - BlockSize = refsBlockSize < MinBlockSize ? MinBlockSize : refsBlockSize - }; + this.Refs[i] = new Vp8LBackwardRefs(memoryAllocator, pixelCount); } } @@ -1071,9 +1066,8 @@ private void StoreImageToBitMask( int histogramIx = histogramSymbols[0]; Span codes = huffmanCodes.AsSpan(5 * histogramIx); - for (int i = 0; i < backwardRefs.Refs.Count; i++) + foreach (PixOrCopy v in backwardRefs) { - PixOrCopy v = backwardRefs.Refs[i]; if (tileX != (x & tileMask) || tileY != (y & tileMask)) { tileX = x & tileMask; @@ -1907,9 +1901,9 @@ public void AllocateTransformBuffer(int width, int height) /// public void ClearRefs() { - foreach (Vp8LBackwardRefs t in this.Refs) + foreach (Vp8LBackwardRefs refs in this.Refs) { - t.Refs.Clear(); + refs.Clear(); } } @@ -1921,6 +1915,12 @@ public void Dispose() this.BgraScratch?.Dispose(); this.Palette.Dispose(); this.TransformData?.Dispose(); + + foreach (Vp8LBackwardRefs refs in this.Refs) + { + refs.Dispose(); + } + this.HashChain.Dispose(); } diff --git a/src/ImageSharp/Formats/Webp/Lossless/Vp8LHistogram.cs b/src/ImageSharp/Formats/Webp/Lossless/Vp8LHistogram.cs index f473977908..03bedfe672 100644 --- a/src/ImageSharp/Formats/Webp/Lossless/Vp8LHistogram.cs +++ b/src/ImageSharp/Formats/Webp/Lossless/Vp8LHistogram.cs @@ -138,9 +138,9 @@ public void Clear() /// The backward references. public void StoreRefs(Vp8LBackwardRefs refs) { - for (int i = 0; i < refs.Refs.Count; i++) + foreach (PixOrCopy v in refs) { - this.AddSinglePixOrCopy(refs.Refs[i], false); + this.AddSinglePixOrCopy(in v, false); } } @@ -150,7 +150,7 @@ public void StoreRefs(Vp8LBackwardRefs refs) /// The token to add. /// Indicates whether to use the distance modifier. /// xSize is only used when useDistanceModifier is true. - public void AddSinglePixOrCopy(PixOrCopy v, bool useDistanceModifier, int xSize = 0) + public void AddSinglePixOrCopy(in PixOrCopy v, bool useDistanceModifier, int xSize = 0) { if (v.IsLiteral()) { diff --git a/tests/ImageSharp.Tests/Formats/WebP/Vp8LHistogramTests.cs b/tests/ImageSharp.Tests/Formats/WebP/Vp8LHistogramTests.cs index cfe79e49e6..7777c61084 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/Vp8LHistogramTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/Vp8LHistogramTests.cs @@ -66,18 +66,14 @@ private static void RunAddVectorTest() // All remaining values are expected to be zero. literals.AsSpan().CopyTo(expectedLiterals); - Vp8LBackwardRefs backwardRefs = new(pixelData.Length); + MemoryAllocator memoryAllocator = Configuration.Default.MemoryAllocator; + + using Vp8LBackwardRefs backwardRefs = new(memoryAllocator, pixelData.Length); for (int i = 0; i < pixelData.Length; i++) { - backwardRefs.Add(new PixOrCopy() - { - BgraOrDistance = pixelData[i], - Len = 1, - Mode = PixOrCopyMode.Literal - }); + backwardRefs.Add(PixOrCopy.CreateLiteral(pixelData[i])); } - MemoryAllocator memoryAllocator = Configuration.Default.MemoryAllocator; using OwnedVp8LHistogram histogram0 = OwnedVp8LHistogram.Create(memoryAllocator, backwardRefs, 3); using OwnedVp8LHistogram histogram1 = OwnedVp8LHistogram.Create(memoryAllocator, backwardRefs, 3); for (int i = 0; i < 5; i++)