From 8538db75e414f494642057e161571fc14c8348b3 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 13 Aug 2020 16:57:48 +0200 Subject: [PATCH] Fix some issues: - Fix wrong palette code bits init in CalculateBestCacheSize - Fix wrong slice start index in PrepareMapToPalette - PixOrCopy: Change len from short to ushort --- .../WebP/Lossless/BackwardReferenceEncoder.cs | 26 +++++++-------- .../Formats/WebP/Lossless/HuffmanUtils.cs | 3 +- .../Formats/WebP/Lossless/PixOrCopy.cs | 6 ++-- .../Formats/WebP/Lossless/Vp8LEncoder.cs | 33 ++++++++++++------- .../Formats/WebP/Lossless/Vp8LHistogram.cs | 2 +- 5 files changed, 39 insertions(+), 31 deletions(-) diff --git a/src/ImageSharp/Formats/WebP/Lossless/BackwardReferenceEncoder.cs b/src/ImageSharp/Formats/WebP/Lossless/BackwardReferenceEncoder.cs index d509372588..a0ade54fa6 100644 --- a/src/ImageSharp/Formats/WebP/Lossless/BackwardReferenceEncoder.cs +++ b/src/ImageSharp/Formats/WebP/Lossless/BackwardReferenceEncoder.cs @@ -230,12 +230,10 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless } /// - /// Evaluates best possible backward references for specified quality. - /// The input cacheBits to 'GetBackwardReferences' sets the maximum cache - /// bits to use (passing 0 implies disabling the local color cache). + /// Evaluates best possible backward references for specified quality. The input cacheBits to 'GetBackwardReferences' + /// sets the maximum cache bits to use (passing 0 implies disabling the local color cache). /// The optimal cache bits is evaluated and set for the cacheBits parameter. - /// The return value is the pointer to the best of the two backward refs viz, - /// refs[0] or refs[1]. + /// The return value is the pointer to the best of the two backward refs viz, refs[0] or refs[1]. /// public static Vp8LBackwardRefs GetBackwardReferences( int width, @@ -335,9 +333,9 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless int pos = 0; var colorCache = new ColorCache[WebPConstants.MaxColorCacheBits + 1]; var histos = new Vp8LHistogram[WebPConstants.MaxColorCacheBits + 1]; - for (int i = 0; i < WebPConstants.MaxColorCacheBits + 1; i++) + for (int i = 0; i <= WebPConstants.MaxColorCacheBits; i++) { - histos[i] = new Vp8LHistogram(bestCacheBits); + histos[i] = new Vp8LHistogram(paletteCodeBits: i); colorCache[i] = new ColorCache(); colorCache[i].Init(i); } @@ -369,7 +367,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless { if (colorCache[i].Lookup(key) == pix) { - ++histos[i].Literal[WebPConstants.NumLiteralCodes + WebPConstants.CodeLengthCodes + key]; + ++histos[i].Literal[WebPConstants.NumLiteralCodes + WebPConstants.NumLengthCodes + key]; } else { @@ -563,7 +561,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless if (len != 1) { int offset = hashChain.FindOffset(i); - backwardRefs.Add(PixOrCopy.CreateCopy((uint)offset, (short)len)); + backwardRefs.Add(PixOrCopy.CreateCopy((uint)offset, (ushort)len)); if (useColorCache) { @@ -689,7 +687,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless } else { - refs.Add(PixOrCopy.CreateCopy((uint)offset, (short)len)); + refs.Add(PixOrCopy.CreateCopy((uint)offset, (ushort)len)); if (useColorCache) { for (j = i; j < i + len; ++j) @@ -908,7 +906,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless int prevRowLen = (i < xSize) ? 0 : FindMatchLength(bgra.Slice(i), bgra.Slice(i - xSize), 0, maxLen); if (rleLen >= prevRowLen && rleLen >= MinLength) { - refs.Add(PixOrCopy.CreateCopy(1, (short)rleLen)); + refs.Add(PixOrCopy.CreateCopy(1, (ushort)rleLen)); // We don't need to update the color cache here since it is always the // same pixel being copied, and that does not change the color cache state. @@ -916,7 +914,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless } else if (prevRowLen >= MinLength) { - refs.Add(PixOrCopy.CreateCopy((uint)xSize, (short)prevRowLen)); + refs.Add(PixOrCopy.CreateCopy((uint)xSize, (ushort)prevRowLen)); if (useColorCache) { for (int k = 0; k < prevRowLen; k++) @@ -936,7 +934,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless if (useColorCache) { - // TODO: VP8LColorCacheClear(); + // TODO: VP8LColorCacheClear()? } } @@ -978,7 +976,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless } } - // TODO: VP8LColorCacheClear(colorCache); + // TODO: VP8LColorCacheClear(colorCache)? } private static void BackwardReferences2DLocality(int xSize, Vp8LBackwardRefs refs) diff --git a/src/ImageSharp/Formats/WebP/Lossless/HuffmanUtils.cs b/src/ImageSharp/Formats/WebP/Lossless/HuffmanUtils.cs index db6b2e8f9d..414c607ad1 100644 --- a/src/ImageSharp/Formats/WebP/Lossless/HuffmanUtils.cs +++ b/src/ImageSharp/Formats/WebP/Lossless/HuffmanUtils.cs @@ -279,13 +279,12 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless { int value = tree.CodeLengths[i]; int k = i + 1; - int runs; while (k < depthSize && tree.CodeLengths[k] == value) { k++; } - runs = k - i; + var runs = k - i; if (value == 0) { tokenPos += CodeRepeatedZeros(runs, tokensArray.AsSpan(tokenPos)); diff --git a/src/ImageSharp/Formats/WebP/Lossless/PixOrCopy.cs b/src/ImageSharp/Formats/WebP/Lossless/PixOrCopy.cs index b8378d6cd6..8974092e67 100644 --- a/src/ImageSharp/Formats/WebP/Lossless/PixOrCopy.cs +++ b/src/ImageSharp/Formats/WebP/Lossless/PixOrCopy.cs @@ -10,7 +10,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless { public PixOrCopyMode Mode { get; set; } - public short Len { get; set; } + public ushort Len { get; set; } public uint BgraOrDistance { get; set; } @@ -38,7 +38,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless return retval; } - public static PixOrCopy CreateCopy(uint distance, short len) + public static PixOrCopy CreateCopy(uint distance, ushort len) { var retval = new PixOrCopy() { @@ -60,7 +60,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless return this.BgraOrDistance; } - public short Length() + public ushort Length() { return this.Len; } diff --git a/src/ImageSharp/Formats/WebP/Lossless/Vp8LEncoder.cs b/src/ImageSharp/Formats/WebP/Lossless/Vp8LEncoder.cs index 1f72f38078..9df6591f3a 100644 --- a/src/ImageSharp/Formats/WebP/Lossless/Vp8LEncoder.cs +++ b/src/ImageSharp/Formats/WebP/Lossless/Vp8LEncoder.cs @@ -427,7 +427,16 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless foreach (CrunchSubConfig subConfig in config.SubConfigs) { - Vp8LBackwardRefs refsBest = BackwardReferenceEncoder.GetBackwardReferences(width, height, bgra, quality, subConfig.Lz77, ref cacheBits, hashChain, refsArray[0], refsArray[1]); // TODO : Pass do not cache + Vp8LBackwardRefs refsBest = BackwardReferenceEncoder.GetBackwardReferences( + width, + height, + bgra, + quality, + subConfig.Lz77, + ref cacheBits, + hashChain, + refsArray[0], + refsArray[1]); // TODO : Pass do not cache // Keep the best references aside and use the other element from the first // two as a temporary for later usage. @@ -473,13 +482,13 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless this.bitWriter.PutBits((uint)(writeHistogramImage ? 1 : 0), 1); if (writeHistogramImage) { - using IMemoryOwner histogramArgbBuffer = this.memoryAllocator.Allocate(histogramImageXySize); - Span histogramArgb = histogramArgbBuffer.GetSpan(); + using IMemoryOwner histogramBgraBuffer = this.memoryAllocator.Allocate(histogramImageXySize); + Span histogramBgra = histogramBgraBuffer.GetSpan(); int maxIndex = 0; for (int i = 0; i < histogramImageXySize; i++) { int symbolIndex = histogramSymbols[i] & 0xffff; - histogramArgb[i] = (uint)(symbolIndex << 8); + histogramBgra[i] = (uint)(symbolIndex << 8); if (symbolIndex >= maxIndex) { maxIndex = symbolIndex + 1; @@ -487,7 +496,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless } this.bitWriter.PutBits((uint)(histogramBits - 2), 3); - this.EncodeImageNoHuffman(histogramArgb, hashChain, refsTmp, refsArray[2], LosslessUtils.SubSampleSize(width, histogramBits), LosslessUtils.SubSampleSize(height, histogramBits), quality); + this.EncodeImageNoHuffman(histogramBgra, hashChain, refsTmp, refsArray[2], LosslessUtils.SubSampleSize(width, histogramBits), LosslessUtils.SubSampleSize(height, histogramBits), quality); } // Store Huffman codes. @@ -690,7 +699,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless if (count == 0) { - // emit minimal tree for empty cases + // Emit minimal tree for empty cases. // bits: small tree marker: 1, count-1: 0, large 8-bit code: 0, code: 0 this.bitWriter.PutBits(0x01, 4); } @@ -725,10 +734,12 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless int i; var codeLengthBitDepth = new byte[WebPConstants.CodeLengthCodes]; var codeLengthBitDepthSymbols = new short[WebPConstants.CodeLengthCodes]; - var huffmanCode = new HuffmanTreeCode(); - huffmanCode.NumSymbols = WebPConstants.CodeLengthCodes; - huffmanCode.CodeLengths = codeLengthBitDepth; - huffmanCode.Codes = codeLengthBitDepthSymbols; + var huffmanCode = new HuffmanTreeCode + { + NumSymbols = WebPConstants.CodeLengthCodes, + CodeLengths = codeLengthBitDepth, + Codes = codeLengthBitDepthSymbols + }; this.bitWriter.PutBits(0, 1); var numTokens = HuffmanUtils.CreateCompressedHuffmanTree(tree, tokens); @@ -1313,7 +1324,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless /// private static void PrepareMapToPalette(Span palette, int numColors, uint[] sorted, uint[] idxMap) { - palette.Slice(numColors).CopyTo(sorted); + palette.Slice(0, numColors).CopyTo(sorted); Array.Sort(sorted, PaletteCompareColorsForSort); for (int i = 0; i < numColors; i++) { diff --git a/src/ImageSharp/Formats/WebP/Lossless/Vp8LHistogram.cs b/src/ImageSharp/Formats/WebP/Lossless/Vp8LHistogram.cs index 6fe3496a4d..78420185bc 100644 --- a/src/ImageSharp/Formats/WebP/Lossless/Vp8LHistogram.cs +++ b/src/ImageSharp/Formats/WebP/Lossless/Vp8LHistogram.cs @@ -61,7 +61,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless this.Alpha = new uint[WebPConstants.NumLiteralCodes + 1]; this.Distance = new uint[WebPConstants.NumDistanceCodes]; - var literalSize = WebPConstants.NumLiteralCodes + WebPConstants.NumLengthCodes + ((this.PaletteCodeBits > 0) ? (1 << this.PaletteCodeBits) : 0); + var literalSize = WebPConstants.NumLiteralCodes + WebPConstants.NumLengthCodes + (1 << WebPConstants.MaxColorCacheBits); this.Literal = new uint[literalSize]; // 5 for literal, red, blue, alpha, distance.