refactored GrayImage class --to--> reusable JpegPixelArea cleaned some mess in JpegDecoderCore

src/ImageSharp/Formats/Jpg/Components/Decoder/GrayImage.cs

@@ -1,101 +0,0 @@
-// <copyright file="GrayImage.cs" company="James Jackson-South">
-// Copyright (c) James Jackson-South and contributors.
-// Licensed under the Apache License, Version 2.0.
-// </copyright>
-
-namespace ImageSharp.Formats.Jpg
-{
-    /// <summary>
-    /// Represents a grayscale image
-    /// </summary>
-    internal class GrayImage
-    {
-        /// <summary>
-        /// Initializes a new instance of the <see cref="GrayImage"/> class.
-        /// </summary>
-        /// <param name="width">The width.</param>
-        /// <param name="height">The height.</param>
-        public GrayImage(int width, int height)
-        {
-            this.Width = width;
-            this.Height = height;
-            this.Pixels = new byte[width * height];
-            this.Stride = width;
-            this.Offset = 0;
-        }
-
-        /// <summary>
-        /// Prevents a default instance of the <see cref="GrayImage"/> class from being created.
-        /// </summary>
-        private GrayImage()
-        {
-        }
-
-        /// <summary>
-        /// Gets or sets the pixels.
-        /// </summary>
-        public byte[] Pixels { get; set; }
-
-        /// <summary>
-        /// Gets or sets the stride.
-        /// </summary>
-        public int Stride { get; set; }
-
-        /// <summary>
-        /// Gets or sets the horizontal position.
-        /// </summary>
-        public int X { get; set; }
-
-        /// <summary>
-        /// Gets or sets the vertical position.
-        /// </summary>
-        public int Y { get; set; }
-
-        /// <summary>
-        /// Gets or sets the width.
-        /// </summary>
-        public int Width { get; set; }
-
-        /// <summary>
-        /// Gets or sets the height.
-        /// </summary>
-        public int Height { get; set; }
-
-        /// <summary>
-        /// Gets or sets the offset
-        /// </summary>
-        public int Offset { get; set; }
-
-        /// <summary>
-        /// Gets an image made up of a subset of the originals pixels.
-        /// </summary>
-        /// <param name="x">The x-coordinate of the image.</param>
-        /// <param name="y">The y-coordinate of the image.</param>
-        /// <param name="width">The width.</param>
-        /// <param name="height">The height.</param>
-        /// <returns>
-        /// The <see cref="GrayImage"/>.
-        /// </returns>
-        public GrayImage Subimage(int x, int y, int width, int height)
-        {
-            return new GrayImage
-            {
-                Width = width,
-                Height = height,
-                Pixels = this.Pixels,
-                Stride = this.Stride,
-                Offset = (y * this.Stride) + x
-            };
-        }
-
-        /// <summary>
-        /// Gets the row offset at the given position
-        /// </summary>
-        /// <param name="y">The y-coordinate of the image.</param>
-        /// <returns>The <see cref="int"/></returns>
-        public int GetRowOffset(int y)
-        {
-            return this.Offset + (y * this.Stride);
-        }
-    }
-}

src/ImageSharp/Formats/Jpg/Components/Decoder/JpegPixelArea.cs

