[{"id":31937,"web_url":"https://patchwork.libcamera.org/comment/31937/","msgid":"<3u4djitcrcv62qodtapynuzw6xtjdikmnanyrsnit53mcthnwt@yvy2sonegonl>","date":"2024-10-28T10:36:17","subject":"Re: [PATCH v4 1/2] libcamera: Extend u32 control type","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Harvey\n\nOn Fri, Oct 25, 2024 at 03:38:50PM +0000, Harvey Yang wrote:\n> From: Yudhistira Erlandinata \n>\n> V4L2 Controls support a wide variety of types not yet supported by the\n> ControlValue type system.\n>\n> Extend the libcamera ControlValue types to support an explicit 32 bit\n> unsigned integer type, and map that to the corresponding\n> V4L2_CTRL_TYPE_U32 type within the v4l2_device support class.\n>\n> Signed-off-by: Yudhistira Erlandinata \n> Co-developed-by: Harvey Yang \n> Signed-off-by: Harvey Yang \n> ---\n> include/libcamera/controls.h | 7 ++++++\n> src/libcamera/controls.cpp | 12 ++++++++--\n> src/libcamera/v4l2_device.cpp | 14 ++++++++++++\n> test/controls/control_value.cpp | 40 +++++++++++++++++++++++++++++++++\n> 4 files changed, 71 insertions(+), 2 deletions(-)\n>\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index ca60bbaca..6da8ad2c3 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -29,6 +29,7 @@ enum ControlType {\n> \tControlTypeNone,\n> \tControlTypeBool,\n> \tControlTypeByte,\n> +\tControlTypeUnsigned32,\n> \tControlTypeInteger32,\n> \tControlTypeInteger64,\n> \tControlTypeFloat,\n> @@ -62,6 +63,12 @@ struct control_type {\n> \tstatic constexpr std::size_t size = 0;\n> };\n>\n> +template<>\n> +struct control_type {\n> +\tstatic constexpr ControlType value = ControlTypeUnsigned32;\n> +\tstatic constexpr std::size_t size = 0;\n> +};\n> +\n> template<>\n> struct control_type {\n> \tstatic constexpr ControlType value = ControlTypeInteger32;\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 62185d643..8ae295191 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -54,6 +54,7 @@ static constexpr size_t ControlValueSize[] = {\n> \t[ControlTypeNone]\t\t= 0,\n> \t[ControlTypeBool]\t\t= sizeof(bool),\n> \t[ControlTypeByte]\t\t= sizeof(uint8_t),\n> +\t[ControlTypeUnsigned32]\t\t= sizeof(uint32_t),\n> \t[ControlTypeInteger32]\t\t= sizeof(int32_t),\n> \t[ControlTypeInteger64]\t\t= sizeof(int64_t),\n> \t[ControlTypeFloat]\t\t= sizeof(float),\n> @@ -74,10 +75,12 @@ static constexpr size_t ControlValueSize[] = {\n> * The control stores a boolean value\n> * \\var ControlTypeByte\n> * The control stores a byte value as an unsigned 8-bit integer\n> + * \\var ControlTypeUnsigned32\n> + * The control stores an unsigned 32-bit integer value\n> * \\var ControlTypeInteger32\n> - * The control stores a 32-bit integer value\n> + * The control stores a signed 32-bit integer value\n> * \\var ControlTypeInteger64\n> - * The control stores a 64-bit integer value\n> + * The control stores a signed 64-bit integer value\n> * \\var ControlTypeFloat\n> * The control stores a 32-bit floating point value\n> * \\var ControlTypeString\n> @@ -230,6 +233,11 @@ std::string ControlValue::toString() const\n> \t\t\tstr += std::to_string(*value);\n> \t\t\tbreak;\n> \t\t}\n> +\t\tcase ControlTypeUnsigned32: {\n> +\t\t\tconst uint32_t *value = reinterpret_cast(data);\n> +\t\t\tstr += std::to_string(*value);\n> +\t\t\tbreak;\n> +\t\t}\n> \t\tcase ControlTypeInteger32: {\n> \t\t\tconst int32_t *value = reinterpret_cast(data);\n> \t\t\tstr += std::to_string(*value);\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 68add4f2e..f39ec3760 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -9,6 +9,7 @@\n>\n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -210,6 +211,10 @@ ControlList V4L2Device::getControls(const std::vector &ids)\n> \t\t\t\ttype = ControlTypeByte;\n> \t\t\t\tbreak;\n>\n> +\t\t\tcase V4L2_CTRL_TYPE_U32:\n> +\t\t\t\ttype = ControlTypeUnsigned32;\n> +\t\t\t\tbreak;\n> +\n\nThis is not enough.\n\nAfter the switch we have\n\n\t\t\tControlValue &value = ctrl.second;\n\t\t\tvalue.reserve(type, true, info.elems);\n\t\t\tSpan data = value.data();\n\n\t\t\tv4l2Ctrl.p_u8 = data.data();\n\t\t\tv4l2Ctrl.size = data.size();\n\nHowever controls of type V4L2_CTRL_TYPE_U32 use the p_u32 union member\nof 'struct v4l2_ext_control'\nhttps://www.kernel.org/doc/html/v4.9/media/uapi/v4l/vidioc-g-ext-ctrls.html#id1\n\nSame for U16 control types.\n\nBe careful about one thing. v4l2Ctrl.p_u32 is of type __u32* so it\nwould feel natural to\n\n\t\t\tSpan data = value.data();\n\t\t\tv4l2Ctrl.p_u32 = data.data();\n\nas data.data() returns a T*\n\nHowever v4l2Ctrl.size wants the size in bytes while data.size()\nreturns the number of elements, so you should probably use\ndata.size_bytes().\n\nOr you can do as we do in setControls and cast the data.data()\npointer.\n\n Span data = value.data();\n v4l2Ctrl.p_u32 = reinterpret_cast(data.data());\n v4l2Ctrl.size = data.size();\n\nAlso, you need to handle U32 and U16 also setControl().\n\nIf you can test this by reading and writing u16 and u32 controls this\nwould make me way more confident this patch is correct.\n\nThanks\n j\n\n> \t\t\tdefault:\n> \t\t\t\tLOG(V4L2, Error)\n> \t\t\t\t\t<< \"Unsupported payload control type \"\n> @@ -488,6 +493,9 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)\n> \tcase V4L2_CTRL_TYPE_BOOLEAN:\n> \t\treturn ControlTypeBool;\n>\n> +\tcase V4L2_CTRL_TYPE_U32:\n> +\t\treturn ControlTypeUnsigned32;\n> +\n> \tcase V4L2_CTRL_TYPE_INTEGER:\n> \t\treturn ControlTypeInteger32;\n>\n> @@ -536,6 +544,11 @@ std::optional V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl\n> \t\t\t\t static_cast(ctrl.maximum),\n> \t\t\t\t static_cast(ctrl.default_value));\n>\n> +\tcase V4L2_CTRL_TYPE_U32:\n> +\t\treturn ControlInfo(static_cast(ctrl.minimum),\n> +\t\t\t\t static_cast(ctrl.maximum),\n> +\t\t\t\t static_cast(ctrl.default_value));\n> +\n> \tcase V4L2_CTRL_TYPE_BOOLEAN:\n> \t\treturn ControlInfo(static_cast(ctrl.minimum),\n> \t\t\t\t static_cast(ctrl.maximum),\n> @@ -622,6 +635,7 @@ void V4L2Device::listControls()\n> \t\tcase V4L2_CTRL_TYPE_BITMASK:\n> \t\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n> \t\tcase V4L2_CTRL_TYPE_U8:\n> +\t\tcase V4L2_CTRL_TYPE_U32:\n> \t\t\tbreak;\n> \t\t/* \\todo Support other control types. */\n> \t\tdefault:\n> diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n> index 344107fae..6ca85b739 100644\n> --- a/test/controls/control_value.cpp\n> +++ b/test/controls/control_value.cpp\n> @@ -109,6 +109,46 @@ protected:\n> \t\t\treturn TestFail;\n> \t\t}\n>\n> +\t\t/*\n> +\t\t * Unsigned Integer32 type.\n> +\t\t */\n> +\t\tvalue.set(static_cast(42));\n> +\t\tif (value.isNone() || value.isArray() ||\n> +\t\t value.type() != ControlTypeUnsigned32) {\n> +\t\t\tcerr << \"Control type mismatch after setting to uint32_t\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (value.get() != 42) {\n> +\t\t\tcerr << \"Control value mismatch after setting to uint32_t\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (value.toString() != \"42\") {\n> +\t\t\tcerr << \"Control string mismatch after setting to uint32_t\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tstd::array uint32s{ 3, 14, 15, 9 };\n> +\t\tvalue.set(Span(uint32s));\n> +\t\tif (value.isNone() || !value.isArray() ||\n> +\t\t value.type() != ControlTypeUnsigned32) {\n> +\t\t\tcerr << \"Control type mismatch after setting to uint32_t array\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tSpan uint32sResult = value.get>();\n> +\t\tif (uint32s.size() != uint32sResult.size() ||\n> +\t\t !std::equal(uint32s.begin(), uint32s.end(), uint32sResult.begin())) {\n> +\t\t\tcerr << \"Control value mismatch after setting to uint32_t array\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (value.toString() != \"[ 3, 14, 15, 9 ]\") {\n> +\t\t\tcerr << \"Control string mismatch after setting to uint32_t array\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> \t\t/*\n> \t\t * Integer32 type.\n> \t\t */\n> --\n> 2.47.0.163.g1226f6d8fa-goog\n>","headers":{"Return-Path":"","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 D24BEBD78E\n\tfor ;\n\tMon, 28 Oct 2024 10:36:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D4E9A6539E;\n\tMon, 28 Oct 2024 11:36:26 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C61CF60360\n\tfor ;\n\tMon, 28 Oct 2024 11:36:24 +0100 (CET)","from ideasonboard.com (mob-5-90-59-111.net.vodafone.it\n\t[5.90.59.111])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 60F93346;\n\tMon, 28 Oct 2024 11:36:19 +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=\"NjP/GOAE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730111780;\n\tbh=i5wnjxPdN0wF0gdaqDC8Ay9r1jauSWsV2YRHxP8iT8E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NjP/GOAELx4U21U1KbibBOjmJ+ryxmyFym7bYHslnwk0Ns7JR/WxpsfzsbW1WceeA\n\tUlvp1DzNiWOi9OVXSucdnGB20TD8ZZi56BapQTcxc0qx7UxkwvcPw2njkzQkofHp3F\n\t5uPGDqg0gj88nbENdYfoKpVzeHseKkjb3vgmnzDE=","Date":"Mon, 28 Oct 2024 11:36:17 +0100","From":"Jacopo Mondi ","To":"Harvey Yang ","Cc":"libcamera-devel@lists.libcamera.org, \n\tYudhistira Erlandinata ","Subject":"Re: [PATCH v4 1/2] libcamera: Extend u32 control type","Message-ID":"<3u4djitcrcv62qodtapynuzw6xtjdikmnanyrsnit53mcthnwt@yvy2sonegonl>","References":"<20241025153948.1213629-1-chenghaoyang@chromium.org>\n\t<20241025153948.1213629-2-chenghaoyang@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241025153948.1213629-2-chenghaoyang@chromium.org>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31957,"web_url":"https://patchwork.libcamera.org/comment/31957/","msgid":"","date":"2024-10-29T10:08:44","subject":"Re: [PATCH v4 1/2] libcamera: Extend u32 control type","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Jacopo,\n\nOn Mon, Oct 28, 2024 at 6:36 PM Jacopo Mondi\n wrote:\n>\n> Hi Harvey\n>\n> On Fri, Oct 25, 2024 at 03:38:50PM +0000, Harvey Yang wrote:\n> > From: Yudhistira Erlandinata \n> >\n> > V4L2 Controls support a wide variety of types not yet supported by the\n> > ControlValue type system.\n> >\n> > Extend the libcamera ControlValue types to support an explicit 32 bit\n> > unsigned integer type, and map that to the corresponding\n> > V4L2_CTRL_TYPE_U32 type within the v4l2_device support class.\n> >\n> > Signed-off-by: Yudhistira Erlandinata \n> > Co-developed-by: Harvey Yang \n> > Signed-off-by: Harvey Yang \n> > ---\n> > include/libcamera/controls.h | 7 ++++++\n> > src/libcamera/controls.cpp | 12 ++++++++--\n> > src/libcamera/v4l2_device.cpp | 14 ++++++++++++\n> > test/controls/control_value.cpp | 40 +++++++++++++++++++++++++++++++++\n> > 4 files changed, 71 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > index ca60bbaca..6da8ad2c3 100644\n> > --- a/include/libcamera/controls.h\n> > +++ b/include/libcamera/controls.h\n> > @@ -29,6 +29,7 @@ enum ControlType {\n> > ControlTypeNone,\n> > ControlTypeBool,\n> > ControlTypeByte,\n> > + ControlTypeUnsigned32,\n> > ControlTypeInteger32,\n> > ControlTypeInteger64,\n> > ControlTypeFloat,\n> > @@ -62,6 +63,12 @@ struct control_type {\n> > static constexpr std::size_t size = 0;\n> > };\n> >\n> > +template<>\n> > +struct control_type {\n> > + static constexpr ControlType value = ControlTypeUnsigned32;\n> > + static constexpr std::size_t size = 0;\n> > +};\n> > +\n> > template<>\n> > struct control_type {\n> > static constexpr ControlType value = ControlTypeInteger32;\n> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > index 62185d643..8ae295191 100644\n> > --- a/src/libcamera/controls.cpp\n> > +++ b/src/libcamera/controls.cpp\n> > @@ -54,6 +54,7 @@ static constexpr size_t ControlValueSize[] = {\n> > [ControlTypeNone] = 0,\n> > [ControlTypeBool] = sizeof(bool),\n> > [ControlTypeByte] = sizeof(uint8_t),\n> > + [ControlTypeUnsigned32] = sizeof(uint32_t),\n> > [ControlTypeInteger32] = sizeof(int32_t),\n> > [ControlTypeInteger64] = sizeof(int64_t),\n> > [ControlTypeFloat] = sizeof(float),\n> > @@ -74,10 +75,12 @@ static constexpr size_t ControlValueSize[] = {\n> > * The control stores a boolean value\n> > * \\var ControlTypeByte\n> > * The control stores a byte value as an unsigned 8-bit integer\n> > + * \\var ControlTypeUnsigned32\n> > + * The control stores an unsigned 32-bit integer value\n> > * \\var ControlTypeInteger32\n> > - * The control stores a 32-bit integer value\n> > + * The control stores a signed 32-bit integer value\n> > * \\var ControlTypeInteger64\n> > - * The control stores a 64-bit integer value\n> > + * The control stores a signed 64-bit integer value\n> > * \\var ControlTypeFloat\n> > * The control stores a 32-bit floating point value\n> > * \\var ControlTypeString\n> > @@ -230,6 +233,11 @@ std::string ControlValue::toString() const\n> > str += std::to_string(*value);\n> > break;\n> > }\n> > + case ControlTypeUnsigned32: {\n> > + const uint32_t *value = reinterpret_cast(data);\n> > + str += std::to_string(*value);\n> > + break;\n> > + }\n> > case ControlTypeInteger32: {\n> > const int32_t *value = reinterpret_cast(data);\n> > str += std::to_string(*value);\n> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > index 68add4f2e..f39ec3760 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -9,6 +9,7 @@\n> >\n> > #include \n> > #include \n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -210,6 +211,10 @@ ControlList V4L2Device::getControls(const std::vector &ids)\n> > type = ControlTypeByte;\n> > break;\n> >\n> > + case V4L2_CTRL_TYPE_U32:\n> > + type = ControlTypeUnsigned32;\n> > + break;\n> > +\n>\n> This is not enough.\n>\n> After the switch we have\n>\n> ControlValue &value = ctrl.second;\n> value.reserve(type, true, info.elems);\n> Span data = value.data();\n>\n> v4l2Ctrl.p_u8 = data.data();\n> v4l2Ctrl.size = data.size();\n>\n> However controls of type V4L2_CTRL_TYPE_U32 use the p_u32 union member\n> of 'struct v4l2_ext_control'\n> https://www.kernel.org/doc/html/v4.9/media/uapi/v4l/vidioc-g-ext-ctrls.html#id1\n>\n> Same for U16 control types.\n>\n> Be careful about one thing. v4l2Ctrl.p_u32 is of type __u32* so it\n> would feel natural to\n>\n> Span data = value.data();\n> v4l2Ctrl.p_u32 = data.data();\n>\n> as data.data() returns a T*\n>\n> However v4l2Ctrl.size wants the size in bytes while data.size()\n> returns the number of elements, so you should probably use\n> data.size_bytes().\n>\n> Or you can do as we do in setControls and cast the data.data()\n> pointer.\n>\n> Span data = value.data();\n> v4l2Ctrl.p_u32 = reinterpret_cast(data.data());\n> v4l2Ctrl.size = data.size();\n>\n> Also, you need to handle U32 and U16 also setControl().\n\nThanks for the catch!\nPlease help take a look at my WIP patch:\nhttps://gitlab.freedesktop.org/chenghaoyang/libcamera/-/commit/faec47f605881062e58718371183a0c2d3ffe772\n, before I upload another version.\nLet me know if you think I should merge the two `switch`es and duplicate\nthe code in the middle.\n\n>\n> If you can test this by reading and writing u16 and u32 controls this\n> would make me way more confident this patch is correct.\n\nHmm, I couldn't find such a use case. Can you suggest how we could\ntest it?\n\nBR,\nHarvey\n\n>\n> Thanks\n> j\n>\n> > default:\n> > LOG(V4L2, Error)\n> > << \"Unsupported payload control type \"\n> > @@ -488,6 +493,9 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)\n> > case V4L2_CTRL_TYPE_BOOLEAN:\n> > return ControlTypeBool;\n> >\n> > + case V4L2_CTRL_TYPE_U32:\n> > + return ControlTypeUnsigned32;\n> > +\n> > case V4L2_CTRL_TYPE_INTEGER:\n> > return ControlTypeInteger32;\n> >\n> > @@ -536,6 +544,11 @@ std::optional V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl\n> > static_cast(ctrl.maximum),\n> > static_cast(ctrl.default_value));\n> >\n> > + case V4L2_CTRL_TYPE_U32:\n> > + return ControlInfo(static_cast(ctrl.minimum),\n> > + static_cast(ctrl.maximum),\n> > + static_cast(ctrl.default_value));\n> > +\n> > case V4L2_CTRL_TYPE_BOOLEAN:\n> > return ControlInfo(static_cast(ctrl.minimum),\n> > static_cast(ctrl.maximum),\n> > @@ -622,6 +635,7 @@ void V4L2Device::listControls()\n> > case V4L2_CTRL_TYPE_BITMASK:\n> > case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > case V4L2_CTRL_TYPE_U8:\n> > + case V4L2_CTRL_TYPE_U32:\n> > break;\n> > /* \\todo Support other control types. */\n> > default:\n> > diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n> > index 344107fae..6ca85b739 100644\n> > --- a/test/controls/control_value.cpp\n> > +++ b/test/controls/control_value.cpp\n> > @@ -109,6 +109,46 @@ protected:\n> > return TestFail;\n> > }\n> >\n> > + /*\n> > + * Unsigned Integer32 type.\n> > + */\n> > + value.set(static_cast(42));\n> > + if (value.isNone() || value.isArray() ||\n> > + value.type() != ControlTypeUnsigned32) {\n> > + cerr << \"Control type mismatch after setting to uint32_t\" << endl;\n> > + return TestFail;\n> > + }\n> > +\n> > + if (value.get() != 42) {\n> > + cerr << \"Control value mismatch after setting to uint32_t\" << endl;\n> > + return TestFail;\n> > + }\n> > +\n> > + if (value.toString() != \"42\") {\n> > + cerr << \"Control string mismatch after setting to uint32_t\" << endl;\n> > + return TestFail;\n> > + }\n> > +\n> > + std::array uint32s{ 3, 14, 15, 9 };\n> > + value.set(Span(uint32s));\n> > + if (value.isNone() || !value.isArray() ||\n> > + value.type() != ControlTypeUnsigned32) {\n> > + cerr << \"Control type mismatch after setting to uint32_t array\" << endl;\n> > + return TestFail;\n> > + }\n> > +\n> > + Span uint32sResult = value.get>();\n> > + if (uint32s.size() != uint32sResult.size() ||\n> > + !std::equal(uint32s.begin(), uint32s.end(), uint32sResult.begin())) {\n> > + cerr << \"Control value mismatch after setting to uint32_t array\" << endl;\n> > + return TestFail;\n> > + }\n> > +\n> > + if (value.toString() != \"[ 3, 14, 15, 9 ]\") {\n> > + cerr << \"Control string mismatch after setting to uint32_t array\" << endl;\n> > + return TestFail;\n> > + }\n> > +\n> > /*\n> > * Integer32 type.\n> > */\n> > --\n> > 2.47.0.163.g1226f6d8fa-goog\n> >","headers":{"Return-Path":"","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 72069C31E9\n\tfor ;\n\tTue, 29 Oct 2024 10:08:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 628E66539E;\n\tTue, 29 Oct 2024 11:08:58 +0100 (CET)","from mail-lj1-x22c.google.com (mail-lj1-x22c.google.com\n\t[IPv6:2a00:1450:4864:20::22c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5369860366\n\tfor ;\n\tTue, 29 Oct 2024 11:08:56 +0100 (CET)","by mail-lj1-x22c.google.com with SMTP id\n\t38308e7fff4ca-2fb3debdc09so40611121fa.3\n\tfor ;\n\tTue, 29 Oct 2024 03:08:56 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"UKHyDJp6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1730196535; x=1730801335;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=muE8D9HE+m8XdhGoIfTVy6DlX87whntWexZ6Zr6sB5Y=;\n\tb=UKHyDJp6RGRSQ0OlSHyOZuiFAttmxp6LEZZANG4xepu+Halg2IqkanYH5SJRjX7lnS\n\t+vAQQBXAgGqnJ/b18k31BMG+JTsrvFv7ibB8t0UToAnqHAFkejBNN62WDQm1dgwunCTo\n\tWq6B9HBnwsLv/HMTAGhwEn0DBfuxNXx5Gl+S4=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1730196535; x=1730801335;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=muE8D9HE+m8XdhGoIfTVy6DlX87whntWexZ6Zr6sB5Y=;\n\tb=ksJAXzkTPa1DKJkVLF+2nSMhsBTgBF3pqJfp9gWzvjjUcvMZE6Vkf/skQXoA5m1Qvy\n\tyU3bFgkP3LZ27NtwxGIO9atHfiEVxbOQTd8K8bLZ5limPtRH1vQGZH0jmR59vvCN5Glc\n\tLRnYrwkx6ZTKFqRgQhEs4kzBdvXfnmMTzyLMAdamkQv2o3YNp/EamiGSsEhodg+/L0VG\n\tmB5noZDRo11ChtrGBBjy3YOAT/9/Pu4Cn3ctClxhNK0UOtGOrS4zzzVOCILLa5jaqpH+\n\tvszlfGah8M9RyOnajZaR/yHs5qpbH8i6zIL6twHkA2fCISTT8PmtllwW0pidHpOHXHjX\n\tJ22A==","X-Gm-Message-State":"AOJu0YxBAlLGHPt6QNkclKZ142BE9/BnWRxtJ8kSz68+mK6cA1lIBz0g\n\t/7THd6GBOhEKGGO0LoFJvXvYrPDnuk+02ygQQ0j9J9iyuLIRajSEjFgP0/RUiIlSqGu0buG58xd\n\td7YOVOrESw66NtfSpCTY1ZNYSumbT8WvXdFe+acqiab5i7YeZeA==","X-Google-Smtp-Source":"AGHT+IF4bMY/8nhpND0VZ59Ct5gWQj+qR5Zz/5tx0ghxN2LUowgBvyaVZMrYGeOFf6XxlRpHyBD/fC2XLpgTuyyypQA=","X-Received":"by 2002:a05:651c:505:b0:2f6:6074:db71 with SMTP id\n\t38308e7fff4ca-2fcbdfc67cdmr51920091fa.17.1730196535238;\n\tTue, 29 Oct 2024 03:08:55 -0700 (PDT)","MIME-Version":"1.0","References":"<20241025153948.1213629-1-chenghaoyang@chromium.org>\n\t<20241025153948.1213629-2-chenghaoyang@chromium.org>\n\t<3u4djitcrcv62qodtapynuzw6xtjdikmnanyrsnit53mcthnwt@yvy2sonegonl>","In-Reply-To":"<3u4djitcrcv62qodtapynuzw6xtjdikmnanyrsnit53mcthnwt@yvy2sonegonl>","From":"Cheng-Hao Yang ","Date":"Tue, 29 Oct 2024 18:08:44 +0800","Message-ID":"","Subject":"Re: [PATCH v4 1/2] libcamera: Extend u32 control type","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org, \n\tYudhistira Erlandinata ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31958,"web_url":"https://patchwork.libcamera.org/comment/31958/","msgid":"","date":"2024-10-29T10:33:13","subject":"Re: [PATCH v4 1/2] libcamera: Extend u32 control type","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"On Tue, Oct 29, 2024 at 06:08:44PM +0800, Cheng-Hao Yang wrote:\n> Hi Jacopo,\n>\n> On Mon, Oct 28, 2024 at 6:36 PM Jacopo Mondi\n> wrote:\n> >\n> > Hi Harvey\n> >\n> > On Fri, Oct 25, 2024 at 03:38:50PM +0000, Harvey Yang wrote:\n> > > From: Yudhistira Erlandinata \n> > >\n> > > V4L2 Controls support a wide variety of types not yet supported by the\n> > > ControlValue type system.\n> > >\n> > > Extend the libcamera ControlValue types to support an explicit 32 bit\n> > > unsigned integer type, and map that to the corresponding\n> > > V4L2_CTRL_TYPE_U32 type within the v4l2_device support class.\n> > >\n> > > Signed-off-by: Yudhistira Erlandinata \n> > > Co-developed-by: Harvey Yang \n> > > Signed-off-by: Harvey Yang \n> > > ---\n> > > include/libcamera/controls.h | 7 ++++++\n> > > src/libcamera/controls.cpp | 12 ++++++++--\n> > > src/libcamera/v4l2_device.cpp | 14 ++++++++++++\n> > > test/controls/control_value.cpp | 40 +++++++++++++++++++++++++++++++++\n> > > 4 files changed, 71 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > > index ca60bbaca..6da8ad2c3 100644\n> > > --- a/include/libcamera/controls.h\n> > > +++ b/include/libcamera/controls.h\n> > > @@ -29,6 +29,7 @@ enum ControlType {\n> > > ControlTypeNone,\n> > > ControlTypeBool,\n> > > ControlTypeByte,\n> > > + ControlTypeUnsigned32,\n> > > ControlTypeInteger32,\n> > > ControlTypeInteger64,\n> > > ControlTypeFloat,\n> > > @@ -62,6 +63,12 @@ struct control_type {\n> > > static constexpr std::size_t size = 0;\n> > > };\n> > >\n> > > +template<>\n> > > +struct control_type {\n> > > + static constexpr ControlType value = ControlTypeUnsigned32;\n> > > + static constexpr std::size_t size = 0;\n> > > +};\n> > > +\n> > > template<>\n> > > struct control_type {\n> > > static constexpr ControlType value = ControlTypeInteger32;\n> > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > > index 62185d643..8ae295191 100644\n> > > --- a/src/libcamera/controls.cpp\n> > > +++ b/src/libcamera/controls.cpp\n> > > @@ -54,6 +54,7 @@ static constexpr size_t ControlValueSize[] = {\n> > > [ControlTypeNone] = 0,\n> > > [ControlTypeBool] = sizeof(bool),\n> > > [ControlTypeByte] = sizeof(uint8_t),\n> > > + [ControlTypeUnsigned32] = sizeof(uint32_t),\n> > > [ControlTypeInteger32] = sizeof(int32_t),\n> > > [ControlTypeInteger64] = sizeof(int64_t),\n> > > [ControlTypeFloat] = sizeof(float),\n> > > @@ -74,10 +75,12 @@ static constexpr size_t ControlValueSize[] = {\n> > > * The control stores a boolean value\n> > > * \\var ControlTypeByte\n> > > * The control stores a byte value as an unsigned 8-bit integer\n> > > + * \\var ControlTypeUnsigned32\n> > > + * The control stores an unsigned 32-bit integer value\n> > > * \\var ControlTypeInteger32\n> > > - * The control stores a 32-bit integer value\n> > > + * The control stores a signed 32-bit integer value\n> > > * \\var ControlTypeInteger64\n> > > - * The control stores a 64-bit integer value\n> > > + * The control stores a signed 64-bit integer value\n> > > * \\var ControlTypeFloat\n> > > * The control stores a 32-bit floating point value\n> > > * \\var ControlTypeString\n> > > @@ -230,6 +233,11 @@ std::string ControlValue::toString() const\n> > > str += std::to_string(*value);\n> > > break;\n> > > }\n> > > + case ControlTypeUnsigned32: {\n> > > + const uint32_t *value = reinterpret_cast(data);\n> > > + str += std::to_string(*value);\n> > > + break;\n> > > + }\n> > > case ControlTypeInteger32: {\n> > > const int32_t *value = reinterpret_cast(data);\n> > > str += std::to_string(*value);\n> > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > > index 68add4f2e..f39ec3760 100644\n> > > --- a/src/libcamera/v4l2_device.cpp\n> > > +++ b/src/libcamera/v4l2_device.cpp\n> > > @@ -9,6 +9,7 @@\n> > >\n> > > #include \n> > > #include \n> > > +#include \n> > > #include \n> > > #include \n> > > #include \n> > > @@ -210,6 +211,10 @@ ControlList V4L2Device::getControls(const std::vector &ids)\n> > > type = ControlTypeByte;\n> > > break;\n> > >\n> > > + case V4L2_CTRL_TYPE_U32:\n> > > + type = ControlTypeUnsigned32;\n> > > + break;\n> > > +\n> >\n> > This is not enough.\n> >\n> > After the switch we have\n> >\n> > ControlValue &value = ctrl.second;\n> > value.reserve(type, true, info.elems);\n> > Span data = value.data();\n> >\n> > v4l2Ctrl.p_u8 = data.data();\n> > v4l2Ctrl.size = data.size();\n> >\n> > However controls of type V4L2_CTRL_TYPE_U32 use the p_u32 union member\n> > of 'struct v4l2_ext_control'\n> > https://www.kernel.org/doc/html/v4.9/media/uapi/v4l/vidioc-g-ext-ctrls.html#id1\n> >\n> > Same for U16 control types.\n> >\n> > Be careful about one thing. v4l2Ctrl.p_u32 is of type __u32* so it\n> > would feel natural to\n> >\n> > Span data = value.data();\n> > v4l2Ctrl.p_u32 = data.data();\n> >\n> > as data.data() returns a T*\n> >\n> > However v4l2Ctrl.size wants the size in bytes while data.size()\n> > returns the number of elements, so you should probably use\n> > data.size_bytes().\n> >\n> > Or you can do as we do in setControls and cast the data.data()\n> > pointer.\n> >\n> > Span data = value.data();\n> > v4l2Ctrl.p_u32 = reinterpret_cast(data.data());\n> > v4l2Ctrl.size = data.size();\n> >\n> > Also, you need to handle U32 and U16 also setControl().\n>\n> Thanks for the catch!\n> Please help take a look at my WIP patch:\n> https://gitlab.freedesktop.org/chenghaoyang/libcamera/-/commit/faec47f605881062e58718371183a0c2d3ffe772\n> , before I upload another version.\n> Let me know if you think I should merge the two `switch`es and duplicate\n> the code in the middle.\n\n\nYou would have to copy\n\n\t\t\tvalue.reserve(type, true, info.elems);\n\t\t\tSpan data = value.data();\n\nIn the two switch branches but you can keep\n\n v4l2Ctrl.size = data.size();\n\nAfter the switch.\n\nUp to you, I would prefer a single switch but I don't mind too much\n\n>\n> >\n> > If you can test this by reading and writing u16 and u32 controls this\n> > would make me way more confident this patch is correct.\n>\n> Hmm, I couldn't find such a use case. Can you suggest how we could\n> test it?\n\nWell, I presume you have a device with a u16/u32 control if you sent\nthis :) Just calling getControls(), trying to set them to some value\nand reading them back would validate the implementation. Or have I\nmissed your question ?\n\nThanks\n j\n\n>\n> BR,\n> Harvey\n>\n> >\n> > Thanks\n> > j\n> >\n> > > default:\n> > > LOG(V4L2, Error)\n> > > << \"Unsupported payload control type \"\n> > > @@ -488,6 +493,9 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)\n> > > case V4L2_CTRL_TYPE_BOOLEAN:\n> > > return ControlTypeBool;\n> > >\n> > > + case V4L2_CTRL_TYPE_U32:\n> > > + return ControlTypeUnsigned32;\n> > > +\n> > > case V4L2_CTRL_TYPE_INTEGER:\n> > > return ControlTypeInteger32;\n> > >\n> > > @@ -536,6 +544,11 @@ std::optional V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl\n> > > static_cast(ctrl.maximum),\n> > > static_cast(ctrl.default_value));\n> > >\n> > > + case V4L2_CTRL_TYPE_U32:\n> > > + return ControlInfo(static_cast(ctrl.minimum),\n> > > + static_cast(ctrl.maximum),\n> > > + static_cast(ctrl.default_value));\n> > > +\n> > > case V4L2_CTRL_TYPE_BOOLEAN:\n> > > return ControlInfo(static_cast(ctrl.minimum),\n> > > static_cast(ctrl.maximum),\n> > > @@ -622,6 +635,7 @@ void V4L2Device::listControls()\n> > > case V4L2_CTRL_TYPE_BITMASK:\n> > > case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > > case V4L2_CTRL_TYPE_U8:\n> > > + case V4L2_CTRL_TYPE_U32:\n> > > break;\n> > > /* \\todo Support other control types. */\n> > > default:\n> > > diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n> > > index 344107fae..6ca85b739 100644\n> > > --- a/test/controls/control_value.cpp\n> > > +++ b/test/controls/control_value.cpp\n> > > @@ -109,6 +109,46 @@ protected:\n> > > return TestFail;\n> > > }\n> > >\n> > > + /*\n> > > + * Unsigned Integer32 type.\n> > > + */\n> > > + value.set(static_cast(42));\n> > > + if (value.isNone() || value.isArray() ||\n> > > + value.type() != ControlTypeUnsigned32) {\n> > > + cerr << \"Control type mismatch after setting to uint32_t\" << endl;\n> > > + return TestFail;\n> > > + }\n> > > +\n> > > + if (value.get() != 42) {\n> > > + cerr << \"Control value mismatch after setting to uint32_t\" << endl;\n> > > + return TestFail;\n> > > + }\n> > > +\n> > > + if (value.toString() != \"42\") {\n> > > + cerr << \"Control string mismatch after setting to uint32_t\" << endl;\n> > > + return TestFail;\n> > > + }\n> > > +\n> > > + std::array uint32s{ 3, 14, 15, 9 };\n> > > + value.set(Span(uint32s));\n> > > + if (value.isNone() || !value.isArray() ||\n> > > + value.type() != ControlTypeUnsigned32) {\n> > > + cerr << \"Control type mismatch after setting to uint32_t array\" << endl;\n> > > + return TestFail;\n> > > + }\n> > > +\n> > > + Span uint32sResult = value.get>();\n> > > + if (uint32s.size() != uint32sResult.size() ||\n> > > + !std::equal(uint32s.begin(), uint32s.end(), uint32sResult.begin())) {\n> > > + cerr << \"Control value mismatch after setting to uint32_t array\" << endl;\n> > > + return TestFail;\n> > > + }\n> > > +\n> > > + if (value.toString() != \"[ 3, 14, 15, 9 ]\") {\n> > > + cerr << \"Control string mismatch after setting to uint32_t array\" << endl;\n> > > + return TestFail;\n> > > + }\n> > > +\n> > > /*\n> > > * Integer32 type.\n> > > */\n> > > --\n> > > 2.47.0.163.g1226f6d8fa-goog\n> > >","headers":{"Return-Path":"","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 57BF7C31E9\n\tfor ;\n\tTue, 29 Oct 2024 10:33:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7707C65398;\n\tTue, 29 Oct 2024 11:33:20 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0168160366\n\tfor ;\n\tTue, 29 Oct 2024 11:33:17 +0100 (CET)","from ideasonboard.com (mob-5-90-50-182.net.vodafone.it\n\t[5.90.50.182])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 72C974D4;\n\tTue, 29 Oct 2024 11:33: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=\"FqVQviCn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730197995;\n\tbh=TKe36uD994jaWlY+k6LlgCFJ/wpL4H27GEvMZGXjwrE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FqVQviCn7xAS+wP7b50vrfeFylQVDTqVbWjfZpvxawRYKhl/m9v/DyM5HgMk2/F2l\n\taLP3p0giEv6qstXI86XCyzZEO7OScM7JbuuzswCeZg6Mo/0w1FqV/a/mggYA6qt2H6\n\tWiA2ygckHbsn754e+5l2ioOzm33fAp8Atvd6ZNx0=","Date":"Tue, 29 Oct 2024 11:33:13 +0100","From":"Jacopo Mondi ","To":"Cheng-Hao Yang ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org,\n\tYudhistira Erlandinata ","Subject":"Re: [PATCH v4 1/2] libcamera: Extend u32 control type","Message-ID":"","References":"<20241025153948.1213629-1-chenghaoyang@chromium.org>\n\t<20241025153948.1213629-2-chenghaoyang@chromium.org>\n\t<3u4djitcrcv62qodtapynuzw6xtjdikmnanyrsnit53mcthnwt@yvy2sonegonl>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31962,"web_url":"https://patchwork.libcamera.org/comment/31962/","msgid":"","date":"2024-10-29T16:11:12","subject":"Re: [PATCH v4 1/2] libcamera: Extend u32 control type","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Jacopo,\n\nOn Tue, Oct 29, 2024 at 6:33 PM Jacopo Mondi\n wrote:\n>\n>\n> On Tue, Oct 29, 2024 at 06:08:44PM +0800, Cheng-Hao Yang wrote:\n> > Hi Jacopo,\n> >\n> > On Mon, Oct 28, 2024 at 6:36 PM Jacopo Mondi\n> > wrote:\n> > >\n> > > Hi Harvey\n> > >\n> > > On Fri, Oct 25, 2024 at 03:38:50PM +0000, Harvey Yang wrote:\n> > > > From: Yudhistira Erlandinata \n> > > >\n> > > > V4L2 Controls support a wide variety of types not yet supported by the\n> > > > ControlValue type system.\n> > > >\n> > > > Extend the libcamera ControlValue types to support an explicit 32 bit\n> > > > unsigned integer type, and map that to the corresponding\n> > > > V4L2_CTRL_TYPE_U32 type within the v4l2_device support class.\n> > > >\n> > > > Signed-off-by: Yudhistira Erlandinata \n> > > > Co-developed-by: Harvey Yang \n> > > > Signed-off-by: Harvey Yang \n> > > > ---\n> > > > include/libcamera/controls.h | 7 ++++++\n> > > > src/libcamera/controls.cpp | 12 ++++++++--\n> > > > src/libcamera/v4l2_device.cpp | 14 ++++++++++++\n> > > > test/controls/control_value.cpp | 40 +++++++++++++++++++++++++++++++++\n> > > > 4 files changed, 71 insertions(+), 2 deletions(-)\n> > > >\n> > > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > > > index ca60bbaca..6da8ad2c3 100644\n> > > > --- a/include/libcamera/controls.h\n> > > > +++ b/include/libcamera/controls.h\n> > > > @@ -29,6 +29,7 @@ enum ControlType {\n> > > > ControlTypeNone,\n> > > > ControlTypeBool,\n> > > > ControlTypeByte,\n> > > > + ControlTypeUnsigned32,\n> > > > ControlTypeInteger32,\n> > > > ControlTypeInteger64,\n> > > > ControlTypeFloat,\n> > > > @@ -62,6 +63,12 @@ struct control_type {\n> > > > static constexpr std::size_t size = 0;\n> > > > };\n> > > >\n> > > > +template<>\n> > > > +struct control_type {\n> > > > + static constexpr ControlType value = ControlTypeUnsigned32;\n> > > > + static constexpr std::size_t size = 0;\n> > > > +};\n> > > > +\n> > > > template<>\n> > > > struct control_type {\n> > > > static constexpr ControlType value = ControlTypeInteger32;\n> > > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > > > index 62185d643..8ae295191 100644\n> > > > --- a/src/libcamera/controls.cpp\n> > > > +++ b/src/libcamera/controls.cpp\n> > > > @@ -54,6 +54,7 @@ static constexpr size_t ControlValueSize[] = {\n> > > > [ControlTypeNone] = 0,\n> > > > [ControlTypeBool] = sizeof(bool),\n> > > > [ControlTypeByte] = sizeof(uint8_t),\n> > > > + [ControlTypeUnsigned32] = sizeof(uint32_t),\n> > > > [ControlTypeInteger32] = sizeof(int32_t),\n> > > > [ControlTypeInteger64] = sizeof(int64_t),\n> > > > [ControlTypeFloat] = sizeof(float),\n> > > > @@ -74,10 +75,12 @@ static constexpr size_t ControlValueSize[] = {\n> > > > * The control stores a boolean value\n> > > > * \\var ControlTypeByte\n> > > > * The control stores a byte value as an unsigned 8-bit integer\n> > > > + * \\var ControlTypeUnsigned32\n> > > > + * The control stores an unsigned 32-bit integer value\n> > > > * \\var ControlTypeInteger32\n> > > > - * The control stores a 32-bit integer value\n> > > > + * The control stores a signed 32-bit integer value\n> > > > * \\var ControlTypeInteger64\n> > > > - * The control stores a 64-bit integer value\n> > > > + * The control stores a signed 64-bit integer value\n> > > > * \\var ControlTypeFloat\n> > > > * The control stores a 32-bit floating point value\n> > > > * \\var ControlTypeString\n> > > > @@ -230,6 +233,11 @@ std::string ControlValue::toString() const\n> > > > str += std::to_string(*value);\n> > > > break;\n> > > > }\n> > > > + case ControlTypeUnsigned32: {\n> > > > + const uint32_t *value = reinterpret_cast(data);\n> > > > + str += std::to_string(*value);\n> > > > + break;\n> > > > + }\n> > > > case ControlTypeInteger32: {\n> > > > const int32_t *value = reinterpret_cast(data);\n> > > > str += std::to_string(*value);\n> > > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > > > index 68add4f2e..f39ec3760 100644\n> > > > --- a/src/libcamera/v4l2_device.cpp\n> > > > +++ b/src/libcamera/v4l2_device.cpp\n> > > > @@ -9,6 +9,7 @@\n> > > >\n> > > > #include \n> > > > #include \n> > > > +#include \n> > > > #include \n> > > > #include \n> > > > #include \n> > > > @@ -210,6 +211,10 @@ ControlList V4L2Device::getControls(const std::vector &ids)\n> > > > type = ControlTypeByte;\n> > > > break;\n> > > >\n> > > > + case V4L2_CTRL_TYPE_U32:\n> > > > + type = ControlTypeUnsigned32;\n> > > > + break;\n> > > > +\n> > >\n> > > This is not enough.\n> > >\n> > > After the switch we have\n> > >\n> > > ControlValue &value = ctrl.second;\n> > > value.reserve(type, true, info.elems);\n> > > Span data = value.data();\n> > >\n> > > v4l2Ctrl.p_u8 = data.data();\n> > > v4l2Ctrl.size = data.size();\n> > >\n> > > However controls of type V4L2_CTRL_TYPE_U32 use the p_u32 union member\n> > > of 'struct v4l2_ext_control'\n> > > https://www.kernel.org/doc/html/v4.9/media/uapi/v4l/vidioc-g-ext-ctrls.html#id1\n> > >\n> > > Same for U16 control types.\n> > >\n> > > Be careful about one thing. v4l2Ctrl.p_u32 is of type __u32* so it\n> > > would feel natural to\n> > >\n> > > Span data = value.data();\n> > > v4l2Ctrl.p_u32 = data.data();\n> > >\n> > > as data.data() returns a T*\n> > >\n> > > However v4l2Ctrl.size wants the size in bytes while data.size()\n> > > returns the number of elements, so you should probably use\n> > > data.size_bytes().\n> > >\n> > > Or you can do as we do in setControls and cast the data.data()\n> > > pointer.\n> > >\n> > > Span data = value.data();\n> > > v4l2Ctrl.p_u32 = reinterpret_cast(data.data());\n> > > v4l2Ctrl.size = data.size();\n> > >\n> > > Also, you need to handle U32 and U16 also setControl().\n> >\n> > Thanks for the catch!\n> > Please help take a look at my WIP patch:\n> > https://gitlab.freedesktop.org/chenghaoyang/libcamera/-/commit/faec47f605881062e58718371183a0c2d3ffe772\n> > , before I upload another version.\n> > Let me know if you think I should merge the two `switch`es and duplicate\n> > the code in the middle.\n>\n>\n> You would have to copy\n>\n> value.reserve(type, true, info.elems);\n> Span data = value.data();\n>\n> In the two switch branches but you can keep\n>\n> v4l2Ctrl.size = data.size();\n>\n> After the switch.\n\nDone.\n\n>\n> Up to you, I would prefer a single switch but I don't mind too much\n>\n> >\n> > >\n> > > If you can test this by reading and writing u16 and u32 controls this\n> > > would make me way more confident this patch is correct.\n> >\n> > Hmm, I couldn't find such a use case. Can you suggest how we could\n> > test it?\n>\n> Well, I presume you have a device with a u16/u32 control if you sent\n> this :) Just calling getControls(), trying to set them to some value\n> and reading them back would validate the implementation. Or have I\n> missed your question ?\n\nActually, the two new types are not being used in setControls() / getControls().\nu32, for example, is only used for listControls with VIDIOC_QUERY_EXT_CTRL [1],\nand V4L2Device needs to recognize u32.\nThat's also why I didn't have the changes in setControls() / getControls().\n\nBR,\nHarvey\n\n[1]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/mtkisp7/src/libcamera/v4l2_device.cpp;l=681\n\n>\n> Thanks\n> j\n>\n> >\n> > BR,\n> > Harvey\n> >\n> > >\n> > > Thanks\n> > > j\n> > >\n> > > > default:\n> > > > LOG(V4L2, Error)\n> > > > << \"Unsupported payload control type \"\n> > > > @@ -488,6 +493,9 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)\n> > > > case V4L2_CTRL_TYPE_BOOLEAN:\n> > > > return ControlTypeBool;\n> > > >\n> > > > + case V4L2_CTRL_TYPE_U32:\n> > > > + return ControlTypeUnsigned32;\n> > > > +\n> > > > case V4L2_CTRL_TYPE_INTEGER:\n> > > > return ControlTypeInteger32;\n> > > >\n> > > > @@ -536,6 +544,11 @@ std::optional V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl\n> > > > static_cast(ctrl.maximum),\n> > > > static_cast(ctrl.default_value));\n> > > >\n> > > > + case V4L2_CTRL_TYPE_U32:\n> > > > + return ControlInfo(static_cast(ctrl.minimum),\n> > > > + static_cast(ctrl.maximum),\n> > > > + static_cast(ctrl.default_value));\n> > > > +\n> > > > case V4L2_CTRL_TYPE_BOOLEAN:\n> > > > return ControlInfo(static_cast(ctrl.minimum),\n> > > > static_cast(ctrl.maximum),\n> > > > @@ -622,6 +635,7 @@ void V4L2Device::listControls()\n> > > > case V4L2_CTRL_TYPE_BITMASK:\n> > > > case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > > > case V4L2_CTRL_TYPE_U8:\n> > > > + case V4L2_CTRL_TYPE_U32:\n> > > > break;\n> > > > /* \\todo Support other control types. */\n> > > > default:\n> > > > diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n> > > > index 344107fae..6ca85b739 100644\n> > > > --- a/test/controls/control_value.cpp\n> > > > +++ b/test/controls/control_value.cpp\n> > > > @@ -109,6 +109,46 @@ protected:\n> > > > return TestFail;\n> > > > }\n> > > >\n> > > > + /*\n> > > > + * Unsigned Integer32 type.\n> > > > + */\n> > > > + value.set(static_cast(42));\n> > > > + if (value.isNone() || value.isArray() ||\n> > > > + value.type() != ControlTypeUnsigned32) {\n> > > > + cerr << \"Control type mismatch after setting to uint32_t\" << endl;\n> > > > + return TestFail;\n> > > > + }\n> > > > +\n> > > > + if (value.get() != 42) {\n> > > > + cerr << \"Control value mismatch after setting to uint32_t\" << endl;\n> > > > + return TestFail;\n> > > > + }\n> > > > +\n> > > > + if (value.toString() != \"42\") {\n> > > > + cerr << \"Control string mismatch after setting to uint32_t\" << endl;\n> > > > + return TestFail;\n> > > > + }\n> > > > +\n> > > > + std::array uint32s{ 3, 14, 15, 9 };\n> > > > + value.set(Span(uint32s));\n> > > > + if (value.isNone() || !value.isArray() ||\n> > > > + value.type() != ControlTypeUnsigned32) {\n> > > > + cerr << \"Control type mismatch after setting to uint32_t array\" << endl;\n> > > > + return TestFail;\n> > > > + }\n> > > > +\n> > > > + Span uint32sResult = value.get>();\n> > > > + if (uint32s.size() != uint32sResult.size() ||\n> > > > + !std::equal(uint32s.begin(), uint32s.end(), uint32sResult.begin())) {\n> > > > + cerr << \"Control value mismatch after setting to uint32_t array\" << endl;\n> > > > + return TestFail;\n> > > > + }\n> > > > +\n> > > > + if (value.toString() != \"[ 3, 14, 15, 9 ]\") {\n> > > > + cerr << \"Control string mismatch after setting to uint32_t array\" << endl;\n> > > > + return TestFail;\n> > > > + }\n> > > > +\n> > > > /*\n> > > > * Integer32 type.\n> > > > */\n> > > > --\n> > > > 2.47.0.163.g1226f6d8fa-goog\n> > > >","headers":{"Return-Path":"","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 1FF6DC31E9\n\tfor ;\n\tTue, 29 Oct 2024 16:11:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 106E06539F;\n\tTue, 29 Oct 2024 17:11:27 +0100 (CET)","from mail-lj1-x229.google.com (mail-lj1-x229.google.com\n\t[IPv6:2a00:1450:4864:20::229])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5E8F760366\n\tfor ;\n\tTue, 29 Oct 2024 17:11:24 +0100 (CET)","by mail-lj1-x229.google.com with SMTP id\n\t38308e7fff4ca-2fb3debdc09so44196901fa.3\n\tfor ;\n\tTue, 29 Oct 2024 09:11:24 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"KBNcl4gg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1730218283; x=1730823083;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=zubuD4g5akPasJ8eeYryPCVnq9tNQOnHSiTNCaxb0Ag=;\n\tb=KBNcl4ggUNu18PiQw+UtVI/fUCnjd9OtG62lH6qJg6y7+dhf+gRs2f7k1wxu2zyMzs\n\tTXZfBENqoWkJYjjsA0DI1/xJ9uun+agJIY8Ib9+3WT/DgjNWhOtZZQ5t+vW864RxkHSn\n\tiuQ35R1w8lgqRZX04m0jDyKGwslBlk7uHkOic=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1730218283; x=1730823083;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=zubuD4g5akPasJ8eeYryPCVnq9tNQOnHSiTNCaxb0Ag=;\n\tb=q/PWdmYeJYvQMzfAhRQlOOzWziKeggwwqikfmJtClq0hQrNce1ru0kEzXQ8FHDRylt\n\tQYCKmThs5w9ri5+FQYRVERbffuQ4dZ2D6u8HLIElRc8GuyJiQ6E9Lw2u1Js3GfPNVkeX\n\tpwaQ2laudRiaTZ9CdU8+0+HIFTtAZ4JCU6XQeZ5O3HRle6JdfxqQCAtwAhWI05f6TAgM\n\tSvHcOw3jmfZHe7NtR+DE5PiDvkXj6P1yQHnEc8Ths8/fB0ayEMshmBY4YwKLIfn7I54k\n\tOc+bCnVE6yXC2TnKRn7ZDQAij9DFmV4Se8S0KccU7fL/6jJ8WaKElVqhdk5wp8Q6nsok\n\tgbRw==","X-Gm-Message-State":"AOJu0YyAa10Mhspjc6QYjLDVd3XwRl8xRuz92afTrMX19I+vht3KJM2D\n\tIrVIbvI4G88rkxJONOXP3MNZe9sVmrg7gRnKbzWDfSJemti3KsoZ6KYKQL/MWfBkeNHA9QJtwmI\n\tovMJmXqvWI+UJB4CsJu/9eCvec+nkZ21Lz76e","X-Google-Smtp-Source":"AGHT+IH0+rPe4ZEJ7dVJgqgf2CSUJS1EJ0xdbEkr23KQUfGx2T2ooeJ6W0iIOI+TXaVi+UYhs/YLoywIRTwua/8NC70=","X-Received":"by 2002:a2e:619:0:b0:2fb:6027:7bfd with SMTP id\n\t38308e7fff4ca-2fcbdffb3efmr44961961fa.27.1730218283380;\n\tTue, 29 Oct 2024 09:11:23 -0700 (PDT)","MIME-Version":"1.0","References":"<20241025153948.1213629-1-chenghaoyang@chromium.org>\n\t<20241025153948.1213629-2-chenghaoyang@chromium.org>\n\t<3u4djitcrcv62qodtapynuzw6xtjdikmnanyrsnit53mcthnwt@yvy2sonegonl>\n\t\n\t","In-Reply-To":"","From":"Cheng-Hao Yang ","Date":"Wed, 30 Oct 2024 00:11:12 +0800","Message-ID":"","Subject":"Re: [PATCH v4 1/2] libcamera: Extend u32 control type","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org, \n\tYudhistira Erlandinata ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31963,"web_url":"https://patchwork.libcamera.org/comment/31963/","msgid":"<20241029161426.GQ22600@pendragon.ideasonboard.com>","date":"2024-10-29T16:14:26","subject":"Re: [PATCH v4 1/2] libcamera: Extend u32 control type","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Harvey,\n\nOn Wed, Oct 30, 2024 at 12:11:12AM +0800, Cheng-Hao Yang wrote:\n> On Tue, Oct 29, 2024 at 6:33 PM Jacopo Mondi wrote:\n> > On Tue, Oct 29, 2024 at 06:08:44PM +0800, Cheng-Hao Yang wrote:\n> > > On Mon, Oct 28, 2024 at 6:36 PM Jacopo Mondi wrote:\n> > > > On Fri, Oct 25, 2024 at 03:38:50PM +0000, Harvey Yang wrote:\n> > > > > From: Yudhistira Erlandinata \n> > > > >\n> > > > > V4L2 Controls support a wide variety of types not yet supported by the\n> > > > > ControlValue type system.\n> > > > >\n> > > > > Extend the libcamera ControlValue types to support an explicit 32 bit\n> > > > > unsigned integer type, and map that to the corresponding\n> > > > > V4L2_CTRL_TYPE_U32 type within the v4l2_device support class.\n> > > > >\n> > > > > Signed-off-by: Yudhistira Erlandinata \n> > > > > Co-developed-by: Harvey Yang \n> > > > > Signed-off-by: Harvey Yang \n> > > > > ---\n> > > > > include/libcamera/controls.h | 7 ++++++\n> > > > > src/libcamera/controls.cpp | 12 ++++++++--\n> > > > > src/libcamera/v4l2_device.cpp | 14 ++++++++++++\n> > > > > test/controls/control_value.cpp | 40 +++++++++++++++++++++++++++++++++\n> > > > > 4 files changed, 71 insertions(+), 2 deletions(-)\n> > > > >\n> > > > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > > > > index ca60bbaca..6da8ad2c3 100644\n> > > > > --- a/include/libcamera/controls.h\n> > > > > +++ b/include/libcamera/controls.h\n> > > > > @@ -29,6 +29,7 @@ enum ControlType {\n> > > > > ControlTypeNone,\n> > > > > ControlTypeBool,\n> > > > > ControlTypeByte,\n> > > > > + ControlTypeUnsigned32,\n> > > > > ControlTypeInteger32,\n> > > > > ControlTypeInteger64,\n> > > > > ControlTypeFloat,\n> > > > > @@ -62,6 +63,12 @@ struct control_type {\n> > > > > static constexpr std::size_t size = 0;\n> > > > > };\n> > > > >\n> > > > > +template<>\n> > > > > +struct control_type {\n> > > > > + static constexpr ControlType value = ControlTypeUnsigned32;\n> > > > > + static constexpr std::size_t size = 0;\n> > > > > +};\n> > > > > +\n> > > > > template<>\n> > > > > struct control_type {\n> > > > > static constexpr ControlType value = ControlTypeInteger32;\n> > > > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > > > > index 62185d643..8ae295191 100644\n> > > > > --- a/src/libcamera/controls.cpp\n> > > > > +++ b/src/libcamera/controls.cpp\n> > > > > @@ -54,6 +54,7 @@ static constexpr size_t ControlValueSize[] = {\n> > > > > [ControlTypeNone] = 0,\n> > > > > [ControlTypeBool] = sizeof(bool),\n> > > > > [ControlTypeByte] = sizeof(uint8_t),\n> > > > > + [ControlTypeUnsigned32] = sizeof(uint32_t),\n> > > > > [ControlTypeInteger32] = sizeof(int32_t),\n> > > > > [ControlTypeInteger64] = sizeof(int64_t),\n> > > > > [ControlTypeFloat] = sizeof(float),\n> > > > > @@ -74,10 +75,12 @@ static constexpr size_t ControlValueSize[] = {\n> > > > > * The control stores a boolean value\n> > > > > * \\var ControlTypeByte\n> > > > > * The control stores a byte value as an unsigned 8-bit integer\n> > > > > + * \\var ControlTypeUnsigned32\n> > > > > + * The control stores an unsigned 32-bit integer value\n> > > > > * \\var ControlTypeInteger32\n> > > > > - * The control stores a 32-bit integer value\n> > > > > + * The control stores a signed 32-bit integer value\n> > > > > * \\var ControlTypeInteger64\n> > > > > - * The control stores a 64-bit integer value\n> > > > > + * The control stores a signed 64-bit integer value\n> > > > > * \\var ControlTypeFloat\n> > > > > * The control stores a 32-bit floating point value\n> > > > > * \\var ControlTypeString\n> > > > > @@ -230,6 +233,11 @@ std::string ControlValue::toString() const\n> > > > > str += std::to_string(*value);\n> > > > > break;\n> > > > > }\n> > > > > + case ControlTypeUnsigned32: {\n> > > > > + const uint32_t *value = reinterpret_cast(data);\n> > > > > + str += std::to_string(*value);\n> > > > > + break;\n> > > > > + }\n> > > > > case ControlTypeInteger32: {\n> > > > > const int32_t *value = reinterpret_cast(data);\n> > > > > str += std::to_string(*value);\n> > > > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > > > > index 68add4f2e..f39ec3760 100644\n> > > > > --- a/src/libcamera/v4l2_device.cpp\n> > > > > +++ b/src/libcamera/v4l2_device.cpp\n> > > > > @@ -9,6 +9,7 @@\n> > > > >\n> > > > > #include \n> > > > > #include \n> > > > > +#include \n> > > > > #include \n> > > > > #include \n> > > > > #include \n> > > > > @@ -210,6 +211,10 @@ ControlList V4L2Device::getControls(const std::vector &ids)\n> > > > > type = ControlTypeByte;\n> > > > > break;\n> > > > >\n> > > > > + case V4L2_CTRL_TYPE_U32:\n> > > > > + type = ControlTypeUnsigned32;\n> > > > > + break;\n> > > > > +\n> > > >\n> > > > This is not enough.\n> > > >\n> > > > After the switch we have\n> > > >\n> > > > ControlValue &value = ctrl.second;\n> > > > value.reserve(type, true, info.elems);\n> > > > Span data = value.data();\n> > > >\n> > > > v4l2Ctrl.p_u8 = data.data();\n> > > > v4l2Ctrl.size = data.size();\n> > > >\n> > > > However controls of type V4L2_CTRL_TYPE_U32 use the p_u32 union member\n> > > > of 'struct v4l2_ext_control'\n> > > > https://www.kernel.org/doc/html/v4.9/media/uapi/v4l/vidioc-g-ext-ctrls.html#id1\n> > > >\n> > > > Same for U16 control types.\n> > > >\n> > > > Be careful about one thing. v4l2Ctrl.p_u32 is of type __u32* so it\n> > > > would feel natural to\n> > > >\n> > > > Span data = value.data();\n> > > > v4l2Ctrl.p_u32 = data.data();\n> > > >\n> > > > as data.data() returns a T*\n> > > >\n> > > > However v4l2Ctrl.size wants the size in bytes while data.size()\n> > > > returns the number of elements, so you should probably use\n> > > > data.size_bytes().\n> > > >\n> > > > Or you can do as we do in setControls and cast the data.data()\n> > > > pointer.\n> > > >\n> > > > Span data = value.data();\n> > > > v4l2Ctrl.p_u32 = reinterpret_cast(data.data());\n> > > > v4l2Ctrl.size = data.size();\n> > > >\n> > > > Also, you need to handle U32 and U16 also setControl().\n> > >\n> > > Thanks for the catch!\n> > > Please help take a look at my WIP patch:\n> > > https://gitlab.freedesktop.org/chenghaoyang/libcamera/-/commit/faec47f605881062e58718371183a0c2d3ffe772\n> > > , before I upload another version.\n> > > Let me know if you think I should merge the two `switch`es and duplicate\n> > > the code in the middle.\n> >\n> >\n> > You would have to copy\n> >\n> > value.reserve(type, true, info.elems);\n> > Span data = value.data();\n> >\n> > In the two switch branches but you can keep\n> >\n> > v4l2Ctrl.size = data.size();\n> >\n> > After the switch.\n> \n> Done.\n> \n> > Up to you, I would prefer a single switch but I don't mind too much\n> >\n> > > > If you can test this by reading and writing u16 and u32 controls this\n> > > > would make me way more confident this patch is correct.\n> > >\n> > > Hmm, I couldn't find such a use case. Can you suggest how we could\n> > > test it?\n> >\n> > Well, I presume you have a device with a u16/u32 control if you sent\n> > this :) Just calling getControls(), trying to set them to some value\n> > and reading them back would validate the implementation. Or have I\n> > missed your question ?\n> \n> Actually, the two new types are not being used in setControls() / getControls().\n> u32, for example, is only used for listControls with VIDIOC_QUERY_EXT_CTRL [1],\n> and V4L2Device needs to recognize u32.\n\nBut surely that's used with a device ? Can you point to the driver that\nexposes these controls ?\n\n> That's also why I didn't have the changes in setControls() / getControls().\n> \n> BR,\n> Harvey\n> \n> [1]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/mtkisp7/src/libcamera/v4l2_device.cpp;l=681\n> \n> > > > > default:\n> > > > > LOG(V4L2, Error)\n> > > > > << \"Unsupported payload control type \"\n> > > > > @@ -488,6 +493,9 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)\n> > > > > case V4L2_CTRL_TYPE_BOOLEAN:\n> > > > > return ControlTypeBool;\n> > > > >\n> > > > > + case V4L2_CTRL_TYPE_U32:\n> > > > > + return ControlTypeUnsigned32;\n> > > > > +\n> > > > > case V4L2_CTRL_TYPE_INTEGER:\n> > > > > return ControlTypeInteger32;\n> > > > >\n> > > > > @@ -536,6 +544,11 @@ std::optional V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl\n> > > > > static_cast(ctrl.maximum),\n> > > > > static_cast(ctrl.default_value));\n> > > > >\n> > > > > + case V4L2_CTRL_TYPE_U32:\n> > > > > + return ControlInfo(static_cast(ctrl.minimum),\n> > > > > + static_cast(ctrl.maximum),\n> > > > > + static_cast(ctrl.default_value));\n> > > > > +\n> > > > > case V4L2_CTRL_TYPE_BOOLEAN:\n> > > > > return ControlInfo(static_cast(ctrl.minimum),\n> > > > > static_cast(ctrl.maximum),\n> > > > > @@ -622,6 +635,7 @@ void V4L2Device::listControls()\n> > > > > case V4L2_CTRL_TYPE_BITMASK:\n> > > > > case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > > > > case V4L2_CTRL_TYPE_U8:\n> > > > > + case V4L2_CTRL_TYPE_U32:\n> > > > > break;\n> > > > > /* \\todo Support other control types. */\n> > > > > default:\n> > > > > diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n> > > > > index 344107fae..6ca85b739 100644\n> > > > > --- a/test/controls/control_value.cpp\n> > > > > +++ b/test/controls/control_value.cpp\n> > > > > @@ -109,6 +109,46 @@ protected:\n> > > > > return TestFail;\n> > > > > }\n> > > > >\n> > > > > + /*\n> > > > > + * Unsigned Integer32 type.\n> > > > > + */\n> > > > > + value.set(static_cast(42));\n> > > > > + if (value.isNone() || value.isArray() ||\n> > > > > + value.type() != ControlTypeUnsigned32) {\n> > > > > + cerr << \"Control type mismatch after setting to uint32_t\" << endl;\n> > > > > + return TestFail;\n> > > > > + }\n> > > > > +\n> > > > > + if (value.get() != 42) {\n> > > > > + cerr << \"Control value mismatch after setting to uint32_t\" << endl;\n> > > > > + return TestFail;\n> > > > > + }\n> > > > > +\n> > > > > + if (value.toString() != \"42\") {\n> > > > > + cerr << \"Control string mismatch after setting to uint32_t\" << endl;\n> > > > > + return TestFail;\n> > > > > + }\n> > > > > +\n> > > > > + std::array uint32s{ 3, 14, 15, 9 };\n> > > > > + value.set(Span(uint32s));\n> > > > > + if (value.isNone() || !value.isArray() ||\n> > > > > + value.type() != ControlTypeUnsigned32) {\n> > > > > + cerr << \"Control type mismatch after setting to uint32_t array\" << endl;\n> > > > > + return TestFail;\n> > > > > + }\n> > > > > +\n> > > > > + Span uint32sResult = value.get>();\n> > > > > + if (uint32s.size() != uint32sResult.size() ||\n> > > > > + !std::equal(uint32s.begin(), uint32s.end(), uint32sResult.begin())) {\n> > > > > + cerr << \"Control value mismatch after setting to uint32_t array\" << endl;\n> > > > > + return TestFail;\n> > > > > + }\n> > > > > +\n> > > > > + if (value.toString() != \"[ 3, 14, 15, 9 ]\") {\n> > > > > + cerr << \"Control string mismatch after setting to uint32_t array\" << endl;\n> > > > > + return TestFail;\n> > > > > + }\n> > > > > +\n> > > > > /*\n> > > > > * Integer32 type.\n> > > > > */","headers":{"Return-Path":"","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 60884C31E9\n\tfor ;\n\tTue, 29 Oct 2024 16:14:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 906B165399;\n\tTue, 29 Oct 2024 17:14:34 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 164F960366\n\tfor ;\n\tTue, 29 Oct 2024 17:14:33 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 316C6AF;\n\tTue, 29 Oct 2024 17:14:30 +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=\"VF8uuHA2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730218470;\n\tbh=1Hr2gudVSOLESt+1Uw2XWhopOmyAqNCr/qRLP0ExDiw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VF8uuHA2QQD8pXICax2vRVg+GNynVIT/oEN9Ldl96mv9bWHyL6fVnodHRs/qtLUeA\n\tqAHFCLAzogmqbHzpDE8gitPmOIwcpqJMx0PGWqlVsx5afM81vVXwDK4x+WP69sPwve\n\tSiqiXXBU1Oz+1M5v/krakOTdIkr9tcrEbjdCUPVw=","Date":"Tue, 29 Oct 2024 18:14:26 +0200","From":"Laurent Pinchart ","To":"Cheng-Hao Yang ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org,\n\tYudhistira Erlandinata ","Subject":"Re: [PATCH v4 1/2] libcamera: Extend u32 control type","Message-ID":"<20241029161426.GQ22600@pendragon.ideasonboard.com>","References":"<20241025153948.1213629-1-chenghaoyang@chromium.org>\n\t<20241025153948.1213629-2-chenghaoyang@chromium.org>\n\t<3u4djitcrcv62qodtapynuzw6xtjdikmnanyrsnit53mcthnwt@yvy2sonegonl>\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31964,"web_url":"https://patchwork.libcamera.org/comment/31964/","msgid":"","date":"2024-10-29T16:50:08","subject":"Re: [PATCH v4 1/2] libcamera: Extend u32 control type","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Laurent,\n\nOn Wed, Oct 30, 2024 at 12:14 AM Laurent Pinchart\n wrote:\n>\n> Hi Harvey,\n>\n> On Wed, Oct 30, 2024 at 12:11:12AM +0800, Cheng-Hao Yang wrote:\n> > On Tue, Oct 29, 2024 at 6:33 PM Jacopo Mondi wrote:\n> > > On Tue, Oct 29, 2024 at 06:08:44PM +0800, Cheng-Hao Yang wrote:\n> > > > On Mon, Oct 28, 2024 at 6:36 PM Jacopo Mondi wrote:\n> > > > > On Fri, Oct 25, 2024 at 03:38:50PM +0000, Harvey Yang wrote:\n> > > > > > From: Yudhistira Erlandinata \n> > > > > >\n> > > > > > V4L2 Controls support a wide variety of types not yet supported by the\n> > > > > > ControlValue type system.\n> > > > > >\n> > > > > > Extend the libcamera ControlValue types to support an explicit 32 bit\n> > > > > > unsigned integer type, and map that to the corresponding\n> > > > > > V4L2_CTRL_TYPE_U32 type within the v4l2_device support class.\n> > > > > >\n> > > > > > Signed-off-by: Yudhistira Erlandinata \n> > > > > > Co-developed-by: Harvey Yang \n> > > > > > Signed-off-by: Harvey Yang \n> > > > > > ---\n> > > > > > include/libcamera/controls.h | 7 ++++++\n> > > > > > src/libcamera/controls.cpp | 12 ++++++++--\n> > > > > > src/libcamera/v4l2_device.cpp | 14 ++++++++++++\n> > > > > > test/controls/control_value.cpp | 40 +++++++++++++++++++++++++++++++++\n> > > > > > 4 files changed, 71 insertions(+), 2 deletions(-)\n> > > > > >\n> > > > > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > > > > > index ca60bbaca..6da8ad2c3 100644\n> > > > > > --- a/include/libcamera/controls.h\n> > > > > > +++ b/include/libcamera/controls.h\n> > > > > > @@ -29,6 +29,7 @@ enum ControlType {\n> > > > > > ControlTypeNone,\n> > > > > > ControlTypeBool,\n> > > > > > ControlTypeByte,\n> > > > > > + ControlTypeUnsigned32,\n> > > > > > ControlTypeInteger32,\n> > > > > > ControlTypeInteger64,\n> > > > > > ControlTypeFloat,\n> > > > > > @@ -62,6 +63,12 @@ struct control_type {\n> > > > > > static constexpr std::size_t size = 0;\n> > > > > > };\n> > > > > >\n> > > > > > +template<>\n> > > > > > +struct control_type {\n> > > > > > + static constexpr ControlType value = ControlTypeUnsigned32;\n> > > > > > + static constexpr std::size_t size = 0;\n> > > > > > +};\n> > > > > > +\n> > > > > > template<>\n> > > > > > struct control_type {\n> > > > > > static constexpr ControlType value = ControlTypeInteger32;\n> > > > > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > > > > > index 62185d643..8ae295191 100644\n> > > > > > --- a/src/libcamera/controls.cpp\n> > > > > > +++ b/src/libcamera/controls.cpp\n> > > > > > @@ -54,6 +54,7 @@ static constexpr size_t ControlValueSize[] = {\n> > > > > > [ControlTypeNone] = 0,\n> > > > > > [ControlTypeBool] = sizeof(bool),\n> > > > > > [ControlTypeByte] = sizeof(uint8_t),\n> > > > > > + [ControlTypeUnsigned32] = sizeof(uint32_t),\n> > > > > > [ControlTypeInteger32] = sizeof(int32_t),\n> > > > > > [ControlTypeInteger64] = sizeof(int64_t),\n> > > > > > [ControlTypeFloat] = sizeof(float),\n> > > > > > @@ -74,10 +75,12 @@ static constexpr size_t ControlValueSize[] = {\n> > > > > > * The control stores a boolean value\n> > > > > > * \\var ControlTypeByte\n> > > > > > * The control stores a byte value as an unsigned 8-bit integer\n> > > > > > + * \\var ControlTypeUnsigned32\n> > > > > > + * The control stores an unsigned 32-bit integer value\n> > > > > > * \\var ControlTypeInteger32\n> > > > > > - * The control stores a 32-bit integer value\n> > > > > > + * The control stores a signed 32-bit integer value\n> > > > > > * \\var ControlTypeInteger64\n> > > > > > - * The control stores a 64-bit integer value\n> > > > > > + * The control stores a signed 64-bit integer value\n> > > > > > * \\var ControlTypeFloat\n> > > > > > * The control stores a 32-bit floating point value\n> > > > > > * \\var ControlTypeString\n> > > > > > @@ -230,6 +233,11 @@ std::string ControlValue::toString() const\n> > > > > > str += std::to_string(*value);\n> > > > > > break;\n> > > > > > }\n> > > > > > + case ControlTypeUnsigned32: {\n> > > > > > + const uint32_t *value = reinterpret_cast(data);\n> > > > > > + str += std::to_string(*value);\n> > > > > > + break;\n> > > > > > + }\n> > > > > > case ControlTypeInteger32: {\n> > > > > > const int32_t *value = reinterpret_cast(data);\n> > > > > > str += std::to_string(*value);\n> > > > > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > > > > > index 68add4f2e..f39ec3760 100644\n> > > > > > --- a/src/libcamera/v4l2_device.cpp\n> > > > > > +++ b/src/libcamera/v4l2_device.cpp\n> > > > > > @@ -9,6 +9,7 @@\n> > > > > >\n> > > > > > #include \n> > > > > > #include \n> > > > > > +#include \n> > > > > > #include \n> > > > > > #include \n> > > > > > #include \n> > > > > > @@ -210,6 +211,10 @@ ControlList V4L2Device::getControls(const std::vector &ids)\n> > > > > > type = ControlTypeByte;\n> > > > > > break;\n> > > > > >\n> > > > > > + case V4L2_CTRL_TYPE_U32:\n> > > > > > + type = ControlTypeUnsigned32;\n> > > > > > + break;\n> > > > > > +\n> > > > >\n> > > > > This is not enough.\n> > > > >\n> > > > > After the switch we have\n> > > > >\n> > > > > ControlValue &value = ctrl.second;\n> > > > > value.reserve(type, true, info.elems);\n> > > > > Span data = value.data();\n> > > > >\n> > > > > v4l2Ctrl.p_u8 = data.data();\n> > > > > v4l2Ctrl.size = data.size();\n> > > > >\n> > > > > However controls of type V4L2_CTRL_TYPE_U32 use the p_u32 union member\n> > > > > of 'struct v4l2_ext_control'\n> > > > > https://www.kernel.org/doc/html/v4.9/media/uapi/v4l/vidioc-g-ext-ctrls.html#id1\n> > > > >\n> > > > > Same for U16 control types.\n> > > > >\n> > > > > Be careful about one thing. v4l2Ctrl.p_u32 is of type __u32* so it\n> > > > > would feel natural to\n> > > > >\n> > > > > Span data = value.data();\n> > > > > v4l2Ctrl.p_u32 = data.data();\n> > > > >\n> > > > > as data.data() returns a T*\n> > > > >\n> > > > > However v4l2Ctrl.size wants the size in bytes while data.size()\n> > > > > returns the number of elements, so you should probably use\n> > > > > data.size_bytes().\n> > > > >\n> > > > > Or you can do as we do in setControls and cast the data.data()\n> > > > > pointer.\n> > > > >\n> > > > > Span data = value.data();\n> > > > > v4l2Ctrl.p_u32 = reinterpret_cast(data.data());\n> > > > > v4l2Ctrl.size = data.size();\n> > > > >\n> > > > > Also, you need to handle U32 and U16 also setControl().\n> > > >\n> > > > Thanks for the catch!\n> > > > Please help take a look at my WIP patch:\n> > > > https://gitlab.freedesktop.org/chenghaoyang/libcamera/-/commit/faec47f605881062e58718371183a0c2d3ffe772\n> > > > , before I upload another version.\n> > > > Let me know if you think I should merge the two `switch`es and duplicate\n> > > > the code in the middle.\n> > >\n> > >\n> > > You would have to copy\n> > >\n> > > value.reserve(type, true, info.elems);\n> > > Span data = value.data();\n> > >\n> > > In the two switch branches but you can keep\n> > >\n> > > v4l2Ctrl.size = data.size();\n> > >\n> > > After the switch.\n> >\n> > Done.\n> >\n> > > Up to you, I would prefer a single switch but I don't mind too much\n> > >\n> > > > > If you can test this by reading and writing u16 and u32 controls this\n> > > > > would make me way more confident this patch is correct.\n> > > >\n> > > > Hmm, I couldn't find such a use case. Can you suggest how we could\n> > > > test it?\n> > >\n> > > Well, I presume you have a device with a u16/u32 control if you sent\n> > > this :) Just calling getControls(), trying to set them to some value\n> > > and reading them back would validate the implementation. Or have I\n> > > missed your question ?\n> >\n> > Actually, the two new types are not being used in setControls() / getControls().\n> > u32, for example, is only used for listControls with VIDIOC_QUERY_EXT_CTRL [1],\n> > and V4L2Device needs to recognize u32.\n>\n> But surely that's used with a device ? Can you point to the driver that\n> exposes these controls ?\n\nYes, the failure happens in Aie [1]\n, and `sourceVideo_` should be \"mtk-aie-5.3-source\" [2].\nLet me know if I missed anything.\n\nBR,\nHarvey\n\n[1]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/mtkisp7/src/libcamera/pipeline/mtkisp7/face_detect/aie.cpp;l=156\n[2]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/mtkisp7/src/libcamera/pipeline/mtkisp7/face_detect/aie.cpp;l=121\n\n>\n> > That's also why I didn't have the changes in setControls() / getControls().\n> >\n> > BR,\n> > Harvey\n> >\n> > [1]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/mtkisp7/src/libcamera/v4l2_device.cpp;l=681\n> >\n> > > > > > default:\n> > > > > > LOG(V4L2, Error)\n> > > > > > << \"Unsupported payload control type \"\n> > > > > > @@ -488,6 +493,9 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)\n> > > > > > case V4L2_CTRL_TYPE_BOOLEAN:\n> > > > > > return ControlTypeBool;\n> > > > > >\n> > > > > > + case V4L2_CTRL_TYPE_U32:\n> > > > > > + return ControlTypeUnsigned32;\n> > > > > > +\n> > > > > > case V4L2_CTRL_TYPE_INTEGER:\n> > > > > > return ControlTypeInteger32;\n> > > > > >\n> > > > > > @@ -536,6 +544,11 @@ std::optional V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl\n> > > > > > static_cast(ctrl.maximum),\n> > > > > > static_cast(ctrl.default_value));\n> > > > > >\n> > > > > > + case V4L2_CTRL_TYPE_U32:\n> > > > > > + return ControlInfo(static_cast(ctrl.minimum),\n> > > > > > + static_cast(ctrl.maximum),\n> > > > > > + static_cast(ctrl.default_value));\n> > > > > > +\n> > > > > > case V4L2_CTRL_TYPE_BOOLEAN:\n> > > > > > return ControlInfo(static_cast(ctrl.minimum),\n> > > > > > static_cast(ctrl.maximum),\n> > > > > > @@ -622,6 +635,7 @@ void V4L2Device::listControls()\n> > > > > > case V4L2_CTRL_TYPE_BITMASK:\n> > > > > > case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > > > > > case V4L2_CTRL_TYPE_U8:\n> > > > > > + case V4L2_CTRL_TYPE_U32:\n> > > > > > break;\n> > > > > > /* \\todo Support other control types. */\n> > > > > > default:\n> > > > > > diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n> > > > > > index 344107fae..6ca85b739 100644\n> > > > > > --- a/test/controls/control_value.cpp\n> > > > > > +++ b/test/controls/control_value.cpp\n> > > > > > @@ -109,6 +109,46 @@ protected:\n> > > > > > return TestFail;\n> > > > > > }\n> > > > > >\n> > > > > > + /*\n> > > > > > + * Unsigned Integer32 type.\n> > > > > > + */\n> > > > > > + value.set(static_cast(42));\n> > > > > > + if (value.isNone() || value.isArray() ||\n> > > > > > + value.type() != ControlTypeUnsigned32) {\n> > > > > > + cerr << \"Control type mismatch after setting to uint32_t\" << endl;\n> > > > > > + return TestFail;\n> > > > > > + }\n> > > > > > +\n> > > > > > + if (value.get() != 42) {\n> > > > > > + cerr << \"Control value mismatch after setting to uint32_t\" << endl;\n> > > > > > + return TestFail;\n> > > > > > + }\n> > > > > > +\n> > > > > > + if (value.toString() != \"42\") {\n> > > > > > + cerr << \"Control string mismatch after setting to uint32_t\" << endl;\n> > > > > > + return TestFail;\n> > > > > > + }\n> > > > > > +\n> > > > > > + std::array uint32s{ 3, 14, 15, 9 };\n> > > > > > + value.set(Span(uint32s));\n> > > > > > + if (value.isNone() || !value.isArray() ||\n> > > > > > + value.type() != ControlTypeUnsigned32) {\n> > > > > > + cerr << \"Control type mismatch after setting to uint32_t array\" << endl;\n> > > > > > + return TestFail;\n> > > > > > + }\n> > > > > > +\n> > > > > > + Span uint32sResult = value.get>();\n> > > > > > + if (uint32s.size() != uint32sResult.size() ||\n> > > > > > + !std::equal(uint32s.begin(), uint32s.end(), uint32sResult.begin())) {\n> > > > > > + cerr << \"Control value mismatch after setting to uint32_t array\" << endl;\n> > > > > > + return TestFail;\n> > > > > > + }\n> > > > > > +\n> > > > > > + if (value.toString() != \"[ 3, 14, 15, 9 ]\") {\n> > > > > > + cerr << \"Control string mismatch after setting to uint32_t array\" << endl;\n> > > > > > + return TestFail;\n> > > > > > + }\n> > > > > > +\n> > > > > > /*\n> > > > > > * Integer32 type.\n> > > > > > */\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","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 0A0A3C3220\n\tfor ;\n\tTue, 29 Oct 2024 16:50:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3AD3A60366;\n\tTue, 29 Oct 2024 17:50:22 +0100 (CET)","from mail-lj1-x233.google.com (mail-lj1-x233.google.com\n\t[IPv6:2a00:1450:4864:20::233])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CF74460366\n\tfor ;\n\tTue, 29 Oct 2024 17:50:20 +0100 (CET)","by mail-lj1-x233.google.com with SMTP id\n\t38308e7fff4ca-2fb5fa911aaso78677441fa.2\n\tfor ;\n\tTue, 29 Oct 2024 09:50:20 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"G0Dp6JNp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1730220620; x=1730825420;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=LMyHZ5jaKFAs35kFbI+2vK4TTOyeVR84sMSTFTwT2eE=;\n\tb=G0Dp6JNpJp2h0XO/6y+Q449Ln4nLfsHQqHHhumF367K5Us8JEo4PxCpYlSkyUcxY6A\n\tNZucOm+yoFw/AV3RLNlCRX+jxbPEn2n5DERictObrWkLKIhAvP23E1X6CcoB7iraqu4g\n\tZsxl62UgoPDqeJvh6xIvG2hlvfzAbrwWYtbvc=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1730220620; x=1730825420;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=LMyHZ5jaKFAs35kFbI+2vK4TTOyeVR84sMSTFTwT2eE=;\n\tb=iWhXxBKaiBzl7+Jil7muKiQOC8O10oSWQtmNplYLznTgdFeePM5QoyI0H2YaWV61Sd\n\tn9fIWsAv6ggR09r5Zby66LVwQ97YUpsNd7rV1c5TF8k/sePA22zYYuHsSL/30NovaTbS\n\tgDOjZUIiaAZk0jNwnp3/SxhdaI95RidBp7mxR4hchgT3esHHm07KmS4gRmuF+3bzpmBh\n\txae4sBHKrnltJyM3BSCIR4ZKD4i89NV00kMsqm5LoDbN9psfndCC4Vc77viKfzDScIkv\n\teMiST+CScDlVke0HdL4gQ2j/6Z91++6yVRS6Hj0Vm1HPPZ3nlXx7vvHG3EkRxAmewBBO\n\tEvCw==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCXG9uOxKQrXeXBHqhFllh5NNFAmEx7Si0JCCPtDnNoCd4JqGojGXEJmexKPvn5o5/ur8hOEO7LzADEviyxvPsI=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YzHr2QMvpLpuBosGsFHRs4yn3OL9P9M5m9NDtecH+a+bEg2H/lo\n\tzeY2sDzZESIWEyZl+CQUqnbL605IyyAyBWhq7RiIye2PJ95dBA7lBJw3MJPZMDtAqY44aAB51mH\n\tNZWhXPFysHnVbNxOIjYONpCPFdY1G4v0OtKE3SxpmRwV29cpH1w==","X-Google-Smtp-Source":"AGHT+IEi4JTXw8ephm4YGeq7n7B/YXzvktAddCJaNtSjYkH645Gz1C3zLzQsKtZTywqtFcoBhDIR7BTCnApFDWCWdA0=","X-Received":"by 2002:a2e:f11:0:b0:2fb:6509:b6ba with SMTP id\n\t38308e7fff4ca-2fcbe065723mr75581231fa.33.1730220619881;\n\tTue, 29 Oct 2024 09:50:19 -0700 (PDT)","MIME-Version":"1.0","References":"<20241025153948.1213629-1-chenghaoyang@chromium.org>\n\t<20241025153948.1213629-2-chenghaoyang@chromium.org>\n\t<3u4djitcrcv62qodtapynuzw6xtjdikmnanyrsnit53mcthnwt@yvy2sonegonl>\n\t\n\t\n\t\n\t<20241029161426.GQ22600@pendragon.ideasonboard.com>","In-Reply-To":"<20241029161426.GQ22600@pendragon.ideasonboard.com>","From":"Cheng-Hao Yang ","Date":"Wed, 30 Oct 2024 00:50:08 +0800","Message-ID":"","Subject":"Re: [PATCH v4 1/2] libcamera: Extend u32 control type","To":"Laurent Pinchart ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org, \n\tYudhistira Erlandinata ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]