From 6decd7b2261bda8a91a2a7127eaed8a04faaf272 Mon Sep 17 00:00:00 2001 From: James South Date: Thu, 30 Oct 2014 22:27:38 +0000 Subject: [PATCH] Fix memory leak in GifEncoder Former-commit-id: b3ee297b52fa012dcc31d210b48489338ef746fb Former-commit-id: d455ea683b3235c18e9b8772cb7ff66e87a3105d --- src/ImageProcessor.Playground/Program.cs | 8 +-- .../AssertionHelpers.cs | 38 ++++++++++---- .../ImageFactoryUnitTests.cs | 18 +++---- .../Imaging/Formats/GifEncoder.cs | 49 ++++++++++++------- .../Imaging/Formats/GifFormat.cs | 11 ++--- 5 files changed, 75 insertions(+), 49 deletions(-) diff --git a/src/ImageProcessor.Playground/Program.cs b/src/ImageProcessor.Playground/Program.cs index 15c446519..692593f76 100644 --- a/src/ImageProcessor.Playground/Program.cs +++ b/src/ImageProcessor.Playground/Program.cs @@ -50,7 +50,7 @@ namespace ImageProcessor.PlayGround } Image mask = Image.FromFile(Path.Combine(resolvedPath, "mask2.png")); - IEnumerable files = GetFilesByExtensions(di, ".jfif"); + IEnumerable files = GetFilesByExtensions(di, ".gif"); //IEnumerable files = GetFilesByExtensions(di, ".gif", ".webp", ".bmp", ".jpg", ".png", ".tif"); foreach (FileInfo fileInfo in files) @@ -85,9 +85,9 @@ namespace ImageProcessor.PlayGround //.BackgroundColor(Color.Cyan) //.ReplaceColor(Color.FromArgb(255, 1, 107, 165), Color.FromArgb(255, 1, 165, 13), 80) //.Resize(size) - .Resize(new ResizeLayer(size, ResizeMode.Max)) - .Resize(new ResizeLayer(size, ResizeMode.Stretch)) - //.DetectEdges(new SobelEdgeFilter(), true) + // .Resize(new ResizeLayer(size, ResizeMode.Max)) + // .Resize(new ResizeLayer(size, ResizeMode.Stretch)) + .DetectEdges(new SobelEdgeFilter(), true) //.DetectEdges(new LaplacianOfGaussianEdgeFilter()) //.EntropyCrop() //.Filter(MatrixFilters.Invert) diff --git a/src/ImageProcessor.UnitTests/AssertionHelpers.cs b/src/ImageProcessor.UnitTests/AssertionHelpers.cs index 205480f9e..fe394a543 100644 --- a/src/ImageProcessor.UnitTests/AssertionHelpers.cs +++ b/src/ImageProcessor.UnitTests/AssertionHelpers.cs @@ -4,13 +4,14 @@ // Licensed under the Apache License, Version 2.0. // // -------------------------------------------------------------------------------------------------------------------- + namespace ImageProcessor.UnitTests { using System.Drawing; using System.Drawing.Imaging; - using System.Linq; using System.IO; - using NUnit.Framework; + using System.Linq; + using FluentAssertions; /// @@ -21,9 +22,19 @@ namespace ImageProcessor.UnitTests /// /// Asserts that two images are identical /// - /// The expected result - /// The tested image - public static void AssertImagesAreIdentical(Image expected, Image tested, string because, params string[] becauseArgs) + /// + /// The expected result + /// + /// + /// The tested image + /// + /// + /// The because message. + /// + /// + /// The because arguments. + /// + public static void AssertImagesAreIdentical(Image expected, Image tested, string because, params object[] becauseArgs) { ToByteArray(expected).SequenceEqual(ToByteArray(tested)).Should().BeTrue(because, becauseArgs); } @@ -31,9 +42,19 @@ namespace ImageProcessor.UnitTests /// /// Asserts that two images are different /// - /// The not-expected result - /// The tested image - public static void AssertImagesAreDifferent(Image expected, Image tested, string because, params string[] becauseArgs) + /// + /// The not-expected result + /// + /// + /// The tested image + /// + /// + /// The because message. + /// + /// + /// The because arguments. + /// + public static void AssertImagesAreDifferent(Image expected, Image tested, string because, params object[] becauseArgs) { ToByteArray(expected).SequenceEqual(ToByteArray(tested)).Should().BeFalse(because, becauseArgs); } @@ -42,7 +63,6 @@ namespace ImageProcessor.UnitTests /// Converts an image to a byte array /// /// The image to convert - /// The format to use /// An array of bytes representing the image public static byte[] ToByteArray(Image imageIn) { diff --git a/src/ImageProcessor.UnitTests/ImageFactoryUnitTests.cs b/src/ImageProcessor.UnitTests/ImageFactoryUnitTests.cs index 222771bc6..14ca2e062 100644 --- a/src/ImageProcessor.UnitTests/ImageFactoryUnitTests.cs +++ b/src/ImageProcessor.UnitTests/ImageFactoryUnitTests.cs @@ -9,15 +9,15 @@ namespace ImageProcessor.UnitTests using System; using System.Collections.Generic; using System.Drawing; - using System.Drawing.Imaging; using System.IO; using System.Linq; + using FluentAssertions; + using ImageProcessor.Imaging; using ImageProcessor.Imaging.Filters.EdgeDetection; using ImageProcessor.Imaging.Filters.Photo; - using FluentAssertions; using NUnit.Framework; /// @@ -299,7 +299,7 @@ namespace ImageProcessor.UnitTests { Image original = (Image)imageFactory.Image.Clone(); - List filters = new List() + List filters = new List { MatrixFilters.BlackWhite, MatrixFilters.Comic, @@ -533,7 +533,7 @@ namespace ImageProcessor.UnitTests { Image original = (Image)imageFactory.Image.Clone(); - List filters = new List() + List filters = new List { new KayyaliEdgeFilter(), new KirschEdgeFilter(), @@ -606,22 +606,22 @@ namespace ImageProcessor.UnitTests /// The list of images private IEnumerable ListInputImages() { - if (imagesFactories == null || imagesFactories.Count() == 0) + if (this.imagesFactories == null || !this.imagesFactories.Any()) { - imagesFactories = new List(); + this.imagesFactories = new List(); foreach (FileInfo fi in this.ListInputFiles()) { - imagesFactories.Add((new ImageFactory()).Load(fi.FullName)); + this.imagesFactories.Add((new ImageFactory()).Load(fi.FullName)); } } // reset all the images whenever we call this - foreach (ImageFactory image in imagesFactories) + foreach (ImageFactory image in this.imagesFactories) { image.Reset(); } - return imagesFactories; + return this.imagesFactories; } } } \ No newline at end of file diff --git a/src/ImageProcessor/Imaging/Formats/GifEncoder.cs b/src/ImageProcessor/Imaging/Formats/GifEncoder.cs index a418c951e..89a2ec908 100644 --- a/src/ImageProcessor/Imaging/Formats/GifEncoder.cs +++ b/src/ImageProcessor/Imaging/Formats/GifEncoder.cs @@ -17,6 +17,7 @@ namespace ImageProcessor.Imaging.Formats { using System; + using System.Drawing; using System.Drawing.Imaging; using System.IO; using System.Linq; @@ -114,7 +115,7 @@ namespace ImageProcessor.Imaging.Formats /// The stream. /// // ReSharper disable once FieldCanBeMadeReadOnly.Local - private MemoryStream inputStream; + private MemoryStream imageStream; /// /// The height. @@ -154,9 +155,6 @@ namespace ImageProcessor.Imaging.Formats /// /// Initializes a new instance of the class. /// - /// - /// The stream that will be written to. - /// /// /// Sets the width for this gif or null to use the first frame's width. /// @@ -166,9 +164,9 @@ namespace ImageProcessor.Imaging.Formats /// /// The number of times to repeat the animation. /// - public GifEncoder(MemoryStream stream, int? width = null, int? height = null, int? repeatCount = null) + public GifEncoder(int? width = null, int? height = null, int? repeatCount = null) { - this.inputStream = stream; + this.imageStream = new MemoryStream(); this.width = width; this.height = height; this.repeatCount = repeatCount; @@ -232,6 +230,23 @@ namespace ImageProcessor.Imaging.Formats // from executing a second time. GC.SuppressFinalize(this); } + + /// + /// Saves the completed gif to an + /// + /// The completed animated gif. + public Image Save() + { + // Complete File + this.WriteByte(FileTrailer); + + // Push the data + this.imageStream.Flush(); + this.imageStream.Position = 0; + byte[] bytes = this.imageStream.ToArray(); + ImageConverter converter = new ImageConverter(); + return (Image)converter.ConvertFrom(bytes); + } #endregion #region Methods @@ -250,11 +265,7 @@ namespace ImageProcessor.Imaging.Formats if (disposing) { - // Complete File - this.WriteByte(FileTrailer); - - // Push the data - this.inputStream.Flush(); + this.imageStream.Dispose(); } // Call the appropriate methods to clean up @@ -319,7 +330,7 @@ namespace ImageProcessor.Imaging.Formats /// private void WriteByte(int value) { - this.inputStream.WriteByte(Convert.ToByte(value)); + this.imageStream.WriteByte(Convert.ToByte(value)); } /// @@ -333,7 +344,7 @@ namespace ImageProcessor.Imaging.Formats sourceGif.Position = SourceColorBlockPosition; // Locating the image color table byte[] colorTable = new byte[SourceColorBlockLength]; sourceGif.Read(colorTable, 0, colorTable.Length); - this.inputStream.Write(colorTable, 0, colorTable.Length); + this.imageStream.Write(colorTable, 0, colorTable.Length); } /// @@ -415,12 +426,12 @@ namespace ImageProcessor.Imaging.Formats byte[] imgData = new byte[dataLength]; sourceGif.Read(imgData, 0, dataLength); - this.inputStream.WriteByte(Convert.ToByte(dataLength)); - this.inputStream.Write(imgData, 0, dataLength); + this.imageStream.WriteByte(Convert.ToByte(dataLength)); + this.imageStream.Write(imgData, 0, dataLength); dataLength = sourceGif.ReadByte(); } - this.inputStream.WriteByte(0); // Terminator + this.imageStream.WriteByte(0); // Terminator } /// @@ -432,8 +443,8 @@ namespace ImageProcessor.Imaging.Formats private void WriteShort(int value) { // Leave only one significant byte. - this.inputStream.WriteByte(Convert.ToByte(value & 0xff)); - this.inputStream.WriteByte(Convert.ToByte((value >> 8) & 0xff)); + this.imageStream.WriteByte(Convert.ToByte(value & 0xff)); + this.imageStream.WriteByte(Convert.ToByte((value >> 8) & 0xff)); } /// @@ -444,7 +455,7 @@ namespace ImageProcessor.Imaging.Formats /// private void WriteString(string value) { - this.inputStream.Write(value.ToArray().Select(c => (byte)c).ToArray(), 0, value.Length); + this.imageStream.Write(value.ToArray().Select(c => (byte)c).ToArray(), 0, value.Length); } #endregion } diff --git a/src/ImageProcessor/Imaging/Formats/GifFormat.cs b/src/ImageProcessor/Imaging/Formats/GifFormat.cs index 5307fcfd9..091db1037 100644 --- a/src/ImageProcessor/Imaging/Formats/GifFormat.cs +++ b/src/ImageProcessor/Imaging/Formats/GifFormat.cs @@ -79,11 +79,7 @@ namespace ImageProcessor.Imaging.Formats if (decoder.IsAnimated) { OctreeQuantizer quantizer = new OctreeQuantizer(255, 8); - - // We don't dispose of the memory stream as that is disposed when a new image is created and doing so - // beforehand will cause an exception. - MemoryStream stream = new MemoryStream(); - using (GifEncoder encoder = new GifEncoder(stream, null, null, decoder.LoopCount)) + using (GifEncoder encoder = new GifEncoder(null, null, decoder.LoopCount)) { foreach (GifFrame frame in decoder.GifFrames) { @@ -91,10 +87,9 @@ namespace ImageProcessor.Imaging.Formats frame.Image = quantizer.Quantize(processor.Invoke(factory)); encoder.AddFrame(frame); } - } - stream.Position = 0; - factory.Image = Image.FromStream(stream); + factory.Image = encoder.Save(); + } } else {