[PATCH v3 5/6] ipa: simple: Report contrast in metadata

[Laurent Pinchart]

On Mon, Jan 27, 2025 at 06:37:31PM +0000, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2025-01-27 07:09:35)
> > On Mon, Jan 13, 2025 at 02:34:04PM +0100, Milan Zamazal wrote:
> > > Provide the requested contrast value, if any, in the metadata to add to
> > > the completed requests.
> > > 
> > > Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> > > ---
> > >  src/ipa/simple/algorithms/lut.cpp | 11 +++++++++++
> > >  src/ipa/simple/algorithms/lut.h   |  5 +++++
> > >  2 files changed, 16 insertions(+)
> > > 
> > > diff --git a/src/ipa/simple/algorithms/lut.cpp b/src/ipa/simple/algorithms/lut.cpp
> > > index 0ba2391f..31208bfe 100644
> > > --- a/src/ipa/simple/algorithms/lut.cpp
> > > +++ b/src/ipa/simple/algorithms/lut.cpp
> > > @@ -116,6 +116,17 @@ void Lut::prepare(IPAContext &context,
> > >       }
> > >  }
> > >  
> > > +void Lut::process(IPAContext &context,
> > > +               [[maybe_unused]] const uint32_t frame,
> > > +               [[maybe_unused]] IPAFrameContext &frameContext,
> > > +               [[maybe_unused]] const SwIspStats *stats,
> > > +               ControlList &metadata)
> > > +{
> > > +     const auto contrast = context.activeState.knobs.contrast;
> > 
> > Don't copy.
> > 
> > > +     if (contrast)
> > > +             metadata.set(controls::Contrast, contrast.value());
> > 
> > This will not report the right value for the frame, but given that it's
> > not worse than the other patches,
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> But we /have/ a frame context to store the actual contrast value applied
> on the frame during prepare() ... so this should be done correctly, not
> reading the active state, but the frameContext.

I think it can wait until we solve the synchronization issue on rkisp1.
I then expect we'll apply the same solution to other IPA modules.

> > > +}
> > > +
> > >  REGISTER_IPA_ALGORITHM(Lut, "Lut")
> > >  
> > >  } /* namespace ipa::soft::algorithms */
> > > diff --git a/src/ipa/simple/algorithms/lut.h b/src/ipa/simple/algorithms/lut.h
> > > index 889f864b..ab4e1094 100644
> > > --- a/src/ipa/simple/algorithms/lut.h
> > > +++ b/src/ipa/simple/algorithms/lut.h
> > > @@ -30,6 +30,11 @@ public:
> > >                    const uint32_t frame,
> > >                    IPAFrameContext &frameContext,
> > >                    DebayerParams *params) override;
> > > +     void process(IPAContext &context,
> > > +                  const uint32_t frame,
> > > +                  IPAFrameContext &frameContext,
> > > +                  const SwIspStats *stats,
> > > +                  ControlList &metadata) override;
> > >  
> > >  private:
> > >       void updateGammaTable(IPAContext &context);

-- 
Regards,

Laurent Pinchart