[libcamera-devel] [PATCH v4 08/32] libcamera: buffer: Add FrameBuffer interface

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Sun, Jan 12, 2020 at 02:01:48AM +0100, Niklas Söderlund wrote:
> Add a new FrameBuffer class to describe memory used to store frames.
> This change just adds the new interface, future patches will migrate all
> parts of libcamera to use this and replace the old Buffer interface.
> 
> This change needs to specify the const keyword for Plane::length() as to
> not get Doxygen confused with FrameBuffer::Plane::length added in this
> change.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
> * Changes since v3
> - Delete moved-constructor and move-assignment operator
> - Update documentation.
> 
> * Changes since v2
> - Drop default value for planes argument for FrameBuffer::FrameBuffer(planes, cookie).
> - Deleted FrameBuffer copy constructor and assignment operator.
> - Add a Move constructor for FrameBuffer.
> - Update documentation.
> 
> Changes since v1:
> - Rename FrameBuffer::info() to FrameBuffer::metadata()
> - Fixup commit message
> - Reorder method order
> - Rewrite documentation
> - Add operator==() and operator=!() for FrameBuffer::Plane
> ---
>  include/libcamera/buffer.h |  38 ++++++++++++
>  src/libcamera/buffer.cpp   | 117 ++++++++++++++++++++++++++++++++++++-
>  2 files changed, 154 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h
> index 0b95e41a2dd205b2..e66e9c9cf828160a 100644
> --- a/include/libcamera/buffer.h
> +++ b/include/libcamera/buffer.h
> @@ -11,6 +11,8 @@
>  #include <stdint.h>
>  #include <vector>
>  
> +#include <libcamera/file_descriptor.h>
> +
>  namespace libcamera {
>  
>  class Request;
> @@ -33,6 +35,42 @@ struct FrameMetadata {
>  	std::vector<Plane> planes;
>  };
>  
> +class FrameBuffer final
> +{
> +public:
> +	struct Plane {
> +		FileDescriptor fd;
> +		unsigned int length;
> +	};
> +
> +	FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie = 0);
> +
> +	FrameBuffer(const FrameBuffer &) = delete;
> +	FrameBuffer(FrameBuffer &&) = delete;
> +
> +	FrameBuffer &operator=(const FrameBuffer &) = delete;
> +	FrameBuffer &operator=(FrameBuffer &&) = delete;
> +
> +	const std::vector<Plane> &planes() const { return planes_; }
> +
> +	Request *request() const { return request_; }
> +	const FrameMetadata &metadata() const { return metadata_; };
> +
> +	unsigned int cookie() const { return cookie_; }
> +	void setCookie(unsigned int cookie) { cookie_ = cookie; }
> +
> +private:
> +	friend class Request; /* Needed to update request_. */
> +	friend class V4L2VideoDevice; /* Needed to update metadata_. */
> +
> +	std::vector<Plane> planes_;
> +
> +	Request *request_;
> +	FrameMetadata metadata_;
> +
> +	unsigned int cookie_;
> +};
> +
>  class Plane final
>  {
>  public:
> diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp
> index 360eb26cb4ca4906..2178bd2fe17111dc 100644
> --- a/src/libcamera/buffer.cpp
> +++ b/src/libcamera/buffer.cpp
> @@ -240,7 +240,7 @@ void *Plane::mem()
>  }
>  
>  /**
> - * \fn Plane::length()
> + * \fn Plane::length() const
>   * \brief Retrieve the length of the memory region
>   * \return The length of the memory region
>   */
> @@ -473,4 +473,119 @@ void Buffer::cancel()
>   * The intended callers are Request::addBuffer() and Request::completeBuffer().
>   */
>  
> +/**
> + * \class FrameBuffer
> + * \brief Frame buffer data and its associated dynamic metadata
> + *
> + * The FrameBuffer class is the primary interface for applications, IPAs and
> + * pipeline handlers to interact with frame memory. It contains all the static
> + * and dynamic information to manage the whole life cycle of a frame capture,
> + * from buffer creation to consumption.
> + *
> + * The static information describes the memory planes that make a frame. The
> + * planes are specified when creating the FrameBuffer and are expressed as a set
> + * of dmabuf file descriptors and length.
> + *
> + * The dynamic information is grouped in a FrameMetadata instance. It is updated
> + * during the processing of a queued capture request, and is valid from the
> + * completion of the buffer as signaled by Camera::bufferComplete() until the
> + * FrameBuffer is either reused in a new request or deleted.
> + *
> + * The creator of a FrameBuffer (application, IPA or pipeline handler) may
> + * associate to it an integer cookie for any private purpose. The cookie may be
> + * set when creating the FrameBuffer, and updated at any time with setCookie().
> + * The cookie is transparent to the libcamera core and shall only be set by the
> + * creator of the FrameBuffer. This mechanism supplements the Request cookie.
> + */
> +
> +/**
> + * \struct FrameBuffer::Plane
> + * \brief A memory region to store a single plane of a frame
> + *
> + * Planar pixel formats use multiple memory regions to store the different
> + * colour components of a frame. The Plane structure describes such a memory
> + * region by a dmabuf file descriptor and a length. A FrameBuffer then
> + * contains one or multiple planes, depending on the pixel format of the
> + * frames it is meant to store.
> + *
> + * To support DMA access, planes are associated with dmabuf objects represented
> + * by FileDescriptor handles. The Plane class doesn't handle mapping of the
> + * memory to the CPU, but applications and IPAs may use the dmabuf file
> + * descriptors to map the plane memory with mmap() and access its contents.
> + *
> + * \todo Once we have a Kernel API which can express offsets within a plane
> + * this structure shall be extended to contain this information. See commit
> + * 83148ce8be55e for initial documentation of this feature.
> + */
> +
> +/**
> + * \var FrameBuffer::Plane::fd
> + * \brief The dmabuf file descriptor
> + */
> +
> +/**
> + * \var FrameBuffer::Plane::length
> + * \brief The plane length in bytes
> + */
> +
> +/**
> + * \brief Construct a FrameBuffer with an array of planes
> + * \param[in] planes The frame memory planes
> + * \param[in] cookie Cookie
> + */
> +FrameBuffer::FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie)
> +	: planes_(planes), request_(nullptr), cookie_(cookie)
> +{
> +}
> +
> +/**
> + * \fn FrameBuffer::planes()
> + * \brief Retrieve the static plane descriptors
> + * \return Array of plane descriptors
> + */
> +
> +/**
> + * \fn FrameBuffer::request()
> + * \brief Retrieve the request this buffer belongs to
> + *
> + * The intended callers of this method are buffer completion handlers that
> + * need to associate a buffer to the request it belongs to.
> + *
> + * A Buffer is associated to a request by Request::addBuffer() and the
> + * association is valid until the buffer completes. The returned request
> + * pointer is valid only during that interval.
> + *
> + * \return The Request the Buffer belongs to, or nullptr if the buffer is
> + * not associated with a request
> + */
> +
> +/**
> + * \fn FrameBuffer::metadata()
> + * \brief Retrieve the dynamic metadata
> + * \return Dynamic metadata for the frame contained in the buffer
> + */
> +
> +/**
> + * \fn FrameBuffer::cookie()
> + * \brief Retrieve the cookie
> + *
> + * The cookie belongs to the creator of the FrameBuffer, which controls its
> + * lifetime and value.
> + *
> + * \sa setCookie()
> + *
> + * \return The cookie
> + */
> +
> +/**
> + * \fn FrameBuffer::setCookie()
> + * \brief Set the cookie
> + * \param[in] cookie Cookie to set
> + *
> + * The cookie belongs to the creator of the FrameBuffer. Its value may be
> + * modified at any time with this method. Applications and IPAs shall not modify
> + * the cookie value of buffers they haven't created themselves. The libcamera
> + * core never modifies the buffer cookie.
> + */
> +
>  } /* namespace libcamera */

-- 
Regards,

Laurent Pinchart