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

[Kieran Bingham]

Quoting Laurent Pinchart (2025-01-27 07:04:27)
> Hi Milan,
> 
> Thank you for the patch.
> 
> On Mon, Jan 13, 2025 at 02:34:02PM +0100, Milan Zamazal wrote:
> > From: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > 
> > Provide the determined colour gains back into the metadata
> > to add to completed requests.
> > 
> > Metadata must be set before computing the new gain values in order to
> > report the metadata used to process the image rather than the metadata
> > determined from the image (and to be used for processing the next
> > image).
> 
> That's not right. Setting the metadata before computing the gains won't
> magically solve the synchronization problem. The code can be kept as-is
> (ideally with a comment to indicate synchronization needs to be fixed),
> but the commit message shouldn't be misleading.

I think this patch was written before we had a frame context queue.

Now this patch should be discarded, (or re-written) to store during
prepare() the gains used for that frame in the frame context, and set
the metadata from the values that were used when preparing the frame.


--
Kieran
 
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> >  src/ipa/simple/algorithms/awb.cpp | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp
> > index 195de41d..95ff4434 100644
> > --- a/src/ipa/simple/algorithms/awb.cpp
> > +++ b/src/ipa/simple/algorithms/awb.cpp
> > @@ -14,6 +14,8 @@
> >  
> >  #include "simple/ipa_context.h"
> >  
> > +#include "control_ids.h"
> > +
> >  namespace libcamera {
> >  
> >  LOG_DEFINE_CATEGORY(IPASoftAwb)
> > @@ -33,10 +35,16 @@ void Awb::process(IPAContext &context,
> >                 [[maybe_unused]] const uint32_t frame,
> >                 [[maybe_unused]] IPAFrameContext &frameContext,
> >                 const SwIspStats *stats,
> > -               [[maybe_unused]] ControlList &metadata)
> > +               ControlList &metadata)
> >  {
> >       const SwIspStats::Histogram &histogram = stats->yHistogram;
> >       const uint8_t blackLevel = context.activeState.blc.level;
> > +     auto &gains = context.activeState.gains;
> > +
> > +     const float maxGain = 1024.0;
> > +     const float mdGains[] = { static_cast<float>(gains.red / maxGain),
> > +                               static_cast<float>(gains.blue / maxGain) };
> > +     metadata.set(controls::ColourGains, mdGains);
> >  
> >       /*
> >        * Black level must be subtracted to get the correct AWB ratios, they
> > @@ -54,7 +62,6 @@ void Awb::process(IPAContext &context,
> >        * Calculate red and blue gains for AWB.
> >        * Clamp max gain at 4.0, this also avoids 0 division.
> >        */
> > -     auto &gains = context.activeState.gains;
> >       gains.red = sumR <= sumG / 4 ? 4.0 : static_cast<double>(sumG) / sumR;
> >       gains.blue = sumB <= sumG / 4 ? 4.0 : static_cast<double>(sumG) / sumB;
> >       /* Green gain is fixed to 1.0 */
> 
> -- 
> Regards,
> 
> Laurent Pinchart