From bccff8beb41c2dc4a9deab9688909574262d492e Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 28 Apr 2018 11:04:48 +1000 Subject: [PATCH 01/17] Increase Identify performance and reduce allocations --- .../Jpeg/GolangPort/OrigJpegDecoderCore.cs | 33 +-- .../Components/DoubleBufferedStreamReader.cs | 151 +++++++++++ .../PdfJsPort/Components/PdfJsFileMarker.cs | 24 +- .../PdfJsPort/Components/PdfJsScanDecoder.cs | 44 ++-- .../Jpeg/PdfJsPort/PdfJsJpegConstants.cs | 56 ++--- .../Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs | 237 ++++++++++-------- .../Codecs/Jpeg/Identify.cs | 52 ++++ .../General/DoubleBufferedStreams.cs | 53 ++++ .../Jpg/DoubleBufferedStreamReaderTests.cs | 132 ++++++++++ 9 files changed, 603 insertions(+), 179 deletions(-) create mode 100644 src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs create mode 100644 tests/ImageSharp.Benchmarks/Codecs/Jpeg/Identify.cs create mode 100644 tests/ImageSharp.Benchmarks/General/DoubleBufferedStreams.cs create mode 100644 tests/ImageSharp.Tests/Formats/Jpg/DoubleBufferedStreamReaderTests.cs diff --git a/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs index a4fbb17be..fd83f9568 100644 --- a/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs @@ -87,8 +87,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort { this.IgnoreMetadata = options.IgnoreMetadata; this.configuration = configuration ?? Configuration.Default; - this.HuffmanTrees = OrigHuffmanTree.CreateHuffmanTrees(); - this.QuantizationTables = new Block8x8F[MaxTq + 1]; this.Temp = new byte[2 * Block8x8F.Size]; } @@ -103,10 +101,10 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort /// /// Gets the huffman trees /// - public OrigHuffmanTree[] HuffmanTrees { get; } + public OrigHuffmanTree[] HuffmanTrees { get; private set; } /// - public Block8x8F[] QuantizationTables { get; } + public Block8x8F[] QuantizationTables { get; private set; } /// /// Gets the temporary buffer used to store bytes read from the stream. @@ -233,6 +231,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort this.InputStream = stream; this.InputProcessor = new InputProcessor(stream, this.Temp); + if (!metadataOnly) + { + this.HuffmanTrees = OrigHuffmanTree.CreateHuffmanTrees(); + this.QuantizationTables = new Block8x8F[MaxTq + 1]; + } + // Check for the Start Of Image marker. this.InputProcessor.ReadFull(this.Temp, 0, 2); @@ -331,11 +335,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort case OrigJpegConstants.Markers.SOF1: case OrigJpegConstants.Markers.SOF2: this.IsProgressive = marker == OrigJpegConstants.Markers.SOF2; - this.ProcessStartOfFrameMarker(remaining); - if (metadataOnly && this.isJFif) - { - return; - } + this.ProcessStartOfFrameMarker(remaining, metadataOnly); break; case OrigJpegConstants.Markers.DHT: @@ -425,7 +425,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort /// private void InitDerivedMetaDataProperties() { - if (this.isExif) + if (this.isJFif) + { + this.MetaData.HorizontalResolution = this.jFif.XDensity; + this.MetaData.VerticalResolution = this.jFif.YDensity; + } + else if (this.isExif) { double horizontalValue = this.MetaData.ExifProfile.TryGetValue(ExifTag.XResolution, out ExifValue horizonalTag) ? ((Rational)horizonalTag.Value).ToDouble() @@ -441,11 +446,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort this.MetaData.VerticalResolution = verticalValue; } } - else if (this.isJFif) - { - this.MetaData.HorizontalResolution = this.jFif.XDensity; - this.MetaData.VerticalResolution = this.jFif.YDensity; - } } /// @@ -634,7 +634,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort /// Processes the Start of Frame marker. Specified in section B.2.2. /// /// The remaining bytes in the segment block. - private void ProcessStartOfFrameMarker(int remaining) + /// Whether to parse metadata only + private void ProcessStartOfFrameMarker(int remaining, bool metadataOnly) { if (this.ComponentCount != 0) { diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs new file mode 100644 index 000000000..0818d7309 --- /dev/null +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs @@ -0,0 +1,151 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.IO; + +// TODO: This could be useful elsewhere. +namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components +{ + /// + /// A stream reader that add a secondary level buffer in addition to native stream buffered reading + /// to reduce the overhead of small incremental reads. + /// + internal class DoubleBufferedStreamReader + { + /// + /// The length, in bytes, of the chunk + /// + public const int ChunkLength = 4096; + + private readonly Stream stream; + + private readonly byte[] chunk; + + private int bytesRead; + + private long position; + + /// + /// Initializes a new instance of the class. + /// + /// The input stream. + public DoubleBufferedStreamReader(Stream stream) + { + this.stream = stream; + this.Length = stream.Length; + + // TODO: Consider pooling this. + this.chunk = new byte[ChunkLength]; + } + + /// + /// Gets the length, in bytes, of the stream + /// + public long Length { get; } + + /// + /// Gets or sets the current position within the stream + /// + public long Position + { + get + { + return this.position; + } + + set + { + // Reset everything. It's easier than tracking. + this.position = value; + this.stream.Seek(this.position, SeekOrigin.Begin); + this.bytesRead = ChunkLength; + } + } + + /// + /// Reads a byte from the stream and advances the position within the stream by one + /// byte, or returns -1 if at the end of the stream. + /// + /// The unsigned byte cast to an , or -1 if at the end of the stream. + public int ReadByte() + { + if (this.position >= this.Length) + { + return -1; + } + + if (this.position == 0 || this.bytesRead >= ChunkLength) + { + this.stream.Seek(this.position, SeekOrigin.Begin); + this.stream.Read(this.chunk, 0, ChunkLength); + this.bytesRead = 0; + } + + this.position++; + return this.chunk[this.bytesRead++]; + } + + /// + /// Skips the number of bytes in the stream + /// + /// The number of bytes to skip + public void Skip(int count) + { + this.position += count; + this.bytesRead += count; + } + + /// + /// Reads a sequence of bytes from the current stream and advances the position within the stream + /// by the number of bytes read. + /// + /// + /// An array of bytes. When this method returns, the buffer contains the specified + /// byte array with the values between offset and (offset + count - 1) replaced by + /// the bytes read from the current source. + /// + /// + /// The zero-based byte offset in buffer at which to begin storing the data read + /// from the current stream. + /// + /// The maximum number of bytes to be read from the current stream. + /// + /// The total number of bytes read into the buffer. This can be less than the number + /// of bytes requested if that many bytes are not currently available, or zero (0) + /// if the end of the stream has been reached. + /// + public int Read(byte[] buffer, int offset, int count) + { + int n = 0; + if (buffer.Length <= ChunkLength) + { + if (this.position == 0 || count + this.bytesRead > ChunkLength) + { + // Refill our buffer then copy. + this.stream.Seek(this.position, SeekOrigin.Begin); + this.stream.Read(this.chunk, 0, ChunkLength); + this.bytesRead = 0; + } + + Buffer.BlockCopy(this.chunk, this.bytesRead, buffer, offset, count); + this.position += count; + this.bytesRead += count; + + n = Math.Min(count, (int)(this.Length - this.position)); + } + else + { + // Read to target but don't copy to our chunk. + this.stream.Seek(this.position, SeekOrigin.Begin); + n = this.stream.Read(buffer, offset, count); + + // Ensure next read fills the chunk + this.bytesRead = ChunkLength; + this.position += count; + } + + return Math.Max(n, 0); + } + } +} \ No newline at end of file diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsFileMarker.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsFileMarker.cs index 8e51c0b7c..85c9f9466 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsFileMarker.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsFileMarker.cs @@ -1,6 +1,8 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. +using System.Runtime.CompilerServices; + namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components { /// @@ -13,7 +15,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components /// /// The marker /// The position within the stream - public PdfJsFileMarker(ushort marker, long position) + public PdfJsFileMarker(byte marker, long position) { this.Marker = marker; this.Position = position; @@ -26,7 +28,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components /// The marker /// The position within the stream /// Whether the current marker is invalid - public PdfJsFileMarker(ushort marker, long position, bool invalid) + public PdfJsFileMarker(byte marker, long position, bool invalid) { this.Marker = marker; this.Position = position; @@ -36,17 +38,29 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components /// /// Gets a value indicating whether the current marker is invalid /// - public bool Invalid { get; } + public bool Invalid + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get; + } /// /// Gets the position of the marker within a stream /// - public ushort Marker { get; } + public byte Marker + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get; + } /// /// Gets the position of the marker within a stream /// - public long Position { get; } + public long Position + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get; + } /// public override string ToString() diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs index 5fcaa6cea..4415a681b 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs @@ -60,7 +60,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components /// The successive approximation bit low end public void DecodeScan( PdfJsFrame frame, - Stream stream, + DoubleBufferedStreamReader stream, PdfJsHuffmanTables dcHuffmanTables, PdfJsHuffmanTables acHuffmanTables, PdfJsFrameComponent[] components, @@ -176,7 +176,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components int mcusPerLine, int mcuToRead, ref int mcu, - Stream stream) + DoubleBufferedStreamReader stream) { if (componentsLength == 1) { @@ -237,7 +237,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components int mcusPerLine, int mcuToRead, ref int mcu, - Stream stream) + DoubleBufferedStreamReader stream) { if (componentsLength == 1) { @@ -331,7 +331,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeBlockBaseline(ref PdfJsHuffmanTable dcHuffmanTable, ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcu, Stream stream) + private void DecodeBlockBaseline(ref PdfJsHuffmanTable dcHuffmanTable, ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcu, DoubleBufferedStreamReader stream) { int blockRow = mcu / component.WidthInBlocks; int blockCol = mcu % component.WidthInBlocks; @@ -340,7 +340,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeMcuBaseline(ref PdfJsHuffmanTable dcHuffmanTable, ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcusPerLine, int mcu, int row, int col, Stream stream) + private void DecodeMcuBaseline(ref PdfJsHuffmanTable dcHuffmanTable, ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcusPerLine, int mcu, int row, int col, DoubleBufferedStreamReader stream) { int mcuRow = mcu / mcusPerLine; int mcuCol = mcu % mcusPerLine; @@ -351,7 +351,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeBlockDCFirst(ref PdfJsHuffmanTable dcHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcu, Stream stream) + private void DecodeBlockDCFirst(ref PdfJsHuffmanTable dcHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcu, DoubleBufferedStreamReader stream) { int blockRow = mcu / component.WidthInBlocks; int blockCol = mcu % component.WidthInBlocks; @@ -360,7 +360,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeMcuDCFirst(ref PdfJsHuffmanTable dcHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcusPerLine, int mcu, int row, int col, Stream stream) + private void DecodeMcuDCFirst(ref PdfJsHuffmanTable dcHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcusPerLine, int mcu, int row, int col, DoubleBufferedStreamReader stream) { int mcuRow = mcu / mcusPerLine; int mcuCol = mcu % mcusPerLine; @@ -371,7 +371,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeBlockDCSuccessive(PdfJsFrameComponent component, ref short blockDataRef, int mcu, Stream stream) + private void DecodeBlockDCSuccessive(PdfJsFrameComponent component, ref short blockDataRef, int mcu, DoubleBufferedStreamReader stream) { int blockRow = mcu / component.WidthInBlocks; int blockCol = mcu % component.WidthInBlocks; @@ -380,7 +380,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeMcuDCSuccessive(PdfJsFrameComponent component, ref short blockDataRef, int mcusPerLine, int mcu, int row, int col, Stream stream) + private void DecodeMcuDCSuccessive(PdfJsFrameComponent component, ref short blockDataRef, int mcusPerLine, int mcu, int row, int col, DoubleBufferedStreamReader stream) { int mcuRow = mcu / mcusPerLine; int mcuCol = mcu % mcusPerLine; @@ -391,7 +391,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeBlockACFirst(ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcu, Stream stream) + private void DecodeBlockACFirst(ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcu, DoubleBufferedStreamReader stream) { int blockRow = mcu / component.WidthInBlocks; int blockCol = mcu % component.WidthInBlocks; @@ -400,7 +400,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeMcuACFirst(ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcusPerLine, int mcu, int row, int col, Stream stream) + private void DecodeMcuACFirst(ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcusPerLine, int mcu, int row, int col, DoubleBufferedStreamReader stream) { int mcuRow = mcu / mcusPerLine; int mcuCol = mcu % mcusPerLine; @@ -411,7 +411,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeBlockACSuccessive(ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcu, Stream stream) + private void DecodeBlockACSuccessive(ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcu, DoubleBufferedStreamReader stream) { int blockRow = mcu / component.WidthInBlocks; int blockCol = mcu % component.WidthInBlocks; @@ -420,7 +420,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeMcuACSuccessive(ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcusPerLine, int mcu, int row, int col, Stream stream) + private void DecodeMcuACSuccessive(ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcusPerLine, int mcu, int row, int col, DoubleBufferedStreamReader stream) { int mcuRow = mcu / mcusPerLine; int mcuCol = mcu % mcusPerLine; @@ -431,7 +431,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private int ReadBit(Stream stream) + private int ReadBit(DoubleBufferedStreamReader stream) { // TODO: I wonder if we can do this two bytes at a time; libjpeg turbo seems to do that? if (this.bitsCount > 0) @@ -471,7 +471,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private short DecodeHuffman(ref PdfJsHuffmanTable tree, Stream stream) + private short DecodeHuffman(ref PdfJsHuffmanTable tree, DoubleBufferedStreamReader stream) { // TODO: Implement fast Huffman decoding. // NOTES # During investigation of the libjpeg implementation it appears that they pull 32bits at a time and operate on those bits @@ -503,7 +503,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private int Receive(int length, Stream stream) + private int Receive(int length, DoubleBufferedStreamReader stream) { int n = 0; while (length > 0) @@ -522,7 +522,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private int ReceiveAndExtend(int length, Stream stream) + private int ReceiveAndExtend(int length, DoubleBufferedStreamReader stream) { if (length == 1) { @@ -538,7 +538,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components return n + (-1 << length) + 1; } - private void DecodeBaseline(PdfJsFrameComponent component, ref short blockDataRef, int offset, ref PdfJsHuffmanTable dcHuffmanTable, ref PdfJsHuffmanTable acHuffmanTable, Stream stream) + private void DecodeBaseline(PdfJsFrameComponent component, ref short blockDataRef, int offset, ref PdfJsHuffmanTable dcHuffmanTable, ref PdfJsHuffmanTable acHuffmanTable, DoubleBufferedStreamReader stream) { short t = this.DecodeHuffman(ref dcHuffmanTable, stream); if (this.endOfStreamReached || this.unexpectedMarkerReached) @@ -587,7 +587,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeDCFirst(PdfJsFrameComponent component, ref short blockDataRef, int offset, ref PdfJsHuffmanTable dcHuffmanTable, Stream stream) + private void DecodeDCFirst(PdfJsFrameComponent component, ref short blockDataRef, int offset, ref PdfJsHuffmanTable dcHuffmanTable, DoubleBufferedStreamReader stream) { short t = this.DecodeHuffman(ref dcHuffmanTable, stream); if (this.endOfStreamReached || this.unexpectedMarkerReached) @@ -600,7 +600,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeDCSuccessive(PdfJsFrameComponent component, ref short blockDataRef, int offset, Stream stream) + private void DecodeDCSuccessive(PdfJsFrameComponent component, ref short blockDataRef, int offset, DoubleBufferedStreamReader stream) { int bit = this.ReadBit(stream); if (this.endOfStreamReached || this.unexpectedMarkerReached) @@ -611,7 +611,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components Unsafe.Add(ref blockDataRef, offset) |= (short)(bit << this.successiveState); } - private void DecodeACFirst(PdfJsFrameComponent component, ref short blockDataRef, int offset, ref PdfJsHuffmanTable acHuffmanTable, Stream stream) + private void DecodeACFirst(PdfJsFrameComponent component, ref short blockDataRef, int offset, ref PdfJsHuffmanTable acHuffmanTable, DoubleBufferedStreamReader stream) { if (this.eobrun > 0) { @@ -652,7 +652,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } } - private void DecodeACSuccessive(PdfJsFrameComponent component, ref short blockDataRef, int offset, ref PdfJsHuffmanTable acHuffmanTable, Stream stream) + private void DecodeACSuccessive(PdfJsFrameComponent component, ref short blockDataRef, int offset, ref PdfJsHuffmanTable acHuffmanTable, DoubleBufferedStreamReader stream) { int k = this.specStart; int e = this.specEnd; diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegConstants.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegConstants.cs index 2c369d390..437f77286 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegConstants.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegConstants.cs @@ -22,98 +22,98 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort /// /// The Start of Image marker /// - public const ushort SOI = 0xFFD8; + public const byte SOI = 0xD8; /// /// The End of Image marker /// - public const ushort EOI = 0xFFD9; + public const byte EOI = 0xD9; /// /// Application specific marker for marking the jpeg format. /// /// - public const ushort APP0 = 0xFFE0; + public const byte APP0 = 0xE0; /// /// Application specific marker for marking where to store metadata. /// - public const ushort APP1 = 0xFFE1; + public const byte APP1 = 0xE1; /// /// Application specific marker for marking where to store ICC profile information. /// - public const ushort APP2 = 0xFFE2; + public const byte APP2 = 0xE2; /// /// Application specific marker. /// - public const ushort APP3 = 0xFFE3; + public const byte APP3 = 0xE3; /// /// Application specific marker. /// - public const ushort APP4 = 0xFFE4; + public const byte APP4 = 0xE4; /// /// Application specific marker. /// - public const ushort APP5 = 0xFFE5; + public const byte APP5 = 0xE5; /// /// Application specific marker. /// - public const ushort APP6 = 0xFFE6; + public const byte APP6 = 0xE6; /// /// Application specific marker. /// - public const ushort APP7 = 0xFFE7; + public const byte APP7 = 0xE7; /// /// Application specific marker. /// - public const ushort APP8 = 0xFFE8; + public const byte APP8 = 0xE8; /// /// Application specific marker. /// - public const ushort APP9 = 0xFFE9; + public const byte APP9 = 0xE9; /// /// Application specific marker. /// - public const ushort APP10 = 0xFFEA; + public const byte APP10 = 0xEA; /// /// Application specific marker. /// - public const ushort APP11 = 0xFFEB; + public const byte APP11 = 0xEB; /// /// Application specific marker. /// - public const ushort APP12 = 0xFFEC; + public const byte APP12 = 0xEC; /// /// Application specific marker. /// - public const ushort APP13 = 0xFFED; + public const byte APP13 = 0xED; /// /// Application specific marker used by Adobe for storing encoding information for DCT filters. /// - public const ushort APP14 = 0xFFEE; + public const byte APP14 = 0xEE; /// /// Application specific marker used by GraphicConverter to store JPEG quality. /// - public const ushort APP15 = 0xFFEF; + public const byte APP15 = 0xEF; /// /// The text comment marker /// - public const ushort COM = 0xFFFE; + public const byte COM = 0xFE; /// /// Define Quantization Table(s) marker @@ -121,7 +121,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort /// Specifies one or more quantization tables. /// /// - public const ushort DQT = 0xFFDB; + public const byte DQT = 0xDB; /// /// Start of Frame (baseline DCT) @@ -130,7 +130,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort /// and component subsampling (e.g., 4:2:0). /// /// - public const ushort SOF0 = 0xFFC0; + public const byte SOF0 = 0xC0; /// /// Start Of Frame (Extended Sequential DCT) @@ -139,7 +139,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort /// and component subsampling (e.g., 4:2:0). /// /// - public const ushort SOF1 = 0xFFC1; + public const byte SOF1 = 0xC1; /// /// Start Of Frame (progressive DCT) @@ -148,7 +148,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort /// and component subsampling (e.g., 4:2:0). /// /// - public const ushort SOF2 = 0xFFC2; + public const byte SOF2 = 0xC2; /// /// Define Huffman Table(s) @@ -156,7 +156,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort /// Specifies one or more Huffman tables. /// /// - public const ushort DHT = 0xFFC4; + public const byte DHT = 0xC4; /// /// Define Restart Interval @@ -164,7 +164,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort /// Specifies the interval between RSTn markers, in macroblocks.This marker is followed by two bytes indicating the fixed size so it can be treated like any other variable size segment. /// /// - public const ushort DRI = 0xFFDD; + public const byte DRI = 0xDD; /// /// Start of Scan @@ -174,7 +174,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort /// will contain, and is immediately followed by entropy-coded data. /// /// - public const ushort SOS = 0xFFDA; + public const byte SOS = 0xDA; /// /// Define First Restart @@ -183,7 +183,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort /// Not used if there was no DRI marker. The low three bits of the marker code cycle in value from 0 to 7. /// /// - public const ushort RST0 = 0xFFD0; + public const byte RST0 = 0xD0; /// /// Define Eigth Restart @@ -192,7 +192,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort /// Not used if there was no DRI marker. The low three bits of the marker code cycle in value from 0 to 7. /// /// - public const ushort RST7 = 0xFFD7; + public const byte RST7 = 0xD7; /// /// Contains Adobe specific markers diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs index 244d97fba..1ba4826a2 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs @@ -111,7 +111,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort /// /// Gets the input stream. /// - public Stream InputStream { get; private set; } + public DoubleBufferedStreamReader InputStream { get; private set; } /// /// Gets a value indicating whether the metadata should be ignored when the image is being decoded. @@ -144,7 +144,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort /// The buffer to read file markers to /// The input stream /// The - public static PdfJsFileMarker FindNextFileMarker(byte[] marker, Stream stream) + public static PdfJsFileMarker FindNextFileMarker(byte[] marker, DoubleBufferedStreamReader stream) { int value = stream.Read(marker, 0, 2); @@ -157,7 +157,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort { // 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 (marker[1] == PdfJsJpegConstants.Markers.Prefix) + int m = marker[1]; + while (m == PdfJsJpegConstants.Markers.Prefix) { int suffix = stream.ReadByte(); if (suffix == -1) @@ -165,13 +166,15 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort return new PdfJsFileMarker(PdfJsJpegConstants.Markers.EOI, stream.Length - 2); } - marker[1] = (byte)suffix; + m = suffix; } - return new PdfJsFileMarker(BinaryPrimitives.ReadUInt16BigEndian(marker), stream.Position - 2); + marker[1] = (byte)m; + + return new PdfJsFileMarker((byte)m, stream.Position - 2); } - return new PdfJsFileMarker(BinaryPrimitives.ReadUInt16BigEndian(marker), stream.Position - 2, true); + return new PdfJsFileMarker(marker[1], stream.Position - 2, true); } /// @@ -194,7 +197,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort public IImageInfo Identify(Stream stream) { this.ParseStream(stream, true); - this.AssignResolution(); return new ImageInfo(new PixelTypeInfo(this.BitsPerPixel), this.ImageWidth, this.ImageHeight, this.MetaData); } @@ -206,119 +208,127 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort public void ParseStream(Stream stream, bool metadataOnly = false) { this.MetaData = new ImageMetaData(); - this.InputStream = stream; + this.InputStream = new DoubleBufferedStreamReader(stream); // Check for the Start Of Image marker. - var fileMarker = new PdfJsFileMarker(this.ReadUint16(), 0); + this.InputStream.Read(this.markerBuffer, 0, 2); + var fileMarker = new PdfJsFileMarker(this.markerBuffer[1], 0); if (fileMarker.Marker != PdfJsJpegConstants.Markers.SOI) { throw new ImageFormatException("Missing SOI marker."); } - ushort marker = this.ReadUint16(); + this.InputStream.Read(this.markerBuffer, 0, 2); + byte marker = this.markerBuffer[1]; fileMarker = new PdfJsFileMarker(marker, (int)this.InputStream.Position - 2); - this.QuantizationTables = new Block8x8F[4]; - - // this.quantizationTables = new PdfJsQuantizationTables(this.configuration.MemoryManager); - this.dcHuffmanTables = new PdfJsHuffmanTables(); - this.acHuffmanTables = new PdfJsHuffmanTables(); + // Only assign what we need + if (!metadataOnly) + { + this.QuantizationTables = new Block8x8F[4]; + this.dcHuffmanTables = new PdfJsHuffmanTables(); + this.acHuffmanTables = new PdfJsHuffmanTables(); + } while (fileMarker.Marker != PdfJsJpegConstants.Markers.EOI) { - // Get the marker length - int remaining = this.ReadUint16() - 2; - - switch (fileMarker.Marker) + if (!fileMarker.Invalid) { - case PdfJsJpegConstants.Markers.APP0: - this.ProcessApplicationHeaderMarker(remaining); - break; + // Get the marker length + int remaining = this.ReadUint16() - 2; - case PdfJsJpegConstants.Markers.APP1: - this.ProcessApp1Marker(remaining); - break; + switch (fileMarker.Marker) + { + case PdfJsJpegConstants.Markers.SOF0: + case PdfJsJpegConstants.Markers.SOF1: + case PdfJsJpegConstants.Markers.SOF2: - case PdfJsJpegConstants.Markers.APP2: - this.ProcessApp2Marker(remaining); - break; - case PdfJsJpegConstants.Markers.APP3: - case PdfJsJpegConstants.Markers.APP4: - case PdfJsJpegConstants.Markers.APP5: - case PdfJsJpegConstants.Markers.APP6: - case PdfJsJpegConstants.Markers.APP7: - case PdfJsJpegConstants.Markers.APP8: - case PdfJsJpegConstants.Markers.APP9: - case PdfJsJpegConstants.Markers.APP10: - case PdfJsJpegConstants.Markers.APP11: - case PdfJsJpegConstants.Markers.APP12: - case PdfJsJpegConstants.Markers.APP13: - this.InputStream.Skip(remaining); - break; + this.ProcessStartOfFrameMarker(remaining, fileMarker, metadataOnly); + break; - case PdfJsJpegConstants.Markers.APP14: - this.ProcessApp14Marker(remaining); - break; + case PdfJsJpegConstants.Markers.SOS: + if (!metadataOnly) + { + this.ProcessStartOfScanMarker(); + } - case PdfJsJpegConstants.Markers.APP15: - case PdfJsJpegConstants.Markers.COM: - this.InputStream.Skip(remaining); - break; + break; - case PdfJsJpegConstants.Markers.DQT: - if (metadataOnly) - { - this.InputStream.Skip(remaining); - } - else - { - this.ProcessDefineQuantizationTablesMarker(remaining); - } + case PdfJsJpegConstants.Markers.DHT: + if (metadataOnly) + { + this.InputStream.Skip(remaining); + } + else + { + this.ProcessDefineHuffmanTablesMarker(remaining); + } - break; + break; - case PdfJsJpegConstants.Markers.SOF0: - case PdfJsJpegConstants.Markers.SOF1: - case PdfJsJpegConstants.Markers.SOF2: - this.ProcessStartOfFrameMarker(remaining, fileMarker); - if (metadataOnly && !this.jFif.Equals(default)) - { - this.InputStream.Skip(remaining); - } + case PdfJsJpegConstants.Markers.DQT: + if (metadataOnly) + { + this.InputStream.Skip(remaining); + } + else + { + this.ProcessDefineQuantizationTablesMarker(remaining); + } - break; + break; - case PdfJsJpegConstants.Markers.DHT: - if (metadataOnly) - { - this.InputStream.Skip(remaining); - } - else - { - this.ProcessDefineHuffmanTablesMarker(remaining); - } + case PdfJsJpegConstants.Markers.DRI: + if (metadataOnly) + { + this.InputStream.Skip(remaining); + } + else + { + this.ProcessDefineRestartIntervalMarker(remaining); + } - break; + break; - case PdfJsJpegConstants.Markers.DRI: - if (metadataOnly) - { + case PdfJsJpegConstants.Markers.APP0: + + this.ProcessApplicationHeaderMarker(remaining); + break; + + case PdfJsJpegConstants.Markers.APP1: + + this.ProcessApp1Marker(remaining); + break; + + case PdfJsJpegConstants.Markers.APP2: + + this.ProcessApp2Marker(remaining); + break; + + case PdfJsJpegConstants.Markers.APP3: + case PdfJsJpegConstants.Markers.APP4: + case PdfJsJpegConstants.Markers.APP5: + case PdfJsJpegConstants.Markers.APP6: + case PdfJsJpegConstants.Markers.APP7: + case PdfJsJpegConstants.Markers.APP8: + case PdfJsJpegConstants.Markers.APP9: + case PdfJsJpegConstants.Markers.APP10: + case PdfJsJpegConstants.Markers.APP11: + case PdfJsJpegConstants.Markers.APP12: + case PdfJsJpegConstants.Markers.APP13: this.InputStream.Skip(remaining); - } - else - { - this.ProcessDefineRestartIntervalMarker(remaining); - } + break; - break; + case PdfJsJpegConstants.Markers.APP14: - case PdfJsJpegConstants.Markers.SOS: - if (!metadataOnly) - { - this.ProcessStartOfScanMarker(); - } + this.ProcessApp14Marker(remaining); + break; - break; + case PdfJsJpegConstants.Markers.APP15: + case PdfJsJpegConstants.Markers.COM: + this.InputStream.Skip(remaining); + break; + } } // Read on. @@ -328,6 +338,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort this.ImageWidth = this.Frame.SamplesPerLine; this.ImageHeight = this.Frame.Scanlines; this.ComponentCount = this.Frame.ComponentCount; + this.AssignResolution(); } /// @@ -379,7 +390,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort /// private void AssignResolution() { - if (this.isExif) + if (this.jFif.XDensity > 0 && this.jFif.YDensity > 0) + { + this.MetaData.HorizontalResolution = this.jFif.XDensity; + this.MetaData.VerticalResolution = this.jFif.YDensity; + } + else if (this.isExif) { double horizontalValue = this.MetaData.ExifProfile.TryGetValue(ExifTag.XResolution, out ExifValue horizontalTag) ? ((Rational)horizontalTag.Value).ToDouble() @@ -395,11 +411,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort this.MetaData.VerticalResolution = verticalValue; } } - else if (this.jFif.XDensity > 0 && this.jFif.YDensity > 0) - { - this.MetaData.HorizontalResolution = this.jFif.XDensity; - this.MetaData.VerticalResolution = this.jFif.YDensity; - } } /// @@ -593,7 +604,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort /// /// The remaining bytes in the segment block. /// The current frame marker. - private void ProcessStartOfFrameMarker(int remaining, PdfJsFileMarker frameMarker) + /// Whether to parse metadata only + private void ProcessStartOfFrameMarker(int remaining, PdfJsFileMarker frameMarker, bool metadataOnly) { if (this.Frame != null) { @@ -622,11 +634,14 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort int maxV = 0; int index = 6; - // No need to pool this. They max out at 4 - this.Frame.ComponentIds = new byte[this.Frame.ComponentCount]; - this.Frame.Components = new PdfJsFrameComponent[this.Frame.ComponentCount]; + if (!metadataOnly) + { + // No need to pool this. They max out at 4 + this.Frame.ComponentIds = new byte[this.Frame.ComponentCount]; + this.Frame.Components = new PdfJsFrameComponent[this.Frame.ComponentCount]; + } - for (int i = 0; i < this.Frame.Components.Length; i++) + for (int i = 0; i < this.Frame.ComponentCount; i++) { byte hv = this.temp[index + 1]; int h = hv >> 4; @@ -642,17 +657,24 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort maxV = v; } - var component = new PdfJsFrameComponent(this.configuration.MemoryManager, this.Frame, this.temp[index], h, v, this.temp[index + 2], i); + if (!metadataOnly) + { + var component = new PdfJsFrameComponent(this.configuration.MemoryManager, this.Frame, this.temp[index], h, v, this.temp[index + 2], i); - this.Frame.Components[i] = component; - this.Frame.ComponentIds[i] = component.Id; + this.Frame.Components[i] = component; + this.Frame.ComponentIds[i] = component.Id; + } index += 3; } this.Frame.MaxHorizontalFactor = maxH; this.Frame.MaxVerticalFactor = maxV; - this.Frame.InitComponents(); + + if (!metadataOnly) + { + this.Frame.InitComponents(); + } } /// @@ -799,7 +821,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort where TPixel : struct, IPixel { this.ColorSpace = this.DeduceJpegColorSpace(); - this.AssignResolution(); using (var postProcessor = new JpegImagePostProcessor(this.configuration.MemoryManager, this)) { var image = new Image(this.configuration, this.ImageWidth, this.ImageHeight, this.MetaData); diff --git a/tests/ImageSharp.Benchmarks/Codecs/Jpeg/Identify.cs b/tests/ImageSharp.Benchmarks/Codecs/Jpeg/Identify.cs new file mode 100644 index 000000000..79e9e9e76 --- /dev/null +++ b/tests/ImageSharp.Benchmarks/Codecs/Jpeg/Identify.cs @@ -0,0 +1,52 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +using System.IO; +using BenchmarkDotNet.Attributes; +using SixLabors.ImageSharp.Formats.Jpeg.GolangPort; +using SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort; +using SixLabors.ImageSharp.Tests; + +namespace SixLabors.ImageSharp.Benchmarks.Codecs.Jpeg +{ + [Config(typeof(Config.ShortClr))] + public class Identify + { + private byte[] jpegBytes; + + private string TestImageFullPath => Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, this.TestImage); + + [Params(TestImages.Jpeg.Baseline.Jpeg420Exif, TestImages.Jpeg.Baseline.Calliphora)] + public string TestImage { get; set; } + + [GlobalSetup] + public void ReadImages() + { + if (this.jpegBytes == null) + { + this.jpegBytes = File.ReadAllBytes(this.TestImageFullPath); + } + } + + [Benchmark] + public IImageInfo IdentifyGolang() + { + using (var memoryStream = new MemoryStream(this.jpegBytes)) + { + var decoder = new OrigJpegDecoder(); + + return decoder.Identify(Configuration.Default, memoryStream); + } + } + + [Benchmark] + public IImageInfo IdentifyPdfJs() + { + using (var memoryStream = new MemoryStream(this.jpegBytes)) + { + var decoder = new PdfJsJpegDecoder(); + return decoder.Identify(Configuration.Default, memoryStream); + } + } + } +} diff --git a/tests/ImageSharp.Benchmarks/General/DoubleBufferedStreams.cs b/tests/ImageSharp.Benchmarks/General/DoubleBufferedStreams.cs new file mode 100644 index 000000000..665d0cbad --- /dev/null +++ b/tests/ImageSharp.Benchmarks/General/DoubleBufferedStreams.cs @@ -0,0 +1,53 @@ +using System; +using System.IO; +using BenchmarkDotNet.Attributes; +using SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components; + +namespace SixLabors.ImageSharp.Benchmarks.General +{ + [Config(typeof(Config.ShortClr))] + public class DoubleBufferedStreams + { + private byte[] buffer = CreateTestBytes(); + + [Benchmark] + public int StandardStream() + { + int r = 0; + using (var stream = new MemoryStream(this.buffer)) + { + for (int i = 0; i < stream.Length; i++) + { + r += stream.ReadByte(); + } + } + + return r; + } + + [Benchmark] + public int ChunkedStream() + { + int r = 0; + using (var stream = new MemoryStream(this.buffer)) + { + var reader = new DoubleBufferedStreamReader(stream); + for (int i = 0; i < reader.Length; i++) + { + r += reader.ReadByte(); + } + } + + return r; + } + + private static byte[] CreateTestBytes() + { + byte[] buffer = new byte[DoubleBufferedStreamReader.ChunkLength * 3]; + var random = new Random(); + random.NextBytes(buffer); + + return buffer; + } + } +} diff --git a/tests/ImageSharp.Tests/Formats/Jpg/DoubleBufferedStreamReaderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/DoubleBufferedStreamReaderTests.cs new file mode 100644 index 000000000..c1049f025 --- /dev/null +++ b/tests/ImageSharp.Tests/Formats/Jpg/DoubleBufferedStreamReaderTests.cs @@ -0,0 +1,132 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.IO; +using SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components; +using Xunit; + +namespace SixLabors.ImageSharp.Tests.Formats.Jpg +{ + public class DoubleBufferedStreamReaderTests + { + [Fact] + public void DoubleBufferedStreamReaderCanReadSingleByteFromOrigin() + { + using (MemoryStream stream = CreateTestStream()) + { + byte[] expected = stream.ToArray(); + var reader = new DoubleBufferedStreamReader(stream); + + Assert.Equal(expected[0], reader.ReadByte()); + + // We've read a whole chunk but increment by 1 in our reader. + Assert.Equal(stream.Position, DoubleBufferedStreamReader.ChunkLength); + Assert.Equal(1, reader.Position); + } + } + + [Fact] + public void DoubleBufferedStreamReaderCanReadSubsequentSingleByteCorrectly() + { + using (MemoryStream stream = CreateTestStream()) + { + byte[] expected = stream.ToArray(); + var reader = new DoubleBufferedStreamReader(stream); + + for (int i = 0; i < expected.Length; i++) + { + Assert.Equal(expected[i], reader.ReadByte()); + Assert.Equal(i + 1, reader.Position); + + if (i < DoubleBufferedStreamReader.ChunkLength) + { + Assert.Equal(stream.Position, DoubleBufferedStreamReader.ChunkLength); + } + else if (i >= DoubleBufferedStreamReader.ChunkLength && i < DoubleBufferedStreamReader.ChunkLength * 2) + { + // We should have advanced to the second chunk now. + Assert.Equal(stream.Position, DoubleBufferedStreamReader.ChunkLength * 2); + } + else + { + // We should have advanced to the third chunk now. + Assert.Equal(stream.Position, DoubleBufferedStreamReader.ChunkLength * 3); + } + } + } + } + + [Fact] + public void DoubleBufferedStreamReaderCanReadMultipleBytesFromOrigin() + { + using (MemoryStream stream = CreateTestStream()) + { + byte[] buffer = new byte[2]; + byte[] expected = stream.ToArray(); + var reader = new DoubleBufferedStreamReader(stream); + + Assert.Equal(2, reader.Read(buffer, 0, 2)); + Assert.Equal(expected[0], buffer[0]); + Assert.Equal(expected[1], buffer[1]); + + // We've read a whole chunk but increment by the buffer length in our reader. + Assert.Equal(stream.Position, DoubleBufferedStreamReader.ChunkLength); + Assert.Equal(buffer.Length, reader.Position); + } + } + + [Fact] + public void DoubleBufferedStreamReaderCanReadSubsequentMultipleByteCorrectly() + { + using (MemoryStream stream = CreateTestStream()) + { + byte[] buffer = new byte[2]; + byte[] expected = stream.ToArray(); + var reader = new DoubleBufferedStreamReader(stream); + + for (int i = 0, o = 0; i < expected.Length / 2; i++, o += 2) + { + if (o + 2 == expected.Length) + { + // We've reached the end of the stream + Assert.Equal(0, reader.Read(buffer, 0, 2)); + } + else + { + Assert.Equal(2, reader.Read(buffer, 0, 2)); + } + + Assert.Equal(expected[o], buffer[0]); + Assert.Equal(expected[o + 1], buffer[1]); + Assert.Equal(o + 2, reader.Position); + + int offset = i * 2; + if (offset < DoubleBufferedStreamReader.ChunkLength) + { + Assert.Equal(stream.Position, DoubleBufferedStreamReader.ChunkLength); + } + else if (offset >= DoubleBufferedStreamReader.ChunkLength && offset < DoubleBufferedStreamReader.ChunkLength * 2) + { + // We should have advanced to the second chunk now. + Assert.Equal(stream.Position, DoubleBufferedStreamReader.ChunkLength * 2); + } + else + { + // We should have advanced to the third chunk now. + Assert.Equal(stream.Position, DoubleBufferedStreamReader.ChunkLength * 3); + } + } + } + } + + private MemoryStream CreateTestStream() + { + byte[] buffer = new byte[DoubleBufferedStreamReader.ChunkLength * 3]; + var random = new Random(); + random.NextBytes(buffer); + + return new MemoryStream(buffer); + } + } +} From 571f66e8a1773a4713157852c8c64581ef6511d4 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 28 Apr 2018 12:10:41 +1000 Subject: [PATCH 02/17] Golang is skipping EXIF reading when it shouldn't. --- .../Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs | 9 ++------- .../Codecs/Jpeg/{Identify.cs => IdentifyJpeg.cs} | 2 +- .../Formats/Jpg/JpegDecoderTests.cs | 2 ++ .../Formats/Jpg/ParseStreamTests.cs | 16 ++++++++-------- 4 files changed, 13 insertions(+), 16 deletions(-) rename tests/ImageSharp.Benchmarks/Codecs/Jpeg/{Identify.cs => IdentifyJpeg.cs} (98%) diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs index 1ba4826a2..2322758ee 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs @@ -22,7 +22,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort { /// /// Performs the jpeg decoding operation. - /// Ported from with additional fixes to handle common encoding errors + /// Originally ported from + /// with additional fixes for both performance and common encoding errors. /// internal sealed class PdfJsJpegDecoderCore : IRawJpegData { @@ -31,7 +32,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort /// public const int SupportedPrecision = 8; -#pragma warning disable SA1401 // Fields should be private /// /// The global configuration /// @@ -242,7 +242,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort case PdfJsJpegConstants.Markers.SOF0: case PdfJsJpegConstants.Markers.SOF1: case PdfJsJpegConstants.Markers.SOF2: - this.ProcessStartOfFrameMarker(remaining, fileMarker, metadataOnly); break; @@ -291,17 +290,14 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort break; case PdfJsJpegConstants.Markers.APP0: - this.ProcessApplicationHeaderMarker(remaining); break; case PdfJsJpegConstants.Markers.APP1: - this.ProcessApp1Marker(remaining); break; case PdfJsJpegConstants.Markers.APP2: - this.ProcessApp2Marker(remaining); break; @@ -320,7 +316,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort break; case PdfJsJpegConstants.Markers.APP14: - this.ProcessApp14Marker(remaining); break; diff --git a/tests/ImageSharp.Benchmarks/Codecs/Jpeg/Identify.cs b/tests/ImageSharp.Benchmarks/Codecs/Jpeg/IdentifyJpeg.cs similarity index 98% rename from tests/ImageSharp.Benchmarks/Codecs/Jpeg/Identify.cs rename to tests/ImageSharp.Benchmarks/Codecs/Jpeg/IdentifyJpeg.cs index 79e9e9e76..c3c128100 100644 --- a/tests/ImageSharp.Benchmarks/Codecs/Jpeg/Identify.cs +++ b/tests/ImageSharp.Benchmarks/Codecs/Jpeg/IdentifyJpeg.cs @@ -10,7 +10,7 @@ using SixLabors.ImageSharp.Tests; namespace SixLabors.ImageSharp.Benchmarks.Codecs.Jpeg { [Config(typeof(Config.ShortClr))] - public class Identify + public class IdentifyJpeg { private byte[] jpegBytes; diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 0b8daac72..701d6b5d7 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -478,6 +478,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg [InlineData(TestImages.Jpeg.Baseline.Ycck, 32)] [InlineData(TestImages.Jpeg.Baseline.Jpeg400, 8)] [InlineData(TestImages.Jpeg.Baseline.Snake, 24)] + [InlineData(TestImages.Jpeg.Baseline.Jpeg420Exif, 24)] public void DetectPixelSizeGolang(string imagePath, int expectedPixelSize) { var testFile = TestFile.Create(imagePath); @@ -494,6 +495,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg [InlineData(TestImages.Jpeg.Baseline.Ycck, 32)] [InlineData(TestImages.Jpeg.Baseline.Jpeg400, 8)] [InlineData(TestImages.Jpeg.Baseline.Snake, 24)] + [InlineData(TestImages.Jpeg.Baseline.Jpeg420Exif, 24)] public void DetectPixelSizePdfJs(string imagePath, int expectedPixelSize) { var testFile = TestFile.Create(imagePath); diff --git a/tests/ImageSharp.Tests/Formats/Jpg/ParseStreamTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/ParseStreamTests.cs index 0d563a7b7..b665d69e8 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/ParseStreamTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/ParseStreamTests.cs @@ -33,7 +33,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg { var expecteColorSpace = (JpegColorSpace)expectedColorSpaceValue; - using (OrigJpegDecoderCore decoder = JpegFixture.ParseStream(imageFile, true)) + using (OrigJpegDecoderCore decoder = JpegFixture.ParseStream(imageFile, false)) { Assert.Equal(expecteColorSpace, decoder.ColorSpace); } @@ -42,11 +42,11 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg [Fact] public void ComponentScalingIsCorrect_1ChannelJpeg() { - using (OrigJpegDecoderCore decoder = JpegFixture.ParseStream(TestImages.Jpeg.Baseline.Jpeg400, true)) + using (OrigJpegDecoderCore decoder = JpegFixture.ParseStream(TestImages.Jpeg.Baseline.Jpeg400, false)) { Assert.Equal(1, decoder.ComponentCount); Assert.Equal(1, decoder.Components.Length); - + Size expectedSizeInBlocks = decoder.ImageSizeInPixels.DivideRoundUp(8); Assert.Equal(expectedSizeInBlocks, decoder.ImageSizeInMCU); @@ -68,7 +68,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg { var sb = new StringBuilder(); - using (OrigJpegDecoderCore decoder = JpegFixture.ParseStream(imageFile, true)) + using (OrigJpegDecoderCore decoder = JpegFixture.ParseStream(imageFile, false)) { sb.AppendLine(imageFile); sb.AppendLine($"Size:{decoder.ImageSizeInPixels} MCU:{decoder.ImageSizeInMCU}"); @@ -103,23 +103,23 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg Size fLuma = (Size)expectedLumaFactors; Size fChroma = (Size)expectedChromaFactors; - using (OrigJpegDecoderCore decoder = JpegFixture.ParseStream(imageFile, true)) + using (OrigJpegDecoderCore decoder = JpegFixture.ParseStream(imageFile, false)) { Assert.Equal(componentCount, decoder.ComponentCount); Assert.Equal(componentCount, decoder.Components.Length); - + OrigComponent c0 = decoder.Components[0]; OrigComponent c1 = decoder.Components[1]; OrigComponent c2 = decoder.Components[2]; var uniform1 = new Size(1, 1); - Size expectedLumaSizeInBlocks = decoder.ImageSizeInMCU.MultiplyBy(fLuma) ; + Size expectedLumaSizeInBlocks = decoder.ImageSizeInMCU.MultiplyBy(fLuma); Size divisor = fLuma.DivideBy(fChroma); Size expectedChromaSizeInBlocks = expectedLumaSizeInBlocks.DivideRoundUp(divisor); - + VerifyJpeg.VerifyComponent(c0, expectedLumaSizeInBlocks, fLuma, uniform1); VerifyJpeg.VerifyComponent(c1, expectedChromaSizeInBlocks, fChroma, divisor); VerifyJpeg.VerifyComponent(c2, expectedChromaSizeInBlocks, fChroma, divisor); From 01ab0d16ecf7fa2b14da2c2cf740bf1bc50743f1 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 28 Apr 2018 12:48:36 +1000 Subject: [PATCH 03/17] Pool buffer --- .../Components/DoubleBufferedStreamReader.cs | 18 ++++- .../Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs | 4 +- .../Codecs/Jpeg/DoubleBufferedStreams.cs | 73 +++++++++++++++++++ .../General/DoubleBufferedStreams.cs | 53 -------------- 4 files changed, 90 insertions(+), 58 deletions(-) create mode 100644 tests/ImageSharp.Benchmarks/Codecs/Jpeg/DoubleBufferedStreams.cs delete mode 100644 tests/ImageSharp.Benchmarks/General/DoubleBufferedStreams.cs diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs index 0818d7309..d8a43428d 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs @@ -3,6 +3,7 @@ using System; using System.IO; +using SixLabors.ImageSharp.Memory; // TODO: This could be useful elsewhere. namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components @@ -11,7 +12,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components /// A stream reader that add a secondary level buffer in addition to native stream buffered reading /// to reduce the overhead of small incremental reads. /// - internal class DoubleBufferedStreamReader + internal class DoubleBufferedStreamReader : IDisposable { /// /// The length, in bytes, of the chunk @@ -20,6 +21,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components private readonly Stream stream; + private readonly IManagedByteBuffer buffer; + private readonly byte[] chunk; private int bytesRead; @@ -29,14 +32,15 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components /// /// Initializes a new instance of the class. /// + /// The to use for buffer allocations. /// The input stream. - public DoubleBufferedStreamReader(Stream stream) + public DoubleBufferedStreamReader(MemoryManager memoryManager, Stream stream) { this.stream = stream; this.Length = stream.Length; - // TODO: Consider pooling this. - this.chunk = new byte[ChunkLength]; + this.buffer = memoryManager.AllocateCleanManagedByteBuffer(ChunkLength); + this.chunk = this.buffer.Array; } /// @@ -147,5 +151,11 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components return Math.Max(n, 0); } + + /// + public void Dispose() + { + this.buffer?.Dispose(); + } } } \ No newline at end of file diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs index 2322758ee..c20f283d7 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs @@ -208,7 +208,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort public void ParseStream(Stream stream, bool metadataOnly = false) { this.MetaData = new ImageMetaData(); - this.InputStream = new DoubleBufferedStreamReader(stream); + this.InputStream = new DoubleBufferedStreamReader(this.configuration.MemoryManager, stream); // Check for the Start Of Image marker. this.InputStream.Read(this.markerBuffer, 0, 2); @@ -339,9 +339,11 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort /// public void Dispose() { + this.InputStream?.Dispose(); this.Frame?.Dispose(); // Set large fields to null. + this.InputStream = null; this.Frame = null; this.dcHuffmanTables = null; this.acHuffmanTables = null; diff --git a/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DoubleBufferedStreams.cs b/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DoubleBufferedStreams.cs new file mode 100644 index 000000000..d178a4970 --- /dev/null +++ b/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DoubleBufferedStreams.cs @@ -0,0 +1,73 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.IO; +using BenchmarkDotNet.Attributes; +using SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components; + +namespace SixLabors.ImageSharp.Benchmarks.Codecs.Jpeg +{ + [Config(typeof(Config.ShortClr))] + public class DoubleBufferedStreams + { + private byte[] buffer = CreateTestBytes(); + + private MemoryStream stream1; + private MemoryStream stream2; + DoubleBufferedStreamReader reader; + + [GlobalSetup] + public void CreateStreams() + { + this.stream1 = new MemoryStream(this.buffer); + this.stream2 = new MemoryStream(this.buffer); + this.reader = new DoubleBufferedStreamReader(Configuration.Default.MemoryManager, this.stream2); + } + + [GlobalCleanup] + public void DestroyStreams() + { + this.stream1?.Dispose(); + this.stream2?.Dispose(); + this.reader?.Dispose(); + } + + [Benchmark(Baseline = true)] + public int StandardStream() + { + int r = 0; + Stream stream = this.stream1; + + for (int i = 0; i < stream.Length; i++) + { + r += stream.ReadByte(); + } + + return r; + } + + [Benchmark] + public int DoubleBufferedStream() + { + int r = 0; + DoubleBufferedStreamReader reader = this.reader; + + for (int i = 0; i < reader.Length; i++) + { + r += reader.ReadByte(); + } + + return r; + } + + private static byte[] CreateTestBytes() + { + byte[] buffer = new byte[DoubleBufferedStreamReader.ChunkLength * 3]; + var random = new Random(); + random.NextBytes(buffer); + + return buffer; + } + } +} diff --git a/tests/ImageSharp.Benchmarks/General/DoubleBufferedStreams.cs b/tests/ImageSharp.Benchmarks/General/DoubleBufferedStreams.cs deleted file mode 100644 index 665d0cbad..000000000 --- a/tests/ImageSharp.Benchmarks/General/DoubleBufferedStreams.cs +++ /dev/null @@ -1,53 +0,0 @@ -using System; -using System.IO; -using BenchmarkDotNet.Attributes; -using SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components; - -namespace SixLabors.ImageSharp.Benchmarks.General -{ - [Config(typeof(Config.ShortClr))] - public class DoubleBufferedStreams - { - private byte[] buffer = CreateTestBytes(); - - [Benchmark] - public int StandardStream() - { - int r = 0; - using (var stream = new MemoryStream(this.buffer)) - { - for (int i = 0; i < stream.Length; i++) - { - r += stream.ReadByte(); - } - } - - return r; - } - - [Benchmark] - public int ChunkedStream() - { - int r = 0; - using (var stream = new MemoryStream(this.buffer)) - { - var reader = new DoubleBufferedStreamReader(stream); - for (int i = 0; i < reader.Length; i++) - { - r += reader.ReadByte(); - } - } - - return r; - } - - private static byte[] CreateTestBytes() - { - byte[] buffer = new byte[DoubleBufferedStreamReader.ChunkLength * 3]; - var random = new Random(); - random.NextBytes(buffer); - - return buffer; - } - } -} From a5c0ec7582c6d77bb6443c78c572b8fcc745c8bf Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 28 Apr 2018 17:59:19 +1000 Subject: [PATCH 04/17] Ensure buffer is always aligned and fix tests --- .../Components/DoubleBufferedStreamReader.cs | 11 +++-------- .../Formats/Jpg/DoubleBufferedStreamReaderTests.cs | 11 +++++++---- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs index d8a43428d..eb562424d 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs @@ -15,7 +15,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components internal class DoubleBufferedStreamReader : IDisposable { /// - /// The length, in bytes, of the chunk + /// The length, in bytes, of the buffering chunk /// public const int ChunkLength = 4096; @@ -38,7 +38,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components { this.stream = stream; this.Length = stream.Length; - this.buffer = memoryManager.AllocateCleanManagedByteBuffer(ChunkLength); this.chunk = this.buffer.Array; } @@ -62,7 +61,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components { // Reset everything. It's easier than tracking. this.position = value; - this.stream.Seek(this.position, SeekOrigin.Begin); this.bytesRead = ChunkLength; } } @@ -96,8 +94,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components /// The number of bytes to skip public void Skip(int count) { - this.position += count; - this.bytesRead += count; + this.Position += count; } /// @@ -144,9 +141,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components this.stream.Seek(this.position, SeekOrigin.Begin); n = this.stream.Read(buffer, offset, count); - // Ensure next read fills the chunk - this.bytesRead = ChunkLength; - this.position += count; + this.Position += count; } return Math.Max(n, 0); diff --git a/tests/ImageSharp.Tests/Formats/Jpg/DoubleBufferedStreamReaderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/DoubleBufferedStreamReaderTests.cs index c1049f025..61017ce9b 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/DoubleBufferedStreamReaderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/DoubleBufferedStreamReaderTests.cs @@ -4,19 +4,22 @@ using System; using System.IO; using SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components; +using SixLabors.ImageSharp.Memory; using Xunit; namespace SixLabors.ImageSharp.Tests.Formats.Jpg { public class DoubleBufferedStreamReaderTests { + private MemoryManager manager = Configuration.Default.MemoryManager; + [Fact] public void DoubleBufferedStreamReaderCanReadSingleByteFromOrigin() { using (MemoryStream stream = CreateTestStream()) { byte[] expected = stream.ToArray(); - var reader = new DoubleBufferedStreamReader(stream); + var reader = new DoubleBufferedStreamReader(this.manager, stream); Assert.Equal(expected[0], reader.ReadByte()); @@ -32,7 +35,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg using (MemoryStream stream = CreateTestStream()) { byte[] expected = stream.ToArray(); - var reader = new DoubleBufferedStreamReader(stream); + var reader = new DoubleBufferedStreamReader(this.manager, stream); for (int i = 0; i < expected.Length; i++) { @@ -64,7 +67,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg { byte[] buffer = new byte[2]; byte[] expected = stream.ToArray(); - var reader = new DoubleBufferedStreamReader(stream); + var reader = new DoubleBufferedStreamReader(this.manager, stream); Assert.Equal(2, reader.Read(buffer, 0, 2)); Assert.Equal(expected[0], buffer[0]); @@ -83,7 +86,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg { byte[] buffer = new byte[2]; byte[] expected = stream.ToArray(); - var reader = new DoubleBufferedStreamReader(stream); + var reader = new DoubleBufferedStreamReader(this.manager, stream); for (int i = 0, o = 0; i < expected.Length / 2; i++, o += 2) { From 35c1fa7473a905fa7e580e45c26eb25ba7e149be Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 28 Apr 2018 23:02:58 +1000 Subject: [PATCH 05/17] Ben Adams is a wizard --- .../Components/DoubleBufferedStreamReader.cs | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs index eb562424d..458ddc462 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs @@ -3,6 +3,7 @@ using System; using System.IO; +using System.Runtime.CompilerServices; using SixLabors.ImageSharp.Memory; // TODO: This could be useful elsewhere. @@ -70,6 +71,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components /// byte, or returns -1 if at the end of the stream. /// /// The unsigned byte cast to an , or -1 if at the end of the stream. + [MethodImpl(MethodImplOptions.AggressiveInlining)] public int ReadByte() { if (this.position >= this.Length) @@ -79,13 +81,13 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components if (this.position == 0 || this.bytesRead >= ChunkLength) { - this.stream.Seek(this.position, SeekOrigin.Begin); - this.stream.Read(this.chunk, 0, ChunkLength); - this.bytesRead = 0; + return this.ReadByteSlow(); + } + else + { + this.position++; + return this.chunk[this.bytesRead++]; } - - this.position++; - return this.chunk[this.bytesRead++]; } /// @@ -152,5 +154,16 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components { this.buffer?.Dispose(); } + + [MethodImpl(MethodImplOptions.NoInlining)] + private int ReadByteSlow() + { + this.stream.Seek(this.position, SeekOrigin.Begin); + this.stream.Read(this.chunk, 0, ChunkLength); + this.bytesRead = 0; + + this.position++; + return this.chunk[this.bytesRead++]; + } } } \ No newline at end of file From 76429a03c623688d338b5e1a45112b89de76527a Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 29 Apr 2018 18:37:01 +1000 Subject: [PATCH 06/17] Faster multibyte reading and fix test --- .../Components/DoubleBufferedStreamReader.cs | 150 ++++++++++++++---- .../Codecs/Jpeg/DoubleBufferedStreams.cs | 53 ++++++- .../Jpg/DoubleBufferedStreamReaderTests.cs | 10 +- 3 files changed, 164 insertions(+), 49 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs index 458ddc462..90f55bc5d 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs @@ -20,15 +20,21 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components /// public const int ChunkLength = 4096; + private const int ChunkLengthMinusOne = ChunkLength - 1; + + private const int ChunkLengthPlusOne = ChunkLength + 1; + private readonly Stream stream; private readonly IManagedByteBuffer buffer; - private readonly byte[] chunk; + private readonly byte[] bufferChunk; private int bytesRead; - private long position; + private int position; + + private int length; /// /// Initializes a new instance of the class. @@ -38,30 +44,27 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components public DoubleBufferedStreamReader(MemoryManager memoryManager, Stream stream) { this.stream = stream; - this.Length = stream.Length; + this.length = (int)stream.Length; this.buffer = memoryManager.AllocateCleanManagedByteBuffer(ChunkLength); - this.chunk = this.buffer.Array; + this.bufferChunk = this.buffer.Array; } /// /// Gets the length, in bytes, of the stream /// - public long Length { get; } + public long Length => this.length; /// /// Gets or sets the current position within the stream /// public long Position { - get - { - return this.position; - } + get => this.position; set { // Reset everything. It's easier than tracking. - this.position = value; + this.position = (int)value; this.bytesRead = ChunkLength; } } @@ -74,19 +77,19 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components [MethodImpl(MethodImplOptions.AggressiveInlining)] public int ReadByte() { - if (this.position >= this.Length) + if (this.position >= this.length) { return -1; } - if (this.position == 0 || this.bytesRead >= ChunkLength) + if (this.position == 0 || this.bytesRead > ChunkLengthMinusOne) { return this.ReadByteSlow(); } else { this.position++; - return this.chunk[this.bytesRead++]; + return this.bufferChunk[this.bytesRead++]; } } @@ -94,6 +97,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components /// Skips the number of bytes in the stream /// /// The number of bytes to skip + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Skip(int count) { this.Position += count; @@ -118,35 +122,50 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components /// of bytes requested if that many bytes are not currently available, or zero (0) /// if the end of the stream has been reached. /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] public int Read(byte[] buffer, int offset, int count) { - int n = 0; - if (buffer.Length <= ChunkLength) + if (buffer.Length < ChunkLengthPlusOne) { if (this.position == 0 || count + this.bytesRead > ChunkLength) { - // Refill our buffer then copy. - this.stream.Seek(this.position, SeekOrigin.Begin); - this.stream.Read(this.chunk, 0, ChunkLength); - this.bytesRead = 0; + return this.ReadToChunkSlow(buffer, offset, count); } - Buffer.BlockCopy(this.chunk, this.bytesRead, buffer, offset, count); - this.position += count; - this.bytesRead += count; + int n = this.length - this.position; + if (n > count) + { + n = count; + } - n = Math.Min(count, (int)(this.Length - this.position)); - } - else - { - // Read to target but don't copy to our chunk. - this.stream.Seek(this.position, SeekOrigin.Begin); - n = this.stream.Read(buffer, offset, count); + if (n < 0) + { + n = 0; + } - this.Position += count; + if (n < 9) + { + int byteCount = n; + int read = this.bytesRead; + byte[] chunk = this.bufferChunk; + + while (--byteCount > -1) + { + buffer[offset + byteCount] = chunk[read + byteCount]; + } + } + else + { + Buffer.BlockCopy(this.bufferChunk, this.bytesRead, buffer, offset, n); + } + + this.position += n; + this.bytesRead += n; + + return n; } - return Math.Max(n, 0); + return this.ReadToBufferSlow(buffer, offset, count); } /// @@ -158,12 +177,75 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components [MethodImpl(MethodImplOptions.NoInlining)] private int ReadByteSlow() { - this.stream.Seek(this.position, SeekOrigin.Begin); - this.stream.Read(this.chunk, 0, ChunkLength); + if (this.position != this.stream.Position) + { + this.stream.Seek(this.position, SeekOrigin.Begin); + } + + this.stream.Read(this.bufferChunk, 0, ChunkLength); this.bytesRead = 0; this.position++; - return this.chunk[this.bytesRead++]; + return this.bufferChunk[this.bytesRead++]; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private int ReadToChunkSlow(byte[] buffer, int offset, int count) + { + // Refill our buffer then copy. + if (this.position != this.stream.Position) + { + this.stream.Seek(this.position, SeekOrigin.Begin); + } + + this.stream.Read(this.bufferChunk, 0, ChunkLength); + this.bytesRead = 0; + + int n = this.length - this.position; + if (n > count) + { + n = count; + } + + if (n < 0) + { + n = 0; + } + + if (n < 9) + { + int byteCount = n; + int read = this.bytesRead; + byte[] chunk = this.bufferChunk; + + while (--byteCount > -1) + { + buffer[offset + byteCount] = chunk[read + byteCount]; + } + } + else + { + Buffer.BlockCopy(this.bufferChunk, this.bytesRead, buffer, offset, n); + } + + this.position += n; + this.bytesRead += n; + + return n; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private int ReadToBufferSlow(byte[] buffer, int offset, int count) + { + // Read to target but don't copy to our chunk. + if (this.position != this.stream.Position) + { + this.stream.Seek(this.position, SeekOrigin.Begin); + } + + int n = this.stream.Read(buffer, offset, count); + this.Position += n; + return n; } } } \ No newline at end of file diff --git a/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DoubleBufferedStreams.cs b/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DoubleBufferedStreams.cs index d178a4970..1d76d58a5 100644 --- a/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DoubleBufferedStreams.cs +++ b/tests/ImageSharp.Benchmarks/Codecs/Jpeg/DoubleBufferedStreams.cs @@ -12,17 +12,25 @@ namespace SixLabors.ImageSharp.Benchmarks.Codecs.Jpeg public class DoubleBufferedStreams { private byte[] buffer = CreateTestBytes(); + private byte[] chunk1 = new byte[2]; + private byte[] chunk2 = new byte[2]; private MemoryStream stream1; private MemoryStream stream2; - DoubleBufferedStreamReader reader; + private MemoryStream stream3; + private MemoryStream stream4; + DoubleBufferedStreamReader reader1; + DoubleBufferedStreamReader reader2; [GlobalSetup] public void CreateStreams() { this.stream1 = new MemoryStream(this.buffer); this.stream2 = new MemoryStream(this.buffer); - this.reader = new DoubleBufferedStreamReader(Configuration.Default.MemoryManager, this.stream2); + this.stream3 = new MemoryStream(this.buffer); + this.stream4 = new MemoryStream(this.buffer); + this.reader1 = new DoubleBufferedStreamReader(Configuration.Default.MemoryManager, this.stream2); + this.reader2 = new DoubleBufferedStreamReader(Configuration.Default.MemoryManager, this.stream2); } [GlobalCleanup] @@ -30,11 +38,14 @@ namespace SixLabors.ImageSharp.Benchmarks.Codecs.Jpeg { this.stream1?.Dispose(); this.stream2?.Dispose(); - this.reader?.Dispose(); + this.stream3?.Dispose(); + this.stream4?.Dispose(); + this.reader1?.Dispose(); + this.reader2?.Dispose(); } [Benchmark(Baseline = true)] - public int StandardStream() + public int StandardStreamReadByte() { int r = 0; Stream stream = this.stream1; @@ -48,10 +59,25 @@ namespace SixLabors.ImageSharp.Benchmarks.Codecs.Jpeg } [Benchmark] - public int DoubleBufferedStream() + public int StandardStreamRead() { int r = 0; - DoubleBufferedStreamReader reader = this.reader; + Stream stream = this.stream1; + byte[] b = this.chunk1; + + for (int i = 0; i < stream.Length / 2; i++) + { + r += stream.Read(b, 0, 2); + } + + return r; + } + + [Benchmark] + public int DoubleBufferedStreamReadByte() + { + int r = 0; + DoubleBufferedStreamReader reader = this.reader1; for (int i = 0; i < reader.Length; i++) { @@ -61,6 +87,21 @@ namespace SixLabors.ImageSharp.Benchmarks.Codecs.Jpeg return r; } + [Benchmark] + public int DoubleBufferedStreamRead() + { + int r = 0; + DoubleBufferedStreamReader reader = this.reader2; + byte[] b = this.chunk2; + + for (int i = 0; i < reader.Length / 2; i++) + { + r += reader.Read(b, 0, 2); + } + + return r; + } + private static byte[] CreateTestBytes() { byte[] buffer = new byte[DoubleBufferedStreamReader.ChunkLength * 3]; diff --git a/tests/ImageSharp.Tests/Formats/Jpg/DoubleBufferedStreamReaderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/DoubleBufferedStreamReaderTests.cs index 61017ce9b..bc099bdc5 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/DoubleBufferedStreamReaderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/DoubleBufferedStreamReaderTests.cs @@ -90,16 +90,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg for (int i = 0, o = 0; i < expected.Length / 2; i++, o += 2) { - if (o + 2 == expected.Length) - { - // We've reached the end of the stream - Assert.Equal(0, reader.Read(buffer, 0, 2)); - } - else - { - Assert.Equal(2, reader.Read(buffer, 0, 2)); - } + Assert.Equal(2, reader.Read(buffer, 0, 2)); Assert.Equal(expected[o], buffer[0]); Assert.Equal(expected[o + 1], buffer[1]); Assert.Equal(o + 2, reader.Position); From a4f3392013b3dc03576101da16dbba2aa3b9861f Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 30 Apr 2018 00:06:16 +1000 Subject: [PATCH 07/17] Slight perf tweak plus duplicate refactoring --- .../Components/DoubleBufferedStreamReader.cs | 128 ++++++++---------- 1 file changed, 57 insertions(+), 71 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs index 90f55bc5d..164ca7cc1 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs @@ -4,6 +4,7 @@ using System; using System.IO; using System.Runtime.CompilerServices; + using SixLabors.ImageSharp.Memory; // TODO: This could be useful elsewhere. @@ -22,20 +23,18 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components private const int ChunkLengthMinusOne = ChunkLength - 1; - private const int ChunkLengthPlusOne = ChunkLength + 1; - private readonly Stream stream; - private readonly IManagedByteBuffer buffer; + private readonly IManagedByteBuffer managedBuffer; private readonly byte[] bufferChunk; + private readonly int length; + private int bytesRead; private int position; - private int length; - /// /// Initializes a new instance of the class. /// @@ -45,8 +44,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components { this.stream = stream; this.length = (int)stream.Length; - this.buffer = memoryManager.AllocateCleanManagedByteBuffer(ChunkLength); - this.bufferChunk = this.buffer.Array; + this.managedBuffer = memoryManager.AllocateCleanManagedByteBuffer(ChunkLength); + this.bufferChunk = this.managedBuffer.Array; } /// @@ -86,11 +85,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components { return this.ReadByteSlow(); } - else - { - this.position++; - return this.bufferChunk[this.bytesRead++]; - } + + this.position++; + return this.bufferChunk[this.bytesRead++]; } /// @@ -125,53 +122,29 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components [MethodImpl(MethodImplOptions.AggressiveInlining)] public int Read(byte[] buffer, int offset, int count) { - if (buffer.Length < ChunkLengthPlusOne) + if (buffer.Length > ChunkLength) { - if (this.position == 0 || count + this.bytesRead > ChunkLength) - { - return this.ReadToChunkSlow(buffer, offset, count); - } - - int n = this.length - this.position; - if (n > count) - { - n = count; - } - - if (n < 0) - { - n = 0; - } + return this.ReadToBufferSlow(buffer, offset, count); + } - if (n < 9) - { - int byteCount = n; - int read = this.bytesRead; - byte[] chunk = this.bufferChunk; - - while (--byteCount > -1) - { - buffer[offset + byteCount] = chunk[read + byteCount]; - } - } - else - { - Buffer.BlockCopy(this.bufferChunk, this.bytesRead, buffer, offset, n); - } + if (this.position == 0 || count + this.bytesRead > ChunkLength) + { + return this.ReadToChunkSlow(buffer, offset, count); + } - this.position += n; - this.bytesRead += n; + int n = this.GetCount(count); + this.CopyBytes(buffer, offset, n); - return n; - } + this.position += n; + this.bytesRead += n; - return this.ReadToBufferSlow(buffer, offset, count); + return n; } /// public void Dispose() { - this.buffer?.Dispose(); + this.managedBuffer?.Dispose(); } [MethodImpl(MethodImplOptions.NoInlining)] @@ -201,6 +174,32 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components this.stream.Read(this.bufferChunk, 0, ChunkLength); this.bytesRead = 0; + int n = this.GetCount(count); + this.CopyBytes(buffer, offset, n); + + this.position += n; + this.bytesRead += n; + + return n; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private int ReadToBufferSlow(byte[] buffer, int offset, int count) + { + // Read to target but don't copy to our chunk. + if (this.position != this.stream.Position) + { + this.stream.Seek(this.position, SeekOrigin.Begin); + } + + int n = this.stream.Read(buffer, offset, count); + this.Position += n; + return n; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private int GetCount(int count) + { int n = this.length - this.position; if (n > count) { @@ -212,9 +211,15 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components n = 0; } - if (n < 9) + return n; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private void CopyBytes(byte[] buffer, int offset, int count) + { + if (count < 9) { - int byteCount = n; + int byteCount = count; int read = this.bytesRead; byte[] chunk = this.bufferChunk; @@ -225,27 +230,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } else { - Buffer.BlockCopy(this.bufferChunk, this.bytesRead, buffer, offset, n); + Buffer.BlockCopy(this.bufferChunk, this.bytesRead, buffer, offset, count); } - - this.position += n; - this.bytesRead += n; - - return n; - } - - [MethodImpl(MethodImplOptions.NoInlining)] - private int ReadToBufferSlow(byte[] buffer, int offset, int count) - { - // Read to target but don't copy to our chunk. - if (this.position != this.stream.Position) - { - this.stream.Seek(this.position, SeekOrigin.Begin); - } - - int n = this.stream.Read(buffer, offset, count); - this.Position += n; - return n; } } } \ No newline at end of file From 63189cac83f06a04272d59964f98c5419ff8e37d Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 1 May 2018 16:05:58 +1000 Subject: [PATCH 08/17] Minor scan decoder optimization --- .../PdfJsPort/Components/PdfJsScanDecoder.cs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs index 4415a681b..1b5e69230 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs @@ -396,7 +396,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components int blockRow = mcu / component.WidthInBlocks; int blockCol = mcu % component.WidthInBlocks; int offset = component.GetBlockBufferOffset(blockRow, blockCol); - this.DecodeACFirst(component, ref blockDataRef, offset, ref acHuffmanTable, stream); + this.DecodeACFirst(ref blockDataRef, offset, ref acHuffmanTable, stream); } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -407,7 +407,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components int blockRow = (mcuRow * component.VerticalSamplingFactor) + row; int blockCol = (mcuCol * component.HorizontalSamplingFactor) + col; int offset = component.GetBlockBufferOffset(blockRow, blockCol); - this.DecodeACFirst(component, ref blockDataRef, offset, ref acHuffmanTable, stream); + this.DecodeACFirst(ref blockDataRef, offset, ref acHuffmanTable, stream); } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -416,7 +416,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components int blockRow = mcu / component.WidthInBlocks; int blockCol = mcu % component.WidthInBlocks; int offset = component.GetBlockBufferOffset(blockRow, blockCol); - this.DecodeACSuccessive(component, ref blockDataRef, offset, ref acHuffmanTable, stream); + this.DecodeACSuccessive(ref blockDataRef, offset, ref acHuffmanTable, stream); } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -427,7 +427,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components int blockRow = (mcuRow * component.VerticalSamplingFactor) + row; int blockCol = (mcuCol * component.HorizontalSamplingFactor) + col; int offset = component.GetBlockBufferOffset(blockRow, blockCol); - this.DecodeACSuccessive(component, ref blockDataRef, offset, ref acHuffmanTable, stream); + this.DecodeACSuccessive(ref blockDataRef, offset, ref acHuffmanTable, stream); } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -574,11 +574,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components k += r; - if (k > 63) - { - break; - } - byte z = this.dctZigZag[k]; short re = (short)this.ReceiveAndExtend(s, stream); Unsafe.Add(ref blockDataRef, offset + z) = re; @@ -611,7 +606,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components Unsafe.Add(ref blockDataRef, offset) |= (short)(bit << this.successiveState); } - private void DecodeACFirst(PdfJsFrameComponent component, ref short blockDataRef, int offset, ref PdfJsHuffmanTable acHuffmanTable, DoubleBufferedStreamReader stream) + private void DecodeACFirst(ref short blockDataRef, int offset, ref PdfJsHuffmanTable acHuffmanTable, DoubleBufferedStreamReader stream) { if (this.eobrun > 0) { @@ -652,7 +647,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } } - private void DecodeACSuccessive(PdfJsFrameComponent component, ref short blockDataRef, int offset, ref PdfJsHuffmanTable acHuffmanTable, DoubleBufferedStreamReader stream) + private void DecodeACSuccessive(ref short blockDataRef, int offset, ref PdfJsHuffmanTable acHuffmanTable, DoubleBufferedStreamReader stream) { int k = this.specStart; int e = this.specEnd; From e968a4e62224438d14ece7e7fab021530c3503f6 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 1 May 2018 19:42:03 +1000 Subject: [PATCH 09/17] Refactor buffer-filling to be more like libjpegturbo + add optimization notes --- .../PdfJsPort/Components/PdfJsScanDecoder.cs | 94 ++++++++++++++----- 1 file changed, 72 insertions(+), 22 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs index 1b5e69230..61506cb78 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs @@ -122,6 +122,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components // Find marker this.bitsCount = 0; + this.bitsData = 0; fileMarker = PdfJsJpegDecoderCore.FindNextFileMarker(this.markerBuffer, stream); // Some bad images seem to pad Scan blocks with e.g. zero bytes, skip past @@ -433,49 +434,98 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components [MethodImpl(MethodImplOptions.AggressiveInlining)] private int ReadBit(DoubleBufferedStreamReader stream) { - // TODO: I wonder if we can do this two bytes at a time; libjpeg turbo seems to do that? - if (this.bitsCount > 0) + if (this.bitsCount == 0) { - this.bitsCount--; - return (this.bitsData >> this.bitsCount) & 1; + this.FillBits(stream); } - this.bitsData = stream.ReadByte(); + this.bitsCount--; + return (this.bitsData >> this.bitsCount) & 1; + } - if (this.bitsData == -0x1) + [MethodImpl(MethodImplOptions.NoInlining)] + private void FillBits(DoubleBufferedStreamReader stream) + { + // TODO: Read more then 1 byte at a time. + // In LibJpegTurbo this is be 25 bits (32-7) but I cannot get this to work + // for some images, I'm assuming because I am crossing MCU boundaries and not managing + // to detect it. + const int MinGetBits = 7; + + // Attempt to load to the minimum bit count. + while (this.bitsCount < MinGetBits) { - // We've encountered the end of the file stream which means there's no EOI marker ref the image - this.endOfStreamReached = true; - } + int c = stream.ReadByte(); - if (this.bitsData == PdfJsJpegConstants.Markers.Prefix) - { - int nextByte = stream.ReadByte(); - if (nextByte != 0) + if (c == -0x1) + { + // We've encountered the end of the file stream which means there's no EOI marker in the image. + this.endOfStreamReached = true; + + // Fill buffer with zero bits. + this.bitsData <<= MinGetBits - this.bitsCount; + this.bitsCount = MinGetBits; + break; + } + + if (c == PdfJsJpegConstants.Markers.Prefix) { + int nextByte = stream.ReadByte(); + if (nextByte != 0) + { #if DEBUG - Debug.WriteLine($"DecodeScan - Unexpected marker {(this.bitsData << 8) | nextByte:X} at {stream.Position}"); + Debug.WriteLine($"DecodeScan - Unexpected marker {(c << 8) | nextByte:X} at {stream.Position}"); #endif - // We've encountered an unexpected marker. Reverse the stream and exit. - this.unexpectedMarkerReached = true; - stream.Position -= 2; + // We've encountered an unexpected marker. Reverse the stream and exit. + this.unexpectedMarkerReached = true; + stream.Position -= 2; + + // Fill buffer with zero bits. + this.bitsData <<= MinGetBits - this.bitsCount; + this.bitsCount = MinGetBits; + break; + } } - // Unstuff 0 + // OK, load c into get_buffer + this.bitsData = (this.bitsData << 8) | c; + this.bitsCount += 8; } + } - this.bitsCount = 7; + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private int PeekBits(int count) + { + return this.bitsData >> (this.bitsCount - count) & ((1 << count) - 1); + } - return this.bitsData >> 7; + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private void DropBits(int count) + { + this.bitsCount -= count; } [MethodImpl(MethodImplOptions.AggressiveInlining)] private short DecodeHuffman(ref PdfJsHuffmanTable tree, DoubleBufferedStreamReader stream) { // TODO: Implement fast Huffman decoding. - // NOTES # During investigation of the libjpeg implementation it appears that they pull 32bits at a time and operate on those bits - // using 3 methods: FillBits, PeekBits, and ReadBits. We should attempt to do the same. + // In LibJpegTurbo a minimum of 25 bits (32-7) is collected from the stream + // Then a LUT is used to avoid the loop when decoding the Huffman value. + // using 3 methods: FillBits, PeekBits, and DropBits. + // The LUT has been ported from LibJpegTurbo as has this code but it doesn't work. + // this.FillBits(stream); + // + // const int LookAhead = 8; + // int look = this.PeekBits(LookAhead); + // look = tree.Lookahead[look]; + // int bits = look >> LookAhead; + // + // if (bits <= LookAhead) + // { + // this.DropBits(bits); + // return (short)(look & ((1 << LookAhead) - 1)); + // } short code = (short)this.ReadBit(stream); if (this.endOfStreamReached || this.unexpectedMarkerReached) { From c314fa04b0c5e7abbcb69f53a2c14a69225f4e3b Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 3 May 2018 00:05:38 +0200 Subject: [PATCH 10/17] fix indentation --- .../Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs index 61506cb78..f102d625e 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs @@ -474,7 +474,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components if (nextByte != 0) { #if DEBUG - Debug.WriteLine($"DecodeScan - Unexpected marker {(c << 8) | nextByte:X} at {stream.Position}"); + Debug.WriteLine($"DecodeScan - Unexpected marker {(c << 8) | nextByte:X} at {stream.Position}"); #endif // We've encountered an unexpected marker. Reverse the stream and exit. From 77bc3c8b1ab102fe44110e3c1d7bf7e949d5a10b Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 3 May 2018 01:15:11 +0200 Subject: [PATCH 11/17] better unit tests for Identify & MetaData parsing --- .../Formats/Jpg/JpegDecoderTests.MetaData.cs | 158 ++++++++++++++++++ .../Formats/Jpg/JpegDecoderTests.cs | 76 +-------- 2 files changed, 163 insertions(+), 71 deletions(-) create mode 100644 tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.MetaData.cs diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.MetaData.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.MetaData.cs new file mode 100644 index 000000000..7fc949b09 --- /dev/null +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.MetaData.cs @@ -0,0 +1,158 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +using System.IO; +using SixLabors.ImageSharp.Formats; +using SixLabors.ImageSharp.MetaData.Profiles.Exif; +using SixLabors.ImageSharp.MetaData.Profiles.Icc; +using SixLabors.ImageSharp.PixelFormats; +using Xunit; + +// ReSharper disable InconsistentNaming +namespace SixLabors.ImageSharp.Tests.Formats.Jpg +{ + using System.Runtime.CompilerServices; + + using SixLabors.ImageSharp.Formats.Jpeg; + + public partial class JpegDecoderTests + { + // TODO: A JPEGsnoop & metadata expert should review if the Exif/Icc expectations are correct. + // I'm seeing several entries with Exif-related names in images where we do not decode an exif profile. (- Anton) + public static readonly TheoryData MetaDataTestData = + new TheoryData + { + { false, TestImages.Jpeg.Progressive.Progress, 24, false, false }, + { false, TestImages.Jpeg.Progressive.Fb, 24, false, true }, + { false, TestImages.Jpeg.Baseline.Cmyk, 32, false, true }, + { false, TestImages.Jpeg.Baseline.Ycck, 32, true, true }, + { false, TestImages.Jpeg.Baseline.Jpeg400, 8, false, false }, + { false, TestImages.Jpeg.Baseline.Snake, 24, true, true }, + { false, TestImages.Jpeg.Baseline.Jpeg420Exif, 24, true, false }, + + { true, TestImages.Jpeg.Progressive.Progress, 24, false, false }, + { true, TestImages.Jpeg.Progressive.Fb, 24, false, true }, + { true, TestImages.Jpeg.Baseline.Cmyk, 32, false, true }, + { true, TestImages.Jpeg.Baseline.Ycck, 32, true, true }, + { true, TestImages.Jpeg.Baseline.Jpeg400, 8, false, false }, + { true, TestImages.Jpeg.Baseline.Snake, 24, true, true }, + { true, TestImages.Jpeg.Baseline.Jpeg420Exif, 24, true, false }, + }; + + [Theory] + [MemberData(nameof(MetaDataTestData))] + public void MetaDataIsParsedCorrectly_Orig( + bool useIdentify, + string imagePath, + int expectedPixelSize, + bool exifProfilePresent, + bool iccProfilePresent) + { + TestMetaDataImpl( + useIdentify, + OrigJpegDecoder, + imagePath, + expectedPixelSize, + exifProfilePresent, + iccProfilePresent); + } + + [Theory] + [MemberData(nameof(MetaDataTestData))] + public void MetaDataIsParsedCorrectly_PdfJs( + bool useIdentify, + string imagePath, + int expectedPixelSize, + bool exifProfilePresent, + bool iccProfilePresent) + { + TestMetaDataImpl( + useIdentify, + PdfJsJpegDecoder, + imagePath, + expectedPixelSize, + exifProfilePresent, + iccProfilePresent); + } + + private static void TestMetaDataImpl( + bool useIdentify, + IImageDecoder decoder, + string imagePath, + int expectedPixelSize, + bool exifProfilePresent, + bool iccProfilePresent) + { + var testFile = TestFile.Create(imagePath); + using (var stream = new MemoryStream(testFile.Bytes, false)) + { + IImageInfo imageInfo = useIdentify + ? ((IImageInfoDetector)decoder).Identify(Configuration.Default, stream) + : decoder.Decode(Configuration.Default, stream); + + Assert.NotNull(imageInfo); + Assert.NotNull(imageInfo.PixelType); + + if (useIdentify) + { + Assert.Equal(expectedPixelSize, imageInfo.PixelType.BitsPerPixel); + } + else + { + // When full Image decoding is performed, BitsPerPixel will match TPixel + int bpp32 = Unsafe.SizeOf() * 8; + Assert.Equal(bpp32, imageInfo.PixelType.BitsPerPixel); + } + + ExifProfile exifProfile = imageInfo.MetaData.ExifProfile; + + if (exifProfilePresent) + { + Assert.NotNull(exifProfile); + Assert.NotEmpty(exifProfile.Values); + } + else + { + Assert.Null(exifProfile); + } + + IccProfile iccProfile = imageInfo.MetaData.IccProfile; + + if (iccProfilePresent) + { + Assert.NotNull(iccProfile); + Assert.NotEmpty(iccProfile.Entries); + } + else + { + Assert.Null(iccProfile); + } + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void IgnoreMetaData_ControlsWhetherMetaDataIsParsed(bool ignoreMetaData) + { + var decoder = new JpegDecoder() { IgnoreMetadata = ignoreMetaData }; + + // Snake.jpg has both Exif and ICC profiles defined: + var testFile = TestFile.Create(TestImages.Jpeg.Baseline.Snake); + + using (Image image = testFile.CreateImage(decoder)) + { + if (ignoreMetaData) + { + Assert.Null(image.MetaData.ExifProfile); + Assert.Null(image.MetaData.IccProfile); + } + else + { + Assert.NotNull(image.MetaData.ExifProfile); + Assert.NotNull(image.MetaData.IccProfile); + } + } + } + } +} \ No newline at end of file diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 701d6b5d7..3138300b9 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -22,7 +22,7 @@ using Xunit.Abstractions; namespace SixLabors.ImageSharp.Tests.Formats.Jpg { // TODO: Scatter test cases into multiple test classes - public class JpegDecoderTests + public partial class JpegDecoderTests { public static string[] BaselineTestJpegs = { @@ -115,9 +115,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg private ITestOutputHelper Output { get; } - private static IImageDecoder OrigJpegDecoder => new OrigJpegDecoder(); + private static OrigJpegDecoder OrigJpegDecoder => new OrigJpegDecoder(); - private static IImageDecoder PdfJsJpegDecoder => new PdfJsJpegDecoder(); + private static PdfJsJpegDecoder PdfJsJpegDecoder => new PdfJsJpegDecoder(); [Fact] public void ParseStream_BasicPropertiesAreCorrect1_PdfJs() @@ -151,7 +151,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg // For 32 bit test enviroments: provider.Configuration.MemoryManager = ArrayPoolMemoryManager.CreateWithModeratePooling(); - IImageDecoder decoder = useOldDecoder ? OrigJpegDecoder : PdfJsJpegDecoder; + IImageDecoder decoder = useOldDecoder ? (IImageDecoder)OrigJpegDecoder : PdfJsJpegDecoder; using (Image image = provider.GetImage(decoder)) { image.DebugSave(provider); @@ -406,39 +406,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg Assert.Equal(72, image.MetaData.VerticalResolution); } } - - [Fact] - public void Decode_IgnoreMetadataIsFalse_ExifProfileIsRead() - { - var decoder = new JpegDecoder() - { - IgnoreMetadata = false - }; - - var testFile = TestFile.Create(TestImages.Jpeg.Baseline.Floorplan); - - using (Image image = testFile.CreateImage(decoder)) - { - Assert.NotNull(image.MetaData.ExifProfile); - } - } - - [Fact] - public void Decode_IgnoreMetadataIsTrue_ExifProfileIgnored() - { - var options = new JpegDecoder() - { - IgnoreMetadata = true - }; - - var testFile = TestFile.Create(TestImages.Jpeg.Baseline.Floorplan); - - using (Image image = testFile.CreateImage(options)) - { - Assert.Null(image.MetaData.ExifProfile); - } - } - + // DEBUG ONLY! // The PDF.js output should be saved by "tests\ImageSharp.Tests\Formats\Jpg\pdfjs\jpeg-converter.htm" // into "\tests\Images\ActualOutput\JpegDecoderTests\" @@ -470,39 +438,5 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg this.Output.WriteLine($"Difference for PORT: {portReport.DifferencePercentageString}"); } } - - [Theory] - [InlineData(TestImages.Jpeg.Progressive.Progress, 24)] - [InlineData(TestImages.Jpeg.Progressive.Fb, 24)] - [InlineData(TestImages.Jpeg.Baseline.Cmyk, 32)] - [InlineData(TestImages.Jpeg.Baseline.Ycck, 32)] - [InlineData(TestImages.Jpeg.Baseline.Jpeg400, 8)] - [InlineData(TestImages.Jpeg.Baseline.Snake, 24)] - [InlineData(TestImages.Jpeg.Baseline.Jpeg420Exif, 24)] - public void DetectPixelSizeGolang(string imagePath, int expectedPixelSize) - { - var testFile = TestFile.Create(imagePath); - using (var stream = new MemoryStream(testFile.Bytes, false)) - { - Assert.Equal(expectedPixelSize, ((IImageInfoDetector)OrigJpegDecoder).Identify(Configuration.Default, stream)?.PixelType?.BitsPerPixel); - } - } - - [Theory] - [InlineData(TestImages.Jpeg.Progressive.Progress, 24)] - [InlineData(TestImages.Jpeg.Progressive.Fb, 24)] - [InlineData(TestImages.Jpeg.Baseline.Cmyk, 32)] - [InlineData(TestImages.Jpeg.Baseline.Ycck, 32)] - [InlineData(TestImages.Jpeg.Baseline.Jpeg400, 8)] - [InlineData(TestImages.Jpeg.Baseline.Snake, 24)] - [InlineData(TestImages.Jpeg.Baseline.Jpeg420Exif, 24)] - public void DetectPixelSizePdfJs(string imagePath, int expectedPixelSize) - { - var testFile = TestFile.Create(imagePath); - using (var stream = new MemoryStream(testFile.Bytes, false)) - { - Assert.Equal(expectedPixelSize, ((IImageInfoDetector)PdfJsJpegDecoder).Identify(Configuration.Default, stream)?.PixelType?.BitsPerPixel); - } - } } } \ No newline at end of file From 527534bc53913cc89719be2ee9ef2c192b060562 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 3 May 2018 10:12:24 +1000 Subject: [PATCH 12/17] Update external refs --- tests/Images/External | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Images/External b/tests/Images/External index 5a9a88380..f641620eb 160000 --- a/tests/Images/External +++ b/tests/Images/External @@ -1 +1 @@ -Subproject commit 5a9a88380166a87521d10048f53cda7f5f761d66 +Subproject commit f641620eb5378db49d6153bbf1443ad13bda2379 From 9fcab9aa939e593efaf220d04c5946862e1f194b Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 3 May 2018 10:26:28 +1000 Subject: [PATCH 13/17] Skip SOS contents when parsing metadata only --- .../Jpeg/GolangPort/OrigJpegDecoderCore.cs | 15 +++++++++------ .../Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs | 6 +++++- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs index 6b4da1ba1..c7e26e04a 100644 --- a/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs @@ -361,15 +361,18 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort break; case OrigJpegConstants.Markers.SOS: - if (!metadataOnly) + if (metadataOnly) { - this.ProcessStartOfScanMarker(remaining); + this.InputProcessor.Skip(remaining); } - - if (this.InputProcessor.ReachedEOF) + else { - // If unexpected EOF reached. We can stop processing bytes as we now have the image data. - processBytes = false; + this.ProcessStartOfScanMarker(remaining); + if (this.InputProcessor.ReachedEOF) + { + // If unexpected EOF reached. We can stop processing bytes as we now have the image data. + processBytes = false; + } } break; diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs index c20f283d7..03ca170cf 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs @@ -246,7 +246,11 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort break; case PdfJsJpegConstants.Markers.SOS: - if (!metadataOnly) + if (metadataOnly) + { + this.InputStream.Skip(remaining); + } + else { this.ProcessStartOfScanMarker(); } From fd55ea8f8de93dd537fa013b339b9fd86fc350be Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 4 May 2018 15:12:08 +1000 Subject: [PATCH 14/17] Better sanitation for scan decoder. --- .../PdfJsPort/Components/PdfJsScanDecoder.cs | 276 ++++++++++-------- .../Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs | 2 - 2 files changed, 162 insertions(+), 116 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs index f102d625e..b43c51e0a 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs @@ -5,7 +5,6 @@ using System; #if DEBUG using System.Diagnostics; #endif -using System.IO; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using SixLabors.ImageSharp.Formats.Jpeg.Common; @@ -95,7 +94,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components mcuExpected = mcusPerLine * frame.McusPerColumn; } - PdfJsFileMarker fileMarker; while (mcu < mcuExpected) { // Reset interval stuff @@ -120,21 +118,15 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components this.DecodeScanProgressive(huffmanTables, isAc, isFirst, components, componentsLength, mcusPerLine, mcuToRead, ref mcu, stream); } - // Find marker + // Reset this.bitsCount = 0; this.bitsData = 0; - fileMarker = PdfJsJpegDecoderCore.FindNextFileMarker(this.markerBuffer, stream); + this.unexpectedMarkerReached = false; - // Some bad images seem to pad Scan blocks with e.g. zero bytes, skip past - // those to attempt to find a valid marker (fixes issue4090.pdf) in original code. - if (fileMarker.Invalid) - { -#if DEBUG - Debug.WriteLine($"DecodeScan - Unexpected MCU data at {stream.Position}, next marker is: {fileMarker.Marker:X}"); -#endif - } - - ushort marker = fileMarker.Marker; + // Some images include more scan blocks than expected, skip past those and + // attempt to find the next valid marker + PdfJsFileMarker fileMarker = PdfJsJpegDecoderCore.FindNextFileMarker(this.markerBuffer, stream); + byte marker = fileMarker.Marker; // RSTn - We've already read the bytes and altered the position so no need to skip if (marker >= PdfJsJpegConstants.Markers.RST0 && marker <= PdfJsJpegConstants.Markers.RST7) @@ -149,24 +141,11 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components stream.Position = fileMarker.Position; break; } - } - fileMarker = PdfJsJpegDecoderCore.FindNextFileMarker(this.markerBuffer, stream); - - // Some images include more Scan blocks than expected, skip past those and - // attempt to find the next valid marker (fixes issue8182.pdf) ref original code. - if (fileMarker.Invalid) - { #if DEBUG Debug.WriteLine($"DecodeScan - Unexpected MCU data at {stream.Position}, next marker is: {fileMarker.Marker:X}"); #endif } - else - { - // We've found a valid marker. - // Rewind the stream to the position of the marker - stream.Position = fileMarker.Position; - } } private void DecodeScanBaseline( @@ -295,9 +274,10 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components { for (int k = 0; k < h; k++) { + // No need to continue here. if (this.endOfStreamReached || this.unexpectedMarkerReached) { - continue; + break; } if (isAC) @@ -432,19 +412,24 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private int ReadBit(DoubleBufferedStreamReader stream) + private bool TryReadBit(DoubleBufferedStreamReader stream, out int bits) { if (this.bitsCount == 0) { - this.FillBits(stream); + if (!this.TryFillBits(stream)) + { + bits = 0; + return false; + } } this.bitsCount--; - return (this.bitsData >> this.bitsCount) & 1; + bits = (this.bitsData >> this.bitsCount) & 1; + return true; } [MethodImpl(MethodImplOptions.NoInlining)] - private void FillBits(DoubleBufferedStreamReader stream) + private bool TryFillBits(DoubleBufferedStreamReader stream) { // TODO: Read more then 1 byte at a time. // In LibJpegTurbo this is be 25 bits (32-7) but I cannot get this to work @@ -452,46 +437,61 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components // to detect it. const int MinGetBits = 7; - // Attempt to load to the minimum bit count. - while (this.bitsCount < MinGetBits) + if (!this.unexpectedMarkerReached) { - int c = stream.ReadByte(); - - if (c == -0x1) + // Attempt to load to the minimum bit count. + while (this.bitsCount < MinGetBits) { - // We've encountered the end of the file stream which means there's no EOI marker in the image. - this.endOfStreamReached = true; + int c = stream.ReadByte(); - // Fill buffer with zero bits. - this.bitsData <<= MinGetBits - this.bitsCount; - this.bitsCount = MinGetBits; - break; - } - - if (c == PdfJsJpegConstants.Markers.Prefix) - { - int nextByte = stream.ReadByte(); - if (nextByte != 0) + switch (c) { + case -0x1: + + // We've encountered the end of the file stream which means there's no EOI marker in the image. + this.endOfStreamReached = true; + return false; + + case PdfJsJpegConstants.Markers.Prefix: + int nextByte = stream.ReadByte(); + + if (nextByte == -0x1) + { + this.endOfStreamReached = true; + return false; + } + + if (nextByte != 0) + { #if DEBUG - Debug.WriteLine($"DecodeScan - Unexpected marker {(c << 8) | nextByte:X} at {stream.Position}"); + Debug.WriteLine($"DecodeScan - Unexpected marker {(c << 8) | nextByte:X} at {stream.Position}"); #endif - // We've encountered an unexpected marker. Reverse the stream and exit. - this.unexpectedMarkerReached = true; - stream.Position -= 2; + // We've encountered an unexpected marker. Reverse the stream and exit. + this.unexpectedMarkerReached = true; + stream.Position -= 2; - // Fill buffer with zero bits. - this.bitsData <<= MinGetBits - this.bitsCount; - this.bitsCount = MinGetBits; - break; + // TODO: double check we need this. + // Fill buffer with zero bits. + if (this.bitsCount == 0) + { + this.bitsData <<= MinGetBits; + this.bitsCount = MinGetBits; + } + + return true; + } + + break; } - } - // OK, load c into get_buffer - this.bitsData = (this.bitsData << 8) | c; - this.bitsCount += 8; + // OK, load the next byte into bitsData + this.bitsData = (this.bitsData << 8) | c; + this.bitsCount += 8; + } } + + return true; } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -507,14 +507,16 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private short DecodeHuffman(ref PdfJsHuffmanTable tree, DoubleBufferedStreamReader stream) + private bool TryDecodeHuffman(ref PdfJsHuffmanTable tree, DoubleBufferedStreamReader stream, out short value) { + value = -1; + // TODO: Implement fast Huffman decoding. // In LibJpegTurbo a minimum of 25 bits (32-7) is collected from the stream // Then a LUT is used to avoid the loop when decoding the Huffman value. // using 3 methods: FillBits, PeekBits, and DropBits. // The LUT has been ported from LibJpegTurbo as has this code but it doesn't work. - // this.FillBits(stream); + // this.TryFillBits(stream); // // const int LookAhead = 8; // int look = this.PeekBits(LookAhead); @@ -524,86 +526,104 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components // if (bits <= LookAhead) // { // this.DropBits(bits); - // return (short)(look & ((1 << LookAhead) - 1)); + // value = (short)(look & ((1 << LookAhead) - 1)); + // return true; // } - short code = (short)this.ReadBit(stream); - if (this.endOfStreamReached || this.unexpectedMarkerReached) + if (!this.TryReadBit(stream, out int bit)) { - return -1; + return false; } + short code = (short)bit; + // "DECODE", section F.2.2.3, figure F.16, page 109 of T.81 int i = 1; while (code > tree.MaxCode[i]) { - code <<= 1; - code |= (short)this.ReadBit(stream); - - if (this.endOfStreamReached || this.unexpectedMarkerReached) + if (!this.TryReadBit(stream, out bit)) { - return -1; + return false; } + code <<= 1; + code |= (short)bit; i++; } int j = tree.ValOffset[i]; - return tree.HuffVal[(j + code) & 0xFF]; + value = tree.HuffVal[(j + code) & 0xFF]; + return true; } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private int Receive(int length, DoubleBufferedStreamReader stream) + private bool TryReceive(int length, DoubleBufferedStreamReader stream, out int value) { - int n = 0; + value = 0; while (length > 0) { - int bit = this.ReadBit(stream); - if (this.endOfStreamReached || this.unexpectedMarkerReached) + if (!this.TryReadBit(stream, out int bit)) { - return -1; + return false; } - n = (n << 1) | bit; + value = (value << 1) | bit; length--; } - return n; + return true; } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private int ReceiveAndExtend(int length, DoubleBufferedStreamReader stream) + private bool TryReceiveAndExtend(int length, DoubleBufferedStreamReader stream, out int value) { if (length == 1) { - return this.ReadBit(stream) == 1 ? 1 : -1; - } + if (!this.TryReadBit(stream, out value)) + { + return false; + } - int n = this.Receive(length, stream); - if (n >= 1 << (length - 1)) + value = value == 1 ? 1 : -1; + } + else { - return n; + if (!this.TryReceive(length, stream, out value)) + { + return false; + } + + if (value < 1 << (length - 1)) + { + value += (-1 << length) + 1; + } } - return n + (-1 << length) + 1; + return true; } private void DecodeBaseline(PdfJsFrameComponent component, ref short blockDataRef, int offset, ref PdfJsHuffmanTable dcHuffmanTable, ref PdfJsHuffmanTable acHuffmanTable, DoubleBufferedStreamReader stream) { - short t = this.DecodeHuffman(ref dcHuffmanTable, stream); - if (this.endOfStreamReached || this.unexpectedMarkerReached) + if (!this.TryDecodeHuffman(ref dcHuffmanTable, stream, out short t)) { return; } - int diff = t == 0 ? 0 : this.ReceiveAndExtend(t, stream); + int diff = 0; + if (t != 0) + { + if (!this.TryReceiveAndExtend(t, stream, out diff)) + { + return; + } + } + Unsafe.Add(ref blockDataRef, offset) = (short)(component.Pred += diff); int k = 1; while (k < 64) { - short rs = this.DecodeHuffman(ref acHuffmanTable, stream); - if (this.endOfStreamReached || this.unexpectedMarkerReached) + if (!this.TryDecodeHuffman(ref acHuffmanTable, stream, out short rs)) { return; } @@ -625,8 +645,13 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components k += r; byte z = this.dctZigZag[k]; - short re = (short)this.ReceiveAndExtend(s, stream); - Unsafe.Add(ref blockDataRef, offset + z) = re; + + if (!this.TryReceiveAndExtend(s, stream, out int re)) + { + return; + } + + Unsafe.Add(ref blockDataRef, offset + z) = (short)re; k++; } } @@ -634,21 +659,27 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components [MethodImpl(MethodImplOptions.AggressiveInlining)] private void DecodeDCFirst(PdfJsFrameComponent component, ref short blockDataRef, int offset, ref PdfJsHuffmanTable dcHuffmanTable, DoubleBufferedStreamReader stream) { - short t = this.DecodeHuffman(ref dcHuffmanTable, stream); - if (this.endOfStreamReached || this.unexpectedMarkerReached) + if (!this.TryDecodeHuffman(ref dcHuffmanTable, stream, out short t)) { return; } - int diff = t == 0 ? 0 : this.ReceiveAndExtend(t, stream) << this.successiveState; - Unsafe.Add(ref blockDataRef, offset) = (short)(component.Pred += diff); + int diff = 0; + if (t != 0) + { + if (!this.TryReceiveAndExtend(t, stream, out diff)) + { + return; + } + } + + Unsafe.Add(ref blockDataRef, offset) = (short)(component.Pred += diff << this.successiveState); } [MethodImpl(MethodImplOptions.AggressiveInlining)] private void DecodeDCSuccessive(PdfJsFrameComponent component, ref short blockDataRef, int offset, DoubleBufferedStreamReader stream) { - int bit = this.ReadBit(stream); - if (this.endOfStreamReached || this.unexpectedMarkerReached) + if (!this.TryReadBit(stream, out int bit)) { return; } @@ -668,8 +699,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components int e = this.specEnd; while (k <= e) { - short rs = this.DecodeHuffman(ref acHuffmanTable, stream); - if (this.endOfStreamReached || this.unexpectedMarkerReached) + if (!this.TryDecodeHuffman(ref acHuffmanTable, stream, out short rs)) { return; } @@ -681,7 +711,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components { if (r < 15) { - this.eobrun = this.Receive(r, stream) + (1 << r) - 1; + if (!this.TryReceive(r, stream, out int eob)) + { + return; + } + + this.eobrun = eob + (1 << r) - 1; break; } @@ -692,7 +727,13 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components k += r; byte z = this.dctZigZag[k]; - Unsafe.Add(ref blockDataRef, offset + z) = (short)(this.ReceiveAndExtend(s, stream) * (1 << this.successiveState)); + + if (!this.TryReceiveAndExtend(s, stream, out int v)) + { + return; + } + + Unsafe.Add(ref blockDataRef, offset + z) = (short)(v * (1 << this.successiveState)); k++; } } @@ -712,8 +753,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components switch (this.successiveACState) { case 0: // Initial state - short rs = this.DecodeHuffman(ref acHuffmanTable, stream); - if (this.endOfStreamReached || this.unexpectedMarkerReached) + + if (!this.TryDecodeHuffman(ref acHuffmanTable, stream, out short rs)) { return; } @@ -724,7 +765,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components { if (r < 15) { - this.eobrun = this.Receive(r, stream) + (1 << r); + if (!this.TryReceive(r, stream, out int eob)) + { + return; + } + + this.eobrun = eob + (1 << r); this.successiveACState = 4; } else @@ -740,7 +786,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components throw new ImageFormatException("Invalid ACn encoding"); } - this.successiveACNextValue = this.ReceiveAndExtend(s, stream); + if (!this.TryReceiveAndExtend(s, stream, out int v)) + { + return; + } + + this.successiveACNextValue = v; this.successiveACState = r > 0 ? 2 : 3; } @@ -749,8 +800,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components case 2: if (blockOffsetZRef != 0) { - int bit = this.ReadBit(stream); - if (this.endOfStreamReached || this.unexpectedMarkerReached) + if (!this.TryReadBit(stream, out int bit)) { return; } @@ -770,8 +820,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components case 3: // Set value for a zero item if (blockOffsetZRef != 0) { - int bit = this.ReadBit(stream); - if (this.endOfStreamReached || this.unexpectedMarkerReached) + if (!this.TryReadBit(stream, out int bit)) { return; } @@ -788,8 +837,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components case 4: // Eob if (blockOffsetZRef != 0) { - int bit = this.ReadBit(stream); - if (this.endOfStreamReached || this.unexpectedMarkerReached) + if (!this.TryReadBit(stream, out int bit)) { return; } diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs index 03ca170cf..772d07f33 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs @@ -169,8 +169,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort m = suffix; } - marker[1] = (byte)m; - return new PdfJsFileMarker((byte)m, stream.Position - 2); } From 5e184bcffa2ef9beabeabbefcaf2f6cea75caba3 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 5 May 2018 10:04:20 +1000 Subject: [PATCH 15/17] Break out after SOS marker --- .../Jpeg/GolangPort/OrigJpegDecoderCore.cs | 15 +++++++------ .../PdfJsPort/Components/PdfJsScanDecoder.cs | 6 ++--- .../Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs | 22 ++++++++++--------- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs index c7e26e04a..875f16ec2 100644 --- a/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/GolangPort/OrigJpegDecoderCore.cs @@ -191,7 +191,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort where TPixel : struct, IPixel { this.ParseStream(stream); - return this.PostProcessIntoImage(); } @@ -202,7 +201,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort public IImageInfo Identify(Stream stream) { this.ParseStream(stream, true); - return new ImageInfo(new PixelTypeInfo(this.BitsPerPixel), this.ImageWidth, this.ImageHeight, this.MetaData); } @@ -361,11 +359,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort break; case OrigJpegConstants.Markers.SOS: - if (metadataOnly) - { - this.InputProcessor.Skip(remaining); - } - else + if (!metadataOnly) { this.ProcessStartOfScanMarker(remaining); if (this.InputProcessor.ReachedEOF) @@ -374,8 +368,15 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.GolangPort processBytes = false; } } + else + { + // It's highly unlikely that APPn related data will be found after the SOS marker + // We should have gathered everything we need by now. + processBytes = false; + } break; + case OrigJpegConstants.Markers.DRI: if (metadataOnly) { diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs index b43c51e0a..261cd61b1 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs @@ -412,19 +412,19 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private bool TryReadBit(DoubleBufferedStreamReader stream, out int bits) + private bool TryReadBit(DoubleBufferedStreamReader stream, out int bit) { if (this.bitsCount == 0) { if (!this.TryFillBits(stream)) { - bits = 0; + bit = 0; return false; } } this.bitsCount--; - bits = (this.bitsData >> this.bitsCount) & 1; + bit = (this.bitsData >> this.bitsCount) & 1; return true; } diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs index 772d07f33..df803a920 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/PdfJsJpegDecoderCore.cs @@ -185,6 +185,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort where TPixel : struct, IPixel { this.ParseStream(stream); + this.AssignResolution(); return this.PostProcessIntoImage(); } @@ -195,6 +196,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort public IImageInfo Identify(Stream stream) { this.ParseStream(stream, true); + this.AssignResolution(); return new ImageInfo(new PixelTypeInfo(this.BitsPerPixel), this.ImageWidth, this.ImageHeight, this.MetaData); } @@ -244,17 +246,18 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort break; case PdfJsJpegConstants.Markers.SOS: - if (metadataOnly) + if (!metadataOnly) { - this.InputStream.Skip(remaining); + this.ProcessStartOfScanMarker(); + break; } else { - this.ProcessStartOfScanMarker(); + // It's highly unlikely that APPn related data will be found after the SOS marker + // We should have gathered everything we need by now. + return; } - break; - case PdfJsJpegConstants.Markers.DHT: if (metadataOnly) { @@ -331,11 +334,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort // Read on. fileMarker = FindNextFileMarker(this.markerBuffer, this.InputStream); } - - this.ImageWidth = this.Frame.SamplesPerLine; - this.ImageHeight = this.Frame.Scanlines; - this.ComponentCount = this.Frame.ComponentCount; - this.AssignResolution(); } /// @@ -389,6 +387,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort /// private void AssignResolution() { + this.ImageWidth = this.Frame.SamplesPerLine; + this.ImageHeight = this.Frame.Scanlines; + if (this.jFif.XDensity > 0 && this.jFif.YDensity > 0) { this.MetaData.HorizontalResolution = this.jFif.XDensity; @@ -633,6 +634,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort int maxV = 0; int index = 6; + this.ComponentCount = this.Frame.ComponentCount; if (!metadataOnly) { // No need to pool this. They max out at 4 From 0ef7235c98c194330ec1dcc10cb3d41b6cb9e517 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 5 May 2018 22:43:03 +1000 Subject: [PATCH 16/17] Add skip test --- .../Components/DoubleBufferedStreamReader.cs | 1 + .../Jpg/DoubleBufferedStreamReaderTests.cs | 42 ++++++++++++++++--- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs index 164ca7cc1..eb91590e8 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/DoubleBufferedStreamReader.cs @@ -64,6 +64,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components { // Reset everything. It's easier than tracking. this.position = (int)value; + this.stream.Seek(this.position, SeekOrigin.Begin); this.bytesRead = ChunkLength; } } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/DoubleBufferedStreamReaderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/DoubleBufferedStreamReaderTests.cs index bc099bdc5..be71e554f 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/DoubleBufferedStreamReaderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/DoubleBufferedStreamReaderTests.cs @@ -11,12 +11,12 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg { public class DoubleBufferedStreamReaderTests { - private MemoryManager manager = Configuration.Default.MemoryManager; + private readonly MemoryManager manager = Configuration.Default.MemoryManager; [Fact] public void DoubleBufferedStreamReaderCanReadSingleByteFromOrigin() { - using (MemoryStream stream = CreateTestStream()) + using (MemoryStream stream = this.CreateTestStream()) { byte[] expected = stream.ToArray(); var reader = new DoubleBufferedStreamReader(this.manager, stream); @@ -32,7 +32,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg [Fact] public void DoubleBufferedStreamReaderCanReadSubsequentSingleByteCorrectly() { - using (MemoryStream stream = CreateTestStream()) + using (MemoryStream stream = this.CreateTestStream()) { byte[] expected = stream.ToArray(); var reader = new DoubleBufferedStreamReader(this.manager, stream); @@ -63,7 +63,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg [Fact] public void DoubleBufferedStreamReaderCanReadMultipleBytesFromOrigin() { - using (MemoryStream stream = CreateTestStream()) + using (MemoryStream stream = this.CreateTestStream()) { byte[] buffer = new byte[2]; byte[] expected = stream.ToArray(); @@ -82,7 +82,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg [Fact] public void DoubleBufferedStreamReaderCanReadSubsequentMultipleByteCorrectly() { - using (MemoryStream stream = CreateTestStream()) + using (MemoryStream stream = this.CreateTestStream()) { byte[] buffer = new byte[2]; byte[] expected = stream.ToArray(); @@ -115,6 +115,38 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg } } + [Fact] + public void DoubleBufferedStreamReaderCanSkip() + { + using (MemoryStream stream = this.CreateTestStream()) + { + byte[] expected = stream.ToArray(); + var reader = new DoubleBufferedStreamReader(this.manager, stream); + + int skip = 50; + int plusOne = 1; + int skip2 = DoubleBufferedStreamReader.ChunkLength; + + // Skip + reader.Skip(skip); + Assert.Equal(skip, reader.Position); + Assert.Equal(stream.Position, reader.Position); + + // Read + Assert.Equal(expected[skip], reader.ReadByte()); + + // Skip Again + reader.Skip(skip2); + + // First Skap + First Read + Second Skip + int position = skip + plusOne + skip2; + + Assert.Equal(position, reader.Position); + Assert.Equal(stream.Position, reader.Position); + Assert.Equal(expected[position], reader.ReadByte()); + } + } + private MemoryStream CreateTestStream() { byte[] buffer = new byte[DoubleBufferedStreamReader.ChunkLength * 3]; From 4c98463ac22ad4d66fcc0b3c13535fc51aae46ae Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 7 May 2018 15:33:55 +1000 Subject: [PATCH 17/17] Improve readability in scan decoder (less params, no discernable perf hit) --- .../PdfJsPort/Components/PdfJsScanDecoder.cs | 126 +++++++++--------- 1 file changed, 63 insertions(+), 63 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs index 261cd61b1..c6b14d6fb 100644 --- a/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/PdfJsPort/Components/PdfJsScanDecoder.cs @@ -20,6 +20,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components private byte[] markerBuffer; + private int mcuToRead; + + private int mcusPerLine; + + private int mcu; + private int bitsData; private int bitsCount; @@ -81,9 +87,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components this.unexpectedMarkerReached = false; bool progressive = frame.Progressive; - int mcusPerLine = frame.McusPerLine; + this.mcusPerLine = frame.McusPerLine; - int mcu = 0; + this.mcu = 0; int mcuExpected; if (componentsLength == 1) { @@ -91,13 +97,13 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } else { - mcuExpected = mcusPerLine * frame.McusPerColumn; + mcuExpected = this.mcusPerLine * frame.McusPerColumn; } - while (mcu < mcuExpected) + while (this.mcu < mcuExpected) { // Reset interval stuff - int mcuToRead = resetInterval != 0 ? Math.Min(mcuExpected - mcu, resetInterval) : mcuExpected; + this.mcuToRead = resetInterval != 0 ? Math.Min(mcuExpected - this.mcu, resetInterval) : mcuExpected; for (int i = 0; i < components.Length; i++) { PdfJsFrameComponent c = components[i]; @@ -108,17 +114,18 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components if (!progressive) { - this.DecodeScanBaseline(dcHuffmanTables, acHuffmanTables, components, componentsLength, mcusPerLine, mcuToRead, ref mcu, stream); + this.DecodeScanBaseline(dcHuffmanTables, acHuffmanTables, components, componentsLength, stream); } else { bool isAc = this.specStart != 0; bool isFirst = successivePrev == 0; PdfJsHuffmanTables huffmanTables = isAc ? acHuffmanTables : dcHuffmanTables; - this.DecodeScanProgressive(huffmanTables, isAc, isFirst, components, componentsLength, mcusPerLine, mcuToRead, ref mcu, stream); + this.DecodeScanProgressive(huffmanTables, isAc, isFirst, components, componentsLength, stream); } // Reset + // TODO: I do not understand why these values are reset? We should surely be tracking the bits across mcu's? this.bitsCount = 0; this.bitsData = 0; this.unexpectedMarkerReached = false; @@ -153,9 +160,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components PdfJsHuffmanTables acHuffmanTables, PdfJsFrameComponent[] components, int componentsLength, - int mcusPerLine, - int mcuToRead, - ref int mcu, DoubleBufferedStreamReader stream) { if (componentsLength == 1) @@ -165,20 +169,20 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components ref PdfJsHuffmanTable dcHuffmanTable = ref dcHuffmanTables[component.DCHuffmanTableId]; ref PdfJsHuffmanTable acHuffmanTable = ref acHuffmanTables[component.ACHuffmanTableId]; - for (int n = 0; n < mcuToRead; n++) + for (int n = 0; n < this.mcuToRead; n++) { if (this.endOfStreamReached || this.unexpectedMarkerReached) { continue; } - this.DecodeBlockBaseline(ref dcHuffmanTable, ref acHuffmanTable, component, ref blockDataRef, mcu, stream); - mcu++; + this.DecodeBlockBaseline(ref dcHuffmanTable, ref acHuffmanTable, component, ref blockDataRef, stream); + this.mcu++; } } else { - for (int n = 0; n < mcuToRead; n++) + for (int n = 0; n < this.mcuToRead; n++) { for (int i = 0; i < componentsLength; i++) { @@ -198,12 +202,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components continue; } - this.DecodeMcuBaseline(ref dcHuffmanTable, ref acHuffmanTable, component, ref blockDataRef, mcusPerLine, mcu, j, k, stream); + this.DecodeMcuBaseline(ref dcHuffmanTable, ref acHuffmanTable, component, ref blockDataRef, j, k, stream); } } } - mcu++; + this.mcu++; } } } @@ -214,9 +218,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components bool isFirst, PdfJsFrameComponent[] components, int componentsLength, - int mcusPerLine, - int mcuToRead, - ref int mcu, DoubleBufferedStreamReader stream) { if (componentsLength == 1) @@ -225,7 +226,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components ref short blockDataRef = ref MemoryMarshal.GetReference(MemoryMarshal.Cast(component.SpectralBlocks.Span)); ref PdfJsHuffmanTable huffmanTable = ref huffmanTables[isAC ? component.ACHuffmanTableId : component.DCHuffmanTableId]; - for (int n = 0; n < mcuToRead; n++) + for (int n = 0; n < this.mcuToRead; n++) { if (this.endOfStreamReached || this.unexpectedMarkerReached) { @@ -236,31 +237,31 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components { if (isFirst) { - this.DecodeBlockACFirst(ref huffmanTable, component, ref blockDataRef, mcu, stream); + this.DecodeBlockACFirst(ref huffmanTable, component, ref blockDataRef, stream); } else { - this.DecodeBlockACSuccessive(ref huffmanTable, component, ref blockDataRef, mcu, stream); + this.DecodeBlockACSuccessive(ref huffmanTable, component, ref blockDataRef, stream); } } else { if (isFirst) { - this.DecodeBlockDCFirst(ref huffmanTable, component, ref blockDataRef, mcu, stream); + this.DecodeBlockDCFirst(ref huffmanTable, component, ref blockDataRef, stream); } else { - this.DecodeBlockDCSuccessive(component, ref blockDataRef, mcu, stream); + this.DecodeBlockDCSuccessive(component, ref blockDataRef, stream); } } - mcu++; + this.mcu++; } } else { - for (int n = 0; n < mcuToRead; n++) + for (int n = 0; n < this.mcuToRead; n++) { for (int i = 0; i < componentsLength; i++) { @@ -284,47 +285,47 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components { if (isFirst) { - this.DecodeMcuACFirst(ref huffmanTable, component, ref blockDataRef, mcusPerLine, mcu, j, k, stream); + this.DecodeMcuACFirst(ref huffmanTable, component, ref blockDataRef, j, k, stream); } else { - this.DecodeMcuACSuccessive(ref huffmanTable, component, ref blockDataRef, mcusPerLine, mcu, j, k, stream); + this.DecodeMcuACSuccessive(ref huffmanTable, component, ref blockDataRef, j, k, stream); } } else { if (isFirst) { - this.DecodeMcuDCFirst(ref huffmanTable, component, ref blockDataRef, mcusPerLine, mcu, j, k, stream); + this.DecodeMcuDCFirst(ref huffmanTable, component, ref blockDataRef, j, k, stream); } else { - this.DecodeMcuDCSuccessive(component, ref blockDataRef, mcusPerLine, mcu, j, k, stream); + this.DecodeMcuDCSuccessive(component, ref blockDataRef, j, k, stream); } } } } } - mcu++; + this.mcu++; } } } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeBlockBaseline(ref PdfJsHuffmanTable dcHuffmanTable, ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcu, DoubleBufferedStreamReader stream) + private void DecodeBlockBaseline(ref PdfJsHuffmanTable dcHuffmanTable, ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, DoubleBufferedStreamReader stream) { - int blockRow = mcu / component.WidthInBlocks; - int blockCol = mcu % component.WidthInBlocks; + int blockRow = this.mcu / component.WidthInBlocks; + int blockCol = this.mcu % component.WidthInBlocks; int offset = component.GetBlockBufferOffset(blockRow, blockCol); this.DecodeBaseline(component, ref blockDataRef, offset, ref dcHuffmanTable, ref acHuffmanTable, stream); } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeMcuBaseline(ref PdfJsHuffmanTable dcHuffmanTable, ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcusPerLine, int mcu, int row, int col, DoubleBufferedStreamReader stream) + private void DecodeMcuBaseline(ref PdfJsHuffmanTable dcHuffmanTable, ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int row, int col, DoubleBufferedStreamReader stream) { - int mcuRow = mcu / mcusPerLine; - int mcuCol = mcu % mcusPerLine; + int mcuRow = this.mcu / this.mcusPerLine; + int mcuCol = this.mcu % this.mcusPerLine; int blockRow = (mcuRow * component.VerticalSamplingFactor) + row; int blockCol = (mcuCol * component.HorizontalSamplingFactor) + col; int offset = component.GetBlockBufferOffset(blockRow, blockCol); @@ -332,19 +333,19 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeBlockDCFirst(ref PdfJsHuffmanTable dcHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcu, DoubleBufferedStreamReader stream) + private void DecodeBlockDCFirst(ref PdfJsHuffmanTable dcHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, DoubleBufferedStreamReader stream) { - int blockRow = mcu / component.WidthInBlocks; - int blockCol = mcu % component.WidthInBlocks; + int blockRow = this.mcu / component.WidthInBlocks; + int blockCol = this.mcu % component.WidthInBlocks; int offset = component.GetBlockBufferOffset(blockRow, blockCol); this.DecodeDCFirst(component, ref blockDataRef, offset, ref dcHuffmanTable, stream); } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeMcuDCFirst(ref PdfJsHuffmanTable dcHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcusPerLine, int mcu, int row, int col, DoubleBufferedStreamReader stream) + private void DecodeMcuDCFirst(ref PdfJsHuffmanTable dcHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int row, int col, DoubleBufferedStreamReader stream) { - int mcuRow = mcu / mcusPerLine; - int mcuCol = mcu % mcusPerLine; + int mcuRow = this.mcu / this.mcusPerLine; + int mcuCol = this.mcu % this.mcusPerLine; int blockRow = (mcuRow * component.VerticalSamplingFactor) + row; int blockCol = (mcuCol * component.HorizontalSamplingFactor) + col; int offset = component.GetBlockBufferOffset(blockRow, blockCol); @@ -352,19 +353,19 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeBlockDCSuccessive(PdfJsFrameComponent component, ref short blockDataRef, int mcu, DoubleBufferedStreamReader stream) + private void DecodeBlockDCSuccessive(PdfJsFrameComponent component, ref short blockDataRef, DoubleBufferedStreamReader stream) { - int blockRow = mcu / component.WidthInBlocks; - int blockCol = mcu % component.WidthInBlocks; + int blockRow = this.mcu / component.WidthInBlocks; + int blockCol = this.mcu % component.WidthInBlocks; int offset = component.GetBlockBufferOffset(blockRow, blockCol); this.DecodeDCSuccessive(component, ref blockDataRef, offset, stream); } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeMcuDCSuccessive(PdfJsFrameComponent component, ref short blockDataRef, int mcusPerLine, int mcu, int row, int col, DoubleBufferedStreamReader stream) + private void DecodeMcuDCSuccessive(PdfJsFrameComponent component, ref short blockDataRef, int row, int col, DoubleBufferedStreamReader stream) { - int mcuRow = mcu / mcusPerLine; - int mcuCol = mcu % mcusPerLine; + int mcuRow = this.mcu / this.mcusPerLine; + int mcuCol = this.mcu % this.mcusPerLine; int blockRow = (mcuRow * component.VerticalSamplingFactor) + row; int blockCol = (mcuCol * component.HorizontalSamplingFactor) + col; int offset = component.GetBlockBufferOffset(blockRow, blockCol); @@ -372,19 +373,19 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeBlockACFirst(ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcu, DoubleBufferedStreamReader stream) + private void DecodeBlockACFirst(ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, DoubleBufferedStreamReader stream) { - int blockRow = mcu / component.WidthInBlocks; - int blockCol = mcu % component.WidthInBlocks; + int blockRow = this.mcu / component.WidthInBlocks; + int blockCol = this.mcu % component.WidthInBlocks; int offset = component.GetBlockBufferOffset(blockRow, blockCol); this.DecodeACFirst(ref blockDataRef, offset, ref acHuffmanTable, stream); } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeMcuACFirst(ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcusPerLine, int mcu, int row, int col, DoubleBufferedStreamReader stream) + private void DecodeMcuACFirst(ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int row, int col, DoubleBufferedStreamReader stream) { - int mcuRow = mcu / mcusPerLine; - int mcuCol = mcu % mcusPerLine; + int mcuRow = this.mcu / this.mcusPerLine; + int mcuCol = this.mcu % this.mcusPerLine; int blockRow = (mcuRow * component.VerticalSamplingFactor) + row; int blockCol = (mcuCol * component.HorizontalSamplingFactor) + col; int offset = component.GetBlockBufferOffset(blockRow, blockCol); @@ -392,19 +393,19 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeBlockACSuccessive(ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcu, DoubleBufferedStreamReader stream) + private void DecodeBlockACSuccessive(ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, DoubleBufferedStreamReader stream) { - int blockRow = mcu / component.WidthInBlocks; - int blockCol = mcu % component.WidthInBlocks; + int blockRow = this.mcu / component.WidthInBlocks; + int blockCol = this.mcu % component.WidthInBlocks; int offset = component.GetBlockBufferOffset(blockRow, blockCol); this.DecodeACSuccessive(ref blockDataRef, offset, ref acHuffmanTable, stream); } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void DecodeMcuACSuccessive(ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int mcusPerLine, int mcu, int row, int col, DoubleBufferedStreamReader stream) + private void DecodeMcuACSuccessive(ref PdfJsHuffmanTable acHuffmanTable, PdfJsFrameComponent component, ref short blockDataRef, int row, int col, DoubleBufferedStreamReader stream) { - int mcuRow = mcu / mcusPerLine; - int mcuCol = mcu % mcusPerLine; + int mcuRow = this.mcu / this.mcusPerLine; + int mcuCol = this.mcu % this.mcusPerLine; int blockRow = (mcuRow * component.VerticalSamplingFactor) + row; int blockCol = (mcuCol * component.HorizontalSamplingFactor) + col; int offset = component.GetBlockBufferOffset(blockRow, blockCol); @@ -433,8 +434,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.PdfJsPort.Components { // TODO: Read more then 1 byte at a time. // In LibJpegTurbo this is be 25 bits (32-7) but I cannot get this to work - // for some images, I'm assuming because I am crossing MCU boundaries and not managing - // to detect it. + // for some images, I'm assuming because I am crossing MCU boundaries and not maintining the correct buffer state. const int MinGetBits = 7; if (!this.unexpectedMarkerReached)