[PATCH v6 11/18] libcamera: software_isp: Call Algorithm::prepare

[Kieran Bingham]

Quoting Dan Scally (2024-09-09 09:21:45)
> Hi Milan
> 
> On 06/09/2024 13:09, Milan Zamazal wrote:
> > This patch adds Algorithm::prepare call for the defined algorithms.
> > This is preparation only since there are currently no Algorithm based
> > algorithms defined.
> >
> > Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> > Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> > ---
> 
> Reviewed-by: Daniel Scally <dan.scally at ideasonboard.com>
> 
> >   include/libcamera/ipa/soft.mojom            | 1 +
> >   src/ipa/simple/soft_simple.cpp              | 8 ++++++++
> >   src/libcamera/software_isp/software_isp.cpp | 1 +
> >   3 files changed, 10 insertions(+)
> >
> > diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
> > index ddccd154..347fd69b 100644
> > --- a/include/libcamera/ipa/soft.mojom
> > +++ b/include/libcamera/ipa/soft.mojom
> > @@ -24,6 +24,7 @@ interface IPASoftInterface {
> >               => (int32 ret);
> >   
> >       [async] queueRequest(uint32 frame, libcamera.ControlList sensorControls);
> > +     [async] fillParamsBuffer(uint32 frame);
> >       [async] processStats(uint32 frame,
> >                            uint32 bufferId,
> >                            libcamera.ControlList sensorControls);
> > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> > index eb3bbd92..3a0cb6e0 100644
> > --- a/src/ipa/simple/soft_simple.cpp
> > +++ b/src/ipa/simple/soft_simple.cpp
> > @@ -79,6 +79,7 @@ public:
> >       void stop() override;
> >   
> >       void queueRequest(const uint32_t frame, const ControlList &controls) override;
> > +     void fillParamsBuffer(const uint32_t frame) override;
> >       void processStats(const uint32_t frame, const uint32_t bufferId,
> >                         const ControlList &sensorControls) override;
> >   
> > @@ -279,6 +280,13 @@ void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &contro
> >               algo->queueRequest(context_, frame, frameContext, controls);
> >   }
> >   
> > +void IPASoftSimple::fillParamsBuffer(const uint32_t frame)
> > +{
> > +     IPAFrameContext &frameContext = context_.frameContexts.get(frame);
> > +     for (auto const &algo : algorithms())
> > +             algo->prepare(context_, frame, frameContext, params_);

This doesn't fill a parameter buffer. Do we anticipate having a
structured buffer to pass to the SoftISP components?

Otherwise perhaps we should just call this
	'IPASoftSimple::prepare(const uint32_t frame)'


> > +}
> > +
> >   void IPASoftSimple::processStats([[maybe_unused]] const uint32_t frame,
> >                                [[maybe_unused]] const uint32_t bufferId,
> >                                const ControlList &sensorControls)
> > diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> > index eda18947..564e44e8 100644
> > --- a/src/libcamera/software_isp/software_isp.cpp
> > +++ b/src/libcamera/software_isp/software_isp.cpp
> > @@ -352,6 +352,7 @@ void SoftwareIsp::stop()
> >    */
> >   void SoftwareIsp::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output)
> >   {
> > +     ipa_->fillParamsBuffer(frame);

Oh... are we mixing prepare and process calls?

I'll have to take a look at the bigger picture ....

> >       debayer_->invokeMethod(&DebayerCpu::process,
> >                              ConnectionTypeQueued, frame, input, output, debayerParams_);
> >   }