From 3d624b745b31383dbcd94d96246fab865820085f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Niklas=20S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>
Date: Sun, 18 Oct 2020 03:47:23 +0200
Subject: libcamera: v4l2_device: Move start of frame detection to V4L2Device
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The V4L2_EVENT_FRAME_SYNC event may occur on both V4L2 video-devices
(V4L2VideoDevice) and sub-devices (V4L2Subdevice). Move the start of
frame detection to the common base class of the two, V4L2Device.

There is no functional change.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/libcamera/internal/v4l2_device.h      | 12 +++++
 include/libcamera/internal/v4l2_videodevice.h |  8 ---
 src/libcamera/v4l2_device.cpp                 | 76 ++++++++++++++++++++++++++-
 src/libcamera/v4l2_videodevice.cpp            | 75 +-------------------------
 4 files changed, 87 insertions(+), 84 deletions(-)

diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
index 722fb722..c56a950e 100644
--- a/include/libcamera/internal/v4l2_device.h
+++ b/include/libcamera/internal/v4l2_device.h
@@ -13,11 +13,15 @@
 
 #include <linux/videodev2.h>
 
+#include <libcamera/signal.h>
+
 #include "libcamera/internal/log.h"
 #include "libcamera/internal/v4l2_controls.h"
 
 namespace libcamera {
 
+class EventNotifier;
+
 class V4L2Device : protected Loggable
 {
 public:
@@ -34,6 +38,9 @@ public:
 	const std::string &deviceNode() const { return deviceNode_; }
 	std::string devicePath() const;
 
+	int setFrameStartEnabled(bool enable);
+	Signal<uint32_t> frameStart;
+
 protected:
 	V4L2Device(const std::string &deviceNode);
 	~V4L2Device();
@@ -51,11 +58,16 @@ private:
 			    const struct v4l2_ext_control *v4l2Ctrls,
 			    unsigned int count);
 
+	void eventAvailable(EventNotifier *notifier);
+
 	std::map<unsigned int, struct v4l2_query_ext_ctrl> controlInfo_;
 	std::vector<std::unique_ptr<V4L2ControlId>> controlIds_;
 	ControlInfoMap controls_;
 	std::string deviceNode_;
 	int fd_;
+
+	EventNotifier *fdEventNotifier_;
+	bool frameStartEnabled_;
 };
 
 } /* namespace libcamera */
diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
index dcb9654a..661503d1 100644
--- a/include/libcamera/internal/v4l2_videodevice.h
+++ b/include/libcamera/internal/v4l2_videodevice.h
@@ -207,9 +207,6 @@ public:
 	int queueBuffer(FrameBuffer *buffer);
 	Signal<FrameBuffer *> bufferReady;
 
-	int setFrameStartEnabled(bool enable);
-	Signal<uint32_t> frameStart;
-
 	int streamOn();
 	int streamOff();
 
@@ -243,8 +240,6 @@ private:
 	void bufferAvailable(EventNotifier *notifier);
 	FrameBuffer *dequeueBuffer();
 
-	void eventAvailable(EventNotifier *notifier);
-
 	V4L2Capability caps_;
 
 	enum v4l2_buf_type bufferType_;
@@ -254,9 +249,6 @@ private:
 	std::map<unsigned int, FrameBuffer *> queuedBuffers_;
 
 	EventNotifier *fdBufferNotifier_;
-	EventNotifier *fdEventNotifier_;
-
-	bool frameStartEnabled_;
 };
 
 class V4L2M2MDevice
diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
index f747ba79..fd0b140f 100644
--- a/src/libcamera/v4l2_device.cpp
+++ b/src/libcamera/v4l2_device.cpp
@@ -16,6 +16,8 @@
 #include <sys/syscall.h>
 #include <unistd.h>
 
+#include <libcamera/event_notifier.h>
+
 #include "libcamera/internal/log.h"
 #include "libcamera/internal/sysfs.h"
 #include "libcamera/internal/utils.h"
