[libcamera-devel] [PATCH 3/3] libcamera: v4l2_videodevice: Prevent queueing buffers without a cache

[Jacopo Mondi]

Hi Kieran,

On Wed, Mar 03, 2021 at 05:04:26PM +0000, Kieran Bingham wrote:
> The v4l2 buffer cache allows us to map incoming buffers to an instance
> of the V4L2 Buffer required to actually queue.
>
> If the cache_ is not available, then the buffers required to allow
> queuing to a device have been released, and this indicates an issue at
> the pipeline handler.
>
> This could be a common mistake, as it could happen if a pipeline handler
> always requeues buffers to the device after they complete, without
> checking if they are cancelled.
>
> That can then lead to trying to queue a buffer to a device which no
> longer expects buffers, so add a loud message to indicate what has
> happened but return an error without fatally crashing.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Personally I lean a bit towards Laurent's opinion that this should be
rather be made fatal and pipeline handler fixed.

Ater all this is not application facing and happens during pipeline
development, so it won't disappoint any user but rather make sure
developers notices this.

If the two of you agree this is ok as an Error, plese add my tag
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
   j

> ---
>  src/libcamera/v4l2_videodevice.cpp | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index c77e1aff7978..6129ce529afc 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -1390,6 +1390,16 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)
>  	struct v4l2_buffer buf = {};
>  	int ret;
>
> +	/*
> +	 * Pipeline handlers should not requeue buffers after releasing the
> +	 * buffers on the device. Any occurence of this error should be fixed
> +	 * in the pipeline handler directly.
> +	 */
> +	if (!cache_) {
> +		LOG(V4L2, Error) << "No BufferCache available to queue.";
> +		return -ENOENT;
> +	}
> +
>  	ret = cache_->get(*buffer);
>  	if (ret < 0)
>  		return ret;
> --
> 2.25.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel