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

[Kieran Bingham]

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.



> 
> > +}
> > +
> >  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