Merge pull request #1845 from br3aker/jpeg-decoder-size-fix

Jpeg decoder mcu size fix

Author: JimBobSquarePants

src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs

@@ -151,6 +151,7 @@ private void ParseBaselineData()
             if (this.componentsCount == this.frame.ComponentCount)
             {
                 this.ParseBaselineDataInterleaved();
+                this.spectralConverter.CommitConversion();
             }
             else
             {

src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter.cs

@@ -13,6 +13,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder
     /// </remarks>
     internal abstract class SpectralConverter
     {
+        /// <summary>
+        /// Gets a value indicating whether this converter has converted spectral
+        /// data of the current image or not.
+        /// </summary>
+        protected bool Converted { get; private set; }
+
         /// <summary>
         /// Injects jpeg image decoding metadata.
         /// </summary>
@@ -33,6 +39,19 @@ internal abstract class SpectralConverter
         /// </remarks>
         public abstract void ConvertStrideBaseline();
 
+        /// <summary>
+        /// Marks current converter state as 'converted'.
+        /// </summary>
+        /// <remarks>
+        /// This must be called only for baseline interleaved jpeg's.
+        /// </remarks>
+        public void CommitConversion()
+        {
+            DebugGuard.IsFalse(this.Converted, nameof(this.Converted), $"{nameof(this.CommitConversion)} must be called only once");
+
+            this.Converted = true;
+        }
+
         /// <summary>
         /// Gets the color converter.
         /// </summary>

src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Buffers;
+using System.Linq;
 using System.Numerics;
 using System.Threading;
 using SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder.ColorConverters;
@@ -29,8 +30,6 @@ internal class SpectralConverter<TPixel> : SpectralConverter, IDisposable
 
         private Buffer2D<TPixel> pixelBuffer;
 
-        private int blockRowsPerStep;
-
         private int pixelRowsPerStep;
 
         private int pixelRowCounter;
@@ -41,8 +40,6 @@ public SpectralConverter(Configuration configuration, CancellationToken cancella
             this.cancellationToken = cancellationToken;
         }
 
-        private bool Converted => this.pixelRowCounter >= this.pixelBuffer.Height;
-
         public Buffer2D<TPixel> GetPixelBuffer()
         {
             if (!this.Converted)
@@ -52,7 +49,7 @@ public Buffer2D<TPixel> GetPixelBuffer()
                 for (int step = 0; step < steps; step++)
                 {
                     this.cancellationToken.ThrowIfCancellationRequested();
-                    this.ConvertNextStride(step);
+                    this.ConvertStride(step);
                 }
             }
 
@@ -65,26 +62,26 @@ public override void InjectFrameData(JpegFrame frame, IRawJpegData jpegData)
             MemoryAllocator allocator = this.configuration.MemoryAllocator;
 
             // iteration data
-            IJpegComponent c0 = frame.Components[0];
+            int majorBlockWidth = frame.Components.Max((component) => component.SizeInBlocks.Width);
+            int majorVerticalSamplingFactor = frame.Components.Max((component) => component.SamplingFactors.Height);
 
             const int blockPixelHeight = 8;
-            this.blockRowsPerStep = c0.SamplingFactors.Height;
-            this.pixelRowsPerStep = this.blockRowsPerStep * blockPixelHeight;
+            this.pixelRowsPerStep = majorVerticalSamplingFactor * blockPixelHeight;
 
             // pixel buffer for resulting image
             this.pixelBuffer = allocator.Allocate2D<TPixel>(frame.PixelWidth, frame.PixelHeight);
             this.paddedProxyPixelRow = allocator.Allocate<TPixel>(frame.PixelWidth + 3);
 
             // component processors from spectral to Rgba32
-            var postProcessorBufferSize = new Size(c0.SizeInBlocks.Width * 8, this.pixelRowsPerStep);
+            const int blockPixelWidth = 8;
+            var postProcessorBufferSize = new Size(majorBlockWidth * blockPixelWidth, this.pixelRowsPerStep);
             this.componentProcessors = new JpegComponentPostProcessor[frame.Components.Length];
             for (int i = 0; i < this.componentProcessors.Length; i++)
             {
                 this.componentProcessors[i] = new JpegComponentPostProcessor(allocator, frame, jpegData, postProcessorBufferSize, frame.Components[i]);
             }
 
             // single 'stride' rgba32 buffer for conversion between spectral and TPixel
-            // this.rgbaBuffer = allocator.Allocate<Vector4>(frame.PixelWidth);
             this.rgbBuffer = allocator.Allocate<byte>(frame.PixelWidth * 3);
 
             // color converter from Rgba32 to TPixel
@@ -95,18 +92,17 @@ public override void InjectFrameData(JpegFrame frame, IRawJpegData jpegData)
         public override void ConvertStrideBaseline()
         {
             // Convert next pixel stride using single spectral `stride'
-            // Note that zero passing eliminates the need of virtual call from JpegComponentPostProcessor
-            this.ConvertNextStride(spectralStep: 0);
+            // Note that zero passing eliminates the need of virtual call
+            // from JpegComponentPostProcessor
+            this.ConvertStride(spectralStep: 0);
 
-            // Clear spectral stride - this is VERY important as jpeg possibly won't fill entire buffer each stride
-            // Which leads to decoding artifacts
-            // Note that this code clears all buffers of the post processors, it's their responsibility to allocate only single stride
             foreach (JpegComponentPostProcessor cpp in this.componentProcessors)
             {
                 cpp.ClearSpectralBuffers();
             }
         }
 
+        /// <inheritdoc/>
         public void Dispose()
         {
             if (this.componentProcessors != null)
@@ -121,7 +117,7 @@ public void Dispose()
             this.paddedProxyPixelRow?.Dispose();
         }
 
-        private void ConvertNextStride(int spectralStep)
+        private void ConvertStride(int spectralStep)
         {
             int maxY = Math.Min(this.pixelBuffer.Height, this.pixelRowCounter + this.pixelRowsPerStep);
 

tests/ImageSharp.Benchmarks/Codecs/Jpeg/DecodeJpeg.cs

@@ -74,8 +74,8 @@ public void Cleanup()
 
 |                              Method |      Mean |     Error |    StdDev |
 |------------------------------------ |----------:|----------:|----------:|
-|        'Baseline 4:4:4 Interleaved' | 12.710 ms | 0.1120 ms | 0.0990 ms |
-|        'Baseline 4:2:0 Interleaved' |  8.855 ms | 0.1447 ms | 0.1353 ms |
-|        'Baseline 4:0:0 (grayscale)' |  1.660 ms | 0.0106 ms | 0.0088 ms |
-| 'Progressive 4:2:0 Non-Interleaved' | 14.138 ms | 0.2797 ms | 0.3330 ms |
+|        'Baseline 4:4:4 Interleaved' | 11.781 ms | 0.0737 ms | 0.0654 ms |
+|        'Baseline 4:2:0 Interleaved' |  8.688 ms | 0.0345 ms | 0.0306 ms |
+|        'Baseline 4:0:0 (grayscale)' |  1.643 ms | 0.0092 ms | 0.0086 ms |
+| 'Progressive 4:2:0 Non-Interleaved' | 13.770 ms | 0.0928 ms | 0.0823 ms |
 */

tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs

@@ -20,6 +20,7 @@ public partial class JpegDecoderTests
                 TestImages.Jpeg.Baseline.Jpeg420Small,
                 TestImages.Jpeg.Issues.Fuzz.AccessViolationException922,
                 TestImages.Jpeg.Baseline.Jpeg444,
+                TestImages.Jpeg.Baseline.Jpeg422,
                 TestImages.Jpeg.Baseline.Bad.BadEOF,
                 TestImages.Jpeg.Baseline.MultiScanBaselineCMYK,
                 TestImages.Jpeg.Baseline.YcckSubsample1222,
@@ -100,6 +101,7 @@ public partial class JpegDecoderTests
                 [TestImages.Jpeg.Baseline.Bad.BadEOF] = 0.38f / 100,
                 [TestImages.Jpeg.Baseline.Bad.BadRST] = 0.0589f / 100,
 
+                [TestImages.Jpeg.Baseline.Jpeg422] = 0.0013f / 100,
                 [TestImages.Jpeg.Baseline.Testorig420] = 0.38f / 100,
                 [TestImages.Jpeg.Baseline.Jpeg420Small] = 0.287f / 100,
                 [TestImages.Jpeg.Baseline.Turtle420] = 1.0f / 100,