[libcamera-devel] [PATCH 09/30] libcamera: buffers: Remove Plane class

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Wed, Nov 27, 2019 at 12:35:59AM +0100, Niklas Söderlund wrote:
> There are no users left of the Plane class, drop it.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

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

> ---
>  include/libcamera/buffer.h |  21 ------
>  src/libcamera/buffer.cpp   | 149 -------------------------------------
>  2 files changed, 170 deletions(-)
> 
> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h
> index 2dd5bcf3b49c4ee8..afcef805801a43b5 100644
> --- a/include/libcamera/buffer.h
> +++ b/include/libcamera/buffer.h
> @@ -77,27 +77,6 @@ private:
>  	void *mem_;
>  };
>  
> -class Plane final
> -{
> -public:
> -	Plane();
> -	~Plane();
> -
> -	int dmabuf() const { return fd_; }
> -	int setDmabuf(int fd, unsigned int length);
> -
> -	void *mem();
> -	unsigned int length() const { return length_; }
> -
> -private:
> -	int mmap();
> -	int munmap();
> -
> -	int fd_;
> -	unsigned int length_;
> -	void *mem_;
> -};
> -
>  class BufferMemory final
>  {
>  public:
> diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp
> index 07647124a2cd9c62..82b4799a2510d02f 100644
> --- a/src/libcamera/buffer.cpp
> +++ b/src/libcamera/buffer.cpp
> @@ -155,28 +155,6 @@ FileDescriptor::~FileDescriptor()
>   * \return A valid file descriptor or a negative error code
>   */
>  
> -/**
> - * \class Plane
> - * \brief A memory region to store a single plane of a frame
> - *
> - * Planar pixel formats use multiple memory regions to store planes
> - * corresponding to the different colour components of a frame. The Plane class
> - * tracks the specific details of a memory region used to store a single plane
> - * for a given frame and provides the means to access the memory, both for the
> - * application and for DMA. A Buffer then contains one or multiple planes
> - * depending on its pixel format.
> - *
> - * To support DMA access, planes are associated with dmabuf objects represented
> - * by file handles. Each plane carries a dmabuf file handle and an offset within
> - * the buffer. Those file handles may refer to the same dmabuf object, depending
> - * on whether the devices accessing the memory regions composing the image
> - * support non-contiguous DMA to planes ore require DMA-contiguous memory.
> - *
> - * To support CPU access, planes carry the CPU address of their backing memory.
> - * Similarly to the dmabuf file handles, the CPU addresses for planes composing
> - * an image may or may not be contiguous.
> - */
> -
>  /**
>   * \class Dmabuf
>   * \brief A memory region to store a single plane of a frame
> @@ -290,133 +268,6 @@ int Dmabuf::munmap()
>  	return ret;
>  }
>  
> -Plane::Plane()
> -	: fd_(-1), length_(0), mem_(0)
> -{
> -}
> -
> -Plane::~Plane()
> -{
> -	munmap();
> -
> -	if (fd_ != -1)
> -		close(fd_);
> -}
> -
> -/**
> - * \fn Plane::dmabuf()
> - * \brief Get the dmabuf file handle backing the buffer
> - */
> -
> -/**
> - * \brief Set the dmabuf file handle backing the buffer
> - * \param[in] fd The dmabuf file handle
> - * \param[in] length The size of the memory region
> - *
> - * The \a fd dmabuf file handle is duplicated and stored. The caller may close
> - * the original file handle.
> - *
> - * \return 0 on success or a negative error code otherwise
> - */
> -int Plane::setDmabuf(int fd, unsigned int length)
> -{
> -	if (fd < 0) {
> -		LOG(Buffer, Error) << "Invalid dmabuf fd provided";
> -		return -EINVAL;
> -	}
> -
> -	if (fd_ != -1) {
> -		close(fd_);
> -		fd_ = -1;
> -	}
> -
> -	fd_ = dup(fd);
> -	if (fd_ == -1) {
> -		int ret = -errno;
> -		LOG(Buffer, Error)
> -			<< "Failed to duplicate dmabuf: " << strerror(-ret);
> -		return ret;
> -	}
> -
> -	length_ = length;
> -
> -	return 0;
> -}
> -
> -/**
> - * \brief Map the plane memory data to a CPU accessible address
> - *
> - * The file descriptor to map the memory from must be set by a call to
> - * setDmaBuf() before calling this function.
> - *
> - * \sa setDmaBuf()
> - *
> - * \return 0 on success or a negative error code otherwise
> - */
> -int Plane::mmap()
> -{
> -	void *map;
> -
> -	if (mem_)
> -		return 0;
> -
> -	map = ::mmap(NULL, length_, PROT_READ | PROT_WRITE, MAP_SHARED, fd_, 0);
> -	if (map == MAP_FAILED) {
> -		int ret = -errno;
> -		LOG(Buffer, Error)
> -			<< "Failed to mmap plane: " << strerror(-ret);
> -		return ret;
> -	}
> -
> -	mem_ = map;
> -
> -	return 0;
> -}
> -
> -/**
> - * \brief Unmap any existing CPU accessible mapping
> - *
> - * Unmap the memory mapped by an earlier call to mmap().
> - *
> - * \return 0 on success or a negative error code otherwise
> - */
> -int Plane::munmap()
> -{
> -	int ret = 0;
> -
> -	if (mem_)
> -		ret = ::munmap(mem_, length_);
> -
> -	if (ret) {
> -		ret = -errno;
> -		LOG(Buffer, Warning)
> -			<< "Failed to unmap plane: " << strerror(-ret);
> -	} else {
> -		mem_ = 0;
> -	}
> -
> -	return ret;
> -}
> -
> -/**
> - * \fn Plane::mem()
> - * \brief Retrieve the CPU accessible memory address of the Plane
> - * \return The CPU accessible memory address on success or nullptr otherwise.
> - */
> -void *Plane::mem()
> -{
> -	if (!mem_)
> -		mmap();
> -
> -	return mem_;
> -}
> -
> -/**
> - * \fn Plane::length() const
> - * \brief Retrieve the length of the memory region
> - * \return The length of the memory region
> - */
> -
>  /**
>   * \class BufferMemory
>   * \brief A memory buffer to store an image

-- 
Regards,

Laurent Pinchart