[libcamera-devel] [PATCH 10/15] android: camera_stream: Allocate FrameBuffers

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Mon, Oct 05, 2020 at 01:46:44PM +0300, Laurent Pinchart wrote:
> From: Jacopo Mondi <jacopo at jmondi.org>
> 
> Allocate FrameBuffers using the allocator_ class member in the
> CameraStream class at CameraStream::configure() time.
> 
> As FrameBuffer allocation can only happen after the Camera has been
> correctly configured, move the CameraStream configuration loop
> after the Camera::configure() call in CameraDevice.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/camera_device.cpp | 22 +++++++++++-----------
>  src/android/camera_stream.cpp | 17 +++++++++++++++--
>  src/android/camera_stream.h   |  2 ++
>  3 files changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index adaec54dbfdb..537883b63f15 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1282,6 +1282,17 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>  		return -EINVAL;
>  	}
>  
> +	/*
> +	 * Once the CameraConfiguration has been adjusted/validated
> +	 * it can be applied to the camera.
> +	 */
> +	int ret = camera_->configure(config_.get());
> +	if (ret) {
> +		LOG(HAL, Error) << "Failed to configure camera '"
> +				<< camera_->id() << "'";
> +		return ret;
> +	}
> +
>  	/*
>  	 * Configure the HAL CameraStream instances using the associated
>  	 * StreamConfiguration and set the number of required buffers in
> @@ -1300,17 +1311,6 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>  		stream->max_buffers = cfg.bufferCount;
>  	}
>  
> -	/*
> -	 * Once the CameraConfiguration has been adjusted/validated
> -	 * it can be applied to the camera.
> -	 */
> -	int ret = camera_->configure(config_.get());
> -	if (ret) {
> -		LOG(HAL, Error) << "Failed to configure camera '"
> -				<< camera_->id() << "'";
> -		return ret;
> -	}
> -
>  	return 0;
>  }
>  
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index 9f3e7026b1a4..76e7d240f4e7 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -58,8 +58,21 @@ const Size &CameraStream::size() const
>  
>  int CameraStream::configure(const libcamera::StreamConfiguration &cfg)
>  {
> -	if (encoder_)
> -		return encoder_->configure(cfg);
> +	if (encoder_) {
> +		int ret = encoder_->configure(cfg);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (allocator_) {
> +		int ret = allocator_->allocate(stream());
> +		if (ret < 0)
> +			return ret;
> +
> +		/* Save a pointer to the reserved frame buffers */
> +		for (const auto &frameBuffer : allocator_->buffers(stream()))
> +			buffers_.push_back(frameBuffer.get());

I'd move this to patch 12/15, it's not clear in this patch why you need
it.

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

> +	}
>  
>  	return 0;
>  }
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index eaf4357ed096..e399e17b2a2f 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -8,6 +8,7 @@
>  #define __ANDROID_CAMERA_STREAM_H__
>  
>  #include <memory>
> +#include <vector>
>  
>  #include <hardware/camera3.h>
>  
> @@ -140,6 +141,7 @@ private:
>  	libcamera::CameraConfiguration *config_;
>  	Encoder *encoder_;
>  	libcamera::FrameBufferAllocator *allocator_;
> +	std::vector<libcamera::FrameBuffer *> buffers_;
>  };
>  
>  #endif /* __ANDROID_CAMERA_STREAM__ */

-- 
Regards,

Laurent Pinchart