@@ -52,7 +54,8 @@ LOG_DEFINE_CATEGORY(V4L2)
  * at open() time, and the \a logTag to prefix log messages with.
  */
 V4L2Device::V4L2Device(const std::string &deviceNode)
-	: deviceNode_(deviceNode), fd_(-1)
+	: deviceNode_(deviceNode), fd_(-1), fdEventNotifier_(nullptr),
+	  frameStartEnabled_(false)
 {
 }
 
@@ -87,7 +90,7 @@ int V4L2Device::open(unsigned int flags)
 		return ret;
 	}
 
-	fd_ = ret;
+	setFd(ret);
 
 	listControls();
 
@@ -117,6 +120,10 @@ int V4L2Device::setFd(int fd)
 
 	fd_ = fd;
 
+	fdEventNotifier_ = new EventNotifier(fd_, EventNotifier::Exception);
+	fdEventNotifier_->activated.connect(this, &V4L2Device::eventAvailable);
+	fdEventNotifier_->setEnabled(false);
+
 	return 0;
 }
 
@@ -130,6 +137,8 @@ void V4L2Device::close()
 	if (!isOpen())
 		return;
 
+	delete fdEventNotifier_;
+
 	if (::close(fd_) < 0)
 		LOG(V4L2, Error) << "Failed to close V4L2 device: "
 				 << strerror(errno);
@@ -395,6 +404,40 @@ std::string V4L2Device::devicePath() const
 	return path;
 }
 
