Patch Detail
Show a patch.
GET /api/1.1/patches/23092/?format=api
{ "id": 23092, "url": "https://patchwork.libcamera.org/api/1.1/patches/23092/?format=api", "web_url": "https://patchwork.libcamera.org/patch/23092/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20250401123633.58887-6-stefan.klug@ideasonboard.com>", "date": "2025-04-01T12:36:13", "name": "[v2,5/5] libipa: histogram: Fix interQuantileMean() for small ranges", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "00bd173e7af32d15a402f3172dcc830606b8fa9a", "submitter": { "id": 184, "url": "https://patchwork.libcamera.org/api/1.1/people/184/?format=api", "name": "Stefan Klug", "email": "stefan.klug@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/23092/mbox/", "series": [ { "id": 5099, "url": "https://patchwork.libcamera.org/api/1.1/series/5099/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5099", "date": "2025-04-01T12:36:08", "name": "Fix histogram for some (corner) cases", "version": 2, "mbox": "https://patchwork.libcamera.org/series/5099/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/23092/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/23092/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9A312C3213\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 1 Apr 2025 12:36:56 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 53C9768991;\n\tTue, 1 Apr 2025 14:36:56 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E73F66898A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 1 Apr 2025 14:36:53 +0200 (CEST)", "from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:14c7:4fcc:495b:719f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CA2968DB;\n\tTue, 1 Apr 2025 14:35:01 +0200 (CEST)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"AyH8eUBd\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743510901;\n\tbh=bUNDK7vLq1vWyOMPzMZ+ZDpn7eKJqeahqSjxAG4P9vE=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=AyH8eUBdAqscnPyY8wzlkNDpxBLvUDycIIwG7hpqSusEAKoI0diQAO9jkFlOyv02z\n\tz7g3Z9UkJ3KrOe9PnUT0u+FSl57Op/eXFQgUhi5aePRuXNm4eZ2R7Yli5By1GWdus0\n\tmFf3Gv3utMV0Ew2N4q6lFuymSIts8PpyCIkGXV3g=", "From": "Stefan Klug <stefan.klug@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Stefan Klug <stefan.klug@ideasonboard.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>", "Subject": "[PATCH v2 5/5] libipa: histogram: Fix interQuantileMean() for small\n\tranges", "Date": "Tue, 1 Apr 2025 14:36:13 +0200", "Message-ID": "<20250401123633.58887-6-stefan.klug@ideasonboard.com>", "X-Mailer": "git-send-email 2.43.0", "In-Reply-To": "<20250401123633.58887-1-stefan.klug@ideasonboard.com>", "References": "<20250401123633.58887-1-stefan.klug@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The interQuantileMean() is supposed to return a weighted mean value\nbetween two quantiles. This works for fine histograms, but fails for\ncoarse histograms and small quantile ranges because the weight is always\ntaken from the lower border of the bin.\n\nFix that by rewriting the algorithm to calculate a lower and upper bound\nfor every (partial) bin that goes into the mean calculation and weight\nthe bins by the middle of these bounds.\n\nSigned-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\nAcked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n---\n\nChanges in v2:\n- Order fix patch after test patch\n- Remove should_fail\n- Added documentation based on review\n- Improved code style based on review\n---\n src/ipa/libipa/histogram.cpp | 39 +++++++++++++++++++++++-------------\n test/ipa/libipa/meson.build | 2 +-\n 2 files changed, 26 insertions(+), 15 deletions(-)", "diff": "diff --git a/src/ipa/libipa/histogram.cpp b/src/ipa/libipa/histogram.cpp\nindex ea042f0a17b9..bcf263908236 100644\n--- a/src/ipa/libipa/histogram.cpp\n+++ b/src/ipa/libipa/histogram.cpp\n@@ -149,26 +149,37 @@ double Histogram::quantile(double q, uint32_t first, uint32_t last) const\n double Histogram::interQuantileMean(double lowQuantile, double highQuantile) const\n {\n \tASSERT(highQuantile > lowQuantile);\n-\t/* Proportion of pixels which lies below lowQuantile */\n-\tdouble lowPoint = quantile(lowQuantile);\n-\t/* Proportion of pixels which lies below highQuantile */\n-\tdouble highPoint = quantile(highQuantile, static_cast<uint32_t>(lowPoint));\n-\tdouble sumBinFreq = 0, cumulFreq = 0;\n-\n-\tfor (double p_next = floor(lowPoint) + 1.0;\n-\t p_next <= ceil(highPoint);\n-\t lowPoint = p_next, p_next += 1.0) {\n-\t\tint bin = floor(lowPoint);\n+\n+\t/* Proportion of pixels which lies below lowQuantile and highQuantile. */\n+\tconst double lowPoint = quantile(lowQuantile);\n+\tconst double highPoint = quantile(highQuantile, static_cast<uint32_t>(lowPoint));\n+\n+\tdouble sumBinFreq = 0;\n+\tdouble cumulFreq = 0;\n+\n+\t/*\n+ * Calculate the mean pixel value between the low and high points by\n+ * summing all the pixels between the two points, and dividing the sum\n+ * by the number of pixels. Given the discrete nature of the histogram\n+ * data, the sum of the pixels is approximated by accumulating the\n+ * product of the bin values (calculated as the mid point of the bin) by\n+ * the number of pixels they contain, for each bin in the internal.\n+ */\n+\tfor (unsigned bin = std::floor(lowPoint); bin < std::ceil(highPoint); bin++) {\n+\t\tconst double lowBound = std::max<double>(bin, lowPoint);\n+\t\tconst double highBound = std::min<double>(bin + 1, highPoint);\n+\n \t\tdouble freq = (cumulative_[bin + 1] - cumulative_[bin])\n-\t\t\t* (std::min(p_next, highPoint) - lowPoint);\n+\t\t\t * (highBound - lowBound);\n \n \t\t/* Accumulate weighted bin */\n-\t\tsumBinFreq += bin * freq;\n+\t\tsumBinFreq += (highBound + lowBound) / 2 * freq;\n+\n \t\t/* Accumulate weights */\n \t\tcumulFreq += freq;\n \t}\n-\t/* add 0.5 to give an average for bin mid-points */\n-\treturn sumBinFreq / cumulFreq + 0.5;\n+\n+\treturn sumBinFreq / cumulFreq;\n }\n \n } /* namespace ipa */\ndiff --git a/test/ipa/libipa/meson.build b/test/ipa/libipa/meson.build\nindex 83c84bd8c227..8c63ebd8e2f7 100644\n--- a/test/ipa/libipa/meson.build\n+++ b/test/ipa/libipa/meson.build\n@@ -2,7 +2,7 @@\n \n libipa_test = [\n {'name': 'fixedpoint', 'sources': ['fixedpoint.cpp']},\n- {'name': 'histogram', 'sources': ['histogram.cpp'], 'should_fail': true},\n+ {'name': 'histogram', 'sources': ['histogram.cpp']},\n {'name': 'interpolator', 'sources': ['interpolator.cpp']},\n ]\n \n", "prefixes": [ "v2", "5/5" ] }