Patch Detail
Show a patch.
GET /api/patches/21071/?format=api
{ "id": 21071, "url": "https://patchwork.libcamera.org/api/patches/21071/?format=api", "web_url": "https://patchwork.libcamera.org/patch/21071/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/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": "<20240830072554.180672-16-mzamazal@redhat.com>", "date": "2024-08-30T07:25:51", "name": "[v5,15/18] libcamera: software_isp: Use floating point for color parameters", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "36457da63faa9e6bee6d3d65a01af2fd60769e42", "submitter": { "id": 177, "url": "https://patchwork.libcamera.org/api/people/177/?format=api", "name": "Milan Zamazal", "email": "mzamazal@redhat.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/21071/mbox/", "series": [ { "id": 4548, "url": "https://patchwork.libcamera.org/api/series/4548/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4548", "date": "2024-08-30T07:25:36", "name": "Software ISP refactoring", "version": 5, "mbox": "https://patchwork.libcamera.org/series/4548/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/21071/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/21071/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 4B856C32D5\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 30 Aug 2024 07:26:56 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AF8B0634EB;\n\tFri, 30 Aug 2024 09:26:55 +0200 (CEST)", "from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 59D0C6341E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 30 Aug 2024 09:26:51 +0200 (CEST)", "from mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com\n\t(ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63])\n\tby relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3,\n\tcipher=TLS_AES_256_GCM_SHA384) id us-mta-616-ZH8UeedSOt2ayfoET0nNdw-1;\n\tFri, 30 Aug 2024 03:26:44 -0400", "from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com\n\t(mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com\n\t[10.30.177.40])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (2048 bits)\n\tserver-digest SHA256) (No client certificate requested)\n\tby mx-prod-mc-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTPS id B350919560B1; Fri, 30 Aug 2024 07:26:43 +0000 (UTC)", "from nuthatch.redhat.com (unknown [10.45.225.65])\n\tby mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTP id 5F62C19560A3; Fri, 30 Aug 2024 07:26:41 +0000 (UTC)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"Qzg0HYxD\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1725002810;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=JTLOpHrUNalfC38eZPdCfc/vm6uc9RnUCdBkVqwy9H8=;\n\tb=Qzg0HYxD8AX6YF8ACsRr48qfKxSdiTjymV4s+HjM8bX65haJhT3Gf8gMDjqlhrAtxL2RDC\n\tu2Ej29IReEu/ycfJCfC2HIccMfyNHANZnOR81poJ/OIEXcq0bRjdJbErufRc+ZomPO16bY\n\tkYp5x05vms6cNK3iip2T100Dw5YVxhY=", "X-MC-Unique": "ZH8UeedSOt2ayfoET0nNdw-1", "From": "Milan Zamazal <mzamazal@redhat.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Milan Zamazal <mzamazal@redhat.com>,\n\tUmang Jain <umang.jain@ideasonboard.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tDaniel Scally <dan.scally@ideasonboard.com>", "Subject": "[PATCH v5 15/18] libcamera: software_isp: Use floating point for\n\tcolor parameters", "Date": "Fri, 30 Aug 2024 09:25:51 +0200", "Message-ID": "<20240830072554.180672-16-mzamazal@redhat.com>", "In-Reply-To": "<20240830072554.180672-1-mzamazal@redhat.com>", "References": "<20240830072554.180672-1-mzamazal@redhat.com>", "MIME-Version": "1.0", "X-Scanned-By": "MIMEDefang 3.0 on 10.30.177.40", "X-Mimecast-Spam-Score": "0", "X-Mimecast-Originator": "redhat.com", "Content-Transfer-Encoding": "8bit", "Content-Type": "text/plain; charset=\"US-ASCII\"; x-default=true", "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": "It's more natural to represent color gains and black level as floating\npoint numbers rather than using a particular pixel-related\nrepresentation.\n\ndouble is used rather than float because it's a more common floating\npoint type in libcamera algorithms. Otherwise there is no obvious\nreason to select one over the other here.\n\nThe constructed color tables still use integer representation for\nefficiency.\n\nSigned-off-by: Milan Zamazal <mzamazal@redhat.com>\n---\n src/ipa/simple/algorithms/awb.cpp | 11 +++++------\n src/ipa/simple/algorithms/blc.cpp | 8 ++++----\n src/ipa/simple/algorithms/gamma.cpp | 2 +-\n src/ipa/simple/algorithms/lut.cpp | 13 ++++++++-----\n src/ipa/simple/ipa_context.h | 11 +++++------\n 5 files changed, 23 insertions(+), 22 deletions(-)", "diff": "diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp\nindex 4b49996a..16a0fa61 100644\n--- a/src/ipa/simple/algorithms/awb.cpp\n+++ b/src/ipa/simple/algorithms/awb.cpp\n@@ -24,7 +24,7 @@ int Awb::init(IPAContext &context,\n \t [[maybe_unused]] const YamlObject &tuningData)\n {\n \tauto &gains = context.activeState.gains;\n-\tgains.red = gains.green = gains.blue = 256;\n+\tgains.red = gains.green = gains.blue = 1.0;\n \n \treturn 0;\n }\n@@ -36,7 +36,7 @@ void Awb::process(IPAContext &context,\n \t\t [[maybe_unused]] ControlList &metadata)\n {\n \tconst SwIspStats::Histogram &histogram = stats->yHistogram;\n-\tconst uint8_t blackLevel = context.activeState.black.level;\n+\tconst double blackLevel = context.activeState.black.level;\n \n \t/*\n \t * Black level must be subtracted to get the correct AWB ratios, they\n@@ -53,12 +53,11 @@ void Awb::process(IPAContext &context,\n \t/*\n \t * Calculate red and blue gains for AWB.\n \t * Clamp max gain at 4.0, this also avoids 0 division.\n-\t * Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.\n \t */\n \tauto &gains = context.activeState.gains;\n-\tgains.red = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;\n-\tgains.blue = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;\n-\t/* Green gain is fixed to 256 */\n+\tgains.red = sumR <= sumG / 4 ? 4.0 : static_cast<double>(sumG) / sumR;\n+\tgains.blue = sumB <= sumG / 4 ? 4.0 : static_cast<double>(sumG) / sumB;\n+\t/* Green gain is fixed to 1.0 */\n \n \tLOG(IPASoftAwb, Debug) << \"gain R/B \" << gains.red << \"/\" << gains.blue;\n }\ndiff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp\nindex 3b73d830..ab7e7dd9 100644\n--- a/src/ipa/simple/algorithms/blc.cpp\n+++ b/src/ipa/simple/algorithms/blc.cpp\n@@ -24,7 +24,7 @@ BlackLevel::BlackLevel()\n int BlackLevel::init(IPAContext &context,\n \t\t [[maybe_unused]] const YamlObject &tuningData)\n {\n-\tcontext.activeState.black.level = 255;\n+\tcontext.activeState.black.level = 1.0;\n \treturn 0;\n }\n \n@@ -44,16 +44,16 @@ void BlackLevel::process(IPAContext &context,\n \tconst unsigned int total =\n \t\tstd::accumulate(begin(histogram), end(histogram), 0);\n \tconst unsigned int pixelThreshold = ignoredPercentage_ * total;\n-\tconst unsigned int histogramRatio = 256 / SwIspStats::kYHistogramSize;\n \tconst unsigned int currentBlackIdx =\n-\t\tcontext.activeState.black.level / histogramRatio;\n+\t\tcontext.activeState.black.level * SwIspStats::kYHistogramSize;\n \n \tfor (unsigned int i = 0, seen = 0;\n \t i < currentBlackIdx && i < SwIspStats::kYHistogramSize;\n \t i++) {\n \t\tseen += histogram[i];\n \t\tif (seen >= pixelThreshold) {\n-\t\t\tcontext.activeState.black.level = i * histogramRatio;\n+\t\t\tcontext.activeState.black.level =\n+\t\t\t\tstatic_cast<double>(i) / SwIspStats::kYHistogramSize;\n \t\t\tLOG(IPASoftBL, Debug)\n \t\t\t\t<< \"Auto-set black level: \"\n \t\t\t\t<< i << \"/\" << SwIspStats::kYHistogramSize\ndiff --git a/src/ipa/simple/algorithms/gamma.cpp b/src/ipa/simple/algorithms/gamma.cpp\nindex 0b8ec5ee..b03dff25 100644\n--- a/src/ipa/simple/algorithms/gamma.cpp\n+++ b/src/ipa/simple/algorithms/gamma.cpp\n@@ -34,7 +34,7 @@ void Gamma::updateGammaTable(IPAContext &context)\n \tauto &gammaTable = context.activeState.gamma.gammaTable;\n \tauto blackLevel = context.activeState.black.level;\n \tconst unsigned int blackIndex =\n-\t\tblackLevel * IPAActiveState::kGammaLookupSize / 256;\n+\t\tblackLevel * IPAActiveState::kGammaLookupSize;\n \tstd::fill(gammaTable.begin(), gammaTable.begin() + blackIndex, 0);\n \tconst float divisor = kGammaLookupSize - blackIndex - 1.0;\n \tfor (unsigned int i = blackIndex; i < kGammaLookupSize; i++)\ndiff --git a/src/ipa/simple/algorithms/lut.cpp b/src/ipa/simple/algorithms/lut.cpp\nindex 748f10aa..a0e67080 100644\n--- a/src/ipa/simple/algorithms/lut.cpp\n+++ b/src/ipa/simple/algorithms/lut.cpp\n@@ -37,15 +37,18 @@ void Lut::prepare(IPAContext &context,\n \tauto &gains = context.activeState.gains;\n \tauto &gammaTable = context.activeState.gamma.gammaTable;\n \tfor (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {\n-\t\tconstexpr unsigned int div =\n-\t\t\tstatic_cast<double>(DebayerParams::kRGBLookupSize) * 256 / kGammaLookupSize;\n+\t\tconstexpr double div =\n+\t\t\tstatic_cast<double>(DebayerParams::kRGBLookupSize) / kGammaLookupSize;\n \t\t/* Apply gamma after gain! */\n \t\tunsigned int idx;\n-\t\tidx = std::min({ i * gains.red / div, kGammaLookupSize - 1 });\n+\t\tidx = std::min({ static_cast<unsigned int>(i * gains.red / div),\n+\t\t\t\t kGammaLookupSize - 1 });\n \t\tparams->red[i] = gammaTable[idx];\n-\t\tidx = std::min({ i * gains.green / div, kGammaLookupSize - 1 });\n+\t\tidx = std::min({ static_cast<unsigned int>(i * gains.green / div),\n+\t\t\t\t kGammaLookupSize - 1 });\n \t\tparams->green[i] = gammaTable[idx];\n-\t\tidx = std::min({ i * gains.blue / div, kGammaLookupSize - 1 });\n+\t\tidx = std::min({ static_cast<unsigned int>(i * gains.blue / div),\n+\t\t\t\t kGammaLookupSize - 1 });\n \t\tparams->blue[i] = gammaTable[idx];\n \t}\n }\ndiff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\nindex f8b8834d..28a02475 100644\n--- a/src/ipa/simple/ipa_context.h\n+++ b/src/ipa/simple/ipa_context.h\n@@ -8,7 +8,6 @@\n #pragma once\n \n #include <array>\n-#include <stdint.h>\n \n #include <libipa/fc_queue.h>\n \n@@ -22,17 +21,17 @@ struct IPASessionConfiguration {\n \n struct IPAActiveState {\n \tstruct {\n-\t\tuint8_t level;\n+\t\tdouble level;\n \t} black;\n \tstruct {\n-\t\tunsigned int red;\n-\t\tunsigned int green;\n-\t\tunsigned int blue;\n+\t\tdouble red;\n+\t\tdouble green;\n+\t\tdouble blue;\n \t} gains;\n \tstatic constexpr unsigned int kGammaLookupSize = 1024;\n \tstruct {\n \t\tstd::array<double, kGammaLookupSize> gammaTable;\n-\t\tuint8_t blackLevel;\n+\t\tdouble blackLevel;\n \t} gamma;\n };\n \n", "prefixes": [ "v5", "15/18" ] }