[{"id":36440,"web_url":"https://patchwork.libcamera.org/comment/36440/","msgid":"<18f2c0c1-c9e5-444a-88a2-59956e698abf@ideasonboard.com>","date":"2025-10-24T14:00:32","subject":"Re: [PATCH v2 05/35] libcamera: Add support for V4L2 requests","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 10. 23. 16:48 keltezéssel, Stefan Klug írta:\n> The V4L2 requests API provides support to atomically tie controls to a\n> set of buffers. This is especially common for m2m devices. Such a\n> request is represented by a fd that is allocated vi\n> MEDIA_IOC_REQUEST_ALLOC and then passed to various V4L2 function.\n> \n> Implement a V4L2Request class to wrap such an fd and add the\n> corresponding utility functions.\n> \n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> \n> ---\n> \n> Changes in v2:\n> - Added documentation\n> ---\n>   include/libcamera/internal/media_device.h     |   7 +\n>   include/libcamera/internal/meson.build        |   1 +\n>   include/libcamera/internal/v4l2_device.h      |   5 +-\n>   include/libcamera/internal/v4l2_request.h     |  49 +++++++\n>   include/libcamera/internal/v4l2_videodevice.h |   3 +-\n>   src/libcamera/media_device.cpp                |  47 +++++++\n>   src/libcamera/meson.build                     |   1 +\n>   src/libcamera/v4l2_device.cpp                 |  28 +++-\n>   src/libcamera/v4l2_request.cpp                | 128 ++++++++++++++++++\n>   src/libcamera/v4l2_videodevice.cpp            |  10 +-\n>   10 files changed, 271 insertions(+), 8 deletions(-)\n>   create mode 100644 include/libcamera/internal/v4l2_request.h\n>   create mode 100644 src/libcamera/v4l2_request.cpp\n> \n> [...]\n> diff --git a/include/libcamera/internal/v4l2_request.h b/include/libcamera/internal/v4l2_request.h\n> new file mode 100644\n> index 000000000000..bf1bea3261af\n> --- /dev/null\n> +++ b/include/libcamera/internal/v4l2_request.h\n> @@ -0,0 +1,49 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board\n> + *\n> + * V4L2 requests\n> + */\n> +\n> +#pragma once\n> +\n> +#include <string>\n> +\n> +#include <linux/videodev2.h>\n> +\n> +#include <libcamera/base/event_notifier.h>\n> +#include <libcamera/base/log.h>\n> +#include <libcamera/base/signal.h>\n> +#include <libcamera/base/span.h>\n> +#include <libcamera/base/unique_fd.h>\n> +\n> +#include <libcamera/color_space.h>\n> +#include <libcamera/controls.h>\n> +\n> +namespace libcamera {\n> +\n> +class V4L2Request : protected Loggable\n> +{\n> +public:\n> +\tbool isValid() const { return fd_.isValid(); }\n> +\tint fd() const { return fd_.get(); }\n> +\n> +\tint reinit();\n> +\tint queue();\n> +\n> +\tV4L2Request(int fd = -1);\n\nI think just taking `UniqueFD` would be better as it makes the ownership clear.\n\nIs it is useful to create a `V4L2Request` object without a valid file descriptor?\nThere is no way to change the file descriptor and as far as I can tell most methods\nwould just fail.\n\n\n> +\t~V4L2Request() = default;\n\nI think this is already implied.\n\n\n> +\n> +\tSignal<V4L2Request *> requestDone;\n> +\n> +private:\n> +\tLIBCAMERA_DISABLE_COPY_AND_MOVE(V4L2Request)\n> +\n> +\tvoid requestReady();\n> +\tstd::string logPrefix() const override;\n> +\n> +\tUniqueFD fd_;\n> +\tEventNotifier fdNotifier_;\n> +};\n> +\n> +} /* namespace libcamera */\n> [...]\n> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n> index 353f34a81eca..673c53fefdd7 100644\n> --- a/src/libcamera/media_device.cpp\n> +++ b/src/libcamera/media_device.cpp\n> @@ -20,6 +20,7 @@\n>   #include <linux/media.h>\n>   \n>   #include <libcamera/base/log.h>\n> +#include \"libcamera/internal/v4l2_request.h\"\n>   \n>   /**\n>    * \\file media_device.h\n> @@ -851,4 +852,50 @@ std::vector<MediaEntity *> MediaDevice::locateEntities(unsigned int function)\n>   \treturn found;\n>   }\n>   \n> +/**\n> + * \\brief Allocate requests\n> + * \\param[in] count Number of requests to allocate\n> + * \\param[out] requests Vector to store allocated requests\n> + *\n> + * Allocates and stores \\a count requests in \\a requests. If allocation fails,\n> + * and error is returned and \\a requests is cleared.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int MediaDevice::allocateRequests(unsigned int count,\n> +\t\t\t\t  std::vector<std::unique_ptr<V4L2Request>> *requests)\n> +{\n> +\trequests->resize(count);\n> +\tfor (unsigned int i = 0; i < count; i++) {\n> +\t\tint requestFd;\n> +\t\tint ret = ::ioctl(fd_.get(), MEDIA_IOC_REQUEST_ALLOC, &requestFd);\n> +\t\tif (ret < 0) {\n> +\t\t\trequests->clear();\n> +\t\t\treturn -errno;\n> +\t\t}\n> +\t\t(*requests)[i] = std::make_unique<V4L2Request>(requestFd);\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\brief Check if requests are supported\n> + *\n> + * Checks if the device supports V4L2 requests by trying to allocate a single\n> + * request. The result is cached, so the allocation is only tried once.\n> + *\n> + * \\return True if the device supports requests, false otherwise\n> + */\n> +bool MediaDevice::supportsRequests()\n> +{\n> +\tif (supportsRequests_.has_value())\n> +\t\treturn supportsRequests_.value();\n> +\n> +\tstd::vector<std::unique_ptr<V4L2Request>> requests;\n> +\tsupportsRequests_ = (allocateRequests(1, &requests) == 0);\n\nShouldn't it technically be something like ` != -ENOTTY` ?\n\n\n> +\n> +\treturn supportsRequests_.value();\n> +}\n> +\n>   } /* namespace libcamera */\n> [...]\n> diff --git a/src/libcamera/v4l2_request.cpp b/src/libcamera/v4l2_request.cpp\n> new file mode 100644\n> index 000000000000..708250d86f61\n> --- /dev/null\n> +++ b/src/libcamera/v4l2_request.cpp\n> @@ -0,0 +1,128 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board\n> + *\n> + * V4L2 Request API\n> + */\n> +\n> +#include \"libcamera/internal/v4l2_request.h\"\n> +\n> +#include <fcntl.h>\n> +#include <stdlib.h>\n> +#include <sys/ioctl.h>\n> +#include <sys/syscall.h>\n> +#include <unistd.h>\n> +\n> +#include <linux/media.h>\n> +\n> +#include <libcamera/base/event_notifier.h>\n> +#include <libcamera/base/log.h>\n> +\n> +/**\n> + * \\file v4l2_request.h\n> + * \\brief V4L2 Request\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(V4L2)\n> +\n> +/**\n> + * \\class V4L2Request\n> + * \\brief V4L2Request object and API\n> + *\n> + * The V4L2Request class wraps a V4L2 request fd and provides some convenience\n> + * functions to handle request.\n> + *\n> + * It is usually constructed by calling \\a MediaDevice::allocateRequests().\n> + *\n> + * A request can then be passed to the V4L2Device::setControls(),\n> + * V4L2Device::getControls() and V4L2VideoDevice::queueBuffer().\n> + */\n> +\n> +/**\n> + * \\brief Construct a V4L2Request\n> + * \\param[in] fd The request fd\n> + */\n> +V4L2Request::V4L2Request(int fd)\n> +\t: fd_(fd), fdNotifier_(fd, EventNotifier::Exception)\n> +{\n> +\tif (!fd_.isValid())\n> +\t\treturn;\n> +\n> +\tfdNotifier_.activated.connect(this, &V4L2Request::requestReady);\n\nIt's probably just me, but I feel like I would just use a simple lambda here.\n\n\n> +\tfdNotifier_.setEnabled(false);\n> +}\n> [...]\n\n\nRegards,\nBarnabás Pőcze","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2EEF2C3259\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 24 Oct 2025 14:00:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6A0496097B;\n\tFri, 24 Oct 2025 16:00:37 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CE47B6096E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 24 Oct 2025 16:00:35 +0200 (CEST)","from [192.168.33.13] (185.221.141.231.nat.pool.zt.hu\n\t[185.221.141.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 30CC71AC5;\n\tFri, 24 Oct 2025 15:58:50 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"BRw+RfXt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761314330;\n\tbh=YOv5jgyqEIfHI7Inw+UnSmDFQ9RvC7V3xE0s9T/HBGQ=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=BRw+RfXtgFvMLf2PD1/XVgNSkVGXgZz6SB9zl9XfriQySIItavKjN7iGe6ndwKhWp\n\tMdkJ8TIHIgBDKr/D89AS/Xm+w03kTQlHWCqtqEUzu41JdOYvuLEPz6XxI9ViqgWRnb\n\tNiPDFF2y8Wk1MBKL3DesIZO/V6MyfcVZzvBoJE5o=","Message-ID":"<18f2c0c1-c9e5-444a-88a2-59956e698abf@ideasonboard.com>","Date":"Fri, 24 Oct 2025 16:00:32 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 05/35] libcamera: Add support for V4L2 requests","To":"Stefan Klug <stefan.klug@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251023144841.403689-1-stefan.klug@ideasonboard.com>\n\t<20251023144841.403689-6-stefan.klug@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251023144841.403689-6-stefan.klug@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":36493,"web_url":"https://patchwork.libcamera.org/comment/36493/","msgid":"<176157786037.59544.9769698925401122053@isaac-ThinkPad-T16-Gen-2>","date":"2025-10-27T15:11:00","subject":"Re: [PATCH v2 05/35] libcamera: Add support for V4L2 requests","submitter":{"id":215,"url":"https://patchwork.libcamera.org/api/people/215/","name":"Isaac Scott","email":"isaac.scott@ideasonboard.com"},"content":"Hi Stefan,\n\nThank you for the patch!\n\nSome nitpicks below:\n\nQuoting Stefan Klug (2025-10-23 15:48:06)\n> The V4L2 requests API provides support to atomically tie controls to a\n> set of buffers. This is especially common for m2m devices. Such a\n> request is represented by a fd that is allocated vi\n\ns/vi/via/\n\n> MEDIA_IOC_REQUEST_ALLOC and then passed to various V4L2 function.\n> \n> Implement a V4L2Request class to wrap such an fd and add the\n> corresponding utility functions.\n> \n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> \n> ---\n> \n> Changes in v2:\n> - Added documentation\n> ---\n>  include/libcamera/internal/media_device.h     |   7 +\n>  include/libcamera/internal/meson.build        |   1 +\n>  include/libcamera/internal/v4l2_device.h      |   5 +-\n>  include/libcamera/internal/v4l2_request.h     |  49 +++++++\n>  include/libcamera/internal/v4l2_videodevice.h |   3 +-\n>  src/libcamera/media_device.cpp                |  47 +++++++\n>  src/libcamera/meson.build                     |   1 +\n>  src/libcamera/v4l2_device.cpp                 |  28 +++-\n>  src/libcamera/v4l2_request.cpp                | 128 ++++++++++++++++++\n>  src/libcamera/v4l2_videodevice.cpp            |  10 +-\n>  10 files changed, 271 insertions(+), 8 deletions(-)\n>  create mode 100644 include/libcamera/internal/v4l2_request.h\n>  create mode 100644 src/libcamera/v4l2_request.cpp\n> \n> diff --git a/include/libcamera/internal/media_device.h b/include/libcamera/internal/media_device.h\n> index b3a48b98d64b..74cb9ba1542d 100644\n> --- a/include/libcamera/internal/media_device.h\n> +++ b/include/libcamera/internal/media_device.h\n> @@ -18,6 +18,7 @@\n>  #include <libcamera/base/unique_fd.h>\n>  \n>  #include \"libcamera/internal/media_object.h\"\n> +#include \"libcamera/internal/v4l2_request.h\"\n>  \n>  namespace libcamera {\n>  \n> @@ -57,6 +58,11 @@ public:\n>  \n>         std::vector<MediaEntity *> locateEntities(unsigned int function);\n>  \n> +       int allocateRequests(unsigned int count,\n> +                            std::vector<std::unique_ptr<V4L2Request>> *requests);\n> +\n> +       bool supportsRequests();\n> +\n>  protected:\n>         std::string logPrefix() const override;\n>  \n> @@ -87,6 +93,7 @@ private:\n>         UniqueFD fd_;\n>         bool valid_;\n>         bool acquired_;\n> +       std::optional<bool> supportsRequests_;\n>  \n>         std::map<unsigned int, MediaObject *> objects_;\n>         std::vector<MediaEntity *> entities_;\n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index 45c299f6a332..e9540a2f734f 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -44,6 +44,7 @@ libcamera_internal_headers = files([\n>      'sysfs.h',\n>      'v4l2_device.h',\n>      'v4l2_pixelformat.h',\n> +    'v4l2_request.h',\n>      'v4l2_subdevice.h',\n>      'v4l2_videodevice.h',\n>      'vector.h',\n> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> index 5bc9da96677d..dbbd118abd00 100644\n> --- a/include/libcamera/internal/v4l2_device.h\n> +++ b/include/libcamera/internal/v4l2_device.h\n> @@ -24,6 +24,7 @@\n>  #include <libcamera/controls.h>\n>  \n>  #include \"libcamera/internal/formats.h\"\n> +#include \"libcamera/internal/v4l2_request.h\"\n>  \n>  namespace libcamera {\n>  \n> @@ -37,8 +38,8 @@ public:\n>  \n>         const ControlInfoMap &controls() const { return controls_; }\n>  \n> -       ControlList getControls(Span<const uint32_t> ids);\n> -       int setControls(ControlList *ctrls);\n> +       ControlList getControls(Span<const uint32_t> ids, const V4L2Request *request = nullptr);\n> +       int setControls(ControlList *ctrls, const V4L2Request *request = nullptr);\n>  \n>         const struct v4l2_query_ext_ctrl *controlInfo(uint32_t id) const;\n>  \n> diff --git a/include/libcamera/internal/v4l2_request.h b/include/libcamera/internal/v4l2_request.h\n> new file mode 100644\n> index 000000000000..bf1bea3261af\n> --- /dev/null\n> +++ b/include/libcamera/internal/v4l2_request.h\n> @@ -0,0 +1,49 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board\n> + *\n> + * V4L2 requests\n> + */\n> +\n> +#pragma once\n> +\n> +#include <string>\n> +\n> +#include <linux/videodev2.h>\n> +\n> +#include <libcamera/base/event_notifier.h>\n> +#include <libcamera/base/log.h>\n> +#include <libcamera/base/signal.h>\n> +#include <libcamera/base/span.h>\n> +#include <libcamera/base/unique_fd.h>\n> +\n> +#include <libcamera/color_space.h>\n> +#include <libcamera/controls.h>\n> +\n> +namespace libcamera {\n> +\n> +class V4L2Request : protected Loggable\n> +{\n> +public:\n> +       bool isValid() const { return fd_.isValid(); }\n> +       int fd() const { return fd_.get(); }\n> +\n> +       int reinit();\n> +       int queue();\n> +\n> +       V4L2Request(int fd = -1);\n> +       ~V4L2Request() = default;\n> +\n> +       Signal<V4L2Request *> requestDone;\n> +\n> +private:\n> +       LIBCAMERA_DISABLE_COPY_AND_MOVE(V4L2Request)\n> +\n> +       void requestReady();\n> +       std::string logPrefix() const override;\n> +\n> +       UniqueFD fd_;\n> +       EventNotifier fdNotifier_;\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> index 5a7dcfdda118..2d290971a0ee 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -33,6 +33,7 @@\n>  #include \"libcamera/internal/formats.h\"\n>  #include \"libcamera/internal/v4l2_device.h\"\n>  #include \"libcamera/internal/v4l2_pixelformat.h\"\n> +#include \"libcamera/internal/v4l2_request.h\"\n>  \n>  namespace libcamera {\n>  \n> @@ -217,7 +218,7 @@ public:\n>         int importBuffers(unsigned int count);\n>         int releaseBuffers();\n>  \n> -       int queueBuffer(FrameBuffer *buffer);\n> +       int queueBuffer(FrameBuffer *buffer, const V4L2Request *request = nullptr);\n>         Signal<FrameBuffer *> bufferReady;\n>  \n>         int streamOn();\n> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n> index 353f34a81eca..673c53fefdd7 100644\n> --- a/src/libcamera/media_device.cpp\n> +++ b/src/libcamera/media_device.cpp\n> @@ -20,6 +20,7 @@\n>  #include <linux/media.h>\n>  \n>  #include <libcamera/base/log.h>\n> +#include \"libcamera/internal/v4l2_request.h\"\n>  \n>  /**\n>   * \\file media_device.h\n> @@ -851,4 +852,50 @@ std::vector<MediaEntity *> MediaDevice::locateEntities(unsigned int function)\n>         return found;\n>  }\n>  \n> +/**\n> + * \\brief Allocate requests\n> + * \\param[in] count Number of requests to allocate\n> + * \\param[out] requests Vector to store allocated requests\n> + *\n> + * Allocates and stores \\a count requests in \\a requests. If allocation fails,\n> + * and error is returned and \\a requests is cleared.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int MediaDevice::allocateRequests(unsigned int count,\n> +                                 std::vector<std::unique_ptr<V4L2Request>> *requests)\n> +{\n> +       requests->resize(count);\n> +       for (unsigned int i = 0; i < count; i++) {\n> +               int requestFd;\n> +               int ret = ::ioctl(fd_.get(), MEDIA_IOC_REQUEST_ALLOC, &requestFd);\n> +               if (ret < 0) {\n> +                       requests->clear();\n> +                       return -errno;\n> +               }\n> +               (*requests)[i] = std::make_unique<V4L2Request>(requestFd);\n> +       }\n> +\n> +       return 0;\n> +}\n> +\n> +/**\n> + * \\brief Check if requests are supported\n> + *\n> + * Checks if the device supports V4L2 requests by trying to allocate a single\n> + * request. The result is cached, so the allocation is only tried once.\n> + *\n> + * \\return True if the device supports requests, false otherwise\n> + */\n> +bool MediaDevice::supportsRequests()\n> +{\n> +       if (supportsRequests_.has_value())\n> +               return supportsRequests_.value();\n> +\n> +       std::vector<std::unique_ptr<V4L2Request>> requests;\n> +       supportsRequests_ = (allocateRequests(1, &requests) == 0);\n> +\n> +       return supportsRequests_.value();\n> +}\n> +\n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 5b9b86f211f1..34e20f557514 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -54,6 +54,7 @@ libcamera_internal_sources = files([\n>      'sysfs.cpp',\n>      'v4l2_device.cpp',\n>      'v4l2_pixelformat.cpp',\n> +    'v4l2_request.cpp',\n>      'v4l2_subdevice.cpp',\n>      'v4l2_videodevice.cpp',\n>      'vector.cpp',\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 8c78b8c424e5..7a669a0303c1 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -162,6 +162,7 @@ void V4L2Device::close()\n>  /**\n>   * \\brief Read controls from the device\n>   * \\param[in] ids The list of controls to read, specified by their ID\n> + * \\param[in] request An optional request\n>   *\n>   * This function reads the value of all controls contained in \\a ids, and\n>   * returns their values as a ControlList.\n> @@ -171,10 +172,12 @@ void V4L2Device::close()\n>   * during validation of the requested controls, no control is read and this\n>   * function returns an empty control list.\n>   *\n> + * If \\a request is specified the controls tied to that request are read.\n> + *\n>   * \\return The control values in a ControlList on success, or an empty list on\n>   * error\n>   */\n> -ControlList V4L2Device::getControls(Span<const uint32_t> ids)\n> +ControlList V4L2Device::getControls(Span<const uint32_t> ids, const V4L2Request *request)\n>  {\n>         if (ids.empty())\n>                 return {};\n> @@ -242,10 +245,16 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids)\n>         }\n>  \n>         struct v4l2_ext_controls v4l2ExtCtrls = {};\n> -       v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL;\n>         v4l2ExtCtrls.controls = v4l2Ctrls.data();\n>         v4l2ExtCtrls.count = v4l2Ctrls.size();\n>  \n> +       if (request) {\n> +               v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL;\n> +               v4l2ExtCtrls.request_fd = request->fd();\n> +       } else {\n> +               v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL;\n> +       }\n> +\n>         int ret = ioctl(VIDIOC_G_EXT_CTRLS, &v4l2ExtCtrls);\n>         if (ret) {\n>                 unsigned int errorIdx = v4l2ExtCtrls.error_idx;\n> @@ -273,6 +282,7 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids)\n>  /**\n>   * \\brief Write controls to the device\n>   * \\param[in] ctrls The list of controls to write\n> + * \\param[in] request And optional request\n\ns/And/An/ ?\n\n>   *\n>   * This function writes the value of all controls contained in \\a ctrls, and\n>   * stores the values actually applied to the device in the corresponding\n> @@ -288,11 +298,15 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids)\n>   * are written and their values are updated in \\a ctrls, while all other\n>   * controls are not written and their values are not changed.\n>   *\n> + * If \\a request is set, the controls will be applied to that request. If the\n> + * device doesn't support requests, -EACCESS will be returned. If \\a request is\n> + * invalid, -EINVAL will be returned.\n> + *\n>   * \\return 0 on success or an error code otherwise\n>   * \\retval -EINVAL One of the control is not supported or not accessible\n\ns/control/controls/\n\n>   * \\retval i The index of the control that failed\n>   */\n> -int V4L2Device::setControls(ControlList *ctrls)\n> +int V4L2Device::setControls(ControlList *ctrls, const V4L2Request *request)\n>  {\n>         if (ctrls->empty())\n>                 return 0;\n> @@ -377,10 +391,16 @@ int V4L2Device::setControls(ControlList *ctrls)\n>         }\n>  \n>         struct v4l2_ext_controls v4l2ExtCtrls = {};\n> -       v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL;\n>         v4l2ExtCtrls.controls = v4l2Ctrls.data();\n>         v4l2ExtCtrls.count = v4l2Ctrls.size();\n>  \n> +       if (request) {\n> +               v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL;\n> +               v4l2ExtCtrls.request_fd = request->fd();\n> +       } else {\n> +               v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL;\n> +       }\n> +\n>         int ret = ioctl(VIDIOC_S_EXT_CTRLS, &v4l2ExtCtrls);\n>         if (ret) {\n>                 unsigned int errorIdx = v4l2ExtCtrls.error_idx;\n> diff --git a/src/libcamera/v4l2_request.cpp b/src/libcamera/v4l2_request.cpp\n> new file mode 100644\n> index 000000000000..708250d86f61\n> --- /dev/null\n> +++ b/src/libcamera/v4l2_request.cpp\n> @@ -0,0 +1,128 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board\n> + *\n> + * V4L2 Request API\n> + */\n> +\n> +#include \"libcamera/internal/v4l2_request.h\"\n> +\n> +#include <fcntl.h>\n> +#include <stdlib.h>\n> +#include <sys/ioctl.h>\n> +#include <sys/syscall.h>\n> +#include <unistd.h>\n> +\n> +#include <linux/media.h>\n> +\n> +#include <libcamera/base/event_notifier.h>\n> +#include <libcamera/base/log.h>\n> +\n> +/**\n> + * \\file v4l2_request.h\n> + * \\brief V4L2 Request\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(V4L2)\n> +\n> +/**\n> + * \\class V4L2Request\n> + * \\brief V4L2Request object and API\n> + *\n> + * The V4L2Request class wraps a V4L2 request fd and provides some convenience\n> + * functions to handle request.\n> + *\n> + * It is usually constructed by calling \\a MediaDevice::allocateRequests().\n> + *\n> + * A request can then be passed to the V4L2Device::setControls(),\n> + * V4L2Device::getControls() and V4L2VideoDevice::queueBuffer().\n> + */\n> +\n> +/**\n> + * \\brief Construct a V4L2Request\n> + * \\param[in] fd The request fd\n> + */\n> +V4L2Request::V4L2Request(int fd)\n> +       : fd_(fd), fdNotifier_(fd, EventNotifier::Exception)\n> +{\n> +       if (!fd_.isValid())\n> +               return;\n> +\n> +       fdNotifier_.activated.connect(this, &V4L2Request::requestReady);\n> +       fdNotifier_.setEnabled(false);\n> +}\n> +\n> +/**\n> + * \\fn V4L2Request::isValid()\n> + * \\brief Check if the request is valid\n> + *\n> + * Checks if the underlying fd is valid.\n> + *\n> + * \\return True if the request is valid, false otherwise\n> + */\n> +\n> +/**\n> + * \\fn V4L2Request::fd()\n> + * \\brief Get the file descriptor\n> + *\n> + * \\return The file descriptor wrapped by this V4L2Request\n> + */\n> +\n> +/**\n> + * \\var V4L2Request::requestDone\n> + * \\brief Signal that is emitted when the request is done\n> + */\n> +\n> +/**\n> + * \\brief Reinit the request\n> + *\n> + * Calls MEDIA_REQUEST_IOC_REINIT om the request fd.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int V4L2Request::reinit()\n> +{\n> +       fdNotifier_.setEnabled(false);\n> +\n> +       if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_REINIT) < 0)\n> +               return -errno;\n> +\n> +       return 0;\n> +}\n> +\n> +/**\n> + * \\brief Reinit the request\n> + *\n> + * Calls MEDIA_REQUEST_IOC_QUEUE om the request fd.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int V4L2Request::queue()\n> +{\n> +       if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_QUEUE) < 0)\n> +               return -errno;\n> +\n> +       fdNotifier_.setEnabled(true);\n> +\n> +       return 0;\n> +}\n> +\n> +std::string V4L2Request::logPrefix() const\n> +{\n> +       return \"Request [\" + std::to_string(fd()) + \"]\";\n> +}\n> +\n> +/**\n> + * \\brief Slot to handle request done events\n> + *\n> + * When this slot is called, the request is done and the requestDone will be\n> + * emitted.\n> + */\n> +void V4L2Request::requestReady()\n> +{\n> +       requestDone.emit(this);\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index bb57c1b76a5b..8ce739f4bc65 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -30,6 +30,7 @@\n>  #include \"libcamera/internal/framebuffer.h\"\n>  #include \"libcamera/internal/media_device.h\"\n>  #include \"libcamera/internal/media_object.h\"\n> +#include \"libcamera/internal/v4l2_request.h\"\n>  \n>  /**\n>   * \\file v4l2_videodevice.h\n> @@ -1629,6 +1630,7 @@ int V4L2VideoDevice::releaseBuffers()\n>  /**\n>   * \\brief Queue a buffer to the video device\n>   * \\param[in] buffer The buffer to be queued\n> + * \\param[in] request An optional request\n>   *\n>   * For capture video devices the \\a buffer will be filled with data by the\n>   * device. For output video devices the \\a buffer shall contain valid data and\n> @@ -1641,9 +1643,11 @@ int V4L2VideoDevice::releaseBuffers()\n>   * Note that queueBuffer() will fail if the device is in the process of being\n>   * stopped from a streaming state through streamOff().\n>   *\n> + * If \\a request is specified, the buffer will be tied to that request.\n> + *\n>   * \\return 0 on success or a negative error code otherwise\n>   */\n> -int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)\n> +int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer, const V4L2Request *request)\n>  {\n\nBest wishes,\n\nIsaac\n\n>         struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {};\n>         struct v4l2_buffer buf = {};\n> @@ -1674,6 +1678,10 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)\n>         buf.type = bufferType_;\n>         buf.memory = memoryType_;\n>         buf.field = V4L2_FIELD_NONE;\n> +       if (request) {\n> +               buf.flags = V4L2_BUF_FLAG_REQUEST_FD;\n> +               buf.request_fd = request->fd();\n> +       }\n>  \n>         bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type);\n>         Span<const FrameBuffer::Plane> planes = buffer->planes();\n> -- \n> 2.48.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 152F5BE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 27 Oct 2025 15:11:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 133A560768;\n\tMon, 27 Oct 2025 16:11:05 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 545736069A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Oct 2025 16:11:03 +0100 (CET)","from thinkpad.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 425A77FA;\n\tMon, 27 Oct 2025 16:09:15 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"v+tg3+cq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761577755;\n\tbh=X+zuiVgMm3rxQylqQzPLQngIY9QmUrtBbALugCCRfvw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=v+tg3+cqjtvQ2st3gtlKrfWRr6s7VvxfbQMnC6+kS+0oJN1fLqZQw0B4S2eo3rLk9\n\t9pFbu75J6mtCFvlRmOtzRjspSQWGZrBdyf6O9RW08pwFHloKtcFYuGCc4cO9+qez0k\n\tBazAbxu05BZQk1PrRjE7h0mpxc154khxVzbplJJI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251023144841.403689-6-stefan.klug@ideasonboard.com>","References":"<20251023144841.403689-1-stefan.klug@ideasonboard.com>\n\t<20251023144841.403689-6-stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH v2 05/35] libcamera: Add support for V4L2 requests","From":"Isaac Scott <isaac.scott@ideasonboard.com>","Cc":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 27 Oct 2025 15:11:00 +0000","Message-ID":"<176157786037.59544.9769698925401122053@isaac-ThinkPad-T16-Gen-2>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":36706,"web_url":"https://patchwork.libcamera.org/comment/36706/","msgid":"<176236116296.2116251.11332755061547134434@neptunite.rasen.tech>","date":"2025-11-05T16:46:02","subject":"Re: [PATCH v2 05/35] libcamera: Add support for V4L2 requests","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Quoting Stefan Klug (2025-10-23 23:48:06)\n> The V4L2 requests API provides support to atomically tie controls to a\n> set of buffers. This is especially common for m2m devices. Such a\n> request is represented by a fd that is allocated vi\n\ns/ a / an / s/vi/via/\n\n> MEDIA_IOC_REQUEST_ALLOC and then passed to various V4L2 function.\n\ns/function/functions/\n\n> \n> Implement a V4L2Request class to wrap such an fd and add the\n> corresponding utility functions.\n> \n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> \n> ---\n> \n> Changes in v2:\n> - Added documentation\n> ---\n>  include/libcamera/internal/media_device.h     |   7 +\n>  include/libcamera/internal/meson.build        |   1 +\n>  include/libcamera/internal/v4l2_device.h      |   5 +-\n>  include/libcamera/internal/v4l2_request.h     |  49 +++++++\n>  include/libcamera/internal/v4l2_videodevice.h |   3 +-\n>  src/libcamera/media_device.cpp                |  47 +++++++\n>  src/libcamera/meson.build                     |   1 +\n>  src/libcamera/v4l2_device.cpp                 |  28 +++-\n>  src/libcamera/v4l2_request.cpp                | 128 ++++++++++++++++++\n>  src/libcamera/v4l2_videodevice.cpp            |  10 +-\n>  10 files changed, 271 insertions(+), 8 deletions(-)\n>  create mode 100644 include/libcamera/internal/v4l2_request.h\n>  create mode 100644 src/libcamera/v4l2_request.cpp\n> \n> diff --git a/include/libcamera/internal/media_device.h b/include/libcamera/internal/media_device.h\n> index b3a48b98d64b..74cb9ba1542d 100644\n> --- a/include/libcamera/internal/media_device.h\n> +++ b/include/libcamera/internal/media_device.h\n> @@ -18,6 +18,7 @@\n>  #include <libcamera/base/unique_fd.h>\n>  \n>  #include \"libcamera/internal/media_object.h\"\n> +#include \"libcamera/internal/v4l2_request.h\"\n>  \n>  namespace libcamera {\n>  \n> @@ -57,6 +58,11 @@ public:\n>  \n>         std::vector<MediaEntity *> locateEntities(unsigned int function);\n>  \n> +       int allocateRequests(unsigned int count,\n> +                            std::vector<std::unique_ptr<V4L2Request>> *requests);\n> +\n> +       bool supportsRequests();\n> +\n>  protected:\n>         std::string logPrefix() const override;\n>  \n> @@ -87,6 +93,7 @@ private:\n>         UniqueFD fd_;\n>         bool valid_;\n>         bool acquired_;\n> +       std::optional<bool> supportsRequests_;\n>  \n>         std::map<unsigned int, MediaObject *> objects_;\n>         std::vector<MediaEntity *> entities_;\n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index 45c299f6a332..e9540a2f734f 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -44,6 +44,7 @@ libcamera_internal_headers = files([\n>      'sysfs.h',\n>      'v4l2_device.h',\n>      'v4l2_pixelformat.h',\n> +    'v4l2_request.h',\n>      'v4l2_subdevice.h',\n>      'v4l2_videodevice.h',\n>      'vector.h',\n> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> index 5bc9da96677d..dbbd118abd00 100644\n> --- a/include/libcamera/internal/v4l2_device.h\n> +++ b/include/libcamera/internal/v4l2_device.h\n> @@ -24,6 +24,7 @@\n>  #include <libcamera/controls.h>\n>  \n>  #include \"libcamera/internal/formats.h\"\n> +#include \"libcamera/internal/v4l2_request.h\"\n>  \n>  namespace libcamera {\n>  \n> @@ -37,8 +38,8 @@ public:\n>  \n>         const ControlInfoMap &controls() const { return controls_; }\n>  \n> -       ControlList getControls(Span<const uint32_t> ids);\n> -       int setControls(ControlList *ctrls);\n> +       ControlList getControls(Span<const uint32_t> ids, const V4L2Request *request = nullptr);\n> +       int setControls(ControlList *ctrls, const V4L2Request *request = nullptr);\n>  \n>         const struct v4l2_query_ext_ctrl *controlInfo(uint32_t id) const;\n>  \n> diff --git a/include/libcamera/internal/v4l2_request.h b/include/libcamera/internal/v4l2_request.h\n> new file mode 100644\n> index 000000000000..bf1bea3261af\n> --- /dev/null\n> +++ b/include/libcamera/internal/v4l2_request.h\n> @@ -0,0 +1,49 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board\n> + *\n> + * V4L2 requests\n> + */\n> +\n> +#pragma once\n> +\n> +#include <string>\n> +\n> +#include <linux/videodev2.h>\n> +\n> +#include <libcamera/base/event_notifier.h>\n> +#include <libcamera/base/log.h>\n> +#include <libcamera/base/signal.h>\n> +#include <libcamera/base/span.h>\n> +#include <libcamera/base/unique_fd.h>\n> +\n> +#include <libcamera/color_space.h>\n> +#include <libcamera/controls.h>\n\nDo we need these two headers?\n\n> +\n> +namespace libcamera {\n> +\n> +class V4L2Request : protected Loggable\n> +{\n> +public:\n> +       bool isValid() const { return fd_.isValid(); }\n> +       int fd() const { return fd_.get(); }\n> +\n> +       int reinit();\n> +       int queue();\n> +\n> +       V4L2Request(int fd = -1);\n> +       ~V4L2Request() = default;\n> +\n> +       Signal<V4L2Request *> requestDone;\n> +\n> +private:\n> +       LIBCAMERA_DISABLE_COPY_AND_MOVE(V4L2Request)\n> +\n> +       void requestReady();\n> +       std::string logPrefix() const override;\n> +\n> +       UniqueFD fd_;\n> +       EventNotifier fdNotifier_;\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> index 5a7dcfdda118..2d290971a0ee 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -33,6 +33,7 @@\n>  #include \"libcamera/internal/formats.h\"\n>  #include \"libcamera/internal/v4l2_device.h\"\n>  #include \"libcamera/internal/v4l2_pixelformat.h\"\n> +#include \"libcamera/internal/v4l2_request.h\"\n>  \n>  namespace libcamera {\n\n>  \n> @@ -217,7 +218,7 @@ public:\n\n>         int importBuffers(unsigned int count);\n>         int releaseBuffers();\n>  \n> -       int queueBuffer(FrameBuffer *buffer);\n> +       int queueBuffer(FrameBuffer *buffer, const V4L2Request *request = nullptr);\n>         Signal<FrameBuffer *> bufferReady;\n>  \n>         int streamOn();\n> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n> index 353f34a81eca..673c53fefdd7 100644\n> --- a/src/libcamera/media_device.cpp\n> +++ b/src/libcamera/media_device.cpp\n> @@ -20,6 +20,7 @@\n>  #include <linux/media.h>\n>  \n>  #include <libcamera/base/log.h>\n> +#include \"libcamera/internal/v4l2_request.h\"\n>  \n>  /**\n>   * \\file media_device.h\n> @@ -851,4 +852,50 @@ std::vector<MediaEntity *> MediaDevice::locateEntities(unsigned int function)\n>         return found;\n>  }\n>  \n> +/**\n> + * \\brief Allocate requests\n> + * \\param[in] count Number of requests to allocate\n> + * \\param[out] requests Vector to store allocated requests\n> + *\n> + * Allocates and stores \\a count requests in \\a requests. If allocation fails,\n> + * and error is returned and \\a requests is cleared.\n\ns/\\* and/\\* an/\n\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int MediaDevice::allocateRequests(unsigned int count,\n> +                                 std::vector<std::unique_ptr<V4L2Request>> *requests)\n> +{\n> +       requests->resize(count);\n> +       for (unsigned int i = 0; i < count; i++) {\n> +               int requestFd;\n> +               int ret = ::ioctl(fd_.get(), MEDIA_IOC_REQUEST_ALLOC, &requestFd);\n\nI'm of the opinion that this should be moved into V4L2Request constructor (or\ninit()) so that we can keep all requests handling there. We already have an\nisValid(), and maybe we can add close() to the deconstrctor?\n\n> +               if (ret < 0) {\n> +                       requests->clear();\n> +                       return -errno;\n> +               }\n> +               (*requests)[i] = std::make_unique<V4L2Request>(requestFd);\n> +       }\n> +\n> +       return 0;\n> +}\n> +\n> +/**\n> + * \\brief Check if requests are supported\n> + *\n> + * Checks if the device supports V4L2 requests by trying to allocate a single\n> + * request. The result is cached, so the allocation is only tried once.\n> + *\n> + * \\return True if the device supports requests, false otherwise\n> + */\n> +bool MediaDevice::supportsRequests()\n> +{\n> +       if (supportsRequests_.has_value())\n> +               return supportsRequests_.value();\n> +\n> +       std::vector<std::unique_ptr<V4L2Request>> requests;\n> +       supportsRequests_ = (allocateRequests(1, &requests) == 0);\n\nInitially I was worried about subsequent allocations but then I realized that\nthis will get deallocated after it goes out of scope.\n\nAlthough there is currently no other errno than ENOTTY, the non-explicit check\nmakes me uncomfortable :S\n\n> +\n> +       return supportsRequests_.value();\n> +}\n> +\n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 5b9b86f211f1..34e20f557514 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -54,6 +54,7 @@ libcamera_internal_sources = files([\n>      'sysfs.cpp',\n>      'v4l2_device.cpp',\n>      'v4l2_pixelformat.cpp',\n> +    'v4l2_request.cpp',\n>      'v4l2_subdevice.cpp',\n>      'v4l2_videodevice.cpp',\n>      'vector.cpp',\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 8c78b8c424e5..7a669a0303c1 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -162,6 +162,7 @@ void V4L2Device::close()\n>  /**\n>   * \\brief Read controls from the device\n>   * \\param[in] ids The list of controls to read, specified by their ID\n> + * \\param[in] request An optional request\n>   *\n>   * This function reads the value of all controls contained in \\a ids, and\n>   * returns their values as a ControlList.\n> @@ -171,10 +172,12 @@ void V4L2Device::close()\n>   * during validation of the requested controls, no control is read and this\n>   * function returns an empty control list.\n>   *\n> + * If \\a request is specified the controls tied to that request are read.\n> + *\n>   * \\return The control values in a ControlList on success, or an empty list on\n>   * error\n>   */\n> -ControlList V4L2Device::getControls(Span<const uint32_t> ids)\n> +ControlList V4L2Device::getControls(Span<const uint32_t> ids, const V4L2Request *request)\n>  {\n>         if (ids.empty())\n>                 return {};\n> @@ -242,10 +245,16 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids)\n>         }\n>  \n>         struct v4l2_ext_controls v4l2ExtCtrls = {};\n> -       v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL;\n>         v4l2ExtCtrls.controls = v4l2Ctrls.data();\n>         v4l2ExtCtrls.count = v4l2Ctrls.size();\n>  \n> +       if (request) {\n> +               v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL;\n> +               v4l2ExtCtrls.request_fd = request->fd();\n> +       } else {\n> +               v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL;\n> +       }\n> +\n>         int ret = ioctl(VIDIOC_G_EXT_CTRLS, &v4l2ExtCtrls);\n>         if (ret) {\n>                 unsigned int errorIdx = v4l2ExtCtrls.error_idx;\n> @@ -273,6 +282,7 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids)\n>  /**\n>   * \\brief Write controls to the device\n>   * \\param[in] ctrls The list of controls to write\n> + * \\param[in] request And optional request\n\ns/And/An/\n\n>   *\n>   * This function writes the value of all controls contained in \\a ctrls, and\n>   * stores the values actually applied to the device in the corresponding\n> @@ -288,11 +298,15 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids)\n>   * are written and their values are updated in \\a ctrls, while all other\n>   * controls are not written and their values are not changed.\n>   *\n> + * If \\a request is set, the controls will be applied to that request. If the\n> + * device doesn't support requests, -EACCESS will be returned. If \\a request is\n> + * invalid, -EINVAL will be returned.\n\nDo you need to mention EACCESS in the docunentation for read as well?\n\n> + *\n>   * \\return 0 on success or an error code otherwise\n>   * \\retval -EINVAL One of the control is not supported or not accessible\n>   * \\retval i The index of the control that failed\n>   */\n> -int V4L2Device::setControls(ControlList *ctrls)\n> +int V4L2Device::setControls(ControlList *ctrls, const V4L2Request *request)\n>  {\n>         if (ctrls->empty())\n>                 return 0;\n> @@ -377,10 +391,16 @@ int V4L2Device::setControls(ControlList *ctrls)\n>         }\n>  \n>         struct v4l2_ext_controls v4l2ExtCtrls = {};\n> -       v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL;\n>         v4l2ExtCtrls.controls = v4l2Ctrls.data();\n>         v4l2ExtCtrls.count = v4l2Ctrls.size();\n>  \n> +       if (request) {\n> +               v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL;\n> +               v4l2ExtCtrls.request_fd = request->fd();\n> +       } else {\n> +               v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL;\n> +       }\n> +\n>         int ret = ioctl(VIDIOC_S_EXT_CTRLS, &v4l2ExtCtrls);\n>         if (ret) {\n>                 unsigned int errorIdx = v4l2ExtCtrls.error_idx;\n> diff --git a/src/libcamera/v4l2_request.cpp b/src/libcamera/v4l2_request.cpp\n> new file mode 100644\n> index 000000000000..708250d86f61\n> --- /dev/null\n> +++ b/src/libcamera/v4l2_request.cpp\n> @@ -0,0 +1,128 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board\n> + *\n> + * V4L2 Request API\n> + */\n> +\n> +#include \"libcamera/internal/v4l2_request.h\"\n> +\n> +#include <fcntl.h>\n> +#include <stdlib.h>\n> +#include <sys/ioctl.h>\n> +#include <sys/syscall.h>\n> +#include <unistd.h>\n> +\n> +#include <linux/media.h>\n> +\n> +#include <libcamera/base/event_notifier.h>\n> +#include <libcamera/base/log.h>\n> +\n> +/**\n> + * \\file v4l2_request.h\n> + * \\brief V4L2 Request\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(V4L2)\n> +\n> +/**\n> + * \\class V4L2Request\n> + * \\brief V4L2Request object and API\n> + *\n> + * The V4L2Request class wraps a V4L2 request fd and provides some convenience\n> + * functions to handle request.\n> + *\n> + * It is usually constructed by calling \\a MediaDevice::allocateRequests().\n> + *\n> + * A request can then be passed to the V4L2Device::setControls(),\n> + * V4L2Device::getControls() and V4L2VideoDevice::queueBuffer().\n> + */\n> +\n> +/**\n> + * \\brief Construct a V4L2Request\n> + * \\param[in] fd The request fd\n> + */\n> +V4L2Request::V4L2Request(int fd)\n> +       : fd_(fd), fdNotifier_(fd, EventNotifier::Exception)\n> +{\n> +       if (!fd_.isValid())\n> +               return;\n> +\n> +       fdNotifier_.activated.connect(this, &V4L2Request::requestReady);\n> +       fdNotifier_.setEnabled(false);\n> +}\n> +\n> +/**\n> + * \\fn V4L2Request::isValid()\n> + * \\brief Check if the request is valid\n> + *\n> + * Checks if the underlying fd is valid.\n> + *\n> + * \\return True if the request is valid, false otherwise\n> + */\n> +\n> +/**\n> + * \\fn V4L2Request::fd()\n> + * \\brief Get the file descriptor\n> + *\n> + * \\return The file descriptor wrapped by this V4L2Request\n> + */\n> +\n> +/**\n> + * \\var V4L2Request::requestDone\n> + * \\brief Signal that is emitted when the request is done\n> + */\n> +\n> +/**\n> + * \\brief Reinit the request\n> + *\n> + * Calls MEDIA_REQUEST_IOC_REINIT om the request fd.\n\ns/om/on/\n\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int V4L2Request::reinit()\n> +{\n> +       fdNotifier_.setEnabled(false);\n> +\n> +       if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_REINIT) < 0)\n> +               return -errno;\n> +\n> +       return 0;\n> +}\n> +\n> +/**\n> + * \\brief Reinit the request\n\nReinit? :)\n\n> + *\n> + * Calls MEDIA_REQUEST_IOC_QUEUE om the request fd.\n\ns/om/on/\n\n\nThanks,\n\nPaul\n\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int V4L2Request::queue()\n> +{\n> +       if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_QUEUE) < 0)\n> +               return -errno;\n> +\n> +       fdNotifier_.setEnabled(true);\n> +\n> +       return 0;\n> +}\n> +\n> +std::string V4L2Request::logPrefix() const\n> +{\n> +       return \"Request [\" + std::to_string(fd()) + \"]\";\n> +}\n> +\n> +/**\n> + * \\brief Slot to handle request done events\n> + *\n> + * When this slot is called, the request is done and the requestDone will be\n> + * emitted.\n> + */\n> +void V4L2Request::requestReady()\n> +{\n> +       requestDone.emit(this);\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index bb57c1b76a5b..8ce739f4bc65 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -30,6 +30,7 @@\n>  #include \"libcamera/internal/framebuffer.h\"\n>  #include \"libcamera/internal/media_device.h\"\n>  #include \"libcamera/internal/media_object.h\"\n> +#include \"libcamera/internal/v4l2_request.h\"\n>  \n>  /**\n>   * \\file v4l2_videodevice.h\n> @@ -1629,6 +1630,7 @@ int V4L2VideoDevice::releaseBuffers()\n>  /**\n>   * \\brief Queue a buffer to the video device\n>   * \\param[in] buffer The buffer to be queued\n> + * \\param[in] request An optional request\n>   *\n>   * For capture video devices the \\a buffer will be filled with data by the\n>   * device. For output video devices the \\a buffer shall contain valid data and\n> @@ -1641,9 +1643,11 @@ int V4L2VideoDevice::releaseBuffers()\n>   * Note that queueBuffer() will fail if the device is in the process of being\n>   * stopped from a streaming state through streamOff().\n>   *\n> + * If \\a request is specified, the buffer will be tied to that request.\n> + *\n>   * \\return 0 on success or a negative error code otherwise\n>   */\n> -int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)\n> +int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer, const V4L2Request *request)\n>  {\n>         struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {};\n>         struct v4l2_buffer buf = {};\n> @@ -1674,6 +1678,10 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)\n>         buf.type = bufferType_;\n>         buf.memory = memoryType_;\n>         buf.field = V4L2_FIELD_NONE;\n> +       if (request) {\n> +               buf.flags = V4L2_BUF_FLAG_REQUEST_FD;\n> +               buf.request_fd = request->fd();\n> +       }\n>  \n>         bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type);\n>         Span<const FrameBuffer::Plane> planes = buffer->planes();\n> -- \n> 2.48.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9C0B5BDE4C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  5 Nov 2025 16:46:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4766B60A81;\n\tWed,  5 Nov 2025 17:46:11 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B919E609D8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  5 Nov 2025 17:46:09 +0100 (CET)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:d4d0:27ea:7a74:8a9e])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id BDD6FE45;\n\tWed,  5 Nov 2025 17:44:14 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"doiiic0P\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1762361055;\n\tbh=IZbWVoQxTMIDlFhstx9RTJwVScWsgpt0apLETZ9p/cM=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=doiiic0PNhdbQaPWarLRskjAVzx/qYJziz8NP2yU8K+0KHe5UDLxRBNmjmv7/VCra\n\t2H0OAbF5muY/g2vMXGBSGkYaLe5VfGO1buF4OLeuuJjC6z3YKNLnrbnyej1F9h2cxi\n\tz1OBqwy7XIlnnnijsJc/0uQVvNY48TTOEKrz0rdw=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251023144841.403689-6-stefan.klug@ideasonboard.com>","References":"<20251023144841.403689-1-stefan.klug@ideasonboard.com>\n\t<20251023144841.403689-6-stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH v2 05/35] libcamera: Add support for V4L2 requests","From":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 06 Nov 2025 01:46:02 +0900","Message-ID":"<176236116296.2116251.11332755061547134434@neptunite.rasen.tech>","User-Agent":"alot/0.0.0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":37029,"web_url":"https://patchwork.libcamera.org/comment/37029/","msgid":"<176399806422.1350675.17922189972244077830@localhost>","date":"2025-11-24T15:27:44","subject":"Re: [PATCH v2 05/35] libcamera: Add support for V4L2 requests","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Barnabás,\n\nThank you for the review.\n\nAs always I should have spent more time reading the review earlier and\nnot just before prearing the new series...\n\nQuoting Barnabás Pőcze (2025-10-24 16:00:32)\n> Hi\n> \n> 2025. 10. 23. 16:48 keltezéssel, Stefan Klug írta:\n> > The V4L2 requests API provides support to atomically tie controls to a\n> > set of buffers. This is especially common for m2m devices. Such a\n> > request is represented by a fd that is allocated vi\n> > MEDIA_IOC_REQUEST_ALLOC and then passed to various V4L2 function.\n> > \n> > Implement a V4L2Request class to wrap such an fd and add the\n> > corresponding utility functions.\n> > \n> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> > \n> > ---\n> > \n> > Changes in v2:\n> > - Added documentation\n> > ---\n> >   include/libcamera/internal/media_device.h     |   7 +\n> >   include/libcamera/internal/meson.build        |   1 +\n> >   include/libcamera/internal/v4l2_device.h      |   5 +-\n> >   include/libcamera/internal/v4l2_request.h     |  49 +++++++\n> >   include/libcamera/internal/v4l2_videodevice.h |   3 +-\n> >   src/libcamera/media_device.cpp                |  47 +++++++\n> >   src/libcamera/meson.build                     |   1 +\n> >   src/libcamera/v4l2_device.cpp                 |  28 +++-\n> >   src/libcamera/v4l2_request.cpp                | 128 ++++++++++++++++++\n> >   src/libcamera/v4l2_videodevice.cpp            |  10 +-\n> >   10 files changed, 271 insertions(+), 8 deletions(-)\n> >   create mode 100644 include/libcamera/internal/v4l2_request.h\n> >   create mode 100644 src/libcamera/v4l2_request.cpp\n> > \n> > [...]\n> > diff --git a/include/libcamera/internal/v4l2_request.h b/include/libcamera/internal/v4l2_request.h\n> > new file mode 100644\n> > index 000000000000..bf1bea3261af\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/v4l2_request.h\n> > @@ -0,0 +1,49 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025, Ideas On Board\n> > + *\n> > + * V4L2 requests\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include <string>\n> > +\n> > +#include <linux/videodev2.h>\n> > +\n> > +#include <libcamera/base/event_notifier.h>\n> > +#include <libcamera/base/log.h>\n> > +#include <libcamera/base/signal.h>\n> > +#include <libcamera/base/span.h>\n> > +#include <libcamera/base/unique_fd.h>\n> > +\n> > +#include <libcamera/color_space.h>\n> > +#include <libcamera/controls.h>\n> > +\n> > +namespace libcamera {\n> > +\n> > +class V4L2Request : protected Loggable\n> > +{\n> > +public:\n> > +     bool isValid() const { return fd_.isValid(); }\n> > +     int fd() const { return fd_.get(); }\n> > +\n> > +     int reinit();\n> > +     int queue();\n> > +\n> > +     V4L2Request(int fd = -1);\n> \n> I think just taking `UniqueFD` would be better as it makes the ownership clear.\n> \n> Is it is useful to create a `V4L2Request` object without a valid file descriptor?\n> There is no way to change the file descriptor and as far as I can tell most methods\n> would just fail.\n\nSure, I fixed that.\n\n> \n> \n> > +     ~V4L2Request() = default;\n> \n> I think this is already implied.\n> \n> \n> > +\n> > +     Signal<V4L2Request *> requestDone;\n> > +\n> > +private:\n> > +     LIBCAMERA_DISABLE_COPY_AND_MOVE(V4L2Request)\n> > +\n> > +     void requestReady();\n> > +     std::string logPrefix() const override;\n> > +\n> > +     UniqueFD fd_;\n> > +     EventNotifier fdNotifier_;\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > [...]\n> > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n> > index 353f34a81eca..673c53fefdd7 100644\n> > --- a/src/libcamera/media_device.cpp\n> > +++ b/src/libcamera/media_device.cpp\n> > @@ -20,6 +20,7 @@\n> >   #include <linux/media.h>\n> >   \n> >   #include <libcamera/base/log.h>\n> > +#include \"libcamera/internal/v4l2_request.h\"\n> >   \n> >   /**\n> >    * \\file media_device.h\n> > @@ -851,4 +852,50 @@ std::vector<MediaEntity *> MediaDevice::locateEntities(unsigned int function)\n> >       return found;\n> >   }\n> >   \n> > +/**\n> > + * \\brief Allocate requests\n> > + * \\param[in] count Number of requests to allocate\n> > + * \\param[out] requests Vector to store allocated requests\n> > + *\n> > + * Allocates and stores \\a count requests in \\a requests. If allocation fails,\n> > + * and error is returned and \\a requests is cleared.\n> > + *\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +int MediaDevice::allocateRequests(unsigned int count,\n> > +                               std::vector<std::unique_ptr<V4L2Request>> *requests)\n> > +{\n> > +     requests->resize(count);\n> > +     for (unsigned int i = 0; i < count; i++) {\n> > +             int requestFd;\n> > +             int ret = ::ioctl(fd_.get(), MEDIA_IOC_REQUEST_ALLOC, &requestFd);\n> > +             if (ret < 0) {\n> > +                     requests->clear();\n> > +                     return -errno;\n> > +             }\n> > +             (*requests)[i] = std::make_unique<V4L2Request>(requestFd);\n> > +     }\n> > +\n> > +     return 0;\n> > +}\n> > +\n> > +/**\n> > + * \\brief Check if requests are supported\n> > + *\n> > + * Checks if the device supports V4L2 requests by trying to allocate a single\n> > + * request. The result is cached, so the allocation is only tried once.\n> > + *\n> > + * \\return True if the device supports requests, false otherwise\n> > + */\n> > +bool MediaDevice::supportsRequests()\n> > +{\n> > +     if (supportsRequests_.has_value())\n> > +             return supportsRequests_.value();\n> > +\n> > +     std::vector<std::unique_ptr<V4L2Request>> requests;\n> > +     supportsRequests_ = (allocateRequests(1, &requests) == 0);\n> \n> Shouldn't it technically be something like ` != -ENOTTY` ?\n\nHm, good question. But can you really be sure that it supports requests\nif a error != -ENOTTY is returned? I left it as is for now, to be on the\nsafe side.\n\n> \n> \n> > +\n> > +     return supportsRequests_.value();\n> > +}\n> > +\n> >   } /* namespace libcamera */\n> > [...]\n> > diff --git a/src/libcamera/v4l2_request.cpp b/src/libcamera/v4l2_request.cpp\n> > new file mode 100644\n> > index 000000000000..708250d86f61\n> > --- /dev/null\n> > +++ b/src/libcamera/v4l2_request.cpp\n> > @@ -0,0 +1,128 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025, Ideas On Board\n> > + *\n> > + * V4L2 Request API\n> > + */\n> > +\n> > +#include \"libcamera/internal/v4l2_request.h\"\n> > +\n> > +#include <fcntl.h>\n> > +#include <stdlib.h>\n> > +#include <sys/ioctl.h>\n> > +#include <sys/syscall.h>\n> > +#include <unistd.h>\n> > +\n> > +#include <linux/media.h>\n> > +\n> > +#include <libcamera/base/event_notifier.h>\n> > +#include <libcamera/base/log.h>\n> > +\n> > +/**\n> > + * \\file v4l2_request.h\n> > + * \\brief V4L2 Request\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DECLARE_CATEGORY(V4L2)\n> > +\n> > +/**\n> > + * \\class V4L2Request\n> > + * \\brief V4L2Request object and API\n> > + *\n> > + * The V4L2Request class wraps a V4L2 request fd and provides some convenience\n> > + * functions to handle request.\n> > + *\n> > + * It is usually constructed by calling \\a MediaDevice::allocateRequests().\n> > + *\n> > + * A request can then be passed to the V4L2Device::setControls(),\n> > + * V4L2Device::getControls() and V4L2VideoDevice::queueBuffer().\n> > + */\n> > +\n> > +/**\n> > + * \\brief Construct a V4L2Request\n> > + * \\param[in] fd The request fd\n> > + */\n> > +V4L2Request::V4L2Request(int fd)\n> > +     : fd_(fd), fdNotifier_(fd, EventNotifier::Exception)\n> > +{\n> > +     if (!fd_.isValid())\n> > +             return;\n> > +\n> > +     fdNotifier_.activated.connect(this, &V4L2Request::requestReady);\n> \n> It's probably just me, but I feel like I would just use a simple lambda here.\n\nI'd prefer a lambda, too. I didn't get it to work though. The code I'd\nexpect there would be something like:\n\nfdNotifier_.activated.connect([this] { this->requestDone.emit(this); });\n\nBut that doesn't work as capturing lambdas can't be converted to a\nfunction pointer. I didn't dig deeper into the Signal implementation.\nHow would you do that?\n\nBest regards,\nStefan\n\n> \n> \n> > +     fdNotifier_.setEnabled(false);\n> > +}\n> > [...]\n> \n> \n> Regards,\n> Barnabás Pőcze\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A62C5C32EF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Nov 2025 15:27:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8591E60A9D;\n\tMon, 24 Nov 2025 16:27:49 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 80E956096B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Nov 2025 16:27:47 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:3dd5:c977:12b4:d480])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 0F160D0;\n\tMon, 24 Nov 2025 16:25:38 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"thEaQwww\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763997939;\n\tbh=nJohxEqtFir1n0nKuuwM3n8CLQFCU8GI1bGkAA0Jpsk=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=thEaQwwwi/Dom1EKwF7F1Y+rX7+qbgObtliOiyc322gF9FKsYdNXtxPS61qGAXLWW\n\t+W3USMielayxU1X2DqcqfrfTp6CvyMOzv6KYINOfOWkgJulu1uinsTkwdQMRWr3+Sl\n\tCqWaV2YzVZUZMljawsBtoJOq7YSjUuIKyZVUUG2Q=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<18f2c0c1-c9e5-444a-88a2-59956e698abf@ideasonboard.com>","References":"<20251023144841.403689-1-stefan.klug@ideasonboard.com>\n\t<20251023144841.403689-6-stefan.klug@ideasonboard.com>\n\t<18f2c0c1-c9e5-444a-88a2-59956e698abf@ideasonboard.com>","Subject":"Re: [PATCH v2 05/35] libcamera: Add support for V4L2 requests","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 24 Nov 2025 16:27:44 +0100","Message-ID":"<176399806422.1350675.17922189972244077830@localhost>","User-Agent":"alot/0.12.dev8+g2c003385c862.d20250602","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":37030,"web_url":"https://patchwork.libcamera.org/comment/37030/","msgid":"<176399861506.1350675.15865296105242195420@localhost>","date":"2025-11-24T15:36:55","subject":"Re: [PATCH v2 05/35] libcamera: Add support for V4L2 requests","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Isaac,\n\nQuoting Isaac Scott (2025-10-27 16:11:00)\n> Hi Stefan,\n> \n> Thank you for the patch!\n> \n> Some nitpicks below:\n\nThank you for the review and the nitpicks. I fixed them for v3.\n\nCheers,\nStefan\n\n> \n> Quoting Stefan Klug (2025-10-23 15:48:06)\n> > The V4L2 requests API provides support to atomically tie controls to a\n> > set of buffers. This is especially common for m2m devices. Such a\n> > request is represented by a fd that is allocated vi\n> \n> s/vi/via/\n> \n> > MEDIA_IOC_REQUEST_ALLOC and then passed to various V4L2 function.\n> > \n> > Implement a V4L2Request class to wrap such an fd and add the\n> > corresponding utility functions.\n> > \n> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> > \n> > ---\n> > \n> > Changes in v2:\n> > - Added documentation\n> > ---\n> >  include/libcamera/internal/media_device.h     |   7 +\n> >  include/libcamera/internal/meson.build        |   1 +\n> >  include/libcamera/internal/v4l2_device.h      |   5 +-\n> >  include/libcamera/internal/v4l2_request.h     |  49 +++++++\n> >  include/libcamera/internal/v4l2_videodevice.h |   3 +-\n> >  src/libcamera/media_device.cpp                |  47 +++++++\n> >  src/libcamera/meson.build                     |   1 +\n> >  src/libcamera/v4l2_device.cpp                 |  28 +++-\n> >  src/libcamera/v4l2_request.cpp                | 128 ++++++++++++++++++\n> >  src/libcamera/v4l2_videodevice.cpp            |  10 +-\n> >  10 files changed, 271 insertions(+), 8 deletions(-)\n> >  create mode 100644 include/libcamera/internal/v4l2_request.h\n> >  create mode 100644 src/libcamera/v4l2_request.cpp\n> > \n> > diff --git a/include/libcamera/internal/media_device.h b/include/libcamera/internal/media_device.h\n> > index b3a48b98d64b..74cb9ba1542d 100644\n> > --- a/include/libcamera/internal/media_device.h\n> > +++ b/include/libcamera/internal/media_device.h\n> > @@ -18,6 +18,7 @@\n> >  #include <libcamera/base/unique_fd.h>\n> >  \n> >  #include \"libcamera/internal/media_object.h\"\n> > +#include \"libcamera/internal/v4l2_request.h\"\n> >  \n> >  namespace libcamera {\n> >  \n> > @@ -57,6 +58,11 @@ public:\n> >  \n> >         std::vector<MediaEntity *> locateEntities(unsigned int function);\n> >  \n> > +       int allocateRequests(unsigned int count,\n> > +                            std::vector<std::unique_ptr<V4L2Request>> *requests);\n> > +\n> > +       bool supportsRequests();\n> > +\n> >  protected:\n> >         std::string logPrefix() const override;\n> >  \n> > @@ -87,6 +93,7 @@ private:\n> >         UniqueFD fd_;\n> >         bool valid_;\n> >         bool acquired_;\n> > +       std::optional<bool> supportsRequests_;\n> >  \n> >         std::map<unsigned int, MediaObject *> objects_;\n> >         std::vector<MediaEntity *> entities_;\n> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> > index 45c299f6a332..e9540a2f734f 100644\n> > --- a/include/libcamera/internal/meson.build\n> > +++ b/include/libcamera/internal/meson.build\n> > @@ -44,6 +44,7 @@ libcamera_internal_headers = files([\n> >      'sysfs.h',\n> >      'v4l2_device.h',\n> >      'v4l2_pixelformat.h',\n> > +    'v4l2_request.h',\n> >      'v4l2_subdevice.h',\n> >      'v4l2_videodevice.h',\n> >      'vector.h',\n> > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> > index 5bc9da96677d..dbbd118abd00 100644\n> > --- a/include/libcamera/internal/v4l2_device.h\n> > +++ b/include/libcamera/internal/v4l2_device.h\n> > @@ -24,6 +24,7 @@\n> >  #include <libcamera/controls.h>\n> >  \n> >  #include \"libcamera/internal/formats.h\"\n> > +#include \"libcamera/internal/v4l2_request.h\"\n> >  \n> >  namespace libcamera {\n> >  \n> > @@ -37,8 +38,8 @@ public:\n> >  \n> >         const ControlInfoMap &controls() const { return controls_; }\n> >  \n> > -       ControlList getControls(Span<const uint32_t> ids);\n> > -       int setControls(ControlList *ctrls);\n> > +       ControlList getControls(Span<const uint32_t> ids, const V4L2Request *request = nullptr);\n> > +       int setControls(ControlList *ctrls, const V4L2Request *request = nullptr);\n> >  \n> >         const struct v4l2_query_ext_ctrl *controlInfo(uint32_t id) const;\n> >  \n> > diff --git a/include/libcamera/internal/v4l2_request.h b/include/libcamera/internal/v4l2_request.h\n> > new file mode 100644\n> > index 000000000000..bf1bea3261af\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/v4l2_request.h\n> > @@ -0,0 +1,49 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025, Ideas On Board\n> > + *\n> > + * V4L2 requests\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include <string>\n> > +\n> > +#include <linux/videodev2.h>\n> > +\n> > +#include <libcamera/base/event_notifier.h>\n> > +#include <libcamera/base/log.h>\n> > +#include <libcamera/base/signal.h>\n> > +#include <libcamera/base/span.h>\n> > +#include <libcamera/base/unique_fd.h>\n> > +\n> > +#include <libcamera/color_space.h>\n> > +#include <libcamera/controls.h>\n> > +\n> > +namespace libcamera {\n> > +\n> > +class V4L2Request : protected Loggable\n> > +{\n> > +public:\n> > +       bool isValid() const { return fd_.isValid(); }\n> > +       int fd() const { return fd_.get(); }\n> > +\n> > +       int reinit();\n> > +       int queue();\n> > +\n> > +       V4L2Request(int fd = -1);\n> > +       ~V4L2Request() = default;\n> > +\n> > +       Signal<V4L2Request *> requestDone;\n> > +\n> > +private:\n> > +       LIBCAMERA_DISABLE_COPY_AND_MOVE(V4L2Request)\n> > +\n> > +       void requestReady();\n> > +       std::string logPrefix() const override;\n> > +\n> > +       UniqueFD fd_;\n> > +       EventNotifier fdNotifier_;\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> > index 5a7dcfdda118..2d290971a0ee 100644\n> > --- a/include/libcamera/internal/v4l2_videodevice.h\n> > +++ b/include/libcamera/internal/v4l2_videodevice.h\n> > @@ -33,6 +33,7 @@\n> >  #include \"libcamera/internal/formats.h\"\n> >  #include \"libcamera/internal/v4l2_device.h\"\n> >  #include \"libcamera/internal/v4l2_pixelformat.h\"\n> > +#include \"libcamera/internal/v4l2_request.h\"\n> >  \n> >  namespace libcamera {\n> >  \n> > @@ -217,7 +218,7 @@ public:\n> >         int importBuffers(unsigned int count);\n> >         int releaseBuffers();\n> >  \n> > -       int queueBuffer(FrameBuffer *buffer);\n> > +       int queueBuffer(FrameBuffer *buffer, const V4L2Request *request = nullptr);\n> >         Signal<FrameBuffer *> bufferReady;\n> >  \n> >         int streamOn();\n> > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n> > index 353f34a81eca..673c53fefdd7 100644\n> > --- a/src/libcamera/media_device.cpp\n> > +++ b/src/libcamera/media_device.cpp\n> > @@ -20,6 +20,7 @@\n> >  #include <linux/media.h>\n> >  \n> >  #include <libcamera/base/log.h>\n> > +#include \"libcamera/internal/v4l2_request.h\"\n> >  \n> >  /**\n> >   * \\file media_device.h\n> > @@ -851,4 +852,50 @@ std::vector<MediaEntity *> MediaDevice::locateEntities(unsigned int function)\n> >         return found;\n> >  }\n> >  \n> > +/**\n> > + * \\brief Allocate requests\n> > + * \\param[in] count Number of requests to allocate\n> > + * \\param[out] requests Vector to store allocated requests\n> > + *\n> > + * Allocates and stores \\a count requests in \\a requests. If allocation fails,\n> > + * and error is returned and \\a requests is cleared.\n> > + *\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +int MediaDevice::allocateRequests(unsigned int count,\n> > +                                 std::vector<std::unique_ptr<V4L2Request>> *requests)\n> > +{\n> > +       requests->resize(count);\n> > +       for (unsigned int i = 0; i < count; i++) {\n> > +               int requestFd;\n> > +               int ret = ::ioctl(fd_.get(), MEDIA_IOC_REQUEST_ALLOC, &requestFd);\n> > +               if (ret < 0) {\n> > +                       requests->clear();\n> > +                       return -errno;\n> > +               }\n> > +               (*requests)[i] = std::make_unique<V4L2Request>(requestFd);\n> > +       }\n> > +\n> > +       return 0;\n> > +}\n> > +\n> > +/**\n> > + * \\brief Check if requests are supported\n> > + *\n> > + * Checks if the device supports V4L2 requests by trying to allocate a single\n> > + * request. The result is cached, so the allocation is only tried once.\n> > + *\n> > + * \\return True if the device supports requests, false otherwise\n> > + */\n> > +bool MediaDevice::supportsRequests()\n> > +{\n> > +       if (supportsRequests_.has_value())\n> > +               return supportsRequests_.value();\n> > +\n> > +       std::vector<std::unique_ptr<V4L2Request>> requests;\n> > +       supportsRequests_ = (allocateRequests(1, &requests) == 0);\n> > +\n> > +       return supportsRequests_.value();\n> > +}\n> > +\n> >  } /* namespace libcamera */\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index 5b9b86f211f1..34e20f557514 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -54,6 +54,7 @@ libcamera_internal_sources = files([\n> >      'sysfs.cpp',\n> >      'v4l2_device.cpp',\n> >      'v4l2_pixelformat.cpp',\n> > +    'v4l2_request.cpp',\n> >      'v4l2_subdevice.cpp',\n> >      'v4l2_videodevice.cpp',\n> >      'vector.cpp',\n> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > index 8c78b8c424e5..7a669a0303c1 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -162,6 +162,7 @@ void V4L2Device::close()\n> >  /**\n> >   * \\brief Read controls from the device\n> >   * \\param[in] ids The list of controls to read, specified by their ID\n> > + * \\param[in] request An optional request\n> >   *\n> >   * This function reads the value of all controls contained in \\a ids, and\n> >   * returns their values as a ControlList.\n> > @@ -171,10 +172,12 @@ void V4L2Device::close()\n> >   * during validation of the requested controls, no control is read and this\n> >   * function returns an empty control list.\n> >   *\n> > + * If \\a request is specified the controls tied to that request are read.\n> > + *\n> >   * \\return The control values in a ControlList on success, or an empty list on\n> >   * error\n> >   */\n> > -ControlList V4L2Device::getControls(Span<const uint32_t> ids)\n> > +ControlList V4L2Device::getControls(Span<const uint32_t> ids, const V4L2Request *request)\n> >  {\n> >         if (ids.empty())\n> >                 return {};\n> > @@ -242,10 +245,16 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids)\n> >         }\n> >  \n> >         struct v4l2_ext_controls v4l2ExtCtrls = {};\n> > -       v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL;\n> >         v4l2ExtCtrls.controls = v4l2Ctrls.data();\n> >         v4l2ExtCtrls.count = v4l2Ctrls.size();\n> >  \n> > +       if (request) {\n> > +               v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL;\n> > +               v4l2ExtCtrls.request_fd = request->fd();\n> > +       } else {\n> > +               v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL;\n> > +       }\n> > +\n> >         int ret = ioctl(VIDIOC_G_EXT_CTRLS, &v4l2ExtCtrls);\n> >         if (ret) {\n> >                 unsigned int errorIdx = v4l2ExtCtrls.error_idx;\n> > @@ -273,6 +282,7 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids)\n> >  /**\n> >   * \\brief Write controls to the device\n> >   * \\param[in] ctrls The list of controls to write\n> > + * \\param[in] request And optional request\n> \n> s/And/An/ ?\n> \n> >   *\n> >   * This function writes the value of all controls contained in \\a ctrls, and\n> >   * stores the values actually applied to the device in the corresponding\n> > @@ -288,11 +298,15 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids)\n> >   * are written and their values are updated in \\a ctrls, while all other\n> >   * controls are not written and their values are not changed.\n> >   *\n> > + * If \\a request is set, the controls will be applied to that request. If the\n> > + * device doesn't support requests, -EACCESS will be returned. If \\a request is\n> > + * invalid, -EINVAL will be returned.\n> > + *\n> >   * \\return 0 on success or an error code otherwise\n> >   * \\retval -EINVAL One of the control is not supported or not accessible\n> \n> s/control/controls/\n> \n> >   * \\retval i The index of the control that failed\n> >   */\n> > -int V4L2Device::setControls(ControlList *ctrls)\n> > +int V4L2Device::setControls(ControlList *ctrls, const V4L2Request *request)\n> >  {\n> >         if (ctrls->empty())\n> >                 return 0;\n> > @@ -377,10 +391,16 @@ int V4L2Device::setControls(ControlList *ctrls)\n> >         }\n> >  \n> >         struct v4l2_ext_controls v4l2ExtCtrls = {};\n> > -       v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL;\n> >         v4l2ExtCtrls.controls = v4l2Ctrls.data();\n> >         v4l2ExtCtrls.count = v4l2Ctrls.size();\n> >  \n> > +       if (request) {\n> > +               v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL;\n> > +               v4l2ExtCtrls.request_fd = request->fd();\n> > +       } else {\n> > +               v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL;\n> > +       }\n> > +\n> >         int ret = ioctl(VIDIOC_S_EXT_CTRLS, &v4l2ExtCtrls);\n> >         if (ret) {\n> >                 unsigned int errorIdx = v4l2ExtCtrls.error_idx;\n> > diff --git a/src/libcamera/v4l2_request.cpp b/src/libcamera/v4l2_request.cpp\n> > new file mode 100644\n> > index 000000000000..708250d86f61\n> > --- /dev/null\n> > +++ b/src/libcamera/v4l2_request.cpp\n> > @@ -0,0 +1,128 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025, Ideas On Board\n> > + *\n> > + * V4L2 Request API\n> > + */\n> > +\n> > +#include \"libcamera/internal/v4l2_request.h\"\n> > +\n> > +#include <fcntl.h>\n> > +#include <stdlib.h>\n> > +#include <sys/ioctl.h>\n> > +#include <sys/syscall.h>\n> > +#include <unistd.h>\n> > +\n> > +#include <linux/media.h>\n> > +\n> > +#include <libcamera/base/event_notifier.h>\n> > +#include <libcamera/base/log.h>\n> > +\n> > +/**\n> > + * \\file v4l2_request.h\n> > + * \\brief V4L2 Request\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DECLARE_CATEGORY(V4L2)\n> > +\n> > +/**\n> > + * \\class V4L2Request\n> > + * \\brief V4L2Request object and API\n> > + *\n> > + * The V4L2Request class wraps a V4L2 request fd and provides some convenience\n> > + * functions to handle request.\n> > + *\n> > + * It is usually constructed by calling \\a MediaDevice::allocateRequests().\n> > + *\n> > + * A request can then be passed to the V4L2Device::setControls(),\n> > + * V4L2Device::getControls() and V4L2VideoDevice::queueBuffer().\n> > + */\n> > +\n> > +/**\n> > + * \\brief Construct a V4L2Request\n> > + * \\param[in] fd The request fd\n> > + */\n> > +V4L2Request::V4L2Request(int fd)\n> > +       : fd_(fd), fdNotifier_(fd, EventNotifier::Exception)\n> > +{\n> > +       if (!fd_.isValid())\n> > +               return;\n> > +\n> > +       fdNotifier_.activated.connect(this, &V4L2Request::requestReady);\n> > +       fdNotifier_.setEnabled(false);\n> > +}\n> > +\n> > +/**\n> > + * \\fn V4L2Request::isValid()\n> > + * \\brief Check if the request is valid\n> > + *\n> > + * Checks if the underlying fd is valid.\n> > + *\n> > + * \\return True if the request is valid, false otherwise\n> > + */\n> > +\n> > +/**\n> > + * \\fn V4L2Request::fd()\n> > + * \\brief Get the file descriptor\n> > + *\n> > + * \\return The file descriptor wrapped by this V4L2Request\n> > + */\n> > +\n> > +/**\n> > + * \\var V4L2Request::requestDone\n> > + * \\brief Signal that is emitted when the request is done\n> > + */\n> > +\n> > +/**\n> > + * \\brief Reinit the request\n> > + *\n> > + * Calls MEDIA_REQUEST_IOC_REINIT om the request fd.\n> > + *\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +int V4L2Request::reinit()\n> > +{\n> > +       fdNotifier_.setEnabled(false);\n> > +\n> > +       if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_REINIT) < 0)\n> > +               return -errno;\n> > +\n> > +       return 0;\n> > +}\n> > +\n> > +/**\n> > + * \\brief Reinit the request\n> > + *\n> > + * Calls MEDIA_REQUEST_IOC_QUEUE om the request fd.\n> > + *\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +int V4L2Request::queue()\n> > +{\n> > +       if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_QUEUE) < 0)\n> > +               return -errno;\n> > +\n> > +       fdNotifier_.setEnabled(true);\n> > +\n> > +       return 0;\n> > +}\n> > +\n> > +std::string V4L2Request::logPrefix() const\n> > +{\n> > +       return \"Request [\" + std::to_string(fd()) + \"]\";\n> > +}\n> > +\n> > +/**\n> > + * \\brief Slot to handle request done events\n> > + *\n> > + * When this slot is called, the request is done and the requestDone will be\n> > + * emitted.\n> > + */\n> > +void V4L2Request::requestReady()\n> > +{\n> > +       requestDone.emit(this);\n> > +}\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> > index bb57c1b76a5b..8ce739f4bc65 100644\n> > --- a/src/libcamera/v4l2_videodevice.cpp\n> > +++ b/src/libcamera/v4l2_videodevice.cpp\n> > @@ -30,6 +30,7 @@\n> >  #include \"libcamera/internal/framebuffer.h\"\n> >  #include \"libcamera/internal/media_device.h\"\n> >  #include \"libcamera/internal/media_object.h\"\n> > +#include \"libcamera/internal/v4l2_request.h\"\n> >  \n> >  /**\n> >   * \\file v4l2_videodevice.h\n> > @@ -1629,6 +1630,7 @@ int V4L2VideoDevice::releaseBuffers()\n> >  /**\n> >   * \\brief Queue a buffer to the video device\n> >   * \\param[in] buffer The buffer to be queued\n> > + * \\param[in] request An optional request\n> >   *\n> >   * For capture video devices the \\a buffer will be filled with data by the\n> >   * device. For output video devices the \\a buffer shall contain valid data and\n> > @@ -1641,9 +1643,11 @@ int V4L2VideoDevice::releaseBuffers()\n> >   * Note that queueBuffer() will fail if the device is in the process of being\n> >   * stopped from a streaming state through streamOff().\n> >   *\n> > + * If \\a request is specified, the buffer will be tied to that request.\n> > + *\n> >   * \\return 0 on success or a negative error code otherwise\n> >   */\n> > -int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)\n> > +int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer, const V4L2Request *request)\n> >  {\n> \n> Best wishes,\n> \n> Isaac\n> \n> >         struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {};\n> >         struct v4l2_buffer buf = {};\n> > @@ -1674,6 +1678,10 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)\n> >         buf.type = bufferType_;\n> >         buf.memory = memoryType_;\n> >         buf.field = V4L2_FIELD_NONE;\n> > +       if (request) {\n> > +               buf.flags = V4L2_BUF_FLAG_REQUEST_FD;\n> > +               buf.request_fd = request->fd();\n> > +       }\n> >  \n> >         bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type);\n> >         Span<const FrameBuffer::Plane> planes = buffer->planes();\n> > -- \n> > 2.48.1\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5892EC32E0\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Nov 2025 15:36:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 80CBA60A80;\n\tMon, 24 Nov 2025 16:36:58 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BFCAF6096B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Nov 2025 16:36:57 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:3dd5:c977:12b4:d480])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 48FCED0;\n\tMon, 24 Nov 2025 16:34:49 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"oOtcL5Ns\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763998489;\n\tbh=kFT6Gjvn0qnFzLQEVOM1n32yy6Vp4Rayp/wOhF9YCmQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=oOtcL5NsQFNycYSZdwVWI4CPzmzLoUGXFqNIfwIVd3vg6BphI7imSiOzzvLW6fcZE\n\tVnJBoSkkQKwbExZPPvSXjwrDAcFNdl+1bxD0avZSeZ9JXJPEqfd/FSAmXRsnddM9Nr\n\t/EbBDft7rdn4hkX/5QKYUxlhOAO4Zl/K/qXxjiBg=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<176157786037.59544.9769698925401122053@isaac-ThinkPad-T16-Gen-2>","References":"<20251023144841.403689-1-stefan.klug@ideasonboard.com>\n\t<20251023144841.403689-6-stefan.klug@ideasonboard.com>\n\t<176157786037.59544.9769698925401122053@isaac-ThinkPad-T16-Gen-2>","Subject":"Re: [PATCH v2 05/35] libcamera: Add support for V4L2 requests","From":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"","To":"Isaac Scott <isaac.scott@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 24 Nov 2025 16:36:55 +0100","Message-ID":"<176399861506.1350675.15865296105242195420@localhost>","User-Agent":"alot/0.12.dev8+g2c003385c862.d20250602","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":37034,"web_url":"https://patchwork.libcamera.org/comment/37034/","msgid":"<176400064992.1350675.10953596602547025927@localhost>","date":"2025-11-24T16:10:49","subject":"Re: [PATCH v2 05/35] libcamera: Add support for V4L2 requests","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the review.\n\nQuoting Paul Elder (2025-11-05 17:46:02)\n> Quoting Stefan Klug (2025-10-23 23:48:06)\n> > The V4L2 requests API provides support to atomically tie controls to a\n> > set of buffers. This is especially common for m2m devices. Such a\n> > request is represented by a fd that is allocated vi\n> \n> s/ a / an / s/vi/via/\n> \n> > MEDIA_IOC_REQUEST_ALLOC and then passed to various V4L2 function.\n> \n> s/function/functions/\n\n...and more typos...\n\n> \n> > \n> > Implement a V4L2Request class to wrap such an fd and add the\n> > corresponding utility functions.\n> > \n> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> > \n> > ---\n> > \n> > Changes in v2:\n> > - Added documentation\n> > ---\n> >  include/libcamera/internal/media_device.h     |   7 +\n> >  include/libcamera/internal/meson.build        |   1 +\n> >  include/libcamera/internal/v4l2_device.h      |   5 +-\n> >  include/libcamera/internal/v4l2_request.h     |  49 +++++++\n> >  include/libcamera/internal/v4l2_videodevice.h |   3 +-\n> >  src/libcamera/media_device.cpp                |  47 +++++++\n> >  src/libcamera/meson.build                     |   1 +\n> >  src/libcamera/v4l2_device.cpp                 |  28 +++-\n> >  src/libcamera/v4l2_request.cpp                | 128 ++++++++++++++++++\n> >  src/libcamera/v4l2_videodevice.cpp            |  10 +-\n> >  10 files changed, 271 insertions(+), 8 deletions(-)\n> >  create mode 100644 include/libcamera/internal/v4l2_request.h\n> >  create mode 100644 src/libcamera/v4l2_request.cpp\n> > \n> > diff --git a/include/libcamera/internal/media_device.h b/include/libcamera/internal/media_device.h\n> > index b3a48b98d64b..74cb9ba1542d 100644\n> > --- a/include/libcamera/internal/media_device.h\n> > +++ b/include/libcamera/internal/media_device.h\n> > @@ -18,6 +18,7 @@\n> >  #include <libcamera/base/unique_fd.h>\n> >  \n> >  #include \"libcamera/internal/media_object.h\"\n> > +#include \"libcamera/internal/v4l2_request.h\"\n> >  \n> >  namespace libcamera {\n> >  \n> > @@ -57,6 +58,11 @@ public:\n> >  \n> >         std::vector<MediaEntity *> locateEntities(unsigned int function);\n> >  \n> > +       int allocateRequests(unsigned int count,\n> > +                            std::vector<std::unique_ptr<V4L2Request>> *requests);\n> > +\n> > +       bool supportsRequests();\n> > +\n> >  protected:\n> >         std::string logPrefix() const override;\n> >  \n> > @@ -87,6 +93,7 @@ private:\n> >         UniqueFD fd_;\n> >         bool valid_;\n> >         bool acquired_;\n> > +       std::optional<bool> supportsRequests_;\n> >  \n> >         std::map<unsigned int, MediaObject *> objects_;\n> >         std::vector<MediaEntity *> entities_;\n> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> > index 45c299f6a332..e9540a2f734f 100644\n> > --- a/include/libcamera/internal/meson.build\n> > +++ b/include/libcamera/internal/meson.build\n> > @@ -44,6 +44,7 @@ libcamera_internal_headers = files([\n> >      'sysfs.h',\n> >      'v4l2_device.h',\n> >      'v4l2_pixelformat.h',\n> > +    'v4l2_request.h',\n> >      'v4l2_subdevice.h',\n> >      'v4l2_videodevice.h',\n> >      'vector.h',\n> > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> > index 5bc9da96677d..dbbd118abd00 100644\n> > --- a/include/libcamera/internal/v4l2_device.h\n> > +++ b/include/libcamera/internal/v4l2_device.h\n> > @@ -24,6 +24,7 @@\n> >  #include <libcamera/controls.h>\n> >  \n> >  #include \"libcamera/internal/formats.h\"\n> > +#include \"libcamera/internal/v4l2_request.h\"\n> >  \n> >  namespace libcamera {\n> >  \n> > @@ -37,8 +38,8 @@ public:\n> >  \n> >         const ControlInfoMap &controls() const { return controls_; }\n> >  \n> > -       ControlList getControls(Span<const uint32_t> ids);\n> > -       int setControls(ControlList *ctrls);\n> > +       ControlList getControls(Span<const uint32_t> ids, const V4L2Request *request = nullptr);\n> > +       int setControls(ControlList *ctrls, const V4L2Request *request = nullptr);\n> >  \n> >         const struct v4l2_query_ext_ctrl *controlInfo(uint32_t id) const;\n> >  \n> > diff --git a/include/libcamera/internal/v4l2_request.h b/include/libcamera/internal/v4l2_request.h\n> > new file mode 100644\n> > index 000000000000..bf1bea3261af\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/v4l2_request.h\n> > @@ -0,0 +1,49 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025, Ideas On Board\n> > + *\n> > + * V4L2 requests\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include <string>\n> > +\n> > +#include <linux/videodev2.h>\n> > +\n> > +#include <libcamera/base/event_notifier.h>\n> > +#include <libcamera/base/log.h>\n> > +#include <libcamera/base/signal.h>\n> > +#include <libcamera/base/span.h>\n> > +#include <libcamera/base/unique_fd.h>\n> > +\n> > +#include <libcamera/color_space.h>\n> > +#include <libcamera/controls.h>\n> \n> Do we need these two headers?\n\nUhh, no we don't. Thanks for spotting.\n\n> \n> > +\n> > +namespace libcamera {\n> > +\n> > +class V4L2Request : protected Loggable\n> > +{\n> > +public:\n> > +       bool isValid() const { return fd_.isValid(); }\n> > +       int fd() const { return fd_.get(); }\n> > +\n> > +       int reinit();\n> > +       int queue();\n> > +\n> > +       V4L2Request(int fd = -1);\n> > +       ~V4L2Request() = default;\n> > +\n> > +       Signal<V4L2Request *> requestDone;\n> > +\n> > +private:\n> > +       LIBCAMERA_DISABLE_COPY_AND_MOVE(V4L2Request)\n> > +\n> > +       void requestReady();\n> > +       std::string logPrefix() const override;\n> > +\n> > +       UniqueFD fd_;\n> > +       EventNotifier fdNotifier_;\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> > index 5a7dcfdda118..2d290971a0ee 100644\n> > --- a/include/libcamera/internal/v4l2_videodevice.h\n> > +++ b/include/libcamera/internal/v4l2_videodevice.h\n> > @@ -33,6 +33,7 @@\n> >  #include \"libcamera/internal/formats.h\"\n> >  #include \"libcamera/internal/v4l2_device.h\"\n> >  #include \"libcamera/internal/v4l2_pixelformat.h\"\n> > +#include \"libcamera/internal/v4l2_request.h\"\n> >  \n> >  namespace libcamera {\n> \n> >  \n> > @@ -217,7 +218,7 @@ public:\n> \n> >         int importBuffers(unsigned int count);\n> >         int releaseBuffers();\n> >  \n> > -       int queueBuffer(FrameBuffer *buffer);\n> > +       int queueBuffer(FrameBuffer *buffer, const V4L2Request *request = nullptr);\n> >         Signal<FrameBuffer *> bufferReady;\n> >  \n> >         int streamOn();\n> > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n> > index 353f34a81eca..673c53fefdd7 100644\n> > --- a/src/libcamera/media_device.cpp\n> > +++ b/src/libcamera/media_device.cpp\n> > @@ -20,6 +20,7 @@\n> >  #include <linux/media.h>\n> >  \n> >  #include <libcamera/base/log.h>\n> > +#include \"libcamera/internal/v4l2_request.h\"\n> >  \n> >  /**\n> >   * \\file media_device.h\n> > @@ -851,4 +852,50 @@ std::vector<MediaEntity *> MediaDevice::locateEntities(unsigned int function)\n> >         return found;\n> >  }\n> >  \n> > +/**\n> > + * \\brief Allocate requests\n> > + * \\param[in] count Number of requests to allocate\n> > + * \\param[out] requests Vector to store allocated requests\n> > + *\n> > + * Allocates and stores \\a count requests in \\a requests. If allocation fails,\n> > + * and error is returned and \\a requests is cleared.\n> \n> s/\\* and/\\* an/\n> \n> > + *\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +int MediaDevice::allocateRequests(unsigned int count,\n> > +                                 std::vector<std::unique_ptr<V4L2Request>> *requests)\n> > +{\n> > +       requests->resize(count);\n> > +       for (unsigned int i = 0; i < count; i++) {\n> > +               int requestFd;\n> > +               int ret = ::ioctl(fd_.get(), MEDIA_IOC_REQUEST_ALLOC, &requestFd);\n> \n> I'm of the opinion that this should be moved into V4L2Request constructor (or\n> init()) so that we can keep all requests handling there. We already have an\n> isValid(), and maybe we can add close() to the deconstrctor?\n\nHm, I see the point. I'm a bit undecided here. The request specific\nioctls are prefixed with MEDIA_REQUEST_IOC_ whilst the media device ones are\nprefixed with MEDIA_IOC_, so it is not completely wrong to do it here.\nWe could move the whole function into V4L2Request as\nallocateForMedia(int fd, int count,\nstd::vector<std::unique_ptr<V4L2Request>> *requests)\n\nand this function could then collapse to\n\nreturn V4L2Request::allocateForMedia(fd_.get(), count, requests);\n\nIn that case I dislike passing the media device fd as int. So I'm not\nreally convinced :-)\n\nWould you prefer such a static function?\n\n> \n> > +               if (ret < 0) {\n> > +                       requests->clear();\n> > +                       return -errno;\n> > +               }\n> > +               (*requests)[i] = std::make_unique<V4L2Request>(requestFd);\n> > +       }\n> > +\n> > +       return 0;\n> > +}\n> > +\n> > +/**\n> > + * \\brief Check if requests are supported\n> > + *\n> > + * Checks if the device supports V4L2 requests by trying to allocate a single\n> > + * request. The result is cached, so the allocation is only tried once.\n> > + *\n> > + * \\return True if the device supports requests, false otherwise\n> > + */\n> > +bool MediaDevice::supportsRequests()\n> > +{\n> > +       if (supportsRequests_.has_value())\n> > +               return supportsRequests_.value();\n> > +\n> > +       std::vector<std::unique_ptr<V4L2Request>> requests;\n> > +       supportsRequests_ = (allocateRequests(1, &requests) == 0);\n> \n> Initially I was worried about subsequent allocations but then I realized that\n> this will get deallocated after it goes out of scope.\n> \n> Although there is currently no other errno than ENOTTY, the non-explicit check\n> makes me uncomfortable :S\n\nBarnabás mentioned that also. I don't think we should treat any other\nerror as supportsRequests() == true, But I don't know for sure. Would\nyou prefer returning the error code and leaving the decision to the\nuser? But then the user could call allocateRequests() directly in first\nplace...\n\n> \n> > +\n> > +       return supportsRequests_.value();\n> > +}\n> > +\n> >  } /* namespace libcamera */\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index 5b9b86f211f1..34e20f557514 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -54,6 +54,7 @@ libcamera_internal_sources = files([\n> >      'sysfs.cpp',\n> >      'v4l2_device.cpp',\n> >      'v4l2_pixelformat.cpp',\n> > +    'v4l2_request.cpp',\n> >      'v4l2_subdevice.cpp',\n> >      'v4l2_videodevice.cpp',\n> >      'vector.cpp',\n> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > index 8c78b8c424e5..7a669a0303c1 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -162,6 +162,7 @@ void V4L2Device::close()\n> >  /**\n> >   * \\brief Read controls from the device\n> >   * \\param[in] ids The list of controls to read, specified by their ID\n> > + * \\param[in] request An optional request\n> >   *\n> >   * This function reads the value of all controls contained in \\a ids, and\n> >   * returns their values as a ControlList.\n> > @@ -171,10 +172,12 @@ void V4L2Device::close()\n> >   * during validation of the requested controls, no control is read and this\n> >   * function returns an empty control list.\n> >   *\n> > + * If \\a request is specified the controls tied to that request are read.\n> > + *\n> >   * \\return The control values in a ControlList on success, or an empty list on\n> >   * error\n> >   */\n> > -ControlList V4L2Device::getControls(Span<const uint32_t> ids)\n> > +ControlList V4L2Device::getControls(Span<const uint32_t> ids, const V4L2Request *request)\n> >  {\n> >         if (ids.empty())\n> >                 return {};\n> > @@ -242,10 +245,16 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids)\n> >         }\n> >  \n> >         struct v4l2_ext_controls v4l2ExtCtrls = {};\n> > -       v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL;\n> >         v4l2ExtCtrls.controls = v4l2Ctrls.data();\n> >         v4l2ExtCtrls.count = v4l2Ctrls.size();\n> >  \n> > +       if (request) {\n> > +               v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL;\n> > +               v4l2ExtCtrls.request_fd = request->fd();\n> > +       } else {\n> > +               v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL;\n> > +       }\n> > +\n> >         int ret = ioctl(VIDIOC_G_EXT_CTRLS, &v4l2ExtCtrls);\n> >         if (ret) {\n> >                 unsigned int errorIdx = v4l2ExtCtrls.error_idx;\n> > @@ -273,6 +282,7 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids)\n> >  /**\n> >   * \\brief Write controls to the device\n> >   * \\param[in] ctrls The list of controls to write\n> > + * \\param[in] request And optional request\n> \n> s/And/An/\n> \n> >   *\n> >   * This function writes the value of all controls contained in \\a ctrls, and\n> >   * stores the values actually applied to the device in the corresponding\n> > @@ -288,11 +298,15 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids)\n> >   * are written and their values are updated in \\a ctrls, while all other\n> >   * controls are not written and their values are not changed.\n> >   *\n> > + * If \\a request is set, the controls will be applied to that request. If the\n> > + * device doesn't support requests, -EACCESS will be returned. If \\a request is\n> > + * invalid, -EINVAL will be returned.\n> \n> Do you need to mention EACCESS in the docunentation for read as well?\n> \n> > + *\n> >   * \\return 0 on success or an error code otherwise\n> >   * \\retval -EINVAL One of the control is not supported or not accessible\n> >   * \\retval i The index of the control that failed\n> >   */\n> > -int V4L2Device::setControls(ControlList *ctrls)\n> > +int V4L2Device::setControls(ControlList *ctrls, const V4L2Request *request)\n> >  {\n> >         if (ctrls->empty())\n> >                 return 0;\n> > @@ -377,10 +391,16 @@ int V4L2Device::setControls(ControlList *ctrls)\n> >         }\n> >  \n> >         struct v4l2_ext_controls v4l2ExtCtrls = {};\n> > -       v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL;\n> >         v4l2ExtCtrls.controls = v4l2Ctrls.data();\n> >         v4l2ExtCtrls.count = v4l2Ctrls.size();\n> >  \n> > +       if (request) {\n> > +               v4l2ExtCtrls.which = V4L2_CTRL_WHICH_REQUEST_VAL;\n> > +               v4l2ExtCtrls.request_fd = request->fd();\n> > +       } else {\n> > +               v4l2ExtCtrls.which = V4L2_CTRL_WHICH_CUR_VAL;\n> > +       }\n> > +\n> >         int ret = ioctl(VIDIOC_S_EXT_CTRLS, &v4l2ExtCtrls);\n> >         if (ret) {\n> >                 unsigned int errorIdx = v4l2ExtCtrls.error_idx;\n> > diff --git a/src/libcamera/v4l2_request.cpp b/src/libcamera/v4l2_request.cpp\n> > new file mode 100644\n> > index 000000000000..708250d86f61\n> > --- /dev/null\n> > +++ b/src/libcamera/v4l2_request.cpp\n> > @@ -0,0 +1,128 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025, Ideas On Board\n> > + *\n> > + * V4L2 Request API\n> > + */\n> > +\n> > +#include \"libcamera/internal/v4l2_request.h\"\n> > +\n> > +#include <fcntl.h>\n> > +#include <stdlib.h>\n> > +#include <sys/ioctl.h>\n> > +#include <sys/syscall.h>\n> > +#include <unistd.h>\n> > +\n> > +#include <linux/media.h>\n> > +\n> > +#include <libcamera/base/event_notifier.h>\n> > +#include <libcamera/base/log.h>\n> > +\n> > +/**\n> > + * \\file v4l2_request.h\n> > + * \\brief V4L2 Request\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DECLARE_CATEGORY(V4L2)\n> > +\n> > +/**\n> > + * \\class V4L2Request\n> > + * \\brief V4L2Request object and API\n> > + *\n> > + * The V4L2Request class wraps a V4L2 request fd and provides some convenience\n> > + * functions to handle request.\n> > + *\n> > + * It is usually constructed by calling \\a MediaDevice::allocateRequests().\n> > + *\n> > + * A request can then be passed to the V4L2Device::setControls(),\n> > + * V4L2Device::getControls() and V4L2VideoDevice::queueBuffer().\n> > + */\n> > +\n> > +/**\n> > + * \\brief Construct a V4L2Request\n> > + * \\param[in] fd The request fd\n> > + */\n> > +V4L2Request::V4L2Request(int fd)\n> > +       : fd_(fd), fdNotifier_(fd, EventNotifier::Exception)\n> > +{\n> > +       if (!fd_.isValid())\n> > +               return;\n> > +\n> > +       fdNotifier_.activated.connect(this, &V4L2Request::requestReady);\n> > +       fdNotifier_.setEnabled(false);\n> > +}\n> > +\n> > +/**\n> > + * \\fn V4L2Request::isValid()\n> > + * \\brief Check if the request is valid\n> > + *\n> > + * Checks if the underlying fd is valid.\n> > + *\n> > + * \\return True if the request is valid, false otherwise\n> > + */\n> > +\n> > +/**\n> > + * \\fn V4L2Request::fd()\n> > + * \\brief Get the file descriptor\n> > + *\n> > + * \\return The file descriptor wrapped by this V4L2Request\n> > + */\n> > +\n> > +/**\n> > + * \\var V4L2Request::requestDone\n> > + * \\brief Signal that is emitted when the request is done\n> > + */\n> > +\n> > +/**\n> > + * \\brief Reinit the request\n> > + *\n> > + * Calls MEDIA_REQUEST_IOC_REINIT om the request fd.\n> \n> s/om/on/\n> \n> > + *\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +int V4L2Request::reinit()\n> > +{\n> > +       fdNotifier_.setEnabled(false);\n> > +\n> > +       if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_REINIT) < 0)\n> > +               return -errno;\n> > +\n> > +       return 0;\n> > +}\n> > +\n> > +/**\n> > + * \\brief Reinit the request\n> \n> Reinit? :)\n> \n> > + *\n> > + * Calls MEDIA_REQUEST_IOC_QUEUE om the request fd.\n> \n> s/om/on/\n> \n\nThanks, I fixed all the typos, but left the other ones as is for now,\nbecause I didn't feel like the path forward is completely clear.\n\nCheers,\nStefan\n\n> \n> Thanks,\n> \n> Paul\n> \n> > + *\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +int V4L2Request::queue()\n> > +{\n> > +       if (::ioctl(fd_.get(), MEDIA_REQUEST_IOC_QUEUE) < 0)\n> > +               return -errno;\n> > +\n> > +       fdNotifier_.setEnabled(true);\n> > +\n> > +       return 0;\n> > +}\n> > +\n> > +std::string V4L2Request::logPrefix() const\n> > +{\n> > +       return \"Request [\" + std::to_string(fd()) + \"]\";\n> > +}\n> > +\n> > +/**\n> > + * \\brief Slot to handle request done events\n> > + *\n> > + * When this slot is called, the request is done and the requestDone will be\n> > + * emitted.\n> > + */\n> > +void V4L2Request::requestReady()\n> > +{\n> > +       requestDone.emit(this);\n> > +}\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> > index bb57c1b76a5b..8ce739f4bc65 100644\n> > --- a/src/libcamera/v4l2_videodevice.cpp\n> > +++ b/src/libcamera/v4l2_videodevice.cpp\n> > @@ -30,6 +30,7 @@\n> >  #include \"libcamera/internal/framebuffer.h\"\n> >  #include \"libcamera/internal/media_device.h\"\n> >  #include \"libcamera/internal/media_object.h\"\n> > +#include \"libcamera/internal/v4l2_request.h\"\n> >  \n> >  /**\n> >   * \\file v4l2_videodevice.h\n> > @@ -1629,6 +1630,7 @@ int V4L2VideoDevice::releaseBuffers()\n> >  /**\n> >   * \\brief Queue a buffer to the video device\n> >   * \\param[in] buffer The buffer to be queued\n> > + * \\param[in] request An optional request\n> >   *\n> >   * For capture video devices the \\a buffer will be filled with data by the\n> >   * device. For output video devices the \\a buffer shall contain valid data and\n> > @@ -1641,9 +1643,11 @@ int V4L2VideoDevice::releaseBuffers()\n> >   * Note that queueBuffer() will fail if the device is in the process of being\n> >   * stopped from a streaming state through streamOff().\n> >   *\n> > + * If \\a request is specified, the buffer will be tied to that request.\n> > + *\n> >   * \\return 0 on success or a negative error code otherwise\n> >   */\n> > -int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)\n> > +int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer, const V4L2Request *request)\n> >  {\n> >         struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {};\n> >         struct v4l2_buffer buf = {};\n> > @@ -1674,6 +1678,10 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)\n> >         buf.type = bufferType_;\n> >         buf.memory = memoryType_;\n> >         buf.field = V4L2_FIELD_NONE;\n> > +       if (request) {\n> > +               buf.flags = V4L2_BUF_FLAG_REQUEST_FD;\n> > +               buf.request_fd = request->fd();\n> > +       }\n> >  \n> >         bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type);\n> >         Span<const FrameBuffer::Plane> planes = buffer->planes();\n> > -- \n> > 2.48.1\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 04E36C32E0\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Nov 2025 16:10:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1258C60A80;\n\tMon, 24 Nov 2025 17:10:54 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7A3276096B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Nov 2025 17:10:53 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:3dd5:c977:12b4:d480])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id DE9E1741;\n\tMon, 24 Nov 2025 17:08:44 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"L3jIEc3+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764000525;\n\tbh=sAuhX1NgarYKFM4f3B+d4InG5UQpJ5l6FPQ6gFO6Wk8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=L3jIEc3+xvEHQ2Ck2/+2gbhAk4paBI5Lz5+HWAs7mD6be3eOen90fBcZCvdMcmqjO\n\tlKMcXyoNp8JjnyFwzX9CYEson1uuMnwX3RszJvAn21jyPEMMpQahReRcIFFIBE9Z1H\n\tDGXZ6hmdjs2KW3rDJu5JV5EanM3pbBcTODenVutA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<176236116296.2116251.11332755061547134434@neptunite.rasen.tech>","References":"<20251023144841.403689-1-stefan.klug@ideasonboard.com>\n\t<20251023144841.403689-6-stefan.klug@ideasonboard.com>\n\t<176236116296.2116251.11332755061547134434@neptunite.rasen.tech>","Subject":"Re: [PATCH v2 05/35] libcamera: Add support for V4L2 requests","From":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"","To":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 24 Nov 2025 17:10:49 +0100","Message-ID":"<176400064992.1350675.10953596602547025927@localhost>","User-Agent":"alot/0.12.dev8+g2c003385c862.d20250602","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":37120,"web_url":"https://patchwork.libcamera.org/comment/37120/","msgid":"<6e8e77a1-3f42-4a82-b7da-49778dcf44b0@ideasonboard.com>","date":"2025-12-01T09:48:37","subject":"Re: [PATCH v2 05/35] libcamera: Add support for V4L2 requests","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 11. 24. 16:27 keltezéssel, Stefan Klug írta:\n> Hi Barnabás,\n> \n> Thank you for the review.\n> \n> As always I should have spent more time reading the review earlier and\n> not just before prearing the new series...\n> \n> Quoting Barnabás Pőcze (2025-10-24 16:00:32)\n>> Hi\n>>\n>> 2025. 10. 23. 16:48 keltezéssel, Stefan Klug írta:\n>>> The V4L2 requests API provides support to atomically tie controls to a\n>>> set of buffers. This is especially common for m2m devices. Such a\n>>> request is represented by a fd that is allocated vi\n>>> MEDIA_IOC_REQUEST_ALLOC and then passed to various V4L2 function.\n>>>\n>>> Implement a V4L2Request class to wrap such an fd and add the\n>>> corresponding utility functions.\n>>>\n>>> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n>>>\n>>> ---\n>>>\n>>> Changes in v2:\n>>> - Added documentation\n>>> ---\n>>>    include/libcamera/internal/media_device.h     |   7 +\n>>>    include/libcamera/internal/meson.build        |   1 +\n>>>    include/libcamera/internal/v4l2_device.h      |   5 +-\n>>>    include/libcamera/internal/v4l2_request.h     |  49 +++++++\n>>>    include/libcamera/internal/v4l2_videodevice.h |   3 +-\n>>>    src/libcamera/media_device.cpp                |  47 +++++++\n>>>    src/libcamera/meson.build                     |   1 +\n>>>    src/libcamera/v4l2_device.cpp                 |  28 +++-\n>>>    src/libcamera/v4l2_request.cpp                | 128 ++++++++++++++++++\n>>>    src/libcamera/v4l2_videodevice.cpp            |  10 +-\n>>>    10 files changed, 271 insertions(+), 8 deletions(-)\n>>>    create mode 100644 include/libcamera/internal/v4l2_request.h\n>>>    create mode 100644 src/libcamera/v4l2_request.cpp\n>>>\n>>> [...]\n>>> diff --git a/src/libcamera/v4l2_request.cpp b/src/libcamera/v4l2_request.cpp\n>>> new file mode 100644\n>>> index 000000000000..708250d86f61\n>>> --- /dev/null\n>>> +++ b/src/libcamera/v4l2_request.cpp\n>>> @@ -0,0 +1,128 @@\n>>> [...]\n>>> +/**\n>>> + * \\brief Construct a V4L2Request\n>>> + * \\param[in] fd The request fd\n>>> + */\n>>> +V4L2Request::V4L2Request(int fd)\n>>> +     : fd_(fd), fdNotifier_(fd, EventNotifier::Exception)\n>>> +{\n>>> +     if (!fd_.isValid())\n>>> +             return;\n>>> +\n>>> +     fdNotifier_.activated.connect(this, &V4L2Request::requestReady);\n>>\n>> It's probably just me, but I feel like I would just use a simple lambda here.\n> \n> I'd prefer a lambda, too. I didn't get it to work though. The code I'd\n> expect there would be something like:\n> \n> fdNotifier_.activated.connect([this] { this->requestDone.emit(this); });\n> \n> But that doesn't work as capturing lambdas can't be converted to a\n> function pointer. I didn't dig deeper into the Signal implementation.\n> How would you do that?\n\nI see this is already merged, but it would be:\n\n   fdNotifier_.activated.connect(this, [this] { requestDone.emit(this); });\n\n\n> \n> Best regards,\n> Stefan\n> \n>>\n>>\n>>> +     fdNotifier_.setEnabled(false);\n>>> +}\n>>> [...]\n>>\n>>\n>> Regards,\n>> Barnabás Pőcze\n>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id AB0D9C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  1 Dec 2025 09:48:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A743760AB8;\n\tMon,  1 Dec 2025 10:48:42 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 84416609D8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Dec 2025 10:48:40 +0100 (CET)","from [192.168.33.24] (185.182.214.104.nat.pool.zt.hu\n\t[185.182.214.104])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2DECB4F1;\n\tMon,  1 Dec 2025 10:46:27 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"eAMxK45e\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764582387;\n\tbh=b0P08VFvFCjAtvcggTuZZNRtWiImFrOjWwtOwH1OUeE=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=eAMxK45ejfycBeGoIBDTraeKrXHURdaWAa9TSm5Ao4urnRy8vuGCUfC8Nhgug0S6L\n\tQCb9YiBEsN2lO+G6hIHhgGksnMisFsXzpvBy5OrbH3PFrpnBCayYZllNPAkqwJ1P//\n\tb0HGktIROvEyAnsmWNmiwGzbrpZrTdvqZvKlh/p4=","Message-ID":"<6e8e77a1-3f42-4a82-b7da-49778dcf44b0@ideasonboard.com>","Date":"Mon, 1 Dec 2025 10:48:37 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 05/35] libcamera: Add support for V4L2 requests","To":"Stefan Klug <stefan.klug@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251023144841.403689-1-stefan.klug@ideasonboard.com>\n\t<20251023144841.403689-6-stefan.klug@ideasonboard.com>\n\t<18f2c0c1-c9e5-444a-88a2-59956e698abf@ideasonboard.com>\n\t<176399806422.1350675.17922189972244077830@localhost>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<176399806422.1350675.17922189972244077830@localhost>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]