@@ -0,0 +1,116 @@
+// <copyright file="JpegChannelArea.cs" company="James Jackson-South">
+// Copyright (c) James Jackson-South and contributors.
+// Licensed under the Apache License, Version 2.0.
+// </copyright>
+
+namespace ImageSharp.Formats.Jpg
+{
+    
+    using System;
+    using System.Buffers;
+    using System.Runtime.CompilerServices;
+
+    /// <summary>
+    /// Represents a grayscale image
+    /// </summary>
+    internal struct JpegPixelArea
+    {
+        /// <summary>
+        /// Initializes a new instance of the <see cref="JpegPixelArea" /> class.
+        /// </summary>
+        /// <param name="width">The width.</param>
+        /// <param name="height">The height.</param>
+        public static JpegPixelArea CreatePooled(int width, int height)
+        {
+            int size = width * height;
+            //var pixels = ArrayPool<byte>.Shared.Rent(size);
+            //Array.Clear(pixels, 0, size);
+            var pixels = ArrayPoolManager<byte>.RentCleanArray(size);
+            return new JpegPixelArea(pixels, width, 0);
+        }
+
+        public JpegPixelArea(byte[] pixels, int widthOrStride, int offset)
+        {
+            this.Stride = widthOrStride;
+            this.Pixels = pixels;
+            this.Offset = offset;
+        }
+
+        public void ReturnPooled()
+        {
+            if (this.Pixels == null) return;
+            ArrayPoolManager<byte>.ReturnArray(this.Pixels);
+            this.Pixels = null;
+        }
+
+        /// <summary>
+        /// Gets or sets the pixels.
+        /// </summary>
+        public byte[] Pixels { get; private set; }
+
+        public bool Created => this.Pixels != null;
+
+        /// <summary>
+        /// Gets or sets the width.
+        /// </summary>
+        public int Stride { get; private set; }
+
+        /// <summary>
+        /// Gets or sets the offset
+        /// </summary>
+        public int Offset { get; private set; }
+
+        /// <summary>
+        /// Gets an image made up of a subset of the originals pixels.
+        /// </summary>
+        /// <param name="x">The x-coordinate of the image.</param>
+        /// <param name="y">The y-coordinate of the image.</param>
+        /// <param name="width">The width.</param>
+        /// <param name="height">The height.</param>
+        /// <returns>
+        /// The <see cref="JpegPixelArea"/>.
+        /// </returns>
+        public JpegPixelArea Subimage(int x, int y, int width, int height)
+        {
+            return new JpegPixelArea
+            {
+                Stride = width,
+                Pixels = this.Pixels,
+                Offset = (y * this.Stride) + x
+            };
+        }
+
+        /// <summary>
+        /// Get the subarea that belongs to the given block indices
+        /// </summary>
+        /// <param name="bx">The block X index</param>
+        /// <param name="by">The block Y index</param>
+        /// <returns></returns>
+        public JpegPixelArea GetOffsetedAreaForBlock(int bx, int by)
+        {
+            int offset = this.Offset + 8 * (by * this.Stride + bx);
+            return new JpegPixelArea(this.Pixels, this.Stride, offset);
+        }
+
+        public byte this[int x, int y] => this.Pixels[y * this.Stride + x];
+
+        /// <summary>
+        /// Gets the row offset at the given position
+        /// </summary>
+        /// <param name="y">The y-coordinate of the image.</param>
+        /// <returns>The <see cref="int"/></returns>
+        public int GetRowOffset(int y)
+        {
+            return this.Offset + (y * this.Stride);
+        }
+
+        public MutableSpan<byte> Span => new MutableSpan<byte>(this.Pixels, this.Offset);
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public unsafe void LoadColorsFrom(Block8x8F* block, Block8x8F* temp)
+        {
+            // Level shift by +128, clip to [0, 255], and write to dst.
+            block->CopyColorsTo(new MutableSpan<byte>(this.Pixels, this.Offset), this.Stride, temp);
+        }
+    }
+}

src/ImageSharp/Formats/Jpg/Components/Decoder/YCbCrImage.cs

