[libcamera-devel] [PATCH v2 09/27] libcamera: framebuffer: Add a function to check if planes are contiguous

[Jean-Michel Hautbois]

Hi Laurent,

On 06/09/2021 04:00, Laurent Pinchart wrote:
> Multi-planar frame buffers can store their planes contiguously in
> memory, or split them in discontiguous memory areas. Add a private
> function to check in which of these two categories the frame buffer
> belongs. This will be used to correctly handle the differences between
> the V4L2 single and multi planar APIs.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
> ---
> Changes v1:
> 
> - Merge both loops in FrameBuffer::FrameBuffer()
> ---
>  include/libcamera/internal/framebuffer.h |  2 ++
>  src/libcamera/framebuffer.cpp            | 45 ++++++++++++++++++++++--
>  2 files changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libcamera/internal/framebuffer.h b/include/libcamera/internal/framebuffer.h
> index 606aed2b4782..cd33c295466e 100644
> --- a/include/libcamera/internal/framebuffer.h
> +++ b/include/libcamera/internal/framebuffer.h
> @@ -21,9 +21,11 @@ public:
>  	Private();
>  
>  	void setRequest(Request *request) { request_ = request; }
> +	bool isContiguous() const { return isContiguous_; }
>  
>  private:
>  	Request *request_;
> +	bool isContiguous_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp
> index ad63a34a83bf..e71c2ffae034 100644
> --- a/src/libcamera/framebuffer.cpp
> +++ b/src/libcamera/framebuffer.cpp
> @@ -106,7 +106,7 @@ LOG_DEFINE_CATEGORY(Buffer)
>   */
>  
>  FrameBuffer::Private::Private()
> -	: request_(nullptr)
> +	: request_(nullptr), isContiguous_(true)
>  {
>  }
>  
> @@ -120,6 +120,17 @@ FrameBuffer::Private::Private()
>   * handlers, it is called by the pipeline handlers themselves.
>   */
>  
> +/**
> + * \fn FrameBuffer::Private::isContiguous()
> + * \brief Check if the frame buffer stores planes contiguously in memory
> + *
> + * Multi-planar frame buffers can store their planes contiguously in memory, or
> + * split them into discontiguous memory areas. This function checks in which of
> + * these two categories the frame buffer belongs.
> + *
> + * \return True if the planes are stored contiguously in memory, false otherwise
> + */
> +
>  /**
>   * \class FrameBuffer
>   * \brief Frame buffer data and its associated dynamic metadata
> @@ -199,8 +210,38 @@ FrameBuffer::FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie)
>  	: Extensible(std::make_unique<Private>()), planes_(planes),
>  	  cookie_(cookie)
>  {
> -	for (const auto &plane : planes_)
> +	unsigned int offset = 0;
> +	bool isContiguous = true;
> +	ino_t inode = 0;
> +
> +	for (const auto &plane : planes_) {
>  		ASSERT(plane.offset != Plane::kInvalidOffset);
> +
> +		if (plane.offset != offset) {
> +			isContiguous = false;
> +			break;
> +		}
> +
> +		/*
> +		 * Two different dmabuf file descriptors may still refer to the
> +		 * same dmabuf instance. Check this using inodes.
> +		 */

How is that possible ?

> +		if (plane.fd.fd() != planes_[0].fd.fd()) {
> +			if (!inode)
> +				inode = planes_[0].fd.inode();
> +			if (plane.fd.inode() != inode) {
> +				isContiguous = false;
> +				break;
> +			}
> +		}
> +
> +		offset += plane.length;
> +	}
> +
> +	LOG(Buffer, Debug)
> +		<< "Buffer is " << (isContiguous ? "not " : "") << "contiguous";
> +
> +	_d()->isContiguous_ = isContiguous;
>  }
>  
>  /**
>