| Message ID | 20220119001717.2503111-2-kieran.bingham@ideasonboard.com |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series |
|
| Related | show |
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@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 ? Apart from that: Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> 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 >
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@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@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 > >
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()); + + 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); }
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@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(-)