{"id":20155,"url":"https://patchwork.libcamera.org/api/patches/20155/?format=json","web_url":"https://patchwork.libcamera.org/patch/20155/","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":"<20240530210957.2211897-4-mzamazal@redhat.com>","date":"2024-05-30T21:09:55","name":"[v5,3/5] libcamera: software_isp: Move color mappings out of debayering","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"025fae826bbb5a8bb800a622ab80aff3f6c16a72","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/20155/mbox/","series":[{"id":4342,"url":"https://patchwork.libcamera.org/api/series/4342/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4342","date":"2024-05-30T21:09:52","name":"Software ISP levels cleanup","version":5,"mbox":"https://patchwork.libcamera.org/series/4342/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/20155/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/20155/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 6E9B0C32C8\n\tfor ;\n\tThu, 30 May 2024 21:10:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D0233634C0;\n\tThu, 30 May 2024 23:10:27 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2538D634BC\n\tfor ;\n\tThu, 30 May 2024 23:10:24 +0200 (CEST)","from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73])\n\tby relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3,\n\tcipher=TLS_AES_256_GCM_SHA384) id us-mta-41-Vdut7ObSP86I-Ay17_m7zA-1;\n\tThu, 30 May 2024 17:10:19 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2])\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 563D13806700;\n\tThu, 30 May 2024 21:10:19 +0000 (UTC)","from nuthatch.redhat.com (unknown [10.45.224.24])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id F407840C6EB7;\n\tThu, 30 May 2024 21:10:17 +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=\"KD3KaCVz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1717103423;\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=2KIeslmbtQb94ed+BGAptnB0kbr3TMxKmCBdv3+0szo=;\n\tb=KD3KaCVzazpfCE8PkeZGaHWiQwwTzJ3p2J59hoEXw5/xIoRJivgAcfLLOW7JU1J7wqhNPz\n\tpkExGf212VDBGOo8IoLv13Qnfc3yYYySjmU5pQEIocVuXpVoxRUQxxyEFlBZC64goc3sDC\n\t1SrAF7JIPLNGMRnQCdt0WzBD/GByNpM=","X-MC-Unique":"Vdut7ObSP86I-Ay17_m7zA-1","From":"Milan Zamazal ","To":"libcamera-devel@lists.libcamera.org","Cc":"Milan Zamazal ,\n\tKieran Bingham ,\n\tAndrei Konovalov ,\n\tLaurent Pinchart ","Subject":"[PATCH v5 3/5] libcamera: software_isp: Move color mappings out of\n\tdebayering","Date":"Thu, 30 May 2024 23:09:55 +0200","Message-ID":"<20240530210957.2211897-4-mzamazal@redhat.com>","In-Reply-To":"<20240530210957.2211897-1-mzamazal@redhat.com>","References":"<20240530210957.2211897-1-mzamazal@redhat.com>","MIME-Version":"1.0","X-Scanned-By":"MIMEDefang 3.4.1 on 10.11.54.2","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\nThe same applies to the auxiliary gamma table. As the gamma value is\ncurrently fixed and used in a single place, with the temporary exception\nmentioned below, there is no need to share it anywhere anymore.\n\nIt's necessary to initialize SoftwareIsp::debayerParams_ to default\nvalues. These initial values are used for the first two frames, before\nthey are changed based on determined stats. To avoid sharing the gamma\nvalue constant in artificial ways, we use 0.5 directly in the\ninitialization. This all is not a particularly elegant thing to do,\nsuch a code belongs conceptually to the similar code in stats\nprocessing, but doing better is left for larger refactoring.\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 | 18 +++----\n src/ipa/simple/soft_simple.cpp | 51 +++++++++++++++----\n src/libcamera/software_isp/debayer.cpp | 29 +++++------\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 | 16 ++++--\n 6 files changed, 88 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..463d24b3 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,20 @@\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 \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 f383f994..3388686f 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 * Black level must be subtracted to get the correct AWB ratios,\n@@ -263,13 +267,42 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)\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-\tparams_->gainR = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;\n-\tparams_->gainB = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;\n-\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\tconstexpr float gamma = 0.5;\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 std::pow((i - blackIndex) / divisor, gamma);\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@@ -290,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@@ -338,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..3f3969f7 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,28 @@ 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+ * \\typedef DebayerParams::ColorLookupTable\n+ * \\brief Type of the lookup tables for red, green, blue values\n */\n \n /**\n- * \\var DebayerParams::gainB\n- * \\brief Blue gain\n- *\n- * 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.\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::gamma\n- * \\brief Gamma correction, 1.0 is no correction\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..2cde4f28 100644\n--- a/src/libcamera/software_isp/software_isp.cpp\n+++ b/src/libcamera/software_isp/software_isp.cpp\n@@ -7,6 +7,8 @@\n \n #include \"libcamera/internal/software_isp/software_isp.h\"\n \n+#include \n+#include \n #include \n #include \n #include \n@@ -18,6 +20,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 +66,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] = UINT8_MAX * std::pow(i / 256.0, 0.5);\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":["v5","3/5"]}