From 1e44912d7fe5179e0af28fb01f6196a8cc9ec508 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Thu, 12 Apr 2018 10:33:19 -0700 Subject: [PATCH 01/13] Make PngChunk an immutable struct --- src/ImageSharp/Formats/Png/PngChunk.cs | 30 +++++-- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 95 ++++++++++---------- 2 files changed, 68 insertions(+), 57 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngChunk.cs b/src/ImageSharp/Formats/Png/PngChunk.cs index b944b43a3..399bc95c9 100644 --- a/src/ImageSharp/Formats/Png/PngChunk.cs +++ b/src/ImageSharp/Formats/Png/PngChunk.cs @@ -8,11 +8,14 @@ namespace SixLabors.ImageSharp.Formats.Png /// /// Stores header information about a chunk. /// - internal sealed class PngChunk + internal readonly struct PngChunk { - public PngChunk(int length) + public PngChunk(int length, string type, IManagedByteBuffer data = null, uint crc = default) { this.Length = length; + this.Type = type; + this.Data = data; + this.Crc = crc; } /// @@ -24,21 +27,30 @@ namespace SixLabors.ImageSharp.Formats.Png public int Length { get; } /// - /// Gets or sets the chunk type as string with 4 chars. + /// Gets the chunk type as string with 4 chars. /// - public string Type { get; set; } + public string Type { get; } /// - /// Gets or sets the data bytes appropriate to the chunk type, if any. - /// This field can be of zero length. + /// Gets the data bytes appropriate to the chunk type, if any. + /// This field can be of zero length or null. /// - public IManagedByteBuffer Data { get; set; } + public IManagedByteBuffer Data { get; } /// - /// Gets or sets a CRC (Cyclic Redundancy Check) calculated on the preceding bytes in the chunk, + /// Gets a CRC (Cyclic Redundancy Check) calculated on the preceding bytes in the chunk, /// including the chunk type code and chunk data fields, but not including the length field. /// The CRC is always present, even for chunks containing no data /// - public uint Crc { get; set; } + public uint Crc { get; } + + /// + /// Gets a value indicating whether the given chunk is critical to decoding + /// + public bool IsCritical => + this.Type == PngChunkTypes.Header || + this.Type == PngChunkTypes.Palette || + this.Type == PngChunkTypes.Data || + this.Type == PngChunkTypes.End; } } diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index b3904c0a3..f4b045759 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -262,12 +262,7 @@ namespace SixLabors.ImageSharp.Formats.Png } finally { - // Data is rented in ReadChunkData() - if (chunk.Data != null) - { - chunk.Data.Dispose(); - chunk.Data = null; - } + chunk.Data?.Dispose(); // Data is rented in ReadChunkData() } } } @@ -383,20 +378,6 @@ namespace SixLabors.ImageSharp.Formats.Png return result; } - /// - /// Returns a value indicating whether the given chunk is critical to decoding - /// - /// The chunk - /// The - private static bool IsCriticalChunk(PngChunk chunk) - { - return - chunk.Type == PngChunkTypes.Header || - chunk.Type == PngChunkTypes.Palette || - chunk.Type == PngChunkTypes.Data || - chunk.Type == PngChunkTypes.End; - } - /// /// Reads an integer value from 2 consecutive bytes in LSB order /// @@ -1217,38 +1198,63 @@ namespace SixLabors.ImageSharp.Formats.Png return false; } - chunk = new PngChunk(length); - - if (chunk.Length < 0 || chunk.Length > this.currentStream.Length - this.currentStream.Position) + while (length < 0 || length > (this.currentStream.Length - this.currentStream.Position)) { // Not a valid chunk so we skip back all but one of the four bytes we have just read. // That lets us read one byte at a time until we reach a known chunk. this.currentStream.Position -= 3; - return true; + length = this.ReadChunkLength(); + + if (length == -1) + { + chunk = default; + + return false; + } } - this.ReadChunkType(chunk); + string type = this.ReadChunkType(); - if (chunk.Type == PngChunkTypes.Data) + // NOTE: Handling the chunk data is the responsible of the caller + // It is currently either skipped (in identification) or read during decoding + if (type == PngChunkTypes.Data) { + chunk = new PngChunk(length, type); + return true; } - this.ReadChunkData(chunk); - this.ReadChunkCrc(chunk); + chunk = new PngChunk( + length: length, + type: type, + data: this.ReadChunkData(length), + crc: this.ReadChunkCrc()); + + this.ValidateChunk(chunk); return true; } + private void ValidateChunk(in PngChunk chunk) + { + this.crc.Reset(); + this.crc.Update(this.chunkTypeBuffer); + this.crc.Update(new ReadOnlySpan(chunk.Data.Array, 0, chunk.Length)); + + if (this.crc.Value != chunk.Crc && chunk.IsCritical) + { + throw new ImageFormatException($"CRC Error. PNG {chunk.Type} chunk is corrupt!"); + } + } + /// /// Reads the cycle redundancy chunk from the data. /// - /// The chunk. /// /// Thrown if the input stream is not valid or corrupt. /// - private void ReadChunkCrc(PngChunk chunk) + private uint ReadChunkCrc() { int numBytes = this.currentStream.Read(this.crcBuffer, 0, 4); @@ -1257,22 +1263,13 @@ namespace SixLabors.ImageSharp.Formats.Png throw new ImageFormatException("Image stream is not valid!"); } - chunk.Crc = BinaryPrimitives.ReadUInt32BigEndian(this.crcBuffer); - - this.crc.Reset(); - this.crc.Update(this.chunkTypeBuffer); - this.crc.Update(new ReadOnlySpan(chunk.Data.Array, 0, chunk.Length)); - - if (this.crc.Value != chunk.Crc && IsCriticalChunk(chunk)) - { - throw new ImageFormatException($"CRC Error. PNG {chunk.Type} chunk is corrupt!"); - } + return BinaryPrimitives.ReadUInt32BigEndian(this.crcBuffer); } /// /// Skips the chunk data and the cycle redundancy chunk read from the data. /// - private void SkipChunkDataAndCrc(PngChunk chunk) + private void SkipChunkDataAndCrc(in PngChunk chunk) { this.currentStream.Skip(chunk.Length); this.currentStream.Skip(4); @@ -1281,22 +1278,24 @@ namespace SixLabors.ImageSharp.Formats.Png /// /// Reads the chunk data from the stream. /// - /// The chunk. - private void ReadChunkData(PngChunk chunk) + /// The length of the chunk data to read. + private IManagedByteBuffer ReadChunkData(int length) { // We rent the buffer here to return it afterwards in Decode() - chunk.Data = this.configuration.MemoryManager.AllocateCleanManagedByteBuffer(chunk.Length); - this.currentStream.Read(chunk.Data.Array, 0, chunk.Length); + IManagedByteBuffer buffer = this.configuration.MemoryManager.AllocateCleanManagedByteBuffer(length); + + this.currentStream.Read(buffer.Array, 0, length); + + return buffer; } /// /// Identifies the chunk type from the chunk. /// - /// The chunk. /// /// Thrown if the input stream is not valid. /// - private void ReadChunkType(PngChunk chunk) + private string ReadChunkType() { int numBytes = this.currentStream.Read(this.chunkTypeBuffer, 0, 4); if (numBytes >= 1 && numBytes <= 3) @@ -1309,7 +1308,7 @@ namespace SixLabors.ImageSharp.Formats.Png this.chars[2] = (char)this.chunkTypeBuffer[2]; this.chars[3] = (char)this.chunkTypeBuffer[3]; - chunk.Type = new string(this.chars); + return new string(this.chars); } /// From 09483f9d4b8325a10a47b45c30c2aa81e1062f0f Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Fri, 13 Apr 2018 10:14:55 -0700 Subject: [PATCH 02/13] Don't perform the CRC check on non-critical chunks --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index f4b045759..b27d9a965 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -1216,8 +1216,7 @@ namespace SixLabors.ImageSharp.Formats.Png string type = this.ReadChunkType(); - // NOTE: Handling the chunk data is the responsible of the caller - // It is currently either skipped (in identification) or read during decoding + // NOTE: Reading the chunk data is the responsible of the caller if (type == PngChunkTypes.Data) { chunk = new PngChunk(length, type); @@ -1231,7 +1230,10 @@ namespace SixLabors.ImageSharp.Formats.Png data: this.ReadChunkData(length), crc: this.ReadChunkCrc()); - this.ValidateChunk(chunk); + if (chunk.IsCritical) + { + this.ValidateChunk(chunk); + } return true; } @@ -1240,9 +1242,9 @@ namespace SixLabors.ImageSharp.Formats.Png { this.crc.Reset(); this.crc.Update(this.chunkTypeBuffer); - this.crc.Update(new ReadOnlySpan(chunk.Data.Array, 0, chunk.Length)); + this.crc.Update(chunk.Data.Span); - if (this.crc.Value != chunk.Crc && chunk.IsCritical) + if (this.crc.Value != chunk.Crc) { throw new ImageFormatException($"CRC Error. PNG {chunk.Type} chunk is corrupt!"); } From eabbc796fcf9b46830f2b7eed32bbc2f0da1426d Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Fri, 13 Apr 2018 10:26:03 -0700 Subject: [PATCH 03/13] Use new AsSpan overloads --- src/ImageSharp/Formats/Png/PngEncoderCore.cs | 13 +++--- .../Formats/Png/Zlib/ZlibDeflateStream.cs | 21 ++-------- src/ImageSharp/Image.LoadPixelData.cs | 2 +- src/ImageSharp/ImageExtensions.cs | 2 +- src/ImageSharp/Memory/BasicArrayBuffer.cs | 2 +- .../DataReader/IccDataReader.Primitives.cs | 12 +++--- .../ImageSharp.Tests/Formats/Jpg/DCTTests.cs | 42 +++++++++---------- 7 files changed, 39 insertions(+), 55 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index 676a93ee0..e8a42c0c8 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -5,7 +5,6 @@ using System; using System.Buffers.Binary; using System.IO; using System.Linq; -using System.Runtime.InteropServices; using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.Formats.Png.Filters; using SixLabors.ImageSharp.Formats.Png.Zlib; @@ -415,8 +414,8 @@ namespace SixLabors.ImageSharp.Formats.Png /// The . private void WriteHeaderChunk(Stream stream, in PngHeader header) { - BinaryPrimitives.WriteInt32BigEndian(new Span(this.chunkDataBuffer, 0, 4), header.Width); - BinaryPrimitives.WriteInt32BigEndian(new Span(this.chunkDataBuffer, 4, 4), header.Height); + BinaryPrimitives.WriteInt32BigEndian(this.chunkDataBuffer.AsSpan(0, 4), header.Width); + BinaryPrimitives.WriteInt32BigEndian(this.chunkDataBuffer.AsSpan(4, 4), header.Height); this.chunkDataBuffer[8] = header.BitDepth; this.chunkDataBuffer[9] = (byte)header.ColorType; @@ -500,8 +499,8 @@ namespace SixLabors.ImageSharp.Formats.Png int dpmX = (int)Math.Round(image.MetaData.HorizontalResolution * 39.3700787D); int dpmY = (int)Math.Round(image.MetaData.VerticalResolution * 39.3700787D); - BinaryPrimitives.WriteInt32BigEndian(new Span(this.chunkDataBuffer, 0, 4), dpmX); - BinaryPrimitives.WriteInt32BigEndian(new Span(this.chunkDataBuffer, 4, 4), dpmY); + BinaryPrimitives.WriteInt32BigEndian(this.chunkDataBuffer.AsSpan(0, 4), dpmX); + BinaryPrimitives.WriteInt32BigEndian(this.chunkDataBuffer.AsSpan(4, 4), dpmY); this.chunkDataBuffer[8] = 1; @@ -520,7 +519,7 @@ namespace SixLabors.ImageSharp.Formats.Png // 4-byte unsigned integer of gamma * 100,000. uint gammaValue = (uint)(this.gamma * 100_000F); - BinaryPrimitives.WriteUInt32BigEndian(new Span(this.chunkDataBuffer, 0, 4), gammaValue); + BinaryPrimitives.WriteUInt32BigEndian(this.chunkDataBuffer.AsSpan(0, 4), gammaValue); this.WriteChunk(stream, PngChunkTypes.Gamma, this.chunkDataBuffer, 0, 4); } @@ -643,7 +642,7 @@ namespace SixLabors.ImageSharp.Formats.Png { stream.Write(data, offset, length); - this.crc.Update(new ReadOnlySpan(data, offset, length)); + this.crc.Update(data.AsSpan(offset, length)); } BinaryPrimitives.WriteUInt32BigEndian(this.intBuffer, (uint)this.crc.Value); diff --git a/src/ImageSharp/Formats/Png/Zlib/ZlibDeflateStream.cs b/src/ImageSharp/Formats/Png/Zlib/ZlibDeflateStream.cs index 51e6b4859..8e0bac938 100644 --- a/src/ImageSharp/Formats/Png/Zlib/ZlibDeflateStream.cs +++ b/src/ImageSharp/Formats/Png/Zlib/ZlibDeflateStream.cs @@ -113,26 +113,13 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib public override bool CanWrite => true; /// - public override long Length - { - get - { - throw new NotSupportedException(); - } - } + public override long Length => throw new NotSupportedException(); /// public override long Position { - get - { - throw new NotSupportedException(); - } - - set - { - throw new NotSupportedException(); - } + get => throw new NotSupportedException(); + set => throw new NotSupportedException(); } /// @@ -163,7 +150,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib public override void Write(byte[] buffer, int offset, int count) { this.deflateStream.Write(buffer, offset, count); - this.adler32.Update(new ReadOnlySpan(buffer, offset, count)); + this.adler32.Update(buffer.AsSpan(offset, count)); } /// diff --git a/src/ImageSharp/Image.LoadPixelData.cs b/src/ImageSharp/Image.LoadPixelData.cs index b0bb03580..0179e62ac 100644 --- a/src/ImageSharp/Image.LoadPixelData.cs +++ b/src/ImageSharp/Image.LoadPixelData.cs @@ -99,7 +99,7 @@ namespace SixLabors.ImageSharp public static Image LoadPixelData(Configuration config, TPixel[] data, int width, int height) where TPixel : struct, IPixel { - return LoadPixelData(config, new Span(data), width, height); + return LoadPixelData(config, data.AsSpan(), width, height); } /// diff --git a/src/ImageSharp/ImageExtensions.cs b/src/ImageSharp/ImageExtensions.cs index 1f7e418ad..2cdb71fc0 100644 --- a/src/ImageSharp/ImageExtensions.cs +++ b/src/ImageSharp/ImageExtensions.cs @@ -143,7 +143,7 @@ namespace SixLabors.ImageSharp /// Thrown if the stream is null. public static void SavePixelData(this ImageFrame source, TPixel[] buffer) where TPixel : struct, IPixel - => SavePixelData(source, new Span(buffer)); + => SavePixelData(source, buffer.AsSpan()); /// /// Saves the raw image pixels to a byte array in row-major order. diff --git a/src/ImageSharp/Memory/BasicArrayBuffer.cs b/src/ImageSharp/Memory/BasicArrayBuffer.cs index a4810d037..dd2f7ef86 100644 --- a/src/ImageSharp/Memory/BasicArrayBuffer.cs +++ b/src/ImageSharp/Memory/BasicArrayBuffer.cs @@ -25,7 +25,7 @@ namespace SixLabors.ImageSharp.Memory public int Length { get; } - public Span Span => new Span(this.Array, 0, this.Length); + public Span Span => this.Array.AsSpan(0, this.Length); /// /// Returns a reference to specified element of the buffer. diff --git a/src/ImageSharp/MetaData/Profiles/ICC/DataReader/IccDataReader.Primitives.cs b/src/ImageSharp/MetaData/Profiles/ICC/DataReader/IccDataReader.Primitives.cs index 794d77ba1..482853b14 100644 --- a/src/ImageSharp/MetaData/Profiles/ICC/DataReader/IccDataReader.Primitives.cs +++ b/src/ImageSharp/MetaData/Profiles/ICC/DataReader/IccDataReader.Primitives.cs @@ -18,7 +18,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Icc /// the value public ushort ReadUInt16() { - return BinaryPrimitives.ReadUInt16BigEndian(new Span(this.data, this.AddIndex(2), 2)); + return BinaryPrimitives.ReadUInt16BigEndian(this.data.AsSpan(this.AddIndex(2), 2)); } /// @@ -27,7 +27,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Icc /// the value public short ReadInt16() { - return BinaryPrimitives.ReadInt16BigEndian(new Span(this.data, this.AddIndex(2), 2)); + return BinaryPrimitives.ReadInt16BigEndian(this.data.AsSpan(this.AddIndex(2), 2)); } /// @@ -36,7 +36,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Icc /// the value public uint ReadUInt32() { - return BinaryPrimitives.ReadUInt32BigEndian(new Span(this.data, this.AddIndex(4), 4)); + return BinaryPrimitives.ReadUInt32BigEndian(this.data.AsSpan(this.AddIndex(4), 4)); } /// @@ -45,7 +45,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Icc /// the value public int ReadInt32() { - return BinaryPrimitives.ReadInt32BigEndian(new Span(this.data, this.AddIndex(4), 4)); + return BinaryPrimitives.ReadInt32BigEndian(this.data.AsSpan(this.AddIndex(4), 4)); } /// @@ -54,7 +54,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Icc /// the value public ulong ReadUInt64() { - return BinaryPrimitives.ReadUInt64BigEndian(new Span(this.data, this.AddIndex(8), 8)); + return BinaryPrimitives.ReadUInt64BigEndian(this.data.AsSpan(this.AddIndex(8), 8)); } /// @@ -63,7 +63,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Icc /// the value public long ReadInt64() { - return BinaryPrimitives.ReadInt64BigEndian(new Span(this.data, this.AddIndex(8), 8)); + return BinaryPrimitives.ReadInt64BigEndian(this.data.AsSpan(this.AddIndex(8), 8)); } /// diff --git a/tests/ImageSharp.Tests/Formats/Jpg/DCTTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/DCTTests.cs index ee6f5305f..1c18df76c 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/DCTTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/DCTTests.cs @@ -1,15 +1,14 @@ // ReSharper disable InconsistentNaming -namespace SixLabors.ImageSharp.Tests.Formats.Jpg -{ - using System; +using System; - using SixLabors.ImageSharp.Formats.Jpeg.Common; - using SixLabors.ImageSharp.Formats.Jpeg.GolangPort.Components; - using SixLabors.ImageSharp.Tests.Formats.Jpg.Utils; +using SixLabors.ImageSharp.Formats.Jpeg.Common; +using SixLabors.ImageSharp.Tests.Formats.Jpg.Utils; - using Xunit; - using Xunit.Abstractions; +using Xunit; +using Xunit.Abstractions; +namespace SixLabors.ImageSharp.Tests.Formats.Jpg +{ public static class DCTTests { public class FastFloatingPoint : JpegFixture @@ -19,7 +18,6 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg { } - [Fact] public void iDCT2D8x4_LeftPart() { @@ -28,10 +26,10 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg ReferenceImplementations.LLM_FloatingPoint_DCT.iDCT2D8x4_32f(sourceArray, expectedDestArray); - Block8x8F source = new Block8x8F(); + var source = new Block8x8F(); source.LoadFrom(sourceArray); - Block8x8F dest = new Block8x8F(); + var dest = new Block8x8F(); FastFloatingPointDCT.IDCT8x4_LeftPart(ref source, ref dest); @@ -51,12 +49,12 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg float[] sourceArray = JpegFixture.Create8x8FloatData(); float[] expectedDestArray = new float[64]; - ReferenceImplementations.LLM_FloatingPoint_DCT.iDCT2D8x4_32f(sourceArray.AsSpan().Slice(4), expectedDestArray.AsSpan().Slice(4)); + ReferenceImplementations.LLM_FloatingPoint_DCT.iDCT2D8x4_32f(sourceArray.AsSpan(4), expectedDestArray.AsSpan(4)); - Block8x8F source = new Block8x8F(); + var source = new Block8x8F(); source.LoadFrom(sourceArray); - Block8x8F dest = new Block8x8F(); + var dest = new Block8x8F(); FastFloatingPointDCT.IDCT8x4_RightPart(ref source, ref dest); @@ -115,10 +113,10 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg public void FDCT8x4_LeftPart(int seed) { Span src = JpegFixture.Create8x8RoundedRandomFloatData(-200, 200, seed); - Block8x8F srcBlock = new Block8x8F(); + var srcBlock = new Block8x8F(); srcBlock.LoadFrom(src); - Block8x8F destBlock = new Block8x8F(); + var destBlock = new Block8x8F(); float[] expectedDest = new float[64]; @@ -137,14 +135,14 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg public void FDCT8x4_RightPart(int seed) { Span src = JpegFixture.Create8x8RoundedRandomFloatData(-200, 200, seed); - Block8x8F srcBlock = new Block8x8F(); + var srcBlock = new Block8x8F(); srcBlock.LoadFrom(src); - Block8x8F destBlock = new Block8x8F(); + var destBlock = new Block8x8F(); float[] expectedDest = new float[64]; - ReferenceImplementations.LLM_FloatingPoint_DCT.fDCT2D8x4_32f(src.Slice(4), expectedDest.AsSpan().Slice(4)); + ReferenceImplementations.LLM_FloatingPoint_DCT.fDCT2D8x4_32f(src.Slice(4), expectedDest.AsSpan(4)); FastFloatingPointDCT.FDCT8x4_RightPart(ref srcBlock, ref destBlock); float[] actualDest = new float[64]; @@ -159,14 +157,14 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg public void TransformFDCT(int seed) { Span src = JpegFixture.Create8x8RoundedRandomFloatData(-200, 200, seed); - Block8x8F srcBlock = new Block8x8F(); + var srcBlock = new Block8x8F(); srcBlock.LoadFrom(src); - Block8x8F destBlock = new Block8x8F(); + var destBlock = new Block8x8F(); float[] expectedDest = new float[64]; float[] temp1 = new float[64]; - Block8x8F temp2 = new Block8x8F(); + var temp2 = new Block8x8F(); ReferenceImplementations.LLM_FloatingPoint_DCT.fDCT2D_llm(src, expectedDest, temp1, downscaleBy8: true); FastFloatingPointDCT.TransformFDCT(ref srcBlock, ref destBlock, ref temp2, false); From c089c8cffc996bdd5446e907866c5336c88e2320 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Fri, 13 Apr 2018 16:36:30 -0700 Subject: [PATCH 04/13] Eliminate string allocations for PngChunkType --- src/ImageSharp/Formats/Png/PngChunk.cs | 15 +++--- src/ImageSharp/Formats/Png/PngChunkType.cs | 17 +++++++ ...{PngChunkTypes.cs => PngChunkTypeNames.cs} | 4 +- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 47 ++++++++----------- src/ImageSharp/Formats/Png/PngEncoderCore.cs | 36 ++++++-------- .../Formats/Png/PngDecoderTests.cs | 12 ++--- 6 files changed, 68 insertions(+), 63 deletions(-) create mode 100644 src/ImageSharp/Formats/Png/PngChunkType.cs rename src/ImageSharp/Formats/Png/{PngChunkTypes.cs => PngChunkTypeNames.cs} (95%) diff --git a/src/ImageSharp/Formats/Png/PngChunk.cs b/src/ImageSharp/Formats/Png/PngChunk.cs index 399bc95c9..89352ff7e 100644 --- a/src/ImageSharp/Formats/Png/PngChunk.cs +++ b/src/ImageSharp/Formats/Png/PngChunk.cs @@ -10,7 +10,7 @@ namespace SixLabors.ImageSharp.Formats.Png /// internal readonly struct PngChunk { - public PngChunk(int length, string type, IManagedByteBuffer data = null, uint crc = default) + public PngChunk(int length, PngChunkType type, IManagedByteBuffer data = null, uint crc = default) { this.Length = length; this.Type = type; @@ -27,9 +27,10 @@ namespace SixLabors.ImageSharp.Formats.Png public int Length { get; } /// - /// Gets the chunk type as string with 4 chars. + /// Gets the chunk type. + /// The chunk type value the UInt32BigEndian encoding of its 4 ASCII characters. /// - public string Type { get; } + public PngChunkType Type { get; } /// /// Gets the data bytes appropriate to the chunk type, if any. @@ -48,9 +49,9 @@ namespace SixLabors.ImageSharp.Formats.Png /// Gets a value indicating whether the given chunk is critical to decoding /// public bool IsCritical => - this.Type == PngChunkTypes.Header || - this.Type == PngChunkTypes.Palette || - this.Type == PngChunkTypes.Data || - this.Type == PngChunkTypes.End; + this.Type == PngChunkType.Header || + this.Type == PngChunkType.Palette || + this.Type == PngChunkType.Data || + this.Type == PngChunkType.End; } } diff --git a/src/ImageSharp/Formats/Png/PngChunkType.cs b/src/ImageSharp/Formats/Png/PngChunkType.cs new file mode 100644 index 000000000..14550e8ac --- /dev/null +++ b/src/ImageSharp/Formats/Png/PngChunkType.cs @@ -0,0 +1,17 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +namespace SixLabors.ImageSharp.Formats.Png +{ + internal enum PngChunkType : uint + { + Header = 1229472850U, // IHDR + Palette = 1347179589U, // PLTE + Data = 1229209940U, // IDAT + End = 1229278788U, // IEND + PaletteAlpha = 1951551059U, // tRNS + Text = 1950701684U, // tEXt + Gamma = 1732332865U, // gAMA + Physical = 1883789683U, // pHYs + } +} diff --git a/src/ImageSharp/Formats/Png/PngChunkTypes.cs b/src/ImageSharp/Formats/Png/PngChunkTypeNames.cs similarity index 95% rename from src/ImageSharp/Formats/Png/PngChunkTypes.cs rename to src/ImageSharp/Formats/Png/PngChunkTypeNames.cs index e22f4f0e7..f2864decd 100644 --- a/src/ImageSharp/Formats/Png/PngChunkTypes.cs +++ b/src/ImageSharp/Formats/Png/PngChunkTypeNames.cs @@ -4,9 +4,9 @@ namespace SixLabors.ImageSharp.Formats.Png { /// - /// Contains a list of possible chunk type identifiers. + /// Contains a list of possible chunk type identifier names. /// - internal static class PngChunkTypes + internal static class PngChunkTypeNames { /// /// The first chunk in a png file. Can only exists once. Contains diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index b27d9a965..053e9f712 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -71,11 +71,6 @@ namespace SixLabors.ImageSharp.Formats.Png /// private readonly byte[] crcBuffer = new byte[4]; - /// - /// Reusable buffer for reading char arrays. - /// - private readonly char[] chars = new char[4]; - /// /// Reusable crc for validating chunks. /// @@ -224,14 +219,14 @@ namespace SixLabors.ImageSharp.Formats.Png { switch (chunk.Type) { - case PngChunkTypes.Header: + case PngChunkType.Header: this.ReadHeaderChunk(chunk.Data.Array); this.ValidateHeader(); break; - case PngChunkTypes.Physical: + case PngChunkType.Physical: this.ReadPhysicalChunk(metadata, chunk.Data.Array); break; - case PngChunkTypes.Data: + case PngChunkType.Data: if (image == null) { this.InitializeImage(metadata, out image); @@ -241,21 +236,21 @@ namespace SixLabors.ImageSharp.Formats.Png this.ReadScanlines(deframeStream.CompressedStream, image.Frames.RootFrame); this.currentStream.Read(this.crcBuffer, 0, 4); break; - case PngChunkTypes.Palette: + case PngChunkType.Palette: byte[] pal = new byte[chunk.Length]; Buffer.BlockCopy(chunk.Data.Array, 0, pal, 0, chunk.Length); this.palette = pal; break; - case PngChunkTypes.PaletteAlpha: + case PngChunkType.PaletteAlpha: byte[] alpha = new byte[chunk.Length]; Buffer.BlockCopy(chunk.Data.Array, 0, alpha, 0, chunk.Length); this.paletteAlpha = alpha; this.AssignTransparentMarkers(alpha); break; - case PngChunkTypes.Text: + case PngChunkType.Text: this.ReadTextChunk(metadata, chunk.Data.Array, chunk.Length); break; - case PngChunkTypes.End: + case PngChunkType.End: this.isEndChunkReached = true; break; } @@ -298,20 +293,20 @@ namespace SixLabors.ImageSharp.Formats.Png { switch (chunk.Type) { - case PngChunkTypes.Header: + case PngChunkType.Header: this.ReadHeaderChunk(chunk.Data.Array); this.ValidateHeader(); break; - case PngChunkTypes.Physical: + case PngChunkType.Physical: this.ReadPhysicalChunk(metadata, chunk.Data.Array); break; - case PngChunkTypes.Data: + case PngChunkType.Data: this.SkipChunkDataAndCrc(chunk); break; - case PngChunkTypes.Text: + case PngChunkType.Text: this.ReadTextChunk(metadata, chunk.Data.Array, chunk.Length); break; - case PngChunkTypes.End: + case PngChunkType.End: this.isEndChunkReached = true; break; } @@ -1214,10 +1209,10 @@ namespace SixLabors.ImageSharp.Formats.Png } } - string type = this.ReadChunkType(); + PngChunkType type = this.ReadChunkType(); // NOTE: Reading the chunk data is the responsible of the caller - if (type == PngChunkTypes.Data) + if (type == PngChunkType.Data) { chunk = new PngChunk(length, type); @@ -1246,7 +1241,9 @@ namespace SixLabors.ImageSharp.Formats.Png if (this.crc.Value != chunk.Crc) { - throw new ImageFormatException($"CRC Error. PNG {chunk.Type} chunk is corrupt!"); + string chunkName = Encoding.UTF8.GetString(this.chunkTypeBuffer, 0, 4); + + throw new ImageFormatException($"CRC Error. PNG {chunkName} chunk is corrupt!"); } } @@ -1297,20 +1294,16 @@ namespace SixLabors.ImageSharp.Formats.Png /// /// Thrown if the input stream is not valid. /// - private string ReadChunkType() + private PngChunkType ReadChunkType() { int numBytes = this.currentStream.Read(this.chunkTypeBuffer, 0, 4); + if (numBytes >= 1 && numBytes <= 3) { throw new ImageFormatException("Image stream is not valid!"); } - this.chars[0] = (char)this.chunkTypeBuffer[0]; - this.chars[1] = (char)this.chunkTypeBuffer[1]; - this.chars[2] = (char)this.chunkTypeBuffer[2]; - this.chars[3] = (char)this.chunkTypeBuffer[3]; - - return new string(this.chars); + return (PngChunkType)BinaryPrimitives.ReadUInt32BigEndian(this.chunkTypeBuffer.AsSpan()); } /// diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index e8a42c0c8..96c6a6650 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -27,9 +27,9 @@ namespace SixLabors.ImageSharp.Formats.Png private const int MaxBlockSize = 65535; /// - /// Reusable buffer for writing chunk types. + /// Reusable buffer for writing general data. /// - private readonly byte[] chunkTypeBuffer = new byte[4]; + private readonly byte[] buffer = new byte[8]; /// /// Reusable buffer for writing chunk data. @@ -423,7 +423,7 @@ namespace SixLabors.ImageSharp.Formats.Png this.chunkDataBuffer[11] = header.FilterMethod; this.chunkDataBuffer[12] = (byte)header.InterlaceMethod; - this.WriteChunk(stream, PngChunkTypes.Header, this.chunkDataBuffer, 0, 13); + this.WriteChunk(stream, PngChunkType.Header, this.chunkDataBuffer, 0, 13); } /// @@ -474,12 +474,12 @@ namespace SixLabors.ImageSharp.Formats.Png } } - this.WriteChunk(stream, PngChunkTypes.Palette, colorTable.Array, 0, colorTableLength); + this.WriteChunk(stream, PngChunkType.Palette, colorTable.Array, 0, colorTableLength); // Write the transparency data if (anyAlpha) { - this.WriteChunk(stream, PngChunkTypes.PaletteAlpha, alphaTable.Array, 0, pixelCount); + this.WriteChunk(stream, PngChunkType.PaletteAlpha, alphaTable.Array, 0, pixelCount); } } } @@ -504,7 +504,7 @@ namespace SixLabors.ImageSharp.Formats.Png this.chunkDataBuffer[8] = 1; - this.WriteChunk(stream, PngChunkTypes.Physical, this.chunkDataBuffer, 0, 9); + this.WriteChunk(stream, PngChunkType.Physical, this.chunkDataBuffer, 0, 9); } } @@ -521,7 +521,7 @@ namespace SixLabors.ImageSharp.Formats.Png BinaryPrimitives.WriteUInt32BigEndian(this.chunkDataBuffer.AsSpan(0, 4), gammaValue); - this.WriteChunk(stream, PngChunkTypes.Gamma, this.chunkDataBuffer, 0, 4); + this.WriteChunk(stream, PngChunkType.Gamma, this.chunkDataBuffer, 0, 4); } } @@ -589,7 +589,7 @@ namespace SixLabors.ImageSharp.Formats.Png length = MaxBlockSize; } - this.WriteChunk(stream, PngChunkTypes.Data, buffer, i * MaxBlockSize, length); + this.WriteChunk(stream, PngChunkType.Data, buffer, i * MaxBlockSize, length); } } @@ -599,7 +599,7 @@ namespace SixLabors.ImageSharp.Formats.Png /// The containing image data. private void WriteEndChunk(Stream stream) { - this.WriteChunk(stream, PngChunkTypes.End, null); + this.WriteChunk(stream, PngChunkType.End, null); } /// @@ -608,7 +608,7 @@ namespace SixLabors.ImageSharp.Formats.Png /// The to write to. /// The type of chunk to write. /// The containing data. - private void WriteChunk(Stream stream, string type, byte[] data) + private void WriteChunk(Stream stream, PngChunkType type, byte[] data) { this.WriteChunk(stream, type, data, 0, data?.Length ?? 0); } @@ -621,22 +621,16 @@ namespace SixLabors.ImageSharp.Formats.Png /// The containing data. /// The position to offset the data at. /// The of the data to write. - private void WriteChunk(Stream stream, string type, byte[] data, int offset, int length) + private void WriteChunk(Stream stream, PngChunkType type, byte[] data, int offset, int length) { - BinaryPrimitives.WriteInt32BigEndian(this.intBuffer, length); + BinaryPrimitives.WriteInt32BigEndian(this.buffer, length); + BinaryPrimitives.WriteUInt32BigEndian(this.buffer.AsSpan(4, 4), (uint)type); - stream.Write(this.intBuffer, 0, 4); // write the length - - this.chunkTypeBuffer[0] = (byte)type[0]; - this.chunkTypeBuffer[1] = (byte)type[1]; - this.chunkTypeBuffer[2] = (byte)type[2]; - this.chunkTypeBuffer[3] = (byte)type[3]; - - stream.Write(this.chunkTypeBuffer, 0, 4); + stream.Write(this.buffer, 0, 8); this.crc.Reset(); - this.crc.Update(this.chunkTypeBuffer); + this.crc.Update(this.buffer.AsSpan(4, 4)); // Write the type buffer if (data != null && length > 0) { diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 1de4e1646..85430fea9 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -242,10 +242,10 @@ namespace SixLabors.ImageSharp.Tests } [Theory] - [InlineData(PngChunkTypes.Header)] - [InlineData(PngChunkTypes.Palette)] + [InlineData(PngChunkTypeNames.Header)] + [InlineData(PngChunkTypeNames.Palette)] // [InlineData(PngChunkTypes.Data)] //TODO: Figure out how to test this - [InlineData(PngChunkTypes.End)] + [InlineData(PngChunkTypeNames.End)] public void Decode_IncorrectCRCForCriticalChunk_ExceptionIsThrown(string chunkName) { using (var memStream = new MemoryStream()) @@ -266,9 +266,9 @@ namespace SixLabors.ImageSharp.Tests } [Theory] - [InlineData(PngChunkTypes.Gamma)] - [InlineData(PngChunkTypes.PaletteAlpha)] - [InlineData(PngChunkTypes.Physical)] // It's ok to test physical as we don't throw for duplicate chunks. + [InlineData(PngChunkTypeNames.Gamma)] + [InlineData(PngChunkTypeNames.PaletteAlpha)] + [InlineData(PngChunkTypeNames.Physical)] // It's ok to test physical as we don't throw for duplicate chunks. //[InlineData(PngChunkTypes.Text)] //TODO: Figure out how to test this public void Decode_IncorrectCRCForNonCriticalChunk_ExceptionIsThrown(string chunkName) { From 595daff9cd4bd2e0851ce5044962acfd80686f94 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Fri, 13 Apr 2018 16:53:44 -0700 Subject: [PATCH 05/13] Add test to ensure the chunk type values are correct --- .../Formats/Png/PngChunkTests.cs | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 tests/ImageSharp.Tests/Formats/Png/PngChunkTests.cs diff --git a/tests/ImageSharp.Tests/Formats/Png/PngChunkTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngChunkTests.cs new file mode 100644 index 000000000..3d1da000b --- /dev/null +++ b/tests/ImageSharp.Tests/Formats/Png/PngChunkTests.cs @@ -0,0 +1,29 @@ +using System; +using System.Buffers.Binary; +using System.Text; +using SixLabors.ImageSharp.Formats.Png; +using Xunit; + +namespace SixLabors.ImageSharp.Tests.Formats.Png +{ + public class PngChunkTests + { + [Fact] + public void ChunkTypeIdsAreCorrect() + { + Assert.Equal(PngChunkType.Header, GetType("IHDR")); + Assert.Equal(PngChunkType.Palette, GetType("PLTE")); + Assert.Equal(PngChunkType.Data, GetType("IDAT")); + Assert.Equal(PngChunkType.End, GetType("IEND")); + Assert.Equal(PngChunkType.PaletteAlpha, GetType("tRNS")); + Assert.Equal(PngChunkType.Text, GetType("tEXt")); + Assert.Equal(PngChunkType.Gamma, GetType("gAMA")); + Assert.Equal(PngChunkType.Physical, GetType("pHYs")); + } + + private static PngChunkType GetType(string text) + { + return (PngChunkType)BinaryPrimitives.ReadInt32BigEndian(Encoding.UTF8.GetBytes(text).AsSpan()); + } + } +} From 30d9b322877cbdfa34e0fbd64c4c999e7d7f00dd Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Fri, 13 Apr 2018 17:05:55 -0700 Subject: [PATCH 06/13] Cleanup --- src/ImageSharp/Formats/Png/PngChunk.cs | 4 ++-- src/ImageSharp/Formats/Png/PngChunkType.cs | 3 +++ src/ImageSharp/Formats/Png/PngDecoderCore.cs | 4 ++-- tests/ImageSharp.Tests/Formats/Png/PngChunkTests.cs | 4 ++-- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngChunk.cs b/src/ImageSharp/Formats/Png/PngChunk.cs index 89352ff7e..2566492f4 100644 --- a/src/ImageSharp/Formats/Png/PngChunk.cs +++ b/src/ImageSharp/Formats/Png/PngChunk.cs @@ -10,7 +10,7 @@ namespace SixLabors.ImageSharp.Formats.Png /// internal readonly struct PngChunk { - public PngChunk(int length, PngChunkType type, IManagedByteBuffer data = null, uint crc = default) + public PngChunk(int length, PngChunkType type, IManagedByteBuffer data = null, uint crc = 0) { this.Length = length; this.Type = type; @@ -28,7 +28,7 @@ namespace SixLabors.ImageSharp.Formats.Png /// /// Gets the chunk type. - /// The chunk type value the UInt32BigEndian encoding of its 4 ASCII characters. + /// The value is the equal to the UInt32BigEndian encoding of its 4 ASCII characters. /// public PngChunkType Type { get; } diff --git a/src/ImageSharp/Formats/Png/PngChunkType.cs b/src/ImageSharp/Formats/Png/PngChunkType.cs index 14550e8ac..f2dae5c67 100644 --- a/src/ImageSharp/Formats/Png/PngChunkType.cs +++ b/src/ImageSharp/Formats/Png/PngChunkType.cs @@ -3,6 +3,9 @@ namespace SixLabors.ImageSharp.Formats.Png { + /// + /// Contains a list of possible chunk types. + /// internal enum PngChunkType : uint { Header = 1229472850U, // IHDR diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 053e9f712..4230984e7 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -1241,9 +1241,9 @@ namespace SixLabors.ImageSharp.Formats.Png if (this.crc.Value != chunk.Crc) { - string chunkName = Encoding.UTF8.GetString(this.chunkTypeBuffer, 0, 4); + string chunkTypeName = Encoding.UTF8.GetString(this.chunkTypeBuffer, 0, 4); - throw new ImageFormatException($"CRC Error. PNG {chunkName} chunk is corrupt!"); + throw new ImageFormatException($"CRC Error. PNG {chunkTypeName} chunk is corrupt!"); } } diff --git a/tests/ImageSharp.Tests/Formats/Png/PngChunkTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngChunkTests.cs index 3d1da000b..687548963 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngChunkTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngChunkTests.cs @@ -23,7 +23,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png private static PngChunkType GetType(string text) { - return (PngChunkType)BinaryPrimitives.ReadInt32BigEndian(Encoding.UTF8.GetBytes(text).AsSpan()); + return (PngChunkType)BinaryPrimitives.ReadInt32BigEndian(Encoding.UTF8.GetBytes(text)); } } -} +} \ No newline at end of file From ae00b2b4e15fa4748e27e839dd9266f4e4de4d24 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Fri, 13 Apr 2018 17:11:58 -0700 Subject: [PATCH 07/13] Remove the intBuffer from PngEncoder --- src/ImageSharp/Formats/Png/PngEncoderCore.cs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index 96c6a6650..892b00ea9 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -36,11 +36,6 @@ namespace SixLabors.ImageSharp.Formats.Png /// private readonly byte[] chunkDataBuffer = new byte[16]; - /// - /// Reusable buffer for writing int data. - /// - private readonly byte[] intBuffer = new byte[4]; - /// /// Reusable crc for validating chunks. /// @@ -442,7 +437,7 @@ namespace SixLabors.ImageSharp.Formats.Png // Get max colors for bit depth. int colorTableLength = (int)Math.Pow(2, header.BitDepth) * 3; - var rgba = default(Rgba32); + Rgba32 rgba = default; bool anyAlpha = false; using (IManagedByteBuffer colorTable = this.memoryManager.AllocateManagedByteBuffer(colorTableLength)) @@ -639,9 +634,9 @@ namespace SixLabors.ImageSharp.Formats.Png this.crc.Update(data.AsSpan(offset, length)); } - BinaryPrimitives.WriteUInt32BigEndian(this.intBuffer, (uint)this.crc.Value); + BinaryPrimitives.WriteUInt32BigEndian(this.buffer, (uint)this.crc.Value); - stream.Write(this.intBuffer, 0, 4); // write the crc + stream.Write(this.buffer, 0, 4); // write the crc } } } \ No newline at end of file From 55366d3258f0a8f2657bb71e86a47adabd938502 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Mon, 16 Apr 2018 12:53:07 -0700 Subject: [PATCH 08/13] Remove PngChunkTypeNames --- src/ImageSharp/Formats/Png/PngChunkType.cs | 42 ++++++++++++- .../Formats/Png/PngChunkTypeNames.cs | 60 ------------------- ...{PngChunkTests.cs => PngChunkTypeTests.cs} | 2 +- .../Formats/Png/PngDecoderTests.cs | 12 ++-- 4 files changed, 48 insertions(+), 68 deletions(-) delete mode 100644 src/ImageSharp/Formats/Png/PngChunkTypeNames.cs rename tests/ImageSharp.Tests/Formats/Png/{PngChunkTests.cs => PngChunkTypeTests.cs} (96%) diff --git a/src/ImageSharp/Formats/Png/PngChunkType.cs b/src/ImageSharp/Formats/Png/PngChunkType.cs index f2dae5c67..e26e7e1e8 100644 --- a/src/ImageSharp/Formats/Png/PngChunkType.cs +++ b/src/ImageSharp/Formats/Png/PngChunkType.cs @@ -4,17 +4,57 @@ namespace SixLabors.ImageSharp.Formats.Png { /// - /// Contains a list of possible chunk types. + /// Contains a list of of chunk types. /// internal enum PngChunkType : uint { + /// + /// The first chunk in a png file. Can only exists once. Contains + /// common information like the width and the height of the image or + /// the used compression method. + /// Header = 1229472850U, // IHDR + + /// + /// The PLTE chunk contains from 1 to 256 palette entries, each a three byte + /// series in the RGB format. + /// Palette = 1347179589U, // PLTE + + /// + /// The IDAT chunk contains the actual image data. The image can contains more + /// than one chunk of this type. All chunks together are the whole image. + /// Data = 1229209940U, // IDAT + + /// + /// This chunk must appear last. It marks the end of the PNG data stream. + /// The chunk's data field is empty. + /// End = 1229278788U, // IEND + + /// + /// This chunk specifies that the image uses simple transparency: + /// either alpha values associated with palette entries (for indexed-color images) + /// or a single transparent color (for grayscale and true color images). + /// PaletteAlpha = 1951551059U, // tRNS + + /// + /// Textual information that the encoder wishes to record with the image can be stored in + /// tEXt chunks. Each tEXt chunk contains a keyword and a text string. + /// Text = 1950701684U, // tEXt + + /// + /// This chunk specifies the relationship between the image samples and the desired + /// display output intensity. + /// Gamma = 1732332865U, // gAMA + + /// + /// The pHYs chunk specifies the intended pixel size or aspect ratio for display of the image. + /// Physical = 1883789683U, // pHYs } } diff --git a/src/ImageSharp/Formats/Png/PngChunkTypeNames.cs b/src/ImageSharp/Formats/Png/PngChunkTypeNames.cs deleted file mode 100644 index f2864decd..000000000 --- a/src/ImageSharp/Formats/Png/PngChunkTypeNames.cs +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright (c) Six Labors and contributors. -// Licensed under the Apache License, Version 2.0. - -namespace SixLabors.ImageSharp.Formats.Png -{ - /// - /// Contains a list of possible chunk type identifier names. - /// - internal static class PngChunkTypeNames - { - /// - /// The first chunk in a png file. Can only exists once. Contains - /// common information like the width and the height of the image or - /// the used compression method. - /// - public const string Header = "IHDR"; - - /// - /// The PLTE chunk contains from 1 to 256 palette entries, each a three byte - /// series in the RGB format. - /// - public const string Palette = "PLTE"; - - /// - /// The IDAT chunk contains the actual image data. The image can contains more - /// than one chunk of this type. All chunks together are the whole image. - /// - public const string Data = "IDAT"; - - /// - /// This chunk must appear last. It marks the end of the PNG data stream. - /// The chunk's data field is empty. - /// - public const string End = "IEND"; - - /// - /// This chunk specifies that the image uses simple transparency: - /// either alpha values associated with palette entries (for indexed-color images) - /// or a single transparent color (for grayscale and true color images). - /// - public const string PaletteAlpha = "tRNS"; - - /// - /// Textual information that the encoder wishes to record with the image can be stored in - /// tEXt chunks. Each tEXt chunk contains a keyword and a text string. - /// - public const string Text = "tEXt"; - - /// - /// This chunk specifies the relationship between the image samples and the desired - /// display output intensity. - /// - public const string Gamma = "gAMA"; - - /// - /// The pHYs chunk specifies the intended pixel size or aspect ratio for display of the image. - /// - public const string Physical = "pHYs"; - } -} diff --git a/tests/ImageSharp.Tests/Formats/Png/PngChunkTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngChunkTypeTests.cs similarity index 96% rename from tests/ImageSharp.Tests/Formats/Png/PngChunkTests.cs rename to tests/ImageSharp.Tests/Formats/Png/PngChunkTypeTests.cs index 687548963..016c932dd 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngChunkTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngChunkTypeTests.cs @@ -6,7 +6,7 @@ using Xunit; namespace SixLabors.ImageSharp.Tests.Formats.Png { - public class PngChunkTests + public class PngChunkTypeTests { [Fact] public void ChunkTypeIdsAreCorrect() diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 85430fea9..7adfa3a3a 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -242,10 +242,10 @@ namespace SixLabors.ImageSharp.Tests } [Theory] - [InlineData(PngChunkTypeNames.Header)] - [InlineData(PngChunkTypeNames.Palette)] + [InlineData("IHDR")] // Header + [InlineData("PLTE")] // Palette // [InlineData(PngChunkTypes.Data)] //TODO: Figure out how to test this - [InlineData(PngChunkTypeNames.End)] + [InlineData("IEND")] // End public void Decode_IncorrectCRCForCriticalChunk_ExceptionIsThrown(string chunkName) { using (var memStream = new MemoryStream()) @@ -266,9 +266,9 @@ namespace SixLabors.ImageSharp.Tests } [Theory] - [InlineData(PngChunkTypeNames.Gamma)] - [InlineData(PngChunkTypeNames.PaletteAlpha)] - [InlineData(PngChunkTypeNames.Physical)] // It's ok to test physical as we don't throw for duplicate chunks. + [InlineData("gAMA")] // Gamma + [InlineData("tRNS")] // PaletteAlpha + [InlineData("pHYs")] // Pysical: It's ok to test physical as we don't throw for duplicate chunks. //[InlineData(PngChunkTypes.Text)] //TODO: Figure out how to test this public void Decode_IncorrectCRCForNonCriticalChunk_ExceptionIsThrown(string chunkName) { From fc6ceb8565300b58ecc0fce8510e07b52de1fcc1 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Tue, 17 Apr 2018 13:44:28 -0700 Subject: [PATCH 09/13] Update PngChunkType values to hex --- src/ImageSharp/Formats/Png/PngChunkType.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngChunkType.cs b/src/ImageSharp/Formats/Png/PngChunkType.cs index e26e7e1e8..c81c35b9b 100644 --- a/src/ImageSharp/Formats/Png/PngChunkType.cs +++ b/src/ImageSharp/Formats/Png/PngChunkType.cs @@ -13,48 +13,48 @@ namespace SixLabors.ImageSharp.Formats.Png /// common information like the width and the height of the image or /// the used compression method. /// - Header = 1229472850U, // IHDR + Header = 0x49484452U, // IHDR /// /// The PLTE chunk contains from 1 to 256 palette entries, each a three byte /// series in the RGB format. /// - Palette = 1347179589U, // PLTE + Palette = 0x504C5445U, // PLTE /// /// The IDAT chunk contains the actual image data. The image can contains more /// than one chunk of this type. All chunks together are the whole image. /// - Data = 1229209940U, // IDAT + Data = 0x49444154U, // IDAT /// /// This chunk must appear last. It marks the end of the PNG data stream. /// The chunk's data field is empty. /// - End = 1229278788U, // IEND + End = 0x49454E44U, // IEND /// /// This chunk specifies that the image uses simple transparency: /// either alpha values associated with palette entries (for indexed-color images) /// or a single transparent color (for grayscale and true color images). /// - PaletteAlpha = 1951551059U, // tRNS + PaletteAlpha = 0x74524E53U, // tRNS /// /// Textual information that the encoder wishes to record with the image can be stored in /// tEXt chunks. Each tEXt chunk contains a keyword and a text string. /// - Text = 1950701684U, // tEXt + Text = 0x74455874U, // tEXt /// /// This chunk specifies the relationship between the image samples and the desired /// display output intensity. /// - Gamma = 1732332865U, // gAMA + Gamma = 0x67414D41U, // gAMA /// /// The pHYs chunk specifies the intended pixel size or aspect ratio for display of the image. /// - Physical = 1883789683U, // pHYs + Physical = 0x70485973U // pHYs } } From b460b140068cee53a8981eaf07b3ae53ee6845e4 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Tue, 17 Apr 2018 13:44:57 -0700 Subject: [PATCH 10/13] Use PngChunkType enum in TestData --- .../Formats/Png/PngDecoderTests.cs | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 7adfa3a3a..4dbd214a5 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -11,6 +11,7 @@ using Xunit; namespace SixLabors.ImageSharp.Tests { + using System.Buffers.Binary; using System.Linq; using SixLabors.ImageSharp.Formats.Png; @@ -242,12 +243,14 @@ namespace SixLabors.ImageSharp.Tests } [Theory] - [InlineData("IHDR")] // Header - [InlineData("PLTE")] // Palette + [InlineData((uint)PngChunkType.Header)] // IHDR + [InlineData((uint)PngChunkType.Palette)] // PLTE // [InlineData(PngChunkTypes.Data)] //TODO: Figure out how to test this - [InlineData("IEND")] // End - public void Decode_IncorrectCRCForCriticalChunk_ExceptionIsThrown(string chunkName) + [InlineData((uint)PngChunkType.End)] // IEND + public void Decode_IncorrectCRCForCriticalChunk_ExceptionIsThrown(uint chunkType) { + string chunkName = GetChunkTypeName(chunkType); + using (var memStream = new MemoryStream()) { WriteHeaderChunk(memStream); @@ -266,12 +269,14 @@ namespace SixLabors.ImageSharp.Tests } [Theory] - [InlineData("gAMA")] // Gamma - [InlineData("tRNS")] // PaletteAlpha - [InlineData("pHYs")] // Pysical: It's ok to test physical as we don't throw for duplicate chunks. + [InlineData((uint)PngChunkType.Gamma)] // gAMA + [InlineData((uint)PngChunkType.PaletteAlpha)] // tRNS + [InlineData((uint)PngChunkType.Physical)] // pHYs: It's ok to test physical as we don't throw for duplicate chunks. //[InlineData(PngChunkTypes.Text)] //TODO: Figure out how to test this - public void Decode_IncorrectCRCForNonCriticalChunk_ExceptionIsThrown(string chunkName) + public void Decode_IncorrectCRCForNonCriticalChunk_ExceptionIsThrown(uint chunkType) { + string chunkName = GetChunkTypeName(chunkType); + using (var memStream = new MemoryStream()) { WriteHeaderChunk(memStream); @@ -283,6 +288,15 @@ namespace SixLabors.ImageSharp.Tests } } + private static string GetChunkTypeName(uint value) + { + byte[] data = new byte[4]; + + BinaryPrimitives.WriteUInt32BigEndian(data, value); + + return Encoding.ASCII.GetString(data); + } + private static void WriteHeaderChunk(MemoryStream memStream) { // Writes a 1x1 32bit png header chunk containing a single black pixel From c77f612aabfc6d522ca1a3683a970eeddc3a5534 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Tue, 17 Apr 2018 13:59:41 -0700 Subject: [PATCH 11/13] Move the png header to constants --- src/ImageSharp/Formats/Png/PngConstants.cs | 11 +++++++++++ src/ImageSharp/Formats/Png/PngEncoderCore.cs | 12 +----------- .../ImageSharp.Tests/Formats/Png/PngDecoderTests.cs | 2 -- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngConstants.cs b/src/ImageSharp/Formats/Png/PngConstants.cs index 8b4ad39f2..3b3b02884 100644 --- a/src/ImageSharp/Formats/Png/PngConstants.cs +++ b/src/ImageSharp/Formats/Png/PngConstants.cs @@ -25,5 +25,16 @@ namespace SixLabors.ImageSharp.Formats.Png /// The list of file extensions that equate to a png. /// public static readonly IEnumerable FileExtensions = new[] { "png" }; + + public static readonly byte[] HeaderBytes = { + 0x89, // Set the high bit. + 0x50, // P + 0x4E, // N + 0x47, // G + 0x0D, // Line ending CRLF + 0x0A, // Line ending CRLF + 0x1A, // EOF + 0x0A // LF + }; } } \ No newline at end of file diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index 892b00ea9..777ee1f54 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -167,17 +167,7 @@ namespace SixLabors.ImageSharp.Formats.Png this.width = image.Width; this.height = image.Height; - // Write the png header. - this.chunkDataBuffer[0] = 0x89; // Set the high bit. - this.chunkDataBuffer[1] = 0x50; // P - this.chunkDataBuffer[2] = 0x4E; // N - this.chunkDataBuffer[3] = 0x47; // G - this.chunkDataBuffer[4] = 0x0D; // Line ending CRLF - this.chunkDataBuffer[5] = 0x0A; // Line ending CRLF - this.chunkDataBuffer[6] = 0x1A; // EOF - this.chunkDataBuffer[7] = 0x0A; // LF - - stream.Write(this.chunkDataBuffer, 0, 8); + stream.Write(PngConstants.HeaderBytes, 0, PngConstants.HeaderBytes.Length); QuantizedFrame quantized = null; if (this.pngColorType == PngColorType.Palette) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 4dbd214a5..f97e115b7 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -2,9 +2,7 @@ // Licensed under the Apache License, Version 2.0. using System.IO; -using System.IO.Compression; using System.Text; -using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.PixelFormats; using Xunit; // ReSharper disable InconsistentNaming From 733f76763a59eb8681598a50e9fc39f89cce7137 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Tue, 17 Apr 2018 14:15:55 -0700 Subject: [PATCH 12/13] Optimize png format detection --- src/ImageSharp/Formats/Png/PngConstants.cs | 5 +++++ src/ImageSharp/Formats/Png/PngImageFormatDetector.cs | 12 ++---------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngConstants.cs b/src/ImageSharp/Formats/Png/PngConstants.cs index 3b3b02884..ff25e26b7 100644 --- a/src/ImageSharp/Formats/Png/PngConstants.cs +++ b/src/ImageSharp/Formats/Png/PngConstants.cs @@ -36,5 +36,10 @@ namespace SixLabors.ImageSharp.Formats.Png 0x1A, // EOF 0x0A // LF }; + + /// + /// The header bytes as a big endian coded ulong. + /// + public const ulong HeaderValue = 0x89504E470D0A1A0AUL; } } \ No newline at end of file diff --git a/src/ImageSharp/Formats/Png/PngImageFormatDetector.cs b/src/ImageSharp/Formats/Png/PngImageFormatDetector.cs index 837a147ed..36b43a470 100644 --- a/src/ImageSharp/Formats/Png/PngImageFormatDetector.cs +++ b/src/ImageSharp/Formats/Png/PngImageFormatDetector.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. using System; +using System.Buffers.Binary; namespace SixLabors.ImageSharp.Formats.Png { @@ -26,16 +27,7 @@ namespace SixLabors.ImageSharp.Formats.Png private bool IsSupportedFileFormat(ReadOnlySpan header) { - // TODO: This should be in constants - return header.Length >= this.HeaderSize && - header[0] == 0x89 && - header[1] == 0x50 && // P - header[2] == 0x4E && // N - header[3] == 0x47 && // G - header[4] == 0x0D && // CR - header[5] == 0x0A && // LF - header[6] == 0x1A && // EOF - header[7] == 0x0A; // LF + return header.Length >= this.HeaderSize && BinaryPrimitives.ReadUInt64BigEndian(header) == PngConstants.HeaderValue; } } } \ No newline at end of file From 13c81138cd2d33fa35a468b1c09dcb867e87870d Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Tue, 17 Apr 2018 14:18:01 -0700 Subject: [PATCH 13/13] Remove empty lines and unused using statements --- src/ImageSharp/Formats/Png/IPngDecoderOptions.cs | 6 +----- src/ImageSharp/Formats/Png/IPngEncoderOptions.cs | 2 +- src/ImageSharp/Formats/Png/ImageExtensions.cs | 2 +- src/ImageSharp/Formats/Png/PngChunk.cs | 2 +- src/ImageSharp/Formats/Png/PngChunkType.cs | 2 +- src/ImageSharp/Formats/Png/PngDecoder.cs | 2 +- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 2 +- src/ImageSharp/Formats/Png/Zlib/Adler32.cs | 5 +---- src/ImageSharp/Formats/Png/Zlib/IChecksum.cs | 5 +---- 9 files changed, 9 insertions(+), 19 deletions(-) diff --git a/src/ImageSharp/Formats/Png/IPngDecoderOptions.cs b/src/ImageSharp/Formats/Png/IPngDecoderOptions.cs index e51cc084b..bd0b93205 100644 --- a/src/ImageSharp/Formats/Png/IPngDecoderOptions.cs +++ b/src/ImageSharp/Formats/Png/IPngDecoderOptions.cs @@ -1,11 +1,7 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. -using System; -using System.Collections.Generic; -using System.IO; using System.Text; -using SixLabors.ImageSharp.PixelFormats; namespace SixLabors.ImageSharp.Formats.Png { @@ -24,4 +20,4 @@ namespace SixLabors.ImageSharp.Formats.Png /// Encoding TextEncoding { get; } } -} +} \ No newline at end of file diff --git a/src/ImageSharp/Formats/Png/IPngEncoderOptions.cs b/src/ImageSharp/Formats/Png/IPngEncoderOptions.cs index 1bfa4b063..3f48c4e26 100644 --- a/src/ImageSharp/Formats/Png/IPngEncoderOptions.cs +++ b/src/ImageSharp/Formats/Png/IPngEncoderOptions.cs @@ -45,4 +45,4 @@ namespace SixLabors.ImageSharp.Formats.Png /// bool WriteGamma { get; } } -} +} \ No newline at end of file diff --git a/src/ImageSharp/Formats/Png/ImageExtensions.cs b/src/ImageSharp/Formats/Png/ImageExtensions.cs index f25d2bffe..a65845e02 100644 --- a/src/ImageSharp/Formats/Png/ImageExtensions.cs +++ b/src/ImageSharp/Formats/Png/ImageExtensions.cs @@ -37,4 +37,4 @@ namespace SixLabors.ImageSharp where TPixel : struct, IPixel => source.Save(stream, encoder ?? source.GetConfiguration().ImageFormatsManager.FindEncoder(ImageFormats.Png)); } -} +} \ No newline at end of file diff --git a/src/ImageSharp/Formats/Png/PngChunk.cs b/src/ImageSharp/Formats/Png/PngChunk.cs index 2566492f4..c91f39d7f 100644 --- a/src/ImageSharp/Formats/Png/PngChunk.cs +++ b/src/ImageSharp/Formats/Png/PngChunk.cs @@ -54,4 +54,4 @@ namespace SixLabors.ImageSharp.Formats.Png this.Type == PngChunkType.Data || this.Type == PngChunkType.End; } -} +} \ No newline at end of file diff --git a/src/ImageSharp/Formats/Png/PngChunkType.cs b/src/ImageSharp/Formats/Png/PngChunkType.cs index c81c35b9b..51adc162b 100644 --- a/src/ImageSharp/Formats/Png/PngChunkType.cs +++ b/src/ImageSharp/Formats/Png/PngChunkType.cs @@ -57,4 +57,4 @@ namespace SixLabors.ImageSharp.Formats.Png /// Physical = 0x70485973U // pHYs } -} +} \ No newline at end of file diff --git a/src/ImageSharp/Formats/Png/PngDecoder.cs b/src/ImageSharp/Formats/Png/PngDecoder.cs index 57d45ecdd..39dfb1d0b 100644 --- a/src/ImageSharp/Formats/Png/PngDecoder.cs +++ b/src/ImageSharp/Formats/Png/PngDecoder.cs @@ -60,4 +60,4 @@ namespace SixLabors.ImageSharp.Formats.Png return decoder.Identify(stream); } } -} +} \ No newline at end of file diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 4230984e7..50511611f 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -1353,4 +1353,4 @@ namespace SixLabors.ImageSharp.Formats.Png this.scanline = temp; } } -} +} \ No newline at end of file diff --git a/src/ImageSharp/Formats/Png/Zlib/Adler32.cs b/src/ImageSharp/Formats/Png/Zlib/Adler32.cs index 9c4e9e4b9..a06983b9e 100644 --- a/src/ImageSharp/Formats/Png/Zlib/Adler32.cs +++ b/src/ImageSharp/Formats/Png/Zlib/Adler32.cs @@ -78,10 +78,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib public long Value { [MethodImpl(MethodImplOptions.AggressiveInlining)] - get - { - return this.checksum; - } + get => this.checksum; } /// diff --git a/src/ImageSharp/Formats/Png/Zlib/IChecksum.cs b/src/ImageSharp/Formats/Png/Zlib/IChecksum.cs index a2a57332b..da5deb49e 100644 --- a/src/ImageSharp/Formats/Png/Zlib/IChecksum.cs +++ b/src/ImageSharp/Formats/Png/Zlib/IChecksum.cs @@ -17,10 +17,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib /// /// Gets the data checksum computed so far. /// - long Value - { - get; - } + long Value { get; } /// /// Resets the data checksum as if no update was ever called.