[libcamera-devel,v2,1/2] libcamera: v4l2_videodevice: Track streaming state

10849 diff mbox series

From: Kieran Bingham <kieran.bingham@ideasonboard.com>

Track the state of streamon/streamoff calls to simplify error paths.

Ensuring that streamOff() can be called on non-streaming streams
facilitates simpler error code paths, where a set of devices can all
call streamOff regardless of their initialisation state.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Umang Jain <email@uajain.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 include/libcamera/internal/v4l2_videodevice.h |  2 ++
 src/libcamera/v4l2_videodevice.cpp            | 12 +++++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

Hi Umang and Kieran,

Thank you for the patch.

On Tue, Jan 12, 2021 at 10:21:39AM +0530, Umang Jain wrote:
> From: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> Track the state of streamon/streamoff calls to simplify error paths.
> 
> Ensuring that streamOff() can be called on non-streaming streams
> facilitates simpler error code paths, where a set of devices can all
> call streamOff regardless of their initialisation state.
>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Umang Jain <email@uajain.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  include/libcamera/internal/v4l2_videodevice.h |  2 ++
>  src/libcamera/v4l2_videodevice.cpp            | 12 +++++++++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> index 529ca0e3..626dfbcd 100644
> --- a/include/libcamera/internal/v4l2_videodevice.h
> +++ b/include/libcamera/internal/v4l2_videodevice.h
> @@ -249,6 +249,8 @@ private:
>  	std::map<unsigned int, FrameBuffer *> queuedBuffers_;
>  
>  	EventNotifier *fdBufferNotifier_;
> +
> +	bool streaming_;
>  };
>  
>  class V4L2M2MDevice
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index baf683d6..a9509bff 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -481,7 +481,8 @@ const std::string V4L2DeviceFormat::toString() const
>   * \param[in] deviceNode The file-system path to the video device node
>   */
>  V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode)
> -	: V4L2Device(deviceNode), cache_(nullptr), fdBufferNotifier_(nullptr)
> +	: V4L2Device(deviceNode), cache_(nullptr), fdBufferNotifier_(nullptr),
> +	  streaming_(false)
>  {
>  	/*
>  	 * We default to an MMAP based CAPTURE video device, however this will
> @@ -1554,6 +1555,8 @@ int V4L2VideoDevice::streamOn()
>  		return ret;
>  	}
>  
> +	streaming_ = true;
> +
>  	return 0;
>  }
>  
> @@ -1565,12 +1568,18 @@ int V4L2VideoDevice::streamOn()
>   * and the bufferReady signal is emitted for them. The order in which those
>   * buffers are dequeued is not specified.
>   *
> + * This will be a no-op if the stream is not started in the first place and
> + * has no queued buffers.
> + *
>   * \return 0 on success or a negative error code otherwise
>   */
>  int V4L2VideoDevice::streamOff()
>  {
>  	int ret;
>  
> +	if (!streaming_ && queuedBuffers_.empty())
> +		return 0;
> +
>  	ret = ioctl(VIDIOC_STREAMOFF, &bufferType_);
>  	if (ret < 0) {
>  		LOG(V4L2, Error)
> @@ -1588,6 +1597,7 @@ int V4L2VideoDevice::streamOff()
>  
>  	queuedBuffers_.clear();
>  	fdBufferNotifier_->setEnabled(false);
> +	streaming_ = false;
>  
>  	return 0;
>  }