[libcamera-devel] [RFC PATCH v1 06/12] libcamera: framebuffer: Add a function to check if planes are contiguous

[Kieran Bingham]

On 02/09/2021 05:22, 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>
> ---
>  include/libcamera/internal/framebuffer.h |  2 ++
>  src/libcamera/framebuffer.cpp            | 45 +++++++++++++++++++++++-
>  2 files changed, 46 insertions(+), 1 deletion(-)
> 
> 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 53ef89bf458f..99265b44da43 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 in discontiguous memory areas. This function checks in which of

'split them into discontiguous'

> + * 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,9 +210,41 @@ FrameBuffer::FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie)
>  	: Extensible(std::make_unique<Private>()), planes_(planes),
>  	  cookie_(cookie)
>  {
> +	unsigned int offset = 0;
> +
>  	/* \todo Remove the assertions after sufficient testing */
>  	for (const auto &plane : planes_)
>  		ASSERT(plane.offset != Plane::kInvalidOffset);
> +
> +	bool isContiguous = true;
> +	ino_t inode = 0;
> +
> +	for (const auto &plane : planes_) {
> +		if (plane.offset != offset) {
> +			isContiguous = false;
> +			break;
> +		}
> +
> +		/*
> +		 * Two different dmabuf file descriptors may still refer to the
> +		 * same dmabuf instance. Check this using inodes.

Going back to the FileDescriptor::inode() extension, if that cached the
inode (it can't change) on either first call, or construction, couldn't
this whole check be simplified to:

		if (plane.fd.inode() != planes_[0].fd.inode()) {
			isContiguous = false;
			break;
		}

> +		 */
> +		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;
>  }
>  
>  /**
>