[{"id":18935,"web_url":"https://patchwork.libcamera.org/comment/18935/","msgid":"<YR2EQGv0/AtpM0a1@pendragon.ideasonboard.com>","date":"2021-08-18T22:05:52","subject":"Re: [libcamera-devel] [PATCH v3 9/9] ipa: ipu3: Move IPU3 agc into\n\talgorithms","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nThank you for the patch.\n\nOn Wed, Aug 18, 2021 at 05:54:03PM +0200, Jean-Michel Hautbois wrote:\n> Now that the interface is properly used by the AGC class, move it into\n> ipa::ipu3::algorithms and let the loops do the calls.\n> As we need to exchange the exposure_ and gain_ by passing them through the\n> FrameContext, use the calculated values in setControls() function to\n> ease the reading.\n> \n> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>  .../ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} | 21 +++++++++----------\n>  src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} | 20 +++++++++---------\n>  src/ipa/ipu3/algorithms/meson.build           |  1 +\n>  src/ipa/ipu3/ipu3.cpp                         | 15 +++++--------\n>  src/ipa/ipu3/meson.build                      |  1 -\n>  5 files changed, 26 insertions(+), 32 deletions(-)\n>  rename src/ipa/ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} (93%)\n>  rename src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} (73%)\n> \n> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> similarity index 93%\n> rename from src/ipa/ipu3/ipu3_agc.cpp\n> rename to src/ipa/ipu3/algorithms/agc.cpp\n> index 1c1f5fb5..bb119cb4 100644\n> --- a/src/ipa/ipu3/ipu3_agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -5,7 +5,7 @@\n>   * ipu3_agc.cpp - AGC/AEC control algorithm\n>   */\n>  \n> -#include \"ipu3_agc.h\"\n> +#include \"agc.h\"\n>  \n>  #include <algorithm>\n>  #include <chrono>\n> @@ -21,7 +21,7 @@ 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> @@ -50,14 +50,13 @@ 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), iqMean_(0.0), lineDuration_(0s),\n> -\t  maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s),\n> -\t  currentExposure_(0s), currentExposureNoDg_(0s)\n\nWhy can those two fields not be initialized anymore ?\n\n> +\t  maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s)\n>  {\n>  }\n>  \n> -int IPU3Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n> +int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n>  {\n>  \taeGrid_ = context.configuration.grid.bdsGrid;\n>  \n> @@ -67,7 +66,7 @@ int IPU3Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)\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> @@ -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(uint32_t &exposure, double &gain)\n>  {\n>  \t/* Algorithm initialization should wait for first valid frames */\n>  \t/* \\todo - have a number of frames given by DelayedControls ?\n> @@ -185,7 +184,7 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)\n>  \tlastFrame_ = frameCount_;\n>  }\n>  \n> -void IPU3Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n> +void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>  {\n>  \tuint32_t &exposure = context.frameContext.agc.exposure;\n>  \tdouble &gain = context.frameContext.agc.gain;\n> @@ -194,6 +193,6 @@ void IPU3Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\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 73%\n> rename from src/ipa/ipu3/ipu3_agc.h\n> rename to src/ipa/ipu3/algorithms/agc.h\n> index 0e922664..1739d2fd 100644\n> --- a/src/ipa/ipu3/ipu3_agc.h\n> +++ b/src/ipa/ipu3/algorithms/agc.h\n> @@ -2,10 +2,10 @@\n>  /*\n>   * Copyright (C) 2021, Ideas On Board\n>   *\n> - * ipu3_agc.h - IPU3 AGC/AEC control algorithm\n> + * 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> @@ -13,21 +13,21 @@\n>  \n>  #include <libcamera/geometry.h>\n>  \n> -#include \"algorithms/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 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, const IPAConfigInfo &configInfo) override;\n>  \tvoid process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;\n> @@ -53,8 +53,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 87377f4e..acaadd90 100644\n> --- a/src/ipa/ipu3/algorithms/meson.build\n> +++ b/src/ipa/ipu3/algorithms/meson.build\n> @@ -2,6 +2,7 @@\n>  \n>  ipu3_ipa_algorithms = files([\n>      'algorithm.cpp',\n> +    'agc.cpp',\n\nWrong alphabetical order.\n\n>      'awb.cpp',\n>      'contrast.cpp',\n>  ])\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index f4f49025..d73ec79d 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -30,9 +30,9 @@\n>  #include \"libcamera/internal/mapped_framebuffer.h\"\n>  \n>  #include \"algorithms/algorithm.h\"\n> +#include \"algorithms/agc.h\"\n\nHere too.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n>  #include \"algorithms/awb.h\"\n>  #include \"algorithms/contrast.h\"\n> -#include \"ipu3_agc.h\"\n>  #include \"libipa/camera_sensor_helper.h\"\n>  \n>  static constexpr uint32_t kMaxCellWidthPerSet = 160;\n> @@ -136,8 +136,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> @@ -218,6 +216,7 @@ int IPAIPU3::init(const IPASettings &settings,\n>  \t*ipaControls = ControlInfoMap(std::move(controls), controls::controls);\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> @@ -337,10 +336,6 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)\n>  \t\t\treturn ret;\n>  \t}\n>  \n> -\n> -\tagcAlgo_ = std::make_unique<IPU3Agc>();\n> -\tagcAlgo_->configure(context_, configInfo);\n> -\n>  \treturn 0;\n>  }\n>  \n> @@ -436,9 +431,6 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n>  \tfor (auto const &algo : algorithms_)\n>  \t\talgo->process(context_, stats);\n>  \n> -\tagcAlgo_->process(context_, stats);\n> -\texposure_ = context_.frameContext.agc.exposure;\n> -\tgain_ = camHelper_->gainCode(context_.frameContext.agc.gain);\n>  \tsetControls(frame);\n>  \n>  \t/* \\todo Use VBlank value calculated from each frame exposure. */\n> @@ -458,6 +450,9 @@ void IPAIPU3::setControls(unsigned int frame)\n>  \tIPU3Action op;\n>  \top.op = ActionSetSensorControls;\n>  \n> +\texposure_ = context_.frameContext.agc.exposure;\n> +\tgain_ = camHelper_->gainCode(context_.frameContext.agc.gain);\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 6A7C4BD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 18 Aug 2021 22:06:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 76D97688A3;\n\tThu, 19 Aug 2021 00:06:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4EB666025C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 19 Aug 2021 00:06:01 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 910252A8;\n\tThu, 19 Aug 2021 00:06:00 +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=\"Hs99/dkw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629324360;\n\tbh=0EKr1qByMtmS1hEcJh0gQQllJtklmO/D50kh6QQFB3c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Hs99/dkwjqFcHThkmo1KnRkvWnXCdR/kP63P2+KEav2zWZYiGgubA19TaIws8rL+M\n\tUF07kxPtLIBXWBpcvGiPWkVvbzRWHtlnvvXvrU4FUn4qvoQzxtxNCilb68kmGz+GLR\n\thNPJXpuYon53x7xe8HDOnTGXMnZOeHS0Vdh5xWP8=","Date":"Thu, 19 Aug 2021 01:05:52 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>","Message-ID":"<YR2EQGv0/AtpM0a1@pendragon.ideasonboard.com>","References":"<20210818155403.268694-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20210818155403.268694-10-jeanmichel.hautbois@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210818155403.268694-10-jeanmichel.hautbois@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 9/9] 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>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":18936,"web_url":"https://patchwork.libcamera.org/comment/18936/","msgid":"<f797387b-6630-c306-ef3b-ef0fb389898f@ideasonboard.com>","date":"2021-08-18T22:09:03","subject":"Re: [libcamera-devel] [PATCH v3 9/9] 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 18/08/2021 23:05, Laurent Pinchart wrote:\n> Hi Jean-Michel,\n> \n> Thank you for the patch.\n> \n> On Wed, Aug 18, 2021 at 05:54:03PM +0200, Jean-Michel Hautbois wrote:\n>> Now that the interface is properly used by the AGC class, move it into\n>> ipa::ipu3::algorithms and let the loops do the calls.\n>> As we need to exchange the exposure_ and gain_ by passing them through the\n>> FrameContext, use the calculated values in setControls() function to\n>> ease the reading.\n>>\n>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>> ---\n>>  .../ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} | 21 +++++++++----------\n>>  src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} | 20 +++++++++---------\n>>  src/ipa/ipu3/algorithms/meson.build           |  1 +\n>>  src/ipa/ipu3/ipu3.cpp                         | 15 +++++--------\n>>  src/ipa/ipu3/meson.build                      |  1 -\n>>  5 files changed, 26 insertions(+), 32 deletions(-)\n>>  rename src/ipa/ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} (93%)\n>>  rename src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} (73%)\n>>\n>> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n>> similarity index 93%\n>> rename from src/ipa/ipu3/ipu3_agc.cpp\n>> rename to src/ipa/ipu3/algorithms/agc.cpp\n>> index 1c1f5fb5..bb119cb4 100644\n>> --- a/src/ipa/ipu3/ipu3_agc.cpp\n>> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n>> @@ -5,7 +5,7 @@\n>>   * ipu3_agc.cpp - AGC/AEC control algorithm\n>>   */\n>>  \n>> -#include \"ipu3_agc.h\"\n>> +#include \"agc.h\"\n>>  \n>>  #include <algorithm>\n>>  #include <chrono>\n>> @@ -21,7 +21,7 @@ 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>> @@ -50,14 +50,13 @@ 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), iqMean_(0.0), lineDuration_(0s),\n>> -\t  maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s),\n>> -\t  currentExposure_(0s), currentExposureNoDg_(0s)\n> \n> Why can those two fields not be initialized anymore ?\n> \n>> +\t  maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s)\n>>  {\n>>  }\n>>  \n>> -int IPU3Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n>> +int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n>>  {\n>>  \taeGrid_ = context.configuration.grid.bdsGrid;\n>>  \n>> @@ -67,7 +66,7 @@ int IPU3Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)\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>> @@ -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(uint32_t &exposure, double &gain)\n>>  {\n>>  \t/* Algorithm initialization should wait for first valid frames */\n>>  \t/* \\todo - have a number of frames given by DelayedControls ?\n>> @@ -185,7 +184,7 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)\n>>  \tlastFrame_ = frameCount_;\n>>  }\n>>  \n>> -void IPU3Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>> +void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n>>  {\n>>  \tuint32_t &exposure = context.frameContext.agc.exposure;\n>>  \tdouble &gain = context.frameContext.agc.gain;\n>> @@ -194,6 +193,6 @@ void IPU3Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\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 73%\n>> rename from src/ipa/ipu3/ipu3_agc.h\n>> rename to src/ipa/ipu3/algorithms/agc.h\n>> index 0e922664..1739d2fd 100644\n>> --- a/src/ipa/ipu3/ipu3_agc.h\n>> +++ b/src/ipa/ipu3/algorithms/agc.h\n>> @@ -2,10 +2,10 @@\n>>  /*\n>>   * Copyright (C) 2021, Ideas On Board\n>>   *\n>> - * ipu3_agc.h - IPU3 AGC/AEC control algorithm\n>> + * 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>> @@ -13,21 +13,21 @@\n>>  \n>>  #include <libcamera/geometry.h>\n>>  \n>> -#include \"algorithms/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 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, const IPAConfigInfo &configInfo) override;\n>>  \tvoid process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;\n>> @@ -53,8 +53,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 87377f4e..acaadd90 100644\n>> --- a/src/ipa/ipu3/algorithms/meson.build\n>> +++ b/src/ipa/ipu3/algorithms/meson.build\n>> @@ -2,6 +2,7 @@\n>>  \n>>  ipu3_ipa_algorithms = files([\n>>      'algorithm.cpp',\n>> +    'agc.cpp',\n> \n> Wrong alphabetical order.\n\nThis one doesn't worry me too much if it's sorted alphabetically...\n\n> \n>>      'awb.cpp',\n>>      'contrast.cpp',\n>>  ])\n>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n>> index f4f49025..d73ec79d 100644\n>> --- a/src/ipa/ipu3/ipu3.cpp\n>> +++ b/src/ipa/ipu3/ipu3.cpp\n>> @@ -30,9 +30,9 @@\n>>  #include \"libcamera/internal/mapped_framebuffer.h\"\n>>  \n>>  #include \"algorithms/algorithm.h\"\n>> +#include \"algorithms/agc.h\"\n> \n> Here too.\n> \n\nBut this one does.\n\nIf agc.h comes before algorithm.h, then by definition we're relying upon\nthe inclusion of algorithm.h from agc.h, so there's no point it being\nhere at all.\n\nBut I'd personally keep it, and keep it at the beginning of the\ninclusions, as an exception to the alphabetical ordering.\n\nThe other algorithms can be sorted alphabetically though\n\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n>>  #include \"algorithms/awb.h\"\n>>  #include \"algorithms/contrast.h\"\n>> -#include \"ipu3_agc.h\"\n>>  #include \"libipa/camera_sensor_helper.h\"\n>>  \n>>  static constexpr uint32_t kMaxCellWidthPerSet = 160;\n>> @@ -136,8 +136,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>> @@ -218,6 +216,7 @@ int IPAIPU3::init(const IPASettings &settings,\n>>  \t*ipaControls = ControlInfoMap(std::move(controls), controls::controls);\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>> @@ -337,10 +336,6 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)\n>>  \t\t\treturn ret;\n>>  \t}\n>>  \n>> -\n>> -\tagcAlgo_ = std::make_unique<IPU3Agc>();\n>> -\tagcAlgo_->configure(context_, configInfo);\n>> -\n>>  \treturn 0;\n>>  }\n>>  \n>> @@ -436,9 +431,6 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n>>  \tfor (auto const &algo : algorithms_)\n>>  \t\talgo->process(context_, stats);\n>>  \n>> -\tagcAlgo_->process(context_, stats);\n>> -\texposure_ = context_.frameContext.agc.exposure;\n>> -\tgain_ = camHelper_->gainCode(context_.frameContext.agc.gain);\n>>  \tsetControls(frame);\n>>  \n>>  \t/* \\todo Use VBlank value calculated from each frame exposure. */\n>> @@ -458,6 +450,9 @@ void IPAIPU3::setControls(unsigned int frame)\n>>  \tIPU3Action op;\n>>  \top.op = ActionSetSensorControls;\n>>  \n>> +\texposure_ = context_.frameContext.agc.exposure;\n>> +\tgain_ = camHelper_->gainCode(context_.frameContext.agc.gain);\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 62643BD87D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 18 Aug 2021 22:09:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C5A4368895;\n\tThu, 19 Aug 2021 00:09:08 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8CC196025C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 19 Aug 2021 00:09:07 +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 93B772A8;\n\tThu, 19 Aug 2021 00:09:06 +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=\"n3T2nsgM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629324546;\n\tbh=WKDNNbtk4v6qmDRwGsD6VQALEykeqEQI9IIMWCvXCLE=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=n3T2nsgM7JxCBMBJ+wRb4lwYqeP7C4DadSoPQb39RsMtcljfWAOqCseNP3dL0Mtu8\n\tartSZeUZEolW7MMtV4LjYQdEO2YxJs7XdIbMiTRbhnrSJTYXV4AI1v1x7zgTBDmZAf\n\t8QlFQ765y7eI7hv/pUP61J1eVgG5NO7vIDiopfok=","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tJean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>","References":"<20210818155403.268694-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20210818155403.268694-10-jeanmichel.hautbois@ideasonboard.com>\n\t<YR2EQGv0/AtpM0a1@pendragon.ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<f797387b-6630-c306-ef3b-ef0fb389898f@ideasonboard.com>","Date":"Wed, 18 Aug 2021 23:09:03 +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":"<YR2EQGv0/AtpM0a1@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 9/9] 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>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":18937,"web_url":"https://patchwork.libcamera.org/comment/18937/","msgid":"<YR2HL8dEMJN3u5kT@pendragon.ideasonboard.com>","date":"2021-08-18T22:18:23","subject":"Re: [libcamera-devel] [PATCH v3 9/9] ipa: ipu3: Move IPU3 agc into\n\talgorithms","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Wed, Aug 18, 2021 at 11:09:03PM +0100, Kieran Bingham wrote:\n> On 18/08/2021 23:05, Laurent Pinchart wrote:\n> > On Wed, Aug 18, 2021 at 05:54:03PM +0200, Jean-Michel Hautbois wrote:\n> >> Now that the interface is properly used by the AGC class, move it into\n> >> ipa::ipu3::algorithms and let the loops do the calls.\n> >> As we need to exchange the exposure_ and gain_ by passing them through the\n> >> FrameContext, use the calculated values in setControls() function to\n> >> ease the reading.\n> >>\n> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n> >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> >> ---\n> >>  .../ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} | 21 +++++++++----------\n> >>  src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} | 20 +++++++++---------\n> >>  src/ipa/ipu3/algorithms/meson.build           |  1 +\n> >>  src/ipa/ipu3/ipu3.cpp                         | 15 +++++--------\n> >>  src/ipa/ipu3/meson.build                      |  1 -\n> >>  5 files changed, 26 insertions(+), 32 deletions(-)\n> >>  rename src/ipa/ipu3/{ipu3_agc.cpp => algorithms/agc.cpp} (93%)\n> >>  rename src/ipa/ipu3/{ipu3_agc.h => algorithms/agc.h} (73%)\n> >>\n> >> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> >> similarity index 93%\n> >> rename from src/ipa/ipu3/ipu3_agc.cpp\n> >> rename to src/ipa/ipu3/algorithms/agc.cpp\n> >> index 1c1f5fb5..bb119cb4 100644\n> >> --- a/src/ipa/ipu3/ipu3_agc.cpp\n> >> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> >> @@ -5,7 +5,7 @@\n> >>   * ipu3_agc.cpp - AGC/AEC control algorithm\n> >>   */\n> >>  \n> >> -#include \"ipu3_agc.h\"\n> >> +#include \"agc.h\"\n> >>  \n> >>  #include <algorithm>\n> >>  #include <chrono>\n> >> @@ -21,7 +21,7 @@ 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> >> @@ -50,14 +50,13 @@ 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), iqMean_(0.0), lineDuration_(0s),\n> >> -\t  maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s),\n> >> -\t  currentExposure_(0s), currentExposureNoDg_(0s)\n> > \n> > Why can those two fields not be initialized anymore ?\n> > \n> >> +\t  maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s)\n> >>  {\n> >>  }\n> >>  \n> >> -int IPU3Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n> >> +int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n> >>  {\n> >>  \taeGrid_ = context.configuration.grid.bdsGrid;\n> >>  \n> >> @@ -67,7 +66,7 @@ int IPU3Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)\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> >> @@ -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(uint32_t &exposure, double &gain)\n> >>  {\n> >>  \t/* Algorithm initialization should wait for first valid frames */\n> >>  \t/* \\todo - have a number of frames given by DelayedControls ?\n> >> @@ -185,7 +184,7 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, double &gain)\n> >>  \tlastFrame_ = frameCount_;\n> >>  }\n> >>  \n> >> -void IPU3Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n> >> +void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n> >>  {\n> >>  \tuint32_t &exposure = context.frameContext.agc.exposure;\n> >>  \tdouble &gain = context.frameContext.agc.gain;\n> >> @@ -194,6 +193,6 @@ void IPU3Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\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 73%\n> >> rename from src/ipa/ipu3/ipu3_agc.h\n> >> rename to src/ipa/ipu3/algorithms/agc.h\n> >> index 0e922664..1739d2fd 100644\n> >> --- a/src/ipa/ipu3/ipu3_agc.h\n> >> +++ b/src/ipa/ipu3/algorithms/agc.h\n> >> @@ -2,10 +2,10 @@\n> >>  /*\n> >>   * Copyright (C) 2021, Ideas On Board\n> >>   *\n> >> - * ipu3_agc.h - IPU3 AGC/AEC control algorithm\n> >> + * 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> >> @@ -13,21 +13,21 @@\n> >>  \n> >>  #include <libcamera/geometry.h>\n> >>  \n> >> -#include \"algorithms/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 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, const IPAConfigInfo &configInfo) override;\n> >>  \tvoid process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;\n> >> @@ -53,8 +53,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 87377f4e..acaadd90 100644\n> >> --- a/src/ipa/ipu3/algorithms/meson.build\n> >> +++ b/src/ipa/ipu3/algorithms/meson.build\n> >> @@ -2,6 +2,7 @@\n> >>  \n> >>  ipu3_ipa_algorithms = files([\n> >>      'algorithm.cpp',\n> >> +    'agc.cpp',\n> > \n> > Wrong alphabetical order.\n> \n> This one doesn't worry me too much if it's sorted alphabetically...\n> \n> >>      'awb.cpp',\n> >>      'contrast.cpp',\n> >>  ])\n> >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> >> index f4f49025..d73ec79d 100644\n> >> --- a/src/ipa/ipu3/ipu3.cpp\n> >> +++ b/src/ipa/ipu3/ipu3.cpp\n> >> @@ -30,9 +30,9 @@\n> >>  #include \"libcamera/internal/mapped_framebuffer.h\"\n> >>  \n> >>  #include \"algorithms/algorithm.h\"\n> >> +#include \"algorithms/agc.h\"\n> > \n> > Here too.\n> \n> But this one does.\n> \n> If agc.h comes before algorithm.h, then by definition we're relying upon\n> the inclusion of algorithm.h from agc.h, so there's no point it being\n> here at all.\n\nI've thought the same, but every exception will be flagged repeatedly by\ncheckstyle.py. If it's not clear why the exception is there, someone\nwill probably submit a patch. That's why I'd rather sort headers\nalphabetically here too, and then forget about it :-) I wouldn't be\nagainst dropping algorithm.h if it bothers you. Or algorithm.h could\nmove to the parent directory, with only the algorithm implementation\nstored in algorithms/. It's really nitpicking :-)\n\n> But I'd personally keep it, and keep it at the beginning of the\n> inclusions, as an exception to the alphabetical ordering.\n> \n> The other algorithms can be sorted alphabetically though\n> \n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > \n> >>  #include \"algorithms/awb.h\"\n> >>  #include \"algorithms/contrast.h\"\n> >> -#include \"ipu3_agc.h\"\n> >>  #include \"libipa/camera_sensor_helper.h\"\n> >>  \n> >>  static constexpr uint32_t kMaxCellWidthPerSet = 160;\n> >> @@ -136,8 +136,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> >> @@ -218,6 +216,7 @@ int IPAIPU3::init(const IPASettings &settings,\n> >>  \t*ipaControls = ControlInfoMap(std::move(controls), controls::controls);\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> >> @@ -337,10 +336,6 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)\n> >>  \t\t\treturn ret;\n> >>  \t}\n> >>  \n> >> -\n> >> -\tagcAlgo_ = std::make_unique<IPU3Agc>();\n> >> -\tagcAlgo_->configure(context_, configInfo);\n> >> -\n> >>  \treturn 0;\n> >>  }\n> >>  \n> >> @@ -436,9 +431,6 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n> >>  \tfor (auto const &algo : algorithms_)\n> >>  \t\talgo->process(context_, stats);\n> >>  \n> >> -\tagcAlgo_->process(context_, stats);\n> >> -\texposure_ = context_.frameContext.agc.exposure;\n> >> -\tgain_ = camHelper_->gainCode(context_.frameContext.agc.gain);\n> >>  \tsetControls(frame);\n> >>  \n> >>  \t/* \\todo Use VBlank value calculated from each frame exposure. */\n> >> @@ -458,6 +450,9 @@ void IPAIPU3::setControls(unsigned int frame)\n> >>  \tIPU3Action op;\n> >>  \top.op = ActionSetSensorControls;\n> >>  \n> >> +\texposure_ = context_.frameContext.agc.exposure;\n> >> +\tgain_ = camHelper_->gainCode(context_.frameContext.agc.gain);\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 92C61BD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 18 Aug 2021 22:18:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 121F768895;\n\tThu, 19 Aug 2021 00:18:34 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8B8656025C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 19 Aug 2021 00:18:32 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0E0DF246D;\n\tThu, 19 Aug 2021 00:18:31 +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=\"FTcGid6y\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629325112;\n\tbh=rf7j67Hit+r/5Au+mzxGxigzNMvClzWzmIT9psv7pPU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FTcGid6y37Ta2AQiO8s5Qk3zGaTsJtOViKCT8SmyvQbEXM23u04hNpzXghGhcf0T4\n\t9mkrQP5nXMr/pjAqza58GeRtYW2v706jvowgyhXQ+XCCkOrU+bcaakU4JUpqNyFwnd\n\tQrNyB8ecg4XnjUf067a+SIt4TIDBg7y4BimcsztE=","Date":"Thu, 19 Aug 2021 01:18:23 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<YR2HL8dEMJN3u5kT@pendragon.ideasonboard.com>","References":"<20210818155403.268694-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20210818155403.268694-10-jeanmichel.hautbois@ideasonboard.com>\n\t<YR2EQGv0/AtpM0a1@pendragon.ideasonboard.com>\n\t<f797387b-6630-c306-ef3b-ef0fb389898f@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<f797387b-6630-c306-ef3b-ef0fb389898f@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 9/9] 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>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]