| Message ID | 20190809150459.14421-4-kieran.bingham@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Fri, Aug 09, 2019 at 04:04:56PM +0100, Kieran Bingham wrote: > V4L2 M2M devices represent a V4L2Device with two queues: One output, and > one capture on a single device node. > > Represent this by instantiating a V4L2VideoDevice for each queue type, > and preparing each device for its queue. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/include/v4l2_videodevice.h | 28 ++++ > src/libcamera/v4l2_videodevice.cpp | 186 +++++++++++++++++++++++ > 2 files changed, 214 insertions(+) > > diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h > index f5c8da93fcb5..72dc8c63e4bb 100644 > --- a/src/libcamera/include/v4l2_videodevice.h > +++ b/src/libcamera/include/v4l2_videodevice.h > @@ -71,6 +71,11 @@ struct V4L2Capability final : v4l2_capability { > V4L2_CAP_VIDEO_OUTPUT | > V4L2_CAP_VIDEO_OUTPUT_MPLANE); > } > + bool isM2M() const > + { > + return device_caps() & (V4L2_CAP_VIDEO_M2M | > + V4L2_CAP_VIDEO_M2M_MPLANE); > + } > bool isMeta() const > { > return device_caps() & (V4L2_CAP_META_CAPTURE | > @@ -124,6 +129,7 @@ public: > V4L2VideoDevice &operator=(const V4L2VideoDevice &) = delete; > > int open(); > + int open(int handle, enum v4l2_buf_type type); > void close(); > > const char *driverName() const { return caps_.driver(); } > @@ -152,6 +158,9 @@ protected: > std::string logPrefix() const; > > private: > + int queryBufferType(); > + int queryBufferType(enum v4l2_buf_type type); I think these are leftovers. > + > int getFormatMeta(V4L2DeviceFormat *format); > int setFormatMeta(V4L2DeviceFormat *format); > > @@ -182,6 +191,25 @@ private: > EventNotifier *fdEvent_; > }; > > +class V4L2M2MDevice > +{ > +public: > + V4L2M2MDevice(const std::string &deviceNode); > + ~V4L2M2MDevice(); > + > + int open(); > + void close(); > + > + V4L2VideoDevice *output() { return output_; } > + V4L2VideoDevice *capture() { return capture_; } > + > +private: > + std::string deviceNode_; > + > + V4L2VideoDevice *output_; > + V4L2VideoDevice *capture_; > +}; > + > } /* namespace libcamera */ > > #endif /* __LIBCAMERA_V4L2_VIDEODEVICE_H__ */ > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index 81098dd70190..9c5638995577 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -89,6 +89,12 @@ LOG_DECLARE_CATEGORY(V4L2) > * \return True if the video device can capture or output images > */ > > +/** > + * \fn V4L2Capability::isM2M() > + * \brief Identify if the device is an M2M device > + * \return True if the device can capture and output images using the M2M API > + */ > + > /** > * \fn V4L2Capability::isMeta() > * \brief Identify if the video device captures or outputs image meta-data > @@ -296,6 +302,10 @@ V4L2VideoDevice::~V4L2VideoDevice() > > /** > * \brief Open a V4L2 video device and query its capabilities > + * > + * Opens a video device, then queries the capabilities of the device and s/Opens/This method opens/ > + * establishes the buffer types and device events accordingly. The last part is internal matters I think. I would just drop this paragraph, and update the brief to state "Open a V4L2 video device node and query its capabilities". > + * > * \return 0 on success or a negative error code otherwise > */ > int V4L2VideoDevice::open() > @@ -355,6 +365,83 @@ int V4L2VideoDevice::open() > return 0; > } > > +/** > + * \brief Open a V4L2 video device and query its capabilities And here "Open a V4L2 video device from an opened file handle and query its capabilities". > + * No need for this blank line. > + * \param[in] handle The file descriptor to set s/handle/fd/ ? > + * \param[in] type The device type to operate on > + * > + * Sets the file descriptor for the device and then queries the capabilities of s/Sets/This method sets/ You need a subject before a conjugated verb. > + * the device and establishes the buffer types and device events accordingly. * This methods opens a video device from the existing file descriptor \a fd. * Like open() queries the capabilities of the device, but handles it according * to the given device \a type instead of determining its type from the * capabilities. This can be used to force a given device type for M2M devices. * * The file descriptor \a fd is duplicated, and the caller shall close \a fd * when it has no further use for it. The close() method will close the * duplicated file descriptor, leaving \a fd untouched. > + * > + * \return 0 on success or a negative error code otherwise > + */ > +int V4L2VideoDevice::open(int handle, enum v4l2_buf_type type) > +{ > + int ret; > + int newFd; > + > + newFd = dup(handle); > + if (newFd < 0) { > + ret = -errno; > + LOG(V4L2, Error) << "Failed to duplicate file handle: " > + << strerror(-ret); > + return ret; > + } > + > + ret = V4L2Device::setFd(newFd); > + if (ret < 0) { > + LOG(V4L2, Error) << "Failed to set file handle: " > + << strerror(-ret); You need to ::close(newFd) here. > + return ret; > + } > + > + ret = ioctl(VIDIOC_QUERYCAP, &caps_); > + if (ret < 0) { > + LOG(V4L2, Error) > + << "Failed to query device capabilities: " > + << strerror(-ret); > + return ret; > + } > + > + if (!caps_.hasStreaming()) { > + LOG(V4L2, Error) << "Device does not support streaming I/O"; > + return -EINVAL; > + } > + > + /* > + * Set buffer type and wait for read notifications on CAPTURE video > + * devices (POLLIN), and write notifications for OUTPUT video devices > + * (POLLOUT). > + */ > + switch (type) { > + case V4L2_BUF_TYPE_VIDEO_OUTPUT: > + fdEvent_ = new EventNotifier(fd(), EventNotifier::Write); > + bufferType_ = caps_.isMultiplanar() > + ? V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE > + : V4L2_BUF_TYPE_VIDEO_OUTPUT; > + break; > + case V4L2_BUF_TYPE_VIDEO_CAPTURE: > + fdEvent_ = new EventNotifier(fd(), EventNotifier::Read); > + bufferType_ = caps_.isMultiplanar() > + ? V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE > + : V4L2_BUF_TYPE_VIDEO_CAPTURE; > + break; > + default: > + LOG(V4L2, Error) << "Unsupported buffer type"; > + return -EINVAL; > + } > + > + fdEvent_->activated.connect(this, &V4L2VideoDevice::bufferAvailable); > + fdEvent_->setEnabled(false); > + > + LOG(V4L2, Debug) > + << "Opened device " << caps_.bus_info() << ": " > + << caps_.driver() << ": " << caps_.card(); > + > + return 0; > +} > + > /** > * \brief Close the video device, releasing any resources acquired by open() > */ > @@ -1143,4 +1230,103 @@ V4L2VideoDevice *V4L2VideoDevice::fromEntityName(const MediaDevice *media, > return new V4L2VideoDevice(mediaEntity); > } > > +/** > + * \class V4L2M2MDevice > + * \brief Memory2Memory video device > + * > + * The V4L2M2MDevice manages two V4L2VideoDevice instances on the same > + * deviceNode which operate together using two queues to implement the V4L2 > + * Memory to Memory API. > + * > + * The two devices should be opened by calling open() on the V4L2M2MDevice, and > + * can be closed by calling close on the V4L2M2MDevice. > + * > + * Calling V4L2VideoDevice::open() and V4L2VideoDevice::close() on the capture > + * or output V4L2VideoDevice is not permitted. > + */ > + > +/** > + * \fn V4L2M2MDevice::output > + * \brief Provide access to the output V4L2VideoDevice instance s/Provide access to/Retrieve/ > + * \return The output V4L2Device instance > + */ > + > +/** > + * \fn V4L2M2MDevice::capture > + * \brief Provide access to the capture V4L2VideoDevice instance s/Provide access to/Retrieve/ Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + * \return The capture V4L2Device instance > + */ > + > +/** > + * \brief Create a new V4L2M2MDevice from the \a deviceNode > + */ > +V4L2M2MDevice::V4L2M2MDevice(const std::string &deviceNode) > + : deviceNode_(deviceNode) > +{ > + output_ = new V4L2VideoDevice(deviceNode); > + capture_ = new V4L2VideoDevice(deviceNode); > +} > + > +V4L2M2MDevice::~V4L2M2MDevice() > +{ > + delete capture_; > + delete output_; > +} > + > +/** > + * \brief Open a V4L2 Memory to Memory device > + * > + * Open the device node and prepare the two V4L2VideoDevice instances to handle > + * their respective buffer queues. > + * > + * \return 0 on success or a negative error code otherwise > + */ > +int V4L2M2MDevice::open() > +{ > + int fd; > + int ret; > + > + /* > + * The output and capture V4L2VideoDevice instances use the same file > + * handle for the same device node. The local file handle can be closed > + * as the V4L2VideoDevice::open() retains a handle by duplicating the > + * fd passed in. > + */ > + fd = ::open(deviceNode_.c_str(), O_RDWR | O_NONBLOCK); > + if (fd < 0) { > + ret = -errno; > + LOG(V4L2, Error) > + << "Failed to open V4L2 M2M device: " << strerror(-ret); > + return ret; > + } > + > + ret = output_->open(fd, V4L2_BUF_TYPE_VIDEO_OUTPUT); > + if (ret) > + goto err; > + > + ret = capture_->open(fd, V4L2_BUF_TYPE_VIDEO_CAPTURE); > + if (ret) > + goto err; > + > + ::close(fd); > + > + return 0; > + > +err: > + close(); > + ::close(fd); > + > + return ret; > +} > + > +/** > + * \brief Close the memory-to-memory device, releasing any resources acquired by > + * open() > + */ > +void V4L2M2MDevice::close() > +{ > + capture_->close(); > + output_->close(); > +} > + > } /* namespace libcamera */
Hi Laurent, On 10/08/2019 14:41, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Fri, Aug 09, 2019 at 04:04:56PM +0100, Kieran Bingham wrote: >> V4L2 M2M devices represent a V4L2Device with two queues: One output, and >> one capture on a single device node. >> >> Represent this by instantiating a V4L2VideoDevice for each queue type, >> and preparing each device for its queue. >> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> src/libcamera/include/v4l2_videodevice.h | 28 ++++ >> src/libcamera/v4l2_videodevice.cpp | 186 +++++++++++++++++++++++ >> 2 files changed, 214 insertions(+) >> >> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h >> index f5c8da93fcb5..72dc8c63e4bb 100644 >> --- a/src/libcamera/include/v4l2_videodevice.h >> +++ b/src/libcamera/include/v4l2_videodevice.h >> @@ -71,6 +71,11 @@ struct V4L2Capability final : v4l2_capability { >> V4L2_CAP_VIDEO_OUTPUT | >> V4L2_CAP_VIDEO_OUTPUT_MPLANE); >> } >> + bool isM2M() const >> + { >> + return device_caps() & (V4L2_CAP_VIDEO_M2M | >> + V4L2_CAP_VIDEO_M2M_MPLANE); >> + } >> bool isMeta() const >> { >> return device_caps() & (V4L2_CAP_META_CAPTURE | >> @@ -124,6 +129,7 @@ public: >> V4L2VideoDevice &operator=(const V4L2VideoDevice &) = delete; >> >> int open(); >> + int open(int handle, enum v4l2_buf_type type); >> void close(); >> >> const char *driverName() const { return caps_.driver(); } >> @@ -152,6 +158,9 @@ protected: >> std::string logPrefix() const; >> >> private: >> + int queryBufferType(); >> + int queryBufferType(enum v4l2_buf_type type); > > I think these are leftovers. Yes, removed. >> + >> int getFormatMeta(V4L2DeviceFormat *format); >> int setFormatMeta(V4L2DeviceFormat *format); >> >> @@ -182,6 +191,25 @@ private: >> EventNotifier *fdEvent_; >> }; >> >> +class V4L2M2MDevice >> +{ >> +public: >> + V4L2M2MDevice(const std::string &deviceNode); >> + ~V4L2M2MDevice(); >> + >> + int open(); >> + void close(); >> + >> + V4L2VideoDevice *output() { return output_; } >> + V4L2VideoDevice *capture() { return capture_; } >> + >> +private: >> + std::string deviceNode_; >> + >> + V4L2VideoDevice *output_; >> + V4L2VideoDevice *capture_; >> +}; >> + >> } /* namespace libcamera */ >> >> #endif /* __LIBCAMERA_V4L2_VIDEODEVICE_H__ */ >> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp >> index 81098dd70190..9c5638995577 100644 >> --- a/src/libcamera/v4l2_videodevice.cpp >> +++ b/src/libcamera/v4l2_videodevice.cpp >> @@ -89,6 +89,12 @@ LOG_DECLARE_CATEGORY(V4L2) >> * \return True if the video device can capture or output images >> */ >> >> +/** >> + * \fn V4L2Capability::isM2M() >> + * \brief Identify if the device is an M2M device >> + * \return True if the device can capture and output images using the M2M API >> + */ >> + >> /** >> * \fn V4L2Capability::isMeta() >> * \brief Identify if the video device captures or outputs image meta-data >> @@ -296,6 +302,10 @@ V4L2VideoDevice::~V4L2VideoDevice() >> >> /** >> * \brief Open a V4L2 video device and query its capabilities >> + * >> + * Opens a video device, then queries the capabilities of the device and > > s/Opens/This method opens/ > >> + * establishes the buffer types and device events accordingly. > > The last part is internal matters I think. I would just drop this > paragraph, and update the brief to state "Open a V4L2 video device node > and query its capabilities". You mean simply add the word 'node' to the existing brief? The 'a' doesn't give enough context in that case any more (or perhaps before), because it only opens 'the' device node that exists in this class instance. It doesn't open any device node. Only the one which was previously set in the constructor. I'll change it to : "open /the/ V4L2 video device node and ..." >> + * >> * \return 0 on success or a negative error code otherwise >> */ >> int V4L2VideoDevice::open() >> @@ -355,6 +365,83 @@ int V4L2VideoDevice::open() >> return 0; >> } >> >> +/** >> + * \brief Open a V4L2 video device and query its capabilities > > And here "Open a V4L2 video device from an opened file handle and query > its capabilities". Changed. > >> + * > > No need for this blank line. > >> + * \param[in] handle The file descriptor to set > > s/handle/fd/ ? No,. >> + * \param[in] type The device type to operate on >> + * >> + * Sets the file descriptor for the device and then queries the capabilities of > > s/Sets/This method sets/ > > You need a subject before a conjugated verb. We're /in/ the subject. It can be implicit. >> + * the device and establishes the buffer types and device events accordingly. > > * This methods opens a video device from the existing file descriptor \a fd. We can't use fd as an argument, because we use fd(). We can't use newFd, because that's the result of the dup(). That's why I've used handle. > * Like open() queries the capabilities of the device, but handles it according ^ Isn't this the same issue? > * to the given device \a type instead of determining its type from the > * capabilities. This can be used to force a given device type for M2M devices. > * > * The file descriptor \a fd is duplicated, and the caller shall close \a fd > * when it has no further use for it. The close() method will close the > * duplicated file descriptor, leaving \a fd untouched. Changed to: * This methods opens a video device from the existing file descriptor \a * handle. Like open(), this method queries the capabilities of the device, but * handles it according to the given device \a type instead of determining its * type from the capabilities. This can be used to force a given device type for * M2M devices. * * The file descriptor \a handle is duplicated, and the caller is responsible * for closing the \a handle when it has no further use for it. The close() * method will close the duplicated file descriptor, leaving \a handle * untouched. >> + * >> + * \return 0 on success or a negative error code otherwise >> + */ >> +int V4L2VideoDevice::open(int handle, enum v4l2_buf_type type) >> +{ >> + int ret; >> + int newFd; >> + >> + newFd = dup(handle); >> + if (newFd < 0) { >> + ret = -errno; >> + LOG(V4L2, Error) << "Failed to duplicate file handle: " >> + << strerror(-ret); >> + return ret; >> + } >> + >> + ret = V4L2Device::setFd(newFd); >> + if (ret < 0) { >> + LOG(V4L2, Error) << "Failed to set file handle: " >> + << strerror(-ret); > > You need to ::close(newFd) here. > Ah yes. >> + return ret; >> + } >> + >> + ret = ioctl(VIDIOC_QUERYCAP, &caps_); >> + if (ret < 0) { >> + LOG(V4L2, Error) >> + << "Failed to query device capabilities: " >> + << strerror(-ret); >> + return ret; >> + } >> + >> + if (!caps_.hasStreaming()) { >> + LOG(V4L2, Error) << "Device does not support streaming I/O"; >> + return -EINVAL; >> + } >> + >> + /* >> + * Set buffer type and wait for read notifications on CAPTURE video >> + * devices (POLLIN), and write notifications for OUTPUT video devices >> + * (POLLOUT). >> + */ >> + switch (type) { >> + case V4L2_BUF_TYPE_VIDEO_OUTPUT: >> + fdEvent_ = new EventNotifier(fd(), EventNotifier::Write); >> + bufferType_ = caps_.isMultiplanar() >> + ? V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE >> + : V4L2_BUF_TYPE_VIDEO_OUTPUT; >> + break; >> + case V4L2_BUF_TYPE_VIDEO_CAPTURE: >> + fdEvent_ = new EventNotifier(fd(), EventNotifier::Read); >> + bufferType_ = caps_.isMultiplanar() >> + ? V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE >> + : V4L2_BUF_TYPE_VIDEO_CAPTURE; >> + break; >> + default: >> + LOG(V4L2, Error) << "Unsupported buffer type"; >> + return -EINVAL; >> + } >> + >> + fdEvent_->activated.connect(this, &V4L2VideoDevice::bufferAvailable); >> + fdEvent_->setEnabled(false); >> + >> + LOG(V4L2, Debug) >> + << "Opened device " << caps_.bus_info() << ": " >> + << caps_.driver() << ": " << caps_.card(); >> + >> + return 0; >> +} >> + >> /** >> * \brief Close the video device, releasing any resources acquired by open() >> */ >> @@ -1143,4 +1230,103 @@ V4L2VideoDevice *V4L2VideoDevice::fromEntityName(const MediaDevice *media, >> return new V4L2VideoDevice(mediaEntity); >> } >> >> +/** >> + * \class V4L2M2MDevice >> + * \brief Memory2Memory video device >> + * >> + * The V4L2M2MDevice manages two V4L2VideoDevice instances on the same >> + * deviceNode which operate together using two queues to implement the V4L2 >> + * Memory to Memory API. >> + * >> + * The two devices should be opened by calling open() on the V4L2M2MDevice, and >> + * can be closed by calling close on the V4L2M2MDevice. >> + * >> + * Calling V4L2VideoDevice::open() and V4L2VideoDevice::close() on the capture >> + * or output V4L2VideoDevice is not permitted. >> + */ >> + >> +/** >> + * \fn V4L2M2MDevice::output >> + * \brief Provide access to the output V4L2VideoDevice instance > > s/Provide access to/Retrieve/ Changed. >> + * \return The output V4L2Device instance >> + */ >> + >> +/** >> + * \fn V4L2M2MDevice::capture >> + * \brief Provide access to the capture V4L2VideoDevice instance > > s/Provide access to/Retrieve/ Changed. > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks, >> + * \return The capture V4L2Device instance >> + */ >> + >> +/** >> + * \brief Create a new V4L2M2MDevice from the \a deviceNode >> + */ >> +V4L2M2MDevice::V4L2M2MDevice(const std::string &deviceNode) >> + : deviceNode_(deviceNode) >> +{ >> + output_ = new V4L2VideoDevice(deviceNode); >> + capture_ = new V4L2VideoDevice(deviceNode); >> +} >> + >> +V4L2M2MDevice::~V4L2M2MDevice() >> +{ >> + delete capture_; >> + delete output_; >> +} >> + >> +/** >> + * \brief Open a V4L2 Memory to Memory device >> + * >> + * Open the device node and prepare the two V4L2VideoDevice instances to handle >> + * their respective buffer queues. >> + * >> + * \return 0 on success or a negative error code otherwise >> + */ >> +int V4L2M2MDevice::open() >> +{ >> + int fd; >> + int ret; >> + >> + /* >> + * The output and capture V4L2VideoDevice instances use the same file >> + * handle for the same device node. The local file handle can be closed >> + * as the V4L2VideoDevice::open() retains a handle by duplicating the >> + * fd passed in. >> + */ >> + fd = ::open(deviceNode_.c_str(), O_RDWR | O_NONBLOCK); >> + if (fd < 0) { >> + ret = -errno; >> + LOG(V4L2, Error) >> + << "Failed to open V4L2 M2M device: " << strerror(-ret); >> + return ret; >> + } >> + >> + ret = output_->open(fd, V4L2_BUF_TYPE_VIDEO_OUTPUT); >> + if (ret) >> + goto err; >> + >> + ret = capture_->open(fd, V4L2_BUF_TYPE_VIDEO_CAPTURE); >> + if (ret) >> + goto err; >> + >> + ::close(fd); >> + >> + return 0; >> + >> +err: >> + close(); >> + ::close(fd); >> + >> + return ret; >> +} >> + >> +/** >> + * \brief Close the memory-to-memory device, releasing any resources acquired by >> + * open() >> + */ >> +void V4L2M2MDevice::close() >> +{ >> + capture_->close(); >> + output_->close(); >> +} >> + >> } /* namespace libcamera */ >
Hi Kieran, On Mon, Aug 12, 2019 at 10:01:53AM +0100, Kieran Bingham wrote: > On 10/08/2019 14:41, Laurent Pinchart wrote: > > On Fri, Aug 09, 2019 at 04:04:56PM +0100, Kieran Bingham wrote: > >> V4L2 M2M devices represent a V4L2Device with two queues: One output, and > >> one capture on a single device node. > >> > >> Represent this by instantiating a V4L2VideoDevice for each queue type, > >> and preparing each device for its queue. > >> > >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > >> --- > >> src/libcamera/include/v4l2_videodevice.h | 28 ++++ > >> src/libcamera/v4l2_videodevice.cpp | 186 +++++++++++++++++++++++ > >> 2 files changed, 214 insertions(+) > >> > >> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h > >> index f5c8da93fcb5..72dc8c63e4bb 100644 > >> --- a/src/libcamera/include/v4l2_videodevice.h > >> +++ b/src/libcamera/include/v4l2_videodevice.h > >> @@ -71,6 +71,11 @@ struct V4L2Capability final : v4l2_capability { > >> V4L2_CAP_VIDEO_OUTPUT | > >> V4L2_CAP_VIDEO_OUTPUT_MPLANE); > >> } > >> + bool isM2M() const > >> + { > >> + return device_caps() & (V4L2_CAP_VIDEO_M2M | > >> + V4L2_CAP_VIDEO_M2M_MPLANE); > >> + } > >> bool isMeta() const > >> { > >> return device_caps() & (V4L2_CAP_META_CAPTURE | > >> @@ -124,6 +129,7 @@ public: > >> V4L2VideoDevice &operator=(const V4L2VideoDevice &) = delete; > >> > >> int open(); > >> + int open(int handle, enum v4l2_buf_type type); > >> void close(); > >> > >> const char *driverName() const { return caps_.driver(); } > >> @@ -152,6 +158,9 @@ protected: > >> std::string logPrefix() const; > >> > >> private: > >> + int queryBufferType(); > >> + int queryBufferType(enum v4l2_buf_type type); > > > > I think these are leftovers. > > Yes, removed. > > >> + > >> int getFormatMeta(V4L2DeviceFormat *format); > >> int setFormatMeta(V4L2DeviceFormat *format); > >> > >> @@ -182,6 +191,25 @@ private: > >> EventNotifier *fdEvent_; > >> }; > >> > >> +class V4L2M2MDevice > >> +{ > >> +public: > >> + V4L2M2MDevice(const std::string &deviceNode); > >> + ~V4L2M2MDevice(); > >> + > >> + int open(); > >> + void close(); > >> + > >> + V4L2VideoDevice *output() { return output_; } > >> + V4L2VideoDevice *capture() { return capture_; } > >> + > >> +private: > >> + std::string deviceNode_; > >> + > >> + V4L2VideoDevice *output_; > >> + V4L2VideoDevice *capture_; > >> +}; > >> + > >> } /* namespace libcamera */ > >> > >> #endif /* __LIBCAMERA_V4L2_VIDEODEVICE_H__ */ > >> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > >> index 81098dd70190..9c5638995577 100644 > >> --- a/src/libcamera/v4l2_videodevice.cpp > >> +++ b/src/libcamera/v4l2_videodevice.cpp > >> @@ -89,6 +89,12 @@ LOG_DECLARE_CATEGORY(V4L2) > >> * \return True if the video device can capture or output images > >> */ > >> > >> +/** > >> + * \fn V4L2Capability::isM2M() > >> + * \brief Identify if the device is an M2M device > >> + * \return True if the device can capture and output images using the M2M API > >> + */ > >> + > >> /** > >> * \fn V4L2Capability::isMeta() > >> * \brief Identify if the video device captures or outputs image meta-data > >> @@ -296,6 +302,10 @@ V4L2VideoDevice::~V4L2VideoDevice() > >> > >> /** > >> * \brief Open a V4L2 video device and query its capabilities > >> + * > >> + * Opens a video device, then queries the capabilities of the device and > > > > s/Opens/This method opens/ > > > >> + * establishes the buffer types and device events accordingly. > > > > The last part is internal matters I think. I would just drop this > > paragraph, and update the brief to state "Open a V4L2 video device node > > and query its capabilities". > > You mean simply add the word 'node' to the existing brief? Yes, as the main difference between this method and the fd-based open method is that this one operates on a device node. > The 'a' doesn't give enough context in that case any more (or perhaps > before), because it only opens 'the' device node that exists in this > class instance. It doesn't open any device node. Only the one which was > previously set in the constructor. > > I'll change it to : > > "open /the/ V4L2 video device node and ..." Sounds good to me. > >> + * > >> * \return 0 on success or a negative error code otherwise > >> */ > >> int V4L2VideoDevice::open() > >> @@ -355,6 +365,83 @@ int V4L2VideoDevice::open() > >> return 0; > >> } > >> > >> +/** > >> + * \brief Open a V4L2 video device and query its capabilities > > > > And here "Open a V4L2 video device from an opened file handle and query > > its capabilities". > > Changed. > > >> + * > > > > No need for this blank line. > > > >> + * \param[in] handle The file descriptor to set > > > > s/handle/fd/ ? > > No,. > > >> + * \param[in] type The device type to operate on > >> + * > >> + * Sets the file descriptor for the device and then queries the capabilities of > > > > s/Sets/This method sets/ > > > > You need a subject before a conjugated verb. > > We're /in/ the subject. It can be implicit. It could just be me, and my over-exposure to pesky French grammar and conjugation :-) I've always been taught that a sentence that contain a cnjugated verb at any mood other than the imperative requires an explicit subject. Are the rules different in English ? Interestingly enough the style of the \brief seems fine to me, even though I'm not sure why. "Open a V4L2 video device" sounds correct to my ears (but what tense is "open" conjugated in ?), while starting a sentence with "Opens a V4L2 video device" sounds like something is missing. I'm obviously not challenging your native English knowledge :-) I would however like to understand what the grammatical rules are, in order to improve my own knowledge (and stop perstering everybody with similar grammatical comments). > >> + * the device and establishes the buffer types and device events accordingly. > > > > * This methods opens a video device from the existing file descriptor \a fd. > > We can't use fd as an argument, because we use fd(). Ah yes, I didn't notice that. We could use the qualified V4L2Device::fd() version and name the argument fd, but it's not worth it. Let's thus keep handle. > We can't use newFd, because that's the result of the dup(). > > That's why I've used handle. > > > * Like open() queries the capabilities of the device, but handles it according > > ^ > Isn't this the same issue? Absolutely :-) Sorry for the mistake. > > > * to the given device \a type instead of determining its type from the > > * capabilities. This can be used to force a given device type for M2M devices. > > * > > * The file descriptor \a fd is duplicated, and the caller shall close \a fd > > * when it has no further use for it. The close() method will close the > > * duplicated file descriptor, leaving \a fd untouched. > > Changed to: > > * This methods opens a video device from the existing file descriptor \a s/methods/method/ ? > * handle. Like open(), this method queries the capabilities of the device, but I would have written s/this method/it/ but that's up to you. > * handles it according to the given device \a type instead of determining its > * type from the capabilities. This can be used to force a given device type for > * M2M devices. Sounds very good to me. > * > * The file descriptor \a handle is duplicated, and the caller is responsible > * for closing the \a handle when it has no further use for it. The close() > * method will close the duplicated file descriptor, leaving \a handle > * untouched. > > >> + * > >> + * \return 0 on success or a negative error code otherwise > >> + */ > >> +int V4L2VideoDevice::open(int handle, enum v4l2_buf_type type) > >> +{ > >> + int ret; > >> + int newFd; > >> + > >> + newFd = dup(handle); > >> + if (newFd < 0) { > >> + ret = -errno; > >> + LOG(V4L2, Error) << "Failed to duplicate file handle: " > >> + << strerror(-ret); > >> + return ret; > >> + } > >> + > >> + ret = V4L2Device::setFd(newFd); > >> + if (ret < 0) { > >> + LOG(V4L2, Error) << "Failed to set file handle: " > >> + << strerror(-ret); > > > > You need to ::close(newFd) here. > > > > Ah yes. > > >> + return ret; > >> + } > >> + > >> + ret = ioctl(VIDIOC_QUERYCAP, &caps_); > >> + if (ret < 0) { > >> + LOG(V4L2, Error) > >> + << "Failed to query device capabilities: " > >> + << strerror(-ret); > >> + return ret; > >> + } > >> + > >> + if (!caps_.hasStreaming()) { > >> + LOG(V4L2, Error) << "Device does not support streaming I/O"; > >> + return -EINVAL; > >> + } > >> + > >> + /* > >> + * Set buffer type and wait for read notifications on CAPTURE video > >> + * devices (POLLIN), and write notifications for OUTPUT video devices > >> + * (POLLOUT). > >> + */ > >> + switch (type) { > >> + case V4L2_BUF_TYPE_VIDEO_OUTPUT: > >> + fdEvent_ = new EventNotifier(fd(), EventNotifier::Write); > >> + bufferType_ = caps_.isMultiplanar() > >> + ? V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE > >> + : V4L2_BUF_TYPE_VIDEO_OUTPUT; > >> + break; > >> + case V4L2_BUF_TYPE_VIDEO_CAPTURE: > >> + fdEvent_ = new EventNotifier(fd(), EventNotifier::Read); > >> + bufferType_ = caps_.isMultiplanar() > >> + ? V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE > >> + : V4L2_BUF_TYPE_VIDEO_CAPTURE; > >> + break; > >> + default: > >> + LOG(V4L2, Error) << "Unsupported buffer type"; > >> + return -EINVAL; > >> + } > >> + > >> + fdEvent_->activated.connect(this, &V4L2VideoDevice::bufferAvailable); > >> + fdEvent_->setEnabled(false); > >> + > >> + LOG(V4L2, Debug) > >> + << "Opened device " << caps_.bus_info() << ": " > >> + << caps_.driver() << ": " << caps_.card(); > >> + > >> + return 0; > >> +} > >> + > >> /** > >> * \brief Close the video device, releasing any resources acquired by open() > >> */ > >> @@ -1143,4 +1230,103 @@ V4L2VideoDevice *V4L2VideoDevice::fromEntityName(const MediaDevice *media, > >> return new V4L2VideoDevice(mediaEntity); > >> } > >> > >> +/** > >> + * \class V4L2M2MDevice > >> + * \brief Memory2Memory video device > >> + * > >> + * The V4L2M2MDevice manages two V4L2VideoDevice instances on the same > >> + * deviceNode which operate together using two queues to implement the V4L2 > >> + * Memory to Memory API. > >> + * > >> + * The two devices should be opened by calling open() on the V4L2M2MDevice, and > >> + * can be closed by calling close on the V4L2M2MDevice. > >> + * > >> + * Calling V4L2VideoDevice::open() and V4L2VideoDevice::close() on the capture > >> + * or output V4L2VideoDevice is not permitted. > >> + */ > >> + > >> +/** > >> + * \fn V4L2M2MDevice::output > >> + * \brief Provide access to the output V4L2VideoDevice instance > > > > s/Provide access to/Retrieve/ > > Changed. > > >> + * \return The output V4L2Device instance > >> + */ > >> + > >> +/** > >> + * \fn V4L2M2MDevice::capture > >> + * \brief Provide access to the capture V4L2VideoDevice instance > > > > s/Provide access to/Retrieve/ > > Changed. > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Thanks, > > >> + * \return The capture V4L2Device instance > >> + */ > >> + > >> +/** > >> + * \brief Create a new V4L2M2MDevice from the \a deviceNode > >> + */ > >> +V4L2M2MDevice::V4L2M2MDevice(const std::string &deviceNode) > >> + : deviceNode_(deviceNode) > >> +{ > >> + output_ = new V4L2VideoDevice(deviceNode); > >> + capture_ = new V4L2VideoDevice(deviceNode); > >> +} > >> + > >> +V4L2M2MDevice::~V4L2M2MDevice() > >> +{ > >> + delete capture_; > >> + delete output_; > >> +} > >> + > >> +/** > >> + * \brief Open a V4L2 Memory to Memory device > >> + * > >> + * Open the device node and prepare the two V4L2VideoDevice instances to handle > >> + * their respective buffer queues. > >> + * > >> + * \return 0 on success or a negative error code otherwise > >> + */ > >> +int V4L2M2MDevice::open() > >> +{ > >> + int fd; > >> + int ret; > >> + > >> + /* > >> + * The output and capture V4L2VideoDevice instances use the same file > >> + * handle for the same device node. The local file handle can be closed > >> + * as the V4L2VideoDevice::open() retains a handle by duplicating the > >> + * fd passed in. > >> + */ > >> + fd = ::open(deviceNode_.c_str(), O_RDWR | O_NONBLOCK); > >> + if (fd < 0) { > >> + ret = -errno; > >> + LOG(V4L2, Error) > >> + << "Failed to open V4L2 M2M device: " << strerror(-ret); > >> + return ret; > >> + } > >> + > >> + ret = output_->open(fd, V4L2_BUF_TYPE_VIDEO_OUTPUT); > >> + if (ret) > >> + goto err; > >> + > >> + ret = capture_->open(fd, V4L2_BUF_TYPE_VIDEO_CAPTURE); > >> + if (ret) > >> + goto err; > >> + > >> + ::close(fd); > >> + > >> + return 0; > >> + > >> +err: > >> + close(); > >> + ::close(fd); > >> + > >> + return ret; > >> +} > >> + > >> +/** > >> + * \brief Close the memory-to-memory device, releasing any resources acquired by > >> + * open() > >> + */ > >> +void V4L2M2MDevice::close() > >> +{ > >> + capture_->close(); > >> + output_->close(); > >> +} > >> + > >> } /* namespace libcamera */
Hi Laurent, On 12/08/2019 12:16, Laurent Pinchart wrote: > Hi Kieran, > > On Mon, Aug 12, 2019 at 10:01:53AM +0100, Kieran Bingham wrote: >> On 10/08/2019 14:41, Laurent Pinchart wrote: >>> On Fri, Aug 09, 2019 at 04:04:56PM +0100, Kieran Bingham wrote: >>>> V4L2 M2M devices represent a V4L2Device with two queues: One output, and >>>> one capture on a single device node. >>>> >>>> Represent this by instantiating a V4L2VideoDevice for each queue type, >>>> and preparing each device for its queue. >>>> >>>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >>>> --- >>>> src/libcamera/include/v4l2_videodevice.h | 28 ++++ >>>> src/libcamera/v4l2_videodevice.cpp | 186 +++++++++++++++++++++++ >>>> 2 files changed, 214 insertions(+) >>>> >>>> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h >>>> index f5c8da93fcb5..72dc8c63e4bb 100644 >>>> --- a/src/libcamera/include/v4l2_videodevice.h >>>> +++ b/src/libcamera/include/v4l2_videodevice.h >>>> @@ -71,6 +71,11 @@ struct V4L2Capability final : v4l2_capability { >>>> V4L2_CAP_VIDEO_OUTPUT | >>>> V4L2_CAP_VIDEO_OUTPUT_MPLANE); >>>> } >>>> + bool isM2M() const >>>> + { >>>> + return device_caps() & (V4L2_CAP_VIDEO_M2M | >>>> + V4L2_CAP_VIDEO_M2M_MPLANE); >>>> + } >>>> bool isMeta() const >>>> { >>>> return device_caps() & (V4L2_CAP_META_CAPTURE | >>>> @@ -124,6 +129,7 @@ public: >>>> V4L2VideoDevice &operator=(const V4L2VideoDevice &) = delete; >>>> >>>> int open(); >>>> + int open(int handle, enum v4l2_buf_type type); >>>> void close(); >>>> >>>> const char *driverName() const { return caps_.driver(); } >>>> @@ -152,6 +158,9 @@ protected: >>>> std::string logPrefix() const; >>>> >>>> private: >>>> + int queryBufferType(); >>>> + int queryBufferType(enum v4l2_buf_type type); >>> >>> I think these are leftovers. >> >> Yes, removed. >> >>>> + >>>> int getFormatMeta(V4L2DeviceFormat *format); >>>> int setFormatMeta(V4L2DeviceFormat *format); >>>> >>>> @@ -182,6 +191,25 @@ private: >>>> EventNotifier *fdEvent_; >>>> }; >>>> >>>> +class V4L2M2MDevice >>>> +{ >>>> +public: >>>> + V4L2M2MDevice(const std::string &deviceNode); >>>> + ~V4L2M2MDevice(); >>>> + >>>> + int open(); >>>> + void close(); >>>> + >>>> + V4L2VideoDevice *output() { return output_; } >>>> + V4L2VideoDevice *capture() { return capture_; } >>>> + >>>> +private: >>>> + std::string deviceNode_; >>>> + >>>> + V4L2VideoDevice *output_; >>>> + V4L2VideoDevice *capture_; >>>> +}; >>>> + >>>> } /* namespace libcamera */ >>>> >>>> #endif /* __LIBCAMERA_V4L2_VIDEODEVICE_H__ */ >>>> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp >>>> index 81098dd70190..9c5638995577 100644 >>>> --- a/src/libcamera/v4l2_videodevice.cpp >>>> +++ b/src/libcamera/v4l2_videodevice.cpp >>>> @@ -89,6 +89,12 @@ LOG_DECLARE_CATEGORY(V4L2) >>>> * \return True if the video device can capture or output images >>>> */ >>>> >>>> +/** >>>> + * \fn V4L2Capability::isM2M() >>>> + * \brief Identify if the device is an M2M device >>>> + * \return True if the device can capture and output images using the M2M API >>>> + */ >>>> + >>>> /** >>>> * \fn V4L2Capability::isMeta() >>>> * \brief Identify if the video device captures or outputs image meta-data >>>> @@ -296,6 +302,10 @@ V4L2VideoDevice::~V4L2VideoDevice() >>>> >>>> /** >>>> * \brief Open a V4L2 video device and query its capabilities >>>> + * >>>> + * Opens a video device, then queries the capabilities of the device and >>> >>> s/Opens/This method opens/ >>> >>>> + * establishes the buffer types and device events accordingly. >>> >>> The last part is internal matters I think. I would just drop this >>> paragraph, and update the brief to state "Open a V4L2 video device node >>> and query its capabilities". >> >> You mean simply add the word 'node' to the existing brief? > > Yes, as the main difference between this method and the fd-based open > method is that this one operates on a device node. > >> The 'a' doesn't give enough context in that case any more (or perhaps >> before), because it only opens 'the' device node that exists in this >> class instance. It doesn't open any device node. Only the one which was >> previously set in the constructor. >> >> I'll change it to : >> >> "open /the/ V4L2 video device node and ..." > > Sounds good to me. > >>>> + * >>>> * \return 0 on success or a negative error code otherwise >>>> */ >>>> int V4L2VideoDevice::open() >>>> @@ -355,6 +365,83 @@ int V4L2VideoDevice::open() >>>> return 0; >>>> } >>>> >>>> +/** >>>> + * \brief Open a V4L2 video device and query its capabilities >>> >>> And here "Open a V4L2 video device from an opened file handle and query >>> its capabilities". >> >> Changed. >> >>>> + * >>> >>> No need for this blank line. >>> >>>> + * \param[in] handle The file descriptor to set >>> >>> s/handle/fd/ ? >> >> No,. >> >>>> + * \param[in] type The device type to operate on >>>> + * >>>> + * Sets the file descriptor for the device and then queries the capabilities of >>> >>> s/Sets/This method sets/ >>> >>> You need a subject before a conjugated verb. >> >> We're /in/ the subject. It can be implicit. > > It could just be me, and my over-exposure to pesky French grammar and > conjugation :-) I've always been taught that a sentence that contain a > cnjugated verb at any mood other than the imperative requires an > explicit subject. Are the rules different in English ? Maybe we need someone who's taken languages as a degree for that. <checks with Mrs B> Well Mrs-B agrees with you, saying it should be at least "This sets" (with the 'method' being optional) > Interestingly enough the style of the \brief seems fine to me, even > though I'm not sure why. "Open a V4L2 video device" sounds correct to my > ears (but what tense is "open" conjugated in ?), while starting a Open is not conjugated - it's just the imperative form. > sentence with "Opens a V4L2 video device" sounds like something is > missing. > > I'm obviously not challenging your native English knowledge :-) I would > however like to understand what the grammatical rules are, in order to > improve my own knowledge (and stop perstering everybody with similar > grammatical comments). Well, I don't think my native language processing follows written rules, it just goes with what sounds right :-D ... and provides the subject from the context - so "Opens a V4L2 video device" sounds right to me (when in the context of a description of a function). But if it's not right - then it's not right. >>>> + * the device and establishes the buffer types and device events accordingly. >>> >>> * This methods opens a video device from the existing file descriptor \a fd. >> >> We can't use fd as an argument, because we use fd(). > > Ah yes, I didn't notice that. We could use the qualified > V4L2Device::fd() version and name the argument fd, but it's not worth > it. Let's thus keep handle. That's possibly a good option too, but I do like the clarity of having a distinct name. >> We can't use newFd, because that's the result of the dup(). >> >> That's why I've used handle. >> >>> * Like open() queries the capabilities of the device, but handles it according >> >> ^ >> Isn't this the same issue? > > Absolutely :-) Sorry for the mistake. > >> >>> * to the given device \a type instead of determining its type from the >>> * capabilities. This can be used to force a given device type for M2M devices. >>> * >>> * The file descriptor \a fd is duplicated, and the caller shall close \a fd >>> * when it has no further use for it. The close() method will close the >>> * duplicated file descriptor, leaving \a fd untouched. >> >> Changed to: >> >> * This methods opens a video device from the existing file descriptor \a > > s/methods/method/ ? Good spot - fixed. >> * handle. Like open(), this method queries the capabilities of the device, but > > I would have written s/this method/it/ but that's up to you. I started with 'it', but it seemed we may as well fully-qualify the subject. Maybe I'll drop the 'method', and keep "This queries,' to reduce the repetition. > >> * handles it according to the given device \a type instead of determining its >> * type from the capabilities. This can be used to force a given device type for >> * M2M devices. > > Sounds very good to me. > >> * >> * The file descriptor \a handle is duplicated, and the caller is responsible >> * for closing the \a handle when it has no further use for it. The close() >> * method will close the duplicated file descriptor, leaving \a handle >> * untouched. >> >>>> + * >>>> + * \return 0 on success or a negative error code otherwise >>>> + */ >>>> +int V4L2VideoDevice::open(int handle, enum v4l2_buf_type type) >>>> +{ >>>> + int ret; >>>> + int newFd; >>>> + >>>> + newFd = dup(handle); >>>> + if (newFd < 0) { >>>> + ret = -errno; >>>> + LOG(V4L2, Error) << "Failed to duplicate file handle: " >>>> + << strerror(-ret); >>>> + return ret; >>>> + } >>>> + >>>> + ret = V4L2Device::setFd(newFd); >>>> + if (ret < 0) { >>>> + LOG(V4L2, Error) << "Failed to set file handle: " >>>> + << strerror(-ret); >>> >>> You need to ::close(newFd) here. >>> >> >> Ah yes. >> >>>> + return ret; >>>> + } >>>> + >>>> + ret = ioctl(VIDIOC_QUERYCAP, &caps_); >>>> + if (ret < 0) { >>>> + LOG(V4L2, Error) >>>> + << "Failed to query device capabilities: " >>>> + << strerror(-ret); >>>> + return ret; >>>> + } >>>> + >>>> + if (!caps_.hasStreaming()) { >>>> + LOG(V4L2, Error) << "Device does not support streaming I/O"; >>>> + return -EINVAL; >>>> + } >>>> + >>>> + /* >>>> + * Set buffer type and wait for read notifications on CAPTURE video >>>> + * devices (POLLIN), and write notifications for OUTPUT video devices >>>> + * (POLLOUT). >>>> + */ >>>> + switch (type) { >>>> + case V4L2_BUF_TYPE_VIDEO_OUTPUT: >>>> + fdEvent_ = new EventNotifier(fd(), EventNotifier::Write); >>>> + bufferType_ = caps_.isMultiplanar() >>>> + ? V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE >>>> + : V4L2_BUF_TYPE_VIDEO_OUTPUT; >>>> + break; >>>> + case V4L2_BUF_TYPE_VIDEO_CAPTURE: >>>> + fdEvent_ = new EventNotifier(fd(), EventNotifier::Read); >>>> + bufferType_ = caps_.isMultiplanar() >>>> + ? V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE >>>> + : V4L2_BUF_TYPE_VIDEO_CAPTURE; >>>> + break; >>>> + default: >>>> + LOG(V4L2, Error) << "Unsupported buffer type"; >>>> + return -EINVAL; >>>> + } >>>> + >>>> + fdEvent_->activated.connect(this, &V4L2VideoDevice::bufferAvailable); >>>> + fdEvent_->setEnabled(false); >>>> + >>>> + LOG(V4L2, Debug) >>>> + << "Opened device " << caps_.bus_info() << ": " >>>> + << caps_.driver() << ": " << caps_.card(); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> /** >>>> * \brief Close the video device, releasing any resources acquired by open() >>>> */ >>>> @@ -1143,4 +1230,103 @@ V4L2VideoDevice *V4L2VideoDevice::fromEntityName(const MediaDevice *media, >>>> return new V4L2VideoDevice(mediaEntity); >>>> } >>>> >>>> +/** >>>> + * \class V4L2M2MDevice >>>> + * \brief Memory2Memory video device >>>> + * >>>> + * The V4L2M2MDevice manages two V4L2VideoDevice instances on the same >>>> + * deviceNode which operate together using two queues to implement the V4L2 >>>> + * Memory to Memory API. >>>> + * >>>> + * The two devices should be opened by calling open() on the V4L2M2MDevice, and >>>> + * can be closed by calling close on the V4L2M2MDevice. >>>> + * >>>> + * Calling V4L2VideoDevice::open() and V4L2VideoDevice::close() on the capture >>>> + * or output V4L2VideoDevice is not permitted. >>>> + */ >>>> + >>>> +/** >>>> + * \fn V4L2M2MDevice::output >>>> + * \brief Provide access to the output V4L2VideoDevice instance >>> >>> s/Provide access to/Retrieve/ >> >> Changed. >> >>>> + * \return The output V4L2Device instance >>>> + */ >>>> + >>>> +/** >>>> + * \fn V4L2M2MDevice::capture >>>> + * \brief Provide access to the capture V4L2VideoDevice instance >>> >>> s/Provide access to/Retrieve/ >> >> Changed. >> >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> >> Thanks, >> >>>> + * \return The capture V4L2Device instance >>>> + */ >>>> + >>>> +/** >>>> + * \brief Create a new V4L2M2MDevice from the \a deviceNode >>>> + */ >>>> +V4L2M2MDevice::V4L2M2MDevice(const std::string &deviceNode) >>>> + : deviceNode_(deviceNode) >>>> +{ >>>> + output_ = new V4L2VideoDevice(deviceNode); >>>> + capture_ = new V4L2VideoDevice(deviceNode); >>>> +} >>>> + >>>> +V4L2M2MDevice::~V4L2M2MDevice() >>>> +{ >>>> + delete capture_; >>>> + delete output_; >>>> +} >>>> + >>>> +/** >>>> + * \brief Open a V4L2 Memory to Memory device >>>> + * >>>> + * Open the device node and prepare the two V4L2VideoDevice instances to handle >>>> + * their respective buffer queues. >>>> + * >>>> + * \return 0 on success or a negative error code otherwise >>>> + */ >>>> +int V4L2M2MDevice::open() >>>> +{ >>>> + int fd; >>>> + int ret; >>>> + >>>> + /* >>>> + * The output and capture V4L2VideoDevice instances use the same file >>>> + * handle for the same device node. The local file handle can be closed >>>> + * as the V4L2VideoDevice::open() retains a handle by duplicating the >>>> + * fd passed in. >>>> + */ >>>> + fd = ::open(deviceNode_.c_str(), O_RDWR | O_NONBLOCK); >>>> + if (fd < 0) { >>>> + ret = -errno; >>>> + LOG(V4L2, Error) >>>> + << "Failed to open V4L2 M2M device: " << strerror(-ret); >>>> + return ret; >>>> + } >>>> + >>>> + ret = output_->open(fd, V4L2_BUF_TYPE_VIDEO_OUTPUT); >>>> + if (ret) >>>> + goto err; >>>> + >>>> + ret = capture_->open(fd, V4L2_BUF_TYPE_VIDEO_CAPTURE); >>>> + if (ret) >>>> + goto err; >>>> + >>>> + ::close(fd); >>>> + >>>> + return 0; >>>> + >>>> +err: >>>> + close(); >>>> + ::close(fd); >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +/** >>>> + * \brief Close the memory-to-memory device, releasing any resources acquired by >>>> + * open() >>>> + */ >>>> +void V4L2M2MDevice::close() >>>> +{ >>>> + capture_->close(); >>>> + output_->close(); >>>> +} >>>> + >>>> } /* namespace libcamera */ >
diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index f5c8da93fcb5..72dc8c63e4bb 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -71,6 +71,11 @@ struct V4L2Capability final : v4l2_capability { V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_VIDEO_OUTPUT_MPLANE); } + bool isM2M() const + { + return device_caps() & (V4L2_CAP_VIDEO_M2M | + V4L2_CAP_VIDEO_M2M_MPLANE); + } bool isMeta() const { return device_caps() & (V4L2_CAP_META_CAPTURE | @@ -124,6 +129,7 @@ public: V4L2VideoDevice &operator=(const V4L2VideoDevice &) = delete; int open(); + int open(int handle, enum v4l2_buf_type type); void close(); const char *driverName() const { return caps_.driver(); } @@ -152,6 +158,9 @@ protected: std::string logPrefix() const; private: + int queryBufferType(); + int queryBufferType(enum v4l2_buf_type type); + int getFormatMeta(V4L2DeviceFormat *format); int setFormatMeta(V4L2DeviceFormat *format); @@ -182,6 +191,25 @@ private: EventNotifier *fdEvent_; }; +class V4L2M2MDevice +{ +public: + V4L2M2MDevice(const std::string &deviceNode); + ~V4L2M2MDevice(); + + int open(); + void close(); + + V4L2VideoDevice *output() { return output_; } + V4L2VideoDevice *capture() { return capture_; } + +private: + std::string deviceNode_; + + V4L2VideoDevice *output_; + V4L2VideoDevice *capture_; +}; + } /* namespace libcamera */ #endif /* __LIBCAMERA_V4L2_VIDEODEVICE_H__ */ diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 81098dd70190..9c5638995577 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -89,6 +89,12 @@ LOG_DECLARE_CATEGORY(V4L2) * \return True if the video device can capture or output images */ +/** + * \fn V4L2Capability::isM2M() + * \brief Identify if the device is an M2M device + * \return True if the device can capture and output images using the M2M API + */ + /** * \fn V4L2Capability::isMeta() * \brief Identify if the video device captures or outputs image meta-data @@ -296,6 +302,10 @@ V4L2VideoDevice::~V4L2VideoDevice() /** * \brief Open a V4L2 video device and query its capabilities + * + * Opens a video device, then queries the capabilities of the device and + * establishes the buffer types and device events accordingly. + * * \return 0 on success or a negative error code otherwise */ int V4L2VideoDevice::open() @@ -355,6 +365,83 @@ int V4L2VideoDevice::open() return 0; } +/** + * \brief Open a V4L2 video device and query its capabilities + * + * \param[in] handle The file descriptor to set + * \param[in] type The device type to operate on + * + * Sets the file descriptor for the device and then queries the capabilities of + * the device and establishes the buffer types and device events accordingly. + * + * \return 0 on success or a negative error code otherwise + */ +int V4L2VideoDevice::open(int handle, enum v4l2_buf_type type) +{ + int ret; + int newFd; + + newFd = dup(handle); + if (newFd < 0) { + ret = -errno; + LOG(V4L2, Error) << "Failed to duplicate file handle: " + << strerror(-ret); + return ret; + } + + ret = V4L2Device::setFd(newFd); + if (ret < 0) { + LOG(V4L2, Error) << "Failed to set file handle: " + << strerror(-ret); + return ret; + } + + ret = ioctl(VIDIOC_QUERYCAP, &caps_); + if (ret < 0) { + LOG(V4L2, Error) + << "Failed to query device capabilities: " + << strerror(-ret); + return ret; + } + + if (!caps_.hasStreaming()) { + LOG(V4L2, Error) << "Device does not support streaming I/O"; + return -EINVAL; + } + + /* + * Set buffer type and wait for read notifications on CAPTURE video + * devices (POLLIN), and write notifications for OUTPUT video devices + * (POLLOUT). + */ + switch (type) { + case V4L2_BUF_TYPE_VIDEO_OUTPUT: + fdEvent_ = new EventNotifier(fd(), EventNotifier::Write); + bufferType_ = caps_.isMultiplanar() + ? V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE + : V4L2_BUF_TYPE_VIDEO_OUTPUT; + break; + case V4L2_BUF_TYPE_VIDEO_CAPTURE: + fdEvent_ = new EventNotifier(fd(), EventNotifier::Read); + bufferType_ = caps_.isMultiplanar() + ? V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE + : V4L2_BUF_TYPE_VIDEO_CAPTURE; + break; + default: + LOG(V4L2, Error) << "Unsupported buffer type"; + return -EINVAL; + } + + fdEvent_->activated.connect(this, &V4L2VideoDevice::bufferAvailable); + fdEvent_->setEnabled(false); + + LOG(V4L2, Debug) + << "Opened device " << caps_.bus_info() << ": " + << caps_.driver() << ": " << caps_.card(); + + return 0; +} + /** * \brief Close the video device, releasing any resources acquired by open() */ @@ -1143,4 +1230,103 @@ V4L2VideoDevice *V4L2VideoDevice::fromEntityName(const MediaDevice *media, return new V4L2VideoDevice(mediaEntity); } +/** + * \class V4L2M2MDevice + * \brief Memory2Memory video device + * + * The V4L2M2MDevice manages two V4L2VideoDevice instances on the same + * deviceNode which operate together using two queues to implement the V4L2 + * Memory to Memory API. + * + * The two devices should be opened by calling open() on the V4L2M2MDevice, and + * can be closed by calling close on the V4L2M2MDevice. + * + * Calling V4L2VideoDevice::open() and V4L2VideoDevice::close() on the capture + * or output V4L2VideoDevice is not permitted. + */ + +/** + * \fn V4L2M2MDevice::output + * \brief Provide access to the output V4L2VideoDevice instance + * \return The output V4L2Device instance + */ + +/** + * \fn V4L2M2MDevice::capture + * \brief Provide access to the capture V4L2VideoDevice instance + * \return The capture V4L2Device instance + */ + +/** + * \brief Create a new V4L2M2MDevice from the \a deviceNode + */ +V4L2M2MDevice::V4L2M2MDevice(const std::string &deviceNode) + : deviceNode_(deviceNode) +{ + output_ = new V4L2VideoDevice(deviceNode); + capture_ = new V4L2VideoDevice(deviceNode); +} + +V4L2M2MDevice::~V4L2M2MDevice() +{ + delete capture_; + delete output_; +} + +/** + * \brief Open a V4L2 Memory to Memory device + * + * Open the device node and prepare the two V4L2VideoDevice instances to handle + * their respective buffer queues. + * + * \return 0 on success or a negative error code otherwise + */ +int V4L2M2MDevice::open() +{ + int fd; + int ret; + + /* + * The output and capture V4L2VideoDevice instances use the same file + * handle for the same device node. The local file handle can be closed + * as the V4L2VideoDevice::open() retains a handle by duplicating the + * fd passed in. + */ + fd = ::open(deviceNode_.c_str(), O_RDWR | O_NONBLOCK); + if (fd < 0) { + ret = -errno; + LOG(V4L2, Error) + << "Failed to open V4L2 M2M device: " << strerror(-ret); + return ret; + } + + ret = output_->open(fd, V4L2_BUF_TYPE_VIDEO_OUTPUT); + if (ret) + goto err; + + ret = capture_->open(fd, V4L2_BUF_TYPE_VIDEO_CAPTURE); + if (ret) + goto err; + + ::close(fd); + + return 0; + +err: + close(); + ::close(fd); + + return ret; +} + +/** + * \brief Close the memory-to-memory device, releasing any resources acquired by + * open() + */ +void V4L2M2MDevice::close() +{ + capture_->close(); + output_->close(); +} + } /* namespace libcamera */
V4L2 M2M devices represent a V4L2Device with two queues: One output, and one capture on a single device node. Represent this by instantiating a V4L2VideoDevice for each queue type, and preparing each device for its queue. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/libcamera/include/v4l2_videodevice.h | 28 ++++ src/libcamera/v4l2_videodevice.cpp | 186 +++++++++++++++++++++++ 2 files changed, 214 insertions(+)