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

[Milan Zamazal]

Laurent Pinchart <laurent.pinchart at ideasonboard.com> writes:

> 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.

Well, it's easy to use the frame context and have a bit less incorrect
stuff around, I'll do it in v4.

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