Message ID | 20191025141327.2803-1-kieran.bingham@ideasonboard.com |
---|---|
State | Superseded |
Delegated to: | Kieran Bingham |
Headers | show |
Series |
|
Related | show |
Hi Kieran, Thank you for the patch. On Fri, Oct 25, 2019 at 03:13:27PM +0100, Kieran Bingham wrote: > 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. > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > > Since RFC: > - Rebased to master. > > src/libcamera/include/v4l2_videodevice.h | 2 ++ > src/libcamera/v4l2_videodevice.cpp | 10 +++++++++- > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h > index 4b8cf9394eb9..caf36bf9c1aa 100644 > --- a/src/libcamera/include/v4l2_videodevice.h > +++ b/src/libcamera/include/v4l2_videodevice.h > @@ -187,6 +187,8 @@ private: > std::map<unsigned int, Buffer *> queuedBuffers_; > > EventNotifier *fdEvent_; > + > + bool streaming_; > }; > > class V4L2M2MDevice > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index 208ab54199b1..c0b3a79f76da 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -272,7 +272,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), bufferPool_(nullptr), fdEvent_(nullptr) > + : V4L2Device(deviceNode), bufferPool_(nullptr), fdEvent_(nullptr), > + streaming_(false) > { > /* > * We default to an MMAP based CAPTURE video device, however this will > @@ -1173,6 +1174,8 @@ int V4L2VideoDevice::streamOn() > return ret; > } > > + streaming_ = true; > + > return 0; > } > > @@ -1190,6 +1193,9 @@ int V4L2VideoDevice::streamOff() > { > int ret; > > + if (!streaming_) > + return 0; > + 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. With that, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > ret = ioctl(VIDIOC_STREAMOFF, &bufferType_); > if (ret < 0) { > LOG(V4L2, Error) > @@ -1210,6 +1216,8 @@ int V4L2VideoDevice::streamOff() > queuedBuffers_.clear(); > fdEvent_->setEnabled(false); > > + streaming_ = false; > + > return 0; > } >
Hi Kieran, Thanks for your work. On 2019-10-25 15:13:27 +0100, Kieran Bingham wrote: > 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. > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> With Laurent's comment about documenting the new behavior of streamOff() addressed, Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > > Since RFC: > - Rebased to master. > > src/libcamera/include/v4l2_videodevice.h | 2 ++ > src/libcamera/v4l2_videodevice.cpp | 10 +++++++++- > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h > index 4b8cf9394eb9..caf36bf9c1aa 100644 > --- a/src/libcamera/include/v4l2_videodevice.h > +++ b/src/libcamera/include/v4l2_videodevice.h > @@ -187,6 +187,8 @@ private: > std::map<unsigned int, Buffer *> queuedBuffers_; > > EventNotifier *fdEvent_; > + > + bool streaming_; > }; > > class V4L2M2MDevice > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index 208ab54199b1..c0b3a79f76da 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -272,7 +272,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), bufferPool_(nullptr), fdEvent_(nullptr) > + : V4L2Device(deviceNode), bufferPool_(nullptr), fdEvent_(nullptr), > + streaming_(false) > { > /* > * We default to an MMAP based CAPTURE video device, however this will > @@ -1173,6 +1174,8 @@ int V4L2VideoDevice::streamOn() > return ret; > } > > + streaming_ = true; > + > return 0; > } > > @@ -1190,6 +1193,9 @@ int V4L2VideoDevice::streamOff() > { > int ret; > > + if (!streaming_) > + return 0; > + > ret = ioctl(VIDIOC_STREAMOFF, &bufferType_); > if (ret < 0) { > LOG(V4L2, Error) > @@ -1210,6 +1216,8 @@ int V4L2VideoDevice::streamOff() > queuedBuffers_.clear(); > fdEvent_->setEnabled(false); > > + streaming_ = false; > + > return 0; > } > > -- > 2.20.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 4b8cf9394eb9..caf36bf9c1aa 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -187,6 +187,8 @@ private: std::map<unsigned int, Buffer *> queuedBuffers_; EventNotifier *fdEvent_; + + bool streaming_; }; class V4L2M2MDevice diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 208ab54199b1..c0b3a79f76da 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -272,7 +272,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), bufferPool_(nullptr), fdEvent_(nullptr) + : V4L2Device(deviceNode), bufferPool_(nullptr), fdEvent_(nullptr), + streaming_(false) { /* * We default to an MMAP based CAPTURE video device, however this will @@ -1173,6 +1174,8 @@ int V4L2VideoDevice::streamOn() return ret; } + streaming_ = true; + return 0; } @@ -1190,6 +1193,9 @@ int V4L2VideoDevice::streamOff() { int ret; + if (!streaming_) + return 0; + ret = ioctl(VIDIOC_STREAMOFF, &bufferType_); if (ret < 0) { LOG(V4L2, Error) @@ -1210,6 +1216,8 @@ int V4L2VideoDevice::streamOff() queuedBuffers_.clear(); fdEvent_->setEnabled(false); + streaming_ = false; + return 0; }