[libcamera-devel] [PATCH 06/12] android: Move buffer mapping to CameraStream

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Feb 26, 2021 at 02:29:26PM +0100, Jacopo Mondi wrote:
> The destination buffer for the post-processing component is
> currently first mapped in the CameraDevice class and then passed
> to CameraStream which simply calls the post-processor interface.
> 
> Move the mapping to CameraStream::process() to tie the buffer
> mapping to the lifetime of the CameraBuffer instance.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/camera_device.cpp | 15 ++-------------
>  src/android/camera_device.h   |  1 -
>  src/android/camera_stream.cpp | 16 +++++++++++++---
>  src/android/camera_stream.h   |  2 +-
>  4 files changed, 16 insertions(+), 18 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 1c3b0f56ed28..ae01c362559e 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1857,19 +1857,8 @@ void CameraDevice::requestComplete(Request *request)
>  			continue;
>  		}
>  
> -		/*
> -		 * \todo Buffer mapping and compression should be moved to a
> -		 * separate thread.
> -		 */
> -
> -		CameraBuffer dest(*descriptor->buffers_[i].buffer,
> -				  PROT_READ | PROT_WRITE);
> -		if (!dest.isValid()) {
> -			LOG(HAL, Error) << "Failed to map android blob buffer";
> -			continue;
> -		}
> -
> -		int ret = cameraStream->process(*src, &dest,
> +		int ret = cameraStream->process(*src,
> +						*descriptor->buffers_[i].buffer,
>  						descriptor->settings_,
>  						resultMetadata.get());
>  		if (ret) {
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index e6c192c2100b..4905958e63c6 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -24,7 +24,6 @@
>  #include "libcamera/internal/log.h"
>  #include "libcamera/internal/message.h"
>  
> -#include "camera_buffer.h"
>  #include "camera_metadata.h"
>  #include "camera_stream.h"
>  #include "camera_worker.h"
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index 611ec0d1c42e..61b5bbf429ae 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -7,6 +7,7 @@
>  
>  #include "camera_stream.h"
>  
> +#include "camera_buffer.h"
>  #include "camera_device.h"
>  #include "camera_metadata.h"
>  #include "jpeg/post_processor_jpeg.h"
> @@ -96,15 +97,24 @@ int CameraStream::configure()
>  }
>  
>  int CameraStream::process(const libcamera::FrameBuffer &source,
> -			  libcamera::MappedBuffer *destination,
> +			  const buffer_handle_t camera3Dest,

Shouldn't you drop const, based on patch 04/12 ?

>  			  const CameraMetadata &requestMetadata,
>  			  CameraMetadata *resultMetadata)
>  {
>  	if (!postProcessor_)
>  		return 0;
>  
> -	return postProcessor_->process(source, destination,
> -				       requestMetadata, resultMetadata);
> +	/*
> +	 * \todo Buffer mapping and compression should be moved to a

While at it, s/compression/processing/ as we will have different types
of post-processing ?

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

> +	 * separate thread.
> +	 */
> +	CameraBuffer dest(camera3Dest, PROT_READ | PROT_WRITE);
> +	if (!dest.isValid()) {
> +		LOG(HAL, Error) << "Failed to map android blob buffer";
> +		return -EINVAL;
> +	}
> +
> +	return postProcessor_->process(source, &dest, requestMetadata, resultMetadata);
>  }
>  
>  FrameBuffer *CameraStream::getBuffer()
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index fc242b2aadf1..8673cacf15b0 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -120,7 +120,7 @@ public:
>  
>  	int configure();
>  	int process(const libcamera::FrameBuffer &source,
> -		    libcamera::MappedBuffer *destination,
> +		    const buffer_handle_t camera3Dest,
>  		    const CameraMetadata &requestMetadata,
>  		    CameraMetadata *resultMetadata);
>  	libcamera::FrameBuffer *getBuffer();

-- 
Regards,

Laurent Pinchart