[{"id":36480,"web_url":"https://patchwork.libcamera.org/comment/36480/","msgid":"<c6032170-a497-4407-9422-639900fff148@ideasonboard.com>","date":"2025-10-27T11:48:28","subject":"Re: [PATCH 1/4] libcamera: v4l2: Support string control payloads","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n\n2025. 10. 23. 12:56 keltezéssel, Isaac Scott írta:\n> v4l2 controls can be configured to store a string. This is stored in the\n> v4l2_ctrl_ext when the union within it points to a string as its\n> payload.\n> \n> This can be read similarly to int control payloadsm instead using the\n> maximum length of the string, represented by info.maximum.\n> \n> This pointer can then be reinterpreted as a char *.\n> \n> N.B. It is possible that if the string is less than the maximum length,\n> the data after the string is populated with garbage. This isn't a\n> problem when using the string later, as the std::string will only use up\n> to the null character in the string.\n> \n> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>\n> ---\n>   include/libcamera/controls.h  |  3 ++-\n>   src/libcamera/controls.cpp    | 11 ++++++++---\n>   src/libcamera/v4l2_device.cpp | 28 ++++++++++++++++++++++++++++\n>   3 files changed, 38 insertions(+), 4 deletions(-)\n> \n> [...]\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 8c78b8c42..641ea4981 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -12,6 +12,7 @@\n>   #include <stdint.h>\n>   #include <stdlib.h>\n>   #include <string.h>\n> +#include <string_view>\n>   #include <sys/ioctl.h>\n>   #include <sys/syscall.h>\n>   #include <unistd.h>\n> @@ -25,6 +26,8 @@\n> \n>   #include \"libcamera/internal/formats.h\"\n>   #include \"libcamera/internal/sysfs.h\"\n> +#include \"libcamera/controls.h\"\n> +#include \"linux/videodev2.h\"\n> \n>   /**\n>    * \\file v4l2_device.h\n> @@ -230,6 +233,13 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids)\n>   \t\t\t\tv4l2Ctrl.p_u32 = reinterpret_cast<uint32_t *>(data.data());\n>   \t\t\t\tbreak;\n> \n> +\t\t\tcase V4L2_CTRL_TYPE_STRING:\n> +\t\t\t\ttype = ControlTypeString;\n> +\t\t\t\tvalue.reserve(type, false, info.maximum + 1);\n\nWhy does it need `isArray == false`? I suppose we should finally decide if strings are\narrays or not ( https://gitlab.freedesktop.org/camera/libcamera/-/issues/255 ).\n\nGiven how things are currently implemented, and given that strings are not even\nnull terminated in string controls, I see no reason to make them so special, so\nI think we should remove `ControlTypeString` altogether, and just have an array of `char[]`.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> +\t\t\t\tdata = value.data();\n> +\t\t\t\tv4l2Ctrl.string = reinterpret_cast<char *>(data.data());\n> +\t\t\t\tbreak;\n> +\n>   \t\t\tdefault:\n>   \t\t\t\tLOG(V4L2, Error)\n>   \t\t\t\t\t<< \"Unsupported payload control type \"\n> @@ -570,6 +580,8 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)\n>   \t\t * integer type.\n>   \t\t */\n>   \t\treturn ControlTypeInteger32;\n> +\tcase V4L2_CTRL_TYPE_STRING:\n> +\t\treturn ControlTypeString;\n> \n>   \tdefault:\n>   \t\treturn ControlTypeNone;\n> @@ -709,6 +721,7 @@ void V4L2Device::listControls()\n>   \t\tcase V4L2_CTRL_TYPE_U8:\n>   \t\tcase V4L2_CTRL_TYPE_U16:\n>   \t\tcase V4L2_CTRL_TYPE_U32:\n> +\t\tcase V4L2_CTRL_TYPE_STRING:\n>   \t\t\tbreak;\n>   \t\t/* \\todo Support other control types. */\n>   \t\tdefault:\n> @@ -808,6 +821,21 @@ void V4L2Device::updateControls(ControlList *ctrls,\n>   \t\t\tvalue.set<int64_t>(v4l2Ctrl.value64);\n>   \t\t\tbreak;\n> \n> +\t\tcase ControlTypeString:\n> +\t\t\t/*\n> +\t\t\t * The ControlValue contains storage for the  maximum\n> +\t\t\t * length of the string, and its size matches that. After\n> +\t\t\t * the data is retrieved, it must be resized so ControlValue::numElements()\n> +\t\t\t * is correct.\n> +\t\t\t*\n> +\t\t\t * VIDIOC_G_EXT_CTRLS does not return the length of the string,\n> +\t\t\t * so we must reassign and let the std::string_view constructor\n> +\t\t\t * calculate the true length. Because value is a copy, there will be\n> +\t\t\t * no use-after-free issues.\n> +\t\t\t */\n> +\t\t\tvalue.set<std::string_view>(v4l2Ctrl.string);\n> +\t\t\tbreak;\n> +\n>   \t\tdefault:\n>   \t\t\t/*\n>   \t\t\t * Note: this catches the ControlTypeInteger32 case.\n> --\n> 2.43.0\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 AB071C3259\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 27 Oct 2025 11:48:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B400D60758;\n\tMon, 27 Oct 2025 12:48:34 +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 336A56069A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Oct 2025 12:48:32 +0100 (CET)","from [192.168.33.25] (185.182.215.162.nat.pool.zt.hu\n\t[185.182.215.162])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EDE501122;\n\tMon, 27 Oct 2025 12:46:43 +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=\"L4K4lyiK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761565604;\n\tbh=t0RnSNZROJdz3fy+Pdf2vgBps+TnZXxWiyWf2hXvcl8=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=L4K4lyiK6xj1g6kpXW6qTyO/TwvBsqAEzseJ9fEWNnQNUlPbG99XmOqI08z7zoyIq\n\tS2RH+vsrNlL7Vr01oKZ/6yr/722QlJ1w3FGEUBG9ByimbxzZN8XK/oWI9k3Lacmf+q\n\tzu7j5j81tAeYdGw2PqAPkh8n5Liwa3UsRrS6nHYM=","Message-ID":"<c6032170-a497-4407-9422-639900fff148@ideasonboard.com>","Date":"Mon, 27 Oct 2025 12:48:28 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 1/4] libcamera: v4l2: Support string control payloads","To":"Isaac Scott <isaac.scott@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251023105651.78395-1-isaac.scott@ideasonboard.com>\n\t<eFgOMQeW_ydsxVnt3vwNZlXn9YTomVEUyTycW7BA8S8TgfJ7AJYuw0W8WXcEk8x5HuGTJoKZQu4jZUkhlY0LxQ==@protonmail.internalid>\n\t<20251023105651.78395-2-isaac.scott@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":"<20251023105651.78395-2-isaac.scott@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>"}}]