{"id":19680,"url":"https://patchwork.libcamera.org/api/patches/19680/?format=json","web_url":"https://patchwork.libcamera.org/patch/19680/","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":"<20240311141524.27192-19-hdegoede@redhat.com>","date":"2024-03-11T14:15:22","name":"[v5,18/18] libcamera: Soft IPA: use CameraSensorHelper for analogue gain","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"ac5b72cb3c3f2740d625541ef3624b8db10f7d10","submitter":{"id":102,"url":"https://patchwork.libcamera.org/api/people/102/?format=json","name":"Hans de Goede","email":"hdegoede@redhat.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/19680/mbox/","series":[{"id":4214,"url":"https://patchwork.libcamera.org/api/series/4214/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4214","date":"2024-03-11T14:15:04","name":"libcamera: introduce Software ISP and Software IPA","version":5,"mbox":"https://patchwork.libcamera.org/series/4214/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/19680/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/19680/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 55101C32A3\n\tfor ;\n\tMon, 11 Mar 2024 14:16:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 029C462C89;\n\tMon, 11 Mar 2024 15:16:04 +0100 (CET)","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 CA22A62CA1\n\tfor ;\n\tMon, 11 Mar 2024 15:15:58 +0100 (CET)","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-235-j2zkvOkxM969qVLj5LUJkw-1; Mon, 11 Mar 2024 10:15:54 -0400","from smtp.corp.redhat.com\n\t(int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7])\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 01A6287280C;\n\tMon, 11 Mar 2024 14:15:54 +0000 (UTC)","from x1.localdomain.com (unknown [10.39.195.37])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 0EC641C060D1;\n\tMon, 11 Mar 2024 14:15:52 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"NOKSXJ/3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1710166557;\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=dSqj41lPffk+1GEK+NK9Z/KZ26uEtNk2zpn4m0TKMHw=;\n\tb=NOKSXJ/3nZndz9Z012HyVvewngdOwUtZ3sTxh6JsOaEY8UNlpX+pF5pvoVWczNXFbU97ht\n\tv/Zo79l8ZthALt3w4zV/n3KN0GQXQo6N3RU9QSIKXRMg7mfczfizgCbNeIRxQ5XxmJe3YX\n\tt0/H5sgIRR1u0UFeK9+SmRa3gePTKVc=","X-MC-Unique":"j2zkvOkxM969qVLj5LUJkw-1","From":"Hans de Goede ","To":"libcamera-devel@lists.libcamera.org","Subject":"[PATCH v5 18/18] libcamera: Soft IPA: use CameraSensorHelper for\n\tanalogue gain","Date":"Mon, 11 Mar 2024 15:15:22 +0100","Message-ID":"<20240311141524.27192-19-hdegoede@redhat.com>","In-Reply-To":"<20240311141524.27192-1-hdegoede@redhat.com>","References":"<20240311141524.27192-1-hdegoede@redhat.com>","MIME-Version":"1.0","X-Scanned-By":"MIMEDefang 3.4.1 on 10.11.54.7","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","Cc":"Maxime Ripard , Pavel Machek ,\n\tBryan O'Donoghue ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"From: Andrei Konovalov \n\nUse CameraSensorHelper to convert the analogue gain code read from the\ncamera sensor into real analogue gain value. In the future this makes\nit possible to use faster AE/AGC algorithm. For now the same AE/AGC\nalgorithm is used, but even then the CameraSensorHelper lets us use the\nfull range of analogue gain values.\n\nIf there is no CameraSensorHelper for the camera sensor in use, a\nwarning log message is printed, and the AE/AGC works exactly as before\nthis change.\n\nSigned-off-by: Andrei Konovalov \nReviewed-by: Hans de Goede \nSigned-off-by: Hans de Goede \n---\n .../internal/software_isp/software_isp.h | 3 +-\n src/ipa/simple/soft_simple.cpp | 77 ++++++++++++-------\n src/libcamera/pipeline/simple/simple.cpp | 2 +-\n src/libcamera/software_isp/software_isp.cpp | 8 +-\n 4 files changed, 57 insertions(+), 33 deletions(-)","diff":"diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h\nindex 8d25e979..2a6db7ba 100644\n--- a/include/libcamera/internal/software_isp/software_isp.h\n+++ b/include/libcamera/internal/software_isp/software_isp.h\n@@ -26,6 +26,7 @@\n #include \n #include \n \n+#include \"libcamera/internal/camera_sensor.h\"\n #include \"libcamera/internal/dma_heaps.h\"\n #include \"libcamera/internal/pipeline_handler.h\"\n #include \"libcamera/internal/shared_mem_object.h\"\n@@ -43,7 +44,7 @@ LOG_DECLARE_CATEGORY(SoftwareIsp)\n class SoftwareIsp\n {\n public:\n-\tSoftwareIsp(PipelineHandler *pipe, const ControlInfoMap &sensorControls);\n+\tSoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor);\n \t~SoftwareIsp();\n \n \tint loadConfiguration([[maybe_unused]] const std::string &filename) { return 0; }\ndiff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\nindex ac027568..e4d64762 100644\n--- a/src/ipa/simple/soft_simple.cpp\n+++ b/src/ipa/simple/soft_simple.cpp\n@@ -22,6 +22,8 @@\n #include \"libcamera/internal/software_isp/debayer_params.h\"\n #include \"libcamera/internal/software_isp/swisp_stats.h\"\n \n+#include \"libipa/camera_sensor_helper.h\"\n+\n #include \"black_level.h\"\n \n namespace libcamera {\n@@ -67,18 +69,27 @@ private:\n \tDebayerParams *params_;\n \tSwIspStats *stats_;\n \tBlackLevel blackLevel_;\n+\tstd::unique_ptr camHelper_;\n \n \tint32_t exposure_min_, exposure_max_;\n-\tint32_t again_min_, again_max_;\n-\tint32_t again_, exposure_;\n+\tint32_t exposure_;\n+\tdouble again_min_, again_max_, againMinStep_;\n+\tdouble again_;\n \tunsigned int ignore_updates_;\n };\n \n-int IPASoftSimple::init([[maybe_unused]] const IPASettings &settings,\n+int IPASoftSimple::init(const IPASettings &settings,\n \t\t\tconst SharedFD &fdStats,\n \t\t\tconst SharedFD &fdParams,\n \t\t\tconst ControlInfoMap &sensorInfoMap)\n {\n+\tcamHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);\n+\tif (camHelper_ == nullptr) {\n+\t\tLOG(IPASoft, Warning)\n+\t\t\t<< \"Failed to create camera sensor helper for \"\n+\t\t\t<< settings.sensorModel;\n+\t}\n+\n \tfdStats_ = fdStats;\n \tif (!fdStats_.isValid()) {\n \t\tLOG(IPASoft, Error) << \"Invalid Statistics handle\";\n@@ -132,25 +143,35 @@ int IPASoftSimple::configure(const ControlInfoMap &sensorInfoMap)\n \t\texposure_min_ = 1;\n \t}\n \n-\tagain_min_ = gain_info.min().get();\n-\tagain_max_ = gain_info.max().get();\n-\t/*\n-\t * The camera sensor gain (g) is usually not equal to the value written\n-\t * into the gain register (x). But the way how the AGC algorithm changes\n-\t * the gain value to make the total exposure closer to the optimum assumes\n-\t * that g(x) is not too far from linear function. If the minimal gain is 0,\n-\t * the g(x) is likely to be far from the linear, like g(x) = a / (b * x + c).\n-\t * To avoid unexpected changes to the gain by the AGC algorithm (abrupt near\n-\t * one edge, and very small near the other) we limit the range of the gain\n-\t * values used.\n-\t */\n-\tif (!again_min_) {\n-\t\tLOG(IPASoft, Warning) << \"Minimum gain is zero, that can't be linear\";\n-\t\tagain_min_ = std::min(100, again_min_ / 2 + again_max_ / 2);\n+\tint32_t again_min = gain_info.min().get();\n+\tint32_t again_max = gain_info.max().get();\n+\n+\tif (camHelper_) {\n+\t\tagain_min_ = camHelper_->gain(again_min);\n+\t\tagain_max_ = camHelper_->gain(again_max);\n+\t\tagainMinStep_ = (again_max_ - again_min_) / 100.0;\n+\t} else {\n+\t\t/*\n+\t\t * The camera sensor gain (g) is usually not equal to the value written\n+\t\t * into the gain register (x). But the way how the AGC algorithm changes\n+\t\t * the gain value to make the total exposure closer to the optimum assumes\n+\t\t * that g(x) is not too far from linear function. If the minimal gain is 0,\n+\t\t * the g(x) is likely to be far from the linear, like g(x) = a / (b * x + c).\n+\t\t * To avoid unexpected changes to the gain by the AGC algorithm (abrupt near\n+\t\t * one edge, and very small near the other) we limit the range of the gain\n+\t\t * values used.\n+\t\t */\n+\t\tagain_max_ = again_max;\n+\t\tif (!again_min) {\n+\t\t\tLOG(IPASoft, Warning) << \"Minimum gain is zero, that can't be linear\";\n+\t\t\tagain_min_ = std::min(100, again_min / 2 + again_max / 2);\n+\t\t}\n+\t\tagainMinStep_ = 1.0;\n \t}\n \n \tLOG(IPASoft, Info) << \"Exposure \" << exposure_min_ << \"-\" << exposure_max_\n-\t\t\t << \", gain \" << again_min_ << \"-\" << again_max_;\n+\t\t\t << \", gain \" << again_min_ << \"-\" << again_max_\n+\t\t\t << \" (\" << againMinStep_ << \")\";\n \n \treturn 0;\n }\n@@ -252,12 +273,14 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)\n \tControlList ctrls(sensorControls);\n \n \texposure_ = ctrls.get(V4L2_CID_EXPOSURE).get();\n-\tagain_ = ctrls.get(V4L2_CID_ANALOGUE_GAIN).get();\n+\tint32_t again = ctrls.get(V4L2_CID_ANALOGUE_GAIN).get();\n+\tagain_ = camHelper_ ? camHelper_->gain(again) : again;\n \n \tupdateExposure(exposureMSV);\n \n \tctrls.set(V4L2_CID_EXPOSURE, exposure_);\n-\tctrls.set(V4L2_CID_ANALOGUE_GAIN, again_);\n+\tctrls.set(V4L2_CID_ANALOGUE_GAIN,\n+\t\t static_cast(camHelper_ ? camHelper_->gainCode(again_) : again_));\n \n \tignore_updates_ = 2;\n \n@@ -276,7 +299,7 @@ void IPASoftSimple::updateExposure(double exposureMSV)\n \tstatic constexpr uint8_t kExpNumeratorUp = kExpDenominator + 1;\n \tstatic constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1;\n \n-\tint next;\n+\tdouble next;\n \n \tif (exposureMSV < kExposureOptimal - kExposureSatisfactory) {\n \t\tnext = exposure_ * kExpNumeratorUp / kExpDenominator;\n@@ -286,18 +309,18 @@ void IPASoftSimple::updateExposure(double exposureMSV)\n \t\t\texposure_ = next;\n \t\tif (exposure_ >= exposure_max_) {\n \t\t\tnext = again_ * kExpNumeratorUp / kExpDenominator;\n-\t\t\tif (next - again_ < 1)\n-\t\t\t\tagain_ += 1;\n+\t\t\tif (next - again_ < againMinStep_)\n+\t\t\t\tagain_ += againMinStep_;\n \t\t\telse\n \t\t\t\tagain_ = next;\n \t\t}\n \t}\n \n \tif (exposureMSV > kExposureOptimal + kExposureSatisfactory) {\n-\t\tif (exposure_ == exposure_max_ && again_ != again_min_) {\n+\t\tif (exposure_ == exposure_max_ && again_ > again_min_) {\n \t\t\tnext = again_ * kExpNumeratorDown / kExpDenominator;\n-\t\t\tif (again_ - next < 1)\n-\t\t\t\tagain_ -= 1;\n+\t\t\tif (again_ - next < againMinStep_)\n+\t\t\t\tagain_ -= againMinStep_;\n \t\t\telse\n \t\t\t\tagain_ = next;\n \t\t} else {\ndiff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\nindex e491ab62..32b56c57 100644\n--- a/src/libcamera/pipeline/simple/simple.cpp\n+++ b/src/libcamera/pipeline/simple/simple.cpp\n@@ -526,7 +526,7 @@ int SimpleCameraData::init()\n \t * Instantiate Soft ISP if this is enabled for the given driver and no converter is used.\n \t */\n \tif (!converter_ && pipe->swIspEnabled()) {\n-\t\tswIsp_ = std::make_unique(pipe, sensor_->controls());\n+\t\tswIsp_ = std::make_unique(pipe, sensor_.get());\n \t\tif (!swIsp_->isValid()) {\n \t\t\tLOG(SimplePipeline, Warning)\n \t\t\t\t<< \"Failed to create software ISP, disabling software debayering\";\ndiff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\nindex 9b49be41..ea4d96e4 100644\n--- a/src/libcamera/software_isp/software_isp.cpp\n+++ b/src/libcamera/software_isp/software_isp.cpp\n@@ -60,9 +60,9 @@ LOG_DEFINE_CATEGORY(SoftwareIsp)\n /**\n * \\brief Constructs SoftwareIsp object\n * \\param[in] pipe The pipeline handler in use\n- * \\param[in] sensorControls ControlInfoMap describing the controls supported by the sensor\n+ * \\param[in] sensor Pointer to the CameraSensor instance owned by the pipeline handler\n */\n-SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const ControlInfoMap &sensorControls)\n+SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor)\n \t: debayer_(nullptr),\n \t debayerParams_{ DebayerParams::kGain10, DebayerParams::kGain10, DebayerParams::kGain10, 0.5f, 0 },\n \t dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System)\n@@ -97,10 +97,10 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const ControlInfoMap &sensorCont\n \t\treturn;\n \t}\n \n-\tint ret = ipa_->init(IPASettings{ \"No cfg file\", \"No sensor model\" },\n+\tint ret = ipa_->init(IPASettings{ \"No cfg file\", sensor->model() },\n \t\t\t debayer_->getStatsFD(),\n \t\t\t sharedParams_.fd(),\n-\t\t\t sensorControls);\n+\t\t\t sensor->controls());\n \tif (ret) {\n \t\tLOG(SoftwareIsp, Error) << \"IPA init failed\";\n \t\tdebayer_.reset();\n","prefixes":["v5","18/18"]}