[libcamera-devel] [PATCH v3 11/23] libcamera: camera_sensor: Expose DelayedControls interface

[Jacopo Mondi]

Hi Kieran,

On Fri, Jul 01, 2022 at 12:13:22AM +0100, Kieran Bingham wrote:
> Quoting Jacopo Mondi via libcamera-devel (2022-06-30 14:38:50)
> > Now that the CameraSensor class has an instance of delayed controls
> > expose its interface for pipeline handlers to use it.
> >
> > The interface towards DelayedControls still operates on lists of
> > V4L2 controls.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  include/libcamera/internal/camera_sensor.h    |  4 +++
> >  src/libcamera/camera_sensor/camera_sensor.cpp | 31 +++++++++++++++++++
> >  2 files changed, 35 insertions(+)
> >
> > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> > index bd5aa0dbc27d..a606bc5d1cb5 100644
> > --- a/include/libcamera/internal/camera_sensor.h
> > +++ b/include/libcamera/internal/camera_sensor.h
> > @@ -71,6 +71,10 @@ public:
> >
> >         CameraLens *focusLens() { return focusLens_.get(); }
> >
> > +       void frameStart(uint32_t sequence);
> > +       void pushControls(const ControlList &ctrls);
> > +       ControlList getControls(uint32_t sequence);
> > +
> >  protected:
> >         std::string logPrefix() const override;
> >
> > diff --git a/src/libcamera/camera_sensor/camera_sensor.cpp b/src/libcamera/camera_sensor/camera_sensor.cpp
> > index 211c7461f5c6..e1770e8fa130 100644
> > --- a/src/libcamera/camera_sensor/camera_sensor.cpp
> > +++ b/src/libcamera/camera_sensor/camera_sensor.cpp
> > @@ -1043,6 +1043,37 @@ void CameraSensor::updateControlInfo()
> >         updateControls();
> >  }
> >
> > +/**
> > + * \brief Signal to the camera sensor class that a new frame has start exposing
> > + * \param[in] sequence The frame sequence number
> > + */
> > +void CameraSensor::frameStart(uint32_t sequence)
> > +{
> > +       /* A new capture session requires to start in a clean state. */
> > +       if (sequence == 0)
> > +               delayedCtrls_->reset();
>
> We want to have queued up some controls *before* we start the device,
> and so here this races with anything that might have been queued up
> before the first frame SoF is called.

AH! You're right, there could be controls queued up at Camera::start()
that would be overridden by this.

I tried to be smart and avoid having to add a 'start()' function to
CameraSensor, but that won't work well..

Good catch, we need an explicit reset to be issued at
Camera::configure() time.

>
> I don't know 'how much' of a race it is ... but I don't think it's good
> to have this reset here. I think it should be an explict reset tied in
> elsewhere.
>
>
> > +
> > +       delayedCtrls_->applyControls(sequence);
> > +}
> > +
> > +/**
> > + * \brief Push a new set of controls to the CameraSensor
> > + * \param[in] ctrls The list of controls to push to the sensor
> > + */
> > +void CameraSensor::pushControls(const ControlList &ctrls)
>
> I think we're going to want to pass in the sequence number here, and
> call it setControls probably. But while this is currently push in the
> DelayedControls, keeping the existing implementation during the move is
> good I think.
>
>
>
> > +{
> > +       delayedCtrls_->push(ctrls);
> > +}
> > +
> > +/**
> > + * \brief Get the list of controls applied at frame \a sequence
> > + * \param[in] sequence The frame sequence number
> > + */
> > +ControlList CameraSensor::getControls(uint32_t sequence)
> > +{
> > +       return delayedCtrls_->get(sequence);
> > +}
> > +
> >  /**
> >   * \fn CameraSensor::focusLens()
> >   * \brief Retrieve the focus lens controller
> > --
> > 2.36.1
> >