[libcamera-devel,v8,08/17] v4l2: Allocate buffers based on requested count and MinimumRequests
diff mbox series

Message ID 20210824195636.1110845-9-nfraprado@collabora.com
State Superseded
Headers show
Series
  • lc-compliance: Add test to queue more requests than hardware depth
Related show

Commit Message

Nícolas F. R. A. Prado Aug. 24, 2021, 7:56 p.m. UTC 
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@collabora.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(-)

Comments

Paul Elder Nov. 30, 2022, 11:18 a.m. UTC | #1 
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@collabora.com>

Reviewed-by: Paul Elder <paul.elder@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
>

Patch
diff mbox series 

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 = {};