From 4f76f8a110541ac67f3f167cde1a35ae1575134c Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 12 Nov 2016 12:40:31 +1100 Subject: [PATCH] Fix the png ecoder --- .../Formats/Png/Filters/AverageFilter.cs | 4 +- .../Formats/Png/Filters/NoneFilter.cs | 7 +- .../Formats/Png/Filters/PaethFilter.cs | 4 +- .../Formats/Png/Filters/SubFilter.cs | 4 +- .../Formats/Png/Filters/UpFilter.cs | 4 +- src/ImageSharp/Formats/Png/PngEncoderCore.cs | 67 +++++++++---------- 6 files changed, 42 insertions(+), 48 deletions(-) diff --git a/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs b/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs index cd95eba56..6fe2175d9 100644 --- a/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs @@ -49,13 +49,13 @@ namespace ImageSharp.Formats /// /// The scanline to encode /// The previous scanline. - /// The encoded scanline. /// The bytes per pixel. /// The number of bytes per scanline /// The - public static byte[] Encode(byte[] scanline, byte[] previousScanline, byte[] result, int bytesPerPixel, int bytesPerScanline) + public static byte[] Encode(byte[] scanline, byte[] previousScanline, int bytesPerPixel, int bytesPerScanline) { // Average(x) = Raw(x) - floor((Raw(x-bpp)+Prior(x))/2) + byte[] result = new byte[bytesPerScanline + 1]; fixed (byte* scan = scanline) fixed (byte* prev = previousScanline) fixed (byte* res = result) diff --git a/src/ImageSharp/Formats/Png/Filters/NoneFilter.cs b/src/ImageSharp/Formats/Png/Filters/NoneFilter.cs index ef41fc631..e62442034 100644 --- a/src/ImageSharp/Formats/Png/Filters/NoneFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/NoneFilter.cs @@ -29,13 +29,14 @@ namespace ImageSharp.Formats /// Encodes the scanline /// /// The scanline to encode - /// The encoded scanline. /// The number of bytes per scanline - public static void Encode(byte[] scanline, byte[] result, int bytesPerScanline) + /// The + public static byte[] Encode(byte[] scanline, int bytesPerScanline) { // Insert a byte before the data. - result[0] = 0; + byte[] result = new byte[bytesPerScanline + 1]; Buffer.BlockCopy(scanline, 0, result, 1, bytesPerScanline); + return result; } } } diff --git a/src/ImageSharp/Formats/Png/Filters/PaethFilter.cs b/src/ImageSharp/Formats/Png/Filters/PaethFilter.cs index 16c0378e9..eeb09ea5c 100644 --- a/src/ImageSharp/Formats/Png/Filters/PaethFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/PaethFilter.cs @@ -49,13 +49,13 @@ namespace ImageSharp.Formats /// /// The scanline to encode /// The previous scanline. - /// The encoded scanline. /// The bytes per pixel. /// The number of bytes per scanline /// The - public static byte[] Encode(byte[] scanline, byte[] previousScanline, byte[] result, int bytesPerPixel, int bytesPerScanline) + public static byte[] Encode(byte[] scanline, byte[] previousScanline, int bytesPerPixel, int bytesPerScanline) { // Paeth(x) = Raw(x) - PaethPredictor(Raw(x-bpp), Prior(x), Prior(x - bpp)) + byte[] result = new byte[bytesPerScanline + 1]; fixed (byte* scan = scanline) fixed (byte* prev = previousScanline) fixed (byte* res = result) diff --git a/src/ImageSharp/Formats/Png/Filters/SubFilter.cs b/src/ImageSharp/Formats/Png/Filters/SubFilter.cs index 63e41a1d5..c5b892066 100644 --- a/src/ImageSharp/Formats/Png/Filters/SubFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/SubFilter.cs @@ -41,13 +41,13 @@ namespace ImageSharp.Formats /// Encodes the scanline /// /// The scanline to encode - /// The encoded scanline. /// The bytes per pixel. /// The number of bytes per scanline /// The - public static byte[] Encode(byte[] scanline, byte[] result, int bytesPerPixel, int bytesPerScanline) + public static byte[] Encode(byte[] scanline, int bytesPerPixel, int bytesPerScanline) { // Sub(x) = Raw(x) - Raw(x-bpp) + byte[] result = new byte[bytesPerScanline + 1]; fixed (byte* scan = scanline) fixed (byte* res = result) { diff --git a/src/ImageSharp/Formats/Png/Filters/UpFilter.cs b/src/ImageSharp/Formats/Png/Filters/UpFilter.cs index b40aa944b..6b69a6cd6 100644 --- a/src/ImageSharp/Formats/Png/Filters/UpFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/UpFilter.cs @@ -43,12 +43,12 @@ namespace ImageSharp.Formats /// /// The scanline to encode /// The previous scanline. - /// The encoded scanline. /// The number of bytes per scanline /// The - public static byte[] Encode(byte[] scanline, byte[] previousScanline, byte[] result, int bytesPerScanline) + public static byte[] Encode(byte[] scanline, byte[] previousScanline, int bytesPerScanline) { // Up(x) = Raw(x) - Prior(x) + byte[] result = new byte[bytesPerScanline + 1]; fixed (byte* scan = scanline) fixed (byte* prev = previousScanline) fixed (byte* res = result) diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index 764d2f538..f72fbc180 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -15,7 +15,6 @@ namespace ImageSharp.Formats /// /// Performs the png encoding operation. - /// TODO: Perf. There's lots of array parsing and copying going on here. This should be unmanaged. /// internal sealed class PngEncoderCore { @@ -295,6 +294,7 @@ namespace ImageSharp.Formats where TColor : struct, IPackedPixel where TPacked : struct { + // TODO: This should be possible one row at a time int bpp = this.bytesPerPixel; for (int x = 0; x < this.width; x++) { @@ -312,9 +312,9 @@ namespace ImageSharp.Formats /// The row. /// The previous scanline. /// The raw scanline. - /// The resultant filtered scanline. /// The number of bytes per scanline. - private void EncodePixelRow(PixelAccessor pixels, int row, byte[] previousScanline, byte[] rawScanline, byte[] result, int bytesPerScanline) + /// The + private byte[] EncodePixelRow(PixelAccessor pixels, int row, byte[] previousScanline, byte[] rawScanline, int bytesPerScanline) where TColor : struct, IPackedPixel where TPacked : struct { @@ -332,7 +332,7 @@ namespace ImageSharp.Formats break; } - this.GetOptimalFilteredScanline(rawScanline, previousScanline, result, bytesPerScanline); + return this.GetOptimalFilteredScanline(rawScanline, previousScanline, bytesPerScanline); } /// @@ -341,30 +341,30 @@ namespace ImageSharp.Formats /// /// The raw scanline /// The previous scanline - /// The filtered scanline result /// The number of bytes per scanline - private void GetOptimalFilteredScanline(byte[] rawScanline, byte[] previousScanline, byte[] result, int bytesPerScanline) + /// The + private byte[] GetOptimalFilteredScanline(byte[] rawScanline, byte[] previousScanline, int bytesPerScanline) { // Palette images don't compress well with adaptive filtering. if (this.PngColorType == PngColorType.Palette) { - NoneFilter.Encode(rawScanline, result, bytesPerScanline); - return; + return NoneFilter.Encode(rawScanline, bytesPerScanline); } + // TODO: it would be nice to avoid the memory allocation here. Tuple[] candidates = new Tuple[4]; - byte[] sub = SubFilter.Encode(rawScanline, result, this.bytesPerPixel, bytesPerScanline); - candidates[0] = new Tuple(sub, this.CalculateTotalVariation(sub)); + byte[] sub = SubFilter.Encode(rawScanline, this.bytesPerPixel, bytesPerScanline); + candidates[0] = new Tuple(sub, this.CalculateTotalVariation(sub, bytesPerScanline)); - byte[] up = UpFilter.Encode(rawScanline, previousScanline, result, bytesPerScanline); - candidates[1] = new Tuple(up, this.CalculateTotalVariation(up)); + byte[] up = UpFilter.Encode(rawScanline, previousScanline, bytesPerScanline); + candidates[1] = new Tuple(up, this.CalculateTotalVariation(up, bytesPerScanline)); - byte[] average = AverageFilter.Encode(rawScanline, previousScanline, result, this.bytesPerPixel, bytesPerScanline); - candidates[2] = new Tuple(average, this.CalculateTotalVariation(average)); + byte[] average = AverageFilter.Encode(rawScanline, previousScanline, this.bytesPerPixel, bytesPerScanline); + candidates[2] = new Tuple(average, this.CalculateTotalVariation(average, bytesPerScanline)); - byte[] paeth = PaethFilter.Encode(rawScanline, previousScanline, result, this.bytesPerPixel, bytesPerScanline); - candidates[3] = new Tuple(paeth, this.CalculateTotalVariation(paeth)); + byte[] paeth = PaethFilter.Encode(rawScanline, previousScanline, this.bytesPerPixel, bytesPerScanline); + candidates[3] = new Tuple(paeth, this.CalculateTotalVariation(paeth, bytesPerScanline)); int lowestTotalVariation = int.MaxValue; int lowestTotalVariationIndex = 0; @@ -379,7 +379,7 @@ namespace ImageSharp.Formats } // ReSharper disable once RedundantAssignment - result = candidates[lowestTotalVariationIndex].Item1; + return candidates[lowestTotalVariationIndex].Item1; } /// @@ -387,12 +387,13 @@ namespace ImageSharp.Formats /// neighbor differences. /// /// The scanline bytes + /// The number of bytes per scanline /// The - private int CalculateTotalVariation(byte[] input) + private int CalculateTotalVariation(byte[] input, int bytesPerScanline) { int totalVariation = 0; - for (int i = 1; i < input.Length; i++) + for (int i = 1; i < bytesPerScanline; i++) { totalVariation += Math.Abs(input[i] - input[i - 1]); } @@ -591,12 +592,6 @@ namespace ImageSharp.Formats byte[] previousScanline = ArrayPool.Shared.Rent(bytesPerScanline); byte[] rawScanline = ArrayPool.Shared.Rent(bytesPerScanline); int resultLength = bytesPerScanline + 1; - byte[] result = ArrayPool.Shared.Rent(resultLength); - - // TODO: Clearing this array makes the visual tests work again when encoding multiple images in a row. - // The png analyser tool I use still cannot decompress the image though my own decoder, chome and edge browsers, and paint can. Twitter also cannot read the file. - // It's 2am now so I'm going to check in what I have and cry. :'( - Array.Clear(result, 0, resultLength); byte[] buffer; int bufferLength; @@ -608,26 +603,23 @@ namespace ImageSharp.Formats { for (int y = 0; y < this.height; y++) { - this.EncodePixelRow(pixels, y, previousScanline, rawScanline, result, bytesPerScanline); - deflateStream.Write(result, 0, resultLength); + deflateStream.Write(this.EncodePixelRow(pixels, y, previousScanline, rawScanline, bytesPerScanline), 0, resultLength); // Do a bit of shuffling; byte[] tmp = rawScanline; rawScanline = previousScanline; previousScanline = tmp; } - - deflateStream.Flush(); - buffer = memoryStream.ToArray(); - bufferLength = buffer.Length; } + + buffer = memoryStream.ToArray(); + bufferLength = buffer.Length; } finally { memoryStream?.Dispose(); ArrayPool.Shared.Return(previousScanline); ArrayPool.Shared.Return(rawScanline); - ArrayPool.Shared.Return(result); } // Store the chunks in repeated 64k blocks. @@ -682,10 +674,8 @@ namespace ImageSharp.Formats /// The of the data to write. private void WriteChunk(Stream stream, string type, byte[] data, int offset, int length) { - // Chunk length WriteInteger(stream, length); - // Chunk type this.chunkTypeBuffer[0] = (byte)type[0]; this.chunkTypeBuffer[1] = (byte)type[1]; this.chunkTypeBuffer[2] = (byte)type[2]; @@ -693,13 +683,16 @@ namespace ImageSharp.Formats stream.Write(this.chunkTypeBuffer, 0, 4); + if (data != null) + { + stream.Write(data, offset, length); + } + Crc32 crc32 = new Crc32(); crc32.Update(this.chunkTypeBuffer); - // Chunk data - if (data != null) + if (data != null && length > 0) { - stream.Write(data, offset, length); crc32.Update(data, offset, length); }