[libcamera-devel] [PATCH v4 5/8] libcamera: v4l2_videodevice: Better tracking of the device state
Kieran Bingham
kieran.bingham at ideasonboard.com
Wed Mar 23 10:59:02 CET 2022
Quoting Naushir Patuck via libcamera-devel (2022-03-22 09:22:54)
> Replace the existing streaming_ state variable with an enum to track the
> following three state: Streaming, Stopping, and Stopped. The alternate states
> will be used in a subsequent commit.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> include/libcamera/internal/v4l2_videodevice.h | 3 ++-
> src/libcamera/v4l2_videodevice.cpp | 10 ++++++----
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> index 2d2ccc477c91..32e022543385 100644
> --- a/include/libcamera/internal/v4l2_videodevice.h
> +++ b/include/libcamera/internal/v4l2_videodevice.h
> @@ -258,7 +258,8 @@ private:
>
> EventNotifier *fdBufferNotifier_;
>
> - bool streaming_;
> + enum class State { Streaming, Stopping, Stopped };
> + State state_;
> };
>
> class V4L2M2MDevice
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 5f36ee20710d..9cea6a608660 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -507,7 +507,7 @@ const std::string V4L2DeviceFormat::toString() const
> */
> V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode)
> : V4L2Device(deviceNode), formatInfo_(nullptr), cache_(nullptr),
> - fdBufferNotifier_(nullptr), streaming_(false)
> + fdBufferNotifier_(nullptr), state_(State::Stopped)
> {
> /*
> * We default to an MMAP based CAPTURE video device, however this will
> @@ -1796,7 +1796,7 @@ int V4L2VideoDevice::streamOn()
> return ret;
> }
>
> - streaming_ = true;
> + state_ = State::Streaming;
>
> return 0;
> }
> @@ -1818,7 +1818,7 @@ int V4L2VideoDevice::streamOff()
> {
> int ret;
>
> - if (!streaming_ && queuedBuffers_.empty())
> + if (state_ != State::Streaming && queuedBuffers_.empty())
> return 0;
>
> ret = ioctl(VIDIOC_STREAMOFF, &bufferType_);
> @@ -1828,6 +1828,8 @@ int V4L2VideoDevice::streamOff()
> return ret;
> }
>
> + state_ = State::Stopping;
> +
Should this be before the call for VIDIOC_STREAMOFF?
It may not make a difference in practice, or it might cause
V4L2VideoDevice to reject a buffer (/request) that is queued in parallel
... but I think we already know the request can be queued while calling
streamOff ... so I 'think' it's safe to change the state before calling
the ioctl...
Which ever way you're happier with, or consensus goes to:
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> /* Send back all queued buffers. */
> for (auto it : queuedBuffers_) {
> FrameBuffer *buffer = it.second;
> @@ -1838,7 +1840,7 @@ int V4L2VideoDevice::streamOff()
>
> queuedBuffers_.clear();
> fdBufferNotifier_->setEnabled(false);
> - streaming_ = false;
> + state_ = State::Stopped;
>
> return 0;
> }
> --
> 2.25.1
>
More information about the libcamera-devel
mailing list