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

18022 diff mbox series

From: Nícolas F. R. A. Prado <nfraprado@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@collabora.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

---
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(-)

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@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@collabora.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
>
> ---

Seems reasonable

Reviewed-by: Jacopo Mondi <jacopo@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
>