[libcamera-devel] [PATCH v2 1/2] libcamera: pipeline_handler: Register requests

[Kieran Bingham]

Quoting Jacopo Mondi (2022-01-19 08:42:12)
> Hi Kieran,
> 
> On Wed, Jan 19, 2022 at 12:17:16AM +0000, Kieran Bingham wrote:
> > Provide a call allowing requests to be registered and associated with
> > the pipeline handler after being constructed by the camera.
> >
> > This provides an opportunity for the Pipeline Handler to connect any
> > signals it may be interested in receiving for the request such as
> > getting notifications when the request is ready for processing when
> > using a fence.
> >
> > While here, update the existing usage of the d pointer in
> > Camera::createRequest() to match the style of other functions.
> >
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> >  include/libcamera/internal/pipeline_handler.h |  1 +
> >  src/libcamera/camera.cpp                      | 14 +++++++++++---
> >  src/libcamera/pipeline_handler.cpp            | 19 ++++++++++++++++---
> >  3 files changed, 28 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> > index e5b8ffb4db3d..c3e4c2587ecd 100644
> > --- a/include/libcamera/internal/pipeline_handler.h
> > +++ b/include/libcamera/internal/pipeline_handler.h
> > @@ -59,6 +59,7 @@ public:
> >       void stop(Camera *camera);
> >       bool hasPendingRequests(const Camera *camera) const;
> >
> > +     void registerRequest(Request *request);
> >       void queueRequest(Request *request);
> >
> >       bool completeBuffer(Request *request, FrameBuffer *buffer);
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index 86d84ac0a77d..5e4b84e24235 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -1074,12 +1074,20 @@ int Camera::configure(CameraConfiguration *config)
> >   */
> >  std::unique_ptr<Request> Camera::createRequest(uint64_t cookie)
> >  {
> > -     int ret = _d()->isAccessAllowed(Private::CameraConfigured,
> > -                                     Private::CameraRunning);
> > +     Private *const d = _d();
> > +
> > +     int ret = d->isAccessAllowed(Private::CameraConfigured,
> > +                                  Private::CameraRunning);
> >       if (ret < 0)
> >               return nullptr;
> >
> > -     return std::make_unique<Request>(this, cookie);
> > +     std::unique_ptr<Request> request = std::make_unique<Request>(this, cookie);
> > +
> > +     /* Associate the request with the pipeline handler. */
> > +     if (request)
> > +             d->pipe_->registerRequest(request.get());
> 
> Can a constructor fail ?
> 

Good point, I think we can leave this with the assumption that it
succeeds. C-habbits die hard.

> Apart from that:
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> 

Thanks.

K

> Thanks
>   j
> 
> > +
> > +     return request;
> >  }
> >
> >  /**
> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> > index 03e4b9e66aa6..e27dc182c128 100644
> > --- a/src/libcamera/pipeline_handler.cpp
> > +++ b/src/libcamera/pipeline_handler.cpp
> > @@ -337,6 +337,22 @@ bool PipelineHandler::hasPendingRequests(const Camera *camera) const
> >       return !camera->_d()->queuedRequests_.empty();
> >  }
> >
> > +/**
> > + * \fn PipelineHandler::registerRequest()
> > + * \brief Register a request for use by the Pipeline Handler
> > + * \param[in] request The request to register
> > + */
> > +void PipelineHandler::registerRequest(Request *request)
> > +{
> > +     /*
> > +      * Connect the request prepared signal to notify the pipeline handler
> > +      * when a request is ready to be processed.
> > +      */
> > +     request->_d()->prepared.connect(this, [this]() {
> > +                                             doQueueRequests();
> > +                                     });
> > +}
> > +
> >  /**
> >   * \fn PipelineHandler::queueRequest()
> >   * \brief Queue a request
> > @@ -366,9 +382,6 @@ void PipelineHandler::queueRequest(Request *request)
> >
> >       waitingRequests_.push(request);
> >
> > -     request->_d()->prepared.connect(this, [this]() {
> > -                                             doQueueRequests();
> > -                                     });
> >       request->_d()->prepare(300ms);
> >  }
> >
> > --
> > 2.32.0
> >