From e9aa4bffbb0a6bc6f6ca1828eb2cef60129d4bbc Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 27 Jul 2026 13:15:16 +1000 Subject: [PATCH] Address ANI and icon review feedback --- src/ImageSharp/Formats/Ani/AniDecoderCore.cs | 125 ++++++++++++------ src/ImageSharp/Formats/Ani/AniEncoderCore.cs | 50 ++++++- src/ImageSharp/Formats/Ani/AniMetadata.cs | 6 +- src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs | 11 +- .../Formats/Icon/IconDecoderCore.cs | 25 +++- .../Formats/Icon/IconEncoderCore.cs | 115 ++++++++++------ .../Formats/Ani/AniDecoderTests.cs | 2 +- .../Formats/Ani/AniEncoderTests.cs | 86 ++++++++++++ .../Formats/Icon/Ico/IcoEncoderTests.cs | 20 +++ 9 files changed, 338 insertions(+), 102 deletions(-) diff --git a/src/ImageSharp/Formats/Ani/AniDecoderCore.cs b/src/ImageSharp/Formats/Ani/AniDecoderCore.cs index 5015efc25..39583cfe4 100644 --- a/src/ImageSharp/Formats/Ani/AniDecoderCore.cs +++ b/src/ImageSharp/Formats/Ani/AniDecoderCore.cs @@ -310,7 +310,7 @@ internal sealed class AniDecoderCore : ImageDecoderCore, IDisposable ReadExactly(stream, data, "ANI header"); this.header = AniHeader.Parse(data); - if (this.header.BytesInHeader < AniHeader.Size) + if (this.header.BytesInHeader < AniHeader.Size || this.header.BytesInHeader > chunkSize) { throw new InvalidImageContentException("The ANI animation header declares an invalid size."); } @@ -412,6 +412,12 @@ internal sealed class AniDecoderCore : ImageDecoderCore, IDisposable } int count = (int)Math.Min(chunkSize / sizeof(uint), this.Options.MaxFrames); + if (count is 0) + { + this.ThrowOrIgnoreNonStrictSegmentError($"The ANI {description} chunk does not contain any values."); + return; + } + IMemoryOwner valuesOwner; bool replaceOwner; @@ -532,71 +538,102 @@ internal sealed class AniDecoderCore : ImageDecoderCore, IDisposable bool hasSequence = this.sequence is not null; int decodedResourceCount = 0; int maxDecodedResources = (int)this.Options.MaxFrames; - uint lastRequiredResource = 0; + IMemoryOwner? sortedSequenceOwner = null; - if (hasSequence) + try { - if (sequence.IsEmpty) + ReadOnlySpan requiredResources = sequence; + if (hasSequence) { - return; - } + bool isSorted = true; + for (int i = 1; i < sequence.Length; i++) + { + if (sequence[i] < sequence[i - 1]) + { + isSorted = false; + break; + } + } - // Only resources referenced by the retained sequence steps can contribute to the bounded output frame set. - for (int i = 0; i < sequence.Length; i++) - { - lastRequiredResource = Math.Max(lastRequiredResource, sequence[i]); + if (!isSorted) + { + // Playback order can reference resources arbitrarily. A sorted allocator-owned copy turns the physical + // resource scan into a linear merge instead of searching the complete sequence for every icon chunk. + sortedSequenceOwner = this.Options.Configuration.MemoryAllocator.Allocate(sequence.Length); + Span sortedSequence = sortedSequenceOwner.GetSpan(); + sequence.CopyTo(sortedSequence); + sortedSequence.Sort(); + requiredResources = sortedSequence; + } } - } - foreach ((long start, long end) in this.frameLists) - { - stream.Position = start; + int requiredResourceIndex = 0; + uint lastRequiredResource = hasSequence ? requiredResources[^1] : 0; - while (stream.Position + AniConstants.ChunkHeaderSize <= end) + foreach ((long start, long end) in this.frameLists) { - AniRiffChunkHeader chunk = this.ReadChunkHeader(stream); - long dataStart = stream.Position; - long dataEnd = GetChunkDataEnd(stream, chunk.Size, end); + stream.Position = start; - if ((AniFrameChunkType)chunk.FourCc is AniFrameChunkType.Icon) + while (stream.Position + AniConstants.ChunkHeaderSize <= end) { - int resourceIndex = resources.Count; + AniRiffChunkHeader chunk = this.ReadChunkHeader(stream); + long dataStart = stream.Position; + long dataEnd = GetChunkDataEnd(stream, chunk.Size, end); - // Sequence entries index the physical resource table, so ignored corrupt resources retain an empty slot. - resources.Add(null); - - // Unsequenced resources are consumed in physical order; sequenced files need only the referenced indices. - bool shouldDecode = !hasSequence || sequence.Contains((uint)resourceIndex); - if (shouldDecode) + if ((AniFrameChunkType)chunk.FourCc is AniFrameChunkType.Icon) { - this.ExecuteImageDataSegmentAction(() => + int resourceIndex = resources.Count; + + // Sequence entries index the physical resource table, so ignored corrupt resources retain an empty slot. + resources.Add(null); + + if (hasSequence) { - // Child decoders may seek according to embedded offsets; the bounded view prevents crossing the icon chunk. - frameStream.Reset(dataStart, chunk.Size); - AniFrameFormat format = this.GetFrameFormat(frameStream); + while (requiredResourceIndex < requiredResources.Length && requiredResources[requiredResourceIndex] < (uint)resourceIndex) + { + requiredResourceIndex++; + } + } - // Format probing consumes the directory prefix, while the selected child decoder requires the complete resource. - frameStream.Position = 0; - resources[resourceIndex] = (format, action(format, frameStream)); - }); + // Unsequenced resources are consumed in physical order; sequenced files need only the referenced indices. + bool shouldDecode = !hasSequence + || (requiredResourceIndex < requiredResources.Length && requiredResources[requiredResourceIndex] == (uint)resourceIndex); - if (resources[resourceIndex] is not null) + if (shouldDecode) { - decodedResourceCount++; + this.ExecuteImageDataSegmentAction(() => + { + // Child decoders may seek according to embedded offsets; the bounded view prevents crossing the icon chunk. + frameStream.Reset(dataStart, chunk.Size); + AniFrameFormat format = this.GetFrameFormat(frameStream); + + // Format probing consumes the directory prefix, while the selected child decoder requires the complete resource. + frameStream.Position = 0; + resources[resourceIndex] = (format, action(format, frameStream)); + }); + + if (resources[resourceIndex] is not null) + { + decodedResourceCount++; + } } - } - // Every decoded resource contributes at least one output frame, while a sequence cannot reference later indices. - if ((!hasSequence && decodedResourceCount == maxDecodedResources) - || (hasSequence && (uint)resourceIndex == lastRequiredResource)) - { - return; + // Every decoded resource contributes at least one output frame, while a sequence cannot reference later indices. + if ((!hasSequence && decodedResourceCount == maxDecodedResources) + || (hasSequence && (uint)resourceIndex == lastRequiredResource)) + { + return; + } } - } - stream.Position = GetPaddedEnd(dataEnd, chunk.Size, end); + stream.Position = GetPaddedEnd(dataEnd, chunk.Size, end); + } } } + finally + { + sortedSequenceOwner?.Dispose(); + } } /// diff --git a/src/ImageSharp/Formats/Ani/AniEncoderCore.cs b/src/ImageSharp/Formats/Ani/AniEncoderCore.cs index 2b42d8cb5..96b5fb575 100644 --- a/src/ImageSharp/Formats/Ani/AniEncoderCore.cs +++ b/src/ImageSharp/Formats/Ani/AniEncoderCore.cs @@ -54,11 +54,22 @@ internal sealed class AniEncoderCore AniFrameMetadata firstMetadata = image.Frames.RootFrame.Metadata.GetAniMetadata(); AniFrameFormat firstFormat = firstMetadata.FrameFormat; bool bitmapResources = firstFormat is AniFrameFormat.Bmp; + bool writeSequence = imageMetadata.Flags.HasFlag(AniHeaderFlags.ContainsSequence); uint displayRate = firstMetadata.FrameDelay is 0 ? imageMetadata.DisplayRate : firstMetadata.FrameDelay; bool hasVariableRates = false; int groupCount = 0; int maxGroupSize = 1; + if (bitmapResources && imageMetadata.BitCount is not (0 or 1 or 2 or 4 or 8 or 16 or 24 or 32)) + { + throw new ImageFormatException("ANI bitmap resources require a supported bit depth."); + } + + if (bitmapResources && imageMetadata.Planes is not (0 or 1)) + { + throw new ImageFormatException("ANI bitmap resources require exactly one color plane."); + } + // This validation pass derives the fixed ANI header and largest icon directory without allocating a grouping graph. // Encoding repeats the linear grouping scan below, trading a cheap pass for zero per-group collections. for (int frameIndex = 0; frameIndex < image.Frames.Count;) @@ -66,6 +77,12 @@ internal sealed class AniEncoderCore AniFrameMetadata metadata = image.Frames[frameIndex].Metadata.GetAniMetadata(); int groupSize = 1; + if (metadata.FrameFormat is not (AniFrameFormat.Ico or AniFrameFormat.Cur or AniFrameFormat.Bmp)) + { + // FrameFormat is public metadata and therefore must be validated before any container bytes are written. + throw new ImageFormatException("ANI contains an unsupported embedded frame format."); + } + // Positive sequence numbers group adjacent resolution variants; non-positive values form independent steps. if (metadata.SequenceNumber > 0) { @@ -118,9 +135,9 @@ internal sealed class AniEncoderCore Width = bitmapResources ? imageMetadata.Width is 0 ? (uint)image.Width : imageMetadata.Width : 0, Height = bitmapResources ? imageMetadata.Height is 0 ? (uint)image.Height : imageMetadata.Height : 0, BitCount = bitmapResources ? imageMetadata.BitCount is 0 ? 32U : imageMetadata.BitCount : 0, - Planes = bitmapResources ? imageMetadata.Planes is 0 ? 1U : imageMetadata.Planes : 0, + Planes = bitmapResources ? 1U : 0, DisplayRate = displayRate, - Flags = bitmapResources ? 0 : AniHeaderFlags.IsIcon + Flags = (bitmapResources ? 0 : AniHeaderFlags.IsIcon) | (writeSequence ? AniHeaderFlags.ContainsSequence : 0) }; // One allocator-owned directory buffer is sliced and reused for every icon resource; its capacity is the largest group. @@ -131,6 +148,11 @@ internal sealed class AniEncoderCore long riffSizePosition = this.BeginContainer(stream, AniConstants.RiffFourCc, AniConstants.AniFormTypeFourCc); this.WriteHeader(stream, header); + if (writeSequence) + { + this.WriteSequence(stream, groupCount); + } + if (hasVariableRates) { this.WriteRates(stream, image, displayRate); @@ -182,6 +204,27 @@ internal sealed class AniEncoderCore this.EndChunk(stream, sizePosition); } + /// + /// Writes an identity sequence table when the source metadata declares an explicit sequence. + /// + /// The destination stream. + /// The number of animation steps. + private void WriteSequence(Stream stream, int stepCount) + { + long sizePosition = this.BeginChunk(stream, "seq "u8); + Span value = this.buffer[..sizeof(uint)]; + + // Decoding expands source resource references into presentation order. Encoding writes those expanded steps as + // distinct resources, so an identity table preserves the explicit-sequence flag without changing playback. + for (uint i = 0; i < stepCount; i++) + { + BinaryPrimitives.WriteUInt32LittleEndian(value, i); + stream.Write(value); + } + + this.EndChunk(stream, sizePosition); + } + /// /// Writes per-step rates when they cannot be represented by one header value. /// @@ -291,6 +334,7 @@ internal sealed class AniEncoderCore { PixelSamplingStrategy = this.encoder.PixelSamplingStrategy, Quantizer = this.encoder.Quantizer, + SkipMetadata = this.encoder.SkipMetadata, TransparentColorMode = this.encoder.TransparentColorMode }); @@ -302,6 +346,7 @@ internal sealed class AniEncoderCore { PixelSamplingStrategy = this.encoder.PixelSamplingStrategy, Quantizer = this.encoder.Quantizer, + SkipMetadata = this.encoder.SkipMetadata, TransparentColorMode = this.encoder.TransparentColorMode }); @@ -318,6 +363,7 @@ internal sealed class AniEncoderCore PixelSamplingStrategy = this.encoder.PixelSamplingStrategy, Quantizer = this.encoder.Quantizer, SkipFileHeader = true, + SkipMetadata = this.encoder.SkipMetadata, SupportTransparency = bitCount is 32, TransparentColorMode = this.encoder.TransparentColorMode }; diff --git a/src/ImageSharp/Formats/Ani/AniMetadata.cs b/src/ImageSharp/Formats/Ani/AniMetadata.cs index c66dc1741..b72d7c1e3 100644 --- a/src/ImageSharp/Formats/Ani/AniMetadata.cs +++ b/src/ImageSharp/Formats/Ani/AniMetadata.cs @@ -60,11 +60,11 @@ public class AniMetadata : IFormatMetadata public uint BitCount { get; set; } /// - /// Gets or sets the color-plane count declared by the ANI header. + /// Gets or sets the number of independently addressable color planes declared by the ANI header. /// /// - /// Bitmap-based ANI files use one plane. Icon-based files commonly store zero because this header field is reserved - /// when the embedded resources are ICO or CUR data. + /// Bitmap-based ANI files use the Windows DIB plane value, which must be one. Icon-based ANI files use zero because + /// each embedded ICO or CUR entry describes its own pixel layout. No other values are defined by the format. /// public uint Planes { get; set; } diff --git a/src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs b/src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs index f543e4f3d..111f61fbf 100644 --- a/src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs +++ b/src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs @@ -102,6 +102,11 @@ internal sealed class BmpEncoderCore /// private readonly bool skipFileHeader; + /// + /// Whether optional image metadata should be omitted. + /// + private readonly bool skipMetadata; + /// private readonly bool isDoubleHeight; @@ -122,6 +127,7 @@ internal sealed class BmpEncoderCore this.infoHeaderType = encoder.SupportTransparency ? BmpInfoHeaderType.WinVersion4 : BmpInfoHeaderType.WinVersion3; this.processedAlphaMask = encoder.ProcessedAlphaMask; this.skipFileHeader = encoder.SkipFileHeader; + this.skipMetadata = encoder.SkipMetadata; this.isDoubleHeight = encoder.UseDoubleHeight; } @@ -174,7 +180,7 @@ internal sealed class BmpEncoderCore byte[]? iccProfileData = null; int iccProfileSize = 0; - if (metadata.IccProfile != null) + if (!this.skipMetadata && metadata.IccProfile != null) { this.infoHeaderType = BmpInfoHeaderType.WinVersion5; iccProfileData = metadata.IccProfile.ToByteArray(); @@ -229,7 +235,8 @@ internal sealed class BmpEncoderCore int hResolution = 0; int vResolution = 0; - if (metadata.ResolutionUnits != PixelResolutionUnit.AspectRatio + if (!this.skipMetadata + && metadata.ResolutionUnits != PixelResolutionUnit.AspectRatio && metadata.HorizontalResolution > 0 && metadata.VerticalResolution > 0) { diff --git a/src/ImageSharp/Formats/Icon/IconDecoderCore.cs b/src/ImageSharp/Formats/Icon/IconDecoderCore.cs index c1c6ee308..79d7e3786 100644 --- a/src/ImageSharp/Formats/Icon/IconDecoderCore.cs +++ b/src/ImageSharp/Formats/Icon/IconDecoderCore.cs @@ -78,7 +78,8 @@ internal abstract class IconDecoderCore : ImageDecoderCore throw new InvalidImageContentException("The icon file does not contain any decodable image entries."); } - ImageMetadata metadata = new(); + // General profiles belong to the icon result even though the first successfully decoded child image is temporary. + ImageMetadata metadata = decodedEntries[0].Image.Metadata.DeepClone(); BmpMetadata? bmpMetadata = null; PngMetadata? pngMetadata = null; ImageFrame[] frames = new ImageFrame[decodedCount]; @@ -93,7 +94,7 @@ internal abstract class IconDecoderCore : ImageDecoderCore Image decoded = decodedEntries[i].Image; ref IconDirEntry entry = ref this.entries[decodedEntries[i].EntryIndex]; ImageFrame source = decoded.Frames.RootFrameUnsafe; - ImageFrame target = new(this.Options.Configuration, this.Dimensions); + ImageFrame target = new(this.Options.Configuration, this.Dimensions, source.Metadata.DeepClone()); frames[i] = target; initializedFrameCount++; @@ -109,8 +110,6 @@ internal abstract class IconDecoderCore : ImageDecoderCore { pngMetadata = decoded.Metadata.GetPngMetadata(); } - - target.Metadata.SetFormatMetadata(PngFormat.Instance, source.Metadata.GetPngMetadata()); } else { @@ -194,7 +193,21 @@ internal abstract class IconDecoderCore : ImageDecoderCore bool isPng = flag.SequenceEqual(PngConstants.HeaderBytes); ImageInfo frameInfo = this.GetDecoder(isPng).Identify(this.Options.Configuration, frameStream, cancellationToken); - ImageFrameMetadata frameMetadata = new(); + ImageFrameMetadata frameMetadata = frameInfo.FrameMetadataCollection.Count is 0 ? new ImageFrameMetadata() : frameInfo.FrameMetadataCollection[0].DeepClone(); + + if (frameCount is 0) + { + // The container has one image-level metadata object, so the first valid entry supplies general profiles and resolution. + ImageMetadata sourceMetadata = frameInfo.Metadata; + metadata.HorizontalResolution = sourceMetadata.HorizontalResolution; + metadata.VerticalResolution = sourceMetadata.VerticalResolution; + metadata.ResolutionUnits = sourceMetadata.ResolutionUnits; + metadata.ExifProfile = sourceMetadata.ExifProfile?.DeepClone(); + metadata.IccProfile = sourceMetadata.IccProfile?.DeepClone(); + metadata.IptcProfile = sourceMetadata.IptcProfile?.DeepClone(); + metadata.XmpProfile = sourceMetadata.XmpProfile?.DeepClone(); + metadata.CicpProfile = sourceMetadata.CicpProfile?.DeepClone(); + } if (isPng) { @@ -202,8 +215,6 @@ internal abstract class IconDecoderCore : ImageDecoderCore { pngMetadata = frameInfo.Metadata.GetPngMetadata(); } - - frameMetadata.SetFormatMetadata(PngFormat.Instance, frameInfo.FrameMetadataCollection[0].GetPngMetadata()); } else { diff --git a/src/ImageSharp/Formats/Icon/IconEncoderCore.cs b/src/ImageSharp/Formats/Icon/IconEncoderCore.cs index 97bf96ea2..8d043d727 100644 --- a/src/ImageSharp/Formats/Icon/IconEncoderCore.cs +++ b/src/ImageSharp/Formats/Icon/IconEncoderCore.cs @@ -5,6 +5,7 @@ using System.Buffers; using SixLabors.ImageSharp.Formats.Bmp; using SixLabors.ImageSharp.Formats.Png; using SixLabors.ImageSharp.Memory; +using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing.Processors.Quantization; @@ -116,62 +117,90 @@ internal abstract class IconEncoderCore height = frame.Height; } - long imageStart = stream.Position; - entries[i].Entry.ImageOffset = checked((uint)(imageStart - basePosition)); - - // ANI flattens variants onto a common canvas, while an icon entry can encode a smaller rectangle. - // Child encoders consume Image, so an isolated cropped image is required to prevent encoding the padded canvas. - using Image encodingFrame = new(image.Configuration, width, height); - for (int y = 0; y < height; y++) + if (width > frame.Width || height > frame.Height) { - frame.PixelBuffer.DangerousGetRowSpan(y)[..width].CopyTo(encodingFrame.GetRootFramePixelBuffer().DangerousGetRowSpan(y)); + // EncodingWidth and EncodingHeight are public metadata, so reject a crop that exceeds the source frame here. + throw new ImageFormatException("The icon encoding dimensions exceed the source frame dimensions."); } + long imageStart = stream.Position; + entries[i].Entry.ImageOffset = checked((uint)(imageStart - basePosition)); ref EncodingFrameMetadata encodingMetadata = ref entries[i]; + Image? encodingImage = null; - // Compression and bitmap depth are per-entry, so the concrete encoder configuration must be selected per frame. - switch (encodingMetadata.Compression) + try { - case IconFrameCompression.Bmp: + bool requiresCrop = width != frame.Width || height != frame.Height; + bool requiresIsolatedImage = encodingMetadata.Compression is IconFrameCompression.Png && image.Frames.Count > 1; + + if (requiresCrop || requiresIsolatedImage) { - BmpEncoder bmpEncoder = new() + // PNG accepts Image rather than ImageFrame, and ANI variants may occupy only part of their common canvas. + // Allocate only for those cases; full-sized BMP frames can be encoded directly from their existing storage. + ImageMetadata? metadata = this.encoder.SkipMetadata || encodingMetadata.Compression is not IconFrameCompression.Png ? null : image.Metadata.DeepClone(); + encodingImage = new Image(image.Configuration, width, height, metadata); + + for (int y = 0; y < height; y++) { - Quantizer = this.GetQuantizer(encodingMetadata, colorTable), - ProcessedAlphaMask = true, - UseDoubleHeight = true, - SkipFileHeader = true, - SupportTransparency = false, - TransparentColorMode = this.encoder.TransparentColorMode, - PixelSamplingStrategy = this.encoder.PixelSamplingStrategy, - BitsPerPixel = encodingMetadata.BmpBitsPerPixel, - SkipMetadata = this.encoder.SkipMetadata - }; - - BmpEncoderCore bmpEncoderCore = new(bmpEncoder, image.Configuration.MemoryAllocator); - bmpEncoderCore.Encode(encodingFrame, stream, cancellationToken); - break; + frame.PixelBuffer.DangerousGetRowSpan(y)[..width].CopyTo(encodingImage.GetRootFramePixelBuffer().DangerousGetRowSpan(y)); + } + + if (!this.encoder.SkipMetadata && encodingMetadata.Compression is IconFrameCompression.Png) + { + encodingImage.Frames.RootFrame.Metadata.SetFormatMetadata(PngFormat.Instance, frame.Metadata.GetPngMetadata().DeepClone()); + } } - case IconFrameCompression.Png: + ImageFrame sourceFrame = encodingImage?.Frames.RootFrame ?? frame; + + // Compression and bitmap depth are per-entry, so the concrete encoder configuration must be selected per frame. + switch (encodingMetadata.Compression) { - PngEncoder pngEncoder = new() + case IconFrameCompression.Bmp: + { + BmpEncoder bmpEncoder = new() + { + Quantizer = this.GetQuantizer(encodingMetadata, colorTable), + ProcessedAlphaMask = true, + UseDoubleHeight = true, + SkipFileHeader = true, + SupportTransparency = false, + TransparentColorMode = this.encoder.TransparentColorMode, + PixelSamplingStrategy = this.encoder.PixelSamplingStrategy, + BitsPerPixel = encodingMetadata.BmpBitsPerPixel, + SkipMetadata = this.encoder.SkipMetadata + }; + + BmpEncoderCore bmpEncoderCore = new(bmpEncoder, image.Configuration.MemoryAllocator); + bmpEncoderCore.Encode(sourceFrame, image.Metadata, stream, cancellationToken); + break; + } + + case IconFrameCompression.Png: { - // Only 32bit Png supported. - // https://devblogs.microsoft.com/oldnewthing/20101022-00/?p=12473 - BitDepth = PngBitDepth.Bit8, - ColorType = PngColorType.RgbWithAlpha, - TransparentColorMode = this.encoder.TransparentColorMode, - CompressionLevel = PngCompressionLevel.BestCompression, - SkipMetadata = this.encoder.SkipMetadata - }; - - using PngEncoderCore pngEncoderCore = new(image.Configuration, pngEncoder); - pngEncoderCore.Encode(encodingFrame, stream, cancellationToken); - break; + PngEncoder pngEncoder = new() + { + // Only 32bit Png supported. + // https://devblogs.microsoft.com/oldnewthing/20101022-00/?p=12473 + BitDepth = PngBitDepth.Bit8, + ColorType = PngColorType.RgbWithAlpha, + TransparentColorMode = this.encoder.TransparentColorMode, + CompressionLevel = PngCompressionLevel.BestCompression, + SkipMetadata = this.encoder.SkipMetadata + }; + + using PngEncoderCore pngEncoderCore = new(image.Configuration, pngEncoder); + pngEncoderCore.Encode(encodingImage ?? image, stream, cancellationToken); + break; + } + + default: + throw new NotSupportedException(); } - - default: - throw new NotSupportedException(); + } + finally + { + encodingImage?.Dispose(); } encodingMetadata.Entry.BytesInRes = checked((uint)(stream.Position - imageStart)); diff --git a/tests/ImageSharp.Tests/Formats/Ani/AniDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Ani/AniDecoderTests.cs index bc5e4ebb8..e1ff17fec 100644 --- a/tests/ImageSharp.Tests/Formats/Ani/AniDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Ani/AniDecoderTests.cs @@ -9,7 +9,7 @@ using static SixLabors.ImageSharp.Tests.TestImages.Ani; namespace SixLabors.ImageSharp.Tests.Formats.Ani; -[Trait("format", "Ani")] +[Trait("Format", "Ani")] [ValidateDisposedMemoryAllocations] public class AniDecoderTests { diff --git a/tests/ImageSharp.Tests/Formats/Ani/AniEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Ani/AniEncoderTests.cs index 8f905a093..8b0246c50 100644 --- a/tests/ImageSharp.Tests/Formats/Ani/AniEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Ani/AniEncoderTests.cs @@ -3,8 +3,11 @@ using System.Buffers.Binary; using SixLabors.ImageSharp.Formats.Ani; +using SixLabors.ImageSharp.Formats.Bmp; using SixLabors.ImageSharp.Formats.Icon; +using SixLabors.ImageSharp.Metadata.Profiles.Icc; using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Tests.TestDataIcc; using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; using static SixLabors.ImageSharp.Tests.TestImages.Ani; @@ -133,6 +136,47 @@ public class AniEncoderTests Assert.Equal(frameFormat, decoded.Frames.RootFrame.Metadata.GetAniMetadata().FrameFormat); } + /// + /// Verifies that metadata suppression is propagated to every embedded resource encoder. + /// + [Theory] + [InlineData(AniFrameFormat.Ico)] + [InlineData(AniFrameFormat.Cur)] + [InlineData(AniFrameFormat.Bmp)] + public void AniEncoder_SkipMetadataPropagatesToEmbeddedEncoder(AniFrameFormat frameFormat) + { + using Image image = new(16, 16, Color.Red.ToPixel()); + image.Metadata.IccProfile = new IccProfile(IccTestDataProfiles.ProfileRandomArray); + + AniMetadata imageMetadata = image.Metadata.GetAniMetadata(); + imageMetadata.BitCount = 32; + imageMetadata.Planes = 1; + + AniFrameMetadata frameMetadata = image.Frames.RootFrame.Metadata.GetAniMetadata(); + frameMetadata.FrameFormat = frameFormat; + frameMetadata.Compression = IconFrameCompression.Bmp; + frameMetadata.BmpBitsPerPixel = BmpBitsPerPixel.Bit32; + + using MemoryStream stream = new(); + image.Save(stream, new AniEncoder { SkipMetadata = true }); + + ReadOnlySpan encoded = stream.GetBuffer().AsSpan(0, (int)stream.Length); + int frameChunkOffset = encoded.IndexOf("icon"u8); + Assert.True(frameChunkOffset >= 0); + + ReadOnlySpan resource = encoded[(frameChunkOffset + AniConstants.ChunkHeaderSize)..]; + int dibOffset = 0; + + if (frameFormat is not AniFrameFormat.Bmp) + { + dibOffset = checked((int)BinaryPrimitives.ReadUInt32LittleEndian(resource[(IconDir.Size + IconDirEntry.Size - sizeof(uint))..])); + } + + // Metadata-free ICO/CUR bitmaps use BITMAPINFOHEADER, while raw transparent ANI bitmaps require BITMAPV4HEADER. + int expectedHeaderSize = frameFormat is AniFrameFormat.Bmp ? BmpInfoHeader.SizeV4 : BmpInfoHeader.SizeV3; + Assert.Equal(expectedHeaderSize, BinaryPrimitives.ReadInt32LittleEndian(resource[dibOffset..])); + } + /// /// Verifies that an independent frame cannot collide with an explicit sequence group. /// @@ -153,4 +197,46 @@ public class AniEncoderTests Assert.Equal(1, decoded.Frames[0].Metadata.GetAniMetadata().SequenceNumber); Assert.Equal(2, decoded.Frames[1].Metadata.GetAniMetadata().SequenceNumber); } + + /// + /// Verifies that an explicit source sequence is preserved as an identity table after playback-order expansion. + /// + [Fact] + public void AniEncoder_PreservesExplicitSequence() + { + using Image image = new(16, 16, Color.Red.ToPixel()); + image.Frames.AddFrame(image.Frames.RootFrame); + image.Metadata.GetAniMetadata().Flags = AniHeaderFlags.IsIcon | AniHeaderFlags.ContainsSequence; + + using MemoryStream stream = new(); + image.Save(stream, new AniEncoder()); + + ReadOnlySpan data = stream.GetBuffer().AsSpan(0, (int)stream.Length); + int sequenceOffset = data.IndexOf("seq "u8); + + Assert.True(sequenceOffset >= 0); + Assert.Equal((uint)(2 * sizeof(uint)), BinaryPrimitives.ReadUInt32LittleEndian(data[(sequenceOffset + sizeof(uint))..])); + Assert.Equal(0U, BinaryPrimitives.ReadUInt32LittleEndian(data[(sequenceOffset + AniConstants.ChunkHeaderSize)..])); + Assert.Equal(1U, BinaryPrimitives.ReadUInt32LittleEndian(data[(sequenceOffset + AniConstants.ChunkHeaderSize + sizeof(uint))..])); + + stream.Position = 0; + using Image decoded = Image.Load(stream); + + Assert.True(decoded.Metadata.GetAniMetadata().Flags.HasFlag(AniHeaderFlags.ContainsSequence)); + } + + /// + /// Verifies that unsupported public frame metadata is rejected before any container data is written. + /// + [Fact] + public void AniEncoder_UnsupportedFrameFormatThrowsBeforeWriting() + { + using Image image = new(16, 16); + image.Frames.RootFrame.Metadata.GetAniMetadata().FrameFormat = (AniFrameFormat)byte.MaxValue; + + using MemoryStream stream = new(); + + Assert.Throws(() => image.Save(stream, new AniEncoder())); + Assert.Equal(0, stream.Length); + } } diff --git a/tests/ImageSharp.Tests/Formats/Icon/Ico/IcoEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Icon/Ico/IcoEncoderTests.cs index 4c7438d56..f637d64bf 100644 --- a/tests/ImageSharp.Tests/Formats/Icon/Ico/IcoEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Icon/Ico/IcoEncoderTests.cs @@ -4,6 +4,8 @@ using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Cur; using SixLabors.ImageSharp.Formats.Ico; +using SixLabors.ImageSharp.Formats.Icon; +using SixLabors.ImageSharp.Metadata.Profiles.Exif; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; using static SixLabors.ImageSharp.Tests.TestImages.Cur; @@ -121,4 +123,22 @@ public class IcoEncoderTests } }); } + + [Fact] + public void PngEntry_PreservesExifProfile() + { + using Image image = new(16, 16); + image.Metadata.ExifProfile = new ExifProfile(); + image.Metadata.ExifProfile.SetValue(ExifTag.Software, "ImageSharp"); + image.Frames.RootFrame.Metadata.GetIcoMetadata().Compression = IconFrameCompression.Png; + + using MemoryStream stream = new(); + image.Save(stream, Encoder); + + stream.Position = 0; + using Image decoded = Image.Load(stream); + + Assert.NotNull(decoded.Metadata.ExifProfile); + Assert.Equal(image.Metadata.ExifProfile.Values, decoded.Metadata.ExifProfile.Values); + } }