From d6aa3f6c9c1624e3add3146f7e2ee1d989fab25a Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 30 Jul 2020 20:08:16 +0200 Subject: [PATCH] Fix issues in HistogramCombineStochastic (still needs another review) --- .../Formats/WebP/Lossless/CostManager.cs | 11 +++---- .../Formats/WebP/Lossless/HistogramEncoder.cs | 32 +++++++++---------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/ImageSharp/Formats/WebP/Lossless/CostManager.cs b/src/ImageSharp/Formats/WebP/Lossless/CostManager.cs index 279aa6f51d..d912e2e28e 100644 --- a/src/ImageSharp/Formats/WebP/Lossless/CostManager.cs +++ b/src/ImageSharp/Formats/WebP/Lossless/CostManager.cs @@ -2,17 +2,18 @@ // Licensed under the Apache License, Version 2.0. using System.Collections.Generic; -using System.Linq; namespace SixLabors.ImageSharp.Formats.WebP.Lossless { /// /// The CostManager is in charge of managing intervals and costs. /// It caches the different CostCacheInterval, caches the different - /// GetLengthCost(costModel, k) in cost_cache_ and the CostInterval's. + /// GetLengthCost(costModel, k) in costCache and the CostInterval's. /// internal class CostManager { + private CostInterval head; + public CostManager(short[] distArray, int pixCount, CostModel costModel) { int costCacheSize = (pixCount > BackwardReferenceEncoder.MaxLength) ? BackwardReferenceEncoder.MaxLength : pixCount; @@ -63,15 +64,13 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless cur.End = i + 1; } - // Set the initial costs_ high for every pixel as we will keep the minimum. + // Set the initial costs high for every pixel as we will keep the minimum. for (int i = 0; i < pixCount; i++) { this.Costs[i] = 1e38f; } } - private CostInterval head; - /// /// Gets or sets the number of stored intervals. /// @@ -230,7 +229,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless return; } - ConnectIntervals(interval.Previous, interval.Next); + this.ConnectIntervals(interval.Previous, interval.Next); this.Count--; } diff --git a/src/ImageSharp/Formats/WebP/Lossless/HistogramEncoder.cs b/src/ImageSharp/Formats/WebP/Lossless/HistogramEncoder.cs index 1893e6015c..324a2848b4 100644 --- a/src/ImageSharp/Formats/WebP/Lossless/HistogramEncoder.cs +++ b/src/ImageSharp/Formats/WebP/Lossless/HistogramEncoder.cs @@ -48,6 +48,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless // Copies the histograms and computes its bitCost. histogramSymbols is optimized. HistogramCopyAndAnalyze(origHisto, imageHisto, histogramSymbols); + numUsed = imageHisto.Count(h => h != null); var entropyCombine = (numUsed > entropyCombineNumBins * 2) && (quality < 100); if (entropyCombine) @@ -319,12 +320,17 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless // Priority queue of histogram pairs. Its size impacts the quality of the compression and the speed: // the smaller the faster but the worse for the compression. var histoPriorityList = new List(); - int histoQueueMaxSize = histograms.Count * histograms.Count; + int maxSize = 9; // Fill the initial mapping. var mappings = new int[histograms.Count]; for (int j = 0, iter = 0; iter < histograms.Count; iter++) { + if (histograms[iter] == null) + { + continue; + } + mappings[j++] = iter; } @@ -353,15 +359,14 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless idx2 = mappings[idx2]; // Calculate cost reduction on combination. - var currCost = HistoPriorityListPush(histoPriorityList, histoQueueMaxSize, histograms, idx1, idx2, bestCost); + var currCost = HistoPriorityListPush(histoPriorityList, maxSize, histograms, idx1, idx2, bestCost); // Found a better pair? if (currCost < 0) { bestCost = currCost; - // Empty the queue if we reached full capacity. - if (histoPriorityList.Count == histoQueueMaxSize) + if (histoPriorityList.Count == maxSize) { break; } @@ -377,10 +382,10 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless bestIdx1 = histoPriorityList[0].Idx1; bestIdx2 = histoPriorityList[0].Idx2; + // TODO: Review this again, not sure why this is needed in the reference implementation. // Pop bestIdx2 from mappings. - var mappingIndex = Array.BinarySearch(mappings, bestIdx2); - - // TODO: memmove(mapping_index, mapping_index + 1, sizeof(*mapping_index) *((*num_used) - (mapping_index - mappings) - 1)); + // var mappingIndex = Array.BinarySearch(mappings, bestIdx2); + // memmove(mapping_index, mapping_index + 1, sizeof(*mapping_index) *((*num_used) - (mapping_index - mappings) - 1)); // Merge the histograms and remove bestIdx2 from the queue. HistogramAdd(histograms[bestIdx2], histograms[bestIdx1], histograms[bestIdx1]); @@ -388,8 +393,6 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless histograms.RemoveAt(bestIdx2); numUsed--; - var indicesToRemove = new List(); - int lastIndex = histoPriorityList.Count - 1; for (int j = 0; j < histoPriorityList.Count;) { HistogramPair p = histoPriorityList.ElementAt(j); @@ -401,9 +404,8 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless // check for it all the time nevertheless. if (isIdx1Best && isIdx2Best) { - indicesToRemove.Add(lastIndex); + histoPriorityList.RemoveAt(histoPriorityList.Count - 1); numUsed--; - lastIndex--; continue; } @@ -434,8 +436,7 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless HistoListUpdatePair(histograms[p.Idx1], histograms[p.Idx2], 0.0d, p); if (p.CostDiff >= 0.0d) { - indicesToRemove.Add(lastIndex); - lastIndex--; + histoPriorityList.RemoveAt(histoPriorityList.Count - 1); numUsed--; continue; } @@ -578,10 +579,9 @@ namespace SixLabors.ImageSharp.Formats.WebP.Lossless } /// - /// Create a pair from indices "idx1" and "idx2" provided its cost - /// is inferior to "threshold", a negative entropy. + /// Create a pair from indices "idx1" and "idx2" provided its cost is inferior to "threshold", a negative entropy. /// - /// The cost of the pair, or 0. if it superior to threshold. + /// The cost of the pair, or 0 if it superior to threshold. private static double HistoPriorityListPush(List histoList, int maxSize, List histograms, int idx1, int idx2, double threshold) { var pair = new HistogramPair();