From 91ff2ada2e39b8cf6f9771a5fbac5907a08a91d2 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Mon, 27 Jul 2020 11:32:33 +0200 Subject: [PATCH] Some bug fixes: - Distance was not copied in histogram DeepClone - Do not remove entries from the histogram: instead set them to null like the original code - Fix invalid upper bound in for loop of HistogramRemap --- .../Formats/WebP/Lossless/HistogramEncoder.cs | 62 ++++++++++++------- .../Formats/WebP/Lossless/Vp8LEncoder.cs | 18 +++--- .../Formats/WebP/Lossless/Vp8LHistogram.cs | 1 + 3 files changed, 49 insertions(+), 32 deletions(-) diff --git a/src/ImageSharp/Formats/WebP/Lossless/HistogramEncoder.cs b/src/ImageSharp/Formats/WebP/Lossless/HistogramEncoder.cs index 203f254b72..6366beadc7 100644 --- a/src/ImageSharp/Formats/WebP/Lossless/HistogramEncoder.cs +++ b/src/ImageSharp/Formats/WebP/Lossless/HistogramEncoder.cs @@ -26,6 +26,8 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless private const uint NonTrivialSym = 0xffffffff; + private const short InvalidHistogramSymbol = Int16.MaxValue; + public static void GetHistoImageSymbols(int xSize, int ySize, Vp8LBackwardRefs refs, int quality, int histoBits, int cacheBits, List imageHisto, Vp8LHistogram tmpHisto, short[] histogramSymbols) { int histoXSize = histoBits > 0 ? LosslessUtils.SubSampleSize(xSize, histoBits) : 1; @@ -45,7 +47,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless HistogramBuild(xSize, histoBits, refs, origHisto); // Copies the histograms and computes its bitCost. histogramSymbols is optimized. - HistogramCopyAndAnalyze(origHisto, imageHisto, ref numUsed, histogramSymbols); + HistogramCopyAndAnalyze(origHisto, imageHisto, histogramSymbols); var entropyCombine = (numUsed > entropyCombineNumBins * 2) && (quality < 100); if (entropyCombine) @@ -56,9 +58,9 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless HistogramAnalyzeEntropyBin(imageHisto, binMap); // Collapse histograms with similar entropy. - HistogramCombineEntropyBin(imageHisto, ref numUsed, histogramSymbols, clusterMappings, tmpHisto, binMap, entropyCombineNumBins, combineCostFactor); + HistogramCombineEntropyBin(imageHisto, histogramSymbols, clusterMappings, tmpHisto, binMap, entropyCombineNumBins, combineCostFactor); - OptimizeHistogramSymbols(imageHisto, clusterMappings, numClusters, mapTmp, histogramSymbols); + OptimizeHistogramSymbols(clusterMappings, numClusters, mapTmp, histogramSymbols); } float x = quality / 100.0f; @@ -131,6 +133,11 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless // Analyze the dominant (literal, red and blue) entropy costs. for (int i = 0; i < histoSize; i++) { + if (histograms[i] == null) + { + continue; + } + costRange.UpdateDominantCostRange(histograms[i]); } @@ -138,40 +145,38 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless // symbol costs and store the resulting bin_id for each histogram. for (int i = 0; i < histoSize; i++) { + if (histograms[i] == null) + { + continue; + } + binMap[i] = (short)costRange.GetHistoBinIndex(histograms[i], NumPartitions); } } - private static void HistogramCopyAndAnalyze(List origHistograms, List histograms, ref int numUsed, short[] histogramSymbols) + private static void HistogramCopyAndAnalyze(List origHistograms, List histograms, short[] histogramSymbols) { - int numUsedOrig = numUsed; - var indicesToRemove = new List(); for (int clusterId = 0, i = 0; i < origHistograms.Count; i++) { - Vp8LHistogram histo = origHistograms[i]; - histo.UpdateHistogramCost(); + Vp8LHistogram origHistogram = origHistograms[i]; + origHistogram.UpdateHistogramCost(); - // Skip the histogram if it is completely empty, which can happen for tiles - // with no information (when they are skipped because of LZ77). - if (!histo.IsUsed[0] && !histo.IsUsed[1] && !histo.IsUsed[2] && !histo.IsUsed[3] && !histo.IsUsed[4]) + // Skip the histogram if it is completely empty, which can happen for tiles with no information (when they are skipped because of LZ77). + if (!origHistogram.IsUsed[0] && !origHistogram.IsUsed[1] && !origHistogram.IsUsed[2] && !origHistogram.IsUsed[3] && !origHistogram.IsUsed[4]) { - indicesToRemove.Add(i); + origHistograms[i] = null; + histograms[i] = null; + histogramSymbols[i] = InvalidHistogramSymbol; } else { - histograms[i] = (Vp8LHistogram)histo.DeepClone(); + histograms[i] = (Vp8LHistogram)origHistogram.DeepClone(); histogramSymbols[i] = (short)clusterId++; } } - - foreach (int index in indicesToRemove.OrderByDescending(v => v)) - { - origHistograms.RemoveAt(index); - histograms.RemoveAt(index); - } } - private static void HistogramCombineEntropyBin(List histograms, ref int numUsed, short[] clusters, short[] clusterMappings, Vp8LHistogram curCombo, short[] binMap, int numBins, double combineCostFactor) + private static void HistogramCombineEntropyBin(List histograms, short[] clusters, short[] clusterMappings, Vp8LHistogram curCombo, short[] binMap, int numBins, double combineCostFactor) { var binInfo = new HistogramBinInfo[BinSize]; for (int idx = 0; idx < numBins; idx++) @@ -245,7 +250,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless /// Given a Histogram set, the mapping of clusters 'clusterMapping' and the /// current assignment of the cells in 'symbols', merge the clusters and assign the smallest possible clusters values. /// - private static void OptimizeHistogramSymbols(List histograms, short[] clusterMappings, int numClusters, short[] clusterMappingsTmp, short[] symbols) + private static void OptimizeHistogramSymbols(short[] clusterMappings, int numClusters, short[] clusterMappingsTmp, short[] symbols) { bool doContinue = true; @@ -278,6 +283,11 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless // Re-map the ids. for (int i = 0; i < symbols.Length; i++) { + if (symbols[i] == InvalidHistogramSymbol) + { + continue; + } + int cluster = clusterMappings[symbols[i]]; if (cluster > 0 && clusterMappingsTmp[cluster] == 0) { @@ -466,7 +476,6 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless histograms[idx1].BitCost = histoPriorityList[0].CostCombo; // Remove merged histogram. - // TODO: can the element be removed instead? histograms.RemoveAt(idx2); histograms[idx2] = null; // Remove pairs intersecting the just combined best pair. @@ -501,12 +510,19 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless private static void HistogramRemap(List input, List output, short[] symbols) { - int inSize = symbols.Length; + int inSize = input.Count; int outSize = output.Count; if (outSize > 1) { for (int i = 0; i < inSize; i++) { + if (input[i] == null) + { + // Arbitrarily set to the previous value if unused to help future LZ77. + symbols[i] = symbols[i - 1]; + continue; + } + int bestOut = 0; double bestBits = double.MaxValue; for (int k = 0; k < outSize; k++) diff --git a/src/ImageSharp/Formats/WebP/Lossless/Vp8LEncoder.cs b/src/ImageSharp/Formats/WebP/Lossless/Vp8LEncoder.cs index 61eb589305..2e48bf7dce 100644 --- a/src/ImageSharp/Formats/WebP/Lossless/Vp8LEncoder.cs +++ b/src/ImageSharp/Formats/WebP/Lossless/Vp8LEncoder.cs @@ -9,6 +9,7 @@ using System.IO; using System.Linq; using System.Numerics; using System.Runtime.CompilerServices; + using SixLabors.ImageSharp.Formats.WebP.BitWriter; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.PixelFormats; @@ -417,12 +418,13 @@ 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 + // Keep the best references aside and use the other element from the first // two as a temporary for later usage. Vp8LBackwardRefs refsTmp = refsArray[refsBest.Equals(refsArray[0]) ? 1 : 0]; // TODO : Loop based on cache/no cache - + // TODO: this.bitWriter.Reset(); var tmpHisto = new Vp8LHistogram(cacheBits); var histogramImage = new List(histogramImageXySize); @@ -474,7 +476,6 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless } } - histogramImageSize = maxIndex; this.bitWriter.PutBits((uint)(histogramBits - 2), 3); this.EncodeImageNoHuffman(histogramArgb, hashChain, refsTmp, refsArray[2], LosslessUtils.SubSampleSize(width, histogramBits), LosslessUtils.SubSampleSize(height, histogramBits), quality); } @@ -482,7 +483,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless // Store Huffman codes. // Find maximum number of symbols for the huffman tree-set. int maxTokens = 0; - for (int i = 0; i < 5 * histogramImageSize; i++) + for (int i = 0; i < 5 * histogramImage.Count; i++) { HuffmanTreeCode codes = huffmanCodes[i]; if (maxTokens < codes.NumSymbols) @@ -497,7 +498,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless tokens[i] = new HuffmanTreeToken(); } - for (int i = 0; i < 5 * histogramImageSize; i++) + for (int i = 0; i < 5 * histogramImage.Count; i++) { HuffmanTreeCode codes = huffmanCodes[i]; this.StoreHuffmanCode(huffTree, tokens, codes); @@ -912,7 +913,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless return EntropyIx.Palette; } - using System.Buffers.IMemoryOwner histoBuffer = this.memoryAllocator.Allocate((int)HistoIx.HistoTotal * 256); + using IMemoryOwner histoBuffer = this.memoryAllocator.Allocate((int)HistoIx.HistoTotal * 256); Span histo = histoBuffer.Memory.Span; Bgra32 pixPrev = ToBgra32(image.GetPixelRowSpan(0)[0]); // Skip the first pixel. Span prevRow = null; @@ -955,7 +956,6 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless histo[((int)HistoIx.HistoPalette * 256) + (int)hash]++; } - var histo0 = histo[0]; prevRow = currentRow; } @@ -1149,7 +1149,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless /// private void ApplyPalette(Span src, int srcStride, Span dst, int dstStride, Span palette, int paletteSize, int width, int height, int xBits) { - using System.Buffers.IMemoryOwner tmpRowBuffer = this.memoryAllocator.Allocate(width); + using IMemoryOwner tmpRowBuffer = this.memoryAllocator.Allocate(width); Span tmpRow = tmpRowBuffer.GetSpan(); if (paletteSize < ApplyPaletteGreedyMax) @@ -1209,8 +1209,8 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless } else { - uint[] idxMap = new uint[paletteSize]; - uint[] paletteSorted = new uint[paletteSize]; + var idxMap = new uint[paletteSize]; + var paletteSorted = new uint[paletteSize]; PrepareMapToPalette(palette, paletteSize, paletteSorted, idxMap); ApplyPaletteForWithIdxMap(width, height, palette, src, srcStride, dst, dstStride, tmpRow, idxMap, xBits, paletteSorted, paletteSize); } diff --git a/src/ImageSharp/Formats/WebP/Lossless/Vp8LHistogram.cs b/src/ImageSharp/Formats/WebP/Lossless/Vp8LHistogram.cs index 9b591dcd0e..6fe3496a4d 100644 --- a/src/ImageSharp/Formats/WebP/Lossless/Vp8LHistogram.cs +++ b/src/ImageSharp/Formats/WebP/Lossless/Vp8LHistogram.cs @@ -28,6 +28,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless other.Blue.AsSpan().CopyTo(this.Blue); other.Alpha.AsSpan().CopyTo(this.Alpha); other.Literal.AsSpan().CopyTo(this.Literal); + other.Distance.AsSpan().CopyTo(this.Distance); other.IsUsed.AsSpan().CopyTo(this.IsUsed); this.LiteralCost = other.LiteralCost; this.RedCost = other.RedCost;