Patch Detail
Show a patch.
GET /api/1.1/patches/20928/?format=api
{ "id": 20928, "url": "https://patchwork.libcamera.org/api/1.1/patches/20928/?format=api", "web_url": "https://patchwork.libcamera.org/patch/20928/", "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": "<20240815081622.174210-16-mzamazal@redhat.com>", "date": "2024-08-15T08:16:19", "name": "[v4,15/18] libcamera: software_isp: Use floating point for color parameters", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "1a06d8d2ff66c4d2927ff287543b1d28897359fc", "submitter": { "id": 177, "url": "https://patchwork.libcamera.org/api/1.1/people/177/?format=api", "name": "Milan Zamazal", "email": "mzamazal@redhat.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/20928/mbox/", "series": [ { "id": 4520, "url": "https://patchwork.libcamera.org/api/1.1/series/4520/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4520", "date": "2024-08-15T08:16:04", "name": "Software ISP refactoring", "version": 4, "mbox": "https://patchwork.libcamera.org/series/4520/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/20928/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/20928/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 D5778C32D5\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 15 Aug 2024 08:17:34 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 53388633CD;\n\tThu, 15 Aug 2024 10:17:34 +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 9FA55633D2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 15 Aug 2024 10:17: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-610-88ig3-J4NRCFnuZ2C1P4XQ-1;\n\tThu, 15 Aug 2024 04:17:27 -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-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTPS id 1CDBE1955D48; Thu, 15 Aug 2024 08:17:26 +0000 (UTC)", "from nuthatch.redhat.com (unknown [10.45.225.128])\n\tby mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTP id C6EBC19560A3; Thu, 15 Aug 2024 08:17: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=\"J836sC9/\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1723709850;\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=feNHMmQPRk0OZHvCFmmY2InaFfyBBPiS8Tk9yGS11dY=;\n\tb=J836sC9/FjDUew5tnTAaXmZxW7eeaWkk+k6LR5etp+CiZlJyM69b4AmiJErD0wYXTmOszw\n\tUp08e03MzNlQakhb+EHDBM8WyPMXmlq3mz9ndhUP6KmkwfUnGb2hVJpdoOS6SKOwQxrGHz\n\tuvE7GUb0NjewfxYEu7rWQqvm0nxxybI=", "X-MC-Unique": "88ig3-J4NRCFnuZ2C1P4XQ-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 v4 15/18] libcamera: software_isp: Use floating point for\n\tcolor parameters", "Date": "Thu, 15 Aug 2024 10:16:19 +0200", "Message-ID": "<20240815081622.174210-16-mzamazal@redhat.com>", "In-Reply-To": "<20240815081622.174210-1-mzamazal@redhat.com>", "References": "<20240815081622.174210-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 | 24 +++++++++++++-----------\n src/ipa/simple/algorithms/blc.cpp | 8 ++++----\n src/ipa/simple/algorithms/gamma.cpp | 2 +-\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 1e43f3fd..ff6591de 100644\n--- a/src/ipa/simple/algorithms/awb.cpp\n+++ b/src/ipa/simple/algorithms/awb.cpp\n@@ -27,7 +27,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@@ -40,15 +40,18 @@ void Awb::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 }\n@@ -60,7 +63,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@@ -77,12 +80,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/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": [ "v4", "15/18" ] }