[libcamera-devel] [PATCH 1/5] src: ipa: raspberrypi: Distinguish the first camera start from others

[Laurent Pinchart]

Hi David,

On Sat, Dec 05, 2020 at 10:39:55PM +0000, David Plowman wrote:
> On Sat, 5 Dec 2020 at 19:57, Laurent Pinchart wrote:
> > On Wed, Dec 02, 2020 at 11:52:49AM +0000, David Plowman wrote:
> > > This makes it possible to tell whether we're starting the sensor for
> > > the first time, or whether it's happening because of a mode switch or
> > > because the camera has been paused and re-started. Depending on this,
> > > some sensors may require us to drop different numbers of frames.
> > >
> > > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> >
> > This looks good to me, but I'd squash it with patch 2/5 as the chance is
> > very small and it's easier to review it when also seeing how the new
> > variable is used.
> 
> Will do!
> 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >
> > On a side note, how does mode change and initial start differ ? Don't we
> > stop the sensor when reconfiguring the camera ?
> 
> The difference is that on first camera start, the algorithms are
> starting "from scratch", with no idea what initial values to use. On
> subsequent mode switches, the algorithms are known to be in a sensible
> place and we just leave them be (we don't reset them, though the
> change of camera mode can some some other effects).

Ah yes that makes sense. What if the application stops the camera and
restarts it "a long time" later ? Would we need some kind of timeout
after which algorithms should drop their state (or at least consider the
state doesn't shorten the convergence time anymore) ?

> > > ---
> > >  src/ipa/raspberrypi/raspberrypi.cpp | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> > > index 69be5e4e..b8298768 100644
> > > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > > @@ -67,7 +67,7 @@ public:
> > >       IPARPi()
> > >               : lastMode_({}), controller_(), controllerInit_(false),
> > >                 frameCount_(0), checkCount_(0), mistrustCount_(0),
> > > -               lsTable_(nullptr)
> > > +               lsTable_(nullptr), firstStart_(true)
> > >       {
> > >       }
> > >
> > > @@ -145,6 +145,9 @@ private:
> > >       /* LS table allocation passed in from the pipeline handler. */
> > >       FileDescriptor lsTableHandle_;
> > >       void *lsTable_;
> > > +
> > > +     /* Distinguish the first camera start from others. */
> > > +     bool firstStart_;
> > >  };
> > >
> > >  int IPARPi::init(const IPASettings &settings)
> > > @@ -179,6 +182,8 @@ int IPARPi::start(const IPAOperationData &ipaConfig, IPAOperationData *result)
> > >               result->operation |= RPi::IPA_CONFIG_SENSOR;
> > >       }
> > >
> > > +     firstStart_ = false;
> > > +
> > >       return 0;
> > >  }
> > >

-- 
Regards,

Laurent Pinchart