From 6f45485203e61b6733c7eaf7015b453742d4679d Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Thu, 15 Jul 2021 18:33:54 +0300 Subject: [PATCH 01/21] Huffman tables are now handled by scan decoder, not decoder core --- .../Components/Decoder/HuffmanScanDecoder.cs | 54 +++++++++++++------ .../Formats/Jpeg/JpegDecoderCore.cs | 40 ++------------ 2 files changed, 42 insertions(+), 52 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs index a09c7ada3..97ec45ec1 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs @@ -31,6 +31,16 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder // The End-Of-Block countdown for ending the sequence prematurely when the remaining coefficients are zero. private int eobrun; + /// + /// The DC Huffman tables. + /// + private readonly HuffmanTable[] dcHuffmanTables; + + /// + /// The AC Huffman tables + /// + private readonly HuffmanTable[] acHuffmanTables; + // The unzig data. private ZigZag dctZigZag; @@ -55,12 +65,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder this.stream = stream; this.spectralConverter = converter; this.cancellationToken = cancellationToken; - } - - // huffman tables - public HuffmanTable[] DcHuffmanTables { get; set; } - public HuffmanTable[] AcHuffmanTables { get; set; } + // TODO: this is actually a variable value depending on component count + const int maxTables = 4; + this.dcHuffmanTables = new HuffmanTable[maxTables]; + this.acHuffmanTables = new HuffmanTable[maxTables]; + } // Reset interval public int ResetInterval @@ -148,8 +158,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder int order = this.frame.ComponentOrder[i]; JpegComponent component = this.components[order]; - ref HuffmanTable dcHuffmanTable = ref this.DcHuffmanTables[component.DCHuffmanTableId]; - ref HuffmanTable acHuffmanTable = ref this.AcHuffmanTables[component.ACHuffmanTableId]; + ref HuffmanTable dcHuffmanTable = ref this.dcHuffmanTables[component.DCHuffmanTableId]; + ref HuffmanTable acHuffmanTable = ref this.acHuffmanTables[component.ACHuffmanTableId]; dcHuffmanTable.Configure(); acHuffmanTable.Configure(); } @@ -168,8 +178,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder int order = this.frame.ComponentOrder[k]; JpegComponent component = this.components[order]; - ref HuffmanTable dcHuffmanTable = ref this.DcHuffmanTables[component.DCHuffmanTableId]; - ref HuffmanTable acHuffmanTable = ref this.AcHuffmanTables[component.ACHuffmanTableId]; + ref HuffmanTable dcHuffmanTable = ref this.dcHuffmanTables[component.DCHuffmanTableId]; + ref HuffmanTable acHuffmanTable = ref this.acHuffmanTables[component.ACHuffmanTableId]; int h = component.HorizontalSamplingFactor; int v = component.VerticalSamplingFactor; @@ -221,8 +231,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder int w = component.WidthInBlocks; int h = component.HeightInBlocks; - ref HuffmanTable dcHuffmanTable = ref this.DcHuffmanTables[component.DCHuffmanTableId]; - ref HuffmanTable acHuffmanTable = ref this.AcHuffmanTables[component.ACHuffmanTableId]; + ref HuffmanTable dcHuffmanTable = ref this.dcHuffmanTables[component.DCHuffmanTableId]; + ref HuffmanTable acHuffmanTable = ref this.acHuffmanTables[component.ACHuffmanTableId]; dcHuffmanTable.Configure(); acHuffmanTable.Configure(); @@ -327,7 +337,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder { int order = this.frame.ComponentOrder[k]; JpegComponent component = this.components[order]; - ref HuffmanTable dcHuffmanTable = ref this.DcHuffmanTables[component.DCHuffmanTableId]; + ref HuffmanTable dcHuffmanTable = ref this.dcHuffmanTables[component.DCHuffmanTableId]; dcHuffmanTable.Configure(); } @@ -342,7 +352,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder { int order = this.frame.ComponentOrder[k]; JpegComponent component = this.components[order]; - ref HuffmanTable dcHuffmanTable = ref this.DcHuffmanTables[component.DCHuffmanTableId]; + ref HuffmanTable dcHuffmanTable = ref this.dcHuffmanTables[component.DCHuffmanTableId]; int h = component.HorizontalSamplingFactor; int v = component.VerticalSamplingFactor; @@ -390,7 +400,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder if (this.SpectralStart == 0) { - ref HuffmanTable dcHuffmanTable = ref this.DcHuffmanTables[component.DCHuffmanTableId]; + ref HuffmanTable dcHuffmanTable = ref this.dcHuffmanTables[component.DCHuffmanTableId]; dcHuffmanTable.Configure(); for (int j = 0; j < h; j++) @@ -418,7 +428,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder } else { - ref HuffmanTable acHuffmanTable = ref this.AcHuffmanTables[component.ACHuffmanTableId]; + ref HuffmanTable acHuffmanTable = ref this.acHuffmanTables[component.ACHuffmanTableId]; acHuffmanTable.Configure(); for (int j = 0; j < h; j++) @@ -722,5 +732,19 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder return false; } + + /// + /// Build the huffman table using code lengths and code values. + /// + /// Table type. + /// Table index. + /// Code lengths. + /// Code values. + [MethodImpl(InliningOptions.ShortMethod)] + public void BuildHuffmanTable(int type, int index, ReadOnlySpan codeLengths, ReadOnlySpan values) + { + HuffmanTable[] tables = type == 0 ? this.dcHuffmanTables : this.acHuffmanTables; + tables[index] = new HuffmanTable(codeLengths, values); + } } } diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 922e9797c..ee723f062 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -42,16 +42,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg /// private readonly byte[] markerBuffer = new byte[2]; - /// - /// The DC Huffman tables. - /// - private HuffmanTable[] dcHuffmanTables; - - /// - /// The AC Huffman tables - /// - private HuffmanTable[] acHuffmanTables; - /// /// The reset interval determined by RST markers. /// @@ -270,14 +260,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg fileMarker = new JpegFileMarker(marker, (int)stream.Position - 2); this.QuantizationTables = new Block8x8F[4]; - // Only assign what we need - if (!metadataOnly) - { - const int maxTables = 4; - this.dcHuffmanTables = new HuffmanTable[maxTables]; - this.acHuffmanTables = new HuffmanTable[maxTables]; - } - // Break only when we discover a valid EOI marker. // https://github.com/SixLabors/ImageSharp/issues/695 while (fileMarker.Marker != JpegConstants.Markers.EOI @@ -392,8 +374,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg // Set large fields to null. this.Frame = null; - this.dcHuffmanTables = null; - this.acHuffmanTables = null; + this.scanDecoder = null; } /// @@ -996,8 +977,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg i += 17 + codeLengthSum; - this.BuildHuffmanTable( - tableType == 0 ? this.dcHuffmanTables : this.acHuffmanTables, + this.scanDecoder.BuildHuffmanTable( + tableType, tableIndex, codeLengthsSpan, huffmanValuesSpan); @@ -1071,10 +1052,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg // All the comments below are for separate refactoring PR // Main reason it's not fixed here is to make this commit less intrusive - // Huffman tables can be calculated directly in the scan decoder class - this.scanDecoder.DcHuffmanTables = this.dcHuffmanTables; - this.scanDecoder.AcHuffmanTables = this.acHuffmanTables; - // This can be injectd in DRI marker callback this.scanDecoder.ResetInterval = this.resetInterval; @@ -1090,17 +1067,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg this.scanDecoder.ParseEntropyCodedData(); } - /// - /// Builds the huffman tables - /// - /// The tables - /// The table index - /// The codelengths - /// The values - [MethodImpl(InliningOptions.ShortMethod)] - private void BuildHuffmanTable(HuffmanTable[] tables, int index, ReadOnlySpan codeLengths, ReadOnlySpan values) - => tables[index] = new HuffmanTable(codeLengths, values); - /// /// Reads a from the stream advancing it by two bytes /// From d9745e4d3bd2a6fd14393e1278fa4bd07ed94881 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Thu, 15 Jul 2021 18:42:30 +0300 Subject: [PATCH 02/21] Restart interval is now handled by scan decoder --- .../Jpeg/Components/Decoder/HuffmanScanDecoder.cs | 8 ++++++-- src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 10 +--------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs index 97ec45ec1..688414b33 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs @@ -22,7 +22,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder private JpegFrame frame; private JpegComponent[] components; - // The restart interval. + /// + /// The reset interval determined by RST markers. + /// private int restartInterval; // How many mcu's are left to do. @@ -72,7 +74,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder this.acHuffmanTables = new HuffmanTable[maxTables]; } - // Reset interval + /// + /// Sets reset interval determined by RST markers. + /// public int ResetInterval { set diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index ee723f062..e61909798 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -42,11 +42,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg /// private readonly byte[] markerBuffer = new byte[2]; - /// - /// The reset interval determined by RST markers. - /// - private ushort resetInterval; - /// /// Whether the image has an EXIF marker. /// @@ -1001,7 +996,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg JpegThrowHelper.ThrowBadMarker(nameof(JpegConstants.Markers.DRI), remaining); } - this.resetInterval = this.ReadUint16(stream); + this.scanDecoder.ResetInterval = this.ReadUint16(stream); } /// @@ -1052,9 +1047,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg // All the comments below are for separate refactoring PR // Main reason it's not fixed here is to make this commit less intrusive - // This can be injectd in DRI marker callback - this.scanDecoder.ResetInterval = this.resetInterval; - // This can be passed as ParseEntropyCodedData() parameter as it is used only there this.scanDecoder.ComponentsLength = selectorsCount; From b299e1a2f7600f1611be32be0b703ba2b24dddb6 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Thu, 15 Jul 2021 19:04:03 +0300 Subject: [PATCH 03/21] Scan component count refactor --- .../Components/Decoder/HuffmanScanDecoder.cs | 24 ++++++++++--------- .../Formats/Jpeg/JpegDecoderCore.cs | 8 +------ 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs index 688414b33..ea76df7a8 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs @@ -22,6 +22,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder private JpegFrame frame; private JpegComponent[] components; + // The number of interleaved components. + private int componentsCount; + /// /// The reset interval determined by RST markers. /// @@ -86,9 +89,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder } } - // The number of interleaved components. - public int ComponentsLength { get; set; } - // The spectral selection start. public int SpectralStart { get; set; } @@ -104,10 +104,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// /// Decodes the entropy coded data. /// - public void ParseEntropyCodedData() + public void ParseEntropyCodedData(int componentCount) { this.cancellationToken.ThrowIfCancellationRequested(); + this.componentsCount = componentCount; + this.scanBuffer = new HuffmanScanBuffer(this.stream); bool fullScan = this.frame.Progressive || this.frame.MultiScan; @@ -138,7 +140,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder private void ParseBaselineData() { - if (this.ComponentsLength == this.frame.ComponentCount) + if (this.componentsCount == this.frame.ComponentCount) { this.ParseBaselineDataInterleaved(); } @@ -157,7 +159,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder ref HuffmanScanBuffer buffer = ref this.scanBuffer; // Pre-derive the huffman table to avoid in-loop checks. - for (int i = 0; i < this.ComponentsLength; i++) + for (int i = 0; i < this.componentsCount; i++) { int order = this.frame.ComponentOrder[i]; JpegComponent component = this.components[order]; @@ -177,7 +179,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder { // Scan an interleaved mcu... process components in order int mcuCol = mcu % mcusPerLine; - for (int k = 0; k < this.ComponentsLength; k++) + for (int k = 0; k < this.componentsCount; k++) { int order = this.frame.ComponentOrder[k]; JpegComponent component = this.components[order]; @@ -286,7 +288,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder } // AC scans may have only one component. - if (this.ComponentsLength != 1) + if (this.componentsCount != 1) { invalid = true; } @@ -318,7 +320,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder { this.CheckProgressiveData(); - if (this.ComponentsLength == 1) + if (this.componentsCount == 1) { this.ParseProgressiveDataNonInterleaved(); } @@ -337,7 +339,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder ref HuffmanScanBuffer buffer = ref this.scanBuffer; // Pre-derive the huffman table to avoid in-loop checks. - for (int k = 0; k < this.ComponentsLength; k++) + for (int k = 0; k < this.componentsCount; k++) { int order = this.frame.ComponentOrder[k]; JpegComponent component = this.components[order]; @@ -352,7 +354,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder // Scan an interleaved mcu... process components in order int mcuRow = mcu / mcusPerLine; int mcuCol = mcu % mcusPerLine; - for (int k = 0; k < this.ComponentsLength; k++) + for (int k = 0; k < this.componentsCount; k++) { int order = this.frame.ComponentOrder[k]; JpegComponent component = this.components[order]; diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index e61909798..97a6d3999 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -1044,19 +1044,13 @@ namespace SixLabors.ImageSharp.Formats.Jpeg int spectralEnd = this.temp[1]; int successiveApproximation = this.temp[2]; - // All the comments below are for separate refactoring PR - // Main reason it's not fixed here is to make this commit less intrusive - - // This can be passed as ParseEntropyCodedData() parameter as it is used only there - this.scanDecoder.ComponentsLength = selectorsCount; - // This is okay to inject here, might be good to wrap it in a separate struct but not really necessary this.scanDecoder.SpectralStart = spectralStart; this.scanDecoder.SpectralEnd = spectralEnd; this.scanDecoder.SuccessiveHigh = successiveApproximation >> 4; this.scanDecoder.SuccessiveLow = successiveApproximation & 15; - this.scanDecoder.ParseEntropyCodedData(); + this.scanDecoder.ParseEntropyCodedData(selectorsCount); } /// From 9067c64b3074d067fbf1184377a6956c82093d98 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 02:00:59 +0300 Subject: [PATCH 04/21] Added SOF data precision comments, decoupled precision value from decoder core --- .../Jpeg/Components/Decoder/IRawJpegData.cs | 7 +------ .../Decoder/JpegBlockPostProcessor.cs | 6 ------ .../Decoder/JpegComponentPostProcessor.cs | 13 +++++++++++-- .../Decoder/SpectralConverter{TPixel}.cs | 2 +- src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 18 ++++++++---------- 5 files changed, 21 insertions(+), 25 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/IRawJpegData.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/IRawJpegData.cs index b1ac1f78f..b715eef98 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/IRawJpegData.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/IRawJpegData.cs @@ -26,11 +26,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// JpegColorSpace ColorSpace { get; } - /// - /// Gets the number of bits used for precision. - /// - int Precision { get; } - /// /// Gets the components. /// @@ -41,4 +36,4 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// Block8x8F[] QuantizationTables { get; } } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBlockPostProcessor.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBlockPostProcessor.cs index e0311dafe..7cfbaddcc 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBlockPostProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBlockPostProcessor.cs @@ -38,11 +38,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// private Size subSamplingDivisors; - /// - /// Defines the maximum value derived from the bitdepth. - /// - private readonly int maximumValue; - /// /// Initializes a new instance of the struct. /// @@ -53,7 +48,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder int qtIndex = component.QuantizationTableIndex; this.DequantiazationTable = ZigZag.CreateDequantizationTable(ref decoder.QuantizationTables[qtIndex]); this.subSamplingDivisors = component.SubSamplingDivisors; - this.maximumValue = (int)MathF.Pow(2, decoder.Precision) - 1; this.SourceBlock = default; this.WorkspaceBlock1 = default; diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponentPostProcessor.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponentPostProcessor.cs index 79965a3f0..31214b4c1 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponentPostProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponentPostProcessor.cs @@ -21,11 +21,18 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// private readonly Size blockAreaSize; + /// + /// Jpeg frame instance containing required decoding metadata. + /// + private readonly JpegFrame frame; + /// /// Initializes a new instance of the class. /// - public JpegComponentPostProcessor(MemoryAllocator memoryAllocator, IRawJpegData rawJpeg, Size postProcessorBufferSize, IJpegComponent component) + public JpegComponentPostProcessor(MemoryAllocator memoryAllocator, JpegFrame frame, IRawJpegData rawJpeg, Size postProcessorBufferSize, IJpegComponent component) { + this.frame = frame; + this.Component = component; this.RawJpeg = rawJpeg; this.blockAreaSize = this.Component.SubSamplingDivisors * 8; @@ -70,7 +77,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder Buffer2D spectralBuffer = this.Component.SpectralBlocks; var blockPp = new JpegBlockPostProcessor(this.RawJpeg, this.Component); - float maximumValue = MathF.Pow(2, this.RawJpeg.Precision) - 1; + + // TODO: this is a constant value for ALL components + float maximumValue = MathF.Pow(2, this.frame.Precision) - 1; int destAreaStride = this.ColorBuffer.Width; diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs index 9f3d4195c..50cfa0188 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs @@ -78,7 +78,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder this.componentProcessors = new JpegComponentPostProcessor[frame.Components.Length]; for (int i = 0; i < this.componentProcessors.Length; i++) { - this.componentProcessors[i] = new JpegComponentPostProcessor(allocator, jpegData, postProcessorBufferSize, frame.Components[i]); + this.componentProcessors[i] = new JpegComponentPostProcessor(allocator, frame, jpegData, postProcessorBufferSize, frame.Components[i]); } // single 'stride' rgba32 buffer for conversion between spectral and TPixel diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 97a6d3999..3c262be32 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -30,7 +30,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg /// /// The only supported precision /// - private readonly int[] supportedPrecisions = { 8, 12 }; + private readonly byte[] supportedPrecisions = { 8, 12 }; /// /// The buffer used to temporarily store bytes read from the stream. @@ -148,9 +148,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg /// public JpegColorSpace ColorSpace { get; private set; } - /// - public int Precision { get; private set; } - /// /// Gets the components. /// @@ -825,23 +822,24 @@ namespace SixLabors.ImageSharp.Formats.Jpeg JpegThrowHelper.ThrowInvalidImageContentException("Multiple SOF markers. Only single frame jpegs supported."); } - // Read initial marker definitions. + // Read initial marker definitions const int length = 6; stream.Read(this.temp, 0, length); - // We only support 8-bit and 12-bit precision. - if (Array.IndexOf(this.supportedPrecisions, this.temp[0]) == -1) + // 1 byte: Bits/sample precision + byte precision = this.temp[0]; + + // Validity check: only 8-bit and 12-bit precisions are supported + if (Array.IndexOf(this.supportedPrecisions, precision) == -1) { JpegThrowHelper.ThrowInvalidImageContentException("Only 8-Bit and 12-Bit precision supported."); } - this.Precision = this.temp[0]; - this.Frame = new JpegFrame { Extended = frameMarker.Marker == JpegConstants.Markers.SOF1, Progressive = frameMarker.Marker == JpegConstants.Markers.SOF2, - Precision = this.temp[0], + Precision = precision, PixelHeight = (this.temp[1] << 8) | this.temp[2], PixelWidth = (this.temp[3] << 8) | this.temp[4], ComponentCount = this.temp[5] From 04eef159b3a4a52b99adf935550ee6942cce9e85 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 02:07:32 +0300 Subject: [PATCH 05/21] Added frame dimensions proper check & comments --- .../Formats/Jpeg/JpegDecoderCore.cs | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 3c262be32..45d08dadf 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -835,21 +835,29 @@ namespace SixLabors.ImageSharp.Formats.Jpeg JpegThrowHelper.ThrowInvalidImageContentException("Only 8-Bit and 12-Bit precision supported."); } + // 2 byte: height + int frameHeight = (this.temp[1] << 8) | this.temp[2]; + + // 2 byte: width + int frameWidth = (this.temp[3] << 8) | this.temp[4]; + + // Validity check: width/height > 0 (they are upper-bounded by 2 byte max value so no need to check that) + if (frameHeight == 0 || frameWidth == 0) + { + JpegThrowHelper.ThrowInvalidImageDimensions(this.Frame.PixelWidth, this.Frame.PixelHeight); + } + + this.Frame = new JpegFrame { Extended = frameMarker.Marker == JpegConstants.Markers.SOF1, Progressive = frameMarker.Marker == JpegConstants.Markers.SOF2, Precision = precision, - PixelHeight = (this.temp[1] << 8) | this.temp[2], - PixelWidth = (this.temp[3] << 8) | this.temp[4], + PixelHeight = frameHeight, + PixelWidth = frameWidth, ComponentCount = this.temp[5] }; - if (this.Frame.PixelWidth == 0 || this.Frame.PixelHeight == 0) - { - JpegThrowHelper.ThrowInvalidImageDimensions(this.Frame.PixelWidth, this.Frame.PixelHeight); - } - this.ImageSizeInPixels = new Size(this.Frame.PixelWidth, this.Frame.PixelHeight); this.ComponentCount = this.Frame.ComponentCount; From 24b1ca64d195c801b3acbeb73302c37d2cb2ce87 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 02:22:04 +0300 Subject: [PATCH 06/21] Added component count proper check, comments & decoupled it from actual decoding --- .../Jpeg/Components/Decoder/IRawJpegData.cs | 5 --- .../Jpeg/Components/Decoder/JpegFrame.cs | 5 +++ .../Formats/Jpeg/JpegDecoderCore.cs | 43 ++++++++----------- .../Formats/Jpg/ParseStreamTests.cs | 4 +- 4 files changed, 25 insertions(+), 32 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/IRawJpegData.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/IRawJpegData.cs index b715eef98..948f4dc8c 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/IRawJpegData.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/IRawJpegData.cs @@ -16,11 +16,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// Size ImageSizeInPixels { get; } - /// - /// Gets the number of components. - /// - int ComponentCount { get; } - /// /// Gets the color space /// diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs index 3a136b410..a1242a43a 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs @@ -84,6 +84,11 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// public int McusPerColumn { get; set; } + /// + /// Gets the color depth, in number of bits per pixel. + /// + public int BitsPerPixel => this.ComponentCount * this.Precision; + /// public void Dispose() { diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 45d08dadf..0fd4b2f0f 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -127,11 +127,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg /// public int ImageHeight => this.ImageSizeInPixels.Height; - /// - /// Gets the color depth, in number of bits per pixel. - /// - public int BitsPerPixel => this.ComponentCount * this.Frame.Precision; - /// /// Gets a value indicating whether the metadata should be ignored when the image is being decoded. /// @@ -142,9 +137,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg /// public ImageMetadata Metadata { get; private set; } - /// - public int ComponentCount { get; private set; } - /// public JpegColorSpace ColorSpace { get; private set; } @@ -222,7 +214,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg this.InitIptcProfile(); this.InitDerivedMetadataProperties(); - return new ImageInfo(new PixelTypeInfo(this.BitsPerPixel), this.ImageWidth, this.ImageHeight, this.Metadata); + return new ImageInfo(new PixelTypeInfo(this.Frame.BitsPerPixel), this.ImageWidth, this.ImageHeight, this.Metadata); } /// @@ -373,14 +365,14 @@ namespace SixLabors.ImageSharp.Formats.Jpeg /// Returns the correct colorspace based on the image component count /// /// The - private JpegColorSpace DeduceJpegColorSpace() + private JpegColorSpace DeduceJpegColorSpace(byte componentCount) { - if (this.ComponentCount == 1) + if (componentCount == 1) { return JpegColorSpace.Grayscale; } - if (this.ComponentCount == 3) + if (componentCount == 3) { if (!this.adobe.Equals(default) && this.adobe.ColorTransform == JpegConstants.Adobe.ColorTransformUnknown) { @@ -392,14 +384,14 @@ namespace SixLabors.ImageSharp.Formats.Jpeg return JpegColorSpace.YCbCr; } - if (this.ComponentCount == 4) + if (componentCount == 4) { return this.adobe.ColorTransform == JpegConstants.Adobe.ColorTransformYcck ? JpegColorSpace.Ycck : JpegColorSpace.Cmyk; } - JpegThrowHelper.ThrowInvalidImageContentException($"Unsupported color mode. Supported component counts 1, 3, and 4; found {this.ComponentCount}"); + JpegThrowHelper.ThrowInvalidImageContentException($"Unsupported color mode. Supported component counts 1, 3, and 4; found {componentCount}"); return default; } @@ -835,18 +827,21 @@ namespace SixLabors.ImageSharp.Formats.Jpeg JpegThrowHelper.ThrowInvalidImageContentException("Only 8-Bit and 12-Bit precision supported."); } - // 2 byte: height + // 2 byte: Height int frameHeight = (this.temp[1] << 8) | this.temp[2]; - // 2 byte: width + // 2 byte: Width int frameWidth = (this.temp[3] << 8) | this.temp[4]; // Validity check: width/height > 0 (they are upper-bounded by 2 byte max value so no need to check that) if (frameHeight == 0 || frameWidth == 0) { - JpegThrowHelper.ThrowInvalidImageDimensions(this.Frame.PixelWidth, this.Frame.PixelHeight); + JpegThrowHelper.ThrowInvalidImageDimensions(frameWidth, frameHeight); } + // 1 byte: Number of components + byte componentCount = this.temp[5]; + this.ColorSpace = this.DeduceJpegColorSpace(componentCount); this.Frame = new JpegFrame { @@ -855,13 +850,11 @@ namespace SixLabors.ImageSharp.Formats.Jpeg Precision = precision, PixelHeight = frameHeight, PixelWidth = frameWidth, - ComponentCount = this.temp[5] + ComponentCount = componentCount }; this.ImageSizeInPixels = new Size(this.Frame.PixelWidth, this.Frame.PixelHeight); - this.ComponentCount = this.Frame.ComponentCount; - this.ColorSpace = this.DeduceJpegColorSpace(); this.Metadata.GetJpegMetadata().ColorType = this.ColorSpace == JpegColorSpace.Grayscale ? JpegColorType.Luminance : JpegColorType.YCbCr; if (!metadataOnly) @@ -869,7 +862,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg remaining -= length; const int componentBytes = 3; - if (remaining > this.ComponentCount * componentBytes) + if (remaining > componentCount * componentBytes) { JpegThrowHelper.ThrowBadMarker("SOFn", remaining); } @@ -877,14 +870,14 @@ namespace SixLabors.ImageSharp.Formats.Jpeg stream.Read(this.temp, 0, remaining); // No need to pool this. They max out at 4 - this.Frame.ComponentIds = new byte[this.ComponentCount]; - this.Frame.ComponentOrder = new byte[this.ComponentCount]; - this.Frame.Components = new JpegComponent[this.ComponentCount]; + this.Frame.ComponentIds = new byte[componentCount]; + this.Frame.ComponentOrder = new byte[componentCount]; + this.Frame.Components = new JpegComponent[componentCount]; int maxH = 0; int maxV = 0; int index = 0; - for (int i = 0; i < this.ComponentCount; i++) + for (int i = 0; i < componentCount; i++) { byte hv = this.temp[index + 1]; int h = (hv >> 4) & 15; diff --git a/tests/ImageSharp.Tests/Formats/Jpg/ParseStreamTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/ParseStreamTests.cs index a124ec191..2162ee13c 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/ParseStreamTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/ParseStreamTests.cs @@ -43,7 +43,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg { using (JpegDecoderCore decoder = JpegFixture.ParseJpegStream(TestImages.Jpeg.Baseline.Jpeg400)) { - Assert.Equal(1, decoder.ComponentCount); + Assert.Equal(1, decoder.Frame.ComponentCount); Assert.Equal(1, decoder.Components.Length); Size expectedSizeInBlocks = decoder.ImageSizeInPixels.DivideRoundUp(8); @@ -106,7 +106,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg using (JpegDecoderCore decoder = JpegFixture.ParseJpegStream(imageFile)) { - Assert.Equal(componentCount, decoder.ComponentCount); + Assert.Equal(componentCount, decoder.Frame.ComponentCount); Assert.Equal(componentCount, decoder.Components.Length); JpegComponent c0 = decoder.Components[0]; From 6b214ca1291153099fafd8221b305cd6107ead6b Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 12:32:01 +0300 Subject: [PATCH 07/21] Removed core obsolete properties, decoupled frame metadata from the decoder --- .../Jpeg/Components/Decoder/IRawJpegData.cs | 5 ----- .../Jpeg/Components/Decoder/JpegFrame.cs | 5 +++++ src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 17 +++-------------- 3 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/IRawJpegData.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/IRawJpegData.cs index 948f4dc8c..391dac784 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/IRawJpegData.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/IRawJpegData.cs @@ -11,11 +11,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// internal interface IRawJpegData : IDisposable { - /// - /// Gets the image size in pixels. - /// - Size ImageSizeInPixels { get; } - /// /// Gets the color space /// diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs index a1242a43a..4baaab386 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs @@ -43,6 +43,11 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// public int PixelWidth { get; set; } + /// + /// Gets the pixel size of the image. + /// + public Size PixelSize => new Size(this.PixelWidth, this.PixelHeight); + /// /// Gets or sets the number of components within a frame. In progressive frames this value can range from only 1 to 4. /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 0fd4b2f0f..04495f172 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -110,23 +110,13 @@ namespace SixLabors.ImageSharp.Formats.Jpeg public Size ImageSizeInPixels { get; private set; } /// - Size IImageDecoderInternals.Dimensions => this.ImageSizeInPixels; + Size IImageDecoderInternals.Dimensions => this.Frame.PixelSize; /// /// Gets the number of MCU blocks in the image as . /// public Size ImageSizeInMCU { get; private set; } - /// - /// Gets the image width - /// - public int ImageWidth => this.ImageSizeInPixels.Width; - - /// - /// Gets the image height - /// - public int ImageHeight => this.ImageSizeInPixels.Height; - /// /// Gets a value indicating whether the metadata should be ignored when the image is being decoded. /// @@ -214,7 +204,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg this.InitIptcProfile(); this.InitDerivedMetadataProperties(); - return new ImageInfo(new PixelTypeInfo(this.Frame.BitsPerPixel), this.ImageWidth, this.ImageHeight, this.Metadata); + Size pixelSize = this.Frame.PixelSize; + return new ImageInfo(new PixelTypeInfo(this.Frame.BitsPerPixel), pixelSize.Width, pixelSize.Height, this.Metadata); } /// @@ -853,8 +844,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg ComponentCount = componentCount }; - this.ImageSizeInPixels = new Size(this.Frame.PixelWidth, this.Frame.PixelHeight); - this.Metadata.GetJpegMetadata().ColorType = this.ColorSpace == JpegColorSpace.Grayscale ? JpegColorType.Luminance : JpegColorType.YCbCr; if (!metadataOnly) From 7077473d71bff5029841bd51245d11c085e3900e Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 12:57:12 +0300 Subject: [PATCH 08/21] Decoupled mcu size from the decoder, fixed SOF component bytes length check --- .../Formats/Jpeg/Components/Decoder/JpegFrame.cs | 5 +++++ src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 14 +++----------- .../Formats/Jpg/ParseStreamTests.cs | 6 +++--- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs index 4baaab386..b8f88cfe0 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs @@ -89,6 +89,11 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// public int McusPerColumn { get; set; } + /// + /// Gets the mcu size of the image. + /// + public Size McuSize => new Size(this.McusPerLine, this.McusPerColumn); + /// /// Gets the color depth, in number of bits per pixel. /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 04495f172..3c48aabee 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -112,11 +112,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg /// Size IImageDecoderInternals.Dimensions => this.Frame.PixelSize; - /// - /// Gets the number of MCU blocks in the image as . - /// - public Size ImageSizeInMCU { get; private set; } - /// /// Gets a value indicating whether the metadata should be ignored when the image is being decoded. /// @@ -834,6 +829,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg byte componentCount = this.temp[5]; this.ColorSpace = this.DeduceJpegColorSpace(componentCount); + this.Metadata.GetJpegMetadata().ColorType = this.ColorSpace == JpegColorSpace.Grayscale ? JpegColorType.Luminance : JpegColorType.YCbCr; + this.Frame = new JpegFrame { Extended = frameMarker.Marker == JpegConstants.Markers.SOF1, @@ -844,14 +841,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg ComponentCount = componentCount }; - this.Metadata.GetJpegMetadata().ColorType = this.ColorSpace == JpegColorSpace.Grayscale ? JpegColorType.Luminance : JpegColorType.YCbCr; - if (!metadataOnly) { remaining -= length; const int componentBytes = 3; - if (remaining > componentCount * componentBytes) + if (remaining != componentCount * componentBytes) { JpegThrowHelper.ThrowBadMarker("SOFn", remaining); } @@ -894,9 +889,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg this.Frame.MaxVerticalFactor = maxV; this.Frame.InitComponents(); - this.ImageSizeInMCU = new Size(this.Frame.McusPerLine, this.Frame.McusPerColumn); - - // This can be injected in SOF marker callback this.scanDecoder.InjectFrameData(this.Frame, this); } } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/ParseStreamTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/ParseStreamTests.cs index 2162ee13c..e1307d3fc 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/ParseStreamTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/ParseStreamTests.cs @@ -48,7 +48,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg Size expectedSizeInBlocks = decoder.ImageSizeInPixels.DivideRoundUp(8); - Assert.Equal(expectedSizeInBlocks, decoder.ImageSizeInMCU); + Assert.Equal(expectedSizeInBlocks, decoder.Frame.McuSize); var uniform1 = new Size(1, 1); JpegComponent c0 = decoder.Components[0]; @@ -70,7 +70,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg using (JpegDecoderCore decoder = JpegFixture.ParseJpegStream(imageFile)) { sb.AppendLine(imageFile); - sb.AppendLine($"Size:{decoder.ImageSizeInPixels} MCU:{decoder.ImageSizeInMCU}"); + sb.AppendLine($"Size:{decoder.ImageSizeInPixels} MCU:{decoder.Frame.McuSize}"); JpegComponent c0 = decoder.Components[0]; JpegComponent c1 = decoder.Components[1]; @@ -115,7 +115,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg var uniform1 = new Size(1, 1); - Size expectedLumaSizeInBlocks = decoder.ImageSizeInMCU.MultiplyBy(fLuma); + Size expectedLumaSizeInBlocks = decoder.Frame.McuSize.MultiplyBy(fLuma); Size divisor = fLuma.DivideBy(fChroma); From 4d599d14f63d15f06838f530d067a7558b1d734a Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 13:32:38 +0300 Subject: [PATCH 09/21] Comments, docs, decoupling, removed redundant properties --- .../Jpeg/Components/Decoder/JpegComponent.cs | 11 +++++++--- .../Jpeg/Components/Decoder/JpegFrame.cs | 20 ++++++------------- .../Formats/Jpeg/JpegDecoderCore.cs | 9 +++------ .../Formats/Jpg/ParseStreamTests.cs | 4 ++-- 4 files changed, 19 insertions(+), 25 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponent.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponent.cs index 614e96e54..33b5ef77a 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponent.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponent.cs @@ -106,13 +106,18 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder this.SpectralBlocks = null; } - public void Init() + /// + /// Initializes component for future buffers initialization. + /// + /// Maximal horizontal subsampling factor among all the components. + /// Maximal vertical subsampling factor among all the components. + public void Init(int maxSubFactorH, int maxSubFactorV) { this.WidthInBlocks = (int)MathF.Ceiling( - MathF.Ceiling(this.Frame.PixelWidth / 8F) * this.HorizontalSamplingFactor / this.Frame.MaxHorizontalFactor); + MathF.Ceiling(this.Frame.PixelWidth / 8F) * this.HorizontalSamplingFactor / maxSubFactorH); this.HeightInBlocks = (int)MathF.Ceiling( - MathF.Ceiling(this.Frame.PixelHeight / 8F) * this.VerticalSamplingFactor / this.Frame.MaxVerticalFactor); + MathF.Ceiling(this.Frame.PixelHeight / 8F) * this.VerticalSamplingFactor / maxSubFactorV); int blocksPerLineForMcu = this.Frame.McusPerLine * this.HorizontalSamplingFactor; int blocksPerColumnForMcu = this.Frame.McusPerColumn * this.VerticalSamplingFactor; diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs index b8f88cfe0..01863b7a8 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs @@ -69,16 +69,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// public JpegComponent[] Components { get; set; } - /// - /// Gets or sets the maximum horizontal sampling factor. - /// - public int MaxHorizontalFactor { get; set; } - - /// - /// Gets or sets the maximum vertical sampling factor. - /// - public int MaxVerticalFactor { get; set; } - /// /// Gets or sets the number of MCU's per line. /// @@ -116,15 +106,17 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// /// Allocates the frame component blocks. /// - public void InitComponents() + /// Maximal horizontal subsampling factor among all the components. + /// Maximal vertical subsampling factor among all the components. + public void Init(int maxSubFactorH, int maxSubFactorV) { - this.McusPerLine = (int)Numerics.DivideCeil((uint)this.PixelWidth, (uint)this.MaxHorizontalFactor * 8); - this.McusPerColumn = (int)Numerics.DivideCeil((uint)this.PixelHeight, (uint)this.MaxVerticalFactor * 8); + this.McusPerLine = (int)Numerics.DivideCeil((uint)this.PixelWidth, (uint)maxSubFactorH * 8); + this.McusPerColumn = (int)Numerics.DivideCeil((uint)this.PixelHeight, (uint)maxSubFactorV * 8); for (int i = 0; i < this.ComponentCount; i++) { JpegComponent component = this.Components[i]; - component.Init(); + component.Init(maxSubFactorH, maxSubFactorV); } } diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 3c48aabee..406458fe3 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -106,9 +106,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg /// public JpegFrame Frame { get; private set; } - /// - public Size ImageSizeInPixels { get; private set; } - /// Size IImageDecoderInternals.Dimensions => this.Frame.PixelSize; @@ -845,12 +842,14 @@ namespace SixLabors.ImageSharp.Formats.Jpeg { remaining -= length; + // Validity check: remaining part must be equal to components * 3 const int componentBytes = 3; if (remaining != componentCount * componentBytes) { JpegThrowHelper.ThrowBadMarker("SOFn", remaining); } + // components*3 bytes: component data stream.Read(this.temp, 0, remaining); // No need to pool this. They max out at 4 @@ -885,9 +884,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg index += componentBytes; } - this.Frame.MaxHorizontalFactor = maxH; - this.Frame.MaxVerticalFactor = maxV; - this.Frame.InitComponents(); + this.Frame.Init(maxH, maxV); this.scanDecoder.InjectFrameData(this.Frame, this); } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/ParseStreamTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/ParseStreamTests.cs index e1307d3fc..0a4d85344 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/ParseStreamTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/ParseStreamTests.cs @@ -46,7 +46,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg Assert.Equal(1, decoder.Frame.ComponentCount); Assert.Equal(1, decoder.Components.Length); - Size expectedSizeInBlocks = decoder.ImageSizeInPixels.DivideRoundUp(8); + Size expectedSizeInBlocks = decoder.Frame.PixelSize.DivideRoundUp(8); Assert.Equal(expectedSizeInBlocks, decoder.Frame.McuSize); @@ -70,7 +70,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg using (JpegDecoderCore decoder = JpegFixture.ParseJpegStream(imageFile)) { sb.AppendLine(imageFile); - sb.AppendLine($"Size:{decoder.ImageSizeInPixels} MCU:{decoder.Frame.McuSize}"); + sb.AppendLine($"Size:{decoder.Frame.PixelSize} MCU:{decoder.Frame.McuSize}"); JpegComponent c0 = decoder.Components[0]; JpegComponent c1 = decoder.Components[1]; From c751b27334b579a16482b2d5634c15390f33b776 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 14:34:45 +0300 Subject: [PATCH 10/21] Removed first component dependency in compoentn initialization code --- .../Formats/Jpeg/Components/Decoder/JpegComponent.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponent.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponent.cs index 33b5ef77a..ba3dfb629 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponent.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponent.cs @@ -123,8 +123,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder int blocksPerColumnForMcu = this.Frame.McusPerColumn * this.VerticalSamplingFactor; this.SizeInBlocks = new Size(blocksPerLineForMcu, blocksPerColumnForMcu); - JpegComponent c0 = this.Frame.Components[0]; - this.SubSamplingDivisors = c0.SamplingFactors.DivideBy(this.SamplingFactors); + this.SubSamplingDivisors = new Size(maxSubFactorH, maxSubFactorV).DivideBy(this.SamplingFactors); if (this.SubSamplingDivisors.Width == 0 || this.SubSamplingDivisors.Height == 0) { From 0e4e9501e59f371b2d590173fd1709eaaa641730 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 15:11:10 +0300 Subject: [PATCH 11/21] Introduced JpegFrame ctor, closed some setters --- .../Jpeg/Components/Decoder/JpegFrame.cs | 43 +++++++++++++------ .../Formats/Jpeg/JpegDecoderCore.cs | 10 +---- 2 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs index 01863b7a8..0e842de9d 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs @@ -10,15 +10,29 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// internal sealed class JpegFrame : IDisposable { + public JpegFrame(JpegFileMarker sofMarker, byte precision, int width, int height, byte componentCount) + { + this.Extended = sofMarker.Marker == JpegConstants.Markers.SOF1; + this.Progressive = sofMarker.Marker == JpegConstants.Markers.SOF2; + + this.Precision = precision; + this.MaxColorChannelValue = MathF.Pow(2, precision) - 1; + + this.PixelWidth = width; + this.PixelHeight = height; + + this.ComponentCount = componentCount; + } + /// - /// Gets or sets a value indicating whether the frame uses the extended specification. + /// Gets a value indicating whether the frame uses the extended specification. /// - public bool Extended { get; set; } + public bool Extended { get; private set; } /// - /// Gets or sets a value indicating whether the frame uses the progressive specification. + /// Gets a value indicating whether the frame uses the progressive specification. /// - public bool Progressive { get; set; } + public bool Progressive { get; private set; } /// /// Gets or sets a value indicating whether the frame is encoded using multiple scans (SOS markers). @@ -29,19 +43,24 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder public bool MultiScan { get; set; } /// - /// Gets or sets the precision. + /// Gets the precision. + /// + public byte Precision { get; private set; } + + /// + /// Gets the maximum color value derived from . /// - public byte Precision { get; set; } + public float MaxColorChannelValue { get; private set; } /// - /// Gets or sets the number of scanlines within the frame. + /// Gets the number of pixel per row. /// - public int PixelHeight { get; set; } + public int PixelHeight { get; private set; } /// - /// Gets or sets the number of samples per scanline. + /// Gets the number of pixels per line. /// - public int PixelWidth { get; set; } + public int PixelWidth { get; private set; } /// /// Gets the pixel size of the image. @@ -49,9 +68,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder public Size PixelSize => new Size(this.PixelWidth, this.PixelHeight); /// - /// Gets or sets the number of components within a frame. In progressive frames this value can range from only 1 to 4. + /// Gets the number of components within a frame. In progressive frames this value can range from only 1 to 4. /// - public byte ComponentCount { get; set; } + public byte ComponentCount { get; private set; } /// /// Gets or sets the component id collection. diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 406458fe3..86871fc00 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -828,15 +828,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg this.Metadata.GetJpegMetadata().ColorType = this.ColorSpace == JpegColorSpace.Grayscale ? JpegColorType.Luminance : JpegColorType.YCbCr; - this.Frame = new JpegFrame - { - Extended = frameMarker.Marker == JpegConstants.Markers.SOF1, - Progressive = frameMarker.Marker == JpegConstants.Markers.SOF2, - Precision = precision, - PixelHeight = frameHeight, - PixelWidth = frameWidth, - ComponentCount = componentCount - }; + this.Frame = new JpegFrame(frameMarker, precision, frameWidth, frameHeight, componentCount); if (!metadataOnly) { From 36a1ea6456c7506fa0041e8badae00aff7f595d9 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 15:14:24 +0300 Subject: [PATCH 12/21] Color channel max value is now cached per jpeg frame --- .../Jpeg/Components/Decoder/JpegComponentPostProcessor.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponentPostProcessor.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponentPostProcessor.cs index 31214b4c1..9a659d621 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponentPostProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponentPostProcessor.cs @@ -78,8 +78,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder var blockPp = new JpegBlockPostProcessor(this.RawJpeg, this.Component); - // TODO: this is a constant value for ALL components - float maximumValue = MathF.Pow(2, this.frame.Precision) - 1; + float maximumValue = this.frame.MaxColorChannelValue; int destAreaStride = this.ColorBuffer.Width; From 00c1a2138015d323dec2ab21066d72079f87a617 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 15:38:55 +0300 Subject: [PATCH 13/21] Fixed fuzzed issue related to selectorsCount, added appropriate checks --- .../Formats/Jpeg/Components/Decoder/JpegFrame.cs | 2 +- src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 14 +++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs index 0e842de9d..fc109be26 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegFrame.cs @@ -68,7 +68,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder public Size PixelSize => new Size(this.PixelWidth, this.PixelHeight); /// - /// Gets the number of components within a frame. In progressive frames this value can range from only 1 to 4. + /// Gets the number of components within a frame. /// public byte ComponentCount { get; private set; } diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 86871fc00..00ea05ba5 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -804,7 +804,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg // 1 byte: Bits/sample precision byte precision = this.temp[0]; - // Validity check: only 8-bit and 12-bit precisions are supported + // Validate: only 8-bit and 12-bit precisions are supported if (Array.IndexOf(this.supportedPrecisions, precision) == -1) { JpegThrowHelper.ThrowInvalidImageContentException("Only 8-Bit and 12-Bit precision supported."); @@ -816,7 +816,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg // 2 byte: Width int frameWidth = (this.temp[3] << 8) | this.temp[4]; - // Validity check: width/height > 0 (they are upper-bounded by 2 byte max value so no need to check that) + // Validate: width/height > 0 (they are upper-bounded by 2 byte max value so no need to check that) if (frameHeight == 0 || frameWidth == 0) { JpegThrowHelper.ThrowInvalidImageDimensions(frameWidth, frameHeight); @@ -834,7 +834,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg { remaining -= length; - // Validity check: remaining part must be equal to components * 3 + // Validate: remaining part must be equal to components * 3 const int componentBytes = 3; if (remaining != componentCount * componentBytes) { @@ -978,7 +978,15 @@ namespace SixLabors.ImageSharp.Formats.Jpeg JpegThrowHelper.ThrowInvalidImageContentException("No readable SOFn (Start Of Frame) marker found."); } + // 1 byte: Number of components in scan int selectorsCount = stream.ReadByte(); + + // Validate: 0 < count <= totalComponents + if (selectorsCount == 0 || selectorsCount > this.Frame.ComponentCount) + { + JpegThrowHelper.ThrowInvalidImageContentException($"Invalid number of components in scan: {selectorsCount}. Must be [0 < count <= {this.Frame.ComponentCount}]"); + } + this.Frame.MultiScan = this.Frame.ComponentCount != selectorsCount; for (int i = 0; i < selectorsCount; i++) { From 245b7868403564f3d2cf3b498a8eeec73f28f5bc Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 15:45:14 +0300 Subject: [PATCH 14/21] Small refactoring, added progressive data comments --- src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 00ea05ba5..4e4d6ec52 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -984,7 +984,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg // Validate: 0 < count <= totalComponents if (selectorsCount == 0 || selectorsCount > this.Frame.ComponentCount) { - JpegThrowHelper.ThrowInvalidImageContentException($"Invalid number of components in scan: {selectorsCount}. Must be [0 < count <= {this.Frame.ComponentCount}]"); + JpegThrowHelper.ThrowInvalidImageContentException($"Invalid number of components in scan: {selectorsCount}. Must be [1 <= count <= {this.Frame.ComponentCount}]"); } this.Frame.MultiScan = this.Frame.ComponentCount != selectorsCount; @@ -1008,22 +1008,24 @@ namespace SixLabors.ImageSharp.Formats.Jpeg JpegThrowHelper.ThrowInvalidImageContentException($"Unknown component selector {componentIndex}."); } - ref JpegComponent component = ref this.Frame.Components[componentIndex]; + this.Frame.ComponentOrder[i] = (byte)componentIndex; + int tableSpec = stream.ReadByte(); + ref JpegComponent component = ref this.Frame.Components[componentIndex]; component.DCHuffmanTableId = tableSpec >> 4; component.ACHuffmanTableId = tableSpec & 15; - this.Frame.ComponentOrder[i] = (byte)componentIndex; } + // 3 bytes: Progressive scan decoding data stream.Read(this.temp, 0, 3); int spectralStart = this.temp[0]; - int spectralEnd = this.temp[1]; - int successiveApproximation = this.temp[2]; - - // This is okay to inject here, might be good to wrap it in a separate struct but not really necessary this.scanDecoder.SpectralStart = spectralStart; + + int spectralEnd = this.temp[1]; this.scanDecoder.SpectralEnd = spectralEnd; + + int successiveApproximation = this.temp[2]; this.scanDecoder.SuccessiveHigh = successiveApproximation >> 4; this.scanDecoder.SuccessiveLow = successiveApproximation & 15; From 5596ce1830057a579d7a8903a47b5be2dbe51f42 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 15:55:58 +0300 Subject: [PATCH 15/21] Refactored componentIndex validation --- src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 4e4d6ec52..35f88e495 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -984,28 +984,32 @@ namespace SixLabors.ImageSharp.Formats.Jpeg // Validate: 0 < count <= totalComponents if (selectorsCount == 0 || selectorsCount > this.Frame.ComponentCount) { - JpegThrowHelper.ThrowInvalidImageContentException($"Invalid number of components in scan: {selectorsCount}. Must be [1 <= count <= {this.Frame.ComponentCount}]"); + // TODO: extract as separate method? + JpegThrowHelper.ThrowInvalidImageContentException($"Invalid number of components in scan: {selectorsCount}. Must be [1 <= count <= {this.Frame.ComponentCount}]."); } this.Frame.MultiScan = this.Frame.ComponentCount != selectorsCount; for (int i = 0; i < selectorsCount; i++) { - int componentIndex = -1; - int selector = stream.ReadByte(); + // 1 byte: Component id + int componentSelectorId = stream.ReadByte(); + int componentIndex = -1; for (int j = 0; j < this.Frame.ComponentIds.Length; j++) { byte id = this.Frame.ComponentIds[j]; - if (selector == id) + if (componentSelectorId == id) { componentIndex = j; break; } } - if (componentIndex < 0) + // Validate: must be found among registered components + if (componentIndex == -1) { - JpegThrowHelper.ThrowInvalidImageContentException($"Unknown component selector {componentIndex}."); + // TODO: extract as separate method? + JpegThrowHelper.ThrowInvalidImageContentException($"Invalid component id in scan: {componentSelectorId}. Must be [0 <= id <= {this.Frame.ComponentCount - 1}]"); } this.Frame.ComponentOrder[i] = (byte)componentIndex; From 95bec1cffbe0fad53135f7fbee734b29d6a47b53 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 16:41:22 +0300 Subject: [PATCH 16/21] Added comments, validated huffman table indices --- .../Formats/Jpeg/JpegDecoderCore.cs | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 35f88e495..8dc0bb501 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -985,7 +985,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg if (selectorsCount == 0 || selectorsCount > this.Frame.ComponentCount) { // TODO: extract as separate method? - JpegThrowHelper.ThrowInvalidImageContentException($"Invalid number of components in scan: {selectorsCount}. Must be [1 <= count <= {this.Frame.ComponentCount}]."); + JpegThrowHelper.ThrowInvalidImageContentException($"Invalid number of components in scan: {selectorsCount}."); } this.Frame.MultiScan = this.Frame.ComponentCount != selectorsCount; @@ -1009,15 +1009,28 @@ namespace SixLabors.ImageSharp.Formats.Jpeg if (componentIndex == -1) { // TODO: extract as separate method? - JpegThrowHelper.ThrowInvalidImageContentException($"Invalid component id in scan: {componentSelectorId}. Must be [0 <= id <= {this.Frame.ComponentCount - 1}]"); + JpegThrowHelper.ThrowInvalidImageContentException($"Unknown component id in scan: {componentSelectorId}."); } this.Frame.ComponentOrder[i] = (byte)componentIndex; + JpegComponent component = this.Frame.Components[componentIndex]; + + // 1 byte: Huffman table selectors. + // 4 bits - dc + // 4 bits - ac int tableSpec = stream.ReadByte(); - ref JpegComponent component = ref this.Frame.Components[componentIndex]; - component.DCHuffmanTableId = tableSpec >> 4; - component.ACHuffmanTableId = tableSpec & 15; + int dcTableIndex = tableSpec >> 4; + int acTableIndex = tableSpec & 15; + + // Validate: both must be < 4 + if (dcTableIndex >= 4 || acTableIndex >= 4) + { + JpegThrowHelper.ThrowInvalidImageContentException($"Invalid huffman table for component:{componentSelectorId}: dc={dcTableIndex}, ac={acTableIndex}"); + } + + component.DCHuffmanTableId = dcTableIndex; + component.ACHuffmanTableId = acTableIndex; } // 3 bytes: Progressive scan decoding data From 0ace1a042a5193eed12137962e6c55f94c953d30 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 16:52:36 +0300 Subject: [PATCH 17/21] Added issue-1693 images & tests cases - all passing after fix --- tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs | 4 +++- tests/ImageSharp.Tests/TestImages.cs | 2 ++ .../Jpg/issues/fuzz/Issue1693-IndexOutOfRangeException-A.jpg | 3 +++ .../Jpg/issues/fuzz/Issue1693-IndexOutOfRangeException-B.jpg | 3 +++ 4 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 tests/Images/Input/Jpg/issues/fuzz/Issue1693-IndexOutOfRangeException-A.jpg create mode 100644 tests/Images/Input/Jpg/issues/fuzz/Issue1693-IndexOutOfRangeException-B.jpg diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs index 304dd93a6..d12240cba 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs @@ -87,7 +87,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg TestImages.Jpeg.Issues.Fuzz.ArgumentException826B, TestImages.Jpeg.Issues.Fuzz.ArgumentException826C, TestImages.Jpeg.Issues.Fuzz.AccessViolationException827, - TestImages.Jpeg.Issues.Fuzz.ExecutionEngineException839 + TestImages.Jpeg.Issues.Fuzz.ExecutionEngineException839, + TestImages.Jpeg.Issues.Fuzz.IndexOutOfRangeException1693A, + TestImages.Jpeg.Issues.Fuzz.IndexOutOfRangeException1693B }; private static readonly Dictionary CustomToleranceValues = diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 6d2f65f57..fac8cb4a3 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -261,6 +261,8 @@ namespace SixLabors.ImageSharp.Tests public const string AccessViolationException827 = "Jpg/issues/fuzz/Issue827-AccessViolationException.jpg"; public const string ExecutionEngineException839 = "Jpg/issues/fuzz/Issue839-ExecutionEngineException.jpg"; public const string AccessViolationException922 = "Jpg/issues/fuzz/Issue922-AccessViolationException.jpg"; + public const string IndexOutOfRangeException1693A = "Jpg/issues/fuzz/Issue1693-IndexOutOfRangeException-A.jpg"; + public const string IndexOutOfRangeException1693B = "Jpg/issues/fuzz/Issue1693-IndexOutOfRangeException-B.jpg"; } } diff --git a/tests/Images/Input/Jpg/issues/fuzz/Issue1693-IndexOutOfRangeException-A.jpg b/tests/Images/Input/Jpg/issues/fuzz/Issue1693-IndexOutOfRangeException-A.jpg new file mode 100644 index 000000000..eb8fb9010 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/fuzz/Issue1693-IndexOutOfRangeException-A.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:fbb6acd612cdb09825493d04ec7c6aba8ef2a94cc9a86c6b16218720adfb8f5c +size 58065 diff --git a/tests/Images/Input/Jpg/issues/fuzz/Issue1693-IndexOutOfRangeException-B.jpg b/tests/Images/Input/Jpg/issues/fuzz/Issue1693-IndexOutOfRangeException-B.jpg new file mode 100644 index 000000000..7dd428591 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/fuzz/Issue1693-IndexOutOfRangeException-B.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:8720a9ccf118c3f55407aa250ee490d583286c7e40c8c62a6f8ca449ca3ddff3 +size 58067 From a92161f73a0146f9e8fd5f5289000433694b0de6 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 16:56:15 +0300 Subject: [PATCH 18/21] Fixed some warnings --- .../Formats/Jpeg/JpegDecoderCore.cs | 22 +++++++++---------- .../Formats/Jpg/JpegDecoderTests.cs | 7 ++---- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 8dc0bb501..80155dcb2 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -513,7 +513,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg JpegThrowHelper.ThrowInvalidImageContentException("Bad App1 Marker length."); } - var profile = new byte[remaining]; + byte[] profile = new byte[remaining]; stream.Read(profile, 0, remaining); if (ProfileResolver.IsProfile(profile, ProfileResolver.ExifMarker)) @@ -547,14 +547,14 @@ namespace SixLabors.ImageSharp.Formats.Jpeg return; } - var identifier = new byte[Icclength]; + byte[] identifier = new byte[Icclength]; stream.Read(identifier, 0, Icclength); remaining -= Icclength; // We have read it by this point if (ProfileResolver.IsProfile(identifier, ProfileResolver.IccMarker)) { this.isIcc = true; - var profile = new byte[remaining]; + byte[] profile = new byte[remaining]; stream.Read(profile, 0, remaining); if (this.iccData is null) @@ -592,7 +592,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg remaining -= ProfileResolver.AdobePhotoshopApp13Marker.Length; if (ProfileResolver.IsProfile(this.temp, ProfileResolver.AdobePhotoshopApp13Marker)) { - var resourceBlockData = new byte[remaining]; + byte[] resourceBlockData = new byte[remaining]; stream.Read(resourceBlockData, 0, remaining); Span blockDataSpan = resourceBlockData.AsSpan(); @@ -607,8 +607,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg Span imageResourceBlockId = blockDataSpan.Slice(0, 2); if (ProfileResolver.IsProfile(imageResourceBlockId, ProfileResolver.AdobeIptcMarker)) { - var resourceBlockNameLength = ReadImageResourceNameLength(blockDataSpan); - var resourceDataSize = ReadResourceDataLength(blockDataSpan, resourceBlockNameLength); + int resourceBlockNameLength = ReadImageResourceNameLength(blockDataSpan); + int resourceDataSize = ReadResourceDataLength(blockDataSpan, resourceBlockNameLength); int dataStartIdx = 2 + resourceBlockNameLength + 4; if (resourceDataSize > 0 && blockDataSpan.Length >= dataStartIdx + resourceDataSize) { @@ -619,8 +619,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg } else { - var resourceBlockNameLength = ReadImageResourceNameLength(blockDataSpan); - var resourceDataSize = ReadResourceDataLength(blockDataSpan, resourceBlockNameLength); + int resourceBlockNameLength = ReadImageResourceNameLength(blockDataSpan); + int resourceDataSize = ReadResourceDataLength(blockDataSpan, resourceBlockNameLength); int dataStartIdx = 2 + resourceBlockNameLength + 4; if (blockDataSpan.Length < dataStartIdx + resourceDataSize) { @@ -643,7 +643,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg private static int ReadImageResourceNameLength(Span blockDataSpan) { byte nameLength = blockDataSpan[2]; - var nameDataSize = nameLength == 0 ? 2 : nameLength; + int nameDataSize = nameLength == 0 ? 2 : nameLength; if (nameDataSize % 2 != 0) { nameDataSize++; @@ -660,9 +660,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg /// The block length. [MethodImpl(InliningOptions.ShortMethod)] private static int ReadResourceDataLength(Span blockDataSpan, int resourceBlockNameLength) - { - return BinaryPrimitives.ReadInt32BigEndian(blockDataSpan.Slice(2 + resourceBlockNameLength, 4)); - } + => BinaryPrimitives.ReadInt32BigEndian(blockDataSpan.Slice(2 + resourceBlockNameLength, 4)); /// /// Processes the application header containing the Adobe identifier diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index a052ee88a..674aa6d8f 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -62,10 +62,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg return !TestEnvironment.Is64BitProcess && largeImagesToSkipOn32Bit.Contains(provider.SourceFileOrDescription); } - public JpegDecoderTests(ITestOutputHelper output) - { - this.Output = output; - } + public JpegDecoderTests(ITestOutputHelper output) => this.Output = output; private ITestOutputHelper Output { get; } @@ -163,7 +160,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg { var cts = new CancellationTokenSource(); - var file = Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, TestImages.Jpeg.Baseline.Jpeg420Small); + string file = Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, TestImages.Jpeg.Baseline.Jpeg420Small); using var pausedStream = new PausedStream(file); pausedStream.OnWaiting(s => { From 7c8261a51bacaf00887529bc4fb830963b166db2 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 17:17:16 +0300 Subject: [PATCH 19/21] Reduced number of stream reads in SOS marker, added check for remaining bytes --- .../Formats/Jpeg/JpegDecoderCore.cs | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 80155dcb2..bafd8e215 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -250,7 +250,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg case JpegConstants.Markers.SOS: if (!metadataOnly) { - this.ProcessStartOfScanMarker(stream, cancellationToken); + this.ProcessStartOfScanMarker(stream, remaining, cancellationToken); break; } else @@ -969,7 +969,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg /// /// Processes the SOS (Start of scan marker). /// - private void ProcessStartOfScanMarker(BufferedReadStream stream, CancellationToken cancellationToken) + private void ProcessStartOfScanMarker(BufferedReadStream stream, int remaining, CancellationToken cancellationToken) { if (this.Frame is null) { @@ -986,11 +986,21 @@ namespace SixLabors.ImageSharp.Formats.Jpeg JpegThrowHelper.ThrowInvalidImageContentException($"Invalid number of components in scan: {selectorsCount}."); } + // Validate: marker must contain exactly (4 + selectorsCount*2) bytes + int selectorsBytes = selectorsCount * 2; + if (remaining != 4 + selectorsBytes) + { + JpegThrowHelper.ThrowBadMarker("SOS", remaining); + } + + // selectorsCount*2 bytes: component index + huffman tables indices + stream.Read(this.temp, 0, selectorsBytes); + this.Frame.MultiScan = this.Frame.ComponentCount != selectorsCount; - for (int i = 0; i < selectorsCount; i++) + for (int i = 0; i < selectorsBytes; i += 2) { // 1 byte: Component id - int componentSelectorId = stream.ReadByte(); + int componentSelectorId = this.temp[i]; int componentIndex = -1; for (int j = 0; j < this.Frame.ComponentIds.Length; j++) @@ -1010,14 +1020,14 @@ namespace SixLabors.ImageSharp.Formats.Jpeg JpegThrowHelper.ThrowInvalidImageContentException($"Unknown component id in scan: {componentSelectorId}."); } - this.Frame.ComponentOrder[i] = (byte)componentIndex; + this.Frame.ComponentOrder[i / 2] = (byte)componentIndex; JpegComponent component = this.Frame.Components[componentIndex]; // 1 byte: Huffman table selectors. // 4 bits - dc // 4 bits - ac - int tableSpec = stream.ReadByte(); + int tableSpec = this.temp[i + 1]; int dcTableIndex = tableSpec >> 4; int acTableIndex = tableSpec & 15; From db04e41b88ba9c5ab6770c85ca4baf6363c38f9a Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 17:21:13 +0300 Subject: [PATCH 20/21] Added comments to major properties --- .../Jpeg/Components/Decoder/HuffmanScanDecoder.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs index ea76df7a8..70a446512 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs @@ -18,11 +18,19 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder { private readonly BufferedReadStream stream; - // Frame related + /// + /// instance containing decoding-related information. + /// private JpegFrame frame; + + /// + /// Shortcut for .Components. + /// private JpegComponent[] components; - // The number of interleaved components. + /// + /// Number of component in the current scan. + /// private int componentsCount; /// From 97500145b71a3cd2c302701c7f9c4ba20c24aac1 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Fri, 16 Jul 2021 17:34:59 +0300 Subject: [PATCH 21/21] Added todo --- src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index bafd8e215..77b1b44af 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -1034,6 +1034,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg // Validate: both must be < 4 if (dcTableIndex >= 4 || acTableIndex >= 4) { + // TODO: extract as separate method? JpegThrowHelper.ThrowInvalidImageContentException($"Invalid huffman table for component:{componentSelectorId}: dc={dcTableIndex}, ac={acTableIndex}"); }