| Message ID | 20210419131433.22920-9-jacopo@jmondi.org |
|---|---|
| State | Accepted |
| Delegated to: | Jacopo Mondi |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Mon, Apr 19, 2021 at 03:14:28PM +0200, Jacopo Mondi wrote: > I got fooled by the documentation of setRequest() implying that the > function is meant to be called by pipeline handlers only, which it is > used in the Request class at Request::addBuffer() and Request::reuse() > time. I think it used to be called by pipeline handlers only. A Fixes: tag could be useful. > Expand the documentation to report that. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/buffer.cpp | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp > index 75b2693281a7..d8036ebec63e 100644 > --- a/src/libcamera/buffer.cpp > +++ b/src/libcamera/buffer.cpp > @@ -191,8 +191,15 @@ FrameBuffer::FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie) > * \brief Set the request this buffer belongs to > * \param[in] request Request to set > * > - * The intended callers of this method are pipeline handlers and only for > - * buffers that are internal to the pipeline. > + * This method is also used to associate a buffer with the Request it belongs > + * to, so that it can be retrieved through FrameBuffer::request(). Starting the documentation with "also" is strange. s/also // should be enough. > + * > + * This method is used by Request::addBuffer(), when an application facing > + * buffer is first added to the Request, and when the Request is reused with > + * Request::reuse(). > + * > + * For buffers that are internal to the pipeline the intended callers of this > + * method are pipeline handlers. * For buffers added to requests by applications, this method is called by * Request::addBuffer() or Request::reuse(). For buffers internal to pipeline * handlers, it is called by the pipeline handlers themselves. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > * > * \todo Shall be hidden from applications with a d-pointer design. > */
diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 75b2693281a7..d8036ebec63e 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -191,8 +191,15 @@ FrameBuffer::FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie) * \brief Set the request this buffer belongs to * \param[in] request Request to set * - * The intended callers of this method are pipeline handlers and only for - * buffers that are internal to the pipeline. + * This method is also used to associate a buffer with the Request it belongs + * to, so that it can be retrieved through FrameBuffer::request(). + * + * This method is used by Request::addBuffer(), when an application facing + * buffer is first added to the Request, and when the Request is reused with + * Request::reuse(). + * + * For buffers that are internal to the pipeline the intended callers of this + * method are pipeline handlers. * * \todo Shall be hidden from applications with a d-pointer design. */
I got fooled by the documentation of setRequest() implying that the function is meant to be called by pipeline handlers only, which it is used in the Request class at Request::addBuffer() and Request::reuse() time. Expand the documentation to report that. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/buffer.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)