Message ID | 20190829232653.13214-9-niklas.soderlund@ragnatech.se |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Niklas, Thank you for the patch. On Fri, Aug 30, 2019 at 01:26:47AM +0200, Niklas Söderlund wrote: > Add a RequestData pointer to the Request class, this is intended to be > used by pipeline handlers while handling the request and is invalid > outside the pipeline handler context. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > include/libcamera/request.h | 5 +++++ > src/libcamera/request.cpp | 10 +++++++++- > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/include/libcamera/request.h b/include/libcamera/request.h > index 9edf1cedc054014f..570924c5ef5e2425 100644 > --- a/include/libcamera/request.h > +++ b/include/libcamera/request.h > @@ -21,6 +21,9 @@ class Buffer; > class Camera; > class Stream; > > +class RequestData > +{ > +}; A forward declaration is enough here, let's not expose this class to applications. > > class Request > { > @@ -46,6 +49,8 @@ public: > > bool hasPendingBuffers() const { return !pending_.empty(); } > > + RequestData *data; As a public member ? No way :-) This will create issues to access the data pointer from pipeline handlers though, so I wonder if you wouldn't let pipeline handlers subclass Request instead, given that all requests are created by Camera::createRequest() that could easily be forwarded to pipeline handlers (with a default implementation in the base PipelineHandler class that creates a Request). Bonus points if you can make the Request constructor private. Feel free to propose an alternative scheme if you think that the Request class should be final. > + > private: > friend class Camera; > friend class PipelineHandler; > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > index ee2158fc7a9cf0b9..b8b4dfb4b7549163 100644 > --- a/src/libcamera/request.cpp > +++ b/src/libcamera/request.cpp > @@ -55,7 +55,7 @@ LOG_DEFINE_CATEGORY(Request) > * > */ > Request::Request(Camera *camera, uint64_t cookie) > - : camera_(camera), controls_(camera), cookie_(cookie), > + : data(nullptr), camera_(camera), controls_(camera), cookie_(cookie), > status_(RequestPending), cancelled_(false) > { > } > @@ -178,6 +178,14 @@ Buffer *Request::findBuffer(Stream *stream) const > * otherwise > */ > > +/** > + * \var Request::data > + * \brief Pipeline handler specific data Maybe "Pipeline handler-specific request data" ? > + * > + * Pipeline handlers may associate private data with with an request that s/with with/with/ s/an request/a request/ > + * is valid for the requests lifetime inside the pipeline handler. s/requests/request's/ This isn't very clear. I would explicitly state when this member becomes and stops being valid, or rather when the pipeline handler can touch it. I would also mention that nothing else is allowed to touch this. > + */ > + > /** > * \brief Validate the request and prepare it for the completion handler > *
diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 9edf1cedc054014f..570924c5ef5e2425 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -21,6 +21,9 @@ class Buffer; class Camera; class Stream; +class RequestData +{ +}; class Request { @@ -46,6 +49,8 @@ public: bool hasPendingBuffers() const { return !pending_.empty(); } + RequestData *data; + private: friend class Camera; friend class PipelineHandler; diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index ee2158fc7a9cf0b9..b8b4dfb4b7549163 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -55,7 +55,7 @@ LOG_DEFINE_CATEGORY(Request) * */ Request::Request(Camera *camera, uint64_t cookie) - : camera_(camera), controls_(camera), cookie_(cookie), + : data(nullptr), camera_(camera), controls_(camera), cookie_(cookie), status_(RequestPending), cancelled_(false) { } @@ -178,6 +178,14 @@ Buffer *Request::findBuffer(Stream *stream) const * otherwise */ +/** + * \var Request::data + * \brief Pipeline handler specific data + * + * Pipeline handlers may associate private data with with an request that + * is valid for the requests lifetime inside the pipeline handler. + */ + /** * \brief Validate the request and prepare it for the completion handler *
Add a RequestData pointer to the Request class, this is intended to be used by pipeline handlers while handling the request and is invalid outside the pipeline handler context. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- include/libcamera/request.h | 5 +++++ src/libcamera/request.cpp | 10 +++++++++- 2 files changed, 14 insertions(+), 1 deletion(-)