libipa: histogram: Fix interQuantileMean() for small ranges

The interQuantileMean() is supposed to return a weighted mean value
between two quantiles. This works for fine histograms, but fails for
coarse histograms and small quantile ranges because the weight is always
taken from the lower border of the bin.

Fix that by rewriting the algorithm to calculate a lower and upper bound
for every (partial) bin that goes into the mean calculation and weight
the bins by the middle of these bounds.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

src/ipa/libipa/histogram.cpp

@@ -149,26 +149,37 @@ double Histogram::quantile(double q, uint32_t first, uint32_t last) const
 double Histogram::interQuantileMean(double lowQuantile, double highQuantile) const
 {
 	ASSERT(highQuantile > lowQuantile);
-	/* Proportion of pixels which lies below lowQuantile */
-	double lowPoint = quantile(lowQuantile);
-	/* Proportion of pixels which lies below highQuantile */
-	double highPoint = quantile(highQuantile, static_cast<uint32_t>(lowPoint));
-	double sumBinFreq = 0, cumulFreq = 0;
 
-	for (double p_next = floor(lowPoint) + 1.0;
+	/* Proportion of pixels which lies below lowQuantile and highQuantile. */
-	     p_next <= ceil(highPoint);
+	const double lowPoint = quantile(lowQuantile);
-	     lowPoint = p_next, p_next += 1.0) {
+	const double highPoint = quantile(highQuantile, static_cast<uint32_t>(lowPoint));
-		int bin = floor(lowPoint);
+
+	double sumBinFreq = 0;
+	double cumulFreq = 0;
+
+	/*
+         * Calculate the mean pixel value between the low and high points by
+         * summing all the pixels between the two points, and dividing the sum
+         * by the number of pixels. Given the discrete nature of the histogram
+         * data, the sum of the pixels is approximated by accumulating the
+         * product of the bin values (calculated as the mid point of the bin) by
+         * the number of pixels they contain, for each bin in the internal.
+         */
+	for (unsigned bin = std::floor(lowPoint); bin < std::ceil(highPoint); bin++) {
+		const double lowBound = std::max<double>(bin, lowPoint);
+		const double highBound = std::min<double>(bin + 1, highPoint);
+
 		double freq = (cumulative_[bin + 1] - cumulative_[bin])
-			* (std::min(p_next, highPoint) - lowPoint);
+			    * (highBound - lowBound);
 
 		/* Accumulate weighted bin */
-		sumBinFreq += bin * freq;
+		sumBinFreq += (highBound + lowBound) / 2 * freq;
+
 		/* Accumulate weights */
 		cumulFreq += freq;
 	}
-	/* add 0.5 to give an average for bin mid-points */
+
-	return sumBinFreq / cumulFreq + 0.5;
+	return sumBinFreq / cumulFreq;
 }
 
 } /* namespace ipa */

test/ipa/libipa/meson.build

@@ -2,7 +2,7 @@
 
 libipa_test = [
     {'name': 'fixedpoint', 'sources': ['fixedpoint.cpp']},
-    {'name': 'histogram', 'sources': ['histogram.cpp'], 'should_fail': true},
+    {'name': 'histogram', 'sources': ['histogram.cpp']},
     {'name': 'interpolator', 'sources': ['interpolator.cpp']},
 ]