[PATCH v5 3/6] ipa: simple: Report the ColourGains in metadata

[Milan Zamazal]

Hi Kieran,

thank you for review.

Kieran Bingham <kieran.bingham at ideasonboard.com> writes:

> Quoting Milan Zamazal (2025-03-26 12:48:52)
>> From: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> 
>
>> Provide the determined colour gains back into the metadata
>> to add to completed requests.
>> 
>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
>> ---
>>  src/ipa/simple/algorithms/awb.cpp | 21 ++++++++++++++++++++-
>>  src/ipa/simple/algorithms/awb.h   |  6 +++++-
>>  src/ipa/simple/ipa_context.h      |  4 ++++
>>  3 files changed, 29 insertions(+), 2 deletions(-)
>> 
>> diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp
>> index ec77c6e5..55719059 100644
>> --- a/src/ipa/simple/algorithms/awb.cpp
>> +++ b/src/ipa/simple/algorithms/awb.cpp
>> @@ -17,6 +17,8 @@
>>  #include "libipa/colours.h"
>>  #include "simple/ipa_context.h"
>>  
>> +#include "control_ids.h"
>> +
>>  namespace libcamera {
>>  
>>  LOG_DEFINE_CATEGORY(IPASoftAwb)
>> @@ -32,15 +34,32 @@ int Awb::configure(IPAContext &context,
>>         return 0;
>>  }
>>  
>> +void Awb::prepare(IPAContext &context,
>> +                 [[maybe_unused]] const uint32_t frame,
>> +                 IPAFrameContext &frameContext,
>> +                 [[maybe_unused]] DebayerParams *params)
>> +{
>> +       auto &gains = context.activeState.awb.gains;
>> +       frameContext.gains.red = gains.r();
>> +       frameContext.gains.blue = gains.b();
>
> Interesting that this highlights the white balance parameters are
> probably being used from the wrong 'time' as they should be determined
> here in prepare... (by taking the most recent active state and combining
> any decision from the controls for any manual white balance).

I'm not sure I understand what you mean.  They come from the most recent
active state, don't they?  We don't support manual AWB balance but if we
did the gains could be amended here accordingly.  And later used in Lut
algorithm.  The gains stored here and used to process the frame are then
reported in metadata for this frame.

What's wrong then?

> But - as this series is just trying to get the reporting in - lets leave
> that issue for later, and try to keep this moving forwards:
>
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
>
>
>> +}
>> +
>>  void Awb::process(IPAContext &context,
>>                   [[maybe_unused]] const uint32_t frame,
>> -                 [[maybe_unused]] IPAFrameContext &frameContext,
>> +                 IPAFrameContext &frameContext,
>>                   const SwIspStats *stats,
>>                   ControlList &metadata)
>>  {
>>         const SwIspStats::Histogram &histogram = stats->yHistogram;
>>         const uint8_t blackLevel = context.activeState.blc.level;
>>  
>> +       const float maxGain = 1024.0;
>> +       const float mdGains[] = {
>> +               static_cast<float>(frameContext.gains.red / maxGain),
>> +               static_cast<float>(frameContext.gains.blue / maxGain)
>> +       };
>> +       metadata.set(controls::ColourGains, mdGains);
>> +
>>         /*
>>          * Black level must be subtracted to get the correct AWB ratios, they
>>          * would be off if they were computed from the whole brightness range
>> diff --git a/src/ipa/simple/algorithms/awb.h b/src/ipa/simple/algorithms/awb.h
>> index db1496cd..ad993f39 100644
>> --- a/src/ipa/simple/algorithms/awb.h
>> +++ b/src/ipa/simple/algorithms/awb.h
>> @@ -1,6 +1,6 @@
>>  /* SPDX-License-Identifier: LGPL-2.1-or-later */
>>  /*
>> - * Copyright (C) 2024, Red Hat Inc.
>> + * Copyright (C) 2024-2025 Red Hat Inc.
>>   *
>>   * Auto white balance
>>   */
>> @@ -20,6 +20,10 @@ public:
>>         ~Awb() = default;
>>  
>>         int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
>> +       void prepare(IPAContext &context,
>> +                    const uint32_t frame,
>> +                    IPAFrameContext &frameContext,
>> +                    DebayerParams *params) override;
>>         void process(IPAContext &context,
>>                      const uint32_t frame,
>>                      IPAFrameContext &frameContext,
>> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h
>> index 17bcd4ca..bfac835b 100644
>> --- a/src/ipa/simple/ipa_context.h
>> +++ b/src/ipa/simple/ipa_context.h
>> @@ -70,6 +70,10 @@ struct IPAFrameContext : public FrameContext {
>>                 int32_t exposure;
>>                 double gain;
>>         } sensor;
>> +       struct {
>> +               double red;
>> +               double blue;
>> +       } gains;
>>  };
>>  
>>  struct IPAContext {
>> -- 
>> 2.49.0
>>