Patch Detail
Show a patch.
GET /api/patches/21188/?format=api
{ "id": 21188, "url": "https://patchwork.libcamera.org/api/patches/21188/?format=api", "web_url": "https://patchwork.libcamera.org/patch/21188/", "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": "<20240906120927.4071508-16-mzamazal@redhat.com>", "date": "2024-09-06T12:09:24", "name": "[v6,15/18] libcamera: software_isp: Use floating point for color parameters", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "2be41d5ed98e09ec41175ebd59bc506c497d3e20", "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/21188/mbox/", "series": [ { "id": 4566, "url": "https://patchwork.libcamera.org/api/series/4566/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4566", "date": "2024-09-06T12:09:09", "name": "Software ISP refactoring", "version": 6, "mbox": "https://patchwork.libcamera.org/series/4566/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/21188/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/21188/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 8B3B1BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 6 Sep 2024 12:10:50 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D2CBE634F7;\n\tFri, 6 Sep 2024 14:10:48 +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 20994634FD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 6 Sep 2024 14:10:31 +0200 (CEST)", "from mx-prod-mc-02.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-288-51D37E-6NS-j2iyGdqHnLg-1;\n\tFri, 06 Sep 2024 08:10:26 -0400", "from mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com\n\t(mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com\n\t[10.30.177.15])\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-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTPS id AD5CD1955F40; Fri, 6 Sep 2024 12:10:25 +0000 (UTC)", "from nuthatch.redhat.com (unknown [10.45.224.65])\n\tby mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTP id 946AB1956086; Fri, 6 Sep 2024 12:10:23 +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=\"NXJaox4F\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1725624629;\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=LhPYahubLgT9TeEaeFrjxpAUFrfwXDyiNQQjCtfn0Tc=;\n\tb=NXJaox4FjjH0XygBRJ5gzVYeHqvZ83ks/fk0m1avvUSFwIgujasBZCZqwUZbcaTrrFnNGT\n\t5QWiTevaGqRrjdUJDdqNhbDWNWCX+HL9koUzc+D3jdRkQexiIMmZrTI44qQWyUJQqwMg+/\n\tI5ddUXLVEnapfO6TruYifcCndifECTQ=", "X-MC-Unique": "51D37E-6NS-j2iyGdqHnLg-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 v6 15/18] libcamera: software_isp: Use floating point for\n\tcolor parameters", "Date": "Fri, 6 Sep 2024 14:09:24 +0200", "Message-ID": "<20240906120927.4071508-16-mzamazal@redhat.com>", "In-Reply-To": "<20240906120927.4071508-1-mzamazal@redhat.com>", "References": "<20240906120927.4071508-1-mzamazal@redhat.com>", "MIME-Version": "1.0", "X-Scanned-By": "MIMEDefang 3.0 on 10.30.177.15", "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/lut.cpp | 15 +++++++++------\n src/ipa/simple/ipa_context.h | 11 +++++------\n 4 files changed, 23 insertions(+), 22 deletions(-)", "diff": "diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp\nindex f89f1d3d..61d9fc73 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.blc.level;\n+\tconst double blackLevel = context.activeState.blc.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 08f4345e..1ceae85d 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.blc.level = 255;\n+\tcontext.activeState.blc.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.blc.level / histogramRatio;\n+\t\tcontext.activeState.blc.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.blc.level = i * histogramRatio;\n+\t\t\tcontext.activeState.blc.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/lut.cpp b/src/ipa/simple/algorithms/lut.cpp\nindex 6781f34e..d3b33710 100644\n--- a/src/ipa/simple/algorithms/lut.cpp\n+++ b/src/ipa/simple/algorithms/lut.cpp\n@@ -32,7 +32,7 @@ void Lut::updateGammaTable(IPAContext &context)\n {\n \tauto &gammaTable = context.activeState.gamma.gammaTable;\n \tauto blackLevel = context.activeState.blc.level;\n-\tconst unsigned int blackIndex = blackLevel * gammaTable.size() / 256;\n+\tconst unsigned int blackIndex = blackLevel * gammaTable.size();\n \tstd::fill(gammaTable.begin(), gammaTable.begin() + blackIndex, 0);\n \tconst float divisor = gammaTable.size() - blackIndex - 1.0;\n \tfor (unsigned int i = blackIndex; i < gammaTable.size(); i++)\n@@ -58,15 +58,18 @@ void Lut::prepare(IPAContext &context,\n \tauto &gammaTable = context.activeState.gamma.gammaTable;\n \tconst unsigned int gammaTableSize = gammaTable.size();\n \tfor (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {\n-\t\tconst unsigned int div = static_cast<double>(DebayerParams::kRGBLookupSize) *\n-\t\t\t\t\t 256 / gammaTableSize;\n+\t\tconst double div = static_cast<double>(DebayerParams::kRGBLookupSize) /\n+\t\t\t\t gammaTableSize;\n \t\t/* Apply gamma after gain! */\n \t\tunsigned int idx;\n-\t\tidx = std::min({ i * gains.red / div, gammaTableSize - 1 });\n+\t\tidx = std::min({ static_cast<unsigned int>(i * gains.red / div),\n+\t\t\t\t gammaTableSize - 1 });\n \t\tparams->red[i] = gammaTable[idx];\n-\t\tidx = std::min({ i * gains.green / div, gammaTableSize - 1 });\n+\t\tidx = std::min({ static_cast<unsigned int>(i * gains.green / div),\n+\t\t\t\t gammaTableSize - 1 });\n \t\tparams->green[i] = gammaTable[idx];\n-\t\tidx = std::min({ i * gains.blue / div, gammaTableSize - 1 });\n+\t\tidx = std::min({ static_cast<unsigned int>(i * gains.blue / div),\n+\t\t\t\t gammaTableSize - 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 737bbbc3..d09de9a9 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,19 +21,19 @@ struct IPASessionConfiguration {\n \n struct IPAActiveState {\n \tstruct {\n-\t\tuint8_t level;\n+\t\tdouble level;\n \t} blc;\n \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 \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": [ "v6", "15/18" ] }