From 2f501eb1a49886319744d57dae9e1af4c7a2a69f Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 25 Jun 2017 22:44:07 +1000 Subject: [PATCH] Decoder now doesn't break tests --- .../Jpeg/Port/Components/HuffmanTables.cs | 18 +-- .../Formats/Jpeg/Port/JpegDecoderCore.cs | 117 ++++++------------ tests/ImageSharp.Tests/xunit.runner.json | 3 +- 3 files changed, 45 insertions(+), 93 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Port/Components/HuffmanTables.cs b/src/ImageSharp/Formats/Jpeg/Port/Components/HuffmanTables.cs index a040d21e70..a8644d6451 100644 --- a/src/ImageSharp/Formats/Jpeg/Port/Components/HuffmanTables.cs +++ b/src/ImageSharp/Formats/Jpeg/Port/Components/HuffmanTables.cs @@ -13,9 +13,7 @@ namespace ImageSharp.Formats.Jpeg.Port.Components /// internal class HuffmanTables { - private HuffmanBranch[] first; - - private HuffmanBranch[] second; + private readonly HuffmanBranch[][] tables = new HuffmanBranch[4][]; /// /// Gets or sets the table at the given index. @@ -27,23 +25,13 @@ namespace ImageSharp.Formats.Jpeg.Port.Components [MethodImpl(MethodImplOptions.AggressiveInlining)] get { - if (index == 0) - { - return this.first; - } - - return this.second; + return this.tables[index]; } [MethodImpl(MethodImplOptions.AggressiveInlining)] set { - if (index == 0) - { - this.first = value; - } - - this.second = value; + this.tables[index] = value; } } } diff --git a/src/ImageSharp/Formats/Jpeg/Port/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/Port/JpegDecoderCore.cs index 4bb93151a3..5d50ae5353 100644 --- a/src/ImageSharp/Formats/Jpeg/Port/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/Port/JpegDecoderCore.cs @@ -90,47 +90,32 @@ namespace ImageSharp.Formats.Jpeg.Port public static FileMarker FindNextFileMarkerOld(Stream stream) { byte[] buffer = new byte[2]; - while (true) + int value = stream.Read(buffer, 0, 2); + + if (value == 0) { - int value = stream.Read(buffer, 0, 2); + return new FileMarker(JpegConstants.Markers.EOI, (int)stream.Length, true); + } - if (value == 0) - { - return new FileMarker(JpegConstants.Markers.EOI, (int)stream.Length, true); - } + // According to Section B.1.1.2: + // "Any marker may optionally be preceded by any number of fill bytes, which are bytes assigned code 0xFF." + if (buffer[1] != JpegConstants.Markers.Prefix) + { + return new FileMarker((ushort)((buffer[0] << 8) | buffer[1]), (int)(stream.Position - 2)); + } - while (buffer[0] != JpegConstants.Markers.Prefix) + while (buffer[1] == JpegConstants.Markers.Prefix) + { + int suffix = stream.ReadByte(); + if (suffix == -1) { - // Strictly speaking, this is a format error. However, libjpeg is - // liberal in what it accepts. As of version 9, next_marker in - // jdmarker.c treats this as a warning (JWRN_EXTRANEOUS_DATA) and - // continues to decode the stream. Even before next_marker sees - // extraneous data, jpeg_fill_bit_buffer in jdhuff.c reads as many - // bytes as it can, possibly past the end of a scan's data. It - // effectively puts back any markers that it overscanned (e.g. an - // "\xff\xd9" EOI marker), but it does not put back non-marker data, - // and thus it can silently ignore a small number of extraneous - // non-marker bytes before next_marker has a chance to see them (and - // print a warning). - // We are therefore also liberal in what we accept. Extraneous data - // is silently ignore - // This is similar to, but not exactly the same as, the restart - // mechanism within a scan (the RST[0-7] markers). - // Note that extraneous 0xff bytes in e.g. SOS data are escaped as - // "\xff\x00", and so are detected a little further down below. - buffer[0] = buffer[1]; - - value = stream.ReadByte(); - if (value == -1) - { - return new FileMarker(JpegConstants.Markers.EOI, (int)stream.Length, true); - } - - buffer[1] = (byte)value; + return new FileMarker(JpegConstants.Markers.EOI, (int)stream.Length, true); } - return new FileMarker((ushort)((buffer[0] << 8) | buffer[1]), (int)(stream.Position - 2)); + buffer[1] = (byte)value; } + + return new FileMarker((ushort)((buffer[0] << 8) | buffer[1]), (int)(stream.Position - 2)); } /// @@ -185,45 +170,6 @@ namespace ImageSharp.Formats.Jpeg.Port return new FileMarker(newMarker, newPos, true); } - /// - /// Finds the next file marker within the byte stream. Used for testing. Slower as it only reads on byte at a time - /// - /// The input stream - /// The - public static FileMarker FindNextFileMarkerNew(Stream stream) - { - while (true) - { - int value = stream.ReadByte(); - - if (value == -1) - { - // We've reached the end of the stream - return new FileMarker(JpegConstants.Markers.EOI, (int)stream.Length, true); - } - - byte prefix = (byte)value; - byte suffix = JpegConstants.Markers.Prefix; - - // According to Section B.1.1.2: - // "Any marker may optionally be preceded by any number of fill bytes, which are bytes assigned code 0xFF." - while (prefix == JpegConstants.Markers.Prefix && suffix == JpegConstants.Markers.Prefix) - { - value = stream.ReadByte(); - - if (value == -1) - { - // We've reached the end of the stream - return new FileMarker(JpegConstants.Markers.EOI, (int)stream.Length, true); - } - - suffix = (byte)value; - } - - return new FileMarker((ushort)((prefix << 8) | suffix), (int)(stream.Position - 2)); - } - } - /// /// Decodes the image from the specified and sets /// the data to image. @@ -357,11 +303,12 @@ namespace ImageSharp.Formats.Jpeg.Port break; } - throw new ImageFormatException($"Unknown Marker {fileMarker.Marker} at {fileMarker.Position}"); + // throw new ImageFormatException($"Unknown Marker {fileMarker.Marker} at {fileMarker.Position}"); + break; } // Read on. TODO: Test this on damaged images. - fileMarker = FindNextFileMarkerNew(this.InputStream); + fileMarker = FindNextFileMarkerOld(this.InputStream); } this.width = this.frame.SamplesPerLine; @@ -644,7 +591,6 @@ namespace ImageSharp.Formats.Jpeg.Port i += 17 + codeLengthSum; - // Everything I can discover indicates there's a max of two table per DC AC pair though this limits the index to 16? this.BuildHuffmanTable( huffmanTableSpec >> 4 == 0 ? this.dcHuffmanTables : this.acHuffmanTables, huffmanTableSpec & 15, @@ -679,7 +625,23 @@ namespace ImageSharp.Formats.Jpeg.Port int selectorsCount = this.InputStream.ReadByte(); for (int i = 0; i < selectorsCount; i++) { - byte componentIndex = this.frame.ComponentIds[this.InputStream.ReadByte() - 1]; + int index = -1; + int selector = this.InputStream.ReadByte(); + + foreach (byte id in this.frame.ComponentIds) + { + if (selector == id) + { + index = selector; + } + } + + if (index < 0) + { + throw new ImageFormatException("Unknown component selector"); + } + + byte componentIndex = this.frame.ComponentIds[index]; ref FrameComponent component = ref this.frame.Components[componentIndex]; int tableSpec = this.InputStream.ReadByte(); component.DCHuffmanTableId = tableSpec >> 4; @@ -717,6 +679,7 @@ namespace ImageSharp.Formats.Jpeg.Port /// /// Builds the huffman tables + /// TODO: This is our bottleneck. We should use a faster algorithm with a LUT. /// /// The tables /// The table index diff --git a/tests/ImageSharp.Tests/xunit.runner.json b/tests/ImageSharp.Tests/xunit.runner.json index df1c3d50d0..cbaa8f4325 100644 --- a/tests/ImageSharp.Tests/xunit.runner.json +++ b/tests/ImageSharp.Tests/xunit.runner.json @@ -1,3 +1,4 @@ { - "methodDisplay": "method" + "methodDisplay": "method", + "diagnosticMessages": true } \ No newline at end of file