@@ -5,6 +5,8 @@
 
 namespace ImageSharp.Formats.Jpg
 {
+    using System.Buffers;
+
     /// <summary>
     /// Represents an image made up of three color components (luminance, blue chroma, red chroma)
     /// </summary>
@@ -20,25 +22,29 @@ namespace ImageSharp.Formats.Jpg
         {
             int cw, ch;
             YCbCrSize(width, height, ratio, out cw, out ch);
-            this.YChannel = new byte[width * height];
+            this.YPixels = new byte[width * height];
-            this.CbChannel = new byte[cw * ch];
+            this.CbPixels = new byte[cw * ch];
-            this.CrChannel = new byte[cw * ch];
+            this.CrPixels = new byte[cw * ch];
             this.Ratio = ratio;
             this.YStride = width;
             this.CStride = cw;
             this.X = 0;
             this.Y = 0;
-            this.Width = width;
-            this.Height = height;
         }
 
+        public JpegPixelArea YChannel => new JpegPixelArea(this.YPixels, this.YStride, this.YOffset);
+
+        public JpegPixelArea CbChannel => new JpegPixelArea(this.CbPixels, this.CStride, this.COffset); 
+
+        public JpegPixelArea CrChannel => new JpegPixelArea(this.CrPixels, this.CStride, this.COffset);
+
         /// <summary>
         /// Prevents a default instance of the <see cref="YCbCrImage"/> class from being created.
         /// </summary>
         private YCbCrImage()
         {
         }
-
+        
         /// <summary>
         /// Provides enumeration of the various available subsample ratios.
         /// </summary>
@@ -78,38 +84,38 @@ namespace ImageSharp.Formats.Jpg
         /// <summary>
         /// Gets or sets the luminance components channel.
         /// </summary>
-        public byte[] YChannel { get; set; }
+        public byte[] YPixels { get; private set; }
 
         /// <summary>
         /// Gets or sets the blue chroma components channel.
         /// </summary>
-        public byte[] CbChannel { get; set; }
+        public byte[] CbPixels { get; private set; }
 
         /// <summary>
         /// Gets or sets the red chroma components channel.
         /// </summary>
-        public byte[] CrChannel { get; set; }
+        public byte[] CrPixels { get; private set; }
 
         /// <summary>
         /// Gets or sets the Y slice index delta between vertically adjacent pixels.
         /// </summary>
-        public int YStride { get; set; }
+        public int YStride { get; private set; }
 
         /// <summary>
         /// Gets or sets the red and blue chroma slice index delta between vertically adjacent pixels
         /// that map to separate chroma samples.
         /// </summary>
-        public int CStride { get; set; }
+        public int CStride { get; private set; }
 
         /// <summary>
         /// Gets or sets the index of the first luminance element.
         /// </summary>
-        public int YOffset { get; set; }
+        public int YOffset { get; private set; }
 
         /// <summary>
         /// Gets or sets the index of the first element of red or blue chroma.
         /// </summary>
-        public int COffset { get; set; }
+        public int COffset { get; private set; }
 
         /// <summary>
         /// Gets or sets the horizontal position.
@@ -120,50 +126,38 @@ namespace ImageSharp.Formats.Jpg
         /// Gets or sets the vertical position.
         /// </summary>
         public int Y { get; set; }
-
+        
-        /// <summary>
-        /// Gets or sets the width.
-        /// </summary>
-        public int Width { get; set; }
-
-        /// <summary>
-        /// Gets or sets the height.
-        /// </summary>
-        public int Height { get; set; }
-
         /// <summary>
         /// Gets or sets the subsampling ratio.
         /// </summary>
         public YCbCrSubsampleRatio Ratio { get; set; }
 
-        /// <summary>
+        ///// <summary>
-        /// Gets an image made up of a subset of the originals pixels.
+        ///// Gets an image made up of a subset of the originals pixels.
-        /// </summary>
+        ///// </summary>
-        /// <param name="x">The x-coordinate of the image.</param>
+        ///// <param name="x">The x-coordinate of the image.</param>
-        /// <param name="y">The y-coordinate of the image.</param>
+        ///// <param name="y">The y-coordinate of the image.</param>
-        /// <param name="width">The width.</param>
+        ///// <param name="width">The width.</param>
-        /// <param name="height">The height.</param>
+        ///// <param name="height">The height.</param>
-        /// <returns>
+        ///// <returns>
-        /// The <see cref="YCbCrImage"/>.
+        ///// The <see cref="YCbCrImage"/>.
-        /// </returns>
+        ///// </returns>
-        public YCbCrImage Subimage(int x, int y, int width, int height)
+        //public YCbCrImage Subimage(int x, int y, int width, int height)
-        {
+        //{
-            YCbCrImage ret = new YCbCrImage
+        //    YCbCrImage ret = new YCbCrImage
-            {
+        //    {
-                Width = width,
+        //        YPixels = this.YPixels,
-                Height = height,
+        //        CbPixels = this.CbPixels,
-                YChannel = this.YChannel,
+        //        CrPixels = this.CrPixels,
-                CbChannel = this.CbChannel,
+        //        Ratio = this.Ratio,
-                CrChannel = this.CrChannel,
+        //        YStride = this.YStride,
-                Ratio = this.Ratio,
+        //        CStride = this.CStride,
-                YStride = this.YStride,
+        //        YOffset = (y * this.YStride) + x,
-                CStride = this.CStride,
+        //        COffset = (y * this.CStride) + x
-                YOffset = (y * this.YStride) + x,
+        //    };
-                COffset = (y * this.CStride) + x
+        //    return ret;
-            };
+        //}
-            return ret;
+        
-        }
-
         /// <summary>
         /// Returns the offset of the first luminance component at the given row
         /// </summary>

src/ImageSharp/Formats/Jpg/JpegDecoderCore.cs

@@ -122,22 +122,15 @@ namespace ImageSharp.Formats
         /// <summary>
         /// A grayscale image to decode to.
         /// </summary>
-        private GrayImage grayImage;
+        private JpegPixelArea grayImage;
 
         /// <summary>
         /// The full color image to decode to.
         /// </summary>
         private YCbCrImage ycbcrImage;
 
-        /// <summary>
+        
-        /// The array of keyline pixels in a CMYK image
+        private JpegPixelArea blackImage;
-        /// </summary>
-        private byte[] blackPixels;
-
-        /// <summary>
-        /// The width in bytes or a single row of keyline pixels in a CMYK image
-        /// </summary>
-        private int blackStride;
 
         /// <summary>
         /// The restart interval
@@ -420,7 +413,7 @@ namespace ImageSharp.Formats
                 }
             }
 
