| Message ID | 20190409192548.20325-10-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thanks for your work. On 2019-04-09 21:25:45 +0200, Jacopo Mondi wrote: > Add to the Buffer class methods to set and retrieve a reference to the > Request instance this buffer is part of. > > As Buffers might outlive the Request they are associated with, the > reference is only temporarly valid during the buffer completion interval > (since when the buffer gets queued to Camera for processing, until it > gets marked as completed). I do not like this as the new functions are visible both to applications and pipeline handlers where we really should make the API fool proof. Looking a head in the series I do not like how this is used neither. Something like this might be needed depending on outcome of discussions later patches in this series. The patch itself is proper and if I agreed that it was needed and/or that the usage of it later I would have attached by tag. For now I'm withholding it pending discussion. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > include/libcamera/buffer.h | 5 +++++ > src/libcamera/buffer.cpp | 39 +++++++++++++++++++++++++++++++++++++- > src/libcamera/request.cpp | 4 ++++ > 3 files changed, 47 insertions(+), 1 deletion(-) > > diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h > index 0c844d126a27..10c46cd57ab0 100644 > --- a/include/libcamera/buffer.h > +++ b/include/libcamera/buffer.h > @@ -13,6 +13,7 @@ > namespace libcamera { > > class BufferPool; > +class Request; > > class Plane final > { > @@ -53,6 +54,9 @@ public: > Status status() const { return status_; } > std::vector<Plane> &planes() { return planes_; } > > + void setRequest(Request *req) { request_ = req; } > + Request *request() const { return request_; } > + > private: > friend class BufferPool; > friend class PipelineHandler; > @@ -67,6 +71,7 @@ private: > Status status_; > > std::vector<Plane> planes_; > + Request *request_; > }; > > class BufferPool final > diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp > index e2d1cf04411e..68738b733ba6 100644 > --- a/src/libcamera/buffer.cpp > +++ b/src/libcamera/buffer.cpp > @@ -196,7 +196,7 @@ void *Plane::mem() > */ > > Buffer::Buffer() > - : index_(-1) > + : index_(-1), request_(nullptr) > { > } > > @@ -248,6 +248,43 @@ Buffer::Buffer() > * \return The buffer status > */ > > +/** > + * \fn Buffer::setRequest() > + * \brief Set the request this buffer belongs to > + * > + * Buffers are associated to Streams in a Request, which is then sent to the > + * Camera for processing. This method stores in the Buffer a pointer to the > + * Request this Buffer is part of, for later retrieval through the > + * Buffer::request() method. > + * > + * Buffers are associated to requests at Request::prepare() time and said > + * association is valid until the buffer does not complete at > + * Request::completeBuffer() time. Before and after the buffer completion > + * interval (the time between when the request is queued to the Camera, and > + * the buffer is marked as 'complete' by pipeline handlers) the reference to > + * Request is set to nullptr. > + */ > + > +/** > + * \fn Buffer::request() > + * \brief Retrieve the request this buffer belongs to > + * > + * The intended callers of this method are buffer completion slots > + * implemented in CameraData sub-classes which needs to associated a request > + * to the just completed buffer, before calling Request::completeBuffer(). > + * > + * It is up to the caller of this function to deal with the case the buffer has > + * been already marked as complete, and the reference to the Request has been > + * invalidated and set to nullptr. > + * > + * See also Buffer::setRequest() for a more detailed explanation of the > + * validity interval of the Request reference contained in a Buffer. > + * > + * \return The Request this Buffer belongs to or nullptr if the Buffer has > + * not been queued to the Camera for processing yet or it has completed already. > + * \sa Buffer::setRequest() > + */ > + > /** > * \brief Mark a buffer as cancel by setting its status to BufferCancelled > */ > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > index 3e10c995a6fa..7d80c635fa92 100644 > --- a/src/libcamera/request.cpp > +++ b/src/libcamera/request.cpp > @@ -130,6 +130,8 @@ int Request::prepare() > { > for (auto const &pair : bufferMap_) { > Buffer *buffer = pair.second; > + > + buffer->setRequest(this); > pending_.insert(buffer); > } > > @@ -166,6 +168,8 @@ bool Request::completeBuffer(Buffer *buffer) > int ret = pending_.erase(buffer); > ASSERT(ret == 1); > > + buffer->setRequest(nullptr); > + > return empty(); > } > > -- > 2.21.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 0c844d126a27..10c46cd57ab0 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -13,6 +13,7 @@ namespace libcamera { class BufferPool; +class Request; class Plane final { @@ -53,6 +54,9 @@ public: Status status() const { return status_; } std::vector<Plane> &planes() { return planes_; } + void setRequest(Request *req) { request_ = req; } + Request *request() const { return request_; } + private: friend class BufferPool; friend class PipelineHandler; @@ -67,6 +71,7 @@ private: Status status_; std::vector<Plane> planes_; + Request *request_; }; class BufferPool final diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index e2d1cf04411e..68738b733ba6 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -196,7 +196,7 @@ void *Plane::mem() */ Buffer::Buffer() - : index_(-1) + : index_(-1), request_(nullptr) { } @@ -248,6 +248,43 @@ Buffer::Buffer() * \return The buffer status */ +/** + * \fn Buffer::setRequest() + * \brief Set the request this buffer belongs to + * + * Buffers are associated to Streams in a Request, which is then sent to the + * Camera for processing. This method stores in the Buffer a pointer to the + * Request this Buffer is part of, for later retrieval through the + * Buffer::request() method. + * + * Buffers are associated to requests at Request::prepare() time and said + * association is valid until the buffer does not complete at + * Request::completeBuffer() time. Before and after the buffer completion + * interval (the time between when the request is queued to the Camera, and + * the buffer is marked as 'complete' by pipeline handlers) the reference to + * Request is set to nullptr. + */ + +/** + * \fn Buffer::request() + * \brief Retrieve the request this buffer belongs to + * + * The intended callers of this method are buffer completion slots + * implemented in CameraData sub-classes which needs to associated a request + * to the just completed buffer, before calling Request::completeBuffer(). + * + * It is up to the caller of this function to deal with the case the buffer has + * been already marked as complete, and the reference to the Request has been + * invalidated and set to nullptr. + * + * See also Buffer::setRequest() for a more detailed explanation of the + * validity interval of the Request reference contained in a Buffer. + * + * \return The Request this Buffer belongs to or nullptr if the Buffer has + * not been queued to the Camera for processing yet or it has completed already. + * \sa Buffer::setRequest() + */ + /** * \brief Mark a buffer as cancel by setting its status to BufferCancelled */ diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 3e10c995a6fa..7d80c635fa92 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -130,6 +130,8 @@ int Request::prepare() { for (auto const &pair : bufferMap_) { Buffer *buffer = pair.second; + + buffer->setRequest(this); pending_.insert(buffer); } @@ -166,6 +168,8 @@ bool Request::completeBuffer(Buffer *buffer) int ret = pending_.erase(buffer); ASSERT(ret == 1); + buffer->setRequest(nullptr); + return empty(); }
Add to the Buffer class methods to set and retrieve a reference to the Request instance this buffer is part of. As Buffers might outlive the Request they are associated with, the reference is only temporarly valid during the buffer completion interval (since when the buffer gets queued to Camera for processing, until it gets marked as completed). Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- include/libcamera/buffer.h | 5 +++++ src/libcamera/buffer.cpp | 39 +++++++++++++++++++++++++++++++++++++- src/libcamera/request.cpp | 4 ++++ 3 files changed, 47 insertions(+), 1 deletion(-)