From 46f26ba95ef6e66b92fb666737df80168a2ce627 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 7 Nov 2022 17:39:37 +1000 Subject: [PATCH 01/10] Correctly read/write graphics control extension --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 32 ++++++++++------- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 24 +++++++++---- .../Formats/Gif/MetadataExtensions.cs | 22 +++++++++--- .../Sections/GifGraphicControlExtension.cs | 29 ++++++++++++++- src/ImageSharp/Metadata/ImageFrameMetadata.cs | 31 +++++++++++++++- .../Formats/Gif/GifEncoderTests.cs | 35 +++++++++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Gif/issues/issue_2288.gif | 3 ++ 8 files changed, 152 insertions(+), 25 deletions(-) create mode 100644 tests/Images/Input/Gif/issues/issue_2288.gif diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index ef29863d4f..e67c599e3f 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -125,6 +125,10 @@ public Image Decode(BufferedReadStream stream, CancellationToken } this.ReadFrame(ref image, ref previousFrame); + + // Reset per-frame state. + this.imageDescriptor = default; + this.graphicsControlExtension = default; } else if (nextFlag == GifConstants.ExtensionIntroducer) { @@ -464,7 +468,7 @@ private void ReadFrameColors(ref Image image, ref ImageFrame(this.configuration, imageWidth, imageHeight, this.metadata); } - this.SetFrameMetadata(image.Frames.RootFrame.Metadata); + this.SetFrameMetadata(image.Frames.RootFrame.Metadata, true); imageFrame = image.Frames.RootFrame; } @@ -477,7 +481,7 @@ private void ReadFrameColors(ref Image image, ref ImageFrame(ImageFrame frame) /// Sets the frames metadata. /// /// The metadata. + /// Whether the metadata represents the root frame. [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void SetFrameMetadata(ImageFrameMetadata meta) + private void SetFrameMetadata(ImageFrameMetadata meta, bool isRoot) { - GifFrameMetadata gifMeta = meta.GetGifMetadata(); - if (this.graphicsControlExtension.DelayTime > 0) - { - gifMeta.FrameDelay = this.graphicsControlExtension.DelayTime; - } - // Frames can either use the global table or their own local table. - if (this.logicalScreenDescriptor.GlobalColorTableFlag + if (isRoot && this.logicalScreenDescriptor.GlobalColorTableFlag && this.logicalScreenDescriptor.GlobalColorTableSize > 0) { + GifFrameMetadata gifMeta = meta.GetGifMetadata(); gifMeta.ColorTableLength = this.logicalScreenDescriptor.GlobalColorTableSize; } - else if (this.imageDescriptor.LocalColorTableFlag + + if (this.imageDescriptor.LocalColorTableFlag && this.imageDescriptor.LocalColorTableSize > 0) { + GifFrameMetadata gifMeta = meta.GetGifMetadata(); gifMeta.ColorTableLength = this.imageDescriptor.LocalColorTableSize; } - gifMeta.DisposalMethod = this.graphicsControlExtension.DisposalMethod; + // Graphics control extensions is optional. + if (this.graphicsControlExtension != default) + { + GifFrameMetadata gifMeta = meta.GetGifMetadata(); + gifMeta.FrameDelay = this.graphicsControlExtension.DelayTime; + gifMeta.DisposalMethod = this.graphicsControlExtension.DisposalMethod; + } } /// diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index 14d20cf909..716cb9da3f 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -160,8 +160,12 @@ private void EncodeGlobal(Image image, IndexedImageFrame { ImageFrame frame = image.Frames[i]; ImageFrameMetadata metadata = frame.Metadata; - GifFrameMetadata frameMetadata = metadata.GetGifMetadata(); - this.WriteGraphicalControlExtension(frameMetadata, transparencyIndex, stream); + + if (metadata.TryGetGifMetadata(out GifFrameMetadata frameMetadata)) + { + this.WriteGraphicalControlExtension(frameMetadata, transparencyIndex, stream); + } + this.WriteImageDescriptor(frame, false, stream); if (i == 0) @@ -193,12 +197,13 @@ private void EncodeLocal(Image image, IndexedImageFrame { ImageFrame frame = image.Frames[i]; ImageFrameMetadata metadata = frame.Metadata; - GifFrameMetadata frameMetadata = metadata.GetGifMetadata(); + bool hasMetadata = metadata.TryGetGifMetadata(out GifFrameMetadata frameMetadata); if (quantized is null) { // Allow each frame to be encoded at whatever color depth the frame designates if set. - if (previousFrame != null && previousMeta.ColorTableLength != frameMetadata.ColorTableLength - && frameMetadata.ColorTableLength > 0) + if (previousFrame != null && frameMetadata != null + && previousMeta.ColorTableLength != frameMetadata.ColorTableLength + && frameMetadata.ColorTableLength > 0) { QuantizerOptions options = new() { @@ -218,7 +223,12 @@ private void EncodeLocal(Image image, IndexedImageFrame } this.bitDepth = ColorNumerics.GetBitsNeededForColorDepth(quantized.Palette.Length); - this.WriteGraphicalControlExtension(frameMetadata, GetTransparentIndex(quantized), stream); + + if (hasMetadata) + { + this.WriteGraphicalControlExtension(frameMetadata, GetTransparentIndex(quantized), stream); + } + this.WriteImageDescriptor(frame, true, stream); this.WriteColorTable(quantized, stream); this.WriteImageData(quantized, stream); @@ -407,7 +417,7 @@ private static void WriteCommentSubBlock(Stream stream, ReadOnlySpan comme } /// - /// Writes the graphics control extension to the stream. + /// Writes the optional graphics control extension to the stream. /// /// The metadata of the image or frame. /// The index of the color in the color palette to make transparent. diff --git a/src/ImageSharp/Formats/Gif/MetadataExtensions.cs b/src/ImageSharp/Formats/Gif/MetadataExtensions.cs index 2c1a3cf0d7..7280024e21 100644 --- a/src/ImageSharp/Formats/Gif/MetadataExtensions.cs +++ b/src/ImageSharp/Formats/Gif/MetadataExtensions.cs @@ -14,14 +14,28 @@ public static partial class MetadataExtensions /// /// Gets the gif format specific metadata for the image. /// - /// The metadata this method extends. + /// The metadata this method extends. /// The . - public static GifMetadata GetGifMetadata(this ImageMetadata metadata) => metadata.GetFormatMetadata(GifFormat.Instance); + public static GifMetadata GetGifMetadata(this ImageMetadata source) => source.GetFormatMetadata(GifFormat.Instance); /// /// Gets the gif format specific metadata for the image frame. /// - /// The metadata this method extends. + /// The metadata this method extends. /// The . - public static GifFrameMetadata GetGifMetadata(this ImageFrameMetadata metadata) => metadata.GetFormatMetadata(GifFormat.Instance); + public static GifFrameMetadata GetGifMetadata(this ImageFrameMetadata source) => source.GetFormatMetadata(GifFormat.Instance); + + /// + /// Gets the gif format specific metadata for the image frame. + /// + /// The metadata this method extends. + /// + /// When this method returns, contains the metadata associated with the specified frame, + /// if found; otherwise, the default value for the type of the metadata parameter. + /// This parameter is passed uninitialized. + /// + /// + /// if the gif frame metadata exists; otherwise, . + /// + public static bool TryGetGifMetadata(this ImageFrameMetadata source, out GifFrameMetadata metadata) => source.TryGetFormatMetadata(GifFormat.Instance, out metadata); } diff --git a/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs index db2fbd79c3..ba5398fa7d 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs @@ -11,7 +11,7 @@ namespace SixLabors.ImageSharp.Formats.Gif; /// processing a graphic rendering block. /// [StructLayout(LayoutKind.Sequential, Pack = 1)] -internal readonly struct GifGraphicControlExtension : IGifExtension +internal readonly struct GifGraphicControlExtension : IGifExtension, IEquatable { public GifGraphicControlExtension( byte packed, @@ -64,6 +64,10 @@ public GifGraphicControlExtension( int IGifExtension.ContentLength => 5; + public static bool operator ==(GifGraphicControlExtension left, GifGraphicControlExtension right) => left.Equals(right); + + public static bool operator !=(GifGraphicControlExtension left, GifGraphicControlExtension right) => !(left == right); + public int WriteTo(Span buffer) { ref GifGraphicControlExtension dest = ref Unsafe.As(ref MemoryMarshal.GetReference(buffer)); @@ -101,4 +105,27 @@ Transparent Color Flag | 1 Bit return value; } + + public override bool Equals(object obj) => obj is GifGraphicControlExtension extension && this.Equals(extension); + + public bool Equals(GifGraphicControlExtension other) + => this.BlockSize == other.BlockSize + && this.Packed == other.Packed + && this.DelayTime == other.DelayTime + && this.TransparencyIndex == other.TransparencyIndex + && this.DisposalMethod == other.DisposalMethod + && this.TransparencyFlag == other.TransparencyFlag + && ((IGifExtension)this).Label == ((IGifExtension)other).Label + && ((IGifExtension)this).ContentLength == ((IGifExtension)other).ContentLength; + + public override int GetHashCode() + => HashCode.Combine( + this.BlockSize, + this.Packed, + this.DelayTime, + this.TransparencyIndex, + this.DisposalMethod, + this.TransparencyFlag, + ((IGifExtension)this).Label, + ((IGifExtension)this).ContentLength); } diff --git a/src/ImageSharp/Metadata/ImageFrameMetadata.cs b/src/ImageSharp/Metadata/ImageFrameMetadata.cs index 69ff5201f2..bf65fd6dc1 100644 --- a/src/ImageSharp/Metadata/ImageFrameMetadata.cs +++ b/src/ImageSharp/Metadata/ImageFrameMetadata.cs @@ -69,7 +69,8 @@ internal ImageFrameMetadata(ImageFrameMetadata other) public ImageFrameMetadata DeepClone() => new(this); /// - /// Gets the metadata value associated with the specified key. + /// Gets the metadata value associated with the specified key. This method will always return a result creating + /// a new instance and binding it to the frame metadata if none is found. /// /// The type of format metadata. /// The type of format frame metadata. @@ -90,4 +91,32 @@ public TFormatFrameMetadata GetFormatMetadata + /// Gets the metadata value associated with the specified key. + /// + /// The type of format metadata. + /// The type of format frame metadata. + /// The key of the value to get. + /// + /// When this method returns, contains the metadata associated with the specified key, + /// if the key is found; otherwise, the default value for the type of the metadata parameter. + /// This parameter is passed uninitialized. + /// + /// + /// if the frame metadata exists for the specified key; otherwise, . + /// + public bool TryGetFormatMetadata(IImageFormat key, out TFormatFrameMetadata metadata) + where TFormatMetadata : class + where TFormatFrameMetadata : class, IDeepCloneable + { + if (this.formatMetadata.TryGetValue(key, out IDeepCloneable meta)) + { + metadata = (TFormatFrameMetadata)meta; + return true; + } + + metadata = default; + return false; + } } diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs index 18eb7708ca..7c71fd5469 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs @@ -201,4 +201,39 @@ public void NonMutatingEncodePreservesPaletteCount() image.Dispose(); clone.Dispose(); } + + [Theory] + [WithFile(TestImages.Gif.Issues.Issue2288OptionalExtension, PixelTypes.Rgba32)] + public void OptionalExtensionsShouldBeHandledProperly(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(); + + int count = 0; + foreach (ImageFrame frame in image.Frames) + { + if (frame.Metadata.TryGetGifMetadata(out GifFrameMetadata _)) + { + count++; + } + } + + provider.Utility.SaveTestOutputFile(image, extension: "gif"); + + using FileStream fs = File.OpenRead(provider.Utility.GetTestOutputFileName("gif")); + using Image image2 = Image.Load(fs); + + Assert.Equal(image.Frames.Count, image2.Frames.Count); + + count = 0; + foreach (ImageFrame frame in image2.Frames) + { + if (frame.Metadata.TryGetGifMetadata(out GifFrameMetadata _)) + { + count++; + } + } + + Assert.Equal(image2.Frames.Count, count); + } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index d47f1136d5..ea8fba998f 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -476,6 +476,7 @@ public static class Issues public const string Issue1962NoColorTable = "Gif/issues/issue1962_tiniest_gif_1st.gif"; public const string Issue2012EmptyXmp = "Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif"; public const string Issue2012BadMinCode = "Gif/issues/issue2012_drona1.gif"; + public const string Issue2288OptionalExtension = "Gif/issues/issue_2288.gif"; } public static readonly string[] All = { Rings, Giphy, Cheers, Trans, Kumin, Leo, Ratio4x1, Ratio1x4 }; diff --git a/tests/Images/Input/Gif/issues/issue_2288.gif b/tests/Images/Input/Gif/issues/issue_2288.gif new file mode 100644 index 0000000000..f798e7b8e6 --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue_2288.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d38bc98de425322bc0d435b7ff538c170897bfc728ea77ee26dd172106dcf99a +size 1223216 From 44906d50e32c89bb0c894348021fa5f5886571a9 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 7 Nov 2022 20:27:39 +1000 Subject: [PATCH 02/10] Ensure transparency is written --- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index 716cb9da3f..6ba0493384 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -161,8 +161,10 @@ private void EncodeGlobal(Image image, IndexedImageFrame ImageFrame frame = image.Frames[i]; ImageFrameMetadata metadata = frame.Metadata; - if (metadata.TryGetGifMetadata(out GifFrameMetadata frameMetadata)) + bool hasMeta = metadata.TryGetGifMetadata(out GifFrameMetadata frameMetadata); + if (hasMeta || transparencyIndex > -1) { + frameMetadata ??= metadata.GetGifMetadata(); this.WriteGraphicalControlExtension(frameMetadata, transparencyIndex, stream); } @@ -223,10 +225,11 @@ private void EncodeLocal(Image image, IndexedImageFrame } this.bitDepth = ColorNumerics.GetBitsNeededForColorDepth(quantized.Palette.Length); - - if (hasMetadata) + int index = GetTransparentIndex(quantized); + if (hasMetadata || index > -1) { - this.WriteGraphicalControlExtension(frameMetadata, GetTransparentIndex(quantized), stream); + frameMetadata ??= metadata.GetGifMetadata(); + this.WriteGraphicalControlExtension(frameMetadata, index, stream); } this.WriteImageDescriptor(frame, true, stream); From abd3a5a8ba40a489428903e3988f7c4fd69a2fef Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 8 Nov 2022 16:08:14 +1000 Subject: [PATCH 03/10] Fix transparency handling and optimize encoding --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 10 +- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 135 ++++++++---------- .../Formats/Gif/GifFrameMetadata.cs | 8 +- src/ImageSharp/Formats/Gif/GifMetadata.cs | 2 +- .../Quantization/OctreeQuantizer{TPixel}.cs | 2 +- .../Quantization/PaletteQuantizer.cs | 8 +- .../Formats/Gif/GifEncoderTests.cs | 1 + tests/ImageSharp.Tests/TestImages.cs | 1 + .../Images/Input/Gif/issues/issue_2288_2.gif | 3 + 9 files changed, 83 insertions(+), 87 deletions(-) create mode 100644 tests/Images/Input/Gif/issues/issue_2288_2.gif diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index e67c599e3f..6bec8dc2e8 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -479,7 +479,7 @@ private void ReadFrameColors(ref Image image, ref ImageFrame(this.configuration, previousFrame.Width, previousFrame.Height)); this.SetFrameMetadata(currentFrame.Metadata, false); @@ -548,7 +548,7 @@ private void ReadFrameColors(ref Image image, ref ImageFrame(ref Image image, ref ImageFrame(ImageFrame frame) return; } - var interest = Rectangle.Intersect(frame.Bounds(), this.restoreArea.Value); + Rectangle interest = Rectangle.Intersect(frame.Bounds(), this.restoreArea.Value); Buffer2DRegion pixelRegion = frame.PixelBuffer.GetRegion(interest); pixelRegion.Clear(); @@ -616,6 +616,7 @@ private void SetFrameMetadata(ImageFrameMetadata meta, bool isRoot) && this.logicalScreenDescriptor.GlobalColorTableSize > 0) { GifFrameMetadata gifMeta = meta.GetGifMetadata(); + gifMeta.ColorTableMode = GifColorTableMode.Global; gifMeta.ColorTableLength = this.logicalScreenDescriptor.GlobalColorTableSize; } @@ -623,6 +624,7 @@ private void SetFrameMetadata(ImageFrameMetadata meta, bool isRoot) && this.imageDescriptor.LocalColorTableSize > 0) { GifFrameMetadata gifMeta = meta.GetGifMetadata(); + gifMeta.ColorTableMode = GifColorTableMode.Local; gifMeta.ColorTableLength = this.imageDescriptor.LocalColorTableSize; } diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index 6ba0493384..135dc13291 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -93,7 +93,6 @@ public void Encode(Image image, Stream stream, CancellationToken // Quantize the image returning a palette. IndexedImageFrame quantized; - using (IQuantizer frameQuantizer = this.quantizer.CreatePixelSpecificQuantizer(this.configuration)) { if (useGlobalTable) @@ -133,114 +132,102 @@ public void Encode(Image image, Stream stream, CancellationToken this.WriteApplicationExtensions(stream, image.Frames.Count, gifMetadata.RepeatCount, xmpProfile); } - if (useGlobalTable) - { - this.EncodeGlobal(image, quantized, index, stream); - } - else - { - this.EncodeLocal(image, quantized, stream); - } - - // Clean up. - quantized.Dispose(); + this.EncodeFrames(stream, image, quantized, quantized.Palette.ToArray()); stream.WriteByte(GifConstants.EndIntroducer); } - private void EncodeGlobal(Image image, IndexedImageFrame quantized, int transparencyIndex, Stream stream) + private void EncodeFrames( + Stream stream, + Image image, + IndexedImageFrame quantized, + ReadOnlyMemory palette) where TPixel : unmanaged, IPixel { - // The palette quantizer can reuse the same pixel map across multiple frames - // since the palette is unchanging. This allows a reduction of memory usage across - // multi frame gifs using a global palette. - PaletteQuantizer paletteFrameQuantizer = default; - bool quantizerInitialized = false; + PaletteQuantizer paletteQuantizer = default; + bool hasPaletteQuantizer = false; for (int i = 0; i < image.Frames.Count; i++) { + // Gather the metadata for this frame. ImageFrame frame = image.Frames[i]; ImageFrameMetadata metadata = frame.Metadata; + bool hasMetadata = metadata.TryGetGifMetadata(out GifFrameMetadata frameMetadata); + bool useLocal = this.colorTableMode == GifColorTableMode.Local || (hasMetadata && frameMetadata.ColorTableMode == GifColorTableMode.Local); - bool hasMeta = metadata.TryGetGifMetadata(out GifFrameMetadata frameMetadata); - if (hasMeta || transparencyIndex > -1) + if (!useLocal && !hasPaletteQuantizer && i > 0) { - frameMetadata ??= metadata.GetGifMetadata(); - this.WriteGraphicalControlExtension(frameMetadata, transparencyIndex, stream); + // The palette quantizer can reuse the same pixel map across multiple frames + // since the palette is unchanging. This allows a reduction of memory usage across + // multi frame gifs using a global palette. + hasPaletteQuantizer = true; + paletteQuantizer = new(this.configuration, this.quantizer.Options, palette); } - this.WriteImageDescriptor(frame, false, stream); + this.EncodeFrame(stream, frame, i, useLocal, frameMetadata, ref quantized, ref paletteQuantizer); - if (i == 0) - { - this.WriteImageData(quantized, stream); - } - else - { - if (!quantizerInitialized) - { - quantizerInitialized = true; - paletteFrameQuantizer = new PaletteQuantizer(this.configuration, this.quantizer.Options, quantized.Palette); - } - - using IndexedImageFrame paletteQuantized = paletteFrameQuantizer.QuantizeFrame(frame, frame.Bounds()); - this.WriteImageData(paletteQuantized, stream); - } + // Clean up for the next run. + quantized.Dispose(); + quantized = null; } - paletteFrameQuantizer.Dispose(); + paletteQuantizer.Dispose(); } - private void EncodeLocal(Image image, IndexedImageFrame quantized, Stream stream) + private void EncodeFrame( + Stream stream, + ImageFrame frame, + int frameIndex, + bool useLocal, + GifFrameMetadata metadata, + ref IndexedImageFrame quantized, + ref PaletteQuantizer paletteQuantizer) where TPixel : unmanaged, IPixel { - ImageFrame previousFrame = null; - GifFrameMetadata previousMeta = null; - for (int i = 0; i < image.Frames.Count; i++) + // The first frame has already been quantized so we do not need to do so again. + if (frameIndex > 0) { - ImageFrame frame = image.Frames[i]; - ImageFrameMetadata metadata = frame.Metadata; - bool hasMetadata = metadata.TryGetGifMetadata(out GifFrameMetadata frameMetadata); - if (quantized is null) + if (useLocal) { - // Allow each frame to be encoded at whatever color depth the frame designates if set. - if (previousFrame != null && frameMetadata != null - && previousMeta.ColorTableLength != frameMetadata.ColorTableLength - && frameMetadata.ColorTableLength > 0) + // Reassign using the current frame and details. + QuantizerOptions options = null; + int colorTableLength = metadata?.ColorTableLength ?? 0; + if (colorTableLength > 0) { - QuantizerOptions options = new() + options = new() { Dither = this.quantizer.Options.Dither, DitherScale = this.quantizer.Options.DitherScale, - MaxColors = frameMetadata.ColorTableLength + MaxColors = colorTableLength }; - - using IQuantizer frameQuantizer = this.quantizer.CreatePixelSpecificQuantizer(this.configuration, options); - quantized = frameQuantizer.BuildPaletteAndQuantizeFrame(frame, frame.Bounds()); - } - else - { - using IQuantizer frameQuantizer = this.quantizer.CreatePixelSpecificQuantizer(this.configuration); - quantized = frameQuantizer.BuildPaletteAndQuantizeFrame(frame, frame.Bounds()); } - } - this.bitDepth = ColorNumerics.GetBitsNeededForColorDepth(quantized.Palette.Length); - int index = GetTransparentIndex(quantized); - if (hasMetadata || index > -1) + using IQuantizer frameQuantizer = this.quantizer.CreatePixelSpecificQuantizer(this.configuration, options ?? this.quantizer.Options); + quantized = frameQuantizer.BuildPaletteAndQuantizeFrame(frame, frame.Bounds()); + } + else { - frameMetadata ??= metadata.GetGifMetadata(); - this.WriteGraphicalControlExtension(frameMetadata, index, stream); + // Quantize the image using the global palette. + quantized = paletteQuantizer.QuantizeFrame(frame, frame.Bounds()); } - this.WriteImageDescriptor(frame, true, stream); - this.WriteColorTable(quantized, stream); - this.WriteImageData(quantized, stream); + this.bitDepth = ColorNumerics.GetBitsNeededForColorDepth(quantized.Palette.Length); + } - quantized.Dispose(); - quantized = null; // So next frame can regenerate it - previousFrame = frame; - previousMeta = frameMetadata; + // Do we have extension information to write? + int index = GetTransparentIndex(quantized); + if (metadata != null || index > -1) + { + this.WriteGraphicalControlExtension(metadata ?? new(), index, stream); + } + + this.WriteImageDescriptor(frame, useLocal, stream); + + if (useLocal) + { + this.WriteColorTable(quantized, stream); } + + this.WriteImageData(quantized, stream); } /// diff --git a/src/ImageSharp/Formats/Gif/GifFrameMetadata.cs b/src/ImageSharp/Formats/Gif/GifFrameMetadata.cs index 82694eab9a..7f4b49f0bb 100644 --- a/src/ImageSharp/Formats/Gif/GifFrameMetadata.cs +++ b/src/ImageSharp/Formats/Gif/GifFrameMetadata.cs @@ -21,13 +21,19 @@ public GifFrameMetadata() /// The metadata to create an instance from. private GifFrameMetadata(GifFrameMetadata other) { + this.ColorTableMode = other.ColorTableMode; this.ColorTableLength = other.ColorTableLength; this.FrameDelay = other.FrameDelay; this.DisposalMethod = other.DisposalMethod; } /// - /// Gets or sets the length of the color table for paletted images. + /// Gets or sets the color table mode. + /// + public GifColorTableMode ColorTableMode { get; set; } + + /// + /// Gets or sets the length of the color table. /// If not 0, then this field indicates the maximum number of colors to use when quantizing the /// image frame. /// diff --git a/src/ImageSharp/Formats/Gif/GifMetadata.cs b/src/ImageSharp/Formats/Gif/GifMetadata.cs index 52019c3354..da21e134ec 100644 --- a/src/ImageSharp/Formats/Gif/GifMetadata.cs +++ b/src/ImageSharp/Formats/Gif/GifMetadata.cs @@ -50,7 +50,7 @@ private GifMetadata(GifMetadata other) public int GlobalColorTableLength { get; set; } /// - /// Gets or sets the the collection of comments about the graphics, credits, descriptions or any + /// Gets or sets the collection of comments about the graphics, credits, descriptions or any /// other type of non-control and non-graphic data. /// public IList Comments { get; set; } = new List(); diff --git a/src/ImageSharp/Processing/Processors/Quantization/OctreeQuantizer{TPixel}.cs b/src/ImageSharp/Processing/Processors/Quantization/OctreeQuantizer{TPixel}.cs index bbe8e4113f..71b74242f7 100644 --- a/src/ImageSharp/Processing/Processors/Quantization/OctreeQuantizer{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Quantization/OctreeQuantizer{TPixel}.cs @@ -106,7 +106,7 @@ public void AddPaletteColors(Buffer2DRegion pixelRegion) // for higher bit depths. Lower bit depths will correctly reduce the palette. // TODO: Investigate more evenly reduced palette reduction. int max = this.maxColors; - if (this.bitDepth == 8) + if (this.bitDepth >= 4) { max--; } diff --git a/src/ImageSharp/Processing/Processors/Quantization/PaletteQuantizer.cs b/src/ImageSharp/Processing/Processors/Quantization/PaletteQuantizer.cs index f40cde487f..f3ed39c957 100644 --- a/src/ImageSharp/Processing/Processors/Quantization/PaletteQuantizer.cs +++ b/src/ImageSharp/Processing/Processors/Quantization/PaletteQuantizer.cs @@ -49,12 +49,8 @@ public IQuantizer CreatePixelSpecificQuantizer(Configuration con { Guard.NotNull(options, nameof(options)); - // The palette quantizer can reuse the same pixel map across multiple frames - // since the palette is unchanging. This allows a reduction of memory usage across - // multi frame gifs using a global palette. - int length = Math.Min(this.colorPalette.Length, options.MaxColors); - TPixel[] palette = new TPixel[length]; - + // Always use the palette length over options since the palette cannot be reduced. + TPixel[] palette = new TPixel[this.colorPalette.Length]; Color.ToPixel(configuration, this.colorPalette.Span, palette.AsSpan()); return new PaletteQuantizer(configuration, options, palette); } diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs index 7c71fd5469..15fd528e53 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs @@ -204,6 +204,7 @@ public void NonMutatingEncodePreservesPaletteCount() [Theory] [WithFile(TestImages.Gif.Issues.Issue2288OptionalExtension, PixelTypes.Rgba32)] + [WithFile(TestImages.Gif.Issues.Issue2288OptionalExtension2, PixelTypes.Rgba32)] public void OptionalExtensionsShouldBeHandledProperly(TestImageProvider provider) where TPixel : unmanaged, IPixel { diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index ea8fba998f..7887c09008 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -477,6 +477,7 @@ public static class Issues public const string Issue2012EmptyXmp = "Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif"; public const string Issue2012BadMinCode = "Gif/issues/issue2012_drona1.gif"; public const string Issue2288OptionalExtension = "Gif/issues/issue_2288.gif"; + public const string Issue2288OptionalExtension2 = "Gif/issues/issue_2288_2.gif"; } public static readonly string[] All = { Rings, Giphy, Cheers, Trans, Kumin, Leo, Ratio4x1, Ratio1x4 }; diff --git a/tests/Images/Input/Gif/issues/issue_2288_2.gif b/tests/Images/Input/Gif/issues/issue_2288_2.gif new file mode 100644 index 0000000000..790ef4bf69 --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue_2288_2.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:8919e83c8a19502b3217c75e0a7c98be46732c2126816f8882e9bed19478ded7 +size 811449 From 0dcc73a45a0ae057cb833b70c7f9c720d3201744 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 8 Nov 2022 16:25:36 +1000 Subject: [PATCH 04/10] Fix leak --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 6bec8dc2e8..a16e047838 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -479,7 +479,7 @@ private void ReadFrameColors(ref Image image, ref ImageFrame(this.configuration, previousFrame.Width, previousFrame.Height)); + currentFrame = image.Frames.CreateFrame(); this.SetFrameMetadata(currentFrame.Metadata, false); From 55ae240f87be6a60dace612bbcb60a986f0d65fa Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 8 Nov 2022 18:35:33 +1000 Subject: [PATCH 05/10] Fix gif decoder tests --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index a16e047838..1b21c37f42 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -548,7 +548,7 @@ private void ReadFrameColors(ref Image image, ref ImageFrame Date: Tue, 8 Nov 2022 18:43:53 +1000 Subject: [PATCH 06/10] Fix report --- .../Exceptions/ImageDifferenceIsOverThresholdException.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/TestUtilities/ImageComparison/Exceptions/ImageDifferenceIsOverThresholdException.cs b/tests/ImageSharp.Tests/TestUtilities/ImageComparison/Exceptions/ImageDifferenceIsOverThresholdException.cs index d6cb24fd62..b2fa7271aa 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ImageComparison/Exceptions/ImageDifferenceIsOverThresholdException.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ImageComparison/Exceptions/ImageDifferenceIsOverThresholdException.cs @@ -36,7 +36,7 @@ private static string StringifyReports(IEnumerable report int i = 0; foreach (ImageSimilarityReport r in reports) { - sb.Append("Report ImageFrame {i}: "); + sb.Append($"Report ImageFrame {i}: "); sb.Append(r); sb.Append(Environment.NewLine); i++; From dc58f2bc8a0f7966a1bc70d9bbc0b7e7b82bbb90 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 8 Nov 2022 20:12:33 +1000 Subject: [PATCH 07/10] Better fix for report --- .../TestUtilities/ImageComparison/ExactImageComparer.cs | 3 ++- .../ImageDifferenceIsOverThresholdException.cs | 4 +--- .../TestUtilities/ImageComparison/ImageComparer.cs | 7 ++++--- .../ImageComparison/ImageSimilarityReport.cs | 9 +++++++-- .../ImageComparison/TolerantImageComparer.cs | 4 ++-- 5 files changed, 16 insertions(+), 11 deletions(-) diff --git a/tests/ImageSharp.Tests/TestUtilities/ImageComparison/ExactImageComparer.cs b/tests/ImageSharp.Tests/TestUtilities/ImageComparison/ExactImageComparer.cs index 87725aec93..52f160dedc 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ImageComparison/ExactImageComparer.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ImageComparison/ExactImageComparer.cs @@ -12,6 +12,7 @@ public class ExactImageComparer : ImageComparer public static ExactImageComparer Instance { get; } = new ExactImageComparer(); public override ImageSimilarityReport CompareImagesOrFrames( + int index, ImageFrame expected, ImageFrame actual) { @@ -52,6 +53,6 @@ public override ImageSimilarityReport CompareImagesOrFrames(expected, actual, differences); + return new ImageSimilarityReport(index, expected, actual, differences); } } diff --git a/tests/ImageSharp.Tests/TestUtilities/ImageComparison/Exceptions/ImageDifferenceIsOverThresholdException.cs b/tests/ImageSharp.Tests/TestUtilities/ImageComparison/Exceptions/ImageDifferenceIsOverThresholdException.cs index b2fa7271aa..33b7228aed 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ImageComparison/Exceptions/ImageDifferenceIsOverThresholdException.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ImageComparison/Exceptions/ImageDifferenceIsOverThresholdException.cs @@ -33,13 +33,11 @@ private static string StringifyReports(IEnumerable report sb.AppendFormat("Test Environment is Mono : {0}", TestEnvironment.IsMono); sb.Append(Environment.NewLine); - int i = 0; foreach (ImageSimilarityReport r in reports) { - sb.Append($"Report ImageFrame {i}: "); + sb.AppendFormat("Report ImageFrame {0}: ", r.Index); sb.Append(r); sb.Append(Environment.NewLine); - i++; } return sb.ToString(); diff --git a/tests/ImageSharp.Tests/TestUtilities/ImageComparison/ImageComparer.cs b/tests/ImageSharp.Tests/TestUtilities/ImageComparison/ImageComparer.cs index 9a022a8734..3de525964c 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ImageComparison/ImageComparer.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ImageComparison/ImageComparer.cs @@ -27,6 +27,7 @@ public static ImageComparer TolerantPercentage(float imageThresholdInPercents, i => Tolerant(imageThresholdInPercents / 100F, perPixelManhattanThreshold); public abstract ImageSimilarityReport CompareImagesOrFrames( + int index, ImageFrame expected, ImageFrame actual) where TPixelA : unmanaged, IPixel @@ -40,7 +41,7 @@ public static ImageSimilarityReport CompareImagesOrFrames expected, Image actual) where TPixelA : unmanaged, IPixel - where TPixelB : unmanaged, IPixel => comparer.CompareImagesOrFrames(expected.Frames.RootFrame, actual.Frames.RootFrame); + where TPixelB : unmanaged, IPixel => comparer.CompareImagesOrFrames(0, expected.Frames.RootFrame, actual.Frames.RootFrame); public static IEnumerable> CompareImages( this ImageComparer comparer, @@ -58,7 +59,7 @@ public static IEnumerable> CompareImages for (int i = 0; i < expected.Frames.Count; i++) { - ImageSimilarityReport report = comparer.CompareImagesOrFrames(expected.Frames[i], actual.Frames[i]); + ImageSimilarityReport report = comparer.CompareImagesOrFrames(i, expected.Frames[i], actual.Frames[i]); if (!report.IsEmpty) { result.Add(report); @@ -125,7 +126,7 @@ public static void VerifySimilarityIgnoreRegion( if (outsideChanges.Any()) { - cleanedReports.Add(new ImageSimilarityReport(r.ExpectedImage, r.ActualImage, outsideChanges, null)); + cleanedReports.Add(new ImageSimilarityReport(r.Index, r.ExpectedImage, r.ActualImage, outsideChanges, null)); } } diff --git a/tests/ImageSharp.Tests/TestUtilities/ImageComparison/ImageSimilarityReport.cs b/tests/ImageSharp.Tests/TestUtilities/ImageComparison/ImageSimilarityReport.cs index 8f441cc573..c50ae5e219 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ImageComparison/ImageSimilarityReport.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ImageComparison/ImageSimilarityReport.cs @@ -9,17 +9,21 @@ namespace SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; public class ImageSimilarityReport { protected ImageSimilarityReport( + int index, object expectedImage, object actualImage, IEnumerable differences, float? totalNormalizedDifference = null) { + this.Index = index; this.ExpectedImage = expectedImage; this.ActualImage = actualImage; this.TotalNormalizedDifference = totalNormalizedDifference; this.Differences = differences.ToArray(); } + public int Index { get; } + public object ExpectedImage { get; } public object ActualImage { get; } @@ -89,16 +93,17 @@ public class ImageSimilarityReport : ImageSimilarityReport where TPixelB : unmanaged, IPixel { public ImageSimilarityReport( + int index, ImageFrame expectedImage, ImageFrame actualImage, IEnumerable differences, float? totalNormalizedDifference = null) - : base(expectedImage, actualImage, differences, totalNormalizedDifference) + : base(index, expectedImage, actualImage, differences, totalNormalizedDifference) { } public static ImageSimilarityReport Empty => - new ImageSimilarityReport(null, null, Enumerable.Empty(), 0f); + new ImageSimilarityReport(0, null, null, Enumerable.Empty(), 0f); public new ImageFrame ExpectedImage => (ImageFrame)base.ExpectedImage; diff --git a/tests/ImageSharp.Tests/TestUtilities/ImageComparison/TolerantImageComparer.cs b/tests/ImageSharp.Tests/TestUtilities/ImageComparison/TolerantImageComparer.cs index 793d7af33e..c541133079 100644 --- a/tests/ImageSharp.Tests/TestUtilities/ImageComparison/TolerantImageComparer.cs +++ b/tests/ImageSharp.Tests/TestUtilities/ImageComparison/TolerantImageComparer.cs @@ -56,7 +56,7 @@ public TolerantImageComparer(float imageThreshold, int perPixelManhattanThreshol /// public int PerPixelManhattanThreshold { get; } - public override ImageSimilarityReport CompareImagesOrFrames(ImageFrame expected, ImageFrame actual) + public override ImageSimilarityReport CompareImagesOrFrames(int index, ImageFrame expected, ImageFrame actual) { if (expected.Size() != actual.Size()) { @@ -103,7 +103,7 @@ public override ImageSimilarityReport CompareImagesOrFrames this.ImageThreshold) { - return new ImageSimilarityReport(expected, actual, differences, normalizedDifference); + return new ImageSimilarityReport(index, expected, actual, differences, normalizedDifference); } else { From 0ad766f2667fe6c37cdbb8e9e7d340939ae00e83 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 8 Nov 2022 20:14:29 +1000 Subject: [PATCH 08/10] Skip bad test --- .../Formats/Tiff/TiffEncoderMultiframeTests.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderMultiframeTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderMultiframeTests.cs index 86f45a3a33..b74093fcc1 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderMultiframeTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffEncoderMultiframeTests.cs @@ -30,7 +30,9 @@ public void TiffEncoder_EncodeMultiframe_WithPreview(TestImageProvider(TestImageProvider provider) where TPixel : unmanaged, IPixel => TestTiffEncoderCore(provider, TiffBitsPerPixel.Bit48, TiffPhotometricInterpretation.Rgb); From e3872ae7935f75f630fdf16a11446730e725d746 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 9 Nov 2022 11:11:41 +1000 Subject: [PATCH 09/10] Read additional gifs and make transparency handling safer --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 58 ++++++++++++------- src/ImageSharp/Formats/Gif/GifThrowHelper.cs | 9 ++- .../Sections/GifXmpApplicationExtension.cs | 1 - .../Formats/ImageDecoderUtilities.cs | 8 +++ .../Formats/Gif/GifEncoderTests.cs | 6 +- tests/ImageSharp.Tests/TestImages.cs | 6 +- .../Images/Input/Gif/issues/issue_2288_3.gif | 3 + .../Images/Input/Gif/issues/issue_2288_4.gif | 3 + 8 files changed, 67 insertions(+), 27 deletions(-) create mode 100644 tests/Images/Input/Gif/issues/issue_2288_3.gif create mode 100644 tests/Images/Input/Gif/issues/issue_2288_4.gif diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 1b21c37f42..9c33d9e784 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -165,6 +165,11 @@ public Image Decode(BufferedReadStream stream, CancellationToken this.globalColorTable?.Dispose(); } + if (image is null) + { + GifThrowHelper.ThrowNoData(); + } + return image; } @@ -218,6 +223,11 @@ public IImageInfo Identify(BufferedReadStream stream, CancellationToken cancella this.globalColorTable?.Dispose(); } + if (this.logicalScreenDescriptor.Width == 0 && this.logicalScreenDescriptor.Height == 0) + { + GifThrowHelper.ThrowNoHeader(); + } + return new ImageInfo( new PixelTypeInfo(this.logicalScreenDescriptor.BitsPerPixel), this.logicalScreenDescriptor.Width, @@ -281,40 +291,44 @@ private void ReadApplicationExtension() // If the length is 11 then it's a valid extension and most likely // a NETSCAPE, XMP or ANIMEXTS extension. We want the loop count from this. + long position = this.stream.Position; if (appLength == GifConstants.ApplicationBlockSize) { this.stream.Read(this.buffer, 0, GifConstants.ApplicationBlockSize); bool isXmp = this.buffer.AsSpan().StartsWith(GifConstants.XmpApplicationIdentificationBytes); - if (isXmp && !this.skipMetadata) { - var extension = GifXmpApplicationExtension.Read(this.stream, this.memoryAllocator); + GifXmpApplicationExtension extension = GifXmpApplicationExtension.Read(this.stream, this.memoryAllocator); if (extension.Data.Length > 0) { this.metadata.XmpProfile = new XmpProfile(extension.Data); } + else + { + // Reset the stream position and continue. + this.stream.Position = position; + this.SkipBlock(appLength); + } return; } - else - { - int subBlockSize = this.stream.ReadByte(); - // TODO: There's also a NETSCAPE buffer extension. - // http://www.vurdalakov.net/misc/gif/netscape-buffering-application-extension - if (subBlockSize == GifConstants.NetscapeLoopingSubBlockSize) - { - this.stream.Read(this.buffer, 0, GifConstants.NetscapeLoopingSubBlockSize); - this.gifMetadata.RepeatCount = GifNetscapeLoopingApplicationExtension.Parse(this.buffer.AsSpan(1)).RepeatCount; - this.stream.Skip(1); // Skip the terminator. - return; - } + int subBlockSize = this.stream.ReadByte(); - // Could be something else not supported yet. - // Skip the subblock and terminator. - this.SkipBlock(subBlockSize); + // TODO: There's also a NETSCAPE buffer extension. + // http://www.vurdalakov.net/misc/gif/netscape-buffering-application-extension + if (subBlockSize == GifConstants.NetscapeLoopingSubBlockSize) + { + this.stream.Read(this.buffer, 0, GifConstants.NetscapeLoopingSubBlockSize); + this.gifMetadata.RepeatCount = GifNetscapeLoopingApplicationExtension.Parse(this.buffer.AsSpan(1)).RepeatCount; + this.stream.Skip(1); // Skip the terminator. + return; } + // Could be something else not supported yet. + // Skip the subblock and terminator. + this.SkipBlock(subBlockSize); + return; } @@ -500,7 +514,7 @@ private void ReadFrameColors(ref Image image, ref ImageFrame(ref Image image, ref ImageFrame throw new InvalidImageContentException(errorMessage); - public static void ThrowInvalidImageContentException(string errorMessage, Exception innerException) => throw new InvalidImageContentException(errorMessage, innerException); + [DoesNotReturn] + public static void ThrowNoHeader() => throw new InvalidImageContentException("Gif image does not contain a Logical Screen Descriptor."); + + [DoesNotReturn] + public static void ThrowNoData() => throw new InvalidImageContentException("Unable to read Gif image data"); } diff --git a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs index 45f3a92805..1c1127c3be 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs @@ -28,7 +28,6 @@ namespace SixLabors.ImageSharp.Formats.Gif; /// The stream to read from. /// The memory allocator. /// The XMP metadata - /// Thrown if the XMP block is not properly terminated. public static GifXmpApplicationExtension Read(Stream stream, MemoryAllocator allocator) { byte[] xmpBytes = ReadXmpData(stream, allocator); diff --git a/src/ImageSharp/Formats/ImageDecoderUtilities.cs b/src/ImageSharp/Formats/ImageDecoderUtilities.cs index c05e0d83cb..a8cbd5aad9 100644 --- a/src/ImageSharp/Formats/ImageDecoderUtilities.cs +++ b/src/ImageSharp/Formats/ImageDecoderUtilities.cs @@ -68,6 +68,10 @@ internal static IImageInfo Identify( { throw new InvalidImageContentException(decoder.Dimensions, ex); } + catch (Exception) + { + throw; + } } internal static Image Decode( @@ -96,6 +100,10 @@ internal static Image Decode( { throw largeImageExceptionFactory(ex, decoder.Dimensions); } + catch (Exception) + { + throw; + } } private static InvalidImageContentException DefaultLargeImageExceptionFactory( diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs index 15fd528e53..7fc61066a7 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs @@ -203,8 +203,10 @@ public void NonMutatingEncodePreservesPaletteCount() } [Theory] - [WithFile(TestImages.Gif.Issues.Issue2288OptionalExtension, PixelTypes.Rgba32)] - [WithFile(TestImages.Gif.Issues.Issue2288OptionalExtension2, PixelTypes.Rgba32)] + [WithFile(TestImages.Gif.Issues.Issue2288_A, PixelTypes.Rgba32)] + [WithFile(TestImages.Gif.Issues.Issue2288_B, PixelTypes.Rgba32)] + [WithFile(TestImages.Gif.Issues.Issue2288_C, PixelTypes.Rgba32)] + [WithFile(TestImages.Gif.Issues.Issue2288_D, PixelTypes.Rgba32)] public void OptionalExtensionsShouldBeHandledProperly(TestImageProvider provider) where TPixel : unmanaged, IPixel { diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 7887c09008..568d5a4054 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -476,8 +476,10 @@ public static class Issues public const string Issue1962NoColorTable = "Gif/issues/issue1962_tiniest_gif_1st.gif"; public const string Issue2012EmptyXmp = "Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif"; public const string Issue2012BadMinCode = "Gif/issues/issue2012_drona1.gif"; - public const string Issue2288OptionalExtension = "Gif/issues/issue_2288.gif"; - public const string Issue2288OptionalExtension2 = "Gif/issues/issue_2288_2.gif"; + public const string Issue2288_A = "Gif/issues/issue_2288.gif"; + public const string Issue2288_B = "Gif/issues/issue_2288_2.gif"; + public const string Issue2288_C = "Gif/issues/issue_2288_3.gif"; + public const string Issue2288_D = "Gif/issues/issue_2288_4.gif"; } public static readonly string[] All = { Rings, Giphy, Cheers, Trans, Kumin, Leo, Ratio4x1, Ratio1x4 }; diff --git a/tests/Images/Input/Gif/issues/issue_2288_3.gif b/tests/Images/Input/Gif/issues/issue_2288_3.gif new file mode 100644 index 0000000000..fae784409b --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue_2288_3.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d3f0fd68a03e9c1c896e021828f470d9ceeb8f10e1aead230e42e55670520840 +size 958995 diff --git a/tests/Images/Input/Gif/issues/issue_2288_4.gif b/tests/Images/Input/Gif/issues/issue_2288_4.gif new file mode 100644 index 0000000000..cd975a9202 --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue_2288_4.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:0ac8bc8677a1eb4e26161e5255a5c651321ff063892f3caec3f95264aee38057 +size 1971226 From 0e26da8979076635948aa3a4ce224e802ea79324 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 9 Nov 2022 11:27:05 +1000 Subject: [PATCH 10/10] Better trans index fix. --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 9c33d9e784..b95878995e 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -514,7 +514,7 @@ private void ReadFrameColors(ref Image image, ref ImageFrame(ref Image image, ref ImageFrame colorTableMaxIdx || rawIndex == transIndex) { - ref TPixel pixel = ref Unsafe.Add(ref rowRef, x); - Rgb24 rgb = colorTable[index]; - pixel.FromRgb24(rgb); + continue; } + + int index = Numerics.Clamp(rawIndex, 0, colorTableMaxIdx); + ref TPixel pixel = ref Unsafe.Add(ref rowRef, x); + Rgb24 rgb = colorTable[index]; + pixel.FromRgb24(rgb); } } }