[libcamera-devel] [PATCH v8 08/17] v4l2: Allocate buffers based on requested count and MinimumRequests

[Paul Elder]

On Tue, Aug 24, 2021 at 04:56:27PM -0300, Nícolas F. R. A. Prado wrote:
> Currently the number of buffers allocated is based on bufferCount, which
> is hardcoded to 1. Instead allocate buffers based on the requested count
> and on the MinimumRequests property for the camera, which is accessed
> through a newly added getter.
> 
> This allows the removal of bufferCount.
> 
> While at it, fix a typo: s/interval/internal/.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>

Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>

> 
> ---
> 
> Changes in v8:
> - New
> - Fixed typo: s/interval/internal/
> 
>  src/v4l2/v4l2_camera.cpp       | 20 +++++++++++++-------
>  src/v4l2/v4l2_camera.h         |  5 +++--
>  src/v4l2/v4l2_camera_proxy.cpp | 11 +++++------
>  3 files changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index d01eacfa2b84..4db171a3c0d0 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -12,6 +12,8 @@
>  
>  #include <libcamera/base/log.h>
>  
> +#include <libcamera/property_ids.h>
> +
>  using namespace libcamera;
>  
>  LOG_DECLARE_CATEGORY(V4L2Compat)
> @@ -107,15 +109,13 @@ void V4L2Camera::requestComplete(Request *request)
>  }
>  
>  int V4L2Camera::configure(StreamConfiguration *streamConfigOut,
> -			  const Size &size, const PixelFormat &pixelformat,
> -			  unsigned int bufferCount)
> +			  const Size &size, const PixelFormat &pixelformat)
>  {
>  	StreamConfiguration &streamConfig = config_->at(0);
>  	streamConfig.size.width = size.width;
>  	streamConfig.size.height = size.height;
>  	streamConfig.pixelFormat = pixelformat;
> -	streamConfig.bufferCount = bufferCount;
> -	/* \todo memoryType (interval vs external) */
> +	/* \todo memoryType (internal vs external) */
>  
>  	CameraConfiguration::Status validation = config_->validate();
>  	if (validation == CameraConfiguration::Invalid) {
> @@ -146,7 +146,6 @@ int V4L2Camera::validateConfiguration(const PixelFormat &pixelFormat,
>  	StreamConfiguration &cfg = config->at(0);
>  	cfg.size = size;
>  	cfg.pixelFormat = pixelFormat;
> -	cfg.bufferCount = 1;
>  
>  	CameraConfiguration::Status validation = config->validate();
>  	if (validation == CameraConfiguration::Invalid)
> @@ -165,7 +164,9 @@ int V4L2Camera::allocBuffers(unsigned int count)
>  	if (ret < 0)
>  		return ret;
>  
> -	for (unsigned int i = 0; i < count; i++) {
> +	unsigned int allocatedCount = ret;
> +
> +	for (unsigned int i = 0; i < allocatedCount; i++) {
>  		std::unique_ptr<Request> request = camera_->createRequest(i);
>  		if (!request) {
>  			requestPool_.clear();
> @@ -174,7 +175,7 @@ int V4L2Camera::allocBuffers(unsigned int count)
>  		requestPool_.push_back(std::move(request));
>  	}
>  
> -	return ret;
> +	return allocatedCount;
>  }
>  
>  void V4L2Camera::freeBuffers()
> @@ -299,3 +300,8 @@ bool V4L2Camera::isRunning()
>  {
>  	return isRunning_;
>  }
> +
> +unsigned int V4L2Camera::minimumRequests()
> +{
> +	return camera_->properties().get(properties::MinimumRequests);
> +}
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> index a095f4e27524..c0a122906f35 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -45,8 +45,7 @@ public:
>  	std::vector<Buffer> completedBuffers();
>  
>  	int configure(StreamConfiguration *streamConfigOut,
> -		      const Size &size, const PixelFormat &pixelformat,
> -		      unsigned int bufferCount);
> +		      const Size &size, const PixelFormat &pixelformat);
>  	int validateConfiguration(const PixelFormat &pixelformat,
>  				  const Size &size,
>  				  StreamConfiguration *streamConfigOut);
> @@ -65,6 +64,8 @@ public:
>  
>  	bool isRunning();
>  
> +	unsigned int minimumRequests();
> +
>  private:
>  	void requestComplete(Request *request);
>  
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 7682c4bddf90..424262dd205c 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -349,8 +349,7 @@ int V4L2CameraProxy::vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg)
>  	Size size(arg->fmt.pix.width, arg->fmt.pix.height);
>  	V4L2PixelFormat v4l2Format = V4L2PixelFormat(arg->fmt.pix.pixelformat);
>  	ret = vcam_->configure(&streamConfig_, size,
> -			       PixelFormatInfo::info(v4l2Format).format,
> -			       bufferCount_);
> +			       PixelFormatInfo::info(v4l2Format).format);
>  	if (ret < 0)
>  		return -EINVAL;
>  
> @@ -491,15 +490,13 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf
>  	Size size(v4l2PixFormat_.width, v4l2PixFormat_.height);
>  	V4L2PixelFormat v4l2Format = V4L2PixelFormat(v4l2PixFormat_.pixelformat);
>  	int ret = vcam_->configure(&streamConfig_, size,
> -				   PixelFormatInfo::info(v4l2Format).format,
> -				   arg->count);
> +				   PixelFormatInfo::info(v4l2Format).format);
>  	if (ret < 0)
>  		return -EINVAL;
>  
>  	setFmtFromConfig(streamConfig_);
>  
> -	arg->count = streamConfig_.bufferCount;
> -	bufferCount_ = arg->count;
> +	arg->count = std::max(arg->count, vcam_->minimumRequests());
>  
>  	ret = vcam_->allocBuffers(arg->count);
>  	if (ret < 0) {
> @@ -507,6 +504,8 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf
>  		return ret;
>  	}
>  
> +	bufferCount_ = arg->count = ret;
> +
>  	buffers_.resize(arg->count);
>  	for (unsigned int i = 0; i < arg->count; i++) {
>  		struct v4l2_buffer buf = {};
> -- 
> 2.33.0
>