[libcamera-devel] [PATCH 08/13] libcamera: buffer: Re-work setRequest() documentation

[Laurent Pinchart]

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 at 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 at ideasonboard.com>

>   *
>   * \todo Shall be hidden from applications with a d-pointer design.
>   */

-- 
Regards,

Laurent Pinchart