{"id":20109,"url":"https://patchwork.libcamera.org/api/patches/20109/?format=json","web_url":"https://patchwork.libcamera.org/patch/20109/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20240528161126.35119-4-mzamazal@redhat.com>","date":"2024-05-28T16:11:24","name":"[v4,3/5] libcamera: software_isp: Move color mappings out of debayering","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"08ce75d203451260ff7f5b9ec623be5afd6119d2","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/?format=json","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/20109/mbox/","series":[{"id":4330,"url":"https://patchwork.libcamera.org/api/series/4330/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4330","date":"2024-05-28T16:11:21","name":"Software ISP levels cleanup","version":4,"mbox":"https://patchwork.libcamera.org/series/4330/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/20109/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/20109/checks/","tags":{},"headers":{"Return-Path":"","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 3B770BDE6B\n\tfor ;\n\tTue, 28 May 2024 16:12:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6C5EA634B6;\n\tTue, 28 May 2024 18:12:01 +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 7FC62634BA\n\tfor ;\n\tTue, 28 May 2024 18:11:55 +0200 (CEST)","from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com\n\t[66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-318-R5GlQGymMJOFMrRdd0slYQ-1; Tue, 28 May 2024 12:11:52 -0400","from smtp.corp.redhat.com\n\t(int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8])\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 mimecast-mx02.redhat.com (Postfix) with ESMTPS id 2CBAF8025F9;\n\tTue, 28 May 2024 16:11:51 +0000 (UTC)","from nuthatch.redhat.com (unknown [10.45.224.39])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id AD9DAC15BB9;\n\tTue, 28 May 2024 16:11:49 +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=\"diSGiMAl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1716912714;\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=kbcg6TZu3b6IEcKe6EJyu7y+e0rD/izn9bJt78Fzht8=;\n\tb=diSGiMAlVsPk9bNJDZ3eED3V53jvW0Y9mjw7ki9L4YkhlAM36DLBzOf3WSCWlRyXnw2YwH\n\tJDfCZJ0smJ+iRwEdMbIrdWt9sc8pFA6zvrqErHBEAU6dvJBOLiUXiuZGC8k1GNxwgmMqUd\n\t5ogGU2NTwrLsfhVYEZu9EydlWm11usE=","X-MC-Unique":"R5GlQGymMJOFMrRdd0slYQ-1","From":"Milan Zamazal ","To":"libcamera-devel@lists.libcamera.org","Cc":"Milan Zamazal ,\n\tKieran Bingham ,\n\tAndrei Konovalov ,\n\tLaurent Pinchart ","Subject":"[PATCH v4 3/5] libcamera: software_isp: Move color mappings out of\n\tdebayering","Date":"Tue, 28 May 2024 18:11:24 +0200","Message-ID":"<20240528161126.35119-4-mzamazal@redhat.com>","In-Reply-To":"<20240528161126.35119-1-mzamazal@redhat.com>","References":"<20240528161126.35119-1-mzamazal@redhat.com>","MIME-Version":"1.0","X-Scanned-By":"MIMEDefang 3.4.1 on 10.11.54.8","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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"Constructing the color mapping tables is related to stats rather than\ndebayering, where they are applied. Let's move the corresponding code\nto stats processing.\n\nThis is a preliminary step towards building this functionality on top of\nlibipa/algorithm.h, which should follow.\n\nSigned-off-by: Milan Zamazal \nReviewed-by: Andrei Konovalov \n---\n .../internal/software_isp/debayer_params.h | 19 +++----\n src/ipa/simple/soft_simple.cpp | 50 +++++++++++++++----\n src/libcamera/software_isp/debayer.cpp | 34 +++++++------\n src/libcamera/software_isp/debayer_cpu.cpp | 43 +++-------------\n src/libcamera/software_isp/debayer_cpu.h | 11 ++--\n src/libcamera/software_isp/software_isp.cpp | 15 ++++--\n 6 files changed, 92 insertions(+), 80 deletions(-)","diff":"diff --git a/include/libcamera/internal/software_isp/debayer_params.h b/include/libcamera/internal/software_isp/debayer_params.h\nindex ce1b5945..69fed1e7 100644\n--- a/include/libcamera/internal/software_isp/debayer_params.h\n+++ b/include/libcamera/internal/software_isp/debayer_params.h\n@@ -1,6 +1,6 @@\n /* SPDX-License-Identifier: LGPL-2.1-or-later */\n /*\n- * Copyright (C) 2023, Red Hat Inc.\n+ * Copyright (C) 2023, 2024 Red Hat Inc.\n *\n * Authors:\n * Hans de Goede \n@@ -10,20 +10,21 @@\n \n #pragma once\n \n+#include \n+#include \n+\n namespace libcamera {\n \n struct DebayerParams {\n \tstatic constexpr unsigned int kGain10 = 256;\n+\tstatic constexpr unsigned int kRGBLookupSize = 256;\n+\tstatic constexpr float kGamma = 0.5;\n \n-\tunsigned int gainR;\n-\tunsigned int gainG;\n-\tunsigned int gainB;\n+\tusing ColorLookupTable = std::array;\n \n-\tfloat gamma;\n-\t/**\n-\t * \\brief Level of the black point, 0..255, 0 is no correction.\n-\t */\n-\tunsigned int blackLevel;\n+\tColorLookupTable red;\n+\tColorLookupTable green;\n+\tColorLookupTable blue;\n };\n \n } /* namespace libcamera */\ndiff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\nindex 722aac83..2c9fe98e 100644\n--- a/src/ipa/simple/soft_simple.cpp\n+++ b/src/ipa/simple/soft_simple.cpp\n@@ -5,6 +5,7 @@\n * Simple Software Image Processing Algorithm module\n */\n \n+#include \n #include \n #include \n #include \n@@ -84,6 +85,10 @@ private:\n \tControlInfoMap sensorInfoMap_;\n \tBlackLevel blackLevel_;\n \n+\tstatic constexpr unsigned int kGammaLookupSize = 1024;\n+\tstd::array gammaTable_;\n+\tint lastBlackLevel_ = -1;\n+\n \tint32_t exposureMin_, exposureMax_;\n \tint32_t exposure_;\n \tdouble againMin_, againMax_, againMinStep_;\n@@ -246,7 +251,6 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)\n \tif (ignoreUpdates_ > 0)\n \t\tblackLevel_.update(histogram);\n \tconst uint8_t blackLevel = blackLevel_.get();\n-\tparams_->blackLevel = blackLevel;\n \n \t/*\n \t * Calculate red and blue gains for AWB.\n@@ -265,12 +269,40 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)\n \tconst uint64_t sumG = subtractBlackLevel(stats_->sumG_, 2);\n \tconst uint64_t sumB = subtractBlackLevel(stats_->sumB_, 4);\n \n-\tparams_->gainR = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;\n-\tparams_->gainB = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;\n-\n+\t/* Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc. */\n+\tconst unsigned int gainR = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;\n+\tconst unsigned int gainB = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;\n \t/* Green gain and gamma values are fixed */\n-\tparams_->gainG = 256;\n-\tparams_->gamma = 0.5;\n+\tconstexpr unsigned int gainG = 256;\n+\n+\t/* Update the gamma table if needed */\n+\tif (blackLevel != lastBlackLevel_) {\n+\t\tconst unsigned int blackIndex = blackLevel * kGammaLookupSize / 256;\n+\t\tstd::fill(gammaTable_.begin(), gammaTable_.begin() + blackIndex, 0);\n+\t\tconst float divisor = kGammaLookupSize - blackIndex - 1.0;\n+\t\tfor (unsigned int i = blackIndex; i < kGammaLookupSize; i++)\n+\t\t\tgammaTable_[i] = UINT8_MAX *\n+\t\t\t\t\t powf((i - blackIndex) / divisor, DebayerParams::kGamma);\n+\n+\t\tlastBlackLevel_ = blackLevel;\n+\t}\n+\n+\tfor (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {\n+\t\tconstexpr unsigned int div =\n+\t\t\tDebayerParams::kRGBLookupSize * DebayerParams::kGain10 /\n+\t\t\tkGammaLookupSize;\n+\t\tunsigned int idx;\n+\n+\t\t/* Apply gamma after gain! */\n+\t\tidx = std::min({ i * gainR / div, (kGammaLookupSize - 1) });\n+\t\tparams_->red[i] = gammaTable_[idx];\n+\n+\t\tidx = std::min({ i * gainG / div, (kGammaLookupSize - 1) });\n+\t\tparams_->green[i] = gammaTable_[idx];\n+\n+\t\tidx = std::min({ i * gainB / div, (kGammaLookupSize - 1) });\n+\t\tparams_->blue[i] = gammaTable_[idx];\n+\t}\n \n \tsetIspParams.emit();\n \n@@ -291,7 +323,7 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)\n \t * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf\n \t */\n \tconst unsigned int blackLevelHistIdx =\n-\t\tparams_->blackLevel / (256 / SwIspStats::kYHistogramSize);\n+\t\tblackLevel / (256 / SwIspStats::kYHistogramSize);\n \tconst unsigned int histogramSize =\n \t\tSwIspStats::kYHistogramSize - blackLevelHistIdx;\n \tconst unsigned int yHistValsPerBin = histogramSize / kExposureBinsCount;\n@@ -339,8 +371,8 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)\n \n \tLOG(IPASoft, Debug) << \"exposureMSV \" << exposureMSV\n \t\t\t << \" exp \" << exposure_ << \" again \" << again_\n-\t\t\t << \" gain R/B \" << params_->gainR << \"/\" << params_->gainB\n-\t\t\t << \" black level \" << params_->blackLevel;\n+\t\t\t << \" gain R/B \" << gainR << \"/\" << gainB\n+\t\t\t << \" black level \" << static_cast(blackLevel);\n }\n \n void IPASoftSimple::updateExposure(double exposureMSV)\ndiff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp\nindex efe75ea8..028bf27e 100644\n--- a/src/libcamera/software_isp/debayer.cpp\n+++ b/src/libcamera/software_isp/debayer.cpp\n@@ -1,7 +1,7 @@\n /* SPDX-License-Identifier: LGPL-2.1-or-later */\n /*\n * Copyright (C) 2023, Linaro Ltd\n- * Copyright (C) 2023, Red Hat Inc.\n+ * Copyright (C) 2023, 2024 Red Hat Inc.\n *\n * Authors:\n * Hans de Goede \n@@ -24,29 +24,33 @@ namespace libcamera {\n */\n \n /**\n- * \\var DebayerParams::gainR\n- * \\brief Red gain\n- *\n- * 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.\n+ * \\var DebayerParams::kRGBLookupSize\n+ * \\brief Size of a color lookup table\n */\n \n /**\n- * \\var DebayerParams::gainG\n- * \\brief Green gain\n- *\n- * 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.\n+ * \\var DebayerParams::kGamma\n+ * \\brief Gamma correction, 1.0 is no correction\n */\n \n /**\n- * \\var DebayerParams::gainB\n- * \\brief Blue gain\n- *\n- * 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.\n+ * \\typedef DebayerParams::ColorLookupTable\n+ * \\brief Type of the lookup tables for red, green, blue values\n */\n \n /**\n- * \\var DebayerParams::gamma\n- * \\brief Gamma correction, 1.0 is no correction\n+ * \\var DebayerParams::red\n+ * \\brief Lookup table for red color, mapping input values to output values\n+ */\n+\n+/**\n+ * \\var DebayerParams::green\n+ * \\brief Lookup table for green color, mapping input values to output values\n+ */\n+\n+/**\n+ * \\var DebayerParams::blue\n+ * \\brief Lookup table for blue color, mapping input values to output values\n */\n \n /**\ndiff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\nindex 8254bbe9..c038eed4 100644\n--- a/src/libcamera/software_isp/debayer_cpu.cpp\n+++ b/src/libcamera/software_isp/debayer_cpu.cpp\n@@ -11,7 +11,6 @@\n \n #include \"debayer_cpu.h\"\n \n-#include \n #include \n #include \n \n@@ -35,7 +34,7 @@ namespace libcamera {\n * \\param[in] stats Pointer to the stats object to use\n */\n DebayerCpu::DebayerCpu(std::unique_ptr stats)\n-\t: stats_(std::move(stats)), gammaCorrection_(1.0), blackLevel_(0)\n+\t: stats_(std::move(stats))\n {\n \t/*\n \t * Reading from uncached buffers may be very slow.\n@@ -47,9 +46,9 @@ DebayerCpu::DebayerCpu(std::unique_ptr stats)\n \t */\n \tenableInputMemcpy_ = true;\n \n-\t/* Initialize gamma to 1.0 curve */\n-\tfor (unsigned int i = 0; i < kGammaLookupSize; i++)\n-\t\tgamma_[i] = i / (kGammaLookupSize / kRGBLookupSize);\n+\t/* Initialize color lookup tables */\n+\tfor (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++)\n+\t\tred_[i] = green_[i] = blue_[i] = i;\n \n \tfor (unsigned int i = 0; i < kMaxLineBuffers; i++)\n \t\tlineBuffers_[i] = nullptr;\n@@ -698,37 +697,9 @@ void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams\n \t\tclock_gettime(CLOCK_MONOTONIC_RAW, &frameStartTime);\n \t}\n \n-\t/* Apply DebayerParams */\n-\tif (params.gamma != gammaCorrection_ || params.blackLevel != blackLevel_) {\n-\t\tconst unsigned int blackIndex =\n-\t\t\tparams.blackLevel * kGammaLookupSize / 256;\n-\t\tstd::fill(gamma_.begin(), gamma_.begin() + blackIndex, 0);\n-\t\tconst float divisor = kGammaLookupSize - blackIndex - 1.0;\n-\t\tfor (unsigned int i = blackIndex; i < kGammaLookupSize; i++)\n-\t\t\tgamma_[i] = UINT8_MAX * powf((i - blackIndex) / divisor, params.gamma);\n-\n-\t\tgammaCorrection_ = params.gamma;\n-\t\tblackLevel_ = params.blackLevel;\n-\t}\n-\n-\tif (swapRedBlueGains_)\n-\t\tstd::swap(params.gainR, params.gainB);\n-\n-\tfor (unsigned int i = 0; i < kRGBLookupSize; i++) {\n-\t\tconstexpr unsigned int div =\n-\t\t\tkRGBLookupSize * DebayerParams::kGain10 / kGammaLookupSize;\n-\t\tunsigned int idx;\n-\n-\t\t/* Apply gamma after gain! */\n-\t\tidx = std::min({ i * params.gainR / div, (kGammaLookupSize - 1) });\n-\t\tred_[i] = gamma_[idx];\n-\n-\t\tidx = std::min({ i * params.gainG / div, (kGammaLookupSize - 1) });\n-\t\tgreen_[i] = gamma_[idx];\n-\n-\t\tidx = std::min({ i * params.gainB / div, (kGammaLookupSize - 1) });\n-\t\tblue_[i] = gamma_[idx];\n-\t}\n+\tgreen_ = params.green;\n+\tred_ = swapRedBlueGains_ ? params.blue : params.red;\n+\tblue_ = swapRedBlueGains_ ? params.red : params.blue;\n \n \t/* Copy metadata from the input buffer */\n \tFrameMetadata &metadata = output->_d()->metadata();\ndiff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\nindex de216fe3..be7dcdca 100644\n--- a/src/libcamera/software_isp/debayer_cpu.h\n+++ b/src/libcamera/software_isp/debayer_cpu.h\n@@ -122,15 +122,12 @@ private:\n \tvoid process2(const uint8_t *src, uint8_t *dst);\n \tvoid process4(const uint8_t *src, uint8_t *dst);\n \n-\tstatic constexpr unsigned int kGammaLookupSize = 1024;\n-\tstatic constexpr unsigned int kRGBLookupSize = 256;\n \t/* Max. supported Bayer pattern height is 4, debayering this requires 5 lines */\n \tstatic constexpr unsigned int kMaxLineBuffers = 5;\n \n-\tstd::array gamma_;\n-\tstd::array red_;\n-\tstd::array green_;\n-\tstd::array blue_;\n+\tDebayerParams::ColorLookupTable red_;\n+\tDebayerParams::ColorLookupTable green_;\n+\tDebayerParams::ColorLookupTable blue_;\n \tdebayerFn debayer0_;\n \tdebayerFn debayer1_;\n \tdebayerFn debayer2_;\n@@ -146,8 +143,6 @@ private:\n \tunsigned int xShift_; /* Offset of 0/1 applied to window_.x */\n \tbool enableInputMemcpy_;\n \tbool swapRedBlueGains_;\n-\tfloat gammaCorrection_;\n-\tunsigned int blackLevel_;\n \tunsigned int measuredFrames_;\n \tint64_t frameProcessTime_;\n \t/* Skip 30 frames for things to stabilize then measure 30 frames */\ndiff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\nindex c9b6be56..84558c4e 100644\n--- a/src/libcamera/software_isp/software_isp.cpp\n+++ b/src/libcamera/software_isp/software_isp.cpp\n@@ -7,6 +7,7 @@\n \n #include \"libcamera/internal/software_isp/software_isp.h\"\n \n+#include \n #include \n #include \n #include \n@@ -18,6 +19,7 @@\n #include \"libcamera/internal/framebuffer.h\"\n #include \"libcamera/internal/ipa_manager.h\"\n #include \"libcamera/internal/mapped_framebuffer.h\"\n+#include \"libcamera/internal/software_isp/debayer_params.h\"\n \n #include \"debayer_cpu.h\"\n \n@@ -63,10 +65,17 @@ LOG_DEFINE_CATEGORY(SoftwareIsp)\n * handler\n */\n SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor)\n-\t: debayerParams_{ DebayerParams::kGain10, DebayerParams::kGain10,\n-\t\t\t DebayerParams::kGain10, 0.5f, 0 },\n-\t dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System)\n+\t: dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System)\n {\n+\tstd::array gammaTable;\n+\tfor (unsigned int i = 0; i < 256; i++)\n+\t\tgammaTable[i] = powf(i / 256.0, DebayerParams::kGamma);\n+\tfor (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {\n+\t\tdebayerParams_.red[i] = gammaTable[i];\n+\t\tdebayerParams_.green[i] = gammaTable[i];\n+\t\tdebayerParams_.blue[i] = gammaTable[i];\n+\t}\n+\n \tif (!dmaHeap_.isValid()) {\n \t\tLOG(SoftwareIsp, Error) << \"Failed to create DmaHeap object\";\n \t\treturn;\n","prefixes":["v4","3/5"]}