Merge branch 'master' into bp/webpimprovements

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

@@ -0,0 +1,81 @@
+// Copyright (c) Six Labors.
+// Licensed under the Apache License, Version 2.0.
+
+using System.IO;
+using BenchmarkDotNet.Attributes;
+using SixLabors.ImageSharp.Formats.Jpeg;
+using SixLabors.ImageSharp.Tests;
+
+namespace SixLabors.ImageSharp.Benchmarks.Codecs.Jpeg
+{
+    public class DecodeJpeg
+    {
+        private JpegDecoder decoder;
+
+        private MemoryStream preloadedImageStream;
+
+        private void GenericSetup(string imageSubpath)
+        {
+            this.decoder = new JpegDecoder();
+            byte[] bytes = File.ReadAllBytes(Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, imageSubpath));
+            this.preloadedImageStream = new MemoryStream(bytes);
+        }
+
+        private void GenericBechmark()
+        {
+            this.preloadedImageStream.Position = 0;
+            using Image img = this.decoder.Decode(Configuration.Default, this.preloadedImageStream);
+        }
+
+        [GlobalSetup(Target = nameof(JpegBaselineInterleaved444))]
+        public void SetupBaselineInterleaved444() =>
+            this.GenericSetup(TestImages.Jpeg.Baseline.Winter444_Interleaved);
+
+        [GlobalSetup(Target = nameof(JpegBaselineInterleaved420))]
+        public void SetupBaselineInterleaved420() =>
+            this.GenericSetup(TestImages.Jpeg.Baseline.Hiyamugi);
+
+        [GlobalSetup(Target = nameof(JpegBaseline400))]
+        public void SetupBaselineSingleComponent() =>
+            this.GenericSetup(TestImages.Jpeg.Baseline.Jpeg400);
+
+        [GlobalSetup(Target = nameof(JpegProgressiveNonInterleaved420))]
+        public void SetupProgressiveNoninterleaved420() =>
+            this.GenericSetup(TestImages.Jpeg.Progressive.Winter420_NonInterleaved);
+
+        [GlobalCleanup]
+        public void Cleanup()
+        {
+            this.preloadedImageStream.Dispose();
+            this.preloadedImageStream = null;
+        }
+
+        [Benchmark(Description = "Baseline 4:4:4 Interleaved")]
+        public void JpegBaselineInterleaved444() => this.GenericBechmark();
+
+        [Benchmark(Description = "Baseline 4:2:0 Interleaved")]
+        public void JpegBaselineInterleaved420() => this.GenericBechmark();
+
+        [Benchmark(Description = "Baseline 4:0:0 (grayscale)")]
+        public void JpegBaseline400() => this.GenericBechmark();
+
+        [Benchmark(Description = "Progressive 4:2:0 Non-Interleaved")]
+        public void JpegProgressiveNonInterleaved420() => this.GenericBechmark();
+    }
+}
+
+
+/*
+BenchmarkDotNet=v0.13.0, OS=Windows 10.0.19042.1288 (20H2/October2020Update)
+Intel Core i7-6700K CPU 4.00GHz (Skylake), 1 CPU, 8 logical and 4 physical cores
+.NET SDK=6.0.100-preview.3.21202.5
+  [Host]     : .NET Core 3.1.18 (CoreCLR 4.700.21.35901, CoreFX 4.700.21.36305), X64 RyuJIT
+  DefaultJob : .NET Core 3.1.18 (CoreCLR 4.700.21.35901, CoreFX 4.700.21.36305), X64 RyuJIT
+
+|                              Method |      Mean |     Error |    StdDev |
+|------------------------------------ |----------:|----------:|----------:|
+|        '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,

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

@@ -56,7 +56,7 @@ public partial class JpegDecoderTests
             { TestImages.Jpeg.Progressive.Fb, 75 },
             { TestImages.Jpeg.Issues.IncorrectQuality845, 98 },
             { TestImages.Jpeg.Baseline.ForestBridgeDifferentComponentsQuality, 89 },
-            { TestImages.Jpeg.Progressive.Winter, 80 }
+            { TestImages.Jpeg.Progressive.Winter420_NonInterleaved, 80 }
         };
 
         [Theory]

tests/ImageSharp.Tests/TestImages.cs

@@ -163,7 +163,7 @@ public static class Progressive
                 public const string Fb = "Jpg/progressive/fb.jpg";
                 public const string Progress = "Jpg/progressive/progress.jpg";
                 public const string Festzug = "Jpg/progressive/Festzug.jpg";
-                public const string Winter = "Jpg/progressive/winter.jpg";
+                public const string Winter420_NonInterleaved = "Jpg/progressive/winter420_noninterleaved.jpg";
 
                 public static class Bad
                 {
@@ -213,6 +213,7 @@ public static class Bad
                 public const string ArithmeticCoding = "Jpg/baseline/arithmetic_coding.jpg";
                 public const string ArithmeticCodingProgressive = "Jpg/progressive/arithmetic_progressive.jpg";
                 public const string Lossless = "Jpg/baseline/lossless.jpg";
+                public const string Winter444_Interleaved = "Jpg/baseline/winter444_interleaved.jpg";
 
                 public static readonly string[] All =
                 {