Message ID | 20220121142420.3531497-2-kieran.bingham@ideasonboard.com |
---|---|
State | Superseded |
Delegated to: | Kieran Bingham |
Headers | show |
Series |
|
Related | show |
Hi Kieran, Thank you for the patch. On Fri, Jan 21, 2022 at 02:24:19PM +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 s/Pipeline Handler/pipeline handler/ (or PipelineHandler) > 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@ideasonboard.com> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > --- > > v3: > - Remove unneeded conditional on construction of the request. > > v2: > - Rename function to 'registerRequest' rather than 'prepareRequest'. > > include/libcamera/internal/pipeline_handler.h | 1 + > src/libcamera/camera.cpp | 13 ++++++++++--- > src/libcamera/pipeline_handler.cpp | 19 ++++++++++++++++--- > 3 files changed, 27 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..bb856d606f4a 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -1074,12 +1074,19 @@ 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. */ > + d->pipe_->registerRequest(request.get()); > + > + 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 s/Pipeline Handler/pipeline handler/ > + * \param[in] request The request to register Maybe we could add * This function is called when the request is created, and allows the pipeline * handler to perform any one-time initialization it requires for the request. > + */ > +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(); > + }); While at it, this be simplified to request->_d()->prepared.connect(this, &PipelineHandler::doQueueRequests); Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > +} > + > /** > * \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); > } >
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..bb856d606f4a 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -1074,12 +1074,19 @@ 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. */ + d->pipe_->registerRequest(request.get()); + + 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); }