[{"id":18617,"web_url":"https://patchwork.libcamera.org/comment/18617/","msgid":"<0b22177a-e7f2-9ec7-9ec1-a9834bcf1309@ideasonboard.com>","date":"2021-08-09T10:33:47","subject":"Re: [libcamera-devel] [RFC PATCH 5/5] ipa: ipu3: Move IPU3 agc into\n\talgorithms","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi JM,\n\nThanks for the patch.\n\nOn 8/9/21 2:50 PM, Jean-Michel Hautbois wrote:\n> Use the Context class and algorithm interface to properly call the AGC\n> algorithm from IPAIPU3.\n> We need to pass shutter speed and analogue gain through Context. Add a\n> dedicated AGC structure for that.\n>\n> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n> ---\n>   .../ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} | 56 ++++++++++---------\n>   src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} | 31 +++++-----\n>   src/ipa/ipu3/algorithms/meson.build           |  1 +\n>   src/ipa/ipu3/ipa_context.h                    | 11 ++++\n>   src/ipa/ipu3/ipu3.cpp                         | 25 +++++----\n>   src/ipa/ipu3/meson.build                      |  1 -\n>   6 files changed, 70 insertions(+), 55 deletions(-)\n>   rename src/ipa/ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} (81%)\n>   rename src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} (61%)\n>\n> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> similarity index 81%\n> rename from src/ipa/ipu3/ipu3_agc.cpp\n> rename to src/ipa/ipu3/algorithms/agc.cpp\n> index 733814dd..1146a34a 100644\n> --- a/src/ipa/ipu3/ipu3_agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -2,26 +2,25 @@\n>   /*\n>    * Copyright (C) 2021, Ideas On Board\nSPDX license missing, I think it's missing in 4/5 too\n>    *\n> - * ipu3_agc.cpp - AGC/AEC control algorithm\n> + * agc.cpp - AGC/AEC control algorithm\n>    */\n>   \n> -#include \"ipu3_agc.h\"\n> +#include \"agc.h\"\n>   \n> -#include <algorithm>\n>   #include <cmath>\n> +\n> +#include <algorithm>\nI guess this a transient change that creeped in? :)\n>   #include <numeric>\n>   \n>   #include <libcamera/base/log.h>\n>   \n> -#include <libcamera/ipa/core_ipa_interface.h>\n> -\n>   #include \"libipa/histogram.h\"\n>   \n>   namespace libcamera {\n>   \n>   using namespace std::literals::chrono_literals;\n>   \n> -namespace ipa::ipu3 {\n> +namespace ipa::ipu3::algorithms {\n>   \n>   LOG_DEFINE_CATEGORY(IPU3Agc)\n>   \n> @@ -36,8 +35,8 @@ static constexpr uint32_t kMaxISO = 1500;\n>   \n>   /* Maximum analogue gain value\n>    * \\todo grab it from a camera helper */\n> -static constexpr uint32_t kMinGain = kMinISO / 100;\n> -static constexpr uint32_t kMaxGain = kMaxISO / 100;\n> +static constexpr double kMinGain = kMinISO / 100;\n> +static constexpr double kMaxGain = kMaxISO / 100;\nProbably \"/ 100.0\" , if you want converted to double?\n>   \n>   /* \\todo use calculated value based on sensor */\n>   static constexpr uint32_t kMinExposure = 1;\n> @@ -50,24 +49,24 @@ static constexpr double kEvGainTarget = 0.5;\n>   /* A cell is 8 bytes and contains averages for RGB values and saturation ratio */\n>   static constexpr uint8_t kCellSize = 8;\n>   \n> -IPU3Agc::IPU3Agc()\n> +Agc::Agc()\n>   \t: frameCount_(0), lastFrame_(0), converged_(false),\n> -\t  updateControls_(false), iqMean_(0.0),\n> -\t  lineDuration_(0s), maxExposureTime_(0s),\n> +\t  updateControls_(false), iqMean_(0.0), maxExposureTime_(0s),\n>   \t  prevExposure_(0s), prevExposureNoDg_(0s),\n>   \t  currentExposure_(0s), currentExposureNoDg_(0s)\n>   {\n>   }\n>   \n> -void IPU3Agc::initialise(struct ipu3_uapi_grid_config &bdsGrid, const IPACameraSensorInfo &sensorInfo)\n> +int Agc::configure(IPAContext &context)\n>   {\n> -\taeGrid_ = bdsGrid;\n> +\t/* The AGC algorithm uses the AWB statistics */\n> +\taeGrid_ = context.awb.grid.bdsGrid;\n> +\tmaxExposureTime_ = kMaxExposure * context.agc.lineDuration;\n>   \n> -\tlineDuration_ = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate;\n> -\tmaxExposureTime_ = kMaxExposure * lineDuration_;\n> +\treturn 0;\n>   }\n>   \n> -void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)\n> +void Agc::processBrightness(const ipu3_uapi_stats_3a *stats)\n>   {\n>   \tconst struct ipu3_uapi_grid_config statsAeGrid = stats->stats_4a_config.awb_config.grid;\n>   \tRectangle aeRegion = { statsAeGrid.x_start,\n> @@ -108,7 +107,7 @@ void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)\n>   \tiqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);\n>   }\n>   \n> -void IPU3Agc::filterExposure()\n> +void Agc::filterExposure()\n>   {\n>   \tdouble speed = 0.2;\n>   \tif (prevExposure_ == 0s) {\n> @@ -128,7 +127,7 @@ void IPU3Agc::filterExposure()\n>   \t\tprevExposure_ = speed * currentExposure_ +\n>   \t\t\t\tprevExposure_ * (1.0 - speed);\n>   \t\tprevExposureNoDg_ = speed * currentExposureNoDg_ +\n> -\t\t\t\tprevExposureNoDg_ * (1.0 - speed);\n> +\t\t\t\t    prevExposureNoDg_ * (1.0 - speed);\n>   \t}\n>   \t/*\n>   \t * We can't let the no_dg exposure deviate too far below the\n> @@ -142,7 +141,7 @@ void IPU3Agc::filterExposure()\n>   \tLOG(IPU3Agc, Debug) << \"After filtering, total_exposure \" << prevExposure_;\n>   }\n>   \n> -void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)\n> +void Agc::lockExposureGain(IPAContext &context)\n>   {\n>   \tupdateControls_ = false;\n>   \n> @@ -160,7 +159,9 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)\n>   \t\tdouble newGain = kEvGainTarget * knumHistogramBins / iqMean_;\n>   \n>   \t\t/* extracted from Rpi::Agc::computeTargetExposure */\n> -\t\tlibcamera::utils::Duration currentShutter = exposure * lineDuration_;\n> +\t\tlibcamera::utils::Duration currentShutter = context.agc.shutterTime;\n> +\t\tuint32_t exposure = currentShutter / context.agc.lineDuration;\n> +\t\tdouble &gain = context.agc.analogueGain;\n>   \t\tcurrentExposureNoDg_ = currentShutter * gain;\n>   \t\tLOG(IPU3Agc, Debug) << \"Actual total exposure \" << currentExposureNoDg_\n>   \t\t\t\t    << \" Shutter speed \" << currentShutter\n> @@ -177,26 +178,27 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)\n>   \t\tif (currentShutter < maxExposureTime_) {\n>   \t\t\texposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure);\n>   \t\t\tnewExposure = currentExposure_ / exposure;\n> -\t\t\tgain = std::clamp(static_cast<uint32_t>(gain * currentExposure_ / newExposure), kMinGain, kMaxGain);\n> +\t\t\tgain = std::clamp(static_cast<double>(gain * currentExposure_ / newExposure), kMinGain, kMaxGain);\n>   \t\t\tupdateControls_ = true;\n>   \t\t} else if (currentShutter >= maxExposureTime_) {\n> -\t\t\tgain = std::clamp(static_cast<uint32_t>(gain * currentExposure_ / currentExposureNoDg_), kMinGain, kMaxGain);\n> +\t\t\tgain = std::clamp(static_cast<double>(gain * currentExposure_ / currentExposureNoDg_), kMinGain, kMaxGain);\n>   \t\t\tnewExposure = currentExposure_ / gain;\n>   \t\t\texposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / newExposure), kMinExposure, kMaxExposure);\n>   \t\t\tupdateControls_ = true;\n>   \t\t}\n> -\t\tLOG(IPU3Agc, Debug) << \"Adjust exposure \" << exposure * lineDuration_ << \" and gain \" << gain;\n> +\t\tcontext.agc.shutterTime = exposure * context.agc.lineDuration;\n> +\t\tLOG(IPU3Agc, Debug) << \"Adjust exposure \" << exposure * context.agc.lineDuration << \" and gain \" << gain;\n>   \t}\n>   \tlastFrame_ = frameCount_;\n>   }\n>   \n> -void IPU3Agc::process(const ipu3_uapi_stats_3a *stats, uint32_t &exposure, double &gain)\n> +void Agc::process(IPAContext &context)\n>   {\n> -\tprocessBrightness(stats);\n> -\tlockExposureGain(exposure, gain);\n> +\tprocessBrightness(context.stats);\n> +\tlockExposureGain(context);\n>   \tframeCount_++;\n>   }\n>   \n> -} /* namespace ipa::ipu3 */\n> +} /* namespace ipa::ipu3::algorithms */\n>   \n>   } /* namespace libcamera */\n> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/algorithms/agc.h\n> similarity index 61%\n> rename from src/ipa/ipu3/ipu3_agc.h\n> rename to src/ipa/ipu3/algorithms/agc.h\n> index 3b7f781e..e87a06f3 100644\n> --- a/src/ipa/ipu3/ipu3_agc.h\n> +++ b/src/ipa/ipu3/algorithms/agc.h\n> @@ -4,44 +4,44 @@\n>    *\n>    * ipu3_agc.h - IPU3 AGC/AEC control algorithm\n>    */\n> -#ifndef __LIBCAMERA_IPU3_AGC_H__\n> -#define __LIBCAMERA_IPU3_AGC_H__\n> +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__\n> +#define __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__\n> +\n> +#include <linux/intel-ipu3.h>\n>   \n>   #include <array>\n>   #include <unordered_map>\n>   \n> -#include <linux/intel-ipu3.h>\n> -\n>   #include <libcamera/base/utils.h>\n>   \n>   #include <libcamera/geometry.h>\n>   \n> -#include \"algorithms/algorithm.h\"\n> -#include \"libipa/algorithm.h\"\n> +#include \"algorithm.h\"\n>   \n>   namespace libcamera {\n>   \n>   struct IPACameraSensorInfo;\n>   \n> -namespace ipa::ipu3 {\n> +namespace ipa::ipu3::algorithms {\n>   \n>   using utils::Duration;\n>   \n> -class IPU3Agc : public ipa::Algorithm\n> +class Agc : public Algorithm\n>   {\n>   public:\n> -\tIPU3Agc();\n> -\t~IPU3Agc() = default;\n> +\tAgc();\n> +\t~Agc() = default;\n> +\n> +\tint configure(IPAContext &context) override;\n> +\tvoid process(IPAContext &context) override;\n>   \n> -\tvoid initialise(struct ipu3_uapi_grid_config &bdsGrid, const IPACameraSensorInfo &sensorInfo);\n> -\tvoid process(const ipu3_uapi_stats_3a *stats, uint32_t &exposure, double &gain);\n>   \tbool converged() { return converged_; }\n>   \tbool updateControls() { return updateControls_; }\n>   \n>   private:\n>   \tvoid processBrightness(const ipu3_uapi_stats_3a *stats);\n>   \tvoid filterExposure();\n> -\tvoid lockExposureGain(uint32_t &exposure, double &gain);\n> +\tvoid lockExposureGain(IPAContext &context);\n>   \n>   \tstruct ipu3_uapi_grid_config aeGrid_;\n>   \n> @@ -53,7 +53,6 @@ private:\n>   \n>   \tdouble iqMean_;\n>   \n> -\tDuration lineDuration_;\n>   \tDuration maxExposureTime_;\n>   \n>   \tDuration prevExposure_;\n> @@ -62,8 +61,8 @@ private:\n>   \tDuration currentExposureNoDg_;\n>   };\n>   \n> -} /* namespace ipa::ipu3 */\n> +} /* namespace ipa::ipu3::algorithms */\n>   \n>   } /* namespace libcamera */\n>   \n> -#endif /* __LIBCAMERA_IPU3_AGC_H__ */\n> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__ */\n> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build\n> index df36d719..3faf913e 100644\n> --- a/src/ipa/ipu3/algorithms/meson.build\n> +++ b/src/ipa/ipu3/algorithms/meson.build\n> @@ -1,6 +1,7 @@\n>   # SPDX-License-Identifier: CC0-1.0\n>   \n>   ipu3_ipa_algorithms = files([\n> +    'agc.cpp',\n>       'awb.cpp',\n>       'contrast.cpp',\n>   ])\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index c43b275b..1d7a1984 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -11,10 +11,14 @@\n>   \n>   #include <linux/intel-ipu3.h>\n>   \n> +#include <libcamera/base/utils.h>\n> +\n>   #include <libcamera/geometry.h>\n>   \n>   namespace libcamera {\n>   \n> +using libcamera::utils::Duration;\n> +\nI think you can drop \"libcamera::\" here, just utils::Duration\n>   namespace ipa::ipu3 {\n>   \n>   struct IPAContext {\n> @@ -33,6 +37,13 @@ struct IPAContext {\n>   \t\t\tSize bdsOutputSize;\n>   \t\t} grid;\n>   \t} awb;\n> +\n> +\t/* AGC specific parameters to share */\n> +\tstruct Agc {\n> +\t\tDuration lineDuration;\n> +\t\tDuration shutterTime;\n> +\t\tdouble analogueGain;\n> +\t} agc;\n>   };\n>   \n>   } /* namespace ipa::ipu3 */\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 82506461..07b1d11c 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -22,12 +22,12 @@\n>   \n>   #include \"libcamera/internal/framebuffer.h\"\n>   \n> +#include \"algorithms/agc.h\"\n>   #include \"algorithms/algorithm.h\"\n>   #include \"algorithms/awb.h\"\n>   #include \"algorithms/contrast.h\"\n>   #include \"ipa_context.h\"\n>   \n> -#include \"ipu3_agc.h\"\n>   #include \"libipa/camera_sensor_helper.h\"\n>   \n>   static constexpr uint32_t kMaxCellWidthPerSet = 160;\n> @@ -37,6 +37,8 @@ namespace libcamera {\n>   \n>   LOG_DEFINE_CATEGORY(IPAIPU3)\n>   \n> +using namespace std::literals::chrono_literals;\n> +\n>   namespace ipa::ipu3 {\n>   \n>   class IPAIPU3 : public IPAIPU3Interface\n> @@ -81,8 +83,6 @@ private:\n>   \tuint32_t minGain_;\n>   \tuint32_t maxGain_;\n>   \n> -\t/* Interface to the AEC/AGC algorithm */\n> -\tstd::unique_ptr<IPU3Agc> agcAlgo_;\n>   \t/* Interface to the Camera Helper */\n>   \tstd::unique_ptr<CameraSensorHelper> camHelper_;\n>   \n> @@ -103,6 +103,7 @@ int IPAIPU3::init(const IPASettings &settings)\n>   \t}\n>   \n>   \t/* Construct our Algorithms */\n> +\talgorithms_.emplace_back(new algorithms::Agc());\n>   \talgorithms_.emplace_back(new algorithms::Awb());\n>   \talgorithms_.emplace_back(new algorithms::Contrast());\n>   \n> @@ -214,11 +215,14 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)\n>   \tcalculateBdsGrid(configInfo.bdsOutputSize);\n>   \tcontext_.awb.grid.bdsOutputSize = configInfo.bdsOutputSize;\n>   \n> +\t/* Prepare AGC parameters */\n> +\tcontext_.agc.lineDuration = sensorInfo_.lineLength * 1.0s / sensorInfo_.pixelRate;\n> +\tcontext_.agc.shutterTime = exposure_ * context_.agc.lineDuration;\n> +\tcontext_.agc.analogueGain = camHelper_->gain(gain_);\n> +\n>   \tconfigureAlgorithms();\n>   \n>   \tbdsGrid_ = context_.awb.grid.bdsGrid;\n> -\tagcAlgo_ = std::make_unique<IPU3Agc>();\n> -\tagcAlgo_->initialise(bdsGrid_, sensorInfo_);\n>   \n>   \treturn 0;\n>   }\n> @@ -341,12 +345,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n>   \t/* Run the process for each algorithm on the stats */\n>   \tprocessAlgorithms(stats);\n>   \n> -\tdouble gain = camHelper_->gain(gain_);\n> -\tagcAlgo_->process(stats, exposure_, gain);\n> -\tgain_ = camHelper_->gainCode(gain);\n> -\n> -\tif (agcAlgo_->updateControls())\n> -\t\tsetControls(frame);\n> +\tsetControls(frame);\n>   \n>   \t/* \\todo Use VBlank value calculated from each frame exposure. */\n>   \tint64_t frameDuration = sensorInfo_.lineLength * (defVBlank_ + sensorInfo_.outputSize.height) /\n> @@ -365,6 +364,10 @@ void IPAIPU3::setControls(unsigned int frame)\n>   \tIPU3Action op;\n>   \top.op = ActionSetSensorControls;\n>   \n> +\t/* Convert gain and exposure */\n> +\tgain_ = camHelper_->gainCode(context_.agc.analogueGain);\n> +\texposure_ = context_.agc.shutterTime / context_.agc.lineDuration;\n> +\n>   \tControlList ctrls(ctrls_);\n>   \tctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));\n>   \tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));\n> diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build\n> index d1126947..39320980 100644\n> --- a/src/ipa/ipu3/meson.build\n> +++ b/src/ipa/ipu3/meson.build\n> @@ -6,7 +6,6 @@ ipa_name = 'ipa_ipu3'\n>   \n>   ipu3_ipa_sources = files([\n>       'ipu3.cpp',\n> -    'ipu3_agc.cpp',\n>   ])\n>   \n>   ipu3_ipa_sources += ipu3_ipa_algorithms","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 0FE7FBD87D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  9 Aug 2021 10:33:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 75E70687EB;\n\tMon,  9 Aug 2021 12:33:54 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C7EB360269\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  9 Aug 2021 12:33:52 +0200 (CEST)","from [192.168.1.104] (unknown [103.251.226.61])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B4648466;\n\tMon,  9 Aug 2021 12:33:51 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kjRDecTN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628505232;\n\tbh=MbyQ0x8Pd2KohoH3clc22g/6GrfTFpMUaSJc82U0L6c=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=kjRDecTN4F6FwxQTDEDzq5W9Zf6Jz1V3MBqNmH/OwYWjZi8ZeojMXaxBV8P4+7Qcy\n\tUcBi1UKeoqZ/QTlp2cYaDNe63vQ6VC2NcDi23feZP2PXOikMMWzLYQVgf4qYUmP4Df\n\t/Q+X5LpULB6XD4yk2a+fEfoCGNfOExfcGpbRGSjU=","To":"Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210809092007.79067-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20210809092007.79067-6-jeanmichel.hautbois@ideasonboard.com>","From":"Umang Jain <umang.jain@ideasonboard.com>","Message-ID":"<0b22177a-e7f2-9ec7-9ec1-a9834bcf1309@ideasonboard.com>","Date":"Mon, 9 Aug 2021 16:03:47 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20210809092007.79067-6-jeanmichel.hautbois@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [RFC PATCH 5/5] ipa: ipu3: Move IPU3 agc into\n\talgorithms","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":18619,"web_url":"https://patchwork.libcamera.org/comment/18619/","msgid":"<3b8146b3-8c22-3385-b251-f3ba163d109f@ideasonboard.com>","date":"2021-08-09T11:05:42","subject":"Re: [libcamera-devel] [RFC PATCH 5/5] ipa: ipu3: Move IPU3 agc into\n\talgorithms","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 09/08/2021 10:20, Jean-Michel Hautbois wrote:\n> Use the Context class and algorithm interface to properly call the AGC\n> algorithm from IPAIPU3.\n> We need to pass shutter speed and analogue gain through Context. Add a\n> dedicated AGC structure for that.\n\nSame comments from the previous patch I think ;-)\n\n\n\n> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n> ---\n>  .../ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} | 56 ++++++++++---------\n>  src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} | 31 +++++-----\n>  src/ipa/ipu3/algorithms/meson.build           |  1 +\n>  src/ipa/ipu3/ipa_context.h                    | 11 ++++\n>  src/ipa/ipu3/ipu3.cpp                         | 25 +++++----\n>  src/ipa/ipu3/meson.build                      |  1 -\n>  6 files changed, 70 insertions(+), 55 deletions(-)\n>  rename src/ipa/ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} (81%)\n>  rename src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} (61%)\n> \n> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> similarity index 81%\n> rename from src/ipa/ipu3/ipu3_agc.cpp\n> rename to src/ipa/ipu3/algorithms/agc.cpp\n> index 733814dd..1146a34a 100644\n> --- a/src/ipa/ipu3/ipu3_agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -2,26 +2,25 @@\n>  /*\n>   * Copyright (C) 2021, Ideas On Board\n>   *\n> - * ipu3_agc.cpp - AGC/AEC control algorithm\n> + * agc.cpp - AGC/AEC control algorithm\n>   */\n>  \n> -#include \"ipu3_agc.h\"\n> +#include \"agc.h\"\n>  \n> -#include <algorithm>\n>  #include <cmath>\n> +> +#include <algorithm>\n>  #include <numeric>\n\n\nWhat's going on there? Shouldn't cmath algorithm, and numeric all be one\nalphabetically sorted block?\n\n\n\n>  \n>  #include <libcamera/base/log.h>\n>  \n> -#include <libcamera/ipa/core_ipa_interface.h>\n> -\n\nWas this no longer required as part of the conversion, or something that\ncould have been done on it's own?\n\n\n\n>  #include \"libipa/histogram.h\"\n>  \n>  namespace libcamera {\n>  \n>  using namespace std::literals::chrono_literals;\n>  \n> -namespace ipa::ipu3 {\n> +namespace ipa::ipu3::algorithms {\n>  \n>  LOG_DEFINE_CATEGORY(IPU3Agc)\n>  \n> @@ -36,8 +35,8 @@ static constexpr uint32_t kMaxISO = 1500;\n>  \n>  /* Maximum analogue gain value\n>   * \\todo grab it from a camera helper */\n> -static constexpr uint32_t kMinGain = kMinISO / 100;\n> -static constexpr uint32_t kMaxGain = kMaxISO / 100;\n> +static constexpr double kMinGain = kMinISO / 100;\n> +static constexpr double kMaxGain = kMaxISO / 100;\n\n\nWhy do these types change as part of moving the algorithm and restructuring?\n\nIt doesn't sound like something related to moving code - so it should be\nin it's own patch, either before if it's a bug fix, or after if it\ndoesn't matter when the types change.\n\n\n\n>  /* \\todo use calculated value based on sensor */\n>  static constexpr uint32_t kMinExposure = 1;\n> @@ -50,24 +49,24 @@ static constexpr double kEvGainTarget = 0.5;\n>  /* A cell is 8 bytes and contains averages for RGB values and saturation ratio */\n>  static constexpr uint8_t kCellSize = 8;\n>  \n> -IPU3Agc::IPU3Agc()\n> +Agc::Agc()\n>  \t: frameCount_(0), lastFrame_(0), converged_(false),\n> -\t  updateControls_(false), iqMean_(0.0),\n> -\t  lineDuration_(0s), maxExposureTime_(0s),\n> +\t  updateControls_(false), iqMean_(0.0), maxExposureTime_(0s),\n>  \t  prevExposure_(0s), prevExposureNoDg_(0s),\n>  \t  currentExposure_(0s), currentExposureNoDg_(0s)\n>  {\n>  }\n>  \n> -void IPU3Agc::initialise(struct ipu3_uapi_grid_config &bdsGrid, const IPACameraSensorInfo &sensorInfo)\n> +int Agc::configure(IPAContext &context)\n>  {\n> -\taeGrid_ = bdsGrid;\n> +\t/* The AGC algorithm uses the AWB statistics */\n> +\taeGrid_ = context.awb.grid.bdsGrid;\n> +\tmaxExposureTime_ = kMaxExposure * context.agc.lineDuration;\n>  \n> -\tlineDuration_ = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate;\n> -\tmaxExposureTime_ = kMaxExposure * lineDuration_;\n> +\treturn 0;\n>  }\n>  \n> -void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)\n> +void Agc::processBrightness(const ipu3_uapi_stats_3a *stats)\n>  {\n>  \tconst struct ipu3_uapi_grid_config statsAeGrid = stats->stats_4a_config.awb_config.grid;\n>  \tRectangle aeRegion = { statsAeGrid.x_start,\n> @@ -108,7 +107,7 @@ void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)\n>  \tiqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);\n>  }\n>  \n> -void IPU3Agc::filterExposure()\n> +void Agc::filterExposure()\n>  {\n>  \tdouble speed = 0.2;\n>  \tif (prevExposure_ == 0s) {\n> @@ -128,7 +127,7 @@ void IPU3Agc::filterExposure()\n>  \t\tprevExposure_ = speed * currentExposure_ +\n>  \t\t\t\tprevExposure_ * (1.0 - speed);\n>  \t\tprevExposureNoDg_ = speed * currentExposureNoDg_ +\n> -\t\t\t\tprevExposureNoDg_ * (1.0 - speed);\n> +\t\t\t\t    prevExposureNoDg_ * (1.0 - speed);\n\nThis is a minor whitespace fix, so I guess it showed up in checkstyle as\npart of the code move. Could either be it's own patch - or needs to be\nmetnioned in the commit message as something like:\n\n\"Fixed up white-space issues only were fixed as part of the move.\"\n\n\n\n>  \t}\n>  \t/*\n>  \t * We can't let the no_dg exposure deviate too far below the\n> @@ -142,7 +141,7 @@ void IPU3Agc::filterExposure()\n>  \tLOG(IPU3Agc, Debug) << \"After filtering, total_exposure \" << prevExposure_;\n>  }\n>  \n> -void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)\n> +void Agc::lockExposureGain(IPAContext &context)\n\nPrivate algorithm functions don't have to take the IPAContext, these\ncould have been passed in from ::process() still...\n\n\n\n>  {\n>  \tupdateControls_ = false;\n>  \n> @@ -160,7 +159,9 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)\n>  \t\tdouble newGain = kEvGainTarget * knumHistogramBins / iqMean_;\n>  \n>  \t\t/* extracted from Rpi::Agc::computeTargetExposure */\n> -\t\tlibcamera::utils::Duration currentShutter = exposure * lineDuration_;\n> +\t\tlibcamera::utils::Duration currentShutter = context.agc.shutterTime;\n> +\t\tuint32_t exposure = currentShutter / context.agc.lineDuration;\n> +\t\tdouble &gain = context.agc.analogueGain;\n\nI'm not sure I like this.\n\nIf gain is something that is calculated by this function, it should be\nsetting the end result after it is calculated.\n\nI guess this is a shortcut that the answer is calculated in place... but\nit feels like a hack - as reading the code it's not clear where the\nresults go.\n\nI.e. usually, you'd expect calculations and then results to be set at\nthe end or return by the function?\n\n\nTo keep the churn minimal in this patch which just moves to the new\ninterface, I'd stick with the existing function prototype and pass in\nthe references from the IPAContext to the private function.\n\n\n\n>  \t\tcurrentExposureNoDg_ = currentShutter * gain;\n>  \t\tLOG(IPU3Agc, Debug) << \"Actual total exposure \" << currentExposureNoDg_\n>  \t\t\t\t    << \" Shutter speed \" << currentShutter\n> @@ -177,26 +178,27 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)\n>  \t\tif (currentShutter < maxExposureTime_) {\n>  \t\t\texposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure);\n>  \t\t\tnewExposure = currentExposure_ / exposure;\n> -\t\t\tgain = std::clamp(static_cast<uint32_t>(gain * currentExposure_ / newExposure), kMinGain, kMaxGain);\n> +\t\t\tgain = std::clamp(static_cast<double>(gain * currentExposure_ / newExposure), kMinGain, kMaxGain);\n>  \t\t\tupdateControls_ = true;\n>  \t\t} else if (currentShutter >= maxExposureTime_) {\n> -\t\t\tgain = std::clamp(static_cast<uint32_t>(gain * currentExposure_ / currentExposureNoDg_), kMinGain, kMaxGain);\n> +\t\t\tgain = std::clamp(static_cast<double>(gain * currentExposure_ / currentExposureNoDg_), kMinGain, kMaxGain);\n>  \t\t\tnewExposure = currentExposure_ / gain;\n>  \t\t\texposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / newExposure), kMinExposure, kMaxExposure);\n>  \t\t\tupdateControls_ = true;\n>  \t\t}\n> -\t\tLOG(IPU3Agc, Debug) << \"Adjust exposure \" << exposure * lineDuration_ << \" and gain \" << gain;\n> +\t\tcontext.agc.shutterTime = exposure * context.agc.lineDuration;\n\nPerhaps that shutterTime line is something that should happen in\nAgc::process() if the context isn't available in the function.\n\n> +\t\tLOG(IPU3Agc, Debug) << \"Adjust exposure \" << exposure * context.agc.lineDuration << \" and gain \" << gain;\n>  \t}\n>  \tlastFrame_ = frameCount_;\n>  }\n>  \n> -void IPU3Agc::process(const ipu3_uapi_stats_3a *stats, uint32_t &exposure, double &gain)\n> +void Agc::process(IPAContext &context)\n>  {\n> -\tprocessBrightness(stats);\n> -\tlockExposureGain(exposure, gain);\n> +\tprocessBrightness(context.stats);\n> +\tlockExposureGain(context);\n>  \tframeCount_++;\n>  }\n>  \n> -} /* namespace ipa::ipu3 */\n> +} /* namespace ipa::ipu3::algorithms */\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/algorithms/agc.h\n> similarity index 61%\n> rename from src/ipa/ipu3/ipu3_agc.h\n> rename to src/ipa/ipu3/algorithms/agc.h\n> index 3b7f781e..e87a06f3 100644\n> --- a/src/ipa/ipu3/ipu3_agc.h\n> +++ b/src/ipa/ipu3/algorithms/agc.h\n> @@ -4,44 +4,44 @@\n>   *\n>   * ipu3_agc.h - IPU3 AGC/AEC control algorithm\n>   */\n> -#ifndef __LIBCAMERA_IPU3_AGC_H__\n> -#define __LIBCAMERA_IPU3_AGC_H__\n> +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__\n> +#define __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__\n> +\n> +#include <linux/intel-ipu3.h>\n>  \n>  #include <array>\n>  #include <unordered_map>\n>  \n> -#include <linux/intel-ipu3.h>\n> -\n\nWhy did the linux header move? I think it was in the right place here.\n\nhttps://git.libcamera.org/libcamera/libcamera.git/tree/Documentation/coding-style.rst#n64\nstates:\n\n1. The header declaring the API being implemented (if any)\n2. The C and C++ system and standard library headers\n3. Other libraries' headers, with one group per library\n4. Other project's headers\n\n\n>  #include <libcamera/base/utils.h>\n>  \n>  #include <libcamera/geometry.h>\n>  \n> -#include \"algorithms/algorithm.h\"\n> -#include \"libipa/algorithm.h\"\n> +#include \"algorithm.h\"\n>  \n>  namespace libcamera {\n>  \n>  struct IPACameraSensorInfo;\n>  \n> -namespace ipa::ipu3 {\n> +namespace ipa::ipu3::algorithms {\n>  \n>  using utils::Duration;\n>  \n> -class IPU3Agc : public ipa::Algorithm\n> +class Agc : public Algorithm\n>  {\n>  public:\n> -\tIPU3Agc();\n> -\t~IPU3Agc() = default;\n> +\tAgc();\n> +\t~Agc() = default;\n> +\n> +\tint configure(IPAContext &context) override;\n> +\tvoid process(IPAContext &context) override;\n>  \n> -\tvoid initialise(struct ipu3_uapi_grid_config &bdsGrid, const IPACameraSensorInfo &sensorInfo);\n> -\tvoid process(const ipu3_uapi_stats_3a *stats, uint32_t &exposure, double &gain);\n>  \tbool converged() { return converged_; }\n>  \tbool updateControls() { return updateControls_; }\n>  \n>  private:\n>  \tvoid processBrightness(const ipu3_uapi_stats_3a *stats);\n>  \tvoid filterExposure();\n> -\tvoid lockExposureGain(uint32_t &exposure, double &gain);\n> +\tvoid lockExposureGain(IPAContext &context);\n>  \n>  \tstruct ipu3_uapi_grid_config aeGrid_;\n>  \n> @@ -53,7 +53,6 @@ private:\n>  \n>  \tdouble iqMean_;\n>  \n> -\tDuration lineDuration_;\n>  \tDuration maxExposureTime_;\n>  \n>  \tDuration prevExposure_;\n> @@ -62,8 +61,8 @@ private:\n>  \tDuration currentExposureNoDg_;\n>  };\n>  \n> -} /* namespace ipa::ipu3 */\n> +} /* namespace ipa::ipu3::algorithms */\n>  \n>  } /* namespace libcamera */\n>  \n> -#endif /* __LIBCAMERA_IPU3_AGC_H__ */\n> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AGC_H__ */\n> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build\n> index df36d719..3faf913e 100644\n> --- a/src/ipa/ipu3/algorithms/meson.build\n> +++ b/src/ipa/ipu3/algorithms/meson.build\n> @@ -1,6 +1,7 @@\n>  # SPDX-License-Identifier: CC0-1.0\n>  \n>  ipu3_ipa_algorithms = files([\n> +    'agc.cpp',\n>      'awb.cpp',\n>      'contrast.cpp',\n>  ])\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index c43b275b..1d7a1984 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -11,10 +11,14 @@\n>  \n>  #include <linux/intel-ipu3.h>\n>  \n> +#include <libcamera/base/utils.h>\n> +\n>  #include <libcamera/geometry.h>\n>  \n>  namespace libcamera {\n>  \n> +using libcamera::utils::Duration;\n> +\n>  namespace ipa::ipu3 {\n>  \n>  struct IPAContext {\n> @@ -33,6 +37,13 @@ struct IPAContext {\n>  \t\t\tSize bdsOutputSize;\n>  \t\t} grid;\n>  \t} awb;\n> +\n> +\t/* AGC specific parameters to share */\n> +\tstruct Agc {\n> +\t\tDuration lineDuration;\n> +\t\tDuration shutterTime;\n> +\t\tdouble analogueGain;\n> +\t} agc;\n>  };\n>  \n>  } /* namespace ipa::ipu3 */\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 82506461..07b1d11c 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -22,12 +22,12 @@\n>  \n>  #include \"libcamera/internal/framebuffer.h\"\n>  \n> +#include \"algorithms/agc.h\"\n>  #include \"algorithms/algorithm.h\"\n\nI don't like including an algorithm, then including the algorithm header.\n\nEither we include the algorithm header first, and ignore alphabetical\nordering, or we don't need to include algorithm ?\n\nI think this is an instance where we should ignore alphabetical - But\nmaybe Laurent has different ideas.\n\n\n>  #include \"algorithms/awb.h\"\n>  #include \"algorithms/contrast.h\"\n>  #include \"ipa_context.h\"\n>  \n> -#include \"ipu3_agc.h\"\n>  #include \"libipa/camera_sensor_helper.h\"\n>  \n>  static constexpr uint32_t kMaxCellWidthPerSet = 160;\n> @@ -37,6 +37,8 @@ namespace libcamera {\n>  \n>  LOG_DEFINE_CATEGORY(IPAIPU3)\n>  \n> +using namespace std::literals::chrono_literals;\n> +\n\nWhy has this sprung up in here? Presumably regarding Duration or something?\n\nShould this be specified in the header that brings that in ?\n\n\n>  namespace ipa::ipu3 {\n>  \n>  class IPAIPU3 : public IPAIPU3Interface\n> @@ -81,8 +83,6 @@ private:\n>  \tuint32_t minGain_;\n>  \tuint32_t maxGain_;\n>  \n> -\t/* Interface to the AEC/AGC algorithm */\n> -\tstd::unique_ptr<IPU3Agc> agcAlgo_;\n>  \t/* Interface to the Camera Helper */\n>  \tstd::unique_ptr<CameraSensorHelper> camHelper_;\n>  \n> @@ -103,6 +103,7 @@ int IPAIPU3::init(const IPASettings &settings)\n>  \t}\n>  \n>  \t/* Construct our Algorithms */\n> +\talgorithms_.emplace_back(new algorithms::Agc());\n>  \talgorithms_.emplace_back(new algorithms::Awb());\n>  \talgorithms_.emplace_back(new algorithms::Contrast());\n>  \n> @@ -214,11 +215,14 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)\n>  \tcalculateBdsGrid(configInfo.bdsOutputSize);\n>  \tcontext_.awb.grid.bdsOutputSize = configInfo.bdsOutputSize;\n>  \n> +\t/* Prepare AGC parameters */\n> +\tcontext_.agc.lineDuration = sensorInfo_.lineLength * 1.0s / sensorInfo_.pixelRate;\n> +\tcontext_.agc.shutterTime = exposure_ * context_.agc.lineDuration;\n> +\tcontext_.agc.analogueGain = camHelper_->gain(gain_);\n\nHow do we avoid algorithm specific code living in this function.\nCan this be handled during Agc::configure()?\n\nWhat information do we need to pass into configure to make that happen?\n\n\n\n> +\n>  \tconfigureAlgorithms();\n\nit might even be that the implementation of configureAlgorithms() may as\nwell be directly coded in here....\n\nSame for the others potentially too.\n\nMight be something to revisit later.\n\n>  \n>  \tbdsGrid_ = context_.awb.grid.bdsGrid;\n> -\tagcAlgo_ = std::make_unique<IPU3Agc>();\n> -\tagcAlgo_->initialise(bdsGrid_, sensorInfo_);\n>  \n>  \treturn 0;\n>  }\n> @@ -341,12 +345,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n>  \t/* Run the process for each algorithm on the stats */\n>  \tprocessAlgorithms(stats);\n>  \n> -\tdouble gain = camHelper_->gain(gain_);\n> -\tagcAlgo_->process(stats, exposure_, gain);\n> -\tgain_ = camHelper_->gainCode(gain);\n> -\n> -\tif (agcAlgo_->updateControls())\n> -\t\tsetControls(frame);\n> +\tsetControls(frame);\n\nWill we always have controls to set?\n\nShould we have a control list in the IPAContext that can be updated, or\na flag?\n\n\nNot necessarily for this patch - but in trying to think about the\noverall design goals.\n\n>  \n>  \t/* \\todo Use VBlank value calculated from each frame exposure. */\n>  \tint64_t frameDuration = sensorInfo_.lineLength * (defVBlank_ + sensorInfo_.outputSize.height) /\n> @@ -365,6 +364,10 @@ void IPAIPU3::setControls(unsigned int frame)\n>  \tIPU3Action op;\n>  \top.op = ActionSetSensorControls;\n>  \n> +\t/* Convert gain and exposure */\n> +\tgain_ = camHelper_->gainCode(context_.agc.analogueGain);\n> +\texposure_ = context_.agc.shutterTime / context_.agc.lineDuration;\n> +\n>  \tControlList ctrls(ctrls_);\n>  \tctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));\n>  \tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));\n> diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build\n> index d1126947..39320980 100644\n> --- a/src/ipa/ipu3/meson.build\n> +++ b/src/ipa/ipu3/meson.build\n> @@ -6,7 +6,6 @@ ipa_name = 'ipa_ipu3'\n>  \n>  ipu3_ipa_sources = files([\n>      'ipu3.cpp',\n> -    'ipu3_agc.cpp',\n>  ])\n>  \n>  ipu3_ipa_sources += ipu3_ipa_algorithms\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 3CAA0BD87D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  9 Aug 2021 11:05:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9EF3868826;\n\tMon,  9 Aug 2021 13:05:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7B85D60269\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  9 Aug 2021 13:05:45 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EC364466;\n\tMon,  9 Aug 2021 13:05:44 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"dzN/vtrZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628507145;\n\tbh=ltSfMZDm7UxQoRu0gHN0iK9/95Bb1RkhQk94L3dmNfc=;\n\th=To:References:From:Subject:Date:In-Reply-To:From;\n\tb=dzN/vtrZchgUe8EAT2PdUxS/olAStbtu5nim6Po0QYTSU1q4M0EAyWdR08DiysIEn\n\tTCMmGzpEX1bQhO4Hi6CqeQCMIDihKRTxkfxE4PLiHj55cdj0GOwR5GeyFLnaB6QVVZ\n\tWSr3qGpXVDqZxx4t+9cC+u0v0TmbkEhq7xdWX/t8=","To":"Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210809092007.79067-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20210809092007.79067-6-jeanmichel.hautbois@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<3b8146b3-8c22-3385-b251-f3ba163d109f@ideasonboard.com>","Date":"Mon, 9 Aug 2021 12:05:42 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"<20210809092007.79067-6-jeanmichel.hautbois@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [RFC PATCH 5/5] ipa: ipu3: Move IPU3 agc into\n\talgorithms","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]