[libcamera-devel] [PATCH v9 08/18] libcamera: pipeline: vimc, uvcvideo: Don't rely on bufferCount

[Jacopo Mondi]

Hi Paul

On Fri, Dec 16, 2022 at 09:29:29PM +0900, Paul Elder via libcamera-devel wrote:
> From: Nícolas F. R. A. Prado <nfraprado at collabora.com>
>
> Instead of using bufferCount as the number of V4L2 buffer slots to
> reserve in the vimc and uvcvideo pipeline handlers, use a reasonably
> high constant: 16. Overallocating isn't a problem as buffer slots are
> cheap. Having too few, on the other hand, could degrade performance. It
> is expected that this number will be more than enough for most, if not
> all, use cases.
>
> This makes way for removing bufferCount.
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---

Seems reasonable

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

> Changes in v9:
> - rebased
>
> Changes in v8:
> - New
> ---
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 5 +++--
>  src/libcamera/pipeline/vimc/vimc.cpp         | 5 +++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 4ce240a4..18966d01 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -97,6 +97,8 @@ private:
>  	{
>  		return static_cast<UVCCameraData *>(camera->_d());
>  	}
> +
> +	static constexpr unsigned int kUVCBufferSlotCount = 16;
>  };
>
>  UVCCameraConfiguration::UVCCameraConfiguration(UVCCameraData *data)
> @@ -239,9 +241,8 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera,
>  int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList *controls)
>  {
>  	UVCCameraData *data = cameraData(camera);
> -	unsigned int count = data->stream_.configuration().bufferCount;
>
> -	int ret = data->video_->importBuffers(count);
> +	int ret = data->video_->importBuffers(kUVCBufferSlotCount);
>  	if (ret < 0)
>  		return ret;
>
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index e58caf99..eaa6ebf7 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -106,6 +106,8 @@ private:
>  	{
>  		return static_cast<VimcCameraData *>(camera->_d());
>  	}
> +
> +	static constexpr unsigned int kVimcBufferSlotCount = 16;
>  };
>
>  namespace {
> @@ -334,9 +336,8 @@ int PipelineHandlerVimc::exportFrameBuffers(Camera *camera,
>  int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] const ControlList *controls)
>  {
>  	VimcCameraData *data = cameraData(camera);
> -	unsigned int count = data->stream_.configuration().bufferCount;
>
> -	int ret = data->video_->importBuffers(count);
> +	int ret = data->video_->importBuffers(kVimcBufferSlotCount);
>  	if (ret < 0)
>  		return ret;
>
> --
> 2.35.1
>