diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index e67c599e3..6bec8dc2e 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -479,7 +479,7 @@ internal sealed class GifDecoderCore : IImageDecoderInternals prevFrame = previousFrame; } - currentFrame = image.Frames.AddFrame(previousFrame); // This clones the frame and adds it the collection + currentFrame = image.Frames.AddFrame(new ImageFrame(this.configuration, previousFrame.Width, previousFrame.Height)); this.SetFrameMetadata(currentFrame.Metadata, false); @@ -548,7 +548,7 @@ internal sealed class GifDecoderCore : IImageDecoderInternals // #403 The left + width value can be larger than the image width for (int x = descriptorLeft; x < descriptorRight && x < imageWidth; x++) { - int index = Numerics.Clamp(Unsafe.Add(ref indicesRowRef, x - descriptorLeft), 0, colorTableMaxIdx); + int index = Numerics.Clamp(Unsafe.Add(ref indicesRowRef, x - descriptorLeft), 0, Math.Max(transIndex, colorTableMaxIdx)); ref TPixel pixel = ref Unsafe.Add(ref rowRef, x); Rgb24 rgb = colorTable[index]; pixel.FromRgb24(rgb); @@ -558,7 +558,7 @@ internal sealed class GifDecoderCore : IImageDecoderInternals { for (int x = descriptorLeft; x < descriptorRight && x < imageWidth; x++) { - int index = Numerics.Clamp(Unsafe.Add(ref indicesRowRef, x - descriptorLeft), 0, colorTableMaxIdx); + int index = Numerics.Clamp(Unsafe.Add(ref indicesRowRef, x - descriptorLeft), 0, Math.Max(transIndex, colorTableMaxIdx)); if (transIndex != index) { ref TPixel pixel = ref Unsafe.Add(ref rowRef, x); @@ -596,7 +596,7 @@ internal sealed class GifDecoderCore : IImageDecoderInternals 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 @@ internal sealed class GifDecoderCore : IImageDecoderInternals && this.logicalScreenDescriptor.GlobalColorTableSize > 0) { GifFrameMetadata gifMeta = meta.GetGifMetadata(); + gifMeta.ColorTableMode = GifColorTableMode.Global; gifMeta.ColorTableLength = this.logicalScreenDescriptor.GlobalColorTableSize; } @@ -623,6 +624,7 @@ internal sealed class GifDecoderCore : IImageDecoderInternals && 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 6ba049338..135dc1329 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -93,7 +93,6 @@ internal sealed class GifEncoderCore : IImageEncoderInternals // Quantize the image returning a palette. IndexedImageFrame quantized; - using (IQuantizer frameQuantizer = this.quantizer.CreatePixelSpecificQuantizer(this.configuration)) { if (useGlobalTable) @@ -133,114 +132,102 @@ internal sealed class GifEncoderCore : IImageEncoderInternals 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 82694eab9..7f4b49f0b 100644 --- a/src/ImageSharp/Formats/Gif/GifFrameMetadata.cs +++ b/src/ImageSharp/Formats/Gif/GifFrameMetadata.cs @@ -21,13 +21,19 @@ public class GifFrameMetadata : IDeepCloneable /// 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 52019c335..da21e134e 100644 --- a/src/ImageSharp/Formats/Gif/GifMetadata.cs +++ b/src/ImageSharp/Formats/Gif/GifMetadata.cs @@ -50,7 +50,7 @@ public class GifMetadata : IDeepCloneable 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 bbe8e4113..71b74242f 100644 --- a/src/ImageSharp/Processing/Processors/Quantization/OctreeQuantizer{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Quantization/OctreeQuantizer{TPixel}.cs @@ -106,7 +106,7 @@ public struct OctreeQuantizer : IQuantizer // 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 f40cde487..f3ed39c95 100644 --- a/src/ImageSharp/Processing/Processors/Quantization/PaletteQuantizer.cs +++ b/src/ImageSharp/Processing/Processors/Quantization/PaletteQuantizer.cs @@ -49,12 +49,8 @@ public class PaletteQuantizer : IQuantizer { 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 7c71fd546..15fd528e5 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs @@ -204,6 +204,7 @@ public class GifEncoderTests [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 ea8fba998..7887c0900 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -477,6 +477,7 @@ public static class TestImages 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 000000000..790ef4bf6 --- /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