From patchwork Thu Apr 18 12:46:30 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Scally X-Patchwork-Id: 19905 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 94854C328D for ; Thu, 18 Apr 2024 12:46:49 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 4D36E633F3; Thu, 18 Apr 2024 14:46:45 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="WuGOcTNZ"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0EF586334D for ; Thu, 18 Apr 2024 14:46:43 +0200 (CEST) Received: from mail.ideasonboard.com (cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 4BA822BC; Thu, 18 Apr 2024 14:45:55 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1713444355; bh=80DQrLkvj+0my+RFvhP2MkH9Q9t1J3msZ2FZzz7B7Is=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=WuGOcTNZlzqZFngDC67VJp3dS30F6Wm/VmlGP7YX+FYEbk1ioDkm/9r75d4IHQmzr 7VcccO5dWPS8NO4gzTCT7g9IxyNZ94k+uu1wy6BaSJ3F3B91OvpfiVrxe/7wAFJtzy NTzIiTkTnLv9I502qgisKFBcKNdt2xqFRzZ29iwc= From: Daniel Scally To: libcamera-devel@lists.libcamera.org Cc: Daniel Scally Subject: [PATCH 1/3] ipa: ipu3: Refer to integration time instead of shutter speed Date: Thu, 18 Apr 2024 13:46:30 +0100 Message-Id: <20240418124632.652163-2-dan.scally@ideasonboard.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240418124632.652163-1-dan.scally@ideasonboard.com> References: <20240418124632.652163-1-dan.scally@ideasonboard.com> MIME-Version: 1.0 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" Within the IPU3 IPA module there are multiple references to min and max shutter speeds. In calculation and usage however those variables reflect integration time rather than shutter speed - this difference in terminology is particularly problematic because the minimum shutter speed is equivalent to the maximum integration time. Replace references to shutter speed with integration time. Signed-off-by: Daniel Scally Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/ipa/ipu3/algorithms/agc.cpp | 12 ++++++------ src/ipa/ipu3/algorithms/agc.h | 4 ++-- src/ipa/ipu3/ipa_context.cpp | 8 ++++---- src/ipa/ipu3/ipa_context.h | 4 ++-- src/ipa/ipu3/ipu3.cpp | 8 ++++---- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index 46fc3b33..a59b73fb 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -51,7 +51,7 @@ LOG_DEFINE_CATEGORY(IPU3Agc) static constexpr double kMinAnalogueGain = 1.0; /* \todo Honour the FrameDurationLimits control instead of hardcoding a limit */ -static constexpr utils::Duration kMaxShutterSpeed = 60ms; +static constexpr utils::Duration kMaxIntegrationTime = 60ms; /* Histogram constants */ static constexpr uint32_t knumHistogramBins = 256; @@ -71,7 +71,7 @@ static constexpr uint32_t kNumStartupFrames = 10; static constexpr double kRelativeLuminanceTarget = 0.16; Agc::Agc() - : minShutterSpeed_(0s), maxShutterSpeed_(0s) + : minIntegrationTime_(0s), maxIntegrationTime_(0s) { } @@ -114,9 +114,9 @@ int Agc::configure(IPAContext &context, stride_ = configuration.grid.stride; bdsGrid_ = configuration.grid.bdsGrid; - minShutterSpeed_ = configuration.agc.minShutterSpeed; - maxShutterSpeed_ = std::min(configuration.agc.maxShutterSpeed, - kMaxShutterSpeed); + minIntegrationTime_ = configuration.agc.minIntegrationTime; + maxIntegrationTime_ = std::min(configuration.agc.maxIntegrationTime, + kMaxIntegrationTime); minAnalogueGain_ = std::max(configuration.agc.minAnalogueGain, kMinAnalogueGain); maxAnalogueGain_ = configuration.agc.maxAnalogueGain; @@ -129,7 +129,7 @@ int Agc::configure(IPAContext &context, context.activeState.agc.exposureMode = exposureModeHelpers().begin()->first; /* \todo Run this again when FrameDurationLimits is passed in */ - configureExposureModeHelpers(minShutterSpeed_, maxShutterSpeed_, + configureExposureModeHelpers(minIntegrationTime_, maxIntegrationTime_, minAnalogueGain_, maxAnalogueGain_); resetFrameCount(); diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h index 945d1846..631dea52 100644 --- a/src/ipa/ipu3/algorithms/agc.h +++ b/src/ipa/ipu3/algorithms/agc.h @@ -42,8 +42,8 @@ private: Histogram parseStatistics(const ipu3_uapi_stats_3a *stats, const ipu3_uapi_grid_config &grid); - utils::Duration minShutterSpeed_; - utils::Duration maxShutterSpeed_; + utils::Duration minIntegrationTime_; + utils::Duration maxIntegrationTime_; double minAnalogueGain_; double maxAnalogueGain_; diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp index c4fb5642..72f7cec9 100644 --- a/src/ipa/ipu3/ipa_context.cpp +++ b/src/ipa/ipu3/ipa_context.cpp @@ -92,11 +92,11 @@ namespace libcamera::ipa::ipu3 { * \var IPASessionConfiguration::agc * \brief AGC parameters configuration of the IPA * - * \var IPASessionConfiguration::agc.minShutterSpeed - * \brief Minimum shutter speed supported with the configured sensor + * \var IPASessionConfiguration::agc.minIntegrationTime + * \brief Minimum integration time supported with the configured sensor * - * \var IPASessionConfiguration::agc.maxShutterSpeed - * \brief Maximum shutter speed supported with the configured sensor + * \var IPASessionConfiguration::agc.maxIntegrationTime + * \brief Maximum integration time supported with the configured sensor * * \var IPASessionConfiguration::agc.minAnalogueGain * \brief Minimum analogue gain supported with the configured sensor diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h index a92cb6ce..6b1ffdd1 100644 --- a/src/ipa/ipu3/ipa_context.h +++ b/src/ipa/ipu3/ipa_context.h @@ -33,8 +33,8 @@ struct IPASessionConfiguration { } af; struct { - utils::Duration minShutterSpeed; - utils::Duration maxShutterSpeed; + utils::Duration minIntegrationTime; + utils::Duration maxIntegrationTime; double minAnalogueGain; double maxAnalogueGain; } agc; diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 4809eb60..f13cc394 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -217,13 +217,13 @@ void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls) /* * When the AGC computes the new exposure values for a frame, it needs - * to know the limits for shutter speed and analogue gain. + * to know the limits for integration time and analogue gain. * As it depends on the sensor, update it with the controls. * - * \todo take VBLANK into account for maximum shutter speed + * \todo take VBLANK into account for maximum integration time */ - context_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration; - context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration; + context_.configuration.agc.minIntegrationTime = minExposure * context_.configuration.sensor.lineDuration; + context_.configuration.agc.maxIntegrationTime = maxExposure * context_.configuration.sensor.lineDuration; context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain); context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain); } From patchwork Thu Apr 18 12:46:31 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Scally X-Patchwork-Id: 19906 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 AF406C3200 for ; Thu, 18 Apr 2024 12:46:50 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5ED62633F9; Thu, 18 Apr 2024 14:46:46 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Mqgvn4c4"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 42574633EE for ; Thu, 18 Apr 2024 14:46:43 +0200 (CEST) Received: from mail.ideasonboard.com (cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 88A68802; Thu, 18 Apr 2024 14:45:55 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1713444355; bh=xX1X5JPvnzYyK5irf8neEPA3/Vdh8aMQLc3EHp32H+8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Mqgvn4c4rCdVP3swH5ZYW4uEfZZlTAZpR4TxCSk33DLbVyalgFEs1XX80gE+S38EO YY+50UCUexk3iXxJf20InU5O93mkADeajnFFLCEz4FYg/Ym67eC8NJDUxD0Tznh00+ W1LNxDMOSgnSYQsnfDtboVvnyPO+z19R0pi7s0XM= From: Daniel Scally To: libcamera-devel@lists.libcamera.org Cc: Daniel Scally Subject: [PATCH 2/3] ipa: rkisp1: Refer to integration time rather than shutter speed Date: Thu, 18 Apr 2024 13:46:31 +0100 Message-Id: <20240418124632.652163-3-dan.scally@ideasonboard.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240418124632.652163-1-dan.scally@ideasonboard.com> References: <20240418124632.652163-1-dan.scally@ideasonboard.com> MIME-Version: 1.0 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" Multiple variables referencing shutter speed in the RkISP1 IPA module are in fact calculated and used as integration time. The discrepancy is problematic given the minimum shutter speed would produce the max integration time. Replace references to shutter speed with integration time. Signed-off-by: Daniel Scally Reviewed-by: Paul Elder Reviewed-by: Laurent Pinchart --- src/ipa/rkisp1/algorithms/agc.cpp | 4 ++-- src/ipa/rkisp1/ipa_context.cpp | 8 ++++---- src/ipa/rkisp1/ipa_context.h | 4 ++-- src/ipa/rkisp1/rkisp1.cpp | 8 ++++---- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index 27b6f2c1..13f54398 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -95,8 +95,8 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) context.configuration.agc.measureWindow.v_size = 3 * configInfo.outputSize.height / 4; /* \todo Run this again when FrameDurationLimits is passed in */ - configureExposureModeHelpers(context.configuration.sensor.minShutterSpeed, - context.configuration.sensor.maxShutterSpeed, + configureExposureModeHelpers(context.configuration.sensor.minIntegrationTime, + context.configuration.sensor.maxIntegrationTime, context.configuration.sensor.minAnalogueGain, context.configuration.sensor.maxAnalogueGain); diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp index 070834fa..991ca1c2 100644 --- a/src/ipa/rkisp1/ipa_context.cpp +++ b/src/ipa/rkisp1/ipa_context.cpp @@ -78,11 +78,11 @@ namespace libcamera::ipa::rkisp1 { * \var IPASessionConfiguration::sensor * \brief Sensor-specific configuration of the IPA * - * \var IPASessionConfiguration::sensor.minShutterSpeed - * \brief Minimum shutter speed supported with the sensor + * \var IPASessionConfiguration::sensor.minIntegrationTime + * \brief Minimum integration time supported with the sensor * - * \var IPASessionConfiguration::sensor.maxShutterSpeed - * \brief Maximum shutter speed supported with the sensor + * \var IPASessionConfiguration::sensor.maxIntegrationTime + * \brief Maximum integration time supported with the sensor * * \var IPASessionConfiguration::sensor.minAnalogueGain * \brief Minimum analogue gain supported with the sensor diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index 256b75eb..3405a260 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -43,8 +43,8 @@ struct IPASessionConfiguration { } lsc; struct { - utils::Duration minShutterSpeed; - utils::Duration maxShutterSpeed; + utils::Duration minIntegrationTime; + utils::Duration maxIntegrationTime; double minAnalogueGain; double maxAnalogueGain; diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index d8610095..15919d3f 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -245,14 +245,14 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig, /* * When the AGC computes the new exposure values for a frame, it needs - * to know the limits for shutter speed and analogue gain. + * to know the limits for integration time and analogue gain. * As it depends on the sensor, update it with the controls. * - * \todo take VBLANK into account for maximum shutter speed + * \todo take VBLANK into account for maximum integration time */ - context_.configuration.sensor.minShutterSpeed = + context_.configuration.sensor.minIntegrationTime = minExposure * context_.configuration.sensor.lineDuration; - context_.configuration.sensor.maxShutterSpeed = + context_.configuration.sensor.maxIntegrationTime = maxExposure * context_.configuration.sensor.lineDuration; context_.configuration.sensor.minAnalogueGain = camHelper_->gain(minGain); context_.configuration.sensor.maxAnalogueGain = camHelper_->gain(maxGain); From patchwork Thu Apr 18 12:46:32 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Scally X-Patchwork-Id: 19907 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 9B1A8C32C9 for ; Thu, 18 Apr 2024 12:46:51 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 615F7633F7; Thu, 18 Apr 2024 14:46:48 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="A8ywHBYC"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 74E07633F3 for ; Thu, 18 Apr 2024 14:46:43 +0200 (CEST) Received: from mail.ideasonboard.com (cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id C62C727C; Thu, 18 Apr 2024 14:45:55 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1713444355; bh=6XqyCQqXki+3gyfZzJ7+4a2JpEiAEDmlubddBrbgCs8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=A8ywHBYC4dr1XN+at5x/ZyV7C6FKlDkk2yN4PhKXjHQNdCsxkJxcotWyCQJXE1jUk lYDyM7ar+nj3JMPHyxnoKtdA+weVhF4TXoCJZsGxgPxufEf1fDBFLvxmjIGo3NXry+ amcfoADrjmSr6nn4Spmfrnr6PxdmQa6FTMa9xADI= From: Daniel Scally To: libcamera-devel@lists.libcamera.org Cc: Daniel Scally Subject: [PATCH 3/3] ipa: ipu3: Use a Y histogram instead of green Date: Thu, 18 Apr 2024 13:46:32 +0100 Message-Id: <20240418124632.652163-4-dan.scally@ideasonboard.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240418124632.652163-1-dan.scally@ideasonboard.com> References: <20240418124632.652163-1-dan.scally@ideasonboard.com> MIME-Version: 1.0 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" The IPU3 IPA module uses a green histogram for the AGC algorithm's constraint mode calculations. Switch instead to a luminance histogram calculated using the Rec.601 coefficients. Signed-off-by: Daniel Scally Reviewed-by: Kieran Bingham --- src/ipa/ipu3/algorithms/agc.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index a59b73fb..76d2bb60 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -152,18 +152,19 @@ Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats, reinterpret_cast( &stats->awb_raw_buffer.meta_data[cellPosition]); - rgbTriples_.push_back({ - cell->R_avg, - (cell->Gr_avg + cell->Gb_avg) / 2, - cell->B_avg - }); + uint8_t G_avg = (cell->Gr_avg + cell->Gb_avg) / 2; + + rgbTriples_.push_back({cell->R_avg, G_avg, cell->B_avg}); /* - * Store the average green value to estimate the + * Store the average luminance value to estimate the * brightness. Even the overexposed pixels are * taken into account. */ - hist[(cell->Gr_avg + cell->Gb_avg) / 2]++; + uint8_t y = (cell->R_avg * .299) + + (G_avg * .587) + + (cell->B_avg * .114); + hist[y]++; } }