[PATCH v3 7/7] [DNI] ipa: Add debug controls to the agc algorithm

[Kieran Bingham]

Quoting Stefan Klug (2024-10-08 16:29:45)
> Add a few (arbitrary) debug metadata to the agc algorithm. This shows
> how easy it is to add debug metadata to an ipa even for classes that
> don't have direct access to the metadata list or the context like
> AgcMeanLuminance. The control_ids_debug.yaml was autogenerated by
> calling utils/gen-debug-controls.py
> 
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> 
> ---
> Update in v2:
> - Break too long line in agc.cpp
> ---
>  src/ipa/libipa/agc_mean_luminance.cpp |  8 ++++++++
>  src/ipa/libipa/agc_mean_luminance.h   |  4 ++++
>  src/ipa/rkisp1/algorithms/agc.cpp     | 10 ++++++++++
>  src/libcamera/control_ids_debug.yaml  | 21 ++++++++++++++++++++-
>  4 files changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
> index f97ef11771c4..bd0f85afcd2e 100644
> --- a/src/ipa/libipa/agc_mean_luminance.cpp
> +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> @@ -133,6 +133,11 @@ static constexpr double kDefaultRelativeLuminanceTarget = 0.16;
>   * values.
>   */
>  
> +/**
> + * \var AgcMeanLuminance::debugMeta_
> + * \brief DebugMetadata helper
> + */
> +
>  AgcMeanLuminance::AgcMeanLuminance()
>         : frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0)
>  {
> @@ -541,8 +546,11 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex,
>                 exposureModeHelpers_.at(exposureModeIndex);
>  
>         double gain = estimateInitialGain();
> +       debugMeta_.set<float>(controls::debug::AgcInitialGain, static_cast<float>(gain));
>         gain = constraintClampGain(constraintModeIndex, yHist, gain);
>  
> +       debugMeta_.set<float>(controls::debug::AgcNewGain, static_cast<float>(gain));
> +
>         /*
>          * We don't check whether we're already close to the target, because
>          * even if the effective exposure value is the same as the last frame's
> diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
> index 576d28be8eb0..428465a11a36 100644
> --- a/src/ipa/libipa/agc_mean_luminance.h
> +++ b/src/ipa/libipa/agc_mean_luminance.h
> @@ -16,6 +16,7 @@
>  
>  #include <libcamera/controls.h>
>  
> +#include "libcamera/internal/debug_controls.h"
>  #include "libcamera/internal/yaml_parser.h"
>  
>  #include "exposure_mode_helper.h"
> @@ -71,6 +72,9 @@ public:
>                 frameCount_ = 0;
>         }
>  
> +protected:
> +       DebugMetadata debugMeta_;
> +
>  private:
>         virtual double estimateLuminance(const double gain) const = 0;
>  
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 17d074d9c03e..6668db942042 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -139,6 +139,8 @@ int Agc::init(IPAContext &context, const YamlObject &tuningData)
>  {
>         int ret;
>  
> +       debugMeta_.setParent(&context.debugMetadata);
> +
>         ret = parseTuningData(tuningData);
>         if (ret)
>                 return ret;
> @@ -444,6 +446,14 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>                                frameContext.agc.exposureMode,
>                                hist, effectiveExposureValue);
>  
> +       debugMeta_.set<float>(controls::debug::AgcAnalogGain, aGain);
> +       debugMeta_.set<float>(controls::debug::AgcDigitalGain, dGain);
> +
> +       const Span<const uint64_t> orig = hist.data();
> +       const Span<const int64_t> data{ reinterpret_cast<const int64_t *>(&orig[0]),
> +                                       orig.size() };
> +       debugMeta_.set<Span<const int64_t>>(controls::debug::AgcHistogram, data);
> +
>         LOG(RkISP1Agc, Debug)
>                 << "Divided up shutter, analogue gain and digital gain are "
>                 << shutterTime << ", " << aGain << " and " << dGain;
> diff --git a/src/libcamera/control_ids_debug.yaml b/src/libcamera/control_ids_debug.yaml
> index 797532712099..356eea627f0f 100644
> --- a/src/libcamera/control_ids_debug.yaml
> +++ b/src/libcamera/control_ids_debug.yaml
> @@ -1,6 +1,25 @@
>  # SPDX-License-Identifier: LGPL-2.1-or-later
>  #
> +# This file was generated by utils/gen-debug-controls.py
> +#
>  %YAML 1.1
>  ---
>  vendor: debug
> -controls: []
> +controls:
> +- AgcAnalogGain:
> +    type: float
> +    description: Debug control AgcAnalogGain found in src/ipa/rkisp1/algorithms/agc.cpp:448
> +- AgcDigitalGain:
> +    type: float
> +    description: Debug control AgcDigitalGain found in src/ipa/rkisp1/algorithms/agc.cpp:449
> +- AgcHistogram:
> +    type: int64_t
> +    description: Debug control AgcHistogram found in src/ipa/rkisp1/algorithms/agc.cpp:454
> +    size: '[n]'
> +- AgcInitialGain:
> +    type: float
> +    description: Debug control AgcInitialGain found in src/ipa/libipa/agc_mean_luminance.cpp:548
> +- AgcNewGain:
> +    type: float
> +    description: Debug control AgcNewGain found in src/ipa/libipa/agc_mean_luminance.cpp:551

Curiosity hits me here.

What happens if two places try to set the same control? Perhaps in two
IPA modules? Or is this undefined behaviour/not permitted?

--
Kieran


> +
> -- 
> 2.43.0
>