[{"id":17469,"web_url":"https://patchwork.libcamera.org/comment/17469/","msgid":"<20210608074828.GK156622@pyrite.rasen.tech>","date":"2021-06-08T07:48:28","subject":"Re: [libcamera-devel] [PATCH v2 1/3] ipa: ipu3: Calculate line\n\tduration from IPACameraSensorInfo","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Umang,\n\nOn Tue, Jun 08, 2021 at 01:12:23PM +0530, Umang Jain wrote:\n> Squash \\todo by calculating line duration from IPACameraSensorInfo,\n> now passed in, to IPU3Agc::initialise().\n> \n> Since line duration is now calculated from real values, store it as a\n> private member in IPU3Agc class. As a further step, replace the\n> associated global constant, kMaxExposureTime, with a private IPU3Agc\n> class member as well, and assign its value correspondingly in\n> IPU3Agc::initialise(), similar to previous precedence.\n> \n> Signed-off-by: Umang Jain \n> Tested-by: Paul Elder \n> Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart \n\nReviewed-by: Paul Elder \n\n> ---\n> src/ipa/ipu3/ipu3.cpp | 2 +-\n> src/ipa/ipu3/ipu3_agc.cpp | 24 +++++++++++++-----------\n> src/ipa/ipu3/ipu3_agc.h | 7 ++++++-\n> 3 files changed, 20 insertions(+), 13 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 700a5660..2496b0a0 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -174,7 +174,7 @@ void IPAIPU3::configure(const IPAConfigInfo &configInfo)\n> \tawbAlgo_->initialise(params_, configInfo.bdsOutputSize, bdsGrid_);\n> \n> \tagcAlgo_ = std::make_unique();\n> -\tagcAlgo_->initialise(bdsGrid_);\n> +\tagcAlgo_->initialise(bdsGrid_, configInfo.sensorInfo);\n> }\n> \n> void IPAIPU3::mapBuffers(const std::vector &buffers)\n> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp\n> index 8bae423f..8ca95013 100644\n> --- a/src/ipa/ipu3/ipu3_agc.cpp\n> +++ b/src/ipa/ipu3/ipu3_agc.cpp\n> @@ -11,6 +11,8 @@\n> #include \n> #include \n> \n> +#include \n> +\n> #include \"libcamera/internal/log.h\"\n> \n> #include \"libipa/histogram.h\"\n> @@ -39,11 +41,6 @@ static constexpr uint32_t kMaxGain = kMaxISO / 100;\n> static constexpr uint32_t kMinExposure = 1;\n> static constexpr uint32_t kMaxExposure = 1976;\n> \n> -/* \\todo those should be got from IPACameraSensorInfo ! */\n> -/* line duration in microseconds */\n> -static constexpr double kLineDuration = 16.8;\n> -static constexpr double kMaxExposureTime = kMaxExposure * kLineDuration;\n> -\n> /* Histogram constants */\n> static constexpr uint32_t knumHistogramBins = 256;\n> static constexpr double kEvGainTarget = 0.5;\n> @@ -54,14 +51,19 @@ static constexpr uint8_t kCellSize = 8;\n> IPU3Agc::IPU3Agc()\n> \t: frameCount_(0), lastFrame_(0), converged_(false),\n> \t updateControls_(false), iqMean_(0.0), gamma_(1.0),\n> +\t lineDuration_(0.0), maxExposureTime_(0.0),\n> \t prevExposure_(0.0), prevExposureNoDg_(0.0),\n> \t currentExposure_(0.0), currentExposureNoDg_(0.0)\n> {\n> }\n> \n> -void IPU3Agc::initialise(struct ipu3_uapi_grid_config &bdsGrid)\n> +void IPU3Agc::initialise(struct ipu3_uapi_grid_config &bdsGrid, const IPACameraSensorInfo &sensorInfo)\n> {\n> \taeGrid_ = bdsGrid;\n> +\n> +\t/* line duration in microseconds */\n> +\tlineDuration_ = sensorInfo.lineLength * 1000000ULL / static_cast(sensorInfo.pixelRate);\n> +\tmaxExposureTime_ = kMaxExposure * lineDuration_;\n> }\n> \n> void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)\n> @@ -160,13 +162,13 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, uint32_t &gain)\n> \t\tdouble newGain = kEvGainTarget * knumHistogramBins / iqMean_;\n> \n> \t\t/* extracted from Rpi::Agc::computeTargetExposure */\n> -\t\tdouble currentShutter = exposure * kLineDuration;\n> +\t\tdouble currentShutter = exposure * lineDuration_;\n> \t\tcurrentExposureNoDg_ = currentShutter * gain;\n> \t\tLOG(IPU3Agc, Debug) << \"Actual total exposure \" << currentExposureNoDg_\n> \t\t\t\t << \" Shutter speed \" << currentShutter\n> \t\t\t\t << \" Gain \" << gain;\n> \t\tcurrentExposure_ = currentExposureNoDg_ * newGain;\n> -\t\tdouble maxTotalExposure = kMaxExposureTime * kMaxGain;\n> +\t\tdouble maxTotalExposure = maxExposureTime_ * kMaxGain;\n> \t\tcurrentExposure_ = std::min(currentExposure_, maxTotalExposure);\n> \t\tLOG(IPU3Agc, Debug) << \"Target total exposure \" << currentExposure_;\n> \n> @@ -174,18 +176,18 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, uint32_t &gain)\n> \t\tfilterExposure();\n> \n> \t\tdouble newExposure = 0.0;\n> -\t\tif (currentShutter < kMaxExposureTime) {\n> +\t\tif (currentShutter < maxExposureTime_) {\n> \t\t\texposure = std::clamp(static_cast(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure);\n> \t\t\tnewExposure = currentExposure_ / exposure;\n> \t\t\tgain = std::clamp(static_cast(gain * currentExposure_ / newExposure), kMinGain, kMaxGain);\n> \t\t\tupdateControls_ = true;\n> -\t\t} else if (currentShutter >= kMaxExposureTime) {\n> +\t\t} else if (currentShutter >= maxExposureTime_) {\n> \t\t\tgain = std::clamp(static_cast(gain * currentExposure_ / currentExposureNoDg_), kMinGain, kMaxGain);\n> \t\t\tnewExposure = currentExposure_ / gain;\n> \t\t\texposure = std::clamp(static_cast(exposure * currentExposure_ / newExposure), kMinExposure, kMaxExposure);\n> \t\t\tupdateControls_ = true;\n> \t\t}\n> -\t\tLOG(IPU3Agc, Debug) << \"Adjust exposure \" << exposure * kLineDuration << \" and gain \" << gain;\n> +\t\tLOG(IPU3Agc, Debug) << \"Adjust exposure \" << exposure * lineDuration_ << \" and gain \" << gain;\n> \t}\n> \tlastFrame_ = frameCount_;\n> }\n> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/ipu3_agc.h\n> index bfdf45d1..99a582a9 100644\n> --- a/src/ipa/ipu3/ipu3_agc.h\n> +++ b/src/ipa/ipu3/ipu3_agc.h\n> @@ -18,6 +18,8 @@\n> \n> namespace libcamera {\n> \n> +class IPACameraSensorInfo;\n> +\n> namespace ipa::ipu3 {\n> \n> class IPU3Agc : public Algorithm\n> @@ -26,7 +28,7 @@ public:\n> \tIPU3Agc();\n> \t~IPU3Agc() = default;\n> \n> -\tvoid initialise(struct ipu3_uapi_grid_config &bdsGrid);\n> +\tvoid initialise(struct ipu3_uapi_grid_config &bdsGrid, const IPACameraSensorInfo &sensorInfo);\n> \tvoid process(const ipu3_uapi_stats_3a *stats, uint32_t &exposure, uint32_t &gain);\n> \tbool converged() { return converged_; }\n> \tbool updateControls() { return updateControls_; }\n> @@ -49,6 +51,9 @@ private:\n> \tdouble iqMean_;\n> \tdouble gamma_;\n> \n> +\tdouble lineDuration_;\n> +\tdouble maxExposureTime_;\n> +\n> \tdouble prevExposure_;\n> \tdouble prevExposureNoDg_;\n> \tdouble currentExposure_;\n> -- \n> 2.31.1\n>","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 3D7AFBD22E\n\tfor ;\n\tTue, 8 Jun 2021 07:48:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A777B6892D;\n\tTue, 8 Jun 2021 09:48:38 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C53FA68928\n\tfor ;\n\tTue, 8 Jun 2021 09:48:36 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EA04FE71;\n\tTue, 8 Jun 2021 09:48:34 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ffBWlDaB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1623138516;\n\tbh=KLHg/8zXz76zi9+6zXq1Fy9PxuBDu+JOJI9OWcoGQxc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ffBWlDaBsmjLuG9nMoFSXrI8UAfrl68D/9eJ5KyF33i+nIMvZnUkWm6JGCj5zHTdU\n\tlK2ISwjb8nMOX4xF7Y5u4SF7rXmm+gOiD12xpK5SG1x4ilc0HjrezBWr3jqIUW1oz8\n\tBbqq/IVnODCD7iyYDUGVfZWqW4W/XsrrfjNwB3Bk=","Date":"Tue, 8 Jun 2021 16:48:28 +0900","From":"paul.elder@ideasonboard.com","To":"Umang Jain ","Message-ID":"<20210608074828.GK156622@pyrite.rasen.tech>","References":"<20210608074225.59862-1-umang.jain@ideasonboard.com>\n\t<20210608074225.59862-2-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210608074225.59862-2-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 1/3] ipa: ipu3: Calculate line\n\tduration from IPACameraSensorInfo","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":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]