+/**
+ * \brief Enable or disable frame start event notification
+ * \param[in] enable True to enable frame start events, false to disable them
+ *
+ * This function enables or disables generation of frame start events. Once
+ * enabled, the events are signalled through the frameStart signal.
+ *
+ * \return 0 on success, a negative error code otherwise
+ */
+int V4L2Device::setFrameStartEnabled(bool enable)
+{
+	if (frameStartEnabled_ == enable)
+		return 0;
+
+	struct v4l2_event_subscription event{};
+	event.type = V4L2_EVENT_FRAME_SYNC;
+
+	unsigned long request = enable ? VIDIOC_SUBSCRIBE_EVENT
+			      : VIDIOC_UNSUBSCRIBE_EVENT;
+	int ret = ioctl(request, &event);
+	if (enable && ret)
+		return ret;
+
+	fdEventNotifier_->setEnabled(enable);
+	frameStartEnabled_ = enable;
+
+	return ret;
+}
+
+/**
+ * \var V4L2Device::frameStart
+ * \brief A Signal emitted when capture of a frame has started
+ */
+
 /**
  * \brief Perform an IOCTL system call on the device node
  * \param[in] request The IOCTL request code
@@ -518,4 +561,33 @@ void V4L2Device::updateControls(ControlList *ctrls,
 	}
 }
 
+/**
+ * \brief Slot to handle V4L2 events from the V4L2 device
+ * \param[in] notifier The event notifier
+ *
+ * When this slot is called, a V4L2 event is available to be dequeued from the
+ * device.
+ */
+void V4L2Device::eventAvailable([[maybe_unused]] EventNotifier *notifier)
+{
+	struct v4l2_event event{};
+	int ret = ioctl(VIDIOC_DQEVENT, &event);
+	if (ret < 0) {
+		LOG(V4L2, Error)
+			<< "Failed to dequeue event, disabling event notifier";
+		fdEventNotifier_->setEnabled(false);
+		return;
+	}
+
+	if (event.type != V4L2_EVENT_FRAME_SYNC) {
+		LOG(V4L2, Error)
+			<< "Spurious event (" << event.type
+			<< "), disabling event notifier";
+		fdEventNotifier_->setEnabled(false);
+		return;
+	}
+
+	frameStart.emit(event.u.frame_sync.frame_sequence);
+}
+
 } /* namespace libcamera */
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index ac3c8c7d..5ee1b479 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -481,8 +481,7 @@ 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),
-	  fdEventNotifier_(nullptr), frameStartEnabled_(false)
+	: V4L2Device(deviceNode), cache_(nullptr), fdBufferNotifier_(nullptr)
 {
 	/*
 	 * We default to an MMAP based CAPTURE video device, however this will
@@ -575,10 +574,6 @@ int V4L2VideoDevice::open()
 	fdBufferNotifier_->activated.connect(this, &V4L2VideoDevice::bufferAvailable);
 	fdBufferNotifier_->setEnabled(false);
 
-	fdEventNotifier_ = new EventNotifier(fd(), EventNotifier::Exception);
-	fdEventNotifier_->activated.connect(this, &V4L2VideoDevice::eventAvailable);
-	fdEventNotifier_->setEnabled(false);
-
 	LOG(V4L2, Debug)
 		<< "Opened device " << caps_.bus_info() << ": "
 		<< caps_.driver() << ": " << caps_.card();
@@ -668,10 +663,6 @@ int V4L2VideoDevice::open(int handle, enum v4l2_buf_type type)
 	fdBufferNotifier_->activated.connect(this, &V4L2VideoDevice::bufferAvailable);
 	fdBufferNotifier_->setEnabled(false);
 
-	fdEventNotifier_ = new EventNotifier(fd(), EventNotifier::Exception);
-	fdEventNotifier_->activated.connect(this, &V4L2VideoDevice::eventAvailable);
-	fdEventNotifier_->setEnabled(false);
-
 	LOG(V4L2, Debug)
 		<< "Opened device " << caps_.bus_info() << ": "
 		<< caps_.driver() << ": " << caps_.card();
@@ -689,7 +680,6 @@ void V4L2VideoDevice::close()
 
 	releaseBuffers();
 	delete fdBufferNotifier_;
-	delete fdEventNotifier_;
 
 	V4L2Device::close();
 }
@@ -1544,74 +1534,11 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()
 	return buffer;
 }
 
-/**
- * \brief Slot to handle V4L2 events from the V4L2 video device
- * \param[in] notifier The event notifier
- *
- * When this slot is called, a V4L2 event is available to be dequeued from the
- * device.
- */
-void V4L2VideoDevice::eventAvailable([[maybe_unused]] EventNotifier *notifier)
-{
-	struct v4l2_event event{};
-	int ret = ioctl(VIDIOC_DQEVENT, &event);
-	if (ret < 0) {
-		LOG(V4L2, Error)
-			<< "Failed to dequeue event, disabling event notifier";
-		fdEventNotifier_->setEnabled(false);
-		return;
-	}
-
-	if (event.type != V4L2_EVENT_FRAME_SYNC) {
-		LOG(V4L2, Error)
-			<< "Spurious event (" << event.type
-			<< "), disabling event notifier";
-		fdEventNotifier_->setEnabled(false);
-		return;
-	}
-
-	frameStart.emit(event.u.frame_sync.frame_sequence);
-}
-
 /**
  * \var V4L2VideoDevice::bufferReady
  * \brief A Signal emitted when a framebuffer completes
  */
 
-/**
- * \brief Enable or disable frame start event notification
- * \param[in] enable True to enable frame start events, false to disable them
- *
- * This function enables or disables generation of frame start events. Once
- * enabled, the events are signalled through the frameStart signal.
- *
- * \return 0 on success, a negative error code otherwise
- */
-int V4L2VideoDevice::setFrameStartEnabled(bool enable)
-{
-	if (frameStartEnabled_ == enable)
-		return 0;
-
-	struct v4l2_event_subscription event{};
-	event.type = V4L2_EVENT_FRAME_SYNC;
-
-	unsigned long request = enable ? VIDIOC_SUBSCRIBE_EVENT
-			      : VIDIOC_UNSUBSCRIBE_EVENT;
-	int ret = ioctl(request, &event);
-	if (enable && ret)
-		return ret;
-
-	fdEventNotifier_->setEnabled(enable);
-	frameStartEnabled_ = enable;
-
-	return ret;
-}
-
-/**
- * \var V4L2VideoDevice::frameStart
- * \brief A Signal emitted when capture of a frame has started
- */
-
 /**
  * \brief Start the video stream
  * \return 0 on success or a negative error code otherwise
-- 
cgit v1.2.1