-            if (this.grayImage != null)
+            if (this.grayImage.Created)
             {
                 this.ConvertFromGrayScale(this.imageWidth, this.imageHeight, image);
             }
@@ -480,6 +473,8 @@ namespace ImageSharp.Formats
             }
 
             this.bytes.Dispose();
+            this.grayImage.ReturnPooled();
+            this.blackImage.ReturnPooled();
         }
 
         /// <summary>
@@ -1228,9 +1223,9 @@ namespace ImageSharp.Formats
 
                             for (int x = 0; x < width; x++)
                             {
-                                byte yy = this.ycbcrImage.YChannel[yo + x];
+                                byte yy = this.ycbcrImage.YPixels[yo + x];
-                                byte cb = this.ycbcrImage.CbChannel[co + (x / scale)];
+                                byte cb = this.ycbcrImage.CbPixels[co + (x / scale)];
-                                byte cr = this.ycbcrImage.CrChannel[co + (x / scale)];
+                                byte cr = this.ycbcrImage.CrPixels[co + (x / scale)];
 
                                 TColor packed = default(TColor);
                                 this.PackYcck<TColor>(ref packed, yy, cb, cr, x, y);
@@ -1268,9 +1263,9 @@ namespace ImageSharp.Formats
 
                         for (int x = 0; x < width; x++)
                         {
-                            byte cyan = this.ycbcrImage.YChannel[yo + x];
+                            byte cyan = this.ycbcrImage.YPixels[yo + x];
-                            byte magenta = this.ycbcrImage.CbChannel[co + (x / scale)];
+                            byte magenta = this.ycbcrImage.CbPixels[co + (x / scale)];
-                            byte yellow = this.ycbcrImage.CrChannel[co + (x / scale)];
+                            byte yellow = this.ycbcrImage.CrPixels[co + (x / scale)];
 
                             TColor packed = default(TColor);
                             this.PackCmyk<TColor>(ref packed, cyan, magenta, yellow, x, y);
@@ -1343,9 +1338,9 @@ namespace ImageSharp.Formats
 
                             for (int x = 0; x < width; x++)
                             {
-                                byte yy = this.ycbcrImage.YChannel[yo + x];
+                                byte yy = this.ycbcrImage.YPixels[yo + x];
-                                byte cb = this.ycbcrImage.CbChannel[co + (x / scale)];
+                                byte cb = this.ycbcrImage.CbPixels[co + (x / scale)];
-                                byte cr = this.ycbcrImage.CrChannel[co + (x / scale)];
+                                byte cr = this.ycbcrImage.CrPixels[co + (x / scale)];
 
                                 TColor packed = default(TColor);
                                 PackYcbCr<TColor>(ref packed, yy, cb, cr);
@@ -1383,9 +1378,9 @@ namespace ImageSharp.Formats
 
                             for (int x = 0; x < width; x++)
                             {
-                                byte red = this.ycbcrImage.YChannel[yo + x];
+                                byte red = this.ycbcrImage.YPixels[yo + x];
-                                byte green = this.ycbcrImage.CbChannel[co + (x / scale)];
+                                byte green = this.ycbcrImage.CbPixels[co + (x / scale)];
-                                byte blue = this.ycbcrImage.CrChannel[co + (x / scale)];
+                                byte blue = this.ycbcrImage.CrPixels[co + (x / scale)];
 
                                 TColor packed = default(TColor);
                                 packed.PackFromBytes(red, green, blue, 255);
@@ -1506,11 +1501,8 @@ namespace ImageSharp.Formats
             int v0 = this.componentArray[0].VerticalFactor;
             int mxx = (this.imageWidth + (8 * h0) - 1) / (8 * h0);
             int myy = (this.imageHeight + (8 * v0) - 1) / (8 * v0);
-
+            
-            if (this.grayImage == null && this.ycbcrImage == null)
+            this.MakeImage(mxx, myy);
-            {
-                this.MakeImage(mxx, myy);
-            }
 
             if (this.isProgressive)
             {
@@ -1540,6 +1532,8 @@ namespace ImageSharp.Formats
             // blocks: the third block in the first row has (bx, by) = (2, 0).
             int bx, by, blockCount = 0;
 
+            // TODO: A DecoderScanProcessor struct could clean up this mess
+
             Block8x8F b = default(Block8x8F);
             Block8x8F temp1 = default(Block8x8F);
             Block8x8F temp2 = default(Block8x8F);
@@ -1601,7 +1595,8 @@ namespace ImageSharp.Formats
 
                             int qtIndex = this.componentArray[compIndex].Selector;
 
-                            // TODO: Find a way to clean up this mess
+                            // TODO: A DecoderScanProcessor struct could clean up this mess
+                            // TODO: Reading & processing blocks should be done in 2 separate loops. The second one could be parallelized. (The first one could be async) 
                             fixed (Block8x8F* qtp = &this.quantizationTables[qtIndex])
                             {
                                 // Load the previous partially decoded coefficients, if applicable.
@@ -1812,52 +1807,33 @@ namespace ImageSharp.Formats
 
             DCT.TransformIDCT(ref *b, ref *temp1, ref *temp2);
 
-            byte[] dst;
+            var destChannel = this.GetDestinationChannel(compIndex);
-            int offset;
+            var destArea = destChannel.GetOffsetedAreaForBlock(bx, by);
-            int stride;
+            destArea.LoadColorsFrom(temp1, temp2);
+        }
 
+        private JpegPixelArea GetDestinationChannel(int compIndex)
+        {
             if (this.componentCount == 1)
             {
-                dst = this.grayImage.Pixels;
+                return this.grayImage;
-                stride = this.grayImage.Stride;
-                offset = this.grayImage.Offset + (8 * ((@by * this.grayImage.Stride) + bx));
             }
             else
             {
                 switch (compIndex)
                 {
                     case 0:
-                        dst = this.ycbcrImage.YChannel;
+                        return this.ycbcrImage.YChannel;
-                        stride = this.ycbcrImage.YStride;
-                        offset = this.ycbcrImage.YOffset + (8 * ((@by * this.ycbcrImage.YStride) + bx));
-                        break;
-
                     case 1:
-                        dst = this.ycbcrImage.CbChannel;
+                        return this.ycbcrImage.CbChannel;
-                        stride = this.ycbcrImage.CStride;
-                        offset = this.ycbcrImage.COffset + (8 * ((@by * this.ycbcrImage.CStride) + bx));
-                        break;
-
                     case 2:
-                        dst = this.ycbcrImage.CrChannel;
+                        return this.ycbcrImage.CrChannel;
-                        stride = this.ycbcrImage.CStride;
-                        offset = this.ycbcrImage.COffset + (8 * ((@by * this.ycbcrImage.CStride) + bx));
-                        break;
-
                     case 3:
-
+                        return this.blackImage;
-                        dst = this.blackPixels;
-                        stride = this.blackStride;
-                        offset = 8 * ((@by * this.blackStride) + bx);
-                        break;
-
                     default:
                         throw new ImageFormatException("Too many components");
                 }
             }
-
-            // Level shift by +128, clip to [0, 255], and write to dst.
-            temp1->CopyColorsTo(new MutableSpan<byte>(dst, offset), stride, temp2);
         }
 
         private void ProcessScanImpl(int i, ref Scan currentScan, Scan[] scan, ref int totalHv)
@@ -2071,7 +2047,7 @@ namespace ImageSharp.Formats
 
             return zig;
         }
-
+        
         /// <summary>
         /// Makes the image from the buffer.
         /// </summary>
@@ -2079,9 +2055,11 @@ namespace ImageSharp.Formats
         /// <param name="myy">The vertical MCU count</param>
         private void MakeImage(int mxx, int myy)
         {
+            if (this.grayImage.Created || this.ycbcrImage != null) return;
+            
             if (this.componentCount == 1)
             {
-                GrayImage gray = new GrayImage(8 * mxx, 8 * myy);
+                JpegPixelArea gray = JpegPixelArea.CreatePooled(8 * mxx, 8 * myy);
                 this.grayImage = gray.Subimage(0, 0, this.imageWidth, this.imageHeight);
             }
             else
@@ -2114,15 +2092,16 @@ namespace ImageSharp.Formats
                         break;
                 }
 
-                YCbCrImage ycbcr = new YCbCrImage(8 * h0 * mxx, 8 * v0 * myy, ratio);
+                this.ycbcrImage = new YCbCrImage(8 * h0 * mxx, 8 * v0 * myy, ratio);
-                this.ycbcrImage = ycbcr.Subimage(0, 0, this.imageWidth, this.imageHeight);
+                //this.ycbcrImage = ycbcr.Subimage(0, 0, this.imageWidth, this.imageHeight);
 
                 if (this.componentCount == 4)
                 {
                     int h3 = this.componentArray[3].HorizontalFactor;
                     int v3 = this.componentArray[3].VerticalFactor;
-                    this.blackPixels = new byte[8 * h3 * mxx * 8 * v3 * myy];
+
-                    this.blackStride = 8 * h3 * mxx;
+                    this.blackImage = JpegPixelArea.CreatePooled(8 * h3 * mxx, 8 * v3 * myy);
+                   
                 }
             }
         }
@@ -2179,7 +2158,7 @@ namespace ImageSharp.Formats
             float yellow = (y + (1.772F * ccb)).Clamp(0, 255) / 255F;
 
             // Get keyline
-            float keyline = (255 - this.blackPixels[(yy * this.blackStride) + xx]) / 255F;
+            float keyline = (255 - this.blackImage[xx, yy]) / 255F;
 
             // Convert back to RGB
             byte r = (byte)(((1 - cyan) * (1 - keyline)).Clamp(0, 1) * 255);
@@ -2204,7 +2183,7 @@ namespace ImageSharp.Formats
             where TColor : struct, IPackedPixel, IEquatable<TColor>
         {
             // Get keyline
-            float keyline = (255 - this.blackPixels[(yy * this.blackStride) + xx]) / 255F;
+            float keyline = (255 - this.blackImage[xx, yy]) / 255F;
 
             // Convert back to RGB. CMY are not inverted
             byte r = (byte)(((c / 255F) * (1F - keyline)).Clamp(0, 1) * 255);

src/ImageSharp/Formats/Jpg/Utils/ArrayPoolManager.cs

@@ -0,0 +1,13 @@
+namespace ImageSharp.Formats
+{
+    using System.Buffers;
+
+    internal class ArrayPoolManager<T>
+    {
+        private static readonly ArrayPool<T> Pool = ArrayPool<T>.Create();
+
+        public static T[] RentCleanArray(int minimumLength) => Pool.Rent(minimumLength);
+
+        public static void ReturnArray(T[] array) => Pool.Return(array, true);
+    }
+}

tests/ImageSharp.Tests/Formats/Jpg/JpegTests.cs

@@ -70,7 +70,7 @@ namespace ImageSharp.Tests
 
         private const PixelTypes BenchmarkPixels = PixelTypes.StandardImageClass; //PixelTypes.Color | PixelTypes.Argb;
 
-        [Theory] // Benchmark, enable manually
+        //[Theory] // Benchmark, enable manually
         [InlineData(TestImages.Jpeg.Cmyk)]
         [InlineData(TestImages.Jpeg.Ycck)]
         [InlineData(TestImages.Jpeg.Calliphora)]