From cd71a63113988ea03cd5ae1a45de3f1d40d6c494 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Fri, 24 Nov 2017 03:00:44 +0100 Subject: [PATCH 1/3] fix #385 + add test --- .../Jpeg/GolangPort/Components/Decoder/OrigJpegScanDecoder.cs | 3 ++- tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs | 2 ++ tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/External | 2 +- .../Images/Input/Jpg/issues/Issue385-BadZigZag-Progressive.jpg | 3 +++ 5 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 tests/Images/Input/Jpg/issues/Issue385-BadZigZag-Progressive.jpg diff --git a/src/ImageSharp/Formats/Jpeg/GolangPort/Components/Decoder/OrigJpegScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/GolangPort/Components/Decoder/OrigJpegScanDecoder.cs index 92c7ebe114..67abba9f33 100644 --- a/src/ImageSharp/Formats/Jpeg/GolangPort/Components/Decoder/OrigJpegScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/GolangPort/Components/Decoder/OrigJpegScanDecoder.cs @@ -583,7 +583,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort.Components.Decoder } zig = this.RefineNonZeroes(ref bp, zig, val0, delta); - if (bp.ReachedEOF) + + if (bp.ReachedEOF || bp.HasError) { return; } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index e26ab2cc9f..db42be2847 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -50,6 +50,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg TestImages.Jpeg.Progressive.Festzug, TestImages.Jpeg.Progressive.Bad.BadEOF, TestImages.Jpeg.Issues.BadCoeffsProgressive178, TestImages.Jpeg.Issues.MissingFF00ProgressiveGirl159, + TestImages.Jpeg.Issues.BadZigZagProgressive385 }; private static readonly Dictionary CustomToleranceValues = new Dictionary @@ -67,6 +68,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg [TestImages.Jpeg.Progressive.Festzug] = 0.02f / 100, [TestImages.Jpeg.Progressive.Fb] = 0.16f / 100, [TestImages.Jpeg.Progressive.Progress] = 0.31f / 100, + [TestImages.Jpeg.Issues.BadZigZagProgressive385] = 0.23f / 100, }; public const PixelTypes CommonNonDefaultPixelTypes = PixelTypes.Rgba32 | PixelTypes.Argb32 | PixelTypes.RgbaVector; diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 9137049ee1..f342542395 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -122,6 +122,7 @@ namespace SixLabors.ImageSharp.Tests public const string CriticalEOF214 = "Jpg/issues/Issue214-CriticalEOF.jpg"; public const string MissingFF00ProgressiveGirl159 = "Jpg/issues/Issue159-MissingFF00-Progressive-Girl.jpg"; public const string BadCoeffsProgressive178 = "Jpg/issues/Issue178-BadCoeffsProgressive-Lemon.jpg"; + public const string BadZigZagProgressive385 = "Jpg/issues/Issue385-BadZigZag-Progressive.jpg"; } public static readonly string[] All = Baseline.All.Concat(Progressive.All).ToArray(); diff --git a/tests/Images/External b/tests/Images/External index f99c2ea414..ab7c90362a 160000 --- a/tests/Images/External +++ b/tests/Images/External @@ -1 +1 @@ -Subproject commit f99c2ea41419cb3b3e80e5beeab611682252df78 +Subproject commit ab7c90362a4881c790c6f264f400dcf4b4dea0ce diff --git a/tests/Images/Input/Jpg/issues/Issue385-BadZigZag-Progressive.jpg b/tests/Images/Input/Jpg/issues/Issue385-BadZigZag-Progressive.jpg new file mode 100644 index 0000000000..963cb3d978 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/Issue385-BadZigZag-Progressive.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:1052660c765aaad52f7bbad779778b5d15fdc3c92ff4b114c89e579c749c7f6b +size 388517 From 5426cb01d1b324c0ec7aca5febdc11ae07da9026 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 4 Dec 2017 22:29:28 +1100 Subject: [PATCH 2/3] Fix travis --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index a4f68b1d1b..70501a484b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,7 +6,7 @@ matrix: - os: linux # Ubuntu 14.04 dist: trusty sudo: required - dotnet: 1.0.1 + dotnet: 1.0.4 mono: latest # - os: osx # OSX 10.11 # osx_image: xcode7.3.1 From c15f2328ecfac66250d3ed64ca80c3e02a72c609 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 6 Dec 2017 00:58:44 +1100 Subject: [PATCH 3/3] Minor hashing optimizations --- src/ImageSharp/Formats/Png/Zlib/Adler32.cs | 45 ++++++++----------- src/ImageSharp/Formats/Png/Zlib/Crc32.cs | 35 ++++++--------- .../ImageSharp.Benchmarks/Image/DecodePng.cs | 20 ++++++--- 3 files changed, 47 insertions(+), 53 deletions(-) diff --git a/src/ImageSharp/Formats/Png/Zlib/Adler32.cs b/src/ImageSharp/Formats/Png/Zlib/Adler32.cs index 6841c1cb82..1cce90c0b7 100644 --- a/src/ImageSharp/Formats/Png/Zlib/Adler32.cs +++ b/src/ImageSharp/Formats/Png/Zlib/Adler32.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. using System; +using System.Runtime.CompilerServices; namespace SixLabors.ImageSharp.Formats.Png.Zlib { @@ -74,9 +75,17 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib } /// - public long Value => this.checksum; + public long Value + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get + { + return this.checksum; + } + } /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Reset() { this.checksum = 1; @@ -88,6 +97,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib /// /// The data value to add. The high byte of the int is ignored. /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Update(int value) { // We could make a length 1 byte array and call update again, but I @@ -102,6 +112,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib } /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Update(byte[] buffer) { if (buffer == null) @@ -113,32 +124,14 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib } /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Update(byte[] buffer, int offset, int count) { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - - if (offset < 0) - { - throw new ArgumentOutOfRangeException(nameof(offset), "cannot be negative"); - } - - if (count < 0) - { - throw new ArgumentOutOfRangeException(nameof(count), "cannot be negative"); - } - - if (offset >= buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset), "not a valid index into buffer"); - } - - if (offset + count > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(count), "exceeds buffer size"); - } + DebugGuard.NotNull(buffer, nameof(buffer)); + DebugGuard.MustBeGreaterThanOrEqualTo(offset, 0, nameof(offset)); + DebugGuard.MustBeGreaterThanOrEqualTo(count, 0, nameof(count)); + DebugGuard.MustBeLessThan(offset, buffer.Length, nameof(offset)); + DebugGuard.MustBeLessThanOrEqualTo(offset + count, buffer.Length, nameof(count)); // (By Per Bothner) uint s1 = this.checksum & 0xFFFF; @@ -169,4 +162,4 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib this.checksum = (s2 << 16) | s1; } } -} +} \ No newline at end of file diff --git a/src/ImageSharp/Formats/Png/Zlib/Crc32.cs b/src/ImageSharp/Formats/Png/Zlib/Crc32.cs index 14a29b7aff..bd686f2b9f 100644 --- a/src/ImageSharp/Formats/Png/Zlib/Crc32.cs +++ b/src/ImageSharp/Formats/Png/Zlib/Crc32.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. using System; +using System.Runtime.CompilerServices; namespace SixLabors.ImageSharp.Formats.Png.Zlib { @@ -108,18 +109,15 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib /// public long Value { - get - { - return this.crc; - } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => this.crc; - set - { - this.crc = (uint)value; - } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + set => this.crc = (uint)value; } /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Reset() { this.crc = 0; @@ -129,6 +127,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib /// Updates the checksum with the given value. /// /// The byte is taken as the lower 8 bits of value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Update(int value) { this.crc ^= CrcSeed; @@ -137,6 +136,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib } /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Update(byte[] buffer) { if (buffer == null) @@ -148,22 +148,13 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib } /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Update(byte[] buffer, int offset, int count) { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - - if (count < 0) - { - throw new ArgumentOutOfRangeException(nameof(count), "Count cannot be less than zero"); - } - - if (offset < 0 || offset + count > buffer.Length) - { - throw new ArgumentOutOfRangeException(nameof(offset)); - } + DebugGuard.NotNull(buffer, nameof(buffer)); + DebugGuard.MustBeGreaterThanOrEqualTo(count, 0, nameof(count)); + DebugGuard.MustBeGreaterThanOrEqualTo(offset, 0, nameof(offset)); + DebugGuard.MustBeLessThanOrEqualTo(offset + count, buffer.Length, nameof(count)); this.crc ^= CrcSeed; diff --git a/tests/ImageSharp.Benchmarks/Image/DecodePng.cs b/tests/ImageSharp.Benchmarks/Image/DecodePng.cs index b9a9d5cfa2..eb13cf9290 100644 --- a/tests/ImageSharp.Benchmarks/Image/DecodePng.cs +++ b/tests/ImageSharp.Benchmarks/Image/DecodePng.cs @@ -10,29 +10,39 @@ namespace SixLabors.ImageSharp.Benchmarks.Image using BenchmarkDotNet.Attributes; + using SixLabors.ImageSharp.Tests; + using CoreImage = ImageSharp.Image; using CoreSize = SixLabors.Primitives.Size; + [Config(typeof(Config.ShortClr))] public class DecodePng : BenchmarkBase { private byte[] pngBytes; + private string TestImageFullPath => Path.Combine( + TestEnvironment.InputImagesDirectoryFullPath, + this.TestImage); + + [Params(TestImages.Png.Splash)] + public string TestImage { get; set; } + [GlobalSetup] public void ReadImages() { if (this.pngBytes == null) { - this.pngBytes = File.ReadAllBytes("../ImageSharp.Tests/TestImages/Formats/Png/splash.png"); + this.pngBytes = File.ReadAllBytes(this.TestImageFullPath); } } [Benchmark(Baseline = true, Description = "System.Drawing Png")] public Size PngSystemDrawing() { - using (MemoryStream memoryStream = new MemoryStream(this.pngBytes)) + using (var memoryStream = new MemoryStream(this.pngBytes)) { - using (Image image = Image.FromStream(memoryStream)) + using (var image = Image.FromStream(memoryStream)) { return image.Size; } @@ -42,9 +52,9 @@ namespace SixLabors.ImageSharp.Benchmarks.Image [Benchmark(Description = "ImageSharp Png")] public CoreSize PngCore() { - using (MemoryStream memoryStream = new MemoryStream(this.pngBytes)) + using (var memoryStream = new MemoryStream(this.pngBytes)) { - using (Image image = CoreImage.Load(memoryStream)) + using (var image = CoreImage.Load(memoryStream)) { return new CoreSize(image.Width, image.Height); }