[libcamera-devel] [PATCH 1/2] libcamera: v4l2_videodevice: Track streaming state
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Jan 11 18:09:04 CET 2021
Hi Umang,
On 11/01/2021 16:33, Umang Jain wrote:
> From: Kieran Bingham <kieran.bingham at 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 at ideasonboard.com>
> Signed-off-by: Umang Jain <email at uajain.com>
Thanks for re-vitalising this patch, I thought it wasn't useful back
then, so it's nice to see it re-surface and has become useful again.
> ---
> include/libcamera/internal/v4l2_videodevice.h | 2 ++
> src/libcamera/v4l2_videodevice.cpp | 9 ++++++++-
> 2 files changed, 10 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..164954f2 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;
> }
>
> @@ -1571,6 +1574,9 @@ int V4L2VideoDevice::streamOff()
> {
> int ret;
>
> + if (!streaming_ && queuedBuffers_.empty())
> + return 0;
> +
ah nice, I was worried that we were going to be modifying streaming_
when queuing buffers ... so that looks much nicer and doesn't detract
from the meaning of streaming_
So Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Both Niklas and Laurent had reviewed the earlier incantation of this
patch, but it has now changed with this line so I'll let them decide if
they want to re-give tags.
But Laurent's previous comment regarding documentation update still holds:
> I forgot to mention that previously, I think the documentation should be
> updated to state that stopping an already stopped device is a no-op.
--
Kieran
> ret = ioctl(VIDIOC_STREAMOFF, &bufferType_);
> if (ret < 0) {
> LOG(V4L2, Error)
> @@ -1588,6 +1594,7 @@ int V4L2VideoDevice::streamOff()
>
> queuedBuffers_.clear();
> fdBufferNotifier_->setEnabled(false);
> + streaming_ = false;
>
> return 0;
> }
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list