diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanBuffer.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanBuffer.cs index dd5160414..13c89c82c 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanBuffer.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanBuffer.cs @@ -17,7 +17,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder private ulong data; // The number of valid bits left to read in the buffer. - private int remain; + private int remainingBits; // Whether there is no more good data to pull from the stream for the current mcu. private bool badData; @@ -26,10 +26,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder { this.stream = stream; this.data = 0ul; - this.remain = 0; + this.remainingBits = 0; this.Marker = JpegConstants.Markers.XFF; this.MarkerPosition = 0; - this.BadMarker = false; this.badData = false; this.NoData = false; } @@ -44,11 +43,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// public long MarkerPosition { get; private set; } - /// - /// Gets a value indicating whether a bad marker has been detected, I.E. One that is not between RST0 and RST7 - /// - public bool BadMarker { get; private set; } - /// /// Gets a value indicating whether to continue reading the input stream. /// @@ -57,7 +51,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder [MethodImpl(InliningOptions.ShortMethod)] public void CheckBits() { - if (this.remain < 16) + if (this.remainingBits < 16) { this.FillBuffer(); } @@ -67,27 +61,31 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder public void Reset() { this.data = 0ul; - this.remain = 0; + this.remainingBits = 0; this.Marker = JpegConstants.Markers.XFF; this.MarkerPosition = 0; - this.BadMarker = false; this.badData = false; this.NoData = false; } + /// + /// Whether a RST marker has been detected, I.E. One that is between RST0 and RST7 + /// [MethodImpl(InliningOptions.ShortMethod)] - public bool HasRestart() - { - byte m = this.Marker; - return m >= JpegConstants.Markers.RST0 && m <= JpegConstants.Markers.RST7; - } + public bool HasRestartMarker() => HasRestart(this.Marker); + + /// + /// Whether a bad marker has been detected, I.E. One that is not between RST0 and RST7 + /// + [MethodImpl(InliningOptions.ShortMethod)] + public bool HasBadMarker() => this.Marker != JpegConstants.Markers.XFF && !this.HasRestartMarker(); [MethodImpl(InliningOptions.ShortMethod)] public void FillBuffer() { // Attempt to load at least the minimum number of required bits into the buffer. // We fail to do so only if we hit a marker or reach the end of the input stream. - this.remain += 48; + this.remainingBits += 48; this.data = (this.data << 48) | this.GetBytes(); } @@ -101,17 +99,17 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder if (size == JpegConstants.Huffman.SlowBits) { - ulong x = this.data << (JpegConstants.Huffman.RegisterSize - this.remain); + ulong x = this.data << (JpegConstants.Huffman.RegisterSize - this.remainingBits); while (x > h.MaxCode[size]) { size++; } v = (int)(x >> (JpegConstants.Huffman.RegisterSize - size)); - symbol = h.Values[h.ValOffset[size] + v]; + symbol = h.Values[(h.ValOffset[size] + v) & 0xFF]; } - this.remain -= size; + this.remainingBits -= size; return symbol; } @@ -124,10 +122,14 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder } [MethodImpl(InliningOptions.ShortMethod)] - public int GetBits(int nbits) => (int)ExtractBits(this.data, this.remain -= nbits, nbits); + private static bool HasRestart(byte marker) + => marker >= JpegConstants.Markers.RST0 && marker <= JpegConstants.Markers.RST7; [MethodImpl(InliningOptions.ShortMethod)] - public int PeekBits(int nbits) => (int)ExtractBits(this.data, this.remain - nbits, nbits); + public int GetBits(int nbits) => (int)ExtractBits(this.data, this.remainingBits -= nbits, nbits); + + [MethodImpl(InliningOptions.ShortMethod)] + public int PeekBits(int nbits) => (int)ExtractBits(this.data, this.remainingBits - nbits, nbits); [MethodImpl(InliningOptions.ShortMethod)] private static ulong ExtractBits(ulong value, int offset, int size) => (value >> offset) & (ulong)((1 << size) - 1); @@ -149,22 +151,18 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder int c = this.ReadStream(); while (c == JpegConstants.Markers.XFF) { - // Loop here to discard any padding FF's on terminating marker, + // Loop here to discard any padding FF bytes on terminating marker, // so that we can save a valid marker value. c = this.ReadStream(); } - // We accept multiple FF's followed by a 0 as meaning a single FF data byte. + // We accept multiple FF bytes followed by a 0 as meaning a single FF data byte. // This data pattern is not valid according to the standard. if (c != 0) { this.Marker = (byte)c; this.badData = true; - if (!this.HasRestart()) - { - this.MarkerPosition = this.stream.Position - 2; - this.BadMarker = true; - } + this.MarkerPosition = this.stream.Position - 2; } } @@ -174,6 +172,42 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder return temp; } + [MethodImpl(InliningOptions.ShortMethod)] + public bool FindNextMarker() + { + while (true) + { + int b = this.stream.ReadByte(); + if (b == -1) + { + return false; + } + + // Found a marker. + if (b == JpegConstants.Markers.XFF) + { + while (b == JpegConstants.Markers.XFF) + { + // Loop here to discard any padding FF bytes on terminating marker. + b = this.stream.ReadByte(); + if (b == -1) + { + return false; + } + } + + // Found a valid marker. Exit loop + if (b != 0) + { + this.Marker = (byte)b; + this.badData = true; + this.MarkerPosition = this.stream.Position - 2; + return true; + } + } + } + } + [MethodImpl(InliningOptions.ShortMethod)] private int ReadStream() { diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs index c28733250..c50812e25 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs @@ -106,7 +106,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder this.ParseProgressiveData(); } - if (this.scanBuffer.BadMarker) + if (this.scanBuffer.HasBadMarker()) { this.stream.Position = this.scanBuffer.MarkerPosition; } @@ -684,15 +684,23 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder { if (this.restartInterval > 0 && (--this.todo) == 0) { + if (this.scanBuffer.Marker == JpegConstants.Markers.XFF) + { + if (!this.scanBuffer.FindNextMarker()) + { + return false; + } + } + this.todo = this.restartInterval; - if (this.scanBuffer.HasRestart()) + if (this.scanBuffer.HasRestartMarker()) { this.Reset(); return true; } - if (this.scanBuffer.Marker != JpegConstants.Markers.XFF) + if (this.scanBuffer.HasBadMarker()) { this.stream.Position = this.scanBuffer.MarkerPosition; this.Reset(); diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs index 21ed5018f..4685ba289 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanTable.cs @@ -1,20 +1,18 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; -using SixLabors.Memory; namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder { /// - /// Represents a Huffman coding table containing basic coding data plus tables for accellerated computation. + /// Represents a Huffman coding table containing basic coding data plus tables for accelerated computation. /// [StructLayout(LayoutKind.Sequential)] internal unsafe struct HuffmanTable { - private readonly MemoryAllocator memoryAllocator; private bool isConfigured; /// @@ -60,13 +58,11 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// /// Initializes a new instance of the struct. /// - /// The to use for buffer allocations. /// The code lengths /// The huffman values - public HuffmanTable(MemoryAllocator memoryAllocator, ReadOnlySpan codeLengths, ReadOnlySpan values) + public HuffmanTable(ReadOnlySpan codeLengths, ReadOnlySpan values) { this.isConfigured = false; - this.memoryAllocator = memoryAllocator; Unsafe.CopyBlockUnaligned(ref this.Sizes[0], ref MemoryMarshal.GetReference(codeLengths), (uint)codeLengths.Length); Unsafe.CopyBlockUnaligned(ref this.Values[0], ref MemoryMarshal.GetReference(values), (uint)values.Length); } @@ -81,33 +77,31 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder return; } - int p, si; - Span huffsize = stackalloc char[257]; - Span huffcode = stackalloc uint[257]; - uint code; + Span huffSize = stackalloc char[257]; + Span huffCode = stackalloc uint[257]; // Figure C.1: make table of Huffman code length for each symbol - p = 0; + int p = 0; for (int l = 1; l <= 16; l++) { int i = this.Sizes[l]; while (i-- != 0) { - huffsize[p++] = (char)l; + huffSize[p++] = (char)l; } } - huffsize[p] = (char)0; + huffSize[p] = (char)0; // Figure C.2: generate the codes themselves - code = 0; - si = huffsize[0]; + uint code = 0; + int si = huffSize[0]; p = 0; - while (huffsize[p] != 0) + while (huffSize[p] != 0) { - while (huffsize[p] == si) + while (huffSize[p] == si) { - huffcode[p++] = code; + huffCode[p++] = code; code++; } @@ -121,10 +115,10 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder { if (this.Sizes[l] != 0) { - int offset = p - (int)huffcode[p]; + int offset = p - (int)huffCode[p]; this.ValOffset[l] = offset; p += this.Sizes[l]; - this.MaxCode[l] = huffcode[p - 1]; // Maximum code of length l + this.MaxCode[l] = huffCode[p - 1]; // Maximum code of length l this.MaxCode[l] <<= 64 - l; // Left justify this.MaxCode[l] |= (1ul << (64 - l)) - 1; } @@ -150,14 +144,14 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder { for (int i = 1; i <= this.Sizes[length]; i++, p++) { - // length = current code's length, p = its index in huffcode[] & huffval[]. + // length = current code's length, p = its index in huffCode[] & Values[]. // Generate left-justified code followed by all possible bit sequences - int lookbits = (int)(huffcode[p] << (JpegConstants.Huffman.LookupBits - length)); + int lookBits = (int)(huffCode[p] << (JpegConstants.Huffman.LookupBits - length)); for (int ctr = 1 << (JpegConstants.Huffman.LookupBits - length); ctr > 0; ctr--) { - this.LookaheadSize[lookbits] = (byte)length; - this.LookaheadValue[lookbits] = this.Values[p]; - lookbits++; + this.LookaheadSize[lookBits] = (byte)length; + this.LookaheadValue[lookBits] = this.Values[p]; + lookBits++; } } } @@ -165,4 +159,4 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder this.isConfigured = true; } } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index fd0c289b8..bf7c8f9c8 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; @@ -952,7 +952,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg /// The values [MethodImpl(InliningOptions.ShortMethod)] private void BuildHuffmanTable(HuffmanTable[] tables, int index, ReadOnlySpan codeLengths, ReadOnlySpan values) - => tables[index] = new HuffmanTable(this.configuration.MemoryAllocator, codeLengths, values); + => tables[index] = new HuffmanTable(codeLengths, values); /// /// Reads a from the stream advancing it by two bytes @@ -992,4 +992,4 @@ namespace SixLabors.ImageSharp.Formats.Jpeg return image; } } -} \ No newline at end of file +} diff --git a/tests/ImageSharp.Benchmarks/ImageSharp.Benchmarks.csproj b/tests/ImageSharp.Benchmarks/ImageSharp.Benchmarks.csproj index 1e951f5d0..14ad5635c 100644 --- a/tests/ImageSharp.Benchmarks/ImageSharp.Benchmarks.csproj +++ b/tests/ImageSharp.Benchmarks/ImageSharp.Benchmarks.csproj @@ -6,11 +6,8 @@ Exe SixLabors.ImageSharp.Benchmarks netcoreapp2.1;net472 - - - - win7-x64 - false + false + false @@ -18,10 +15,6 @@ - - - - diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs index 56496fad1..4a3ef9b95 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs @@ -15,14 +15,10 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg TestImages.Jpeg.Baseline.Jpeg400, TestImages.Jpeg.Baseline.Testorig420, - // BUG: The following image has a high difference compared to the expected output: + // BUG: The following image has a high difference compared to the expected output: 1.0096% // TestImages.Jpeg.Baseline.Jpeg420Small, - // BUG: While we can decode this image we do not return the same output as libjpeg - // based decoders and are inserting a number of lines equal to the corrupted lines - // below said lines. - // TestImages.Jpeg.Issues.Fuzz.AccessViolationException922, - + TestImages.Jpeg.Issues.Fuzz.AccessViolationException922, TestImages.Jpeg.Baseline.Jpeg444, TestImages.Jpeg.Baseline.Bad.BadEOF, TestImages.Jpeg.Baseline.MultiScanBaselineCMYK, diff --git a/tests/Images/External b/tests/Images/External index 42b3b980e..acc32594c 160000 --- a/tests/Images/External +++ b/tests/Images/External @@ -1 +1 @@ -Subproject commit 42b3b980ed07afd7b6603a5bfa6ffb91d6c8a124 +Subproject commit acc32594c125656840f8a17e69b0ebb49a370fa6