| Message ID | 20251023144841.403689-6-stefan.klug@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi 2025. 10. 23. 16:48 keltezéssel, Stefan Klug írta: > The V4L2 requests API provides support to atomically tie controls to a > set of buffers. This is especially common for m2m devices. Such a > request is represented by a fd that is allocated vi > MEDIA_IOC_REQUEST_ALLOC and then passed to various V4L2 function. > > Implement a V4L2Request class to wrap such an fd and add the > corresponding utility functions. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > --- > > Changes in v2: > - Added documentation > --- > include/libcamera/internal/media_device.h | 7 + > include/libcamera/internal/meson.build | 1 + > include/libcamera/internal/v4l2_device.h | 5 +- > include/libcamera/internal/v4l2_request.h | 49 +++++++ > include/libcamera/internal/v4l2_videodevice.h | 3 +- > src/libcamera/media_device.cpp | 47 +++++++ > src/libcamera/meson.build | 1 + > src/libcamera/v4l2_device.cpp | 28 +++- > src/libcamera/v4l2_request.cpp | 128 ++++++++++++++++++ > src/libcamera/v4l2_videodevice.cpp | 10 +- > 10 files changed, 271 insertions(+), 8 deletions(-) > create mode 100644 include/libcamera/internal/v4l2_request.h > create mode 100644 src/libcamera/v4l2_request.cpp > > [...] > diff --git a/include/libcamera/internal/v4l2_request.h b/include/libcamera/internal/v4l2_request.h > new file mode 100644 > index 000000000000..bf1bea3261af > --- /dev/null > +++ b/include/libcamera/internal/v4l2_request.h > @@ -0,0 +1,49 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas On Board > + * > + * V4L2 requests > + */ > + > +#pragma once > + > +#include <string> > + > +#include <linux/videodev2.h> > + > +#include <libcamera/base/event_notifier.h> > +#include <libcamera/base/log.h> > +#include <libcamera/base/signal.h> > +#include <libcamera/base/span.h> > +#include <libcamera/base/unique_fd.h> > + > +#include <libcamera/color_space.h> > +#include <libcamera/controls.h> > + > +namespace libcamera { > + > +class V4L2Request : protected Loggable > +{ > +public: > + bool isValid() const { return fd_.isValid(); } > + int fd() const { return fd_.get(); } > + > + int reinit(); > + int queue(); > + > + V4L2Request(int fd = -1); I think just taking `UniqueFD` would be better as it makes the ownership clear. Is it is useful to create a `V4L2Request` object without a valid file descriptor? There is no way to change the file descriptor and as far as I can tell most methods would just fail. > + ~V4L2Request() = default; I think this is already implied. > + > + Signal<V4L2Request *> requestDone; > + > +private: > + LIBCAMERA_DISABLE_COPY_AND_MOVE(V4L2Request) > + > + void requestReady(); > + std::string logPrefix() const override; > + > + UniqueFD fd_; > + EventNotifier fdNotifier_; > +}; > + > +} /* namespace libcamera */ > [...] > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp > index 353f34a81eca..673c53fefdd7 100644 > --- a/src/libcamera/media_device.cpp > +++ b/src/libcamera/media_device.cpp > @@ -20,6 +20,7 @@ > #include <linux/media.h> > > #include <libcamera/base/log.h> > +#include "libcamera/internal/v4l2_request.h" > > /** > * \file media_device.h > @@ -851,4 +852,50 @@ std::vector<MediaEntity *> MediaDevice::locateEntities(unsigned int function) > return found; > } > > +/** > + * \brief Allocate requests > + * \param[in] count Number of requests to allocate > + * \param[out] requests Vector to store allocated requests > + * > + * Allocates and stores \a count requests in \a requests. If allocation fails, > + * and error is returned and \a requests is cleared. > + * > + * \return 0 on success or a negative error code otherwise > + */ > +int MediaDevice::allocateRequests(unsigned int count, > + std::vector<std::unique_ptr<V4L2Request>> *requests) > +{ > + requests->resize(count); > + for (unsigned int i = 0; i < count; i++) { > + int requestFd; > + int ret = ::ioctl(fd_.get(), MEDIA_IOC_REQUEST_ALLOC, &requestFd); > + if (ret < 0) { > + requests->clear(); > + return -errno; > + } > + (*requests)[i] = std::make_unique<V4L2Request>(requestFd); > + } > + > + return 0; > +} > + > +/** > + * \brief Check if requests are supported > + * > + * Checks if the device supports V4L2 requests by trying to allocate a single > + * request. The result is cached, so the allocation is only tried once. > + * > + * \return True if the device supports requests, false otherwise > + */ > +bool MediaDevice::supportsRequests() > +{ > + if (supportsRequests_.has_value()) > + return supportsRequests_.value(); > + > + std::vector<std::unique_ptr<V4L2Request>> requests; > + supportsRequests_ = (allocateRequests(1, &requests) == 0); Shouldn't it technically be something like ` != -ENOTTY` ? > + > + return supportsRequests_.value(); > +} > + > } /* namespace libcamera */ > [...] > diff --git a/src/libcamera/v4l2_request.cpp b/src/libcamera/v4l2_request.cpp > new file mode 100644 > index 000000000000..708250d86f61 > --- /dev/null > +++ b/src/libcamera/v4l2_request.cpp > @@ -0,0 +1,128 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas On Board > + * > + * V4L2 Request API > + */ > + > +#include "libcamera/internal/v4l2_request.h" > + > +#include <fcntl.h> > +#include <stdlib.h> > +#include <sys/ioctl.h> > +#include <sys/syscall.h> > +#include <unistd.h> > + > +#include <linux/media.h> > + > +#include <libcamera/base/event_notifier.h> > +#include <libcamera/base/log.h> > + > +/** > + * \file v4l2_request.h > + * \brief V4L2 Request > + */ > + > +namespace libcamera { > + > +LOG_DECLARE_CATEGORY(V4L2) > + > +/** > + * \class V4L2Request > + * \brief V4L2Request object and API > + * > + * The V4L2Request class wraps a V4L2 request fd and provides some convenience > + * functions to handle request. > + * > + * It is usually constructed by calling \a MediaDevice::allocateRequests(). > + * > + * A request can then be passed to the V4L2Device::setControls(), > + * V4L2Device::getControls() and V4L2VideoDevice::queueBuffer(). > + */ > + > +/** > + * \brief Construct a V4L2Request > + * \param[in] fd The request fd > + */ > +V4L2Request::V4L2Request(int fd) > + : fd_(fd), fdNotifier_(fd, EventNotifier::Exception) > +{ > + if (!fd_.isValid()) > + return; > + > + fdNotifier_.activated.connect(this, &V4L2Request::requestReady); It's probably just me, but I feel like I would just use a simple lambda here. > + fdNotifier_.setEnabled(false); > +} > [...] Regards, Barnabás Pőcze
Hi Stefan, Thank you for the patch! Some nitpicks below: Quoting Stefan Klug (2025-10-23 15:48:06) > The V4L2 requests API provides support to atomically tie controls to a > set of buffers. This is especially common for m2m devices. Such a > request is represented by a fd that is allocated vi s/vi/via/ > MEDIA_IOC_REQUEST_ALLOC and then passed to various V4L2 function. > > Implement a V4L2Request class to wrap such an fd and add the > corresponding utility functions. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > --- > > Changes in v2: > - Added documentation > --- > include/libcamera/internal/media_device.h | 7 + > include/libcamera/internal/meson.build | 1 + > include/libcamera/internal/v4l2_device.h | 5 +- > include/libcamera/internal/v4l2_request.h | 49 +++++++ > include/libcamera/internal/v4l2_videodevice.h | 3 +- > src/libcamera/media_device.cpp | 47 +++++++ > src/libcamera/meson.build | 1 + > src/libcamera/v4l2_device.cpp | 28 +++- > src/libcamera/v4l2_request.cpp | 128 ++++++++++++++++++ > src/libcamera/v4l2_videodevice.cpp | 10 +- > 10 files changed, 271 insertions(+), 8 deletions(-) > create mode 100644 include/libcamera/internal/v4l2_request.h > create mode 100644 src/libcamera/v4l2_request.cpp > > diff --git a/include/libcamera/internal/media_device.h b/include/libcamera/internal/media_device.h > index b3a48b98d64b..74cb9ba1542d 100644 > --- a/include/libcamera/internal/media_device.h > +++ b/include/libcamera/internal/media_device.h > @@ -18,6 +18,7 @@ > #include <libcamera/base/unique_fd.h> > > #include "libcamera/internal/media_object.h" > +#include "libcamera/internal/v4l2_request.h" > > namespace libcamera { > > @@ -57,6 +58,11 @@ public: > > std::vector<MediaEntity *> locateEntities(unsigned int function); > > + int allocateRequests(unsigned int count, > + std::vector<std::unique_ptr<V4L2Request>> *requests); > + > + bool supportsRequests(); > + > protected: > std::string logPrefix() const override; > > @@ -87,6 +93,7 @@ private: > UniqueFD fd_; > bool valid_; > bool acquired_; > + std::optional<bool> supportsRequests_; > > std::map<unsigned int, MediaObject *> objects_; > std::vector<MediaEntity *> entities_; > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > index 45c299f6a332..e9540a2f734f 100644 > --- a/include/libcamera/internal/meson.build > +++ b/include/libcamera/internal/meson.build > @@ -44,6 +44,7 @@ libcamera_internal_headers = files([ > 'sysfs.h', > 'v4l2_device.h', > 'v4l2_pixelformat.h', > + 'v4l2_request.h', > 'v4l2_subdevice.h', > 'v4l2_videodevice.h', > 'vector.h', > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h > index 5bc9da96677d..dbbd118abd00 100644 > --- a/include/libcamera/internal/v4l2_device.h > +++ b/include/libcamera/internal/v4l2_device.h > @@ -24,6 +24,7 @@ > #include <libcamera/controls.h> > > #include "libcamera/internal/formats.h" > +#include "libcamera/internal/v4l2_request.h" > > namespace libcamera { > > @@ -37,8 +38,8 @@ public: > > const ControlInfoMap &controls() const { return controls_; } > > - ControlList getControls(Span<const uint32_t> ids); > - int setControls(ControlList *ctrls); > + ControlList getControls(Span<const uint32_t> ids, const V4L2Request *request = nullptr); > + int setControls(ControlList *ctrls, const V4L2Request *request = nullptr); > > const struct v4l2_query_ext_ctrl *controlInfo(uint32_t id) const; > > diff --git a/include/libcamera/internal/v4l2_request.h b/include/libcamera/internal/v4l2_request.h > new file mode 100644 > index 000000000000..bf1bea3261af > --- /dev/null > +++ b/include/libcamera/internal/v4l2_request.h > @@ -0,0 +1,49 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas On Board > + * > + * V4L2 requests > + */ > + > +#pragma once > + > +#include <string> > + > +#include <linux/videodev2.h> > + > +#include <libcamera/base/event_notifier.h> > +#include <libcamera/base/log.h> > +#include <libcamera/base/signal.h> > +#include <libcamera/base/span.h> > +#include <libcamera/base/unique_fd.h> > + > +#include <libcamera/color_space.h> > +#include <libcamera/controls.h> > + > +namespace libcamera { > + > +class V4L2Request : protected Loggable > +{ > +public: > + bool isValid() const { return fd_.isValid(); } > + int fd() const { return fd_.get(); } > + > + int reinit(); > + int queue(); > + > + V4L2Request(int fd = -1); > + ~V4L2Request() = default; > + > + Signal<V4L2Request *> requestDone; > + > +private: > + LIBCAMERA_DISABLE_COPY_AND_MOVE(V4L2Request) > + > + void requestReady(); > + std::string logPrefix() const override; > + > + UniqueFD fd_; > + EventNotifier fdNotifier_; > +}; > + > +} /* namespace libcamera */ > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h > index 5a7dcfdda118..2d290971a0ee 100644 > --- a/include/libcamera/internal/v4l2_videodevice.h > +++ b/include/libcamera/internal/v4l2_videodevice.h > @@ -33,6 +33,7 @@ > #include "libcamera/internal/formats.h" > #include "libcamera/internal/v4l2_device.h" > #include "libcamera/internal/v4l2_pixelformat.h" > +#include "libcamera/internal/v4l2_request.h" > > namespace libcamera { > > @@ -217,7 +218,7 @@ public: > int importBuffers(unsigned int count); > int releaseBuffers(); > > - int queueBuffer(FrameBuffer *buffer); > + int queueBuffer(FrameBuffer *buffer, const V4L2Request *request = nullptr); > Signal<FrameBuffer *> bufferReady; > > int streamOn(); > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp > index 353f34a81eca..673c53fefdd7 100644 > --- a/src/libcamera/media_device.cpp > +++ b/src/libcamera/media_device.cpp > @@ -20,6 +20,7 @@ > #include <linux/media.h> > > #include <libcamera/base/log.h> > +#include "libcamera/internal/v4l2_request.h" > > /** > * \file media_device.h > @@ -851,4 +852,50 @@ std::vector<MediaEntity *> MediaDevice::locateEntities(unsigned int function) > return found; > } > > +/** > + * \brief Allocate requests > + * \param[in] count Number of requests to allocate > + * \param[out] requests Vector to store allocated requests > + * > + * Allocates and stores \a count requests in \a requests. If allocation fails, > + * and error is returned and \a requests is cleared. > + * > + * \return 0 on success or a negative error code otherwise > + */ > +int MediaDevice::allocateRequests(unsigned int count, > + std::vector<std::unique_ptr<V4L2Request>> *requests) > +{ > + requests->resize(count); > + for (unsigned int i = 0; i < count; i++) { > + int requestFd; > + int ret = ::ioctl(fd_.get(), MEDIA_IOC_REQUEST_ALLOC, &requestFd); > + if (ret < 0) { > + requests->clear(); > + return -errno; > + } > + (*requests)[i] = std::make_unique<V4L2Request>(requestFd); > + } > + > + return 0; > +} > + > +/** > + * \brief Check if requests are supported > + * > + * Checks if the device supports V4L2 requests by trying to allocate a single > + * request. The result is cached, so the allocation is only tried once. > + * > + * \return True if the device supports requests, false otherwise > + */ > +bool MediaDevice::supportsRequests() > +{ > + if (supportsRequests_.has_value()) > + return supportsRequests_.value(); > + > + std::vector<std::unique_ptr<V4L2Request>> requests; > + supportsRequests_ = (allocateRequests(1, &requests) == 0); > + > + return supportsRequests_.value(); > +} > + > } /* namespace libcamera */ > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index 5b9b86f211f1..34e20f557514 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -54,6 +54,7 @@ libcamera_internal_sources = files([ > 'sysfs.cpp', > 'v4l2_device.cpp', > 'v4l2_pixelformat.cpp', > + 'v4l2_request.cpp', > 'v4l2_subdevice.cpp', > 'v4l2_videodevice.cpp', > 'vector.cpp', > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index 8c78b8c424e5..7a669a0303c1 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -162,6 +162,7 @@ void V4L2Device::close() > /** > * \brief Read controls from the device > * \param[in] ids The list of controls to read, specified by their ID > + * \param[in] request An optional request > * > * This function reads the value of all controls contained in \a ids, and > * returns their values as a ControlList. > @@ -171,10 +172,12 @@ void V4L2Device::close() > * during validation of the requested controls, no control is read and this > * function returns an empty control list. > * > + * If \a request is specified the controls tied to that request are read. > + * > * \return The control values in a ControlList on success, or an empty list on > * error > */ > -ControlList V4L2Device::getControls(Span<const uint32_t> ids) > +ControlList V4L2Device::getControls(Span<const uint32_t> ids, const V4L2Request *request) > { > if (ids.empty()) > return {}; > @@ -242,10 +245,16 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids) > } > > struct v4l2_ext_controls v4l2ExtCtrls = {}; > - v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; > v4l2ExtCtrls.controls = v4l2Ctrls.data(); > v4l2ExtCtrls.count = v4l2Ctrls.size(); > > + if (request) { > + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL; > + v4l2ExtCtrls.request_fd = request->fd(); > + } else { > + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; > + } > + > int ret = ioctl(VIDIOC_G_EXT_CTRLS, &v4l2ExtCtrls); > if (ret) { > unsigned int errorIdx = v4l2ExtCtrls.error_idx; > @@ -273,6 +282,7 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids) > /** > * \brief Write controls to the device > * \param[in] ctrls The list of controls to write > + * \param[in] request And optional request s/And/An/ ? > * > * This function writes the value of all controls contained in \a ctrls, and > * stores the values actually applied to the device in the corresponding > @@ -288,11 +298,15 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids) > * are written and their values are updated in \a ctrls, while all other > * controls are not written and their values are not changed. > * > + * If \a request is set, the controls will be applied to that request. If the > + * device doesn't support requests, -EACCESS will be returned. If \a request is > + * invalid, -EINVAL will be returned. > + * > * \return 0 on success or an error code otherwise > * \retval -EINVAL One of the control is not supported or not accessible s/control/controls/ > * \retval i The index of the control that failed > */ > -int V4L2Device::setControls(ControlList *ctrls) > +int V4L2Device::setControls(ControlList *ctrls, const V4L2Request *request) > { > if (ctrls->empty()) > return 0; > @@ -377,10 +391,16 @@ int V4L2Device::setControls(ControlList *ctrls) > } > > struct v4l2_ext_controls v4l2ExtCtrls = {}; > - v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; > v4l2ExtCtrls.controls = v4l2Ctrls.data(); > v4l2ExtCtrls.count = v4l2Ctrls.size(); > > + if (request) { > + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL; > + v4l2ExtCtrls.request_fd = request->fd(); > + } else { > + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; > + } > + > int ret = ioctl(VIDIOC_S_EXT_CTRLS, &v4l2ExtCtrls); > if (ret) { > unsigned int errorIdx = v4l2ExtCtrls.error_idx; > diff --git a/src/libcamera/v4l2_request.cpp b/src/libcamera/v4l2_request.cpp > new file mode 100644 > index 000000000000..708250d86f61 > --- /dev/null > +++ b/src/libcamera/v4l2_request.cpp > @@ -0,0 +1,128 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas On Board > + * > + * V4L2 Request API > + */ > + > +#include "libcamera/internal/v4l2_request.h" > + > +#include <fcntl.h> > +#include <stdlib.h> > +#include <sys/ioctl.h> > +#include <sys/syscall.h> > +#include <unistd.h> > + > +#include <linux/media.h> > + > +#include <libcamera/base/event_notifier.h> > +#include <libcamera/base/log.h> > + > +/** > + * \file v4l2_request.h > + * \brief V4L2 Request > + */ > + > +namespace libcamera { > + > +LOG_DECLARE_CATEGORY(V4L2) > + > +/** > + * \class V4L2Request > + * \brief V4L2Request object and API > + * > + * The V4L2Request class wraps a V4L2 request fd and provides some convenience > + * functions to handle request. > + * > + * It is usually constructed by calling \a MediaDevice::allocateRequests(). > + * > + * A request can then be passed to the V4L2Device::setControls(), > + * V4L2Device::getControls() and V4L2VideoDevice::queueBuffer(). > + */ > + > +/** > + * \brief Construct a V4L2Request > + * \param[in] fd The request fd > + */ > +V4L2Request::V4L2Request(int fd) > + : fd_(fd), fdNotifier_(fd, EventNotifier::Exception) > +{ > + if (!fd_.isValid()) > + return; > + > + fdNotifier_.activated.connect(this, &V4L2Request::requestReady); > + fdNotifier_.setEnabled(false); > +} > + > +/** > + * \fn V4L2Request::isValid() > + * \brief Check if the request is valid > + * > + * Checks if the underlying fd is valid. > + * > + * \return True if the request is valid, false otherwise > + */ > + > +/** > + * \fn V4L2Request::fd() > + * \brief Get the file descriptor > + * > + * \return The file descriptor wrapped by this V4L2Request > + */ > + > +/** > + * \var V4L2Request::requestDone > + * \brief Signal that is emitted when the request is done > + */ > + > +/** > + * \brief Reinit the request > + * > + * Calls MEDIA_REQUEST_IOC_REINIT om the request fd. > + * > + * \return 0 on success or a negative error code otherwise > + */ > +int V4L2Request::reinit() > +{ > + fdNotifier_.setEnabled(false); > + > + if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_REINIT) < 0) > + return -errno; > + > + return 0; > +} > + > +/** > + * \brief Reinit the request > + * > + * Calls MEDIA_REQUEST_IOC_QUEUE om the request fd. > + * > + * \return 0 on success or a negative error code otherwise > + */ > +int V4L2Request::queue() > +{ > + if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_QUEUE) < 0) > + return -errno; > + > + fdNotifier_.setEnabled(true); > + > + return 0; > +} > + > +std::string V4L2Request::logPrefix() const > +{ > + return "Request [" + std::to_string(fd()) + "]"; > +} > + > +/** > + * \brief Slot to handle request done events > + * > + * When this slot is called, the request is done and the requestDone will be > + * emitted. > + */ > +void V4L2Request::requestReady() > +{ > + requestDone.emit(this); > +} > + > +} /* namespace libcamera */ > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index bb57c1b76a5b..8ce739f4bc65 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -30,6 +30,7 @@ > #include "libcamera/internal/framebuffer.h" > #include "libcamera/internal/media_device.h" > #include "libcamera/internal/media_object.h" > +#include "libcamera/internal/v4l2_request.h" > > /** > * \file v4l2_videodevice.h > @@ -1629,6 +1630,7 @@ int V4L2VideoDevice::releaseBuffers() > /** > * \brief Queue a buffer to the video device > * \param[in] buffer The buffer to be queued > + * \param[in] request An optional request > * > * For capture video devices the \a buffer will be filled with data by the > * device. For output video devices the \a buffer shall contain valid data and > @@ -1641,9 +1643,11 @@ int V4L2VideoDevice::releaseBuffers() > * Note that queueBuffer() will fail if the device is in the process of being > * stopped from a streaming state through streamOff(). > * > + * If \a request is specified, the buffer will be tied to that request. > + * > * \return 0 on success or a negative error code otherwise > */ > -int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) > +int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer, const V4L2Request *request) > { Best wishes, Isaac > struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {}; > struct v4l2_buffer buf = {}; > @@ -1674,6 +1678,10 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) > buf.type = bufferType_; > buf.memory = memoryType_; > buf.field = V4L2_FIELD_NONE; > + if (request) { > + buf.flags = V4L2_BUF_FLAG_REQUEST_FD; > + buf.request_fd = request->fd(); > + } > > bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); > Span<const FrameBuffer::Plane> planes = buffer->planes(); > -- > 2.48.1 >
Quoting Stefan Klug (2025-10-23 23:48:06) > The V4L2 requests API provides support to atomically tie controls to a > set of buffers. This is especially common for m2m devices. Such a > request is represented by a fd that is allocated vi s/ a / an / s/vi/via/ > MEDIA_IOC_REQUEST_ALLOC and then passed to various V4L2 function. s/function/functions/ > > Implement a V4L2Request class to wrap such an fd and add the > corresponding utility functions. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > --- > > Changes in v2: > - Added documentation > --- > include/libcamera/internal/media_device.h | 7 + > include/libcamera/internal/meson.build | 1 + > include/libcamera/internal/v4l2_device.h | 5 +- > include/libcamera/internal/v4l2_request.h | 49 +++++++ > include/libcamera/internal/v4l2_videodevice.h | 3 +- > src/libcamera/media_device.cpp | 47 +++++++ > src/libcamera/meson.build | 1 + > src/libcamera/v4l2_device.cpp | 28 +++- > src/libcamera/v4l2_request.cpp | 128 ++++++++++++++++++ > src/libcamera/v4l2_videodevice.cpp | 10 +- > 10 files changed, 271 insertions(+), 8 deletions(-) > create mode 100644 include/libcamera/internal/v4l2_request.h > create mode 100644 src/libcamera/v4l2_request.cpp > > diff --git a/include/libcamera/internal/media_device.h b/include/libcamera/internal/media_device.h > index b3a48b98d64b..74cb9ba1542d 100644 > --- a/include/libcamera/internal/media_device.h > +++ b/include/libcamera/internal/media_device.h > @@ -18,6 +18,7 @@ > #include <libcamera/base/unique_fd.h> > > #include "libcamera/internal/media_object.h" > +#include "libcamera/internal/v4l2_request.h" > > namespace libcamera { > > @@ -57,6 +58,11 @@ public: > > std::vector<MediaEntity *> locateEntities(unsigned int function); > > + int allocateRequests(unsigned int count, > + std::vector<std::unique_ptr<V4L2Request>> *requests); > + > + bool supportsRequests(); > + > protected: > std::string logPrefix() const override; > > @@ -87,6 +93,7 @@ private: > UniqueFD fd_; > bool valid_; > bool acquired_; > + std::optional<bool> supportsRequests_; > > std::map<unsigned int, MediaObject *> objects_; > std::vector<MediaEntity *> entities_; > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > index 45c299f6a332..e9540a2f734f 100644 > --- a/include/libcamera/internal/meson.build > +++ b/include/libcamera/internal/meson.build > @@ -44,6 +44,7 @@ libcamera_internal_headers = files([ > 'sysfs.h', > 'v4l2_device.h', > 'v4l2_pixelformat.h', > + 'v4l2_request.h', > 'v4l2_subdevice.h', > 'v4l2_videodevice.h', > 'vector.h', > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h > index 5bc9da96677d..dbbd118abd00 100644 > --- a/include/libcamera/internal/v4l2_device.h > +++ b/include/libcamera/internal/v4l2_device.h > @@ -24,6 +24,7 @@ > #include <libcamera/controls.h> > > #include "libcamera/internal/formats.h" > +#include "libcamera/internal/v4l2_request.h" > > namespace libcamera { > > @@ -37,8 +38,8 @@ public: > > const ControlInfoMap &controls() const { return controls_; } > > - ControlList getControls(Span<const uint32_t> ids); > - int setControls(ControlList *ctrls); > + ControlList getControls(Span<const uint32_t> ids, const V4L2Request *request = nullptr); > + int setControls(ControlList *ctrls, const V4L2Request *request = nullptr); > > const struct v4l2_query_ext_ctrl *controlInfo(uint32_t id) const; > > diff --git a/include/libcamera/internal/v4l2_request.h b/include/libcamera/internal/v4l2_request.h > new file mode 100644 > index 000000000000..bf1bea3261af > --- /dev/null > +++ b/include/libcamera/internal/v4l2_request.h > @@ -0,0 +1,49 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas On Board > + * > + * V4L2 requests > + */ > + > +#pragma once > + > +#include <string> > + > +#include <linux/videodev2.h> > + > +#include <libcamera/base/event_notifier.h> > +#include <libcamera/base/log.h> > +#include <libcamera/base/signal.h> > +#include <libcamera/base/span.h> > +#include <libcamera/base/unique_fd.h> > + > +#include <libcamera/color_space.h> > +#include <libcamera/controls.h> Do we need these two headers? > + > +namespace libcamera { > + > +class V4L2Request : protected Loggable > +{ > +public: > + bool isValid() const { return fd_.isValid(); } > + int fd() const { return fd_.get(); } > + > + int reinit(); > + int queue(); > + > + V4L2Request(int fd = -1); > + ~V4L2Request() = default; > + > + Signal<V4L2Request *> requestDone; > + > +private: > + LIBCAMERA_DISABLE_COPY_AND_MOVE(V4L2Request) > + > + void requestReady(); > + std::string logPrefix() const override; > + > + UniqueFD fd_; > + EventNotifier fdNotifier_; > +}; > + > +} /* namespace libcamera */ > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h > index 5a7dcfdda118..2d290971a0ee 100644 > --- a/include/libcamera/internal/v4l2_videodevice.h > +++ b/include/libcamera/internal/v4l2_videodevice.h > @@ -33,6 +33,7 @@ > #include "libcamera/internal/formats.h" > #include "libcamera/internal/v4l2_device.h" > #include "libcamera/internal/v4l2_pixelformat.h" > +#include "libcamera/internal/v4l2_request.h" > > namespace libcamera { > > @@ -217,7 +218,7 @@ public: > int importBuffers(unsigned int count); > int releaseBuffers(); > > - int queueBuffer(FrameBuffer *buffer); > + int queueBuffer(FrameBuffer *buffer, const V4L2Request *request = nullptr); > Signal<FrameBuffer *> bufferReady; > > int streamOn(); > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp > index 353f34a81eca..673c53fefdd7 100644 > --- a/src/libcamera/media_device.cpp > +++ b/src/libcamera/media_device.cpp > @@ -20,6 +20,7 @@ > #include <linux/media.h> > > #include <libcamera/base/log.h> > +#include "libcamera/internal/v4l2_request.h" > > /** > * \file media_device.h > @@ -851,4 +852,50 @@ std::vector<MediaEntity *> MediaDevice::locateEntities(unsigned int function) > return found; > } > > +/** > + * \brief Allocate requests > + * \param[in] count Number of requests to allocate > + * \param[out] requests Vector to store allocated requests > + * > + * Allocates and stores \a count requests in \a requests. If allocation fails, > + * and error is returned and \a requests is cleared. s/\* and/\* an/ > + * > + * \return 0 on success or a negative error code otherwise > + */ > +int MediaDevice::allocateRequests(unsigned int count, > + std::vector<std::unique_ptr<V4L2Request>> *requests) > +{ > + requests->resize(count); > + for (unsigned int i = 0; i < count; i++) { > + int requestFd; > + int ret = ::ioctl(fd_.get(), MEDIA_IOC_REQUEST_ALLOC, &requestFd); I'm of the opinion that this should be moved into V4L2Request constructor (or init()) so that we can keep all requests handling there. We already have an isValid(), and maybe we can add close() to the deconstrctor? > + if (ret < 0) { > + requests->clear(); > + return -errno; > + } > + (*requests)[i] = std::make_unique<V4L2Request>(requestFd); > + } > + > + return 0; > +} > + > +/** > + * \brief Check if requests are supported > + * > + * Checks if the device supports V4L2 requests by trying to allocate a single > + * request. The result is cached, so the allocation is only tried once. > + * > + * \return True if the device supports requests, false otherwise > + */ > +bool MediaDevice::supportsRequests() > +{ > + if (supportsRequests_.has_value()) > + return supportsRequests_.value(); > + > + std::vector<std::unique_ptr<V4L2Request>> requests; > + supportsRequests_ = (allocateRequests(1, &requests) == 0); Initially I was worried about subsequent allocations but then I realized that this will get deallocated after it goes out of scope. Although there is currently no other errno than ENOTTY, the non-explicit check makes me uncomfortable :S > + > + return supportsRequests_.value(); > +} > + > } /* namespace libcamera */ > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index 5b9b86f211f1..34e20f557514 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -54,6 +54,7 @@ libcamera_internal_sources = files([ > 'sysfs.cpp', > 'v4l2_device.cpp', > 'v4l2_pixelformat.cpp', > + 'v4l2_request.cpp', > 'v4l2_subdevice.cpp', > 'v4l2_videodevice.cpp', > 'vector.cpp', > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index 8c78b8c424e5..7a669a0303c1 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -162,6 +162,7 @@ void V4L2Device::close() > /** > * \brief Read controls from the device > * \param[in] ids The list of controls to read, specified by their ID > + * \param[in] request An optional request > * > * This function reads the value of all controls contained in \a ids, and > * returns their values as a ControlList. > @@ -171,10 +172,12 @@ void V4L2Device::close() > * during validation of the requested controls, no control is read and this > * function returns an empty control list. > * > + * If \a request is specified the controls tied to that request are read. > + * > * \return The control values in a ControlList on success, or an empty list on > * error > */ > -ControlList V4L2Device::getControls(Span<const uint32_t> ids) > +ControlList V4L2Device::getControls(Span<const uint32_t> ids, const V4L2Request *request) > { > if (ids.empty()) > return {}; > @@ -242,10 +245,16 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids) > } > > struct v4l2_ext_controls v4l2ExtCtrls = {}; > - v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; > v4l2ExtCtrls.controls = v4l2Ctrls.data(); > v4l2ExtCtrls.count = v4l2Ctrls.size(); > > + if (request) { > + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL; > + v4l2ExtCtrls.request_fd = request->fd(); > + } else { > + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; > + } > + > int ret = ioctl(VIDIOC_G_EXT_CTRLS, &v4l2ExtCtrls); > if (ret) { > unsigned int errorIdx = v4l2ExtCtrls.error_idx; > @@ -273,6 +282,7 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids) > /** > * \brief Write controls to the device > * \param[in] ctrls The list of controls to write > + * \param[in] request And optional request s/And/An/ > * > * This function writes the value of all controls contained in \a ctrls, and > * stores the values actually applied to the device in the corresponding > @@ -288,11 +298,15 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids) > * are written and their values are updated in \a ctrls, while all other > * controls are not written and their values are not changed. > * > + * If \a request is set, the controls will be applied to that request. If the > + * device doesn't support requests, -EACCESS will be returned. If \a request is > + * invalid, -EINVAL will be returned. Do you need to mention EACCESS in the docunentation for read as well? > + * > * \return 0 on success or an error code otherwise > * \retval -EINVAL One of the control is not supported or not accessible > * \retval i The index of the control that failed > */ > -int V4L2Device::setControls(ControlList *ctrls) > +int V4L2Device::setControls(ControlList *ctrls, const V4L2Request *request) > { > if (ctrls->empty()) > return 0; > @@ -377,10 +391,16 @@ int V4L2Device::setControls(ControlList *ctrls) > } > > struct v4l2_ext_controls v4l2ExtCtrls = {}; > - v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; > v4l2ExtCtrls.controls = v4l2Ctrls.data(); > v4l2ExtCtrls.count = v4l2Ctrls.size(); > > + if (request) { > + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL; > + v4l2ExtCtrls.request_fd = request->fd(); > + } else { > + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; > + } > + > int ret = ioctl(VIDIOC_S_EXT_CTRLS, &v4l2ExtCtrls); > if (ret) { > unsigned int errorIdx = v4l2ExtCtrls.error_idx; > diff --git a/src/libcamera/v4l2_request.cpp b/src/libcamera/v4l2_request.cpp > new file mode 100644 > index 000000000000..708250d86f61 > --- /dev/null > +++ b/src/libcamera/v4l2_request.cpp > @@ -0,0 +1,128 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas On Board > + * > + * V4L2 Request API > + */ > + > +#include "libcamera/internal/v4l2_request.h" > + > +#include <fcntl.h> > +#include <stdlib.h> > +#include <sys/ioctl.h> > +#include <sys/syscall.h> > +#include <unistd.h> > + > +#include <linux/media.h> > + > +#include <libcamera/base/event_notifier.h> > +#include <libcamera/base/log.h> > + > +/** > + * \file v4l2_request.h > + * \brief V4L2 Request > + */ > + > +namespace libcamera { > + > +LOG_DECLARE_CATEGORY(V4L2) > + > +/** > + * \class V4L2Request > + * \brief V4L2Request object and API > + * > + * The V4L2Request class wraps a V4L2 request fd and provides some convenience > + * functions to handle request. > + * > + * It is usually constructed by calling \a MediaDevice::allocateRequests(). > + * > + * A request can then be passed to the V4L2Device::setControls(), > + * V4L2Device::getControls() and V4L2VideoDevice::queueBuffer(). > + */ > + > +/** > + * \brief Construct a V4L2Request > + * \param[in] fd The request fd > + */ > +V4L2Request::V4L2Request(int fd) > + : fd_(fd), fdNotifier_(fd, EventNotifier::Exception) > +{ > + if (!fd_.isValid()) > + return; > + > + fdNotifier_.activated.connect(this, &V4L2Request::requestReady); > + fdNotifier_.setEnabled(false); > +} > + > +/** > + * \fn V4L2Request::isValid() > + * \brief Check if the request is valid > + * > + * Checks if the underlying fd is valid. > + * > + * \return True if the request is valid, false otherwise > + */ > + > +/** > + * \fn V4L2Request::fd() > + * \brief Get the file descriptor > + * > + * \return The file descriptor wrapped by this V4L2Request > + */ > + > +/** > + * \var V4L2Request::requestDone > + * \brief Signal that is emitted when the request is done > + */ > + > +/** > + * \brief Reinit the request > + * > + * Calls MEDIA_REQUEST_IOC_REINIT om the request fd. s/om/on/ > + * > + * \return 0 on success or a negative error code otherwise > + */ > +int V4L2Request::reinit() > +{ > + fdNotifier_.setEnabled(false); > + > + if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_REINIT) < 0) > + return -errno; > + > + return 0; > +} > + > +/** > + * \brief Reinit the request Reinit? :) > + * > + * Calls MEDIA_REQUEST_IOC_QUEUE om the request fd. s/om/on/ Thanks, Paul > + * > + * \return 0 on success or a negative error code otherwise > + */ > +int V4L2Request::queue() > +{ > + if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_QUEUE) < 0) > + return -errno; > + > + fdNotifier_.setEnabled(true); > + > + return 0; > +} > + > +std::string V4L2Request::logPrefix() const > +{ > + return "Request [" + std::to_string(fd()) + "]"; > +} > + > +/** > + * \brief Slot to handle request done events > + * > + * When this slot is called, the request is done and the requestDone will be > + * emitted. > + */ > +void V4L2Request::requestReady() > +{ > + requestDone.emit(this); > +} > + > +} /* namespace libcamera */ > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index bb57c1b76a5b..8ce739f4bc65 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -30,6 +30,7 @@ > #include "libcamera/internal/framebuffer.h" > #include "libcamera/internal/media_device.h" > #include "libcamera/internal/media_object.h" > +#include "libcamera/internal/v4l2_request.h" > > /** > * \file v4l2_videodevice.h > @@ -1629,6 +1630,7 @@ int V4L2VideoDevice::releaseBuffers() > /** > * \brief Queue a buffer to the video device > * \param[in] buffer The buffer to be queued > + * \param[in] request An optional request > * > * For capture video devices the \a buffer will be filled with data by the > * device. For output video devices the \a buffer shall contain valid data and > @@ -1641,9 +1643,11 @@ int V4L2VideoDevice::releaseBuffers() > * Note that queueBuffer() will fail if the device is in the process of being > * stopped from a streaming state through streamOff(). > * > + * If \a request is specified, the buffer will be tied to that request. > + * > * \return 0 on success or a negative error code otherwise > */ > -int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) > +int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer, const V4L2Request *request) > { > struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {}; > struct v4l2_buffer buf = {}; > @@ -1674,6 +1678,10 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) > buf.type = bufferType_; > buf.memory = memoryType_; > buf.field = V4L2_FIELD_NONE; > + if (request) { > + buf.flags = V4L2_BUF_FLAG_REQUEST_FD; > + buf.request_fd = request->fd(); > + } > > bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); > Span<const FrameBuffer::Plane> planes = buffer->planes(); > -- > 2.48.1 >
Hi Barnabás, Thank you for the review. As always I should have spent more time reading the review earlier and not just before prearing the new series... Quoting Barnabás Pőcze (2025-10-24 16:00:32) > Hi > > 2025. 10. 23. 16:48 keltezéssel, Stefan Klug írta: > > The V4L2 requests API provides support to atomically tie controls to a > > set of buffers. This is especially common for m2m devices. Such a > > request is represented by a fd that is allocated vi > > MEDIA_IOC_REQUEST_ALLOC and then passed to various V4L2 function. > > > > Implement a V4L2Request class to wrap such an fd and add the > > corresponding utility functions. > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > > --- > > > > Changes in v2: > > - Added documentation > > --- > > include/libcamera/internal/media_device.h | 7 + > > include/libcamera/internal/meson.build | 1 + > > include/libcamera/internal/v4l2_device.h | 5 +- > > include/libcamera/internal/v4l2_request.h | 49 +++++++ > > include/libcamera/internal/v4l2_videodevice.h | 3 +- > > src/libcamera/media_device.cpp | 47 +++++++ > > src/libcamera/meson.build | 1 + > > src/libcamera/v4l2_device.cpp | 28 +++- > > src/libcamera/v4l2_request.cpp | 128 ++++++++++++++++++ > > src/libcamera/v4l2_videodevice.cpp | 10 +- > > 10 files changed, 271 insertions(+), 8 deletions(-) > > create mode 100644 include/libcamera/internal/v4l2_request.h > > create mode 100644 src/libcamera/v4l2_request.cpp > > > > [...] > > diff --git a/include/libcamera/internal/v4l2_request.h b/include/libcamera/internal/v4l2_request.h > > new file mode 100644 > > index 000000000000..bf1bea3261af > > --- /dev/null > > +++ b/include/libcamera/internal/v4l2_request.h > > @@ -0,0 +1,49 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2025, Ideas On Board > > + * > > + * V4L2 requests > > + */ > > + > > +#pragma once > > + > > +#include <string> > > + > > +#include <linux/videodev2.h> > > + > > +#include <libcamera/base/event_notifier.h> > > +#include <libcamera/base/log.h> > > +#include <libcamera/base/signal.h> > > +#include <libcamera/base/span.h> > > +#include <libcamera/base/unique_fd.h> > > + > > +#include <libcamera/color_space.h> > > +#include <libcamera/controls.h> > > + > > +namespace libcamera { > > + > > +class V4L2Request : protected Loggable > > +{ > > +public: > > + bool isValid() const { return fd_.isValid(); } > > + int fd() const { return fd_.get(); } > > + > > + int reinit(); > > + int queue(); > > + > > + V4L2Request(int fd = -1); > > I think just taking `UniqueFD` would be better as it makes the ownership clear. > > Is it is useful to create a `V4L2Request` object without a valid file descriptor? > There is no way to change the file descriptor and as far as I can tell most methods > would just fail. Sure, I fixed that. > > > > + ~V4L2Request() = default; > > I think this is already implied. > > > > + > > + Signal<V4L2Request *> requestDone; > > + > > +private: > > + LIBCAMERA_DISABLE_COPY_AND_MOVE(V4L2Request) > > + > > + void requestReady(); > > + std::string logPrefix() const override; > > + > > + UniqueFD fd_; > > + EventNotifier fdNotifier_; > > +}; > > + > > +} /* namespace libcamera */ > > [...] > > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp > > index 353f34a81eca..673c53fefdd7 100644 > > --- a/src/libcamera/media_device.cpp > > +++ b/src/libcamera/media_device.cpp > > @@ -20,6 +20,7 @@ > > #include <linux/media.h> > > > > #include <libcamera/base/log.h> > > +#include "libcamera/internal/v4l2_request.h" > > > > /** > > * \file media_device.h > > @@ -851,4 +852,50 @@ std::vector<MediaEntity *> MediaDevice::locateEntities(unsigned int function) > > return found; > > } > > > > +/** > > + * \brief Allocate requests > > + * \param[in] count Number of requests to allocate > > + * \param[out] requests Vector to store allocated requests > > + * > > + * Allocates and stores \a count requests in \a requests. If allocation fails, > > + * and error is returned and \a requests is cleared. > > + * > > + * \return 0 on success or a negative error code otherwise > > + */ > > +int MediaDevice::allocateRequests(unsigned int count, > > + std::vector<std::unique_ptr<V4L2Request>> *requests) > > +{ > > + requests->resize(count); > > + for (unsigned int i = 0; i < count; i++) { > > + int requestFd; > > + int ret = ::ioctl(fd_.get(), MEDIA_IOC_REQUEST_ALLOC, &requestFd); > > + if (ret < 0) { > > + requests->clear(); > > + return -errno; > > + } > > + (*requests)[i] = std::make_unique<V4L2Request>(requestFd); > > + } > > + > > + return 0; > > +} > > + > > +/** > > + * \brief Check if requests are supported > > + * > > + * Checks if the device supports V4L2 requests by trying to allocate a single > > + * request. The result is cached, so the allocation is only tried once. > > + * > > + * \return True if the device supports requests, false otherwise > > + */ > > +bool MediaDevice::supportsRequests() > > +{ > > + if (supportsRequests_.has_value()) > > + return supportsRequests_.value(); > > + > > + std::vector<std::unique_ptr<V4L2Request>> requests; > > + supportsRequests_ = (allocateRequests(1, &requests) == 0); > > Shouldn't it technically be something like ` != -ENOTTY` ? Hm, good question. But can you really be sure that it supports requests if a error != -ENOTTY is returned? I left it as is for now, to be on the safe side. > > > > + > > + return supportsRequests_.value(); > > +} > > + > > } /* namespace libcamera */ > > [...] > > diff --git a/src/libcamera/v4l2_request.cpp b/src/libcamera/v4l2_request.cpp > > new file mode 100644 > > index 000000000000..708250d86f61 > > --- /dev/null > > +++ b/src/libcamera/v4l2_request.cpp > > @@ -0,0 +1,128 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2025, Ideas On Board > > + * > > + * V4L2 Request API > > + */ > > + > > +#include "libcamera/internal/v4l2_request.h" > > + > > +#include <fcntl.h> > > +#include <stdlib.h> > > +#include <sys/ioctl.h> > > +#include <sys/syscall.h> > > +#include <unistd.h> > > + > > +#include <linux/media.h> > > + > > +#include <libcamera/base/event_notifier.h> > > +#include <libcamera/base/log.h> > > + > > +/** > > + * \file v4l2_request.h > > + * \brief V4L2 Request > > + */ > > + > > +namespace libcamera { > > + > > +LOG_DECLARE_CATEGORY(V4L2) > > + > > +/** > > + * \class V4L2Request > > + * \brief V4L2Request object and API > > + * > > + * The V4L2Request class wraps a V4L2 request fd and provides some convenience > > + * functions to handle request. > > + * > > + * It is usually constructed by calling \a MediaDevice::allocateRequests(). > > + * > > + * A request can then be passed to the V4L2Device::setControls(), > > + * V4L2Device::getControls() and V4L2VideoDevice::queueBuffer(). > > + */ > > + > > +/** > > + * \brief Construct a V4L2Request > > + * \param[in] fd The request fd > > + */ > > +V4L2Request::V4L2Request(int fd) > > + : fd_(fd), fdNotifier_(fd, EventNotifier::Exception) > > +{ > > + if (!fd_.isValid()) > > + return; > > + > > + fdNotifier_.activated.connect(this, &V4L2Request::requestReady); > > It's probably just me, but I feel like I would just use a simple lambda here. I'd prefer a lambda, too. I didn't get it to work though. The code I'd expect there would be something like: fdNotifier_.activated.connect([this] { this->requestDone.emit(this); }); But that doesn't work as capturing lambdas can't be converted to a function pointer. I didn't dig deeper into the Signal implementation. How would you do that? Best regards, Stefan > > > > + fdNotifier_.setEnabled(false); > > +} > > [...] > > > Regards, > Barnabás Pőcze >
Hi Isaac, Quoting Isaac Scott (2025-10-27 16:11:00) > Hi Stefan, > > Thank you for the patch! > > Some nitpicks below: Thank you for the review and the nitpicks. I fixed them for v3. Cheers, Stefan > > Quoting Stefan Klug (2025-10-23 15:48:06) > > The V4L2 requests API provides support to atomically tie controls to a > > set of buffers. This is especially common for m2m devices. Such a > > request is represented by a fd that is allocated vi > > s/vi/via/ > > > MEDIA_IOC_REQUEST_ALLOC and then passed to various V4L2 function. > > > > Implement a V4L2Request class to wrap such an fd and add the > > corresponding utility functions. > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > > --- > > > > Changes in v2: > > - Added documentation > > --- > > include/libcamera/internal/media_device.h | 7 + > > include/libcamera/internal/meson.build | 1 + > > include/libcamera/internal/v4l2_device.h | 5 +- > > include/libcamera/internal/v4l2_request.h | 49 +++++++ > > include/libcamera/internal/v4l2_videodevice.h | 3 +- > > src/libcamera/media_device.cpp | 47 +++++++ > > src/libcamera/meson.build | 1 + > > src/libcamera/v4l2_device.cpp | 28 +++- > > src/libcamera/v4l2_request.cpp | 128 ++++++++++++++++++ > > src/libcamera/v4l2_videodevice.cpp | 10 +- > > 10 files changed, 271 insertions(+), 8 deletions(-) > > create mode 100644 include/libcamera/internal/v4l2_request.h > > create mode 100644 src/libcamera/v4l2_request.cpp > > > > diff --git a/include/libcamera/internal/media_device.h b/include/libcamera/internal/media_device.h > > index b3a48b98d64b..74cb9ba1542d 100644 > > --- a/include/libcamera/internal/media_device.h > > +++ b/include/libcamera/internal/media_device.h > > @@ -18,6 +18,7 @@ > > #include <libcamera/base/unique_fd.h> > > > > #include "libcamera/internal/media_object.h" > > +#include "libcamera/internal/v4l2_request.h" > > > > namespace libcamera { > > > > @@ -57,6 +58,11 @@ public: > > > > std::vector<MediaEntity *> locateEntities(unsigned int function); > > > > + int allocateRequests(unsigned int count, > > + std::vector<std::unique_ptr<V4L2Request>> *requests); > > + > > + bool supportsRequests(); > > + > > protected: > > std::string logPrefix() const override; > > > > @@ -87,6 +93,7 @@ private: > > UniqueFD fd_; > > bool valid_; > > bool acquired_; > > + std::optional<bool> supportsRequests_; > > > > std::map<unsigned int, MediaObject *> objects_; > > std::vector<MediaEntity *> entities_; > > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > > index 45c299f6a332..e9540a2f734f 100644 > > --- a/include/libcamera/internal/meson.build > > +++ b/include/libcamera/internal/meson.build > > @@ -44,6 +44,7 @@ libcamera_internal_headers = files([ > > 'sysfs.h', > > 'v4l2_device.h', > > 'v4l2_pixelformat.h', > > + 'v4l2_request.h', > > 'v4l2_subdevice.h', > > 'v4l2_videodevice.h', > > 'vector.h', > > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h > > index 5bc9da96677d..dbbd118abd00 100644 > > --- a/include/libcamera/internal/v4l2_device.h > > +++ b/include/libcamera/internal/v4l2_device.h > > @@ -24,6 +24,7 @@ > > #include <libcamera/controls.h> > > > > #include "libcamera/internal/formats.h" > > +#include "libcamera/internal/v4l2_request.h" > > > > namespace libcamera { > > > > @@ -37,8 +38,8 @@ public: > > > > const ControlInfoMap &controls() const { return controls_; } > > > > - ControlList getControls(Span<const uint32_t> ids); > > - int setControls(ControlList *ctrls); > > + ControlList getControls(Span<const uint32_t> ids, const V4L2Request *request = nullptr); > > + int setControls(ControlList *ctrls, const V4L2Request *request = nullptr); > > > > const struct v4l2_query_ext_ctrl *controlInfo(uint32_t id) const; > > > > diff --git a/include/libcamera/internal/v4l2_request.h b/include/libcamera/internal/v4l2_request.h > > new file mode 100644 > > index 000000000000..bf1bea3261af > > --- /dev/null > > +++ b/include/libcamera/internal/v4l2_request.h > > @@ -0,0 +1,49 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2025, Ideas On Board > > + * > > + * V4L2 requests > > + */ > > + > > +#pragma once > > + > > +#include <string> > > + > > +#include <linux/videodev2.h> > > + > > +#include <libcamera/base/event_notifier.h> > > +#include <libcamera/base/log.h> > > +#include <libcamera/base/signal.h> > > +#include <libcamera/base/span.h> > > +#include <libcamera/base/unique_fd.h> > > + > > +#include <libcamera/color_space.h> > > +#include <libcamera/controls.h> > > + > > +namespace libcamera { > > + > > +class V4L2Request : protected Loggable > > +{ > > +public: > > + bool isValid() const { return fd_.isValid(); } > > + int fd() const { return fd_.get(); } > > + > > + int reinit(); > > + int queue(); > > + > > + V4L2Request(int fd = -1); > > + ~V4L2Request() = default; > > + > > + Signal<V4L2Request *> requestDone; > > + > > +private: > > + LIBCAMERA_DISABLE_COPY_AND_MOVE(V4L2Request) > > + > > + void requestReady(); > > + std::string logPrefix() const override; > > + > > + UniqueFD fd_; > > + EventNotifier fdNotifier_; > > +}; > > + > > +} /* namespace libcamera */ > > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h > > index 5a7dcfdda118..2d290971a0ee 100644 > > --- a/include/libcamera/internal/v4l2_videodevice.h > > +++ b/include/libcamera/internal/v4l2_videodevice.h > > @@ -33,6 +33,7 @@ > > #include "libcamera/internal/formats.h" > > #include "libcamera/internal/v4l2_device.h" > > #include "libcamera/internal/v4l2_pixelformat.h" > > +#include "libcamera/internal/v4l2_request.h" > > > > namespace libcamera { > > > > @@ -217,7 +218,7 @@ public: > > int importBuffers(unsigned int count); > > int releaseBuffers(); > > > > - int queueBuffer(FrameBuffer *buffer); > > + int queueBuffer(FrameBuffer *buffer, const V4L2Request *request = nullptr); > > Signal<FrameBuffer *> bufferReady; > > > > int streamOn(); > > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp > > index 353f34a81eca..673c53fefdd7 100644 > > --- a/src/libcamera/media_device.cpp > > +++ b/src/libcamera/media_device.cpp > > @@ -20,6 +20,7 @@ > > #include <linux/media.h> > > > > #include <libcamera/base/log.h> > > +#include "libcamera/internal/v4l2_request.h" > > > > /** > > * \file media_device.h > > @@ -851,4 +852,50 @@ std::vector<MediaEntity *> MediaDevice::locateEntities(unsigned int function) > > return found; > > } > > > > +/** > > + * \brief Allocate requests > > + * \param[in] count Number of requests to allocate > > + * \param[out] requests Vector to store allocated requests > > + * > > + * Allocates and stores \a count requests in \a requests. If allocation fails, > > + * and error is returned and \a requests is cleared. > > + * > > + * \return 0 on success or a negative error code otherwise > > + */ > > +int MediaDevice::allocateRequests(unsigned int count, > > + std::vector<std::unique_ptr<V4L2Request>> *requests) > > +{ > > + requests->resize(count); > > + for (unsigned int i = 0; i < count; i++) { > > + int requestFd; > > + int ret = ::ioctl(fd_.get(), MEDIA_IOC_REQUEST_ALLOC, &requestFd); > > + if (ret < 0) { > > + requests->clear(); > > + return -errno; > > + } > > + (*requests)[i] = std::make_unique<V4L2Request>(requestFd); > > + } > > + > > + return 0; > > +} > > + > > +/** > > + * \brief Check if requests are supported > > + * > > + * Checks if the device supports V4L2 requests by trying to allocate a single > > + * request. The result is cached, so the allocation is only tried once. > > + * > > + * \return True if the device supports requests, false otherwise > > + */ > > +bool MediaDevice::supportsRequests() > > +{ > > + if (supportsRequests_.has_value()) > > + return supportsRequests_.value(); > > + > > + std::vector<std::unique_ptr<V4L2Request>> requests; > > + supportsRequests_ = (allocateRequests(1, &requests) == 0); > > + > > + return supportsRequests_.value(); > > +} > > + > > } /* namespace libcamera */ > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > index 5b9b86f211f1..34e20f557514 100644 > > --- a/src/libcamera/meson.build > > +++ b/src/libcamera/meson.build > > @@ -54,6 +54,7 @@ libcamera_internal_sources = files([ > > 'sysfs.cpp', > > 'v4l2_device.cpp', > > 'v4l2_pixelformat.cpp', > > + 'v4l2_request.cpp', > > 'v4l2_subdevice.cpp', > > 'v4l2_videodevice.cpp', > > 'vector.cpp', > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > > index 8c78b8c424e5..7a669a0303c1 100644 > > --- a/src/libcamera/v4l2_device.cpp > > +++ b/src/libcamera/v4l2_device.cpp > > @@ -162,6 +162,7 @@ void V4L2Device::close() > > /** > > * \brief Read controls from the device > > * \param[in] ids The list of controls to read, specified by their ID > > + * \param[in] request An optional request > > * > > * This function reads the value of all controls contained in \a ids, and > > * returns their values as a ControlList. > > @@ -171,10 +172,12 @@ void V4L2Device::close() > > * during validation of the requested controls, no control is read and this > > * function returns an empty control list. > > * > > + * If \a request is specified the controls tied to that request are read. > > + * > > * \return The control values in a ControlList on success, or an empty list on > > * error > > */ > > -ControlList V4L2Device::getControls(Span<const uint32_t> ids) > > +ControlList V4L2Device::getControls(Span<const uint32_t> ids, const V4L2Request *request) > > { > > if (ids.empty()) > > return {}; > > @@ -242,10 +245,16 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids) > > } > > > > struct v4l2_ext_controls v4l2ExtCtrls = {}; > > - v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; > > v4l2ExtCtrls.controls = v4l2Ctrls.data(); > > v4l2ExtCtrls.count = v4l2Ctrls.size(); > > > > + if (request) { > > + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL; > > + v4l2ExtCtrls.request_fd = request->fd(); > > + } else { > > + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; > > + } > > + > > int ret = ioctl(VIDIOC_G_EXT_CTRLS, &v4l2ExtCtrls); > > if (ret) { > > unsigned int errorIdx = v4l2ExtCtrls.error_idx; > > @@ -273,6 +282,7 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids) > > /** > > * \brief Write controls to the device > > * \param[in] ctrls The list of controls to write > > + * \param[in] request And optional request > > s/And/An/ ? > > > * > > * This function writes the value of all controls contained in \a ctrls, and > > * stores the values actually applied to the device in the corresponding > > @@ -288,11 +298,15 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids) > > * are written and their values are updated in \a ctrls, while all other > > * controls are not written and their values are not changed. > > * > > + * If \a request is set, the controls will be applied to that request. If the > > + * device doesn't support requests, -EACCESS will be returned. If \a request is > > + * invalid, -EINVAL will be returned. > > + * > > * \return 0 on success or an error code otherwise > > * \retval -EINVAL One of the control is not supported or not accessible > > s/control/controls/ > > > * \retval i The index of the control that failed > > */ > > -int V4L2Device::setControls(ControlList *ctrls) > > +int V4L2Device::setControls(ControlList *ctrls, const V4L2Request *request) > > { > > if (ctrls->empty()) > > return 0; > > @@ -377,10 +391,16 @@ int V4L2Device::setControls(ControlList *ctrls) > > } > > > > struct v4l2_ext_controls v4l2ExtCtrls = {}; > > - v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; > > v4l2ExtCtrls.controls = v4l2Ctrls.data(); > > v4l2ExtCtrls.count = v4l2Ctrls.size(); > > > > + if (request) { > > + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL; > > + v4l2ExtCtrls.request_fd = request->fd(); > > + } else { > > + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; > > + } > > + > > int ret = ioctl(VIDIOC_S_EXT_CTRLS, &v4l2ExtCtrls); > > if (ret) { > > unsigned int errorIdx = v4l2ExtCtrls.error_idx; > > diff --git a/src/libcamera/v4l2_request.cpp b/src/libcamera/v4l2_request.cpp > > new file mode 100644 > > index 000000000000..708250d86f61 > > --- /dev/null > > +++ b/src/libcamera/v4l2_request.cpp > > @@ -0,0 +1,128 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2025, Ideas On Board > > + * > > + * V4L2 Request API > > + */ > > + > > +#include "libcamera/internal/v4l2_request.h" > > + > > +#include <fcntl.h> > > +#include <stdlib.h> > > +#include <sys/ioctl.h> > > +#include <sys/syscall.h> > > +#include <unistd.h> > > + > > +#include <linux/media.h> > > + > > +#include <libcamera/base/event_notifier.h> > > +#include <libcamera/base/log.h> > > + > > +/** > > + * \file v4l2_request.h > > + * \brief V4L2 Request > > + */ > > + > > +namespace libcamera { > > + > > +LOG_DECLARE_CATEGORY(V4L2) > > + > > +/** > > + * \class V4L2Request > > + * \brief V4L2Request object and API > > + * > > + * The V4L2Request class wraps a V4L2 request fd and provides some convenience > > + * functions to handle request. > > + * > > + * It is usually constructed by calling \a MediaDevice::allocateRequests(). > > + * > > + * A request can then be passed to the V4L2Device::setControls(), > > + * V4L2Device::getControls() and V4L2VideoDevice::queueBuffer(). > > + */ > > + > > +/** > > + * \brief Construct a V4L2Request > > + * \param[in] fd The request fd > > + */ > > +V4L2Request::V4L2Request(int fd) > > + : fd_(fd), fdNotifier_(fd, EventNotifier::Exception) > > +{ > > + if (!fd_.isValid()) > > + return; > > + > > + fdNotifier_.activated.connect(this, &V4L2Request::requestReady); > > + fdNotifier_.setEnabled(false); > > +} > > + > > +/** > > + * \fn V4L2Request::isValid() > > + * \brief Check if the request is valid > > + * > > + * Checks if the underlying fd is valid. > > + * > > + * \return True if the request is valid, false otherwise > > + */ > > + > > +/** > > + * \fn V4L2Request::fd() > > + * \brief Get the file descriptor > > + * > > + * \return The file descriptor wrapped by this V4L2Request > > + */ > > + > > +/** > > + * \var V4L2Request::requestDone > > + * \brief Signal that is emitted when the request is done > > + */ > > + > > +/** > > + * \brief Reinit the request > > + * > > + * Calls MEDIA_REQUEST_IOC_REINIT om the request fd. > > + * > > + * \return 0 on success or a negative error code otherwise > > + */ > > +int V4L2Request::reinit() > > +{ > > + fdNotifier_.setEnabled(false); > > + > > + if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_REINIT) < 0) > > + return -errno; > > + > > + return 0; > > +} > > + > > +/** > > + * \brief Reinit the request > > + * > > + * Calls MEDIA_REQUEST_IOC_QUEUE om the request fd. > > + * > > + * \return 0 on success or a negative error code otherwise > > + */ > > +int V4L2Request::queue() > > +{ > > + if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_QUEUE) < 0) > > + return -errno; > > + > > + fdNotifier_.setEnabled(true); > > + > > + return 0; > > +} > > + > > +std::string V4L2Request::logPrefix() const > > +{ > > + return "Request [" + std::to_string(fd()) + "]"; > > +} > > + > > +/** > > + * \brief Slot to handle request done events > > + * > > + * When this slot is called, the request is done and the requestDone will be > > + * emitted. > > + */ > > +void V4L2Request::requestReady() > > +{ > > + requestDone.emit(this); > > +} > > + > > +} /* namespace libcamera */ > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > > index bb57c1b76a5b..8ce739f4bc65 100644 > > --- a/src/libcamera/v4l2_videodevice.cpp > > +++ b/src/libcamera/v4l2_videodevice.cpp > > @@ -30,6 +30,7 @@ > > #include "libcamera/internal/framebuffer.h" > > #include "libcamera/internal/media_device.h" > > #include "libcamera/internal/media_object.h" > > +#include "libcamera/internal/v4l2_request.h" > > > > /** > > * \file v4l2_videodevice.h > > @@ -1629,6 +1630,7 @@ int V4L2VideoDevice::releaseBuffers() > > /** > > * \brief Queue a buffer to the video device > > * \param[in] buffer The buffer to be queued > > + * \param[in] request An optional request > > * > > * For capture video devices the \a buffer will be filled with data by the > > * device. For output video devices the \a buffer shall contain valid data and > > @@ -1641,9 +1643,11 @@ int V4L2VideoDevice::releaseBuffers() > > * Note that queueBuffer() will fail if the device is in the process of being > > * stopped from a streaming state through streamOff(). > > * > > + * If \a request is specified, the buffer will be tied to that request. > > + * > > * \return 0 on success or a negative error code otherwise > > */ > > -int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) > > +int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer, const V4L2Request *request) > > { > > Best wishes, > > Isaac > > > struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {}; > > struct v4l2_buffer buf = {}; > > @@ -1674,6 +1678,10 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) > > buf.type = bufferType_; > > buf.memory = memoryType_; > > buf.field = V4L2_FIELD_NONE; > > + if (request) { > > + buf.flags = V4L2_BUF_FLAG_REQUEST_FD; > > + buf.request_fd = request->fd(); > > + } > > > > bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); > > Span<const FrameBuffer::Plane> planes = buffer->planes(); > > -- > > 2.48.1 > >
Hi Paul, Thank you for the review. Quoting Paul Elder (2025-11-05 17:46:02) > Quoting Stefan Klug (2025-10-23 23:48:06) > > The V4L2 requests API provides support to atomically tie controls to a > > set of buffers. This is especially common for m2m devices. Such a > > request is represented by a fd that is allocated vi > > s/ a / an / s/vi/via/ > > > MEDIA_IOC_REQUEST_ALLOC and then passed to various V4L2 function. > > s/function/functions/ ...and more typos... > > > > > Implement a V4L2Request class to wrap such an fd and add the > > corresponding utility functions. > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > > --- > > > > Changes in v2: > > - Added documentation > > --- > > include/libcamera/internal/media_device.h | 7 + > > include/libcamera/internal/meson.build | 1 + > > include/libcamera/internal/v4l2_device.h | 5 +- > > include/libcamera/internal/v4l2_request.h | 49 +++++++ > > include/libcamera/internal/v4l2_videodevice.h | 3 +- > > src/libcamera/media_device.cpp | 47 +++++++ > > src/libcamera/meson.build | 1 + > > src/libcamera/v4l2_device.cpp | 28 +++- > > src/libcamera/v4l2_request.cpp | 128 ++++++++++++++++++ > > src/libcamera/v4l2_videodevice.cpp | 10 +- > > 10 files changed, 271 insertions(+), 8 deletions(-) > > create mode 100644 include/libcamera/internal/v4l2_request.h > > create mode 100644 src/libcamera/v4l2_request.cpp > > > > diff --git a/include/libcamera/internal/media_device.h b/include/libcamera/internal/media_device.h > > index b3a48b98d64b..74cb9ba1542d 100644 > > --- a/include/libcamera/internal/media_device.h > > +++ b/include/libcamera/internal/media_device.h > > @@ -18,6 +18,7 @@ > > #include <libcamera/base/unique_fd.h> > > > > #include "libcamera/internal/media_object.h" > > +#include "libcamera/internal/v4l2_request.h" > > > > namespace libcamera { > > > > @@ -57,6 +58,11 @@ public: > > > > std::vector<MediaEntity *> locateEntities(unsigned int function); > > > > + int allocateRequests(unsigned int count, > > + std::vector<std::unique_ptr<V4L2Request>> *requests); > > + > > + bool supportsRequests(); > > + > > protected: > > std::string logPrefix() const override; > > > > @@ -87,6 +93,7 @@ private: > > UniqueFD fd_; > > bool valid_; > > bool acquired_; > > + std::optional<bool> supportsRequests_; > > > > std::map<unsigned int, MediaObject *> objects_; > > std::vector<MediaEntity *> entities_; > > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > > index 45c299f6a332..e9540a2f734f 100644 > > --- a/include/libcamera/internal/meson.build > > +++ b/include/libcamera/internal/meson.build > > @@ -44,6 +44,7 @@ libcamera_internal_headers = files([ > > 'sysfs.h', > > 'v4l2_device.h', > > 'v4l2_pixelformat.h', > > + 'v4l2_request.h', > > 'v4l2_subdevice.h', > > 'v4l2_videodevice.h', > > 'vector.h', > > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h > > index 5bc9da96677d..dbbd118abd00 100644 > > --- a/include/libcamera/internal/v4l2_device.h > > +++ b/include/libcamera/internal/v4l2_device.h > > @@ -24,6 +24,7 @@ > > #include <libcamera/controls.h> > > > > #include "libcamera/internal/formats.h" > > +#include "libcamera/internal/v4l2_request.h" > > > > namespace libcamera { > > > > @@ -37,8 +38,8 @@ public: > > > > const ControlInfoMap &controls() const { return controls_; } > > > > - ControlList getControls(Span<const uint32_t> ids); > > - int setControls(ControlList *ctrls); > > + ControlList getControls(Span<const uint32_t> ids, const V4L2Request *request = nullptr); > > + int setControls(ControlList *ctrls, const V4L2Request *request = nullptr); > > > > const struct v4l2_query_ext_ctrl *controlInfo(uint32_t id) const; > > > > diff --git a/include/libcamera/internal/v4l2_request.h b/include/libcamera/internal/v4l2_request.h > > new file mode 100644 > > index 000000000000..bf1bea3261af > > --- /dev/null > > +++ b/include/libcamera/internal/v4l2_request.h > > @@ -0,0 +1,49 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2025, Ideas On Board > > + * > > + * V4L2 requests > > + */ > > + > > +#pragma once > > + > > +#include <string> > > + > > +#include <linux/videodev2.h> > > + > > +#include <libcamera/base/event_notifier.h> > > +#include <libcamera/base/log.h> > > +#include <libcamera/base/signal.h> > > +#include <libcamera/base/span.h> > > +#include <libcamera/base/unique_fd.h> > > + > > +#include <libcamera/color_space.h> > > +#include <libcamera/controls.h> > > Do we need these two headers? Uhh, no we don't. Thanks for spotting. > > > + > > +namespace libcamera { > > + > > +class V4L2Request : protected Loggable > > +{ > > +public: > > + bool isValid() const { return fd_.isValid(); } > > + int fd() const { return fd_.get(); } > > + > > + int reinit(); > > + int queue(); > > + > > + V4L2Request(int fd = -1); > > + ~V4L2Request() = default; > > + > > + Signal<V4L2Request *> requestDone; > > + > > +private: > > + LIBCAMERA_DISABLE_COPY_AND_MOVE(V4L2Request) > > + > > + void requestReady(); > > + std::string logPrefix() const override; > > + > > + UniqueFD fd_; > > + EventNotifier fdNotifier_; > > +}; > > + > > +} /* namespace libcamera */ > > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h > > index 5a7dcfdda118..2d290971a0ee 100644 > > --- a/include/libcamera/internal/v4l2_videodevice.h > > +++ b/include/libcamera/internal/v4l2_videodevice.h > > @@ -33,6 +33,7 @@ > > #include "libcamera/internal/formats.h" > > #include "libcamera/internal/v4l2_device.h" > > #include "libcamera/internal/v4l2_pixelformat.h" > > +#include "libcamera/internal/v4l2_request.h" > > > > namespace libcamera { > > > > > @@ -217,7 +218,7 @@ public: > > > int importBuffers(unsigned int count); > > int releaseBuffers(); > > > > - int queueBuffer(FrameBuffer *buffer); > > + int queueBuffer(FrameBuffer *buffer, const V4L2Request *request = nullptr); > > Signal<FrameBuffer *> bufferReady; > > > > int streamOn(); > > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp > > index 353f34a81eca..673c53fefdd7 100644 > > --- a/src/libcamera/media_device.cpp > > +++ b/src/libcamera/media_device.cpp > > @@ -20,6 +20,7 @@ > > #include <linux/media.h> > > > > #include <libcamera/base/log.h> > > +#include "libcamera/internal/v4l2_request.h" > > > > /** > > * \file media_device.h > > @@ -851,4 +852,50 @@ std::vector<MediaEntity *> MediaDevice::locateEntities(unsigned int function) > > return found; > > } > > > > +/** > > + * \brief Allocate requests > > + * \param[in] count Number of requests to allocate > > + * \param[out] requests Vector to store allocated requests > > + * > > + * Allocates and stores \a count requests in \a requests. If allocation fails, > > + * and error is returned and \a requests is cleared. > > s/\* and/\* an/ > > > + * > > + * \return 0 on success or a negative error code otherwise > > + */ > > +int MediaDevice::allocateRequests(unsigned int count, > > + std::vector<std::unique_ptr<V4L2Request>> *requests) > > +{ > > + requests->resize(count); > > + for (unsigned int i = 0; i < count; i++) { > > + int requestFd; > > + int ret = ::ioctl(fd_.get(), MEDIA_IOC_REQUEST_ALLOC, &requestFd); > > I'm of the opinion that this should be moved into V4L2Request constructor (or > init()) so that we can keep all requests handling there. We already have an > isValid(), and maybe we can add close() to the deconstrctor? Hm, I see the point. I'm a bit undecided here. The request specific ioctls are prefixed with MEDIA_REQUEST_IOC_ whilst the media device ones are prefixed with MEDIA_IOC_, so it is not completely wrong to do it here. We could move the whole function into V4L2Request as allocateForMedia(int fd, int count, std::vector<std::unique_ptr<V4L2Request>> *requests) and this function could then collapse to return V4L2Request::allocateForMedia(fd_.get(), count, requests); In that case I dislike passing the media device fd as int. So I'm not really convinced :-) Would you prefer such a static function? > > > + if (ret < 0) { > > + requests->clear(); > > + return -errno; > > + } > > + (*requests)[i] = std::make_unique<V4L2Request>(requestFd); > > + } > > + > > + return 0; > > +} > > + > > +/** > > + * \brief Check if requests are supported > > + * > > + * Checks if the device supports V4L2 requests by trying to allocate a single > > + * request. The result is cached, so the allocation is only tried once. > > + * > > + * \return True if the device supports requests, false otherwise > > + */ > > +bool MediaDevice::supportsRequests() > > +{ > > + if (supportsRequests_.has_value()) > > + return supportsRequests_.value(); > > + > > + std::vector<std::unique_ptr<V4L2Request>> requests; > > + supportsRequests_ = (allocateRequests(1, &requests) == 0); > > Initially I was worried about subsequent allocations but then I realized that > this will get deallocated after it goes out of scope. > > Although there is currently no other errno than ENOTTY, the non-explicit check > makes me uncomfortable :S Barnabás mentioned that also. I don't think we should treat any other error as supportsRequests() == true, But I don't know for sure. Would you prefer returning the error code and leaving the decision to the user? But then the user could call allocateRequests() directly in first place... > > > + > > + return supportsRequests_.value(); > > +} > > + > > } /* namespace libcamera */ > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > index 5b9b86f211f1..34e20f557514 100644 > > --- a/src/libcamera/meson.build > > +++ b/src/libcamera/meson.build > > @@ -54,6 +54,7 @@ libcamera_internal_sources = files([ > > 'sysfs.cpp', > > 'v4l2_device.cpp', > > 'v4l2_pixelformat.cpp', > > + 'v4l2_request.cpp', > > 'v4l2_subdevice.cpp', > > 'v4l2_videodevice.cpp', > > 'vector.cpp', > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > > index 8c78b8c424e5..7a669a0303c1 100644 > > --- a/src/libcamera/v4l2_device.cpp > > +++ b/src/libcamera/v4l2_device.cpp > > @@ -162,6 +162,7 @@ void V4L2Device::close() > > /** > > * \brief Read controls from the device > > * \param[in] ids The list of controls to read, specified by their ID > > + * \param[in] request An optional request > > * > > * This function reads the value of all controls contained in \a ids, and > > * returns their values as a ControlList. > > @@ -171,10 +172,12 @@ void V4L2Device::close() > > * during validation of the requested controls, no control is read and this > > * function returns an empty control list. > > * > > + * If \a request is specified the controls tied to that request are read. > > + * > > * \return The control values in a ControlList on success, or an empty list on > > * error > > */ > > -ControlList V4L2Device::getControls(Span<const uint32_t> ids) > > +ControlList V4L2Device::getControls(Span<const uint32_t> ids, const V4L2Request *request) > > { > > if (ids.empty()) > > return {}; > > @@ -242,10 +245,16 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids) > > } > > > > struct v4l2_ext_controls v4l2ExtCtrls = {}; > > - v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; > > v4l2ExtCtrls.controls = v4l2Ctrls.data(); > > v4l2ExtCtrls.count = v4l2Ctrls.size(); > > > > + if (request) { > > + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL; > > + v4l2ExtCtrls.request_fd = request->fd(); > > + } else { > > + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; > > + } > > + > > int ret = ioctl(VIDIOC_G_EXT_CTRLS, &v4l2ExtCtrls); > > if (ret) { > > unsigned int errorIdx = v4l2ExtCtrls.error_idx; > > @@ -273,6 +282,7 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids) > > /** > > * \brief Write controls to the device > > * \param[in] ctrls The list of controls to write > > + * \param[in] request And optional request > > s/And/An/ > > > * > > * This function writes the value of all controls contained in \a ctrls, and > > * stores the values actually applied to the device in the corresponding > > @@ -288,11 +298,15 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids) > > * are written and their values are updated in \a ctrls, while all other > > * controls are not written and their values are not changed. > > * > > + * If \a request is set, the controls will be applied to that request. If the > > + * device doesn't support requests, -EACCESS will be returned. If \a request is > > + * invalid, -EINVAL will be returned. > > Do you need to mention EACCESS in the docunentation for read as well? > > > + * > > * \return 0 on success or an error code otherwise > > * \retval -EINVAL One of the control is not supported or not accessible > > * \retval i The index of the control that failed > > */ > > -int V4L2Device::setControls(ControlList *ctrls) > > +int V4L2Device::setControls(ControlList *ctrls, const V4L2Request *request) > > { > > if (ctrls->empty()) > > return 0; > > @@ -377,10 +391,16 @@ int V4L2Device::setControls(ControlList *ctrls) > > } > > > > struct v4l2_ext_controls v4l2ExtCtrls = {}; > > - v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; > > v4l2ExtCtrls.controls = v4l2Ctrls.data(); > > v4l2ExtCtrls.count = v4l2Ctrls.size(); > > > > + if (request) { > > + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL; > > + v4l2ExtCtrls.request_fd = request->fd(); > > + } else { > > + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; > > + } > > + > > int ret = ioctl(VIDIOC_S_EXT_CTRLS, &v4l2ExtCtrls); > > if (ret) { > > unsigned int errorIdx = v4l2ExtCtrls.error_idx; > > diff --git a/src/libcamera/v4l2_request.cpp b/src/libcamera/v4l2_request.cpp > > new file mode 100644 > > index 000000000000..708250d86f61 > > --- /dev/null > > +++ b/src/libcamera/v4l2_request.cpp > > @@ -0,0 +1,128 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2025, Ideas On Board > > + * > > + * V4L2 Request API > > + */ > > + > > +#include "libcamera/internal/v4l2_request.h" > > + > > +#include <fcntl.h> > > +#include <stdlib.h> > > +#include <sys/ioctl.h> > > +#include <sys/syscall.h> > > +#include <unistd.h> > > + > > +#include <linux/media.h> > > + > > +#include <libcamera/base/event_notifier.h> > > +#include <libcamera/base/log.h> > > + > > +/** > > + * \file v4l2_request.h > > + * \brief V4L2 Request > > + */ > > + > > +namespace libcamera { > > + > > +LOG_DECLARE_CATEGORY(V4L2) > > + > > +/** > > + * \class V4L2Request > > + * \brief V4L2Request object and API > > + * > > + * The V4L2Request class wraps a V4L2 request fd and provides some convenience > > + * functions to handle request. > > + * > > + * It is usually constructed by calling \a MediaDevice::allocateRequests(). > > + * > > + * A request can then be passed to the V4L2Device::setControls(), > > + * V4L2Device::getControls() and V4L2VideoDevice::queueBuffer(). > > + */ > > + > > +/** > > + * \brief Construct a V4L2Request > > + * \param[in] fd The request fd > > + */ > > +V4L2Request::V4L2Request(int fd) > > + : fd_(fd), fdNotifier_(fd, EventNotifier::Exception) > > +{ > > + if (!fd_.isValid()) > > + return; > > + > > + fdNotifier_.activated.connect(this, &V4L2Request::requestReady); > > + fdNotifier_.setEnabled(false); > > +} > > + > > +/** > > + * \fn V4L2Request::isValid() > > + * \brief Check if the request is valid > > + * > > + * Checks if the underlying fd is valid. > > + * > > + * \return True if the request is valid, false otherwise > > + */ > > + > > +/** > > + * \fn V4L2Request::fd() > > + * \brief Get the file descriptor > > + * > > + * \return The file descriptor wrapped by this V4L2Request > > + */ > > + > > +/** > > + * \var V4L2Request::requestDone > > + * \brief Signal that is emitted when the request is done > > + */ > > + > > +/** > > + * \brief Reinit the request > > + * > > + * Calls MEDIA_REQUEST_IOC_REINIT om the request fd. > > s/om/on/ > > > + * > > + * \return 0 on success or a negative error code otherwise > > + */ > > +int V4L2Request::reinit() > > +{ > > + fdNotifier_.setEnabled(false); > > + > > + if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_REINIT) < 0) > > + return -errno; > > + > > + return 0; > > +} > > + > > +/** > > + * \brief Reinit the request > > Reinit? :) > > > + * > > + * Calls MEDIA_REQUEST_IOC_QUEUE om the request fd. > > s/om/on/ > Thanks, I fixed all the typos, but left the other ones as is for now, because I didn't feel like the path forward is completely clear. Cheers, Stefan > > Thanks, > > Paul > > > + * > > + * \return 0 on success or a negative error code otherwise > > + */ > > +int V4L2Request::queue() > > +{ > > + if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_QUEUE) < 0) > > + return -errno; > > + > > + fdNotifier_.setEnabled(true); > > + > > + return 0; > > +} > > + > > +std::string V4L2Request::logPrefix() const > > +{ > > + return "Request [" + std::to_string(fd()) + "]"; > > +} > > + > > +/** > > + * \brief Slot to handle request done events > > + * > > + * When this slot is called, the request is done and the requestDone will be > > + * emitted. > > + */ > > +void V4L2Request::requestReady() > > +{ > > + requestDone.emit(this); > > +} > > + > > +} /* namespace libcamera */ > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > > index bb57c1b76a5b..8ce739f4bc65 100644 > > --- a/src/libcamera/v4l2_videodevice.cpp > > +++ b/src/libcamera/v4l2_videodevice.cpp > > @@ -30,6 +30,7 @@ > > #include "libcamera/internal/framebuffer.h" > > #include "libcamera/internal/media_device.h" > > #include "libcamera/internal/media_object.h" > > +#include "libcamera/internal/v4l2_request.h" > > > > /** > > * \file v4l2_videodevice.h > > @@ -1629,6 +1630,7 @@ int V4L2VideoDevice::releaseBuffers() > > /** > > * \brief Queue a buffer to the video device > > * \param[in] buffer The buffer to be queued > > + * \param[in] request An optional request > > * > > * For capture video devices the \a buffer will be filled with data by the > > * device. For output video devices the \a buffer shall contain valid data and > > @@ -1641,9 +1643,11 @@ int V4L2VideoDevice::releaseBuffers() > > * Note that queueBuffer() will fail if the device is in the process of being > > * stopped from a streaming state through streamOff(). > > * > > + * If \a request is specified, the buffer will be tied to that request. > > + * > > * \return 0 on success or a negative error code otherwise > > */ > > -int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) > > +int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer, const V4L2Request *request) > > { > > struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {}; > > struct v4l2_buffer buf = {}; > > @@ -1674,6 +1678,10 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) > > buf.type = bufferType_; > > buf.memory = memoryType_; > > buf.field = V4L2_FIELD_NONE; > > + if (request) { > > + buf.flags = V4L2_BUF_FLAG_REQUEST_FD; > > + buf.request_fd = request->fd(); > > + } > > > > bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); > > Span<const FrameBuffer::Plane> planes = buffer->planes(); > > -- > > 2.48.1 > >
2025. 11. 24. 16:27 keltezéssel, Stefan Klug írta: > Hi Barnabás, > > Thank you for the review. > > As always I should have spent more time reading the review earlier and > not just before prearing the new series... > > Quoting Barnabás Pőcze (2025-10-24 16:00:32) >> Hi >> >> 2025. 10. 23. 16:48 keltezéssel, Stefan Klug írta: >>> The V4L2 requests API provides support to atomically tie controls to a >>> set of buffers. This is especially common for m2m devices. Such a >>> request is represented by a fd that is allocated vi >>> MEDIA_IOC_REQUEST_ALLOC and then passed to various V4L2 function. >>> >>> Implement a V4L2Request class to wrap such an fd and add the >>> corresponding utility functions. >>> >>> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> >>> >>> --- >>> >>> Changes in v2: >>> - Added documentation >>> --- >>> include/libcamera/internal/media_device.h | 7 + >>> include/libcamera/internal/meson.build | 1 + >>> include/libcamera/internal/v4l2_device.h | 5 +- >>> include/libcamera/internal/v4l2_request.h | 49 +++++++ >>> include/libcamera/internal/v4l2_videodevice.h | 3 +- >>> src/libcamera/media_device.cpp | 47 +++++++ >>> src/libcamera/meson.build | 1 + >>> src/libcamera/v4l2_device.cpp | 28 +++- >>> src/libcamera/v4l2_request.cpp | 128 ++++++++++++++++++ >>> src/libcamera/v4l2_videodevice.cpp | 10 +- >>> 10 files changed, 271 insertions(+), 8 deletions(-) >>> create mode 100644 include/libcamera/internal/v4l2_request.h >>> create mode 100644 src/libcamera/v4l2_request.cpp >>> >>> [...] >>> diff --git a/src/libcamera/v4l2_request.cpp b/src/libcamera/v4l2_request.cpp >>> new file mode 100644 >>> index 000000000000..708250d86f61 >>> --- /dev/null >>> +++ b/src/libcamera/v4l2_request.cpp >>> @@ -0,0 +1,128 @@ >>> [...] >>> +/** >>> + * \brief Construct a V4L2Request >>> + * \param[in] fd The request fd >>> + */ >>> +V4L2Request::V4L2Request(int fd) >>> + : fd_(fd), fdNotifier_(fd, EventNotifier::Exception) >>> +{ >>> + if (!fd_.isValid()) >>> + return; >>> + >>> + fdNotifier_.activated.connect(this, &V4L2Request::requestReady); >> >> It's probably just me, but I feel like I would just use a simple lambda here. > > I'd prefer a lambda, too. I didn't get it to work though. The code I'd > expect there would be something like: > > fdNotifier_.activated.connect([this] { this->requestDone.emit(this); }); > > But that doesn't work as capturing lambdas can't be converted to a > function pointer. I didn't dig deeper into the Signal implementation. > How would you do that? I see this is already merged, but it would be: fdNotifier_.activated.connect(this, [this] { requestDone.emit(this); }); > > Best regards, > Stefan > >> >> >>> + fdNotifier_.setEnabled(false); >>> +} >>> [...] >> >> >> Regards, >> Barnabás Pőcze >>
diff --git a/include/libcamera/internal/media_device.h b/include/libcamera/internal/media_device.h index b3a48b98d64b..74cb9ba1542d 100644 --- a/include/libcamera/internal/media_device.h +++ b/include/libcamera/internal/media_device.h @@ -18,6 +18,7 @@ #include <libcamera/base/unique_fd.h> #include "libcamera/internal/media_object.h" +#include "libcamera/internal/v4l2_request.h" namespace libcamera { @@ -57,6 +58,11 @@ public: std::vector<MediaEntity *> locateEntities(unsigned int function); + int allocateRequests(unsigned int count, + std::vector<std::unique_ptr<V4L2Request>> *requests); + + bool supportsRequests(); + protected: std::string logPrefix() const override; @@ -87,6 +93,7 @@ private: UniqueFD fd_; bool valid_; bool acquired_; + std::optional<bool> supportsRequests_; std::map<unsigned int, MediaObject *> objects_; std::vector<MediaEntity *> entities_; diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build index 45c299f6a332..e9540a2f734f 100644 --- a/include/libcamera/internal/meson.build +++ b/include/libcamera/internal/meson.build @@ -44,6 +44,7 @@ libcamera_internal_headers = files([ 'sysfs.h', 'v4l2_device.h', 'v4l2_pixelformat.h', + 'v4l2_request.h', 'v4l2_subdevice.h', 'v4l2_videodevice.h', 'vector.h', diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h index 5bc9da96677d..dbbd118abd00 100644 --- a/include/libcamera/internal/v4l2_device.h +++ b/include/libcamera/internal/v4l2_device.h @@ -24,6 +24,7 @@ #include <libcamera/controls.h> #include "libcamera/internal/formats.h" +#include "libcamera/internal/v4l2_request.h" namespace libcamera { @@ -37,8 +38,8 @@ public: const ControlInfoMap &controls() const { return controls_; } - ControlList getControls(Span<const uint32_t> ids); - int setControls(ControlList *ctrls); + ControlList getControls(Span<const uint32_t> ids, const V4L2Request *request = nullptr); + int setControls(ControlList *ctrls, const V4L2Request *request = nullptr); const struct v4l2_query_ext_ctrl *controlInfo(uint32_t id) const; diff --git a/include/libcamera/internal/v4l2_request.h b/include/libcamera/internal/v4l2_request.h new file mode 100644 index 000000000000..bf1bea3261af --- /dev/null +++ b/include/libcamera/internal/v4l2_request.h @@ -0,0 +1,49 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2025, Ideas On Board + * + * V4L2 requests + */ + +#pragma once + +#include <string> + +#include <linux/videodev2.h> + +#include <libcamera/base/event_notifier.h> +#include <libcamera/base/log.h> +#include <libcamera/base/signal.h> +#include <libcamera/base/span.h> +#include <libcamera/base/unique_fd.h> + +#include <libcamera/color_space.h> +#include <libcamera/controls.h> + +namespace libcamera { + +class V4L2Request : protected Loggable +{ +public: + bool isValid() const { return fd_.isValid(); } + int fd() const { return fd_.get(); } + + int reinit(); + int queue(); + + V4L2Request(int fd = -1); + ~V4L2Request() = default; + + Signal<V4L2Request *> requestDone; + +private: + LIBCAMERA_DISABLE_COPY_AND_MOVE(V4L2Request) + + void requestReady(); + std::string logPrefix() const override; + + UniqueFD fd_; + EventNotifier fdNotifier_; +}; + +} /* namespace libcamera */ diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h index 5a7dcfdda118..2d290971a0ee 100644 --- a/include/libcamera/internal/v4l2_videodevice.h +++ b/include/libcamera/internal/v4l2_videodevice.h @@ -33,6 +33,7 @@ #include "libcamera/internal/formats.h" #include "libcamera/internal/v4l2_device.h" #include "libcamera/internal/v4l2_pixelformat.h" +#include "libcamera/internal/v4l2_request.h" namespace libcamera { @@ -217,7 +218,7 @@ public: int importBuffers(unsigned int count); int releaseBuffers(); - int queueBuffer(FrameBuffer *buffer); + int queueBuffer(FrameBuffer *buffer, const V4L2Request *request = nullptr); Signal<FrameBuffer *> bufferReady; int streamOn(); diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index 353f34a81eca..673c53fefdd7 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -20,6 +20,7 @@ #include <linux/media.h> #include <libcamera/base/log.h> +#include "libcamera/internal/v4l2_request.h" /** * \file media_device.h @@ -851,4 +852,50 @@ std::vector<MediaEntity *> MediaDevice::locateEntities(unsigned int function) return found; } +/** + * \brief Allocate requests + * \param[in] count Number of requests to allocate + * \param[out] requests Vector to store allocated requests + * + * Allocates and stores \a count requests in \a requests. If allocation fails, + * and error is returned and \a requests is cleared. + * + * \return 0 on success or a negative error code otherwise + */ +int MediaDevice::allocateRequests(unsigned int count, + std::vector<std::unique_ptr<V4L2Request>> *requests) +{ + requests->resize(count); + for (unsigned int i = 0; i < count; i++) { + int requestFd; + int ret = ::ioctl(fd_.get(), MEDIA_IOC_REQUEST_ALLOC, &requestFd); + if (ret < 0) { + requests->clear(); + return -errno; + } + (*requests)[i] = std::make_unique<V4L2Request>(requestFd); + } + + return 0; +} + +/** + * \brief Check if requests are supported + * + * Checks if the device supports V4L2 requests by trying to allocate a single + * request. The result is cached, so the allocation is only tried once. + * + * \return True if the device supports requests, false otherwise + */ +bool MediaDevice::supportsRequests() +{ + if (supportsRequests_.has_value()) + return supportsRequests_.value(); + + std::vector<std::unique_ptr<V4L2Request>> requests; + supportsRequests_ = (allocateRequests(1, &requests) == 0); + + return supportsRequests_.value(); +} + } /* namespace libcamera */ diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 5b9b86f211f1..34e20f557514 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -54,6 +54,7 @@ libcamera_internal_sources = files([ 'sysfs.cpp', 'v4l2_device.cpp', 'v4l2_pixelformat.cpp', + 'v4l2_request.cpp', 'v4l2_subdevice.cpp', 'v4l2_videodevice.cpp', 'vector.cpp', diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 8c78b8c424e5..7a669a0303c1 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -162,6 +162,7 @@ void V4L2Device::close() /** * \brief Read controls from the device * \param[in] ids The list of controls to read, specified by their ID + * \param[in] request An optional request * * This function reads the value of all controls contained in \a ids, and * returns their values as a ControlList. @@ -171,10 +172,12 @@ void V4L2Device::close() * during validation of the requested controls, no control is read and this * function returns an empty control list. * + * If \a request is specified the controls tied to that request are read. + * * \return The control values in a ControlList on success, or an empty list on * error */ -ControlList V4L2Device::getControls(Span<const uint32_t> ids) +ControlList V4L2Device::getControls(Span<const uint32_t> ids, const V4L2Request *request) { if (ids.empty()) return {}; @@ -242,10 +245,16 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids) } struct v4l2_ext_controls v4l2ExtCtrls = {}; - v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; v4l2ExtCtrls.controls = v4l2Ctrls.data(); v4l2ExtCtrls.count = v4l2Ctrls.size(); + if (request) { + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL; + v4l2ExtCtrls.request_fd = request->fd(); + } else { + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; + } + int ret = ioctl(VIDIOC_G_EXT_CTRLS, &v4l2ExtCtrls); if (ret) { unsigned int errorIdx = v4l2ExtCtrls.error_idx; @@ -273,6 +282,7 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids) /** * \brief Write controls to the device * \param[in] ctrls The list of controls to write + * \param[in] request And optional request * * This function writes the value of all controls contained in \a ctrls, and * stores the values actually applied to the device in the corresponding @@ -288,11 +298,15 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids) * are written and their values are updated in \a ctrls, while all other * controls are not written and their values are not changed. * + * If \a request is set, the controls will be applied to that request. If the + * device doesn't support requests, -EACCESS will be returned. If \a request is + * invalid, -EINVAL will be returned. + * * \return 0 on success or an error code otherwise * \retval -EINVAL One of the control is not supported or not accessible * \retval i The index of the control that failed */ -int V4L2Device::setControls(ControlList *ctrls) +int V4L2Device::setControls(ControlList *ctrls, const V4L2Request *request) { if (ctrls->empty()) return 0; @@ -377,10 +391,16 @@ int V4L2Device::setControls(ControlList *ctrls) } struct v4l2_ext_controls v4l2ExtCtrls = {}; - v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; v4l2ExtCtrls.controls = v4l2Ctrls.data(); v4l2ExtCtrls.count = v4l2Ctrls.size(); + if (request) { + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL; + v4l2ExtCtrls.request_fd = request->fd(); + } else { + v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL; + } + int ret = ioctl(VIDIOC_S_EXT_CTRLS, &v4l2ExtCtrls); if (ret) { unsigned int errorIdx = v4l2ExtCtrls.error_idx; diff --git a/src/libcamera/v4l2_request.cpp b/src/libcamera/v4l2_request.cpp new file mode 100644 index 000000000000..708250d86f61 --- /dev/null +++ b/src/libcamera/v4l2_request.cpp @@ -0,0 +1,128 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2025, Ideas On Board + * + * V4L2 Request API + */ + +#include "libcamera/internal/v4l2_request.h" + +#include <fcntl.h> +#include <stdlib.h> +#include <sys/ioctl.h> +#include <sys/syscall.h> +#include <unistd.h> + +#include <linux/media.h> + +#include <libcamera/base/event_notifier.h> +#include <libcamera/base/log.h> + +/** + * \file v4l2_request.h + * \brief V4L2 Request + */ + +namespace libcamera { + +LOG_DECLARE_CATEGORY(V4L2) + +/** + * \class V4L2Request + * \brief V4L2Request object and API + * + * The V4L2Request class wraps a V4L2 request fd and provides some convenience + * functions to handle request. + * + * It is usually constructed by calling \a MediaDevice::allocateRequests(). + * + * A request can then be passed to the V4L2Device::setControls(), + * V4L2Device::getControls() and V4L2VideoDevice::queueBuffer(). + */ + +/** + * \brief Construct a V4L2Request + * \param[in] fd The request fd + */ +V4L2Request::V4L2Request(int fd) + : fd_(fd), fdNotifier_(fd, EventNotifier::Exception) +{ + if (!fd_.isValid()) + return; + + fdNotifier_.activated.connect(this, &V4L2Request::requestReady); + fdNotifier_.setEnabled(false); +} + +/** + * \fn V4L2Request::isValid() + * \brief Check if the request is valid + * + * Checks if the underlying fd is valid. + * + * \return True if the request is valid, false otherwise + */ + +/** + * \fn V4L2Request::fd() + * \brief Get the file descriptor + * + * \return The file descriptor wrapped by this V4L2Request + */ + +/** + * \var V4L2Request::requestDone + * \brief Signal that is emitted when the request is done + */ + +/** + * \brief Reinit the request + * + * Calls MEDIA_REQUEST_IOC_REINIT om the request fd. + * + * \return 0 on success or a negative error code otherwise + */ +int V4L2Request::reinit() +{ + fdNotifier_.setEnabled(false); + + if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_REINIT) < 0) + return -errno; + + return 0; +} + +/** + * \brief Reinit the request + * + * Calls MEDIA_REQUEST_IOC_QUEUE om the request fd. + * + * \return 0 on success or a negative error code otherwise + */ +int V4L2Request::queue() +{ + if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_QUEUE) < 0) + return -errno; + + fdNotifier_.setEnabled(true); + + return 0; +} + +std::string V4L2Request::logPrefix() const +{ + return "Request [" + std::to_string(fd()) + "]"; +} + +/** + * \brief Slot to handle request done events + * + * When this slot is called, the request is done and the requestDone will be + * emitted. + */ +void V4L2Request::requestReady() +{ + requestDone.emit(this); +} + +} /* namespace libcamera */ diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index bb57c1b76a5b..8ce739f4bc65 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -30,6 +30,7 @@ #include "libcamera/internal/framebuffer.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/media_object.h" +#include "libcamera/internal/v4l2_request.h" /** * \file v4l2_videodevice.h @@ -1629,6 +1630,7 @@ int V4L2VideoDevice::releaseBuffers() /** * \brief Queue a buffer to the video device * \param[in] buffer The buffer to be queued + * \param[in] request An optional request * * For capture video devices the \a buffer will be filled with data by the * device. For output video devices the \a buffer shall contain valid data and @@ -1641,9 +1643,11 @@ int V4L2VideoDevice::releaseBuffers() * Note that queueBuffer() will fail if the device is in the process of being * stopped from a streaming state through streamOff(). * + * If \a request is specified, the buffer will be tied to that request. + * * \return 0 on success or a negative error code otherwise */ -int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) +int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer, const V4L2Request *request) { struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {}; struct v4l2_buffer buf = {}; @@ -1674,6 +1678,10 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) buf.type = bufferType_; buf.memory = memoryType_; buf.field = V4L2_FIELD_NONE; + if (request) { + buf.flags = V4L2_BUF_FLAG_REQUEST_FD; + buf.request_fd = request->fd(); + } bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); Span<const FrameBuffer::Plane> planes = buffer->planes();
The V4L2 requests API provides support to atomically tie controls to a set of buffers. This is especially common for m2m devices. Such a request is represented by a fd that is allocated vi MEDIA_IOC_REQUEST_ALLOC and then passed to various V4L2 function. Implement a V4L2Request class to wrap such an fd and add the corresponding utility functions. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- Changes in v2: - Added documentation --- include/libcamera/internal/media_device.h | 7 + include/libcamera/internal/meson.build | 1 + include/libcamera/internal/v4l2_device.h | 5 +- include/libcamera/internal/v4l2_request.h | 49 +++++++ include/libcamera/internal/v4l2_videodevice.h | 3 +- src/libcamera/media_device.cpp | 47 +++++++ src/libcamera/meson.build | 1 + src/libcamera/v4l2_device.cpp | 28 +++- src/libcamera/v4l2_request.cpp | 128 ++++++++++++++++++ src/libcamera/v4l2_videodevice.cpp | 10 +- 10 files changed, 271 insertions(+), 8 deletions(-) create mode 100644 include/libcamera/internal/v4l2_request.h create mode 100644 src/libcamera/v4l2_request.cpp