[libcamera-devel] [PATCH v4 6/8] libcamera: v4l2_videodevice: Do not allow buffer queueing in stopping state

[Laurent Pinchart]

Hi Naush,

Thank you for the patch.

On Tue, Mar 22, 2022 at 09:22:55AM +0000, Naushir Patuck via libcamera-devel wrote:
> If the device is in the process of being stopped (i.e. Stopping state), any
> call to queueBuffer() must fail. This is to ensure the integrity of the buffer
> queue, as it gets cleared at the end of streamOff.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  src/libcamera/v4l2_videodevice.cpp | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 9cea6a608660..1516be129d73 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -1491,6 +1491,9 @@ int V4L2VideoDevice::releaseBuffers()
>   * The best available V4L2 buffer is picked for \a buffer using the V4L2 buffer
>   * cache.
>   *
> + * Note that \a queueBuffer will fail if the device is in the process of being

If you write it queueBuffer() doxygen should recognize the function call
and generate proper links, you can drop the \a. Same below.

> + * stopped from a streaming state through the \a streamOff function.
> + *
>   * \return 0 on success or a negative error code otherwise
>   */
>  int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)
> @@ -1499,6 +1502,11 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)
>  	struct v4l2_buffer buf = {};
>  	int ret;
>  
> +	if (state_ == State::Stopping) {
> +		LOG(V4L2, Error) << "Device is in a stopping state.";

Does this currently happen ? I assume it does, causing the unit test
failures in v3 of the series. Is it a bug on the application side, or a
problem elsewhere ? Are the cam, qcam and simple-cam applications
affected ? We should not have any error message printed under normal
operation conditions, so I'd like to at least know what needs to be
fixed.

> +		return -EPERM;

I'm tempted to use -ESHUTDOWN as a more precise alternative. Any opinion
from anyone ?

> +	}
> +
>  	/*
>  	 * Pipeline handlers should not requeue buffers after releasing the
>  	 * buffers on the device. Any occurence of this error should be fixed

-- 
Regards,

Laurent Pinchart