[libcamera-devel] [RFC PATCH 2/8] libcamera: buffer: Convert copyFrom to use MappedFrameBuffer

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Mon, Jul 20, 2020 at 11:42:26PM +0100, Kieran Bingham wrote:
> Utilise the new MappedFrameBuffer helper to handle all mapping and
> unmapping of the copyFrom helper function.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

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

> ---
> 
> This shows how the MappedFrameBuffer can then be utilised, even though I
> believe this particular copyFrom() function is expected to disappear
> sometime soon.
> 
>  src/libcamera/buffer.cpp | 37 ++++++++++++++-----------------------
>  1 file changed, 14 insertions(+), 23 deletions(-)
> 
> diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp
> index 28b43d937f57..af5345b91195 100644
> --- a/src/libcamera/buffer.cpp
> +++ b/src/libcamera/buffer.cpp
> @@ -257,32 +257,23 @@ int FrameBuffer::copyFrom(const FrameBuffer *src)
>  		}
>  	}
>  
> -	for (unsigned int i = 0; i < planes_.size(); i++) {
> -		void *dstmem = mmap(nullptr, planes_[i].length, PROT_WRITE,
> -				    MAP_SHARED, planes_[i].fd.fd(), 0);
> +	MappedFrameBuffer source(src, PROT_READ);
> +	MappedFrameBuffer destination(this, PROT_WRITE);
>  
> -		if (dstmem == MAP_FAILED) {
> -			LOG(Buffer, Error)
> -				<< "Failed to map destination plane " << i;
> -			metadata_.status = FrameMetadata::FrameError;
> -			return -EINVAL;
> -		}
> -
> -		void *srcmem = mmap(nullptr, src->planes_[i].length, PROT_READ,
> -				    MAP_SHARED, src->planes_[i].fd.fd(), 0);
> -
> -		if (srcmem == MAP_FAILED) {
> -			munmap(dstmem, planes_[i].length);
> -			LOG(Buffer, Error)
> -				<< "Failed to map source plane " << i;
> -			metadata_.status = FrameMetadata::FrameError;
> -			return -EINVAL;
> -		}
> +	if (!source.isValid()) {
> +		LOG(Buffer, Error) << "Failed to map source planes";
> +		return -EINVAL;
> +	}
>  
> -		memcpy(dstmem, srcmem, src->planes_[i].length);
> +	if (!destination.isValid()) {
> +		LOG(Buffer, Error) << "Failed to map destination planes";
> +		return -EINVAL;
> +	}
>  
> -		munmap(srcmem, src->planes_[i].length);
> -		munmap(dstmem, planes_[i].length);
> +	for (unsigned int i = 0; i < planes_.size(); i++) {
> +		memcpy(destination.maps()[i].address,
> +		       source.maps()[i].address,
> +		       source.maps()[i].length);
>  	}
>  
>  	metadata_ = src->metadata_;

-- 
Regards,

Laurent Pinchart