[libcamera-devel] [PATCH 08/13] ipa: raspberrypi: Introduce IpaBase class

[Naushir Patuck]

Hi Laurent and Jacopo,

On Fri, 28 Apr 2023 at 14:57, Naushir Patuck <naush at raspberrypi.com> wrote:
>
> Hi Jacopo,
>

 <snip>

> >
> > When it comes to the call order, it's the base class driving the
> > procesing flow, while dispatching calls to the derived classes at the
> > right time. Moving to your suggestion would allow derived classes to
> > call the base class method, which means most of the control is moved
> > to the derived classes with possible code duplication.
> >
> > I think to actually decide where to go, it is useful to analyze what
> > class implements the operations flow control
> >
> > In one case
> >
> >         Base::method()                  Derived::platformMethod()
> >
> >                 doSomeWork();
> >                 ....
> >                 platformMethod() -->    doPlaformWork();
> >                 ...
> >                 finalizeWork();
> >
> > While in the other
> >
> >         Base::method()                  Derived::method()
> >
> >                                         doSomeWork();
> >                                         ....
> >                 method()        <--     Base::method()
> >                                         ...
> >                                         finalizeWork();
> >
> > Is this an over-simplification ?
> >
> > Naush, do you expect the derived classes to control the operation
> > flow, or will this be common to both and could be implemented in the
> > base classes ?
>
> No, I see no explicit need for operation flow to be dictated by the derived
> class.  Hence changing things around to your suggestion and making things more
> consistent.

Having looked further into this, I'm a bit more conflicted.  I initially thought
that I could simply revert to the original mechanism of having the control flow
start from the derived class and call into the base class, as the 2nd case
drawn above.  This does work neatly for the ipa::init() and ipa::configure().
However platformProcessStats() and platformPrepareIsp() require the control flow
from base class to derived class, as in the 1st case drawn above.

I don't think it's sensible to choose flow 1 for some functions and flow 2 for
others.  So that suggests to me that flow 1 (i.e. the existing flow) is probably
the right approach for the IPA changes.  Do folks think that makes sense?
Again, I am happy to change things if it seems more appropriate.

Regards,
Naush