From a8481c80fbd6b5dda642f95415ce6f76432010ef Mon Sep 17 00:00:00 2001 From: Mykhailo Matviiv Date: Fri, 5 May 2017 20:03:18 +0300 Subject: [PATCH 1/3] Remove DecodedBlockArray and replace usages with Buffer to centralize memory management. --- .../Components/Decoder/DecodedBlockArray.cs | 55 ------------------- .../Components/Decoder/JpegBlockProcessor.cs | 8 +-- .../Components/Decoder/JpegScanDecoder.cs | 6 +- .../Formats/Jpeg/JpegDecoderCore.cs | 12 ++-- 4 files changed, 13 insertions(+), 68 deletions(-) delete mode 100644 src/ImageSharp/Formats/Jpeg/Components/Decoder/DecodedBlockArray.cs diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/DecodedBlockArray.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/DecodedBlockArray.cs deleted file mode 100644 index 97a79dd61b..0000000000 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/DecodedBlockArray.cs +++ /dev/null @@ -1,55 +0,0 @@ -// -// Copyright (c) James Jackson-South and contributors. -// Licensed under the Apache License, Version 2.0. -// - -namespace ImageSharp.Formats.Jpg -{ - using System; - using System.Buffers; - - /// - /// Because has no information for rented arrays, - /// we need to store the count and the buffer separately when storing pooled arrays. - /// - internal struct DecodedBlockArray : IDisposable - { - /// - /// The used to pool data in . - /// Should always clean arrays when returning! - /// - private static readonly ArrayPool ArrayPool = ArrayPool.Create(); - - /// - /// Initializes a new instance of the struct. Rents a buffer. - /// - /// The number of valid -s - public DecodedBlockArray(int count) - { - this.Count = count; - this.Buffer = ArrayPool.Rent(count); - } - - /// - /// Gets the number of actual -s inside - /// - public int Count { get; } - - /// - /// Gets the rented buffer. - /// - public DecodedBlock[] Buffer { get; private set; } - - /// - /// Returns the rented buffer to the pool. - /// - public void Dispose() - { - if (this.Buffer != null) - { - ArrayPool.Return(this.Buffer, true); - this.Buffer = null; - } - } - } -} \ No newline at end of file diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBlockProcessor.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBlockProcessor.cs index 0acee3a10b..0ce233739d 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBlockProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBlockProcessor.cs @@ -9,7 +9,7 @@ namespace ImageSharp.Formats.Jpg using System.Runtime.InteropServices; /// - /// Encapsulates the implementation of processing "raw" -s into Jpeg image channels. + /// Encapsulates the implementation of processing "raw" -s into Jpeg image channels. /// [StructLayout(LayoutKind.Sequential)] internal unsafe struct JpegBlockProcessor @@ -47,10 +47,10 @@ namespace ImageSharp.Formats.Jpg /// The instance public void ProcessAllBlocks(JpegDecoderCore decoder) { - DecodedBlockArray blockArray = decoder.DecodedBlocks[this.componentIndex]; - for (int i = 0; i < blockArray.Count; i++) + Buffer blockArray = decoder.DecodedBlocks[this.componentIndex]; + for (int i = 0; i < blockArray.Length; i++) { - this.ProcessBlockColors(decoder, ref blockArray.Buffer[i]); + this.ProcessBlockColors(decoder, ref blockArray[i]); } } diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegScanDecoder.cs index c6362a8713..2a9b0c6b23 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegScanDecoder.cs @@ -171,7 +171,7 @@ namespace ImageSharp.Formats.Jpg // Take an existing block (required when progressive): int blockIndex = this.GetBlockIndex(decoder); - this.data.Block = decoder.DecodedBlocks[this.ComponentIndex].Buffer[blockIndex].Block; + this.data.Block = decoder.DecodedBlocks[this.ComponentIndex][blockIndex].Block; if (!decoder.InputProcessor.UnexpectedEndOfStreamReached) { @@ -179,8 +179,8 @@ namespace ImageSharp.Formats.Jpg } // Store the decoded block - DecodedBlockArray blocks = decoder.DecodedBlocks[this.ComponentIndex]; - blocks.Buffer[blockIndex].SaveBlock(this.bx, this.by, ref this.data.Block); + Buffer blocks = decoder.DecodedBlocks[this.ComponentIndex]; + blocks[blockIndex].SaveBlock(this.bx, this.by, ref this.data.Block); } // for j diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index 22c14ca4ea..da519a6ac1 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -111,7 +111,7 @@ namespace ImageSharp.Formats this.QuantizationTables = new Block8x8F[MaxTq + 1]; this.Temp = new byte[2 * Block8x8F.ScalarCount]; this.ComponentArray = new Component[MaxComponents]; - this.DecodedBlocks = new DecodedBlockArray[MaxComponents]; + this.DecodedBlocks = new Buffer[MaxComponents]; } /// @@ -125,12 +125,12 @@ namespace ImageSharp.Formats public HuffmanTree[] HuffmanTrees { get; } /// - /// Gets the array of -s storing the "raw" frequency-domain decoded blocks. + /// Gets the array of -s storing the "raw" frequency-domain decoded blocks. /// We need to apply IDCT, dequantiazition and unzigging to transform them into color-space blocks. /// This is done by . /// When ==true, we are touching these blocks multiple times - each time we process a Scan. /// - public DecodedBlockArray[] DecodedBlocks { get; } + public Buffer[] DecodedBlocks { get; } /// /// Gets the quantization tables, in zigzag order. @@ -216,9 +216,9 @@ namespace ImageSharp.Formats this.HuffmanTrees[i].Dispose(); } - foreach (DecodedBlockArray blockArray in this.DecodedBlocks) + foreach (Buffer blockArray in this.DecodedBlocks) { - blockArray.Dispose(); + blockArray?.Dispose(); } this.ycbcrImage?.Dispose(); @@ -1308,7 +1308,7 @@ namespace ImageSharp.Formats { int count = this.TotalMCUCount * this.ComponentArray[i].HorizontalFactor * this.ComponentArray[i].VerticalFactor; - this.DecodedBlocks[i] = new DecodedBlockArray(count); + this.DecodedBlocks[i] = Buffer.CreateClean(count); } } } From 0e1d40313a3422b2875d7b990bc1e41d92934b5d Mon Sep 17 00:00:00 2001 From: Mykhailo Matviiv Date: Sat, 6 May 2017 15:56:56 +0300 Subject: [PATCH 2/3] Refactor JpegPixelArea to delegate memory management to Buffer2D. --- .../Jpeg/Components/Decoder/JpegPixelArea.cs | 38 ++++++++----------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegPixelArea.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegPixelArea.cs index 728da8d02e..e46994c32d 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegPixelArea.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegPixelArea.cs @@ -4,7 +4,6 @@ // namespace ImageSharp.Formats.Jpg { - using System.Buffers; using System.Runtime.CompilerServices; /// @@ -15,20 +14,20 @@ namespace ImageSharp.Formats.Jpg /// /// Initializes a new instance of the struct from existing data. /// - /// The pixel array - /// The stride + /// The pixel buffer + /// The stride /// The offset - public JpegPixelArea(byte[] pixels, int striede, int offset) + public JpegPixelArea(Buffer2D pixels, int stride, int offset) { - this.Stride = striede; + this.Stride = stride; this.Pixels = pixels; this.Offset = offset; } /// - /// Gets the pixels. + /// Gets the pixels buffer. /// - public byte[] Pixels { get; private set; } + public Buffer2D Pixels { get; private set; } /// /// Gets a value indicating whether the instance has been initalized. (Is not default(JpegPixelArea)) @@ -36,21 +35,19 @@ namespace ImageSharp.Formats.Jpg public bool IsInitialized => this.Pixels != null; /// - /// Gets or the stride. + /// Gets the stride. /// public int Stride { get; } /// - /// Gets or the offset. + /// Gets the offset. /// public int Offset { get; } /// /// Gets a of bytes to the pixel area /// - public MutableSpan Span => new MutableSpan(this.Pixels, this.Offset); - - private static ArrayPool BytePool => ArrayPool.Shared; + public MutableSpan Span => new MutableSpan(this.Pixels.Array, this.Offset); /// /// Returns the pixel at (x, y) @@ -69,21 +66,16 @@ namespace ImageSharp.Formats.Jpg /// /// Creates a new instance of the struct. - /// Pixel array will be taken from a pool, this instance will be the owner of it's pixel data, therefore - /// should be called when the instance is no longer needed. + /// Pixel array will be handled by , but + /// can be called when the instance is no longer needed. /// /// The width. /// The height. /// A with pooled data - public static JpegPixelArea CreatePooled(int width, int height) - { - int size = width * height; - byte[] pixels = BytePool.Rent(size); - return new JpegPixelArea(pixels, width, 0); - } + public static JpegPixelArea CreatePooled(int width, int height) => new JpegPixelArea(Buffer2D.CreateClean(width, height), width, 0); /// - /// Returns to the pool + /// Dispose . /// public void ReturnPooled() { @@ -92,7 +84,7 @@ namespace ImageSharp.Formats.Jpg return; } - BytePool.Return(this.Pixels); + this.Pixels.Dispose(); this.Pixels = null; } @@ -129,7 +121,7 @@ namespace ImageSharp.Formats.Jpg public unsafe void LoadColorsFrom(Block8x8F* block, Block8x8F* temp) { // Level shift by +128, clip to [0, 255], and write to dst. - block->CopyColorsTo(new MutableSpan(this.Pixels, this.Offset), this.Stride, temp); + block->CopyColorsTo(new MutableSpan(this.Pixels.Array, this.Offset), this.Stride, temp); } } } \ No newline at end of file From ede5e84ae5683eb2725a512821e8c054c27b2d96 Mon Sep 17 00:00:00 2001 From: Mykhailo Matviiv Date: Sun, 7 May 2017 14:29:42 +0300 Subject: [PATCH 3/3] Get rid of CreatePooled and ReturnPooled methods in JpegPixelArea to separate memory management. --- .../Jpeg/Components/Decoder/JpegPixelArea.cs | 34 +++++----------- .../Jpeg/Components/Decoder/YCbCrImage.cs | 18 ++++----- .../Formats/Jpeg/JpegDecoderCore.cs | 40 ++++++++++--------- .../Formats/Jpg/YCbCrImageTests.cs | 6 +-- 4 files changed, 43 insertions(+), 55 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegPixelArea.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegPixelArea.cs index e46994c32d..920457a0cc 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegPixelArea.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegPixelArea.cs @@ -24,6 +24,16 @@ namespace ImageSharp.Formats.Jpg this.Offset = offset; } + /// + /// Initializes a new instance of the struct from existing buffer. + /// will be set to of and will be set to 0. + /// + /// The pixel buffer + public JpegPixelArea(Buffer2D pixels) + : this(pixels, pixels.Width, 0) + { + } + /// /// Gets the pixels buffer. /// @@ -64,30 +74,6 @@ namespace ImageSharp.Formats.Jpg } } - /// - /// Creates a new instance of the struct. - /// Pixel array will be handled by , but - /// can be called when the instance is no longer needed. - /// - /// The width. - /// The height. - /// A with pooled data - public static JpegPixelArea CreatePooled(int width, int height) => new JpegPixelArea(Buffer2D.CreateClean(width, height), width, 0); - - /// - /// Dispose . - /// - public void ReturnPooled() - { - if (this.Pixels == null) - { - return; - } - - this.Pixels.Dispose(); - this.Pixels = null; - } - /// /// Gets the subarea that belongs to the Block8x8 defined by block indices /// diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/YCbCrImage.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/YCbCrImage.cs index a5ca9796b6..86192318b5 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/YCbCrImage.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/YCbCrImage.cs @@ -17,17 +17,17 @@ namespace ImageSharp.Formats.Jpg /// /// Gets the luminance components channel as . /// - public JpegPixelArea YChannel; + public Buffer2D YChannel; /// /// Gets the blue chroma components channel as . /// - public JpegPixelArea CbChannel; + public Buffer2D CbChannel; /// /// Gets an offseted to the Cr channel /// - public JpegPixelArea CrChannel; + public Buffer2D CrChannel; #pragma warning restore SA1401 /// @@ -44,9 +44,9 @@ namespace ImageSharp.Formats.Jpg this.YStride = width; this.CStride = cSize.Width; - this.YChannel = JpegPixelArea.CreatePooled(width, height); - this.CbChannel = JpegPixelArea.CreatePooled(cSize.Width, cSize.Height); - this.CrChannel = JpegPixelArea.CreatePooled(cSize.Width, cSize.Height); + this.YChannel = Buffer2D.CreateClean(width, height); + this.CbChannel = Buffer2D.CreateClean(cSize.Width, cSize.Height); + this.CrChannel = Buffer2D.CreateClean(cSize.Width, cSize.Height); } /// @@ -106,9 +106,9 @@ namespace ImageSharp.Formats.Jpg /// public void Dispose() { - this.YChannel.ReturnPooled(); - this.CbChannel.ReturnPooled(); - this.CrChannel.ReturnPooled(); + this.YChannel.Dispose(); + this.CbChannel.Dispose(); + this.CrChannel.Dispose(); } /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index da519a6ac1..92607883dd 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -223,8 +223,8 @@ namespace ImageSharp.Formats this.ycbcrImage?.Dispose(); this.InputProcessor.Dispose(); - this.grayImage.ReturnPooled(); - this.blackImage.ReturnPooled(); + this.grayImage.Pixels?.Dispose(); + this.blackImage.Pixels?.Dispose(); } /// @@ -243,11 +243,11 @@ namespace ImageSharp.Formats switch (compIndex) { case 0: - return this.ycbcrImage.YChannel; + return new JpegPixelArea(this.ycbcrImage.YChannel); case 1: - return this.ycbcrImage.CbChannel; + return new JpegPixelArea(this.ycbcrImage.CbChannel); case 2: - return this.ycbcrImage.CrChannel; + return new JpegPixelArea(this.ycbcrImage.CrChannel); case 3: return this.blackImage; default: @@ -586,9 +586,9 @@ namespace ImageSharp.Formats for (int x = 0; x < image.Width; x++) { - byte cyan = this.ycbcrImage.YChannel.Pixels[yo + x]; - byte magenta = this.ycbcrImage.CbChannel.Pixels[co + (x / scale)]; - byte yellow = this.ycbcrImage.CrChannel.Pixels[co + (x / scale)]; + byte cyan = this.ycbcrImage.YChannel[yo + x]; + byte magenta = this.ycbcrImage.CbChannel[co + (x / scale)]; + byte yellow = this.ycbcrImage.CrChannel[co + (x / scale)]; TPixel packed = default(TPixel); this.PackCmyk(ref packed, cyan, magenta, yellow, x, y); @@ -655,9 +655,9 @@ namespace ImageSharp.Formats for (int x = 0; x < image.Width; x++) { - byte red = this.ycbcrImage.YChannel.Pixels[yo + x]; - byte green = this.ycbcrImage.CbChannel.Pixels[co + (x / scale)]; - byte blue = this.ycbcrImage.CrChannel.Pixels[co + (x / scale)]; + byte red = this.ycbcrImage.YChannel[yo + x]; + byte green = this.ycbcrImage.CbChannel[co + (x / scale)]; + byte blue = this.ycbcrImage.CrChannel[co + (x / scale)]; TPixel packed = default(TPixel); packed.PackFromBytes(red, green, blue, 255); @@ -687,9 +687,9 @@ namespace ImageSharp.Formats y => { // TODO. This Parallel loop doesn't give us the boost it should. - ref byte ycRef = ref this.ycbcrImage.YChannel.Pixels[0]; - ref byte cbRef = ref this.ycbcrImage.CbChannel.Pixels[0]; - ref byte crRef = ref this.ycbcrImage.CrChannel.Pixels[0]; + ref byte ycRef = ref this.ycbcrImage.YChannel[0]; + ref byte cbRef = ref this.ycbcrImage.CbChannel[0]; + ref byte crRef = ref this.ycbcrImage.CrChannel[0]; fixed (YCbCrToRgbTables* tables = &yCbCrToRgbTables) { // TODO: Simplify + optimize + share duplicate code across converter methods @@ -737,9 +737,9 @@ namespace ImageSharp.Formats for (int x = 0; x < image.Width; x++) { - byte yy = this.ycbcrImage.YChannel.Pixels[yo + x]; - byte cb = this.ycbcrImage.CbChannel.Pixels[co + (x / scale)]; - byte cr = this.ycbcrImage.CrChannel.Pixels[co + (x / scale)]; + byte yy = this.ycbcrImage.YChannel[yo + x]; + byte cb = this.ycbcrImage.CbChannel[co + (x / scale)]; + byte cr = this.ycbcrImage.CrChannel[co + (x / scale)]; TPixel packed = default(TPixel); this.PackYcck(ref packed, yy, cb, cr, x, y); @@ -787,7 +787,8 @@ namespace ImageSharp.Formats if (this.ComponentCount == 1) { - this.grayImage = JpegPixelArea.CreatePooled(8 * this.MCUCountX, 8 * this.MCUCountY); + Buffer2D buffer = Buffer2D.CreateClean(8 * this.MCUCountX, 8 * this.MCUCountY); + this.grayImage = new JpegPixelArea(buffer); } else { @@ -826,7 +827,8 @@ namespace ImageSharp.Formats int h3 = this.ComponentArray[3].HorizontalFactor; int v3 = this.ComponentArray[3].VerticalFactor; - this.blackImage = JpegPixelArea.CreatePooled(8 * h3 * this.MCUCountX, 8 * v3 * this.MCUCountY); + Buffer2D buffer = Buffer2D.CreateClean(8 * h3 * this.MCUCountX, 8 * v3 * this.MCUCountY); + this.blackImage = new JpegPixelArea(buffer); } } } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/YCbCrImageTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/YCbCrImageTests.cs index ee38f500b0..ba55665ca2 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/YCbCrImageTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/YCbCrImageTests.cs @@ -61,9 +61,9 @@ namespace ImageSharp.Tests //this.PrintChannel("Cb", img.CbChannel); //this.PrintChannel("Cr", img.CrChannel); - Assert.Equal(img.YChannel.Stride, 400); - Assert.Equal(img.CbChannel.Stride, 400 / expectedCStrideDiv); - Assert.Equal(img.CrChannel.Stride, 400 / expectedCStrideDiv); + Assert.Equal(img.YChannel.Width, 400); + Assert.Equal(img.CbChannel.Width, 400 / expectedCStrideDiv); + Assert.Equal(img.CrChannel.Width, 400 / expectedCStrideDiv); } }