[{"id":31287,"web_url":"https://patchwork.libcamera.org/comment/31287/","msgid":"<87r09e5cst.fsf@redhat.com>","date":"2024-09-20T19:45:54","subject":"Re: [PATCH v7 17/18] libcamera: software_isp: Move exposure+gain to\n\tan algorithm module","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Milan Zamazal <mzamazal@redhat.com> writes:\n\n> This is the last step to fully convert software ISP to Algorithm-based\n> processing.\n>\n> The newly introduced frameContext.sensor properties are set, and the\n> updated code moved, before calling Algorithm::process() to have the\n> values up-to-date in stats processing.\n>\n> Resolves software ISP TODO #10.\n>\n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>  src/ipa/simple/algorithms/agc.cpp     | 139 +++++++++++++++++++++++\n>  src/ipa/simple/algorithms/agc.h       |  33 ++++++\n>  src/ipa/simple/algorithms/meson.build |   1 +\n>  src/ipa/simple/data/uncalibrated.yaml |   1 +\n>  src/ipa/simple/ipa_context.cpp        |  11 ++\n>  src/ipa/simple/ipa_context.h          |  18 +++\n>  src/ipa/simple/soft_simple.cpp        | 157 +++++---------------------\n>  src/libcamera/software_isp/TODO       |  10 --\n>  8 files changed, 233 insertions(+), 137 deletions(-)\n>  create mode 100644 src/ipa/simple/algorithms/agc.cpp\n>  create mode 100644 src/ipa/simple/algorithms/agc.h\n>\n> diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp\n> new file mode 100644\n> index 00000000..783dfb8b\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/agc.cpp\n> @@ -0,0 +1,139 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Red Hat Inc.\n> + *\n> + * Exposure and gain\n> + */\n> +\n> +#include \"agc.h\"\n> +\n> +#include <stdint.h>\n> +\n> +#include <libcamera/base/log.h>\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(IPASoftExposure)\n> +\n> +namespace ipa::soft::algorithms {\n> +\n> +/*\n> + * The number of bins to use for the optimal exposure calculations.\n> + */\n> +static constexpr unsigned int kExposureBinsCount = 5;\n> +\n> +/*\n> + * The exposure is optimal when the mean sample value of the histogram is\n> + * in the middle of the range.\n> + */\n> +static constexpr float kExposureOptimal = kExposureBinsCount / 2.0;\n> +\n> +/*\n> + * The below value implements the hysteresis for the exposure adjustment.\n> + * It is small enough to have the exposure close to the optimal, and is big\n> + * enough to prevent the exposure from wobbling around the optimal value.\n> + */\n> +static constexpr float kExposureSatisfactory = 0.2;\n> +\n> +Agc::Agc()\n> +{\n> +}\n> +\n> +void Agc::updateExposure(IPAContext &context, double exposureMSV)\n> +{\n> +\t/*\n> +\t * kExpDenominator of 10 gives ~10% increment/decrement;\n> +\t * kExpDenominator of 5 - about ~20%\n> +\t */\n> +\tstatic constexpr uint8_t kExpDenominator = 10;\n> +\tstatic constexpr uint8_t kExpNumeratorUp = kExpDenominator + 1;\n> +\tstatic constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1;\n> +\n> +\tdouble next;\n> +\tint32_t &exposure = context.activeState.agc.exposure;\n> +\tdouble &again = context.activeState.agc.again;\n> +\n> +\tif (exposureMSV < kExposureOptimal - kExposureSatisfactory) {\n> +\t\tnext = exposure * kExpNumeratorUp / kExpDenominator;\n> +\t\tif (next - exposure < 1)\n> +\t\t\texposure += 1;\n> +\t\telse\n> +\t\t\texposure = next;\n> +\t\tif (exposure >= context.configuration.agc.exposureMax) {\n> +\t\t\tnext = again * kExpNumeratorUp / kExpDenominator;\n> +\t\t\tif (next - again < context.configuration.agc.againMinStep)\n> +\t\t\t\tagain += context.configuration.agc.againMinStep;\n> +\t\t\telse\n> +\t\t\t\tagain = next;\n> +\t\t}\n> +\t}\n> +\n> +\tif (exposureMSV > kExposureOptimal + kExposureSatisfactory) {\n> +\t\tif (exposure == context.configuration.agc.exposureMax &&\n> +\t\t    again > context.configuration.agc.againMin) {\n> +\t\t\tnext = again * kExpNumeratorDown / kExpDenominator;\n> +\t\t\tif (again - next < context.configuration.agc.againMinStep)\n> +\t\t\t\tagain -= context.configuration.agc.againMinStep;\n> +\t\t\telse\n> +\t\t\t\tagain = next;\n> +\t\t} else {\n> +\t\t\tnext = exposure * kExpNumeratorDown / kExpDenominator;\n> +\t\t\tif (exposure - next < 1)\n> +\t\t\t\texposure -= 1;\n> +\t\t\telse\n> +\t\t\t\texposure = next;\n> +\t\t}\n> +\t}\n> +\n> +\texposure = std::clamp(exposure, context.configuration.agc.exposureMin,\n> +\t\t\t      context.configuration.agc.exposureMax);\n> +\tagain = std::clamp(again, context.configuration.agc.againMin,\n> +\t\t\t   context.configuration.agc.againMax);\n> +\n> +\tLOG(IPASoftExposure, Debug)\n> +\t\t<< \"exposureMSV \" << exposureMSV\n> +\t\t<< \" exp \" << exposure << \" again \" << again;\n> +}\n> +\n> +void Agc::process(IPAContext &context,\n> +\t\t  [[maybe_unused]] const uint32_t frame,\n> +\t\t  [[maybe_unused]] IPAFrameContext &frameContext,\n> +\t\t  const SwIspStats *stats,\n> +\t\t  [[maybe_unused]] ControlList &metadata)\n> +{\n> +\t/*\n> +\t * Calculate Mean Sample Value (MSV) according to formula from:\n> +\t * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf\n> +\t */\n> +\tconst auto &histogram = stats->yHistogram;\n> +\tconst unsigned int blackLevelHistIdx =\n> +\t\tcontext.activeState.blc.level / (256 / SwIspStats::kYHistogramSize);\n> +\tconst unsigned int histogramSize =\n> +\t\tSwIspStats::kYHistogramSize - blackLevelHistIdx;\n> +\tconst unsigned int yHistValsPerBin = histogramSize / kExposureBinsCount;\n> +\tconst unsigned int yHistValsPerBinMod =\n> +\t\thistogramSize / (histogramSize % kExposureBinsCount + 1);\n> +\tint exposureBins[kExposureBinsCount] = {};\n> +\tunsigned int denom = 0;\n> +\tunsigned int num = 0;\n> +\n> +\tfor (unsigned int i = 0; i < histogramSize; i++) {\n> +\t\tunsigned int idx = (i - (i / yHistValsPerBinMod)) / yHistValsPerBin;\n> +\t\texposureBins[idx] += histogram[blackLevelHistIdx + i];\n> +\t}\n> +\n> +\tfor (unsigned int i = 0; i < kExposureBinsCount; i++) {\n> +\t\tLOG(IPASoftExposure, Debug) << i << \": \" << exposureBins[i];\n> +\t\tdenom += exposureBins[i];\n> +\t\tnum += exposureBins[i] * (i + 1);\n> +\t}\n> +\n> +\tfloat exposureMSV = (denom == 0 ? 0 : static_cast<float>(num) / denom);\n> +\tupdateExposure(context, exposureMSV);\n> +}\n> +\n> +REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n> +\n> +} /* namespace ipa::soft::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/simple/algorithms/agc.h b/src/ipa/simple/algorithms/agc.h\n> new file mode 100644\n> index 00000000..ad5fca9f\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/agc.h\n> @@ -0,0 +1,33 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Red Hat Inc.\n> + *\n> + * Exposure and gain\n> + */\n> +\n> +#pragma once\n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::soft::algorithms {\n> +\n> +class Agc : public Algorithm\n> +{\n> +public:\n> +\tAgc();\n> +\t~Agc() = default;\n> +\n> +\tvoid process(IPAContext &context, const uint32_t frame,\n> +\t\t     IPAFrameContext &frameContext,\n> +\t\t     const SwIspStats *stats,\n> +\t\t     ControlList &metadata) override;\n> +\n> +private:\n> +\tvoid updateExposure(IPAContext &context, double exposureMSV);\n> +};\n> +\n> +} /* namespace ipa::soft::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build\n> index f575611e..37a2eb53 100644\n> --- a/src/ipa/simple/algorithms/meson.build\n> +++ b/src/ipa/simple/algorithms/meson.build\n> @@ -2,6 +2,7 @@\n>  \n>  soft_simple_ipa_algorithms = files([\n>      'awb.cpp',\n> +    'agc.cpp',\n>      'blc.cpp',\n>      'lut.cpp',\n>  ])\n> diff --git a/src/ipa/simple/data/uncalibrated.yaml b/src/ipa/simple/data/uncalibrated.yaml\n> index 893a0b92..3f147112 100644\n> --- a/src/ipa/simple/data/uncalibrated.yaml\n> +++ b/src/ipa/simple/data/uncalibrated.yaml\n> @@ -6,4 +6,5 @@ algorithms:\n>    - BlackLevel:\n>    - Awb:\n>    - Lut:\n> +  - Agc:\n>  ...\n> diff --git a/src/ipa/simple/ipa_context.cpp b/src/ipa/simple/ipa_context.cpp\n> index 5fa492cb..3f94bbeb 100644\n> --- a/src/ipa/simple/ipa_context.cpp\n> +++ b/src/ipa/simple/ipa_context.cpp\n> @@ -77,6 +77,17 @@ namespace libcamera::ipa::soft {\n>   * \\brief Gain of blue color\n>   */\n>  \n> +/**\n> + * \\var IPAActiveState::agc\n> + * \\brief Context for the AGC algorithm\n> + *\n> + * \\var IPAActiveState::agc.exposure\n> + * \\brief Current exposure value\n> + *\n> + * \\var IPAActiveState::agc.again\n> + * \\brief Current analog gain value\n> + */\n> +\n>  /**\n>   * \\var IPAActiveState::gamma\n>   * \\brief Context for gamma in the Colors algorithm\n> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> index cf1ef3fd..eb095c85 100644\n> --- a/src/ipa/simple/ipa_context.h\n> +++ b/src/ipa/simple/ipa_context.h\n> @@ -10,6 +10,8 @@\n>  #include <array>\n>  #include <stdint.h>\n>  \n> +#include <libcamera/controls.h>\n> +\n>  #include <libipa/fc_queue.h>\n>  \n>  namespace libcamera {\n> @@ -18,6 +20,13 @@ namespace ipa::soft {\n>  \n>  struct IPASessionConfiguration {\n>  \tfloat gamma;\n> +\tstruct {\n> +\t\tint32_t exposureMin, exposureMax;\n> +\t\tdouble againMin, againMax, againMinStep;\n> +\t} agc;\n> +\tstruct {\n> +\t\tdouble level;\n> +\t} black;\n\nThis struct should be dropped, it's not used anywhere.\n\n>  };\n>  \n>  struct IPAActiveState {\n> @@ -31,6 +40,11 @@ struct IPAActiveState {\n>  \t\tdouble blue;\n>  \t} gains;\n>  \n> +\tstruct {\n> +\t\tint32_t exposure;\n> +\t\tdouble again;\n> +\t} agc;\n> +\n>  \tstatic constexpr unsigned int kGammaLookupSize = 1024;\n>  \tstruct {\n>  \t\tstd::array<double, kGammaLookupSize> gammaTable;\n> @@ -39,6 +53,10 @@ struct IPAActiveState {\n>  };\n>  \n>  struct IPAFrameContext : public FrameContext {\n> +\tstruct {\n> +\t\tuint32_t exposure;\n> +\t\tdouble gain;\n> +\t} sensor;\n>  };\n>  \n>  struct IPAContext {\n> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> index 60310a8e..b28c7039 100644\n> --- a/src/ipa/simple/soft_simple.cpp\n> +++ b/src/ipa/simple/soft_simple.cpp\n> @@ -34,23 +34,6 @@ LOG_DEFINE_CATEGORY(IPASoft)\n>  \n>  namespace ipa::soft {\n>  \n> -/*\n> - * The number of bins to use for the optimal exposure calculations.\n> - */\n> -static constexpr unsigned int kExposureBinsCount = 5;\n> -\n> -/*\n> - * The exposure is optimal when the mean sample value of the histogram is\n> - * in the middle of the range.\n> - */\n> -static constexpr float kExposureOptimal = kExposureBinsCount / 2.0;\n> -\n> -/*\n> - * The below value implements the hysteresis for the exposure adjustment.\n> - * It is small enough to have the exposure close to the optimal, and is big\n> - * enough to prevent the exposure from wobbling around the optimal value.\n> - */\n> -static constexpr float kExposureSatisfactory = 0.2;\n>  /* Maximum number of frame contexts to be held */\n>  static constexpr uint32_t kMaxFrameContexts = 16;\n>  \n> @@ -58,8 +41,7 @@ class IPASoftSimple : public ipa::soft::IPASoftInterface, public Module\n>  {\n>  public:\n>  \tIPASoftSimple()\n> -\t\t: params_(nullptr), stats_(nullptr),\n> -\t\t  context_({ {}, {}, { kMaxFrameContexts } })\n> +\t\t: context_({ {}, {}, { kMaxFrameContexts } })\n>  \t{\n>  \t}\n>  \n> @@ -92,11 +74,6 @@ private:\n>  \n>  \t/* Local parameter storage */\n>  \tstruct IPAContext context_;\n> -\n> -\tint32_t exposureMin_, exposureMax_;\n> -\tint32_t exposure_;\n> -\tdouble againMin_, againMax_, againMinStep_;\n> -\tdouble again_;\n>  };\n>  \n>  IPASoftSimple::~IPASoftSimple()\n> @@ -207,20 +184,23 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n>  \tconst ControlInfo &exposureInfo = sensorInfoMap_.find(V4L2_CID_EXPOSURE)->second;\n>  \tconst ControlInfo &gainInfo = sensorInfoMap_.find(V4L2_CID_ANALOGUE_GAIN)->second;\n>  \n> -\texposureMin_ = exposureInfo.min().get<int32_t>();\n> -\texposureMax_ = exposureInfo.max().get<int32_t>();\n> -\tif (!exposureMin_) {\n> +\tcontext_.configuration.agc.exposureMin = exposureInfo.min().get<int32_t>();\n> +\tcontext_.configuration.agc.exposureMax = exposureInfo.max().get<int32_t>();\n> +\tif (!context_.configuration.agc.exposureMin) {\n>  \t\tLOG(IPASoft, Warning) << \"Minimum exposure is zero, that can't be linear\";\n> -\t\texposureMin_ = 1;\n> +\t\tcontext_.configuration.agc.exposureMin = 1;\n>  \t}\n>  \n>  \tint32_t againMin = gainInfo.min().get<int32_t>();\n>  \tint32_t againMax = gainInfo.max().get<int32_t>();\n>  \n>  \tif (camHelper_) {\n> -\t\tagainMin_ = camHelper_->gain(againMin);\n> -\t\tagainMax_ = camHelper_->gain(againMax);\n> -\t\tagainMinStep_ = (againMax_ - againMin_) / 100.0;\n> +\t\tcontext_.configuration.agc.againMin = camHelper_->gain(againMin);\n> +\t\tcontext_.configuration.agc.againMax = camHelper_->gain(againMax);\n> +\t\tcontext_.configuration.agc.againMinStep =\n> +\t\t\t(context_.configuration.agc.againMax -\n> +\t\t\t context_.configuration.agc.againMin) /\n> +\t\t\t100.0;\n>  \t} else {\n>  \t\t/*\n>  \t\t * The camera sensor gain (g) is usually not equal to the value written\n> @@ -232,13 +212,14 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n>  \t\t * the AGC algorithm (abrupt near one edge, and very small near the\n>  \t\t * other) we limit the range of the gain values used.\n>  \t\t */\n> -\t\tagainMax_ = againMax;\n> +\t\tcontext_.configuration.agc.againMax = againMax;\n>  \t\tif (!againMin) {\n>  \t\t\tLOG(IPASoft, Warning)\n>  \t\t\t\t<< \"Minimum gain is zero, that can't be linear\";\n> -\t\t\tagainMin_ = std::min(100, againMin / 2 + againMax / 2);\n> +\t\t\tcontext_.configuration.agc.againMin =\n> +\t\t\t\tstd::min(100, againMin / 2 + againMax / 2);\n>  \t\t}\n> -\t\tagainMinStep_ = 1.0;\n> +\t\tcontext_.configuration.agc.againMinStep = 1.0;\n>  \t}\n>  \n>  \tfor (auto const &algo : algorithms()) {\n> @@ -247,9 +228,12 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n>  \t\t\treturn ret;\n>  \t}\n>  \n> -\tLOG(IPASoft, Info) << \"Exposure \" << exposureMin_ << \"-\" << exposureMax_\n> -\t\t\t   << \", gain \" << againMin_ << \"-\" << againMax_\n> -\t\t\t   << \" (\" << againMinStep_ << \")\";\n> +\tLOG(IPASoft, Info)\n> +\t\t<< \"Exposure \" << context_.configuration.agc.exposureMin << \"-\"\n> +\t\t<< context_.configuration.agc.exposureMax\n> +\t\t<< \", gain \" << context_.configuration.agc.againMin << \"-\"\n> +\t\t<< context_.configuration.agc.againMax\n> +\t\t<< \" (\" << context_.configuration.agc.againMinStep << \")\";\n>  \n>  \treturn 0;\n>  }\n> @@ -284,6 +268,12 @@ void IPASoftSimple::processStats(const uint32_t frame,\n>  \t\t\t\t const ControlList &sensorControls)\n>  {\n>  \tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> +\n> +\tframeContext.sensor.exposure =\n> +\t\tsensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();\n> +\tint32_t again = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();\n> +\tframeContext.sensor.gain = camHelper_ ? camHelper_->gain(again) : again;\n> +\n>  \t/*\n>  \t * Software ISP currently does not produce any metadata. Use an empty\n>  \t * ControlList for now.\n> @@ -294,37 +284,6 @@ void IPASoftSimple::processStats(const uint32_t frame,\n>  \tfor (auto const &algo : algorithms())\n>  \t\talgo->process(context_, frame, frameContext, stats_, metadata);\n>  \n> -\t/* \\todo Switch to the libipa/algorithm.h API someday. */\n> -\n> -\t/*\n> -\t * Calculate Mean Sample Value (MSV) according to formula from:\n> -\t * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf\n> -\t */\n> -\tconst uint8_t blackLevel = context_.activeState.blc.level;\n> -\tconst unsigned int blackLevelHistIdx =\n> -\t\tblackLevel / (256 / SwIspStats::kYHistogramSize);\n> -\tconst unsigned int histogramSize =\n> -\t\tSwIspStats::kYHistogramSize - blackLevelHistIdx;\n> -\tconst unsigned int yHistValsPerBin = histogramSize / kExposureBinsCount;\n> -\tconst unsigned int yHistValsPerBinMod =\n> -\t\thistogramSize / (histogramSize % kExposureBinsCount + 1);\n> -\tint exposureBins[kExposureBinsCount] = {};\n> -\tunsigned int denom = 0;\n> -\tunsigned int num = 0;\n> -\n> -\tfor (unsigned int i = 0; i < histogramSize; i++) {\n> -\t\tunsigned int idx = (i - (i / yHistValsPerBinMod)) / yHistValsPerBin;\n> -\t\texposureBins[idx] += stats_->yHistogram[blackLevelHistIdx + i];\n> -\t}\n> -\n> -\tfor (unsigned int i = 0; i < kExposureBinsCount; i++) {\n> -\t\tLOG(IPASoft, Debug) << i << \": \" << exposureBins[i];\n> -\t\tdenom += exposureBins[i];\n> -\t\tnum += exposureBins[i] * (i + 1);\n> -\t}\n> -\n> -\tfloat exposureMSV = static_cast<float>(num) / denom;\n> -\n>  \t/* Sanity check */\n>  \tif (!sensorControls.contains(V4L2_CID_EXPOSURE) ||\n>  \t    !sensorControls.contains(V4L2_CID_ANALOGUE_GAIN)) {\n> @@ -332,70 +291,14 @@ void IPASoftSimple::processStats(const uint32_t frame,\n>  \t\treturn;\n>  \t}\n>  \n> -\texposure_ = sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();\n> -\tint32_t again = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();\n> -\tagain_ = camHelper_ ? camHelper_->gain(again) : again;\n> -\n> -\tupdateExposure(exposureMSV);\n> -\n>  \tControlList ctrls(sensorInfoMap_);\n>  \n> -\tctrls.set(V4L2_CID_EXPOSURE, exposure_);\n> +\tauto &againNew = context_.activeState.agc.again;\n> +\tctrls.set(V4L2_CID_EXPOSURE, context_.activeState.agc.exposure);\n>  \tctrls.set(V4L2_CID_ANALOGUE_GAIN,\n> -\t\t  static_cast<int32_t>(camHelper_ ? camHelper_->gainCode(again_) : again_));\n> +\t\t  static_cast<int32_t>(camHelper_ ? camHelper_->gainCode(againNew) : againNew));\n>  \n>  \tsetSensorControls.emit(ctrls);\n> -\n> -\tLOG(IPASoft, Debug) << \"exposureMSV \" << exposureMSV\n> -\t\t\t    << \" exp \" << exposure_ << \" again \" << again_\n> -\t\t\t    << \" black level \" << static_cast<unsigned int>(blackLevel);\n> -}\n> -\n> -void IPASoftSimple::updateExposure(double exposureMSV)\n> -{\n> -\t/*\n> -\t * kExpDenominator of 10 gives ~10% increment/decrement;\n> -\t * kExpDenominator of 5 - about ~20%\n> -\t */\n> -\tstatic constexpr uint8_t kExpDenominator = 10;\n> -\tstatic constexpr uint8_t kExpNumeratorUp = kExpDenominator + 1;\n> -\tstatic constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1;\n> -\n> -\tdouble next;\n> -\n> -\tif (exposureMSV < kExposureOptimal - kExposureSatisfactory) {\n> -\t\tnext = exposure_ * kExpNumeratorUp / kExpDenominator;\n> -\t\tif (next - exposure_ < 1)\n> -\t\t\texposure_ += 1;\n> -\t\telse\n> -\t\t\texposure_ = next;\n> -\t\tif (exposure_ >= exposureMax_) {\n> -\t\t\tnext = again_ * kExpNumeratorUp / kExpDenominator;\n> -\t\t\tif (next - again_ < againMinStep_)\n> -\t\t\t\tagain_ += againMinStep_;\n> -\t\t\telse\n> -\t\t\t\tagain_ = next;\n> -\t\t}\n> -\t}\n> -\n> -\tif (exposureMSV > kExposureOptimal + kExposureSatisfactory) {\n> -\t\tif (exposure_ == exposureMax_ && again_ > againMin_) {\n> -\t\t\tnext = again_ * kExpNumeratorDown / kExpDenominator;\n> -\t\t\tif (again_ - next < againMinStep_)\n> -\t\t\t\tagain_ -= againMinStep_;\n> -\t\t\telse\n> -\t\t\t\tagain_ = next;\n> -\t\t} else {\n> -\t\t\tnext = exposure_ * kExpNumeratorDown / kExpDenominator;\n> -\t\t\tif (exposure_ - next < 1)\n> -\t\t\t\texposure_ -= 1;\n> -\t\t\telse\n> -\t\t\t\texposure_ = next;\n> -\t\t}\n> -\t}\n> -\n> -\texposure_ = std::clamp(exposure_, exposureMin_, exposureMax_);\n> -\tagain_ = std::clamp(again_, againMin_, againMax_);\n>  }\n>  \n>  std::string IPASoftSimple::logPrefix() const\n> diff --git a/src/libcamera/software_isp/TODO b/src/libcamera/software_isp/TODO\n> index 8eeede46..a50db668 100644\n> --- a/src/libcamera/software_isp/TODO\n> +++ b/src/libcamera/software_isp/TODO\n> @@ -199,16 +199,6 @@ Yes, because, well... all the other IPAs were doing that...\n>  \n>  ---\n>  \n> -10. Switch to libipa/algorithm.h API in processStats\n> -\n> ->> void IPASoftSimple::processStats(const ControlList &sensorControls)\n> ->>\n> -> Do you envision switching to the libipa/algorithm.h API at some point ?\n> -\n> -At some point, yes.\n> -\n> ----\n> -\n>  13. Improve black level and colour gains application\n>  \n>  I think the black level should eventually be moved before debayering, and","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 CD682C324C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 20 Sep 2024 19:46:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A56E5634FC;\n\tFri, 20 Sep 2024 21:46:03 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0614E618E2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 20 Sep 2024 21:46:01 +0200 (CEST)","from mail-wm1-f69.google.com (mail-wm1-f69.google.com\n\t[209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-264-O_V4vV5eOjOKqIV0yIhH4Q-1; Fri, 20 Sep 2024 15:45:59 -0400","by mail-wm1-f69.google.com with SMTP id\n\t5b1f17b1804b1-42cb080ab53so14837645e9.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 20 Sep 2024 12:45:59 -0700 (PDT)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-42e75450114sm57541255e9.23.2024.09.20.12.45.54\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 20 Sep 2024 12:45:55 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"S2/d2ZbH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1726861560;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=Tnpw1WPk1ncoEiQr19FNWBDvxprNo4W0mGKLRk1fSRU=;\n\tb=S2/d2ZbH/ii28bcogU6UJuaz8n4/4snhAeyPfSgi64ExjmLSHRoyu4sGnR9bJhGwjF2Dgj\n\tmSfvJ8de4B4JLzzAQpAnGszl4g2Q6G0xqrOa2F89hRrIXvkbv63MWUefTSjM8s2D/Ekor/\n\tHleuUz1JkR5PtJX+OZPZbHNBNHTEDhs=","X-MC-Unique":"O_V4vV5eOjOKqIV0yIhH4Q-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1726861558; x=1727466358;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=Tnpw1WPk1ncoEiQr19FNWBDvxprNo4W0mGKLRk1fSRU=;\n\tb=Oumn5dHMDk1p/0Nk4rKsCkVq1uOKC7mm4scTqcAG9Bk6R7/iJLRZYJ7+UNpwGhXkUQ\n\tB/WaIKUPBhXulVzZ1pBpIE8tjjQpSBmqKbzW6qh+KtRDkRw0jgOAKuhItk3LvW8v4Hvm\n\tJJeuylTdRfOV/uqDS1OqbExg84IVzmNB+TXBGKIcDq6UlUxNovi1h5u29uJVO8KoYo1I\n\tXV2yWGPDOc/iJ4EhIq8t0+DUGViFEq8oANP14MXtJ0KM0dY7LmUdMUE6DR+5eYo/e9Ae\n\tJzCYXDMxbKSFbnG9QjxPwPGpg4CU9ATCaj+eTVdzrknx21DmbTFM5hEnk6i9nDWRO7V6\n\tC/yg==","X-Gm-Message-State":"AOJu0YyUGsLWvH2s/+bzrNYFEv1WcCLRrcxz+riuD2aFgbbv4BFf5cBf\n\tTWQ/9+PkJccx5MnKBtmk2KbG3muea8FUcooTSzSmj9k2Id/BzgDzeBSbfE0YnGfjDaBdigik17F\n\tIL1d2rNo4E6WRIaAXGE0j+tcVPitk6ONu5BhbsHfX4++ZMBai2nvMtB6sRH2RITchqWxw0pU3Xm\n\tnaC0E=","X-Received":["by 2002:a05:600c:1c84:b0:428:e866:3933 with SMTP id\n\t5b1f17b1804b1-42e7ad880ddmr33680075e9.22.1726861557744; \n\tFri, 20 Sep 2024 12:45:57 -0700 (PDT)","by 2002:a05:600c:1c84:b0:428:e866:3933 with SMTP id\n\t5b1f17b1804b1-42e7ad880ddmr33679785e9.22.1726861557194; \n\tFri, 20 Sep 2024 12:45:57 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IH/djWVV76Rzpra1SMwpGcpANlthpRoCBGiffkbu46Pd35fejSjEJbxwq5lhOToGIn0baYgCg==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"libcamera-devel@lists.libcamera.org","Cc":"Umang Jain <umang.jain@ideasonboard.com>,  Laurent Pinchart\n\t<laurent.pinchart@ideasonboard.com>,  Daniel Scally\n\t<dan.scally@ideasonboard.com>,  Kieran Bingham\n\t<kieran.bingham@ideasonboard.com>","Subject":"Re: [PATCH v7 17/18] libcamera: software_isp: Move exposure+gain to\n\tan algorithm module","In-Reply-To":"<20240920183143.1674117-18-mzamazal@redhat.com> (Milan Zamazal's\n\tmessage of \"Fri, 20 Sep 2024 20:31:42 +0200\")","References":"<20240920183143.1674117-1-mzamazal@redhat.com>\n\t<20240920183143.1674117-18-mzamazal@redhat.com>","Date":"Fri, 20 Sep 2024 21:45:54 +0200","Message-ID":"<87r09e5cst.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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":31429,"web_url":"https://patchwork.libcamera.org/comment/31429/","msgid":"<41c9f1ca-2b7d-4cd1-b1db-0760f9f77d41@ideasonboard.com>","date":"2024-09-27T07:07:55","subject":"Re: [PATCH v7 17/18] libcamera: software_isp: Move exposure+gain to\n\tan algorithm module","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Milan,\n\nThank you for the patch.\n\nOn 21/09/24 12:01 am, Milan Zamazal wrote:\n> This is the last step to fully convert software ISP to Algorithm-based\n> processing.\n>\n> The newly introduced frameContext.sensor properties are set, and the\n\ns/sensor properties/sensor parameters\n\n\n> updated code moved, before calling Algorithm::process() to have the\n> values up-to-date in stats processing.\n>\n> Resolves software ISP TODO #10.\n>\n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>   src/ipa/simple/algorithms/agc.cpp     | 139 +++++++++++++++++++++++\n>   src/ipa/simple/algorithms/agc.h       |  33 ++++++\n>   src/ipa/simple/algorithms/meson.build |   1 +\n>   src/ipa/simple/data/uncalibrated.yaml |   1 +\n>   src/ipa/simple/ipa_context.cpp        |  11 ++\n>   src/ipa/simple/ipa_context.h          |  18 +++\n>   src/ipa/simple/soft_simple.cpp        | 157 +++++---------------------\n>   src/libcamera/software_isp/TODO       |  10 --\n>   8 files changed, 233 insertions(+), 137 deletions(-)\n>   create mode 100644 src/ipa/simple/algorithms/agc.cpp\n>   create mode 100644 src/ipa/simple/algorithms/agc.h\n>\n> diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp\n> new file mode 100644\n> index 00000000..783dfb8b\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/agc.cpp\n> @@ -0,0 +1,139 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Red Hat Inc.\n> + *\n> + * Exposure and gain\n> + */\n> +\n> +#include \"agc.h\"\n> +\n> +#include <stdint.h>\n> +\n> +#include <libcamera/base/log.h>\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(IPASoftExposure)\n> +\n> +namespace ipa::soft::algorithms {\n> +\n> +/*\n> + * The number of bins to use for the optimal exposure calculations.\n> + */\n> +static constexpr unsigned int kExposureBinsCount = 5;\n> +\n> +/*\n> + * The exposure is optimal when the mean sample value of the histogram is\n> + * in the middle of the range.\n> + */\n> +static constexpr float kExposureOptimal = kExposureBinsCount / 2.0;\n> +\n> +/*\n> + * The below value implements the hysteresis for the exposure adjustment.\n\ns/The below value/This/\n> + * It is small enough to have the exposure close to the optimal, and is big\n> + * enough to prevent the exposure from wobbling around the optimal value.\n> + */\n> +static constexpr float kExposureSatisfactory = 0.2;\n> +\n> +Agc::Agc()\n> +{\n> +}\n> +\n> +void Agc::updateExposure(IPAContext &context, double exposureMSV)\n> +{\n> +\t/*\n> +\t * kExpDenominator of 10 gives ~10% increment/decrement;\n> +\t * kExpDenominator of 5 - about ~20%\n> +\t */\n> +\tstatic constexpr uint8_t kExpDenominator = 10;\n> +\tstatic constexpr uint8_t kExpNumeratorUp = kExpDenominator + 1;\n> +\tstatic constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1;\n> +\n> +\tdouble next;\n> +\tint32_t &exposure = context.activeState.agc.exposure;\n> +\tdouble &again = context.activeState.agc.again;\n> +\n> +\tif (exposureMSV < kExposureOptimal - kExposureSatisfactory) {\n> +\t\tnext = exposure * kExpNumeratorUp / kExpDenominator;\n> +\t\tif (next - exposure < 1)\n> +\t\t\texposure += 1;\n> +\t\telse\n> +\t\t\texposure = next;\n> +\t\tif (exposure >= context.configuration.agc.exposureMax) {\n> +\t\t\tnext = again * kExpNumeratorUp / kExpDenominator;\n> +\t\t\tif (next - again < context.configuration.agc.againMinStep)\n> +\t\t\t\tagain += context.configuration.agc.againMinStep;\n> +\t\t\telse\n> +\t\t\t\tagain = next;\n> +\t\t}\n> +\t}\n> +\n> +\tif (exposureMSV > kExposureOptimal + kExposureSatisfactory) {\n> +\t\tif (exposure == context.configuration.agc.exposureMax &&\n> +\t\t    again > context.configuration.agc.againMin) {\n> +\t\t\tnext = again * kExpNumeratorDown / kExpDenominator;\n> +\t\t\tif (again - next < context.configuration.agc.againMinStep)\n> +\t\t\t\tagain -= context.configuration.agc.againMinStep;\n> +\t\t\telse\n> +\t\t\t\tagain = next;\n> +\t\t} else {\n> +\t\t\tnext = exposure * kExpNumeratorDown / kExpDenominator;\n> +\t\t\tif (exposure - next < 1)\n> +\t\t\t\texposure -= 1;\n> +\t\t\telse\n> +\t\t\t\texposure = next;\n> +\t\t}\n> +\t}\n> +\n> +\texposure = std::clamp(exposure, context.configuration.agc.exposureMin,\n> +\t\t\t      context.configuration.agc.exposureMax);\n> +\tagain = std::clamp(again, context.configuration.agc.againMin,\n> +\t\t\t   context.configuration.agc.againMax);\n> +\n> +\tLOG(IPASoftExposure, Debug)\n> +\t\t<< \"exposureMSV \" << exposureMSV\n> +\t\t<< \" exp \" << exposure << \" again \" << again;\n> +}\n> +\n> +void Agc::process(IPAContext &context,\n> +\t\t  [[maybe_unused]] const uint32_t frame,\n> +\t\t  [[maybe_unused]] IPAFrameContext &frameContext,\n> +\t\t  const SwIspStats *stats,\n> +\t\t  [[maybe_unused]] ControlList &metadata)\n> +{\n> +\t/*\n> +\t * Calculate Mean Sample Value (MSV) according to formula from:\n> +\t * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf\n> +\t */\n> +\tconst auto &histogram = stats->yHistogram;\n> +\tconst unsigned int blackLevelHistIdx =\n> +\t\tcontext.activeState.blc.level / (256 / SwIspStats::kYHistogramSize);\n> +\tconst unsigned int histogramSize =\n> +\t\tSwIspStats::kYHistogramSize - blackLevelHistIdx;\n> +\tconst unsigned int yHistValsPerBin = histogramSize / kExposureBinsCount;\n> +\tconst unsigned int yHistValsPerBinMod =\n> +\t\thistogramSize / (histogramSize % kExposureBinsCount + 1);\n> +\tint exposureBins[kExposureBinsCount] = {};\n> +\tunsigned int denom = 0;\n> +\tunsigned int num = 0;\n> +\n> +\tfor (unsigned int i = 0; i < histogramSize; i++) {\n> +\t\tunsigned int idx = (i - (i / yHistValsPerBinMod)) / yHistValsPerBin;\n> +\t\texposureBins[idx] += histogram[blackLevelHistIdx + i];\n> +\t}\n> +\n> +\tfor (unsigned int i = 0; i < kExposureBinsCount; i++) {\n> +\t\tLOG(IPASoftExposure, Debug) << i << \": \" << exposureBins[i];\n> +\t\tdenom += exposureBins[i];\n> +\t\tnum += exposureBins[i] * (i + 1);\n> +\t}\n> +\n> +\tfloat exposureMSV = (denom == 0 ? 0 : static_cast<float>(num) / denom);\n> +\tupdateExposure(context, exposureMSV);\n> +}\n> +\n> +REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n> +\n> +} /* namespace ipa::soft::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/simple/algorithms/agc.h b/src/ipa/simple/algorithms/agc.h\n> new file mode 100644\n> index 00000000..ad5fca9f\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/agc.h\n> @@ -0,0 +1,33 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Red Hat Inc.\n> + *\n> + * Exposure and gain\n> + */\n> +\n> +#pragma once\n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::soft::algorithms {\n> +\n> +class Agc : public Algorithm\n> +{\n> +public:\n> +\tAgc();\n> +\t~Agc() = default;\n> +\n> +\tvoid process(IPAContext &context, const uint32_t frame,\n> +\t\t     IPAFrameContext &frameContext,\n> +\t\t     const SwIspStats *stats,\n> +\t\t     ControlList &metadata) override;\n> +\n> +private:\n> +\tvoid updateExposure(IPAContext &context, double exposureMSV);\n> +};\n> +\n> +} /* namespace ipa::soft::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build\n> index f575611e..37a2eb53 100644\n> --- a/src/ipa/simple/algorithms/meson.build\n> +++ b/src/ipa/simple/algorithms/meson.build\n> @@ -2,6 +2,7 @@\n>   \n>   soft_simple_ipa_algorithms = files([\n>       'awb.cpp',\n> +    'agc.cpp',\n>       'blc.cpp',\n>       'lut.cpp',\n>   ])\n> diff --git a/src/ipa/simple/data/uncalibrated.yaml b/src/ipa/simple/data/uncalibrated.yaml\n> index 893a0b92..3f147112 100644\n> --- a/src/ipa/simple/data/uncalibrated.yaml\n> +++ b/src/ipa/simple/data/uncalibrated.yaml\n> @@ -6,4 +6,5 @@ algorithms:\n>     - BlackLevel:\n>     - Awb:\n>     - Lut:\n> +  - Agc:\n>   ...\n> diff --git a/src/ipa/simple/ipa_context.cpp b/src/ipa/simple/ipa_context.cpp\n> index 5fa492cb..3f94bbeb 100644\n> --- a/src/ipa/simple/ipa_context.cpp\n> +++ b/src/ipa/simple/ipa_context.cpp\n> @@ -77,6 +77,17 @@ namespace libcamera::ipa::soft {\n>    * \\brief Gain of blue color\n>    */\n>   \n> +/**\n> + * \\var IPAActiveState::agc\n> + * \\brief Context for the AGC algorithm\n> + *\n> + * \\var IPAActiveState::agc.exposure\n> + * \\brief Current exposure value\n> + *\n> + * \\var IPAActiveState::agc.again\n> + * \\brief Current analog gain value\n> + */\n> +\n>   /**\n>    * \\var IPAActiveState::gamma\n>    * \\brief Context for gamma in the Colors algorithm\n> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> index cf1ef3fd..eb095c85 100644\n> --- a/src/ipa/simple/ipa_context.h\n> +++ b/src/ipa/simple/ipa_context.h\n> @@ -10,6 +10,8 @@\n>   #include <array>\n>   #include <stdint.h>\n>   \n> +#include <libcamera/controls.h>\n> +\n\nIs there required here?\n\nI see additions pertaining to ints and doubles .. so not sure if it's \nrequired ? Or does this belong to previous patch(es) ?\n\nReviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n>   #include <libipa/fc_queue.h>\n>   \n>   namespace libcamera {\n> @@ -18,6 +20,13 @@ namespace ipa::soft {\n>   \n>   struct IPASessionConfiguration {\n>   \tfloat gamma;\n> +\tstruct {\n> +\t\tint32_t exposureMin, exposureMax;\n> +\t\tdouble againMin, againMax, againMinStep;\n> +\t} agc;\n> +\tstruct {\n> +\t\tdouble level;\n> +\t} black;\n>   };\n>   \n>   struct IPAActiveState {\n> @@ -31,6 +40,11 @@ struct IPAActiveState {\n>   \t\tdouble blue;\n>   \t} gains;\n>   \n> +\tstruct {\n> +\t\tint32_t exposure;\n> +\t\tdouble again;\n> +\t} agc;\n> +\n>   \tstatic constexpr unsigned int kGammaLookupSize = 1024;\n>   \tstruct {\n>   \t\tstd::array<double, kGammaLookupSize> gammaTable;\n> @@ -39,6 +53,10 @@ struct IPAActiveState {\n>   };\n>   \n>   struct IPAFrameContext : public FrameContext {\n> +\tstruct {\n> +\t\tuint32_t exposure;\n> +\t\tdouble gain;\n> +\t} sensor;\n>   };\n>   \n>   struct IPAContext {\n> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> index 60310a8e..b28c7039 100644\n> --- a/src/ipa/simple/soft_simple.cpp\n> +++ b/src/ipa/simple/soft_simple.cpp\n> @@ -34,23 +34,6 @@ LOG_DEFINE_CATEGORY(IPASoft)\n>   \n>   namespace ipa::soft {\n>   \n> -/*\n> - * The number of bins to use for the optimal exposure calculations.\n> - */\n> -static constexpr unsigned int kExposureBinsCount = 5;\n> -\n> -/*\n> - * The exposure is optimal when the mean sample value of the histogram is\n> - * in the middle of the range.\n> - */\n> -static constexpr float kExposureOptimal = kExposureBinsCount / 2.0;\n> -\n> -/*\n> - * The below value implements the hysteresis for the exposure adjustment.\n> - * It is small enough to have the exposure close to the optimal, and is big\n> - * enough to prevent the exposure from wobbling around the optimal value.\n> - */\n> -static constexpr float kExposureSatisfactory = 0.2;\n>   /* Maximum number of frame contexts to be held */\n>   static constexpr uint32_t kMaxFrameContexts = 16;\n>   \n> @@ -58,8 +41,7 @@ class IPASoftSimple : public ipa::soft::IPASoftInterface, public Module\n>   {\n>   public:\n>   \tIPASoftSimple()\n> -\t\t: params_(nullptr), stats_(nullptr),\n> -\t\t  context_({ {}, {}, { kMaxFrameContexts } })\n> +\t\t: context_({ {}, {}, { kMaxFrameContexts } })\n>   \t{\n>   \t}\n>   \n> @@ -92,11 +74,6 @@ private:\n>   \n>   \t/* Local parameter storage */\n>   \tstruct IPAContext context_;\n> -\n> -\tint32_t exposureMin_, exposureMax_;\n> -\tint32_t exposure_;\n> -\tdouble againMin_, againMax_, againMinStep_;\n> -\tdouble again_;\n>   };\n>   \n>   IPASoftSimple::~IPASoftSimple()\n> @@ -207,20 +184,23 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n>   \tconst ControlInfo &exposureInfo = sensorInfoMap_.find(V4L2_CID_EXPOSURE)->second;\n>   \tconst ControlInfo &gainInfo = sensorInfoMap_.find(V4L2_CID_ANALOGUE_GAIN)->second;\n>   \n> -\texposureMin_ = exposureInfo.min().get<int32_t>();\n> -\texposureMax_ = exposureInfo.max().get<int32_t>();\n> -\tif (!exposureMin_) {\n> +\tcontext_.configuration.agc.exposureMin = exposureInfo.min().get<int32_t>();\n> +\tcontext_.configuration.agc.exposureMax = exposureInfo.max().get<int32_t>();\n> +\tif (!context_.configuration.agc.exposureMin) {\n>   \t\tLOG(IPASoft, Warning) << \"Minimum exposure is zero, that can't be linear\";\n> -\t\texposureMin_ = 1;\n> +\t\tcontext_.configuration.agc.exposureMin = 1;\n>   \t}\n>   \n>   \tint32_t againMin = gainInfo.min().get<int32_t>();\n>   \tint32_t againMax = gainInfo.max().get<int32_t>();\n>   \n>   \tif (camHelper_) {\n> -\t\tagainMin_ = camHelper_->gain(againMin);\n> -\t\tagainMax_ = camHelper_->gain(againMax);\n> -\t\tagainMinStep_ = (againMax_ - againMin_) / 100.0;\n> +\t\tcontext_.configuration.agc.againMin = camHelper_->gain(againMin);\n> +\t\tcontext_.configuration.agc.againMax = camHelper_->gain(againMax);\n> +\t\tcontext_.configuration.agc.againMinStep =\n> +\t\t\t(context_.configuration.agc.againMax -\n> +\t\t\t context_.configuration.agc.againMin) /\n> +\t\t\t100.0;\n>   \t} else {\n>   \t\t/*\n>   \t\t * The camera sensor gain (g) is usually not equal to the value written\n> @@ -232,13 +212,14 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n>   \t\t * the AGC algorithm (abrupt near one edge, and very small near the\n>   \t\t * other) we limit the range of the gain values used.\n>   \t\t */\n> -\t\tagainMax_ = againMax;\n> +\t\tcontext_.configuration.agc.againMax = againMax;\n>   \t\tif (!againMin) {\n>   \t\t\tLOG(IPASoft, Warning)\n>   \t\t\t\t<< \"Minimum gain is zero, that can't be linear\";\n> -\t\t\tagainMin_ = std::min(100, againMin / 2 + againMax / 2);\n> +\t\t\tcontext_.configuration.agc.againMin =\n> +\t\t\t\tstd::min(100, againMin / 2 + againMax / 2);\n>   \t\t}\n> -\t\tagainMinStep_ = 1.0;\n> +\t\tcontext_.configuration.agc.againMinStep = 1.0;\n>   \t}\n>   \n>   \tfor (auto const &algo : algorithms()) {\n> @@ -247,9 +228,12 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n>   \t\t\treturn ret;\n>   \t}\n>   \n> -\tLOG(IPASoft, Info) << \"Exposure \" << exposureMin_ << \"-\" << exposureMax_\n> -\t\t\t   << \", gain \" << againMin_ << \"-\" << againMax_\n> -\t\t\t   << \" (\" << againMinStep_ << \")\";\n> +\tLOG(IPASoft, Info)\n> +\t\t<< \"Exposure \" << context_.configuration.agc.exposureMin << \"-\"\n> +\t\t<< context_.configuration.agc.exposureMax\n> +\t\t<< \", gain \" << context_.configuration.agc.againMin << \"-\"\n> +\t\t<< context_.configuration.agc.againMax\n> +\t\t<< \" (\" << context_.configuration.agc.againMinStep << \")\";\n>   \n>   \treturn 0;\n>   }\n> @@ -284,6 +268,12 @@ void IPASoftSimple::processStats(const uint32_t frame,\n>   \t\t\t\t const ControlList &sensorControls)\n>   {\n>   \tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> +\n> +\tframeContext.sensor.exposure =\n> +\t\tsensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();\n> +\tint32_t again = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();\n> +\tframeContext.sensor.gain = camHelper_ ? camHelper_->gain(again) : again;\n> +\n>   \t/*\n>   \t * Software ISP currently does not produce any metadata. Use an empty\n>   \t * ControlList for now.\n> @@ -294,37 +284,6 @@ void IPASoftSimple::processStats(const uint32_t frame,\n>   \tfor (auto const &algo : algorithms())\n>   \t\talgo->process(context_, frame, frameContext, stats_, metadata);\n>   \n> -\t/* \\todo Switch to the libipa/algorithm.h API someday. */\n> -\n> -\t/*\n> -\t * Calculate Mean Sample Value (MSV) according to formula from:\n> -\t * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf\n> -\t */\n> -\tconst uint8_t blackLevel = context_.activeState.blc.level;\n> -\tconst unsigned int blackLevelHistIdx =\n> -\t\tblackLevel / (256 / SwIspStats::kYHistogramSize);\n> -\tconst unsigned int histogramSize =\n> -\t\tSwIspStats::kYHistogramSize - blackLevelHistIdx;\n> -\tconst unsigned int yHistValsPerBin = histogramSize / kExposureBinsCount;\n> -\tconst unsigned int yHistValsPerBinMod =\n> -\t\thistogramSize / (histogramSize % kExposureBinsCount + 1);\n> -\tint exposureBins[kExposureBinsCount] = {};\n> -\tunsigned int denom = 0;\n> -\tunsigned int num = 0;\n> -\n> -\tfor (unsigned int i = 0; i < histogramSize; i++) {\n> -\t\tunsigned int idx = (i - (i / yHistValsPerBinMod)) / yHistValsPerBin;\n> -\t\texposureBins[idx] += stats_->yHistogram[blackLevelHistIdx + i];\n> -\t}\n> -\n> -\tfor (unsigned int i = 0; i < kExposureBinsCount; i++) {\n> -\t\tLOG(IPASoft, Debug) << i << \": \" << exposureBins[i];\n> -\t\tdenom += exposureBins[i];\n> -\t\tnum += exposureBins[i] * (i + 1);\n> -\t}\n> -\n> -\tfloat exposureMSV = static_cast<float>(num) / denom;\n> -\n>   \t/* Sanity check */\n>   \tif (!sensorControls.contains(V4L2_CID_EXPOSURE) ||\n>   \t    !sensorControls.contains(V4L2_CID_ANALOGUE_GAIN)) {\n> @@ -332,70 +291,14 @@ void IPASoftSimple::processStats(const uint32_t frame,\n>   \t\treturn;\n>   \t}\n>   \n> -\texposure_ = sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();\n> -\tint32_t again = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();\n> -\tagain_ = camHelper_ ? camHelper_->gain(again) : again;\n> -\n> -\tupdateExposure(exposureMSV);\n> -\n>   \tControlList ctrls(sensorInfoMap_);\n>   \n> -\tctrls.set(V4L2_CID_EXPOSURE, exposure_);\n> +\tauto &againNew = context_.activeState.agc.again;\n> +\tctrls.set(V4L2_CID_EXPOSURE, context_.activeState.agc.exposure);\n>   \tctrls.set(V4L2_CID_ANALOGUE_GAIN,\n> -\t\t  static_cast<int32_t>(camHelper_ ? camHelper_->gainCode(again_) : again_));\n> +\t\t  static_cast<int32_t>(camHelper_ ? camHelper_->gainCode(againNew) : againNew));\n>   \n>   \tsetSensorControls.emit(ctrls);\n> -\n> -\tLOG(IPASoft, Debug) << \"exposureMSV \" << exposureMSV\n> -\t\t\t    << \" exp \" << exposure_ << \" again \" << again_\n> -\t\t\t    << \" black level \" << static_cast<unsigned int>(blackLevel);\n> -}\n> -\n> -void IPASoftSimple::updateExposure(double exposureMSV)\n> -{\n> -\t/*\n> -\t * kExpDenominator of 10 gives ~10% increment/decrement;\n> -\t * kExpDenominator of 5 - about ~20%\n> -\t */\n> -\tstatic constexpr uint8_t kExpDenominator = 10;\n> -\tstatic constexpr uint8_t kExpNumeratorUp = kExpDenominator + 1;\n> -\tstatic constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1;\n> -\n> -\tdouble next;\n> -\n> -\tif (exposureMSV < kExposureOptimal - kExposureSatisfactory) {\n> -\t\tnext = exposure_ * kExpNumeratorUp / kExpDenominator;\n> -\t\tif (next - exposure_ < 1)\n> -\t\t\texposure_ += 1;\n> -\t\telse\n> -\t\t\texposure_ = next;\n> -\t\tif (exposure_ >= exposureMax_) {\n> -\t\t\tnext = again_ * kExpNumeratorUp / kExpDenominator;\n> -\t\t\tif (next - again_ < againMinStep_)\n> -\t\t\t\tagain_ += againMinStep_;\n> -\t\t\telse\n> -\t\t\t\tagain_ = next;\n> -\t\t}\n> -\t}\n> -\n> -\tif (exposureMSV > kExposureOptimal + kExposureSatisfactory) {\n> -\t\tif (exposure_ == exposureMax_ && again_ > againMin_) {\n> -\t\t\tnext = again_ * kExpNumeratorDown / kExpDenominator;\n> -\t\t\tif (again_ - next < againMinStep_)\n> -\t\t\t\tagain_ -= againMinStep_;\n> -\t\t\telse\n> -\t\t\t\tagain_ = next;\n> -\t\t} else {\n> -\t\t\tnext = exposure_ * kExpNumeratorDown / kExpDenominator;\n> -\t\t\tif (exposure_ - next < 1)\n> -\t\t\t\texposure_ -= 1;\n> -\t\t\telse\n> -\t\t\t\texposure_ = next;\n> -\t\t}\n> -\t}\n> -\n> -\texposure_ = std::clamp(exposure_, exposureMin_, exposureMax_);\n> -\tagain_ = std::clamp(again_, againMin_, againMax_);\n>   }\n>   \n>   std::string IPASoftSimple::logPrefix() const\n> diff --git a/src/libcamera/software_isp/TODO b/src/libcamera/software_isp/TODO\n> index 8eeede46..a50db668 100644\n> --- a/src/libcamera/software_isp/TODO\n> +++ b/src/libcamera/software_isp/TODO\n> @@ -199,16 +199,6 @@ Yes, because, well... all the other IPAs were doing that...\n>   \n>   ---\n>   \n> -10. Switch to libipa/algorithm.h API in processStats\n> -\n> ->> void IPASoftSimple::processStats(const ControlList &sensorControls)\n> ->>\n> -> Do you envision switching to the libipa/algorithm.h API at some point ?\n> -\n> -At some point, yes.\n> -\n> ----\n> -\n>   13. Improve black level and colour gains application\n>   \n>   I think the black level should eventually be moved before debayering, and","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 2972EC3257\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 27 Sep 2024 07:08:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 08B066350F;\n\tFri, 27 Sep 2024 09:08:02 +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 5DDF4618DB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 27 Sep 2024 09:08:00 +0200 (CEST)","from [IPV6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f] (unknown\n\t[IPv6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5BBA48BF;\n\tFri, 27 Sep 2024 09:06:30 +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=\"Qo9AoP2c\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727420791;\n\tbh=+ErU6LNObMhKkU84WmUCCcNvZ5xHT6hjNOZDBa6ZAmA=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=Qo9AoP2c43O6HDki9iaE0mtW6iTQfpyQPx6OeDSw0kM5AlxPLhNCcD+1JxIKZTSFW\n\tZJf1Rx/Ssz1Vk9w0iSz2xJtjxXMP/KAGNqpJsORnvJteYl/CVsf3rdzyyG82pwdFOA\n\tOGyuZCKDs1C7h073/Tjib9HsfYh1qWp+GXkkABkE=","Message-ID":"<41c9f1ca-2b7d-4cd1-b1db-0760f9f77d41@ideasonboard.com>","Date":"Fri, 27 Sep 2024 12:37:55 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v7 17/18] libcamera: software_isp: Move exposure+gain to\n\tan algorithm module","To":"Milan Zamazal <mzamazal@redhat.com>, libcamera-devel@lists.libcamera.org","Cc":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tDaniel Scally <dan.scally@ideasonboard.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>","References":"<20240920183143.1674117-1-mzamazal@redhat.com>\n\t<20240920183143.1674117-18-mzamazal@redhat.com>","Content-Language":"en-US","From":"Umang Jain <umang.jain@ideasonboard.com>","In-Reply-To":"<20240920183143.1674117-18-mzamazal@redhat.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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":31441,"web_url":"https://patchwork.libcamera.org/comment/31441/","msgid":"<877caxp6rr.fsf@redhat.com>","date":"2024-09-27T13:26:16","subject":"Re: [PATCH v7 17/18] libcamera: software_isp: Move exposure+gain to\n\tan algorithm module","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Umang,\n\nthank you for review.\n\nUmang Jain <umang.jain@ideasonboard.com> writes:\n\n> Hi Milan,\n>\n> Thank you for the patch.\n>\n> On 21/09/24 12:01 am, Milan Zamazal wrote:\n>> This is the last step to fully convert software ISP to Algorithm-based\n>> processing.\n>>\n>> The newly introduced frameContext.sensor properties are set, and the\n>\n> s/sensor properties/sensor parameters\n>\n>\n>> updated code moved, before calling Algorithm::process() to have the\n>> values up-to-date in stats processing.\n>>\n>> Resolves software ISP TODO #10.\n>>\n>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>> ---\n>>   src/ipa/simple/algorithms/agc.cpp     | 139 +++++++++++++++++++++++\n>>   src/ipa/simple/algorithms/agc.h       |  33 ++++++\n>>   src/ipa/simple/algorithms/meson.build |   1 +\n>>   src/ipa/simple/data/uncalibrated.yaml |   1 +\n>>   src/ipa/simple/ipa_context.cpp        |  11 ++\n>>   src/ipa/simple/ipa_context.h          |  18 +++\n>>   src/ipa/simple/soft_simple.cpp        | 157 +++++---------------------\n>>   src/libcamera/software_isp/TODO       |  10 --\n>>   8 files changed, 233 insertions(+), 137 deletions(-)\n>>   create mode 100644 src/ipa/simple/algorithms/agc.cpp\n>>   create mode 100644 src/ipa/simple/algorithms/agc.h\n>>\n>> diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp\n>> new file mode 100644\n>> index 00000000..783dfb8b\n>> --- /dev/null\n>> +++ b/src/ipa/simple/algorithms/agc.cpp\n>> @@ -0,0 +1,139 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2024, Red Hat Inc.\n>> + *\n>> + * Exposure and gain\n>> + */\n>> +\n>> +#include \"agc.h\"\n>> +\n>> +#include <stdint.h>\n>> +\n>> +#include <libcamera/base/log.h>\n>> +\n>> +namespace libcamera {\n>> +\n>> +LOG_DEFINE_CATEGORY(IPASoftExposure)\n>> +\n>> +namespace ipa::soft::algorithms {\n>> +\n>> +/*\n>> + * The number of bins to use for the optimal exposure calculations.\n>> + */\n>> +static constexpr unsigned int kExposureBinsCount = 5;\n>> +\n>> +/*\n>> + * The exposure is optimal when the mean sample value of the histogram is\n>> + * in the middle of the range.\n>> + */\n>> +static constexpr float kExposureOptimal = kExposureBinsCount / 2.0;\n>> +\n>> +/*\n>> + * The below value implements the hysteresis for the exposure adjustment.\n>\n> s/The below value/This/\n\nThis one is present in the original code and just moved.  I don't like\nmaking unnecessary changes in moved code since reviewing code movements\nis already difficult enough.  So let's fix this in a separate patch\nlater.\n\n>> + * It is small enough to have the exposure close to the optimal, and is big\n>> + * enough to prevent the exposure from wobbling around the optimal value.\n>> + */\n>> +static constexpr float kExposureSatisfactory = 0.2;\n>> +\n>> +Agc::Agc()\n>> +{\n>> +}\n>> +\n>> +void Agc::updateExposure(IPAContext &context, double exposureMSV)\n>> +{\n>> +\t/*\n>> +\t * kExpDenominator of 10 gives ~10% increment/decrement;\n>> +\t * kExpDenominator of 5 - about ~20%\n>> +\t */\n>> +\tstatic constexpr uint8_t kExpDenominator = 10;\n>> +\tstatic constexpr uint8_t kExpNumeratorUp = kExpDenominator + 1;\n>> +\tstatic constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1;\n>> +\n>> +\tdouble next;\n>> +\tint32_t &exposure = context.activeState.agc.exposure;\n>> +\tdouble &again = context.activeState.agc.again;\n>> +\n>> +\tif (exposureMSV < kExposureOptimal - kExposureSatisfactory) {\n>> +\t\tnext = exposure * kExpNumeratorUp / kExpDenominator;\n>> +\t\tif (next - exposure < 1)\n>> +\t\t\texposure += 1;\n>> +\t\telse\n>> +\t\t\texposure = next;\n>> +\t\tif (exposure >= context.configuration.agc.exposureMax) {\n>> +\t\t\tnext = again * kExpNumeratorUp / kExpDenominator;\n>> +\t\t\tif (next - again < context.configuration.agc.againMinStep)\n>> +\t\t\t\tagain += context.configuration.agc.againMinStep;\n>> +\t\t\telse\n>> +\t\t\t\tagain = next;\n>> +\t\t}\n>> +\t}\n>> +\n>> +\tif (exposureMSV > kExposureOptimal + kExposureSatisfactory) {\n>> +\t\tif (exposure == context.configuration.agc.exposureMax &&\n>> +\t\t    again > context.configuration.agc.againMin) {\n>> +\t\t\tnext = again * kExpNumeratorDown / kExpDenominator;\n>> +\t\t\tif (again - next < context.configuration.agc.againMinStep)\n>> +\t\t\t\tagain -= context.configuration.agc.againMinStep;\n>> +\t\t\telse\n>> +\t\t\t\tagain = next;\n>> +\t\t} else {\n>> +\t\t\tnext = exposure * kExpNumeratorDown / kExpDenominator;\n>> +\t\t\tif (exposure - next < 1)\n>> +\t\t\t\texposure -= 1;\n>> +\t\t\telse\n>> +\t\t\t\texposure = next;\n>> +\t\t}\n>> +\t}\n>> +\n>> +\texposure = std::clamp(exposure, context.configuration.agc.exposureMin,\n>> +\t\t\t      context.configuration.agc.exposureMax);\n>> +\tagain = std::clamp(again, context.configuration.agc.againMin,\n>> +\t\t\t   context.configuration.agc.againMax);\n>> +\n>> +\tLOG(IPASoftExposure, Debug)\n>> +\t\t<< \"exposureMSV \" << exposureMSV\n>> +\t\t<< \" exp \" << exposure << \" again \" << again;\n>> +}\n>> +\n>> +void Agc::process(IPAContext &context,\n>> +\t\t  [[maybe_unused]] const uint32_t frame,\n>> +\t\t  [[maybe_unused]] IPAFrameContext &frameContext,\n>> +\t\t  const SwIspStats *stats,\n>> +\t\t  [[maybe_unused]] ControlList &metadata)\n>> +{\n>> +\t/*\n>> +\t * Calculate Mean Sample Value (MSV) according to formula from:\n>> +\t * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf\n>> +\t */\n>> +\tconst auto &histogram = stats->yHistogram;\n>> +\tconst unsigned int blackLevelHistIdx =\n>> +\t\tcontext.activeState.blc.level / (256 / SwIspStats::kYHistogramSize);\n>> +\tconst unsigned int histogramSize =\n>> +\t\tSwIspStats::kYHistogramSize - blackLevelHistIdx;\n>> +\tconst unsigned int yHistValsPerBin = histogramSize / kExposureBinsCount;\n>> +\tconst unsigned int yHistValsPerBinMod =\n>> +\t\thistogramSize / (histogramSize % kExposureBinsCount + 1);\n>> +\tint exposureBins[kExposureBinsCount] = {};\n>> +\tunsigned int denom = 0;\n>> +\tunsigned int num = 0;\n>> +\n>> +\tfor (unsigned int i = 0; i < histogramSize; i++) {\n>> +\t\tunsigned int idx = (i - (i / yHistValsPerBinMod)) / yHistValsPerBin;\n>> +\t\texposureBins[idx] += histogram[blackLevelHistIdx + i];\n>> +\t}\n>> +\n>> +\tfor (unsigned int i = 0; i < kExposureBinsCount; i++) {\n>> +\t\tLOG(IPASoftExposure, Debug) << i << \": \" << exposureBins[i];\n>> +\t\tdenom += exposureBins[i];\n>> +\t\tnum += exposureBins[i] * (i + 1);\n>> +\t}\n>> +\n>> +\tfloat exposureMSV = (denom == 0 ? 0 : static_cast<float>(num) / denom);\n>> +\tupdateExposure(context, exposureMSV);\n>> +}\n>> +\n>> +REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n>> +\n>> +} /* namespace ipa::soft::algorithms */\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/ipa/simple/algorithms/agc.h b/src/ipa/simple/algorithms/agc.h\n>> new file mode 100644\n>> index 00000000..ad5fca9f\n>> --- /dev/null\n>> +++ b/src/ipa/simple/algorithms/agc.h\n>> @@ -0,0 +1,33 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2024, Red Hat Inc.\n>> + *\n>> + * Exposure and gain\n>> + */\n>> +\n>> +#pragma once\n>> +\n>> +#include \"algorithm.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace ipa::soft::algorithms {\n>> +\n>> +class Agc : public Algorithm\n>> +{\n>> +public:\n>> +\tAgc();\n>> +\t~Agc() = default;\n>> +\n>> +\tvoid process(IPAContext &context, const uint32_t frame,\n>> +\t\t     IPAFrameContext &frameContext,\n>> +\t\t     const SwIspStats *stats,\n>> +\t\t     ControlList &metadata) override;\n>> +\n>> +private:\n>> +\tvoid updateExposure(IPAContext &context, double exposureMSV);\n>> +};\n>> +\n>> +} /* namespace ipa::soft::algorithms */\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build\n>> index f575611e..37a2eb53 100644\n>> --- a/src/ipa/simple/algorithms/meson.build\n>> +++ b/src/ipa/simple/algorithms/meson.build\n>> @@ -2,6 +2,7 @@\n>>     soft_simple_ipa_algorithms = files([\n>>       'awb.cpp',\n>> +    'agc.cpp',\n>>       'blc.cpp',\n>>       'lut.cpp',\n>>   ])\n>> diff --git a/src/ipa/simple/data/uncalibrated.yaml b/src/ipa/simple/data/uncalibrated.yaml\n>> index 893a0b92..3f147112 100644\n>> --- a/src/ipa/simple/data/uncalibrated.yaml\n>> +++ b/src/ipa/simple/data/uncalibrated.yaml\n>> @@ -6,4 +6,5 @@ algorithms:\n>>     - BlackLevel:\n>>     - Awb:\n>>     - Lut:\n>> +  - Agc:\n>>   ...\n>> diff --git a/src/ipa/simple/ipa_context.cpp b/src/ipa/simple/ipa_context.cpp\n>> index 5fa492cb..3f94bbeb 100644\n>> --- a/src/ipa/simple/ipa_context.cpp\n>> +++ b/src/ipa/simple/ipa_context.cpp\n>> @@ -77,6 +77,17 @@ namespace libcamera::ipa::soft {\n>>    * \\brief Gain of blue color\n>>    */\n>>   +/**\n>> + * \\var IPAActiveState::agc\n>> + * \\brief Context for the AGC algorithm\n>> + *\n>> + * \\var IPAActiveState::agc.exposure\n>> + * \\brief Current exposure value\n>> + *\n>> + * \\var IPAActiveState::agc.again\n>> + * \\brief Current analog gain value\n>> + */\n>> +\n>>   /**\n>>    * \\var IPAActiveState::gamma\n>>    * \\brief Context for gamma in the Colors algorithm\n>> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n>> index cf1ef3fd..eb095c85 100644\n>> --- a/src/ipa/simple/ipa_context.h\n>> +++ b/src/ipa/simple/ipa_context.h\n>> @@ -10,6 +10,8 @@\n>>   #include <array>\n>>   #include <stdint.h>\n>>   +#include <libcamera/controls.h>\n>> +\n>\n> Is there required here?\n>\n> I see additions pertaining to ints and doubles .. so not sure if it's required ? Or does this belong to\n> previous patch(es) ?\n\nAs far as I can see the include is not needed.  I'll remove it.\n\n> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n>>   #include <libipa/fc_queue.h>\n>>     namespace libcamera {\n>> @@ -18,6 +20,13 @@ namespace ipa::soft {\n>>     struct IPASessionConfiguration {\n>>   \tfloat gamma;\n>> +\tstruct {\n>> +\t\tint32_t exposureMin, exposureMax;\n>> +\t\tdouble againMin, againMax, againMinStep;\n>> +\t} agc;\n>> +\tstruct {\n>> +\t\tdouble level;\n>> +\t} black;\n>>   };\n>>     struct IPAActiveState {\n>> @@ -31,6 +40,11 @@ struct IPAActiveState {\n>>   \t\tdouble blue;\n>>   \t} gains;\n>>   +\tstruct {\n>> +\t\tint32_t exposure;\n>> +\t\tdouble again;\n>> +\t} agc;\n>> +\n>>   \tstatic constexpr unsigned int kGammaLookupSize = 1024;\n>>   \tstruct {\n>>   \t\tstd::array<double, kGammaLookupSize> gammaTable;\n>> @@ -39,6 +53,10 @@ struct IPAActiveState {\n>>   };\n>>     struct IPAFrameContext : public FrameContext {\n>> +\tstruct {\n>> +\t\tuint32_t exposure;\n>> +\t\tdouble gain;\n>> +\t} sensor;\n>>   };\n>>     struct IPAContext {\n>> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n>> index 60310a8e..b28c7039 100644\n>> --- a/src/ipa/simple/soft_simple.cpp\n>> +++ b/src/ipa/simple/soft_simple.cpp\n>> @@ -34,23 +34,6 @@ LOG_DEFINE_CATEGORY(IPASoft)\n>>     namespace ipa::soft {\n>>   -/*\n>> - * The number of bins to use for the optimal exposure calculations.\n>> - */\n>> -static constexpr unsigned int kExposureBinsCount = 5;\n>> -\n>> -/*\n>> - * The exposure is optimal when the mean sample value of the histogram is\n>> - * in the middle of the range.\n>> - */\n>> -static constexpr float kExposureOptimal = kExposureBinsCount / 2.0;\n>> -\n>> -/*\n>> - * The below value implements the hysteresis for the exposure adjustment.\n>> - * It is small enough to have the exposure close to the optimal, and is big\n>> - * enough to prevent the exposure from wobbling around the optimal value.\n>> - */\n>> -static constexpr float kExposureSatisfactory = 0.2;\n>>   /* Maximum number of frame contexts to be held */\n>>   static constexpr uint32_t kMaxFrameContexts = 16;\n>>   @@ -58,8 +41,7 @@ class IPASoftSimple : public ipa::soft::IPASoftInterface, public Module\n>>   {\n>>   public:\n>>   \tIPASoftSimple()\n>> -\t\t: params_(nullptr), stats_(nullptr),\n>> -\t\t  context_({ {}, {}, { kMaxFrameContexts } })\n>> +\t\t: context_({ {}, {}, { kMaxFrameContexts } })\n>>   \t{\n>>   \t}\n>>   @@ -92,11 +74,6 @@ private:\n>>     \t/* Local parameter storage */\n>>   \tstruct IPAContext context_;\n>> -\n>> -\tint32_t exposureMin_, exposureMax_;\n>> -\tint32_t exposure_;\n>> -\tdouble againMin_, againMax_, againMinStep_;\n>> -\tdouble again_;\n>>   };\n>>     IPASoftSimple::~IPASoftSimple()\n>> @@ -207,20 +184,23 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n>>   \tconst ControlInfo &exposureInfo = sensorInfoMap_.find(V4L2_CID_EXPOSURE)->second;\n>>   \tconst ControlInfo &gainInfo = sensorInfoMap_.find(V4L2_CID_ANALOGUE_GAIN)->second;\n>>   -\texposureMin_ = exposureInfo.min().get<int32_t>();\n>> -\texposureMax_ = exposureInfo.max().get<int32_t>();\n>> -\tif (!exposureMin_) {\n>> +\tcontext_.configuration.agc.exposureMin = exposureInfo.min().get<int32_t>();\n>> +\tcontext_.configuration.agc.exposureMax = exposureInfo.max().get<int32_t>();\n>> +\tif (!context_.configuration.agc.exposureMin) {\n>>   \t\tLOG(IPASoft, Warning) << \"Minimum exposure is zero, that can't be linear\";\n>> -\t\texposureMin_ = 1;\n>> +\t\tcontext_.configuration.agc.exposureMin = 1;\n>>   \t}\n>>     \tint32_t againMin = gainInfo.min().get<int32_t>();\n>>   \tint32_t againMax = gainInfo.max().get<int32_t>();\n>>     \tif (camHelper_) {\n>> -\t\tagainMin_ = camHelper_->gain(againMin);\n>> -\t\tagainMax_ = camHelper_->gain(againMax);\n>> -\t\tagainMinStep_ = (againMax_ - againMin_) / 100.0;\n>> +\t\tcontext_.configuration.agc.againMin = camHelper_->gain(againMin);\n>> +\t\tcontext_.configuration.agc.againMax = camHelper_->gain(againMax);\n>> +\t\tcontext_.configuration.agc.againMinStep =\n>> +\t\t\t(context_.configuration.agc.againMax -\n>> +\t\t\t context_.configuration.agc.againMin) /\n>> +\t\t\t100.0;\n>>   \t} else {\n>>   \t\t/*\n>>   \t\t * The camera sensor gain (g) is usually not equal to the value written\n>> @@ -232,13 +212,14 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n>>   \t\t * the AGC algorithm (abrupt near one edge, and very small near the\n>>   \t\t * other) we limit the range of the gain values used.\n>>   \t\t */\n>> -\t\tagainMax_ = againMax;\n>> +\t\tcontext_.configuration.agc.againMax = againMax;\n>>   \t\tif (!againMin) {\n>>   \t\t\tLOG(IPASoft, Warning)\n>>   \t\t\t\t<< \"Minimum gain is zero, that can't be linear\";\n>> -\t\t\tagainMin_ = std::min(100, againMin / 2 + againMax / 2);\n>> +\t\t\tcontext_.configuration.agc.againMin =\n>> +\t\t\t\tstd::min(100, againMin / 2 + againMax / 2);\n>>   \t\t}\n>> -\t\tagainMinStep_ = 1.0;\n>> +\t\tcontext_.configuration.agc.againMinStep = 1.0;\n>>   \t}\n>>     \tfor (auto const &algo : algorithms()) {\n>> @@ -247,9 +228,12 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n>>   \t\t\treturn ret;\n>>   \t}\n>>   -\tLOG(IPASoft, Info) << \"Exposure \" << exposureMin_ << \"-\" << exposureMax_\n>> -\t\t\t   << \", gain \" << againMin_ << \"-\" << againMax_\n>> -\t\t\t   << \" (\" << againMinStep_ << \")\";\n>> +\tLOG(IPASoft, Info)\n>> +\t\t<< \"Exposure \" << context_.configuration.agc.exposureMin << \"-\"\n>> +\t\t<< context_.configuration.agc.exposureMax\n>> +\t\t<< \", gain \" << context_.configuration.agc.againMin << \"-\"\n>> +\t\t<< context_.configuration.agc.againMax\n>> +\t\t<< \" (\" << context_.configuration.agc.againMinStep << \")\";\n>>     \treturn 0;\n>>   }\n>> @@ -284,6 +268,12 @@ void IPASoftSimple::processStats(const uint32_t frame,\n>>   \t\t\t\t const ControlList &sensorControls)\n>>   {\n>>   \tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>> +\n>> +\tframeContext.sensor.exposure =\n>> +\t\tsensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();\n>> +\tint32_t again = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();\n>> +\tframeContext.sensor.gain = camHelper_ ? camHelper_->gain(again) : again;\n>> +\n>>   \t/*\n>>   \t * Software ISP currently does not produce any metadata. Use an empty\n>>   \t * ControlList for now.\n>> @@ -294,37 +284,6 @@ void IPASoftSimple::processStats(const uint32_t frame,\n>>   \tfor (auto const &algo : algorithms())\n>>   \t\talgo->process(context_, frame, frameContext, stats_, metadata);\n>>   -\t/* \\todo Switch to the libipa/algorithm.h API someday. */\n>> -\n>> -\t/*\n>> -\t * Calculate Mean Sample Value (MSV) according to formula from:\n>> -\t * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf\n>> -\t */\n>> -\tconst uint8_t blackLevel = context_.activeState.blc.level;\n>> -\tconst unsigned int blackLevelHistIdx =\n>> -\t\tblackLevel / (256 / SwIspStats::kYHistogramSize);\n>> -\tconst unsigned int histogramSize =\n>> -\t\tSwIspStats::kYHistogramSize - blackLevelHistIdx;\n>> -\tconst unsigned int yHistValsPerBin = histogramSize / kExposureBinsCount;\n>> -\tconst unsigned int yHistValsPerBinMod =\n>> -\t\thistogramSize / (histogramSize % kExposureBinsCount + 1);\n>> -\tint exposureBins[kExposureBinsCount] = {};\n>> -\tunsigned int denom = 0;\n>> -\tunsigned int num = 0;\n>> -\n>> -\tfor (unsigned int i = 0; i < histogramSize; i++) {\n>> -\t\tunsigned int idx = (i - (i / yHistValsPerBinMod)) / yHistValsPerBin;\n>> -\t\texposureBins[idx] += stats_->yHistogram[blackLevelHistIdx + i];\n>> -\t}\n>> -\n>> -\tfor (unsigned int i = 0; i < kExposureBinsCount; i++) {\n>> -\t\tLOG(IPASoft, Debug) << i << \": \" << exposureBins[i];\n>> -\t\tdenom += exposureBins[i];\n>> -\t\tnum += exposureBins[i] * (i + 1);\n>> -\t}\n>> -\n>> -\tfloat exposureMSV = static_cast<float>(num) / denom;\n>> -\n>>   \t/* Sanity check */\n>>   \tif (!sensorControls.contains(V4L2_CID_EXPOSURE) ||\n>>   \t    !sensorControls.contains(V4L2_CID_ANALOGUE_GAIN)) {\n>> @@ -332,70 +291,14 @@ void IPASoftSimple::processStats(const uint32_t frame,\n>>   \t\treturn;\n>>   \t}\n>>   -\texposure_ = sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();\n>> -\tint32_t again = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();\n>> -\tagain_ = camHelper_ ? camHelper_->gain(again) : again;\n>> -\n>> -\tupdateExposure(exposureMSV);\n>> -\n>>   \tControlList ctrls(sensorInfoMap_);\n>>   -\tctrls.set(V4L2_CID_EXPOSURE, exposure_);\n>> +\tauto &againNew = context_.activeState.agc.again;\n>> +\tctrls.set(V4L2_CID_EXPOSURE, context_.activeState.agc.exposure);\n>>   \tctrls.set(V4L2_CID_ANALOGUE_GAIN,\n>> -\t\t  static_cast<int32_t>(camHelper_ ? camHelper_->gainCode(again_) : again_));\n>> +\t\t  static_cast<int32_t>(camHelper_ ? camHelper_->gainCode(againNew) : againNew));\n>>     \tsetSensorControls.emit(ctrls);\n>> -\n>> -\tLOG(IPASoft, Debug) << \"exposureMSV \" << exposureMSV\n>> -\t\t\t    << \" exp \" << exposure_ << \" again \" << again_\n>> -\t\t\t    << \" black level \" << static_cast<unsigned int>(blackLevel);\n>> -}\n>> -\n>> -void IPASoftSimple::updateExposure(double exposureMSV)\n>> -{\n>> -\t/*\n>> -\t * kExpDenominator of 10 gives ~10% increment/decrement;\n>> -\t * kExpDenominator of 5 - about ~20%\n>> -\t */\n>> -\tstatic constexpr uint8_t kExpDenominator = 10;\n>> -\tstatic constexpr uint8_t kExpNumeratorUp = kExpDenominator + 1;\n>> -\tstatic constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1;\n>> -\n>> -\tdouble next;\n>> -\n>> -\tif (exposureMSV < kExposureOptimal - kExposureSatisfactory) {\n>> -\t\tnext = exposure_ * kExpNumeratorUp / kExpDenominator;\n>> -\t\tif (next - exposure_ < 1)\n>> -\t\t\texposure_ += 1;\n>> -\t\telse\n>> -\t\t\texposure_ = next;\n>> -\t\tif (exposure_ >= exposureMax_) {\n>> -\t\t\tnext = again_ * kExpNumeratorUp / kExpDenominator;\n>> -\t\t\tif (next - again_ < againMinStep_)\n>> -\t\t\t\tagain_ += againMinStep_;\n>> -\t\t\telse\n>> -\t\t\t\tagain_ = next;\n>> -\t\t}\n>> -\t}\n>> -\n>> -\tif (exposureMSV > kExposureOptimal + kExposureSatisfactory) {\n>> -\t\tif (exposure_ == exposureMax_ && again_ > againMin_) {\n>> -\t\t\tnext = again_ * kExpNumeratorDown / kExpDenominator;\n>> -\t\t\tif (again_ - next < againMinStep_)\n>> -\t\t\t\tagain_ -= againMinStep_;\n>> -\t\t\telse\n>> -\t\t\t\tagain_ = next;\n>> -\t\t} else {\n>> -\t\t\tnext = exposure_ * kExpNumeratorDown / kExpDenominator;\n>> -\t\t\tif (exposure_ - next < 1)\n>> -\t\t\t\texposure_ -= 1;\n>> -\t\t\telse\n>> -\t\t\t\texposure_ = next;\n>> -\t\t}\n>> -\t}\n>> -\n>> -\texposure_ = std::clamp(exposure_, exposureMin_, exposureMax_);\n>> -\tagain_ = std::clamp(again_, againMin_, againMax_);\n>>   }\n>>     std::string IPASoftSimple::logPrefix() const\n>> diff --git a/src/libcamera/software_isp/TODO b/src/libcamera/software_isp/TODO\n>> index 8eeede46..a50db668 100644\n>> --- a/src/libcamera/software_isp/TODO\n>> +++ b/src/libcamera/software_isp/TODO\n>> @@ -199,16 +199,6 @@ Yes, because, well... all the other IPAs were doing that...\n>>     ---\n>>   -10. Switch to libipa/algorithm.h API in processStats\n>> -\n>> ->> void IPASoftSimple::processStats(const ControlList &sensorControls)\n>> ->>\n>> -> Do you envision switching to the libipa/algorithm.h API at some point ?\n>> -\n>> -At some point, yes.\n>> -\n>> ----\n>> -\n>>   13. Improve black level and colour gains application\n>>     I think the black level should eventually be moved before debayering, and","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 4CFEAC3257\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 27 Sep 2024 13:26:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EE29F63510;\n\tFri, 27 Sep 2024 15:26:24 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1B3A4634F2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 27 Sep 2024 15:26:22 +0200 (CEST)","from mail-wr1-f72.google.com (mail-wr1-f72.google.com\n\t[209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-475-xvuM5xBCNRqeu7hYxwJ_7g-1; Fri, 27 Sep 2024 09:26:20 -0400","by mail-wr1-f72.google.com with SMTP id\n\tffacd0b85a97d-37cd08d3678so852852f8f.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 27 Sep 2024 06:26:19 -0700 (PDT)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-42e96a36251sm75290425e9.37.2024.09.27.06.26.16\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 27 Sep 2024 06:26:17 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"SygPaV9i\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1727443582;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=GKUGGrS/2oqXPS+Jg5skFowz2LgNy/Jyam3xqzXSgEc=;\n\tb=SygPaV9in7SR/21cYU68YY328n3JMIT+Mfm0xH1klyriALpSYYL2pJc+CEVx8fVmUsVlAE\n\tk9hEnqlBsaE8NddvdaezKriMoBeutBzx0pbO6ficwFAM2HANLMLdVMiNDfBalKg12VjPkf\n\tAARl7K/3UXfwhDMkwS2KQTubQBSRoSo=","X-MC-Unique":"xvuM5xBCNRqeu7hYxwJ_7g-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1727443578; x=1728048378;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=GKUGGrS/2oqXPS+Jg5skFowz2LgNy/Jyam3xqzXSgEc=;\n\tb=vrE80+4uI1l2AxZ2c/SLxB9TIG+I7hJFLq/QI4lapoc9w9hZv7ghAXexDtSS/DsVno\n\tAN5hrhV0JBapvc9gjIttjWVzeUBQzIrq9VNK3c8oOmqEv0pvLH4h7J/cNLgwTDXYFzKr\n\tH1vgOHZPta+y2qJbva1CIb8lgv5wJ6bHVkf9xLb6yxaN5hF1yX96M66HN3xZRzX4olYb\n\tV+q6jvkB1gN6FTo/UlfwOww69uYJKOIl/zXidV++m5GQNHJt8AObe/RxfRMZuGIORk3e\n\tDvEqvV7SwDy8p4LbZGLmHOadK+9z1Mw9YpC+oRmoMYH6buvn7ZbIGnoYXZqTCBwSi50u\n\t8e6A==","X-Gm-Message-State":"AOJu0YztGmk4rCdy404jhCzGL5j9woh2lLFSgVsFkIhxsqiBxA7d4ehf\n\tq5LynAQoEBhTbrUbvB41T/Izpn2HIORC48aHL29tVA7AILsQjszudNcqjfrHZdJ/WIBb3pDdP97\n\tI+5zoHO1qN6i60tdCFgIC5vGzbqUU7cqov54GATnh/XjFb1n92z4EjMPe+NqyuLB4byupcdU=","X-Received":["by 2002:a5d:56ca:0:b0:374:c878:21f7 with SMTP id\n\tffacd0b85a97d-37ccdb1bbb7mr3995611f8f.10.1727443578160; \n\tFri, 27 Sep 2024 06:26:18 -0700 (PDT)","by 2002:a5d:56ca:0:b0:374:c878:21f7 with SMTP id\n\tffacd0b85a97d-37ccdb1bbb7mr3995593f8f.10.1727443577565; \n\tFri, 27 Sep 2024 06:26:17 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IFU6xhwaG+1///pVNU4ia4bfe6pulILILyzebeY4t6V9JryvV6/K8Yl2gL6SiwC4nFNJrq6iw==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Umang Jain <umang.jain@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,  Laurent Pinchart\n\t<laurent.pinchart@ideasonboard.com>,  Daniel Scally\n\t<dan.scally@ideasonboard.com>,  Kieran Bingham\n\t<kieran.bingham@ideasonboard.com>","Subject":"Re: [PATCH v7 17/18] libcamera: software_isp: Move exposure+gain to\n\tan algorithm module","In-Reply-To":"<41c9f1ca-2b7d-4cd1-b1db-0760f9f77d41@ideasonboard.com> (Umang\n\tJain's message of \"Fri, 27 Sep 2024 12:37:55 +0530\")","References":"<20240920183143.1674117-1-mzamazal@redhat.com>\n\t<20240920183143.1674117-18-mzamazal@redhat.com>\n\t<41c9f1ca-2b7d-4cd1-b1db-0760f9f77d41@ideasonboard.com>","Date":"Fri, 27 Sep 2024 15:26:16 +0200","Message-ID":"<877caxp6rr.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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>"}}]