From patchwork Tue Mar 19 12:36:05 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 19775 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id EA651C3274 for ; Tue, 19 Mar 2024 12:37:46 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 930FD63055; Tue, 19 Mar 2024 13:37:46 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="VtBOWqGU"; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 33D1562D2E for ; Tue, 19 Mar 2024 13:37:45 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1710851864; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=L6QSVmfOdEG+8GxEYT4oUFQEHFx1W2n55DRmuzC8+Cs=; b=VtBOWqGU3PyqViarLvsMB8mgr5mYlPuiNF1Y02Ft4KT+NPHCd9aCs6o2grn73fa/g5yELC 58abGoUhLlRpzpPfPeL7Il7xY+oRRrIr5GMQwltyx/p4sc2nBR0QzwZ7usDVeQkjlhdxo8 MRo+sSTJxdijjOrjXoTMNd59rR43Gu8= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-550--NdhT1vhOJy1MmnHHFKE3w-1; Tue, 19 Mar 2024 08:37:40 -0400 X-MC-Unique: -NdhT1vhOJy1MmnHHFKE3w-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E69F4852000; Tue, 19 Mar 2024 12:37:39 +0000 (UTC) Received: from nuthatch.redhat.com (unknown [10.45.226.119]) by smtp.corp.redhat.com (Postfix) with ESMTP id BD9F31C060CE; Tue, 19 Mar 2024 12:37:37 +0000 (UTC) From: Milan Zamazal To: libcamera-devel@lists.libcamera.org Cc: Andrei Konovalov , Bryan O'Donoghue , Maxime Ripard , Milan Zamazal , Pavel Machek , Hans de Goede , Kieran Bingham Subject: [PATCH v6 18/18] libcamera: Soft IPA: use CameraSensorHelper for analogue gain Date: Tue, 19 Mar 2024 13:36:05 +0100 Message-ID: <20240319123622.675599-19-mzamazal@redhat.com> In-Reply-To: <20240319123622.675599-1-mzamazal@redhat.com> References: <20240319123622.675599-1-mzamazal@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 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Andrei Konovalov Use CameraSensorHelper to convert the analogue gain code read from the camera sensor into real analogue gain value. In the future this makes it possible to use faster AE/AGC algorithm. For now the same AE/AGC algorithm is used, but even then the CameraSensorHelper lets us use the full range of analogue gain values. If there is no CameraSensorHelper for the camera sensor in use, a warning log message is printed, and the AE/AGC works exactly as before this change. Signed-off-by: Andrei Konovalov Reviewed-by: Hans de Goede Signed-off-by: Hans de Goede Reviewed-by: Milan Zamazal --- .../internal/software_isp/software_isp.h | 3 +- src/ipa/simple/soft_simple.cpp | 77 ++++++++++++------- src/libcamera/pipeline/simple/simple.cpp | 2 +- src/libcamera/software_isp/software_isp.cpp | 8 +- 4 files changed, 57 insertions(+), 33 deletions(-) diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h index 8d25e979..2a6db7ba 100644 --- a/include/libcamera/internal/software_isp/software_isp.h +++ b/include/libcamera/internal/software_isp/software_isp.h @@ -26,6 +26,7 @@ #include #include +#include "libcamera/internal/camera_sensor.h" #include "libcamera/internal/dma_heaps.h" #include "libcamera/internal/pipeline_handler.h" #include "libcamera/internal/shared_mem_object.h" @@ -43,7 +44,7 @@ LOG_DECLARE_CATEGORY(SoftwareIsp) class SoftwareIsp { public: - SoftwareIsp(PipelineHandler *pipe, const ControlInfoMap &sensorControls); + SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor); ~SoftwareIsp(); int loadConfiguration([[maybe_unused]] const std::string &filename) { return 0; } diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index ac027568..e4d64762 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -22,6 +22,8 @@ #include "libcamera/internal/software_isp/debayer_params.h" #include "libcamera/internal/software_isp/swisp_stats.h" +#include "libipa/camera_sensor_helper.h" + #include "black_level.h" namespace libcamera { @@ -67,18 +69,27 @@ private: DebayerParams *params_; SwIspStats *stats_; BlackLevel blackLevel_; + std::unique_ptr camHelper_; int32_t exposure_min_, exposure_max_; - int32_t again_min_, again_max_; - int32_t again_, exposure_; + int32_t exposure_; + double again_min_, again_max_, againMinStep_; + double again_; unsigned int ignore_updates_; }; -int IPASoftSimple::init([[maybe_unused]] const IPASettings &settings, +int IPASoftSimple::init(const IPASettings &settings, const SharedFD &fdStats, const SharedFD &fdParams, const ControlInfoMap &sensorInfoMap) { + camHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel); + if (camHelper_ == nullptr) { + LOG(IPASoft, Warning) + << "Failed to create camera sensor helper for " + << settings.sensorModel; + } + fdStats_ = fdStats; if (!fdStats_.isValid()) { LOG(IPASoft, Error) << "Invalid Statistics handle"; @@ -132,25 +143,35 @@ int IPASoftSimple::configure(const ControlInfoMap &sensorInfoMap) exposure_min_ = 1; } - again_min_ = gain_info.min().get(); - again_max_ = gain_info.max().get(); - /* - * The camera sensor gain (g) is usually not equal to the value written - * into the gain register (x). But the way how the AGC algorithm changes - * the gain value to make the total exposure closer to the optimum assumes - * that g(x) is not too far from linear function. If the minimal gain is 0, - * the g(x) is likely to be far from the linear, like g(x) = a / (b * x + c). - * To avoid unexpected changes to the gain by the AGC algorithm (abrupt near - * one edge, and very small near the other) we limit the range of the gain - * values used. - */ - if (!again_min_) { - LOG(IPASoft, Warning) << "Minimum gain is zero, that can't be linear"; - again_min_ = std::min(100, again_min_ / 2 + again_max_ / 2); + int32_t again_min = gain_info.min().get(); + int32_t again_max = gain_info.max().get(); + + if (camHelper_) { + again_min_ = camHelper_->gain(again_min); + again_max_ = camHelper_->gain(again_max); + againMinStep_ = (again_max_ - again_min_) / 100.0; + } else { + /* + * The camera sensor gain (g) is usually not equal to the value written + * into the gain register (x). But the way how the AGC algorithm changes + * the gain value to make the total exposure closer to the optimum assumes + * that g(x) is not too far from linear function. If the minimal gain is 0, + * the g(x) is likely to be far from the linear, like g(x) = a / (b * x + c). + * To avoid unexpected changes to the gain by the AGC algorithm (abrupt near + * one edge, and very small near the other) we limit the range of the gain + * values used. + */ + again_max_ = again_max; + if (!again_min) { + LOG(IPASoft, Warning) << "Minimum gain is zero, that can't be linear"; + again_min_ = std::min(100, again_min / 2 + again_max / 2); + } + againMinStep_ = 1.0; } LOG(IPASoft, Info) << "Exposure " << exposure_min_ << "-" << exposure_max_ - << ", gain " << again_min_ << "-" << again_max_; + << ", gain " << again_min_ << "-" << again_max_ + << " (" << againMinStep_ << ")"; return 0; } @@ -252,12 +273,14 @@ void IPASoftSimple::processStats(const ControlList &sensorControls) ControlList ctrls(sensorControls); exposure_ = ctrls.get(V4L2_CID_EXPOSURE).get(); - again_ = ctrls.get(V4L2_CID_ANALOGUE_GAIN).get(); + int32_t again = ctrls.get(V4L2_CID_ANALOGUE_GAIN).get(); + again_ = camHelper_ ? camHelper_->gain(again) : again; updateExposure(exposureMSV); ctrls.set(V4L2_CID_EXPOSURE, exposure_); - ctrls.set(V4L2_CID_ANALOGUE_GAIN, again_); + ctrls.set(V4L2_CID_ANALOGUE_GAIN, + static_cast(camHelper_ ? camHelper_->gainCode(again_) : again_)); ignore_updates_ = 2; @@ -276,7 +299,7 @@ void IPASoftSimple::updateExposure(double exposureMSV) static constexpr uint8_t kExpNumeratorUp = kExpDenominator + 1; static constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1; - int next; + double next; if (exposureMSV < kExposureOptimal - kExposureSatisfactory) { next = exposure_ * kExpNumeratorUp / kExpDenominator; @@ -286,18 +309,18 @@ void IPASoftSimple::updateExposure(double exposureMSV) exposure_ = next; if (exposure_ >= exposure_max_) { next = again_ * kExpNumeratorUp / kExpDenominator; - if (next - again_ < 1) - again_ += 1; + if (next - again_ < againMinStep_) + again_ += againMinStep_; else again_ = next; } } if (exposureMSV > kExposureOptimal + kExposureSatisfactory) { - if (exposure_ == exposure_max_ && again_ != again_min_) { + if (exposure_ == exposure_max_ && again_ > again_min_) { next = again_ * kExpNumeratorDown / kExpDenominator; - if (again_ - next < 1) - again_ -= 1; + if (again_ - next < againMinStep_) + again_ -= againMinStep_; else again_ = next; } else { diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index ac796b9b..22639407 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -526,7 +526,7 @@ int SimpleCameraData::init() * Instantiate Soft ISP if this is enabled for the given driver and no converter is used. */ if (!converter_ && pipe->swIspEnabled()) { - swIsp_ = std::make_unique(pipe, sensor_->controls()); + swIsp_ = std::make_unique(pipe, sensor_.get()); if (!swIsp_->isValid()) { LOG(SimplePipeline, Warning) << "Failed to create software ISP, disabling software debayering"; diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp index 9b49be41..ea4d96e4 100644 --- a/src/libcamera/software_isp/software_isp.cpp +++ b/src/libcamera/software_isp/software_isp.cpp @@ -60,9 +60,9 @@ LOG_DEFINE_CATEGORY(SoftwareIsp) /** * \brief Constructs SoftwareIsp object * \param[in] pipe The pipeline handler in use - * \param[in] sensorControls ControlInfoMap describing the controls supported by the sensor + * \param[in] sensor Pointer to the CameraSensor instance owned by the pipeline handler */ -SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const ControlInfoMap &sensorControls) +SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor) : debayer_(nullptr), debayerParams_{ DebayerParams::kGain10, DebayerParams::kGain10, DebayerParams::kGain10, 0.5f, 0 }, dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System) @@ -97,10 +97,10 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const ControlInfoMap &sensorCont return; } - int ret = ipa_->init(IPASettings{ "No cfg file", "No sensor model" }, + int ret = ipa_->init(IPASettings{ "No cfg file", sensor->model() }, debayer_->getStatsFD(), sharedParams_.fd(), - sensorControls); + sensor->controls()); if (ret) { LOG(SoftwareIsp, Error) << "IPA init failed"; debayer_.reset();