[{"id":17047,"web_url":"https://patchwork.libcamera.org/comment/17047/","msgid":"","date":"2021-05-20T09:03:30","subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: V4L2Control: remove\n\tV4L2Control classes","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Gentle ping for review, or could the patch series be merged?\n\nOn Mon, May 10, 2021 at 2:42 PM Hirokazu Honda wrote:\n\n> V4L2ControlId and V4L2ControlInfo are just convenience classes to\n> create ControlId and ControlInfo from v4l2_query_ext_control.\n> Therefore, there is no need of being a class. It is used only\n> from V4L2Device. This removes the classes and put the equivalent\n> functions of creating ControlId and ControlInfo in\n> v4l2_device.cpp.\n>\n> Signed-off-by: Hirokazu Honda \n> Reviewed-by: Jacopo Mondi \n> ---\n> include/libcamera/internal/meson.build | 1 -\n> include/libcamera/internal/v4l2_controls.h | 31 -----\n> include/libcamera/internal/v4l2_device.h | 4 +-\n> src/libcamera/meson.build | 1 -\n> src/libcamera/v4l2_controls.cpp | 151 ---------------------\n> src/libcamera/v4l2_device.cpp | 72 +++++++++-\n> 6 files changed, 71 insertions(+), 189 deletions(-)\n> delete mode 100644 include/libcamera/internal/v4l2_controls.h\n> delete mode 100644 src/libcamera/v4l2_controls.cpp\n>\n> diff --git a/include/libcamera/internal/meson.build\n> b/include/libcamera/internal/meson.build\n> index 1fe3918c..3fb0216d 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -42,7 +42,6 @@ libcamera_internal_headers = files([\n> 'thread.h',\n> 'timer.h',\n> 'utils.h',\n> - 'v4l2_controls.h',\n> 'v4l2_device.h',\n> 'v4l2_pixelformat.h',\n> 'v4l2_subdevice.h',\n> diff --git a/include/libcamera/internal/v4l2_controls.h\n> b/include/libcamera/internal/v4l2_controls.h\n> deleted file mode 100644\n> index 0851b8dd..00000000\n> --- a/include/libcamera/internal/v4l2_controls.h\n> +++ /dev/null\n> @@ -1,31 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2019, Google Inc.\n> - *\n> - * v4l2_controls.h - V4L2 Controls Support\n> - */\n> -\n> -#ifndef __LIBCAMERA_INTERNAL_V4L2_CONTROLS_H__\n> -#define __LIBCAMERA_INTERNAL_V4L2_CONTROLS_H__\n> -\n> -#include \n> -\n> -#include \n> -\n> -namespace libcamera {\n> -\n> -class V4L2ControlId : public ControlId\n> -{\n> -public:\n> - V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl);\n> -};\n> -\n> -class V4L2ControlInfo : public ControlInfo\n> -{\n> -public:\n> - V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);\n> -};\n> -\n> -} /* namespace libcamera */\n> -\n> -#endif /* __LIBCAMERA_INTERNAL_V4L2_CONTROLS_H__ */\n> diff --git a/include/libcamera/internal/v4l2_device.h\n> b/include/libcamera/internal/v4l2_device.h\n> index 087f07e7..116bbbaf 100644\n> --- a/include/libcamera/internal/v4l2_device.h\n> +++ b/include/libcamera/internal/v4l2_device.h\n> @@ -13,11 +13,11 @@\n>\n> #include \n>\n> +#include \n> #include \n> #include \n>\n> #include \"libcamera/internal/log.h\"\n> -#include \"libcamera/internal/v4l2_controls.h\"\n>\n> namespace libcamera {\n>\n> @@ -61,7 +61,7 @@ private:\n> void eventAvailable(EventNotifier *notifier);\n>\n> std::map controlInfo_;\n> - std::vector> controlIds_;\n> + std::vector> controlIds_;\n> ControlInfoMap controls_;\n> std::string deviceNode_;\n> int fd_;\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index e0a48aa2..62251a64 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -51,7 +51,6 @@ libcamera_sources = files([\n> 'timer.cpp',\n> 'transform.cpp',\n> 'utils.cpp',\n> - 'v4l2_controls.cpp',\n> 'v4l2_device.cpp',\n> 'v4l2_pixelformat.cpp',\n> 'v4l2_subdevice.cpp',\n> diff --git a/src/libcamera/v4l2_controls.cpp\n> b/src/libcamera/v4l2_controls.cpp\n> deleted file mode 100644\n> index 3f8ec6ca..00000000\n> --- a/src/libcamera/v4l2_controls.cpp\n> +++ /dev/null\n> @@ -1,151 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2019, Google Inc.\n> - *\n> - * v4l2_controls.cpp - V4L2 Controls Support\n> - */\n> -\n> -#include \"libcamera/internal/v4l2_controls.h\"\n> -\n> -#include \n> -\n> -/**\n> - * \\file v4l2_controls.h\n> - * \\brief Support for V4L2 Controls using the V4L2 Extended Controls APIs\n> - *\n> - * The V4L2 Control API allows application to inspect and modify sets of\n> - * configurable parameters on a video device or subdevice. The nature of\n> the\n> - * parameters an application can modify using the control framework\n> depends on\n> - * what the driver implements support for, and on the characteristics of\n> the\n> - * underlying hardware platform. Generally controls are used to modify\n> user\n> - * visible settings, such as the image brightness and exposure time, or\n> - * non-standard parameters which cannot be controlled through the V4L2\n> format\n> - * negotiation API.\n> - *\n> - * Controls are identified by a numerical ID, defined by the V4L2 kernel\n> headers\n> - * and have an associated type. Each control has a value, which is the\n> data that\n> - * can be modified with V4L2Device::setControls() or retrieved with\n> - * V4L2Device::getControls().\n> - *\n> - * The control's type along with the control's flags define the type of\n> the\n> - * control's value content. Controls can transport a single data value\n> stored in\n> - * variable inside the control, or they might as well deal with more\n> complex\n> - * data types, such as arrays of matrices, stored in a contiguous memory\n> - * locations associated with the control and called 'the payload'. Such\n> controls\n> - * are called 'compound controls' and are currently not supported by the\n> - * libcamera V4L2 control framework.\n> - *\n> - * libcamera implements support for controls using the V4L2 Extended\n> Control\n> - * API, which allows future handling of controls with payloads of\n> arbitrary\n> - * sizes.\n> - *\n> - * The libcamera V4L2 Controls framework operates on lists of controls,\n> wrapped\n> - * by the ControlList class, to match the V4L2 extended controls API. The\n> - * interface to set and get control is implemented by the V4L2Device\n> class, and\n> - * this file only provides the data type definitions.\n> - *\n> - * \\todo Add support for compound controls\n> - */\n> -\n> -namespace libcamera {\n> -\n> -namespace {\n> -\n> -std::string v4l2_ctrl_name(const struct v4l2_query_ext_ctrl &ctrl)\n> -{\n> - size_t len = strnlen(ctrl.name, sizeof(ctrl.name));\n> - return std::string(static_cast(ctrl.name), len);\n> -}\n> -\n> -ControlType v4l2_ctrl_type(const struct v4l2_query_ext_ctrl &ctrl)\n> -{\n> - switch (ctrl.type) {\n> - case V4L2_CTRL_TYPE_U8:\n> - return ControlTypeByte;\n> -\n> - case V4L2_CTRL_TYPE_BOOLEAN:\n> - return ControlTypeBool;\n> -\n> - case V4L2_CTRL_TYPE_INTEGER:\n> - return ControlTypeInteger32;\n> -\n> - case V4L2_CTRL_TYPE_INTEGER64:\n> - return ControlTypeInteger64;\n> -\n> - case V4L2_CTRL_TYPE_MENU:\n> - case V4L2_CTRL_TYPE_BUTTON:\n> - case V4L2_CTRL_TYPE_BITMASK:\n> - case V4L2_CTRL_TYPE_INTEGER_MENU:\n> - /*\n> - * More precise types may be needed, for now use a 32-bit\n> - * integer type.\n> - */\n> - return ControlTypeInteger32;\n> -\n> - default:\n> - return ControlTypeNone;\n> - }\n> -}\n> -\n> -} /* namespace */\n> -\n> -/**\n> - * \\class V4L2ControlId\n> - * \\brief V4L2 control static metadata\n> - *\n> - * The V4L2ControlId class is a specialisation of the ControlId for V4L2\n> - * controls.\n> - */\n> -\n> -/**\n> - * \\brief Construct a V4L2ControlId from a struct v4l2_query_ext_ctrl\n> - * \\param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the\n> kernel\n> - */\n> -V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl)\n> - : ControlId(ctrl.id, v4l2_ctrl_name(ctrl), v4l2_ctrl_type(ctrl))\n> -{\n> -}\n> -\n> -/**\n> - * \\class V4L2ControlInfo\n> - * \\brief Convenience specialisation of ControlInfo for V4L2 controls\n> - *\n> - * The V4L2ControlInfo class is a specialisation of the ControlInfo for\n> V4L2\n> - * controls. It offers a convenience constructor from a struct\n> - * v4l2_query_ext_ctrl, and is otherwise equivalent to the ControlInfo\n> class.\n> - */\n> -\n> -/**\n> - * \\brief Construct a V4L2ControlInfo from a struct v4l2_query_ext_ctrl\n> - * \\param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the\n> kernel\n> - */\n> -V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n> -{\n> - switch (ctrl.type) {\n> - case V4L2_CTRL_TYPE_U8:\n> -\n> ControlInfo::operator=(ControlInfo(static_cast(ctrl.minimum),\n> -\n> static_cast(ctrl.maximum),\n> -\n> static_cast(ctrl.default_value)));\n> - break;\n> -\n> - case V4L2_CTRL_TYPE_BOOLEAN:\n> -\n> ControlInfo::operator=(ControlInfo(static_cast(ctrl.minimum),\n> -\n> static_cast(ctrl.maximum),\n> -\n> static_cast(ctrl.default_value)));\n> - break;\n> -\n> - case V4L2_CTRL_TYPE_INTEGER64:\n> -\n> ControlInfo::operator=(ControlInfo(static_cast(ctrl.minimum),\n> -\n> static_cast(ctrl.maximum),\n> -\n> static_cast(ctrl.default_value)));\n> - break;\n> -\n> - default:\n> -\n> ControlInfo::operator=(ControlInfo(static_cast(ctrl.minimum),\n> -\n> static_cast(ctrl.maximum),\n> -\n> static_cast(ctrl.default_value)));\n> - break;\n> - }\n> -}\n> -\n> -} /* namespace libcamera */\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 397029ac..f9414043 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -20,7 +20,6 @@\n> #include \"libcamera/internal/log.h\"\n> #include \"libcamera/internal/sysfs.h\"\n> #include \"libcamera/internal/utils.h\"\n> -#include \"libcamera/internal/v4l2_controls.h\"\n>\n> /**\n> * \\file v4l2_device.h\n> @@ -31,6 +30,73 @@ namespace libcamera {\n>\n> LOG_DEFINE_CATEGORY(V4L2)\n>\n> +namespace {\n> +\n> +ControlType v4l2CtrlType(uint32_t ctrlType)\n> +{\n> + switch (ctrlType) {\n> + case V4L2_CTRL_TYPE_U8:\n> + return ControlTypeByte;\n> +\n> + case V4L2_CTRL_TYPE_BOOLEAN:\n> + return ControlTypeBool;\n> +\n> + case V4L2_CTRL_TYPE_INTEGER:\n> + return ControlTypeInteger32;\n> +\n> + case V4L2_CTRL_TYPE_INTEGER64:\n> + return ControlTypeInteger64;\n> +\n> + case V4L2_CTRL_TYPE_MENU:\n> + case V4L2_CTRL_TYPE_BUTTON:\n> + case V4L2_CTRL_TYPE_BITMASK:\n> + case V4L2_CTRL_TYPE_INTEGER_MENU:\n> + /*\n> + * More precise types may be needed, for now use a 32-bit\n> + * integer type.\n> + */\n> + return ControlTypeInteger32;\n> +\n> + default:\n> + return ControlTypeNone;\n> + }\n> +}\n> +\n> +std::unique_ptr v4l2ControlId(const v4l2_query_ext_ctrl &ctrl)\n> +{\n> + const size_t len = strnlen(ctrl.name, sizeof(ctrl.name));\n> + const std::string name(static_cast(ctrl.name), len);\n> + const ControlType type = v4l2CtrlType(ctrl.type);\n> +\n> + return std::make_unique(ctrl.id, name, type);\n> +}\n> +\n> +ControlInfo v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl)\n> +{\n> + switch (ctrl.type) {\n> + case V4L2_CTRL_TYPE_U8:\n> + return ControlInfo(static_cast(ctrl.minimum),\n> + static_cast(ctrl.maximum),\n> +\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> + static_cast(ctrl.default_value));\n> +\n> + case V4L2_CTRL_TYPE_INTEGER64:\n> + return ControlInfo(static_cast(ctrl.minimum),\n> + static_cast(ctrl.maximum),\n> +\n> static_cast(ctrl.default_value));\n> +\n> + default:\n> + return ControlInfo(static_cast(ctrl.minimum),\n> + static_cast(ctrl.maximum),\n> +\n> static_cast(ctrl.default_value));\n> + }\n> +}\n> +} /* namespace */\n> +\n> /**\n> * \\class V4L2Device\n> * \\brief Base class for V4L2VideoDevice and V4L2Subdevice\n> @@ -502,10 +568,10 @@ void V4L2Device::listControls()\n> continue;\n> }\n>\n> -\n> controlIds_.emplace_back(std::make_unique(ctrl));\n> + controlIds_.emplace_back(v4l2ControlId(ctrl));\n> controlInfo_.emplace(ctrl.id, ctrl);\n>\n> - ctrls.emplace(controlIds_.back().get(),\n> V4L2ControlInfo(ctrl));\n> + ctrls.emplace(controlIds_.back().get(),\n> v4l2ControlInfo(ctrl));\n> }\n>\n> controls_ = std::move(ctrls);\n> --\n> 2.31.1.607.g51e8a6a459-goog\n>\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 5BBDAC31FF\n\tfor ;\n\tThu, 20 May 2021 09:03:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 137526891D;\n\tThu, 20 May 2021 11:03:44 +0200 (CEST)","from mail-ed1-x52f.google.com (mail-ed1-x52f.google.com\n\t[IPv6:2a00:1450:4864:20::52f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9CCCB68919\n\tfor ;\n\tThu, 20 May 2021 11:03:42 +0200 (CEST)","by mail-ed1-x52f.google.com with SMTP id y7so997563eda.2\n\tfor ;\n\tThu, 20 May 2021 02:03:42 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"LT48fbh0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=S5L2VI71Gk+CHKjJGytm6/2hZTLgijuVkQNA3umQAzw=;\n\tb=LT48fbh08SSH7Ps9Pz8pmyOhgQolQ/7L3ly2+QbF4A07zbTnHwUzjWn2MloIEXs8L1\n\t+N9vDlAMYqc5dbROXJlPE8AYCZiauvVXwM+L2arhddyN4lWRvCm2cUBV7rQZXAR5UHos\n\tUGwiQpuoXVN8ezUeUYIsI5egwAQbGTxyhf+20=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=S5L2VI71Gk+CHKjJGytm6/2hZTLgijuVkQNA3umQAzw=;\n\tb=LmDZosMhzht3TuNE7h304ROWwU/osmUOebLsveYgtgs3Tuz6Y8neMgt00WdwVNv4Zd\n\tjaOH0U8n1kedHt/VivTkvx0bA2hgG7SY09hxP0NDUert7YF1L6uicejSI4A0F6TfOwMp\n\tjP1txR1XZp1ySUJQQDqYwmdszis5eaXxwJGU7QqTUkZqGHBZ7Ngfx/WQ11JkGDZzqK7I\n\t1Y1+aIuaebCBSxgUjV0lgmgNdtlw2qd2a85wn9Ceu9Mj4jodg0TCUHaE5Wkd3Kc8fdUu\n\t1ASt/5Tw/th6uwHpTMY4SCdhry6mG7gIt2RDimeJPPgGzjfbgok00/RW9HremixXh7TQ\n\tbD6A==","X-Gm-Message-State":"AOAM531p74lqOirOd+8eyLNkWSPuj94i4BESMeKqik2K2mUnsTZVAG7v\n\tvuQa+eF16Rbf8WeKucFXoBhzWLx6B8g6T/cV4uxee4hSILU=","X-Google-Smtp-Source":"ABdhPJxk6CbupYOuN/gHb5Qog7mwHZYyxnJW0QbibUwi4CSEhNFLlOh07CmL50b1VXWAtN0YXLX4njH/lkJCE5/r+ys=","X-Received":"by 2002:a05:6402:50c6:: with SMTP id\n\th6mr3853050edb.327.1621501421950; \n\tThu, 20 May 2021 02:03:41 -0700 (PDT)","MIME-Version":"1.0","References":"<20210510054242.2735473-1-hiroh@chromium.org>\n\t<20210510054242.2735473-2-hiroh@chromium.org>","In-Reply-To":"<20210510054242.2735473-2-hiroh@chromium.org>","From":"Hirokazu Honda ","Date":"Thu, 20 May 2021 18:03:30 +0900","Message-ID":"","To":"libcamera devel ","Content-Type":"multipart/alternative; boundary=\"000000000000422c5d05c2bf38f1\"","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: V4L2Control: remove\n\tV4L2Control classes","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":17162,"web_url":"https://patchwork.libcamera.org/comment/17162/","msgid":"","date":"2021-05-24T03:51:28","subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: pipeline: Remove\n\tunnecessary v4l2_controls.h includes","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hiro,\n\nThank you for the patch.\n\nOn Mon, May 10, 2021 at 02:42:41PM +0900, Hirokazu Honda wrote:\n> v4l2_controls.h is included in some places in pipeline codes.\n> But V4l2Control classes are not used there. This removes the\n> redundant v4l2_controls.h includes.\n> \n> Signed-off-by: Hirokazu Honda \n> Reviewed-by: Jacopo Mondi \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 1 -\n> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 1 -\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 1 -\n> src/libcamera/pipeline/vimc/vimc.cpp | 1 -\n> 4 files changed, 4 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index ade8ffbd..411e70b7 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -28,7 +28,6 @@\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> #include \"libcamera/internal/utils.h\"\n> -#include \"libcamera/internal/v4l2_controls.h\"\n> \n> #include \"cio2.h\"\n> #include \"frames.h\"\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index b2256493..e485f25f 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -36,7 +36,6 @@\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> #include \"libcamera/internal/utils.h\"\n> -#include \"libcamera/internal/v4l2_controls.h\"\n> #include \"libcamera/internal/v4l2_videodevice.h\"\n> \n> #include \"dma_heaps.h\"\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index faa8d6b0..12a85b24 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -25,7 +25,6 @@\n> #include \"libcamera/internal/pipeline_handler.h\"\n> #include \"libcamera/internal/sysfs.h\"\n> #include \"libcamera/internal/utils.h\"\n> -#include \"libcamera/internal/v4l2_controls.h\"\n> #include \"libcamera/internal/v4l2_videodevice.h\"\n> \n> namespace libcamera {\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index ce83dcaa..d89ab33a 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -30,7 +30,6 @@\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> #include \"libcamera/internal/utils.h\"\n> -#include \"libcamera/internal/v4l2_controls.h\"\n> #include \"libcamera/internal/v4l2_subdevice.h\"\n> #include \"libcamera/internal/v4l2_videodevice.h\"\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 AD7CEC3200\n\tfor ;\n\tMon, 24 May 2021 03:51:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1307D6891B;\n\tMon, 24 May 2021 05:51:34 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5567B602AD\n\tfor ;\n\tMon, 24 May 2021 05:51:32 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CA8A2476;\n\tMon, 24 May 2021 05:51:31 +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=\"hEeiBPAn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621828292;\n\tbh=k3O6/u/wR+WsIaiGZKVKoszxhUuVVl88zu0J488pw+0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hEeiBPAnIECbc84NV8uNwUdiph3XFjgjJ7C0aKfwRCZO/luzIDT46oJf6gPLNqjzA\n\tRUZFZeQ6BLsAwppOh5fwMMpaFeiINjlNOoq7VgAj4K5iEsz39ZKD1cU6ndZAdveieX\n\tX+6VEuLr23ZE+i8uvORI/9yEBqQ3rG5KGFGYcy6g=","Date":"Mon, 24 May 2021 06:51:28 +0300","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20210510054242.2735473-1-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210510054242.2735473-1-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH v2 1/2] libcamera: pipeline: Remove\n\tunnecessary v4l2_controls.h includes","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17163,"web_url":"https://patchwork.libcamera.org/comment/17163/","msgid":"","date":"2021-05-24T03:57:01","subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: V4L2Control: remove\n\tV4L2Control classes","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hiro,\n\nThank you for the patch.\n\nOn Mon, May 10, 2021 at 02:42:42PM +0900, Hirokazu Honda wrote:\n> V4L2ControlId and V4L2ControlInfo are just convenience classes to\n> create ControlId and ControlInfo from v4l2_query_ext_control.\n> Therefore, there is no need of being a class. It is used only\n> from V4L2Device. This removes the classes and put the equivalent\n> functions of creating ControlId and ControlInfo in\n> v4l2_device.cpp.\n> \n> Signed-off-by: Hirokazu Honda \n> Reviewed-by: Jacopo Mondi \n> ---\n> include/libcamera/internal/meson.build | 1 -\n> include/libcamera/internal/v4l2_controls.h | 31 -----\n> include/libcamera/internal/v4l2_device.h | 4 +-\n> src/libcamera/meson.build | 1 -\n> src/libcamera/v4l2_controls.cpp | 151 ---------------------\n> src/libcamera/v4l2_device.cpp | 72 +++++++++-\n> 6 files changed, 71 insertions(+), 189 deletions(-)\n> delete mode 100644 include/libcamera/internal/v4l2_controls.h\n> delete mode 100644 src/libcamera/v4l2_controls.cpp\n\nNice diffstat :-)\n\nReviewed-by: Laurent Pinchart \n\n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index 1fe3918c..3fb0216d 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -42,7 +42,6 @@ libcamera_internal_headers = files([\n> 'thread.h',\n> 'timer.h',\n> 'utils.h',\n> - 'v4l2_controls.h',\n> 'v4l2_device.h',\n> 'v4l2_pixelformat.h',\n> 'v4l2_subdevice.h',\n> diff --git a/include/libcamera/internal/v4l2_controls.h b/include/libcamera/internal/v4l2_controls.h\n> deleted file mode 100644\n> index 0851b8dd..00000000\n> --- a/include/libcamera/internal/v4l2_controls.h\n> +++ /dev/null\n> @@ -1,31 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2019, Google Inc.\n> - *\n> - * v4l2_controls.h - V4L2 Controls Support\n> - */\n> -\n> -#ifndef __LIBCAMERA_INTERNAL_V4L2_CONTROLS_H__\n> -#define __LIBCAMERA_INTERNAL_V4L2_CONTROLS_H__\n> -\n> -#include \n> -\n> -#include \n> -\n> -namespace libcamera {\n> -\n> -class V4L2ControlId : public ControlId\n> -{\n> -public:\n> -\tV4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl);\n> -};\n> -\n> -class V4L2ControlInfo : public ControlInfo\n> -{\n> -public:\n> -\tV4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);\n> -};\n> -\n> -} /* namespace libcamera */\n> -\n> -#endif /* __LIBCAMERA_INTERNAL_V4L2_CONTROLS_H__ */\n> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> index 087f07e7..116bbbaf 100644\n> --- a/include/libcamera/internal/v4l2_device.h\n> +++ b/include/libcamera/internal/v4l2_device.h\n> @@ -13,11 +13,11 @@\n> \n> #include \n> \n> +#include \n> #include \n> #include \n> \n> #include \"libcamera/internal/log.h\"\n> -#include \"libcamera/internal/v4l2_controls.h\"\n> \n> namespace libcamera {\n> \n> @@ -61,7 +61,7 @@ private:\n> \tvoid eventAvailable(EventNotifier *notifier);\n> \n> \tstd::map controlInfo_;\n> -\tstd::vector> controlIds_;\n> +\tstd::vector> controlIds_;\n> \tControlInfoMap controls_;\n> \tstd::string deviceNode_;\n> \tint fd_;\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index e0a48aa2..62251a64 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -51,7 +51,6 @@ libcamera_sources = files([\n> 'timer.cpp',\n> 'transform.cpp',\n> 'utils.cpp',\n> - 'v4l2_controls.cpp',\n> 'v4l2_device.cpp',\n> 'v4l2_pixelformat.cpp',\n> 'v4l2_subdevice.cpp',\n> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> deleted file mode 100644\n> index 3f8ec6ca..00000000\n> --- a/src/libcamera/v4l2_controls.cpp\n> +++ /dev/null\n> @@ -1,151 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2019, Google Inc.\n> - *\n> - * v4l2_controls.cpp - V4L2 Controls Support\n> - */\n> -\n> -#include \"libcamera/internal/v4l2_controls.h\"\n> -\n> -#include \n> -\n> -/**\n> - * \\file v4l2_controls.h\n> - * \\brief Support for V4L2 Controls using the V4L2 Extended Controls APIs\n> - *\n> - * The V4L2 Control API allows application to inspect and modify sets of\n> - * configurable parameters on a video device or subdevice. The nature of the\n> - * parameters an application can modify using the control framework depends on\n> - * what the driver implements support for, and on the characteristics of the\n> - * underlying hardware platform. Generally controls are used to modify user\n> - * visible settings, such as the image brightness and exposure time, or\n> - * non-standard parameters which cannot be controlled through the V4L2 format\n> - * negotiation API.\n> - *\n> - * Controls are identified by a numerical ID, defined by the V4L2 kernel headers\n> - * and have an associated type. Each control has a value, which is the data that\n> - * can be modified with V4L2Device::setControls() or retrieved with\n> - * V4L2Device::getControls().\n> - *\n> - * The control's type along with the control's flags define the type of the\n> - * control's value content. Controls can transport a single data value stored in\n> - * variable inside the control, or they might as well deal with more complex\n> - * data types, such as arrays of matrices, stored in a contiguous memory\n> - * locations associated with the control and called 'the payload'. Such controls\n> - * are called 'compound controls' and are currently not supported by the\n> - * libcamera V4L2 control framework.\n> - *\n> - * libcamera implements support for controls using the V4L2 Extended Control\n> - * API, which allows future handling of controls with payloads of arbitrary\n> - * sizes.\n> - *\n> - * The libcamera V4L2 Controls framework operates on lists of controls, wrapped\n> - * by the ControlList class, to match the V4L2 extended controls API. The\n> - * interface to set and get control is implemented by the V4L2Device class, and\n> - * this file only provides the data type definitions.\n> - *\n> - * \\todo Add support for compound controls\n> - */\n> -\n> -namespace libcamera {\n> -\n> -namespace {\n> -\n> -std::string v4l2_ctrl_name(const struct v4l2_query_ext_ctrl &ctrl)\n> -{\n> -\tsize_t len = strnlen(ctrl.name, sizeof(ctrl.name));\n> -\treturn std::string(static_cast(ctrl.name), len);\n> -}\n> -\n> -ControlType v4l2_ctrl_type(const struct v4l2_query_ext_ctrl &ctrl)\n> -{\n> -\tswitch (ctrl.type) {\n> -\tcase V4L2_CTRL_TYPE_U8:\n> -\t\treturn ControlTypeByte;\n> -\n> -\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> -\t\treturn ControlTypeBool;\n> -\n> -\tcase V4L2_CTRL_TYPE_INTEGER:\n> -\t\treturn ControlTypeInteger32;\n> -\n> -\tcase V4L2_CTRL_TYPE_INTEGER64:\n> -\t\treturn ControlTypeInteger64;\n> -\n> -\tcase V4L2_CTRL_TYPE_MENU:\n> -\tcase V4L2_CTRL_TYPE_BUTTON:\n> -\tcase V4L2_CTRL_TYPE_BITMASK:\n> -\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n> -\t\t/*\n> -\t\t * More precise types may be needed, for now use a 32-bit\n> -\t\t * integer type.\n> -\t\t */\n> -\t\treturn ControlTypeInteger32;\n> -\n> -\tdefault:\n> -\t\treturn ControlTypeNone;\n> -\t}\n> -}\n> -\n> -} /* namespace */\n> -\n> -/**\n> - * \\class V4L2ControlId\n> - * \\brief V4L2 control static metadata\n> - *\n> - * The V4L2ControlId class is a specialisation of the ControlId for V4L2\n> - * controls.\n> - */\n> -\n> -/**\n> - * \\brief Construct a V4L2ControlId from a struct v4l2_query_ext_ctrl\n> - * \\param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the kernel\n> - */\n> -V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl)\n> -\t: ControlId(ctrl.id, v4l2_ctrl_name(ctrl), v4l2_ctrl_type(ctrl))\n> -{\n> -}\n> -\n> -/**\n> - * \\class V4L2ControlInfo\n> - * \\brief Convenience specialisation of ControlInfo for V4L2 controls\n> - *\n> - * The V4L2ControlInfo class is a specialisation of the ControlInfo for V4L2\n> - * controls. It offers a convenience constructor from a struct\n> - * v4l2_query_ext_ctrl, and is otherwise equivalent to the ControlInfo class.\n> - */\n> -\n> -/**\n> - * \\brief Construct a V4L2ControlInfo from a struct v4l2_query_ext_ctrl\n> - * \\param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the kernel\n> - */\n> -V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n> -{\n> -\tswitch (ctrl.type) {\n> -\tcase V4L2_CTRL_TYPE_U8:\n> -\t\tControlInfo::operator=(ControlInfo(static_cast(ctrl.minimum),\n> -\t\t\t\t\t\t static_cast(ctrl.maximum),\n> -\t\t\t\t\t\t static_cast(ctrl.default_value)));\n> -\t\tbreak;\n> -\n> -\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> -\t\tControlInfo::operator=(ControlInfo(static_cast(ctrl.minimum),\n> -\t\t\t\t\t\t static_cast(ctrl.maximum),\n> -\t\t\t\t\t\t static_cast(ctrl.default_value)));\n> -\t\tbreak;\n> -\n> -\tcase V4L2_CTRL_TYPE_INTEGER64:\n> -\t\tControlInfo::operator=(ControlInfo(static_cast(ctrl.minimum),\n> -\t\t\t\t\t\t static_cast(ctrl.maximum),\n> -\t\t\t\t\t\t static_cast(ctrl.default_value)));\n> -\t\tbreak;\n> -\n> -\tdefault:\n> -\t\tControlInfo::operator=(ControlInfo(static_cast(ctrl.minimum),\n> -\t\t\t\t\t\t static_cast(ctrl.maximum),\n> -\t\t\t\t\t\t static_cast(ctrl.default_value)));\n> -\t\tbreak;\n> -\t}\n> -}\n> -\n> -} /* namespace libcamera */\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 397029ac..f9414043 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -20,7 +20,6 @@\n> #include \"libcamera/internal/log.h\"\n> #include \"libcamera/internal/sysfs.h\"\n> #include \"libcamera/internal/utils.h\"\n> -#include \"libcamera/internal/v4l2_controls.h\"\n> \n> /**\n> * \\file v4l2_device.h\n> @@ -31,6 +30,73 @@ namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(V4L2)\n> \n> +namespace {\n> +\n> +ControlType v4l2CtrlType(uint32_t ctrlType)\n> +{\n> +\tswitch (ctrlType) {\n> +\tcase V4L2_CTRL_TYPE_U8:\n> +\t\treturn ControlTypeByte;\n> +\n> +\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> +\t\treturn ControlTypeBool;\n> +\n> +\tcase V4L2_CTRL_TYPE_INTEGER:\n> +\t\treturn ControlTypeInteger32;\n> +\n> +\tcase V4L2_CTRL_TYPE_INTEGER64:\n> +\t\treturn ControlTypeInteger64;\n> +\n> +\tcase V4L2_CTRL_TYPE_MENU:\n> +\tcase V4L2_CTRL_TYPE_BUTTON:\n> +\tcase V4L2_CTRL_TYPE_BITMASK:\n> +\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n> +\t\t/*\n> +\t\t * More precise types may be needed, for now use a 32-bit\n> +\t\t * integer type.\n> +\t\t */\n> +\t\treturn ControlTypeInteger32;\n> +\n> +\tdefault:\n> +\t\treturn ControlTypeNone;\n> +\t}\n> +}\n> +\n> +std::unique_ptr v4l2ControlId(const v4l2_query_ext_ctrl &ctrl)\n> +{\n> +\tconst size_t len = strnlen(ctrl.name, sizeof(ctrl.name));\n> +\tconst std::string name(static_cast(ctrl.name), len);\n> +\tconst ControlType type = v4l2CtrlType(ctrl.type);\n> +\n> +\treturn std::make_unique(ctrl.id, name, type);\n> +}\n> +\n> +ControlInfo v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl)\n> +{\n> +\tswitch (ctrl.type) {\n> +\tcase V4L2_CTRL_TYPE_U8:\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> +\t\t\t\t static_cast(ctrl.default_value));\n> +\n> +\tcase V4L2_CTRL_TYPE_INTEGER64:\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> +\tdefault:\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> +\t}\n> +}\n> +} /* namespace */\n> +\n> /**\n> * \\class V4L2Device\n> * \\brief Base class for V4L2VideoDevice and V4L2Subdevice\n> @@ -502,10 +568,10 @@ void V4L2Device::listControls()\n> \t\t\tcontinue;\n> \t\t}\n> \n> -\t\tcontrolIds_.emplace_back(std::make_unique(ctrl));\n> +\t\tcontrolIds_.emplace_back(v4l2ControlId(ctrl));\n> \t\tcontrolInfo_.emplace(ctrl.id, ctrl);\n> \n> -\t\tctrls.emplace(controlIds_.back().get(), V4L2ControlInfo(ctrl));\n> +\t\tctrls.emplace(controlIds_.back().get(), v4l2ControlInfo(ctrl));\n> \t}\n> \n> \tcontrols_ = std::move(ctrls);","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 F0BDDC3201\n\tfor ;\n\tMon, 24 May 2021 03:57:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 336986891E;\n\tMon, 24 May 2021 05:57:07 +0200 (CEST)","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 002C0602AD\n\tfor ;\n\tMon, 24 May 2021 05:57:05 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4E282476;\n\tMon, 24 May 2021 05:57:05 +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=\"PUxnOP7O\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621828625;\n\tbh=ey5JLFCGss6G/JqGT4EsmtG4aVSiSv6xDDkHvrP8X+4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PUxnOP7OTOUh4ZGvldCT7GgDr8gmb/a4aou0FVQrABAN898naY4VQx3G2DqtFMqjJ\n\ts3Tl8ojrdJTxxYYaxzdU251Hye3Z1zhBnHSRb7NbgB53oyI8GMRptO81DPltGBfitX\n\tHffMxogN2Tl69fvaSmc4Ujf5gOdwW4jU+zbIsUb8=","Date":"Mon, 24 May 2021 06:57:01 +0300","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20210510054242.2735473-1-hiroh@chromium.org>\n\t<20210510054242.2735473-2-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210510054242.2735473-2-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: V4L2Control: remove\n\tV4L2Control classes","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17164,"web_url":"https://patchwork.libcamera.org/comment/17164/","msgid":"","date":"2021-05-24T04:20:00","subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: V4L2Control: remove\n\tV4L2Control classes","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hiro,\n\nOn Mon, May 24, 2021 at 06:57:03AM +0300, Laurent Pinchart wrote:\n> On Mon, May 10, 2021 at 02:42:42PM +0900, Hirokazu Honda wrote:\n> > V4L2ControlId and V4L2ControlInfo are just convenience classes to\n> > create ControlId and ControlInfo from v4l2_query_ext_control.\n> > Therefore, there is no need of being a class. It is used only\n> > from V4L2Device. This removes the classes and put the equivalent\n> > functions of creating ControlId and ControlInfo in\n> > v4l2_device.cpp.\n> > \n> > Signed-off-by: Hirokazu Honda \n> > Reviewed-by: Jacopo Mondi \n> > ---\n> > include/libcamera/internal/meson.build | 1 -\n> > include/libcamera/internal/v4l2_controls.h | 31 -----\n> > include/libcamera/internal/v4l2_device.h | 4 +-\n> > src/libcamera/meson.build | 1 -\n> > src/libcamera/v4l2_controls.cpp | 151 ---------------------\n> > src/libcamera/v4l2_device.cpp | 72 +++++++++-\n> > 6 files changed, 71 insertions(+), 189 deletions(-)\n> > delete mode 100644 include/libcamera/internal/v4l2_controls.h\n> > delete mode 100644 src/libcamera/v4l2_controls.cpp\n> \n> Nice diffstat :-)\n> \n> Reviewed-by: Laurent Pinchart \n> \n> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> > index 1fe3918c..3fb0216d 100644\n> > --- a/include/libcamera/internal/meson.build\n> > +++ b/include/libcamera/internal/meson.build\n> > @@ -42,7 +42,6 @@ libcamera_internal_headers = files([\n> > 'thread.h',\n> > 'timer.h',\n> > 'utils.h',\n> > - 'v4l2_controls.h',\n> > 'v4l2_device.h',\n> > 'v4l2_pixelformat.h',\n> > 'v4l2_subdevice.h',\n> > diff --git a/include/libcamera/internal/v4l2_controls.h b/include/libcamera/internal/v4l2_controls.h\n> > deleted file mode 100644\n> > index 0851b8dd..00000000\n> > --- a/include/libcamera/internal/v4l2_controls.h\n> > +++ /dev/null\n> > @@ -1,31 +0,0 @@\n> > -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > -/*\n> > - * Copyright (C) 2019, Google Inc.\n> > - *\n> > - * v4l2_controls.h - V4L2 Controls Support\n> > - */\n> > -\n> > -#ifndef __LIBCAMERA_INTERNAL_V4L2_CONTROLS_H__\n> > -#define __LIBCAMERA_INTERNAL_V4L2_CONTROLS_H__\n> > -\n> > -#include \n> > -\n> > -#include \n> > -\n> > -namespace libcamera {\n> > -\n> > -class V4L2ControlId : public ControlId\n> > -{\n> > -public:\n> > -\tV4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl);\n> > -};\n> > -\n> > -class V4L2ControlInfo : public ControlInfo\n> > -{\n> > -public:\n> > -\tV4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);\n> > -};\n> > -\n> > -} /* namespace libcamera */\n> > -\n> > -#endif /* __LIBCAMERA_INTERNAL_V4L2_CONTROLS_H__ */\n> > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> > index 087f07e7..116bbbaf 100644\n> > --- a/include/libcamera/internal/v4l2_device.h\n> > +++ b/include/libcamera/internal/v4l2_device.h\n> > @@ -13,11 +13,11 @@\n> > \n> > #include \n> > \n> > +#include \n> > #include \n> > #include \n> > \n> > #include \"libcamera/internal/log.h\"\n> > -#include \"libcamera/internal/v4l2_controls.h\"\n> > \n> > namespace libcamera {\n> > \n> > @@ -61,7 +61,7 @@ private:\n> > \tvoid eventAvailable(EventNotifier *notifier);\n> > \n> > \tstd::map controlInfo_;\n> > -\tstd::vector> controlIds_;\n> > +\tstd::vector> controlIds_;\n> > \tControlInfoMap controls_;\n> > \tstd::string deviceNode_;\n> > \tint fd_;\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index e0a48aa2..62251a64 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -51,7 +51,6 @@ libcamera_sources = files([\n> > 'timer.cpp',\n> > 'transform.cpp',\n> > 'utils.cpp',\n> > - 'v4l2_controls.cpp',\n> > 'v4l2_device.cpp',\n> > 'v4l2_pixelformat.cpp',\n> > 'v4l2_subdevice.cpp',\n> > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> > deleted file mode 100644\n> > index 3f8ec6ca..00000000\n> > --- a/src/libcamera/v4l2_controls.cpp\n> > +++ /dev/null\n> > @@ -1,151 +0,0 @@\n> > -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > -/*\n> > - * Copyright (C) 2019, Google Inc.\n> > - *\n> > - * v4l2_controls.cpp - V4L2 Controls Support\n> > - */\n> > -\n> > -#include \"libcamera/internal/v4l2_controls.h\"\n> > -\n> > -#include \n> > -\n> > -/**\n> > - * \\file v4l2_controls.h\n> > - * \\brief Support for V4L2 Controls using the V4L2 Extended Controls APIs\n> > - *\n> > - * The V4L2 Control API allows application to inspect and modify sets of\n> > - * configurable parameters on a video device or subdevice. The nature of the\n> > - * parameters an application can modify using the control framework depends on\n> > - * what the driver implements support for, and on the characteristics of the\n> > - * underlying hardware platform. Generally controls are used to modify user\n> > - * visible settings, such as the image brightness and exposure time, or\n> > - * non-standard parameters which cannot be controlled through the V4L2 format\n> > - * negotiation API.\n> > - *\n> > - * Controls are identified by a numerical ID, defined by the V4L2 kernel headers\n> > - * and have an associated type. Each control has a value, which is the data that\n> > - * can be modified with V4L2Device::setControls() or retrieved with\n> > - * V4L2Device::getControls().\n> > - *\n> > - * The control's type along with the control's flags define the type of the\n> > - * control's value content. Controls can transport a single data value stored in\n> > - * variable inside the control, or they might as well deal with more complex\n> > - * data types, such as arrays of matrices, stored in a contiguous memory\n> > - * locations associated with the control and called 'the payload'. Such controls\n> > - * are called 'compound controls' and are currently not supported by the\n> > - * libcamera V4L2 control framework.\n> > - *\n> > - * libcamera implements support for controls using the V4L2 Extended Control\n> > - * API, which allows future handling of controls with payloads of arbitrary\n> > - * sizes.\n> > - *\n> > - * The libcamera V4L2 Controls framework operates on lists of controls, wrapped\n> > - * by the ControlList class, to match the V4L2 extended controls API. The\n> > - * interface to set and get control is implemented by the V4L2Device class, and\n> > - * this file only provides the data type definitions.\n> > - *\n> > - * \\todo Add support for compound controls\n> > - */\n> > -\n> > -namespace libcamera {\n> > -\n> > -namespace {\n> > -\n> > -std::string v4l2_ctrl_name(const struct v4l2_query_ext_ctrl &ctrl)\n> > -{\n> > -\tsize_t len = strnlen(ctrl.name, sizeof(ctrl.name));\n> > -\treturn std::string(static_cast(ctrl.name), len);\n> > -}\n> > -\n> > -ControlType v4l2_ctrl_type(const struct v4l2_query_ext_ctrl &ctrl)\n> > -{\n> > -\tswitch (ctrl.type) {\n> > -\tcase V4L2_CTRL_TYPE_U8:\n> > -\t\treturn ControlTypeByte;\n> > -\n> > -\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> > -\t\treturn ControlTypeBool;\n> > -\n> > -\tcase V4L2_CTRL_TYPE_INTEGER:\n> > -\t\treturn ControlTypeInteger32;\n> > -\n> > -\tcase V4L2_CTRL_TYPE_INTEGER64:\n> > -\t\treturn ControlTypeInteger64;\n> > -\n> > -\tcase V4L2_CTRL_TYPE_MENU:\n> > -\tcase V4L2_CTRL_TYPE_BUTTON:\n> > -\tcase V4L2_CTRL_TYPE_BITMASK:\n> > -\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n> > -\t\t/*\n> > -\t\t * More precise types may be needed, for now use a 32-bit\n> > -\t\t * integer type.\n> > -\t\t */\n> > -\t\treturn ControlTypeInteger32;\n> > -\n> > -\tdefault:\n> > -\t\treturn ControlTypeNone;\n> > -\t}\n> > -}\n> > -\n> > -} /* namespace */\n> > -\n> > -/**\n> > - * \\class V4L2ControlId\n> > - * \\brief V4L2 control static metadata\n> > - *\n> > - * The V4L2ControlId class is a specialisation of the ControlId for V4L2\n> > - * controls.\n> > - */\n> > -\n> > -/**\n> > - * \\brief Construct a V4L2ControlId from a struct v4l2_query_ext_ctrl\n> > - * \\param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the kernel\n> > - */\n> > -V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl)\n> > -\t: ControlId(ctrl.id, v4l2_ctrl_name(ctrl), v4l2_ctrl_type(ctrl))\n> > -{\n> > -}\n> > -\n> > -/**\n> > - * \\class V4L2ControlInfo\n> > - * \\brief Convenience specialisation of ControlInfo for V4L2 controls\n> > - *\n> > - * The V4L2ControlInfo class is a specialisation of the ControlInfo for V4L2\n> > - * controls. It offers a convenience constructor from a struct\n> > - * v4l2_query_ext_ctrl, and is otherwise equivalent to the ControlInfo class.\n> > - */\n> > -\n> > -/**\n> > - * \\brief Construct a V4L2ControlInfo from a struct v4l2_query_ext_ctrl\n> > - * \\param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the kernel\n> > - */\n> > -V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n> > -{\n> > -\tswitch (ctrl.type) {\n> > -\tcase V4L2_CTRL_TYPE_U8:\n> > -\t\tControlInfo::operator=(ControlInfo(static_cast(ctrl.minimum),\n> > -\t\t\t\t\t\t static_cast(ctrl.maximum),\n> > -\t\t\t\t\t\t static_cast(ctrl.default_value)));\n> > -\t\tbreak;\n> > -\n> > -\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> > -\t\tControlInfo::operator=(ControlInfo(static_cast(ctrl.minimum),\n> > -\t\t\t\t\t\t static_cast(ctrl.maximum),\n> > -\t\t\t\t\t\t static_cast(ctrl.default_value)));\n> > -\t\tbreak;\n> > -\n> > -\tcase V4L2_CTRL_TYPE_INTEGER64:\n> > -\t\tControlInfo::operator=(ControlInfo(static_cast(ctrl.minimum),\n> > -\t\t\t\t\t\t static_cast(ctrl.maximum),\n> > -\t\t\t\t\t\t static_cast(ctrl.default_value)));\n> > -\t\tbreak;\n> > -\n> > -\tdefault:\n> > -\t\tControlInfo::operator=(ControlInfo(static_cast(ctrl.minimum),\n> > -\t\t\t\t\t\t static_cast(ctrl.maximum),\n> > -\t\t\t\t\t\t static_cast(ctrl.default_value)));\n> > -\t\tbreak;\n> > -\t}\n> > -}\n> > -\n> > -} /* namespace libcamera */\n> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > index 397029ac..f9414043 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -20,7 +20,6 @@\n> > #include \"libcamera/internal/log.h\"\n> > #include \"libcamera/internal/sysfs.h\"\n> > #include \"libcamera/internal/utils.h\"\n> > -#include \"libcamera/internal/v4l2_controls.h\"\n> > \n> > /**\n> > * \\file v4l2_device.h\n> > @@ -31,6 +30,73 @@ namespace libcamera {\n> > \n> > LOG_DEFINE_CATEGORY(V4L2)\n> > \n> > +namespace {\n> > +\n> > +ControlType v4l2CtrlType(uint32_t ctrlType)\n> > +{\n> > +\tswitch (ctrlType) {\n> > +\tcase V4L2_CTRL_TYPE_U8:\n> > +\t\treturn ControlTypeByte;\n> > +\n> > +\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> > +\t\treturn ControlTypeBool;\n> > +\n> > +\tcase V4L2_CTRL_TYPE_INTEGER:\n> > +\t\treturn ControlTypeInteger32;\n> > +\n> > +\tcase V4L2_CTRL_TYPE_INTEGER64:\n> > +\t\treturn ControlTypeInteger64;\n> > +\n> > +\tcase V4L2_CTRL_TYPE_MENU:\n> > +\tcase V4L2_CTRL_TYPE_BUTTON:\n> > +\tcase V4L2_CTRL_TYPE_BITMASK:\n> > +\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n> > +\t\t/*\n> > +\t\t * More precise types may be needed, for now use a 32-bit\n> > +\t\t * integer type.\n> > +\t\t */\n> > +\t\treturn ControlTypeInteger32;\n> > +\n> > +\tdefault:\n> > +\t\treturn ControlTypeNone;\n> > +\t}\n> > +}\n> > +\n> > +std::unique_ptr v4l2ControlId(const v4l2_query_ext_ctrl &ctrl)\n> > +{\n> > +\tconst size_t len = strnlen(ctrl.name, sizeof(ctrl.name));\n> > +\tconst std::string name(static_cast(ctrl.name), len);\n> > +\tconst ControlType type = v4l2CtrlType(ctrl.type);\n> > +\n> > +\treturn std::make_unique(ctrl.id, name, type);\n> > +}\n> > +\n> > +ControlInfo v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl)\n> > +{\n> > +\tswitch (ctrl.type) {\n> > +\tcase V4L2_CTRL_TYPE_U8:\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> > +\t\t\t\t static_cast(ctrl.default_value));\n> > +\n> > +\tcase V4L2_CTRL_TYPE_INTEGER64:\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> > +\tdefault:\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> > +\t}\n> > +}\n> > +} /* namespace */\n> > +\n> > /**\n> > * \\class V4L2Device\n> > * \\brief Base class for V4L2VideoDevice and V4L2Subdevice\n> > @@ -502,10 +568,10 @@ void V4L2Device::listControls()\n> > \t\t\tcontinue;\n> > \t\t}\n> > \n> > -\t\tcontrolIds_.emplace_back(std::make_unique(ctrl));\n> > +\t\tcontrolIds_.emplace_back(v4l2ControlId(ctrl));\n> > \t\tcontrolInfo_.emplace(ctrl.id, ctrl);\n> > \n> > -\t\tctrls.emplace(controlIds_.back().get(), V4L2ControlInfo(ctrl));\n\nThere's another instance of V4L2ControlInfo in\nV4L2Device::updateControlInfo(). I think it can just be replace with\nv4l2ControlInfo(). If that's fine with you, I can handle this when\napplying.\n\n> > +\t\tctrls.emplace(controlIds_.back().get(), v4l2ControlInfo(ctrl));\n> > \t}\n> > \n> > \tcontrols_ = std::move(ctrls);","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 A0F3BC3200\n\tfor ;\n\tMon, 24 May 2021 04:20:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 065676891B;\n\tMon, 24 May 2021 06:20:06 +0200 (CEST)","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 771D5602AD\n\tfor ;\n\tMon, 24 May 2021 06:20:04 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E1EB5476;\n\tMon, 24 May 2021 06:20:03 +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=\"TR50S49X\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621830004;\n\tbh=U7DzmPWWv9Hk5fZdfVoq8koO8drsziYZ4BNZIHp3bWo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TR50S49XwmPll29b/BR6Pux6UG5RP4zfCPSxIXCGfJVNi5OTE8jjKxnz6xXw+CAhn\n\tMUemmNcjkD4BEZgKzqIh1HTI/r2uP/YamlrCeDjNGqpowMIIFIWfF20kCkzd+9D3lB\n\tfXg74UsTXYCgUwkfR4QHHh0yRrbiIjg1nkv+xskc=","Date":"Mon, 24 May 2021 07:20:00 +0300","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20210510054242.2735473-1-hiroh@chromium.org>\n\t<20210510054242.2735473-2-hiroh@chromium.org>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: V4L2Control: remove\n\tV4L2Control classes","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17166,"web_url":"https://patchwork.libcamera.org/comment/17166/","msgid":"","date":"2021-05-24T04:48:13","subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: V4L2Control: remove\n\tV4L2Control classes","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Laurent,\n\nOn Mon, May 24, 2021 at 1:20 PM Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Hiro,\n>\n> On Mon, May 24, 2021 at 06:57:03AM +0300, Laurent Pinchart wrote:\n> > On Mon, May 10, 2021 at 02:42:42PM +0900, Hirokazu Honda wrote:\n> > > V4L2ControlId and V4L2ControlInfo are just convenience classes to\n> > > create ControlId and ControlInfo from v4l2_query_ext_control.\n> > > Therefore, there is no need of being a class. It is used only\n> > > from V4L2Device. This removes the classes and put the equivalent\n> > > functions of creating ControlId and ControlInfo in\n> > > v4l2_device.cpp.\n> > >\n> > > Signed-off-by: Hirokazu Honda \n> > > Reviewed-by: Jacopo Mondi \n> > > ---\n> > > include/libcamera/internal/meson.build | 1 -\n> > > include/libcamera/internal/v4l2_controls.h | 31 -----\n> > > include/libcamera/internal/v4l2_device.h | 4 +-\n> > > src/libcamera/meson.build | 1 -\n> > > src/libcamera/v4l2_controls.cpp | 151 ---------------------\n> > > src/libcamera/v4l2_device.cpp | 72 +++++++++-\n> > > 6 files changed, 71 insertions(+), 189 deletions(-)\n> > > delete mode 100644 include/libcamera/internal/v4l2_controls.h\n> > > delete mode 100644 src/libcamera/v4l2_controls.cpp\n> >\n> > Nice diffstat :-)\n> >\n> > Reviewed-by: Laurent Pinchart \n> >\n> > > diff --git a/include/libcamera/internal/meson.build\n> b/include/libcamera/internal/meson.build\n> > > index 1fe3918c..3fb0216d 100644\n> > > --- a/include/libcamera/internal/meson.build\n> > > +++ b/include/libcamera/internal/meson.build\n> > > @@ -42,7 +42,6 @@ libcamera_internal_headers = files([\n> > > 'thread.h',\n> > > 'timer.h',\n> > > 'utils.h',\n> > > - 'v4l2_controls.h',\n> > > 'v4l2_device.h',\n> > > 'v4l2_pixelformat.h',\n> > > 'v4l2_subdevice.h',\n> > > diff --git a/include/libcamera/internal/v4l2_controls.h\n> b/include/libcamera/internal/v4l2_controls.h\n> > > deleted file mode 100644\n> > > index 0851b8dd..00000000\n> > > --- a/include/libcamera/internal/v4l2_controls.h\n> > > +++ /dev/null\n> > > @@ -1,31 +0,0 @@\n> > > -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > -/*\n> > > - * Copyright (C) 2019, Google Inc.\n> > > - *\n> > > - * v4l2_controls.h - V4L2 Controls Support\n> > > - */\n> > > -\n> > > -#ifndef __LIBCAMERA_INTERNAL_V4L2_CONTROLS_H__\n> > > -#define __LIBCAMERA_INTERNAL_V4L2_CONTROLS_H__\n> > > -\n> > > -#include \n> > > -\n> > > -#include \n> > > -\n> > > -namespace libcamera {\n> > > -\n> > > -class V4L2ControlId : public ControlId\n> > > -{\n> > > -public:\n> > > - V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl);\n> > > -};\n> > > -\n> > > -class V4L2ControlInfo : public ControlInfo\n> > > -{\n> > > -public:\n> > > - V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);\n> > > -};\n> > > -\n> > > -} /* namespace libcamera */\n> > > -\n> > > -#endif /* __LIBCAMERA_INTERNAL_V4L2_CONTROLS_H__ */\n> > > diff --git a/include/libcamera/internal/v4l2_device.h\n> b/include/libcamera/internal/v4l2_device.h\n> > > index 087f07e7..116bbbaf 100644\n> > > --- a/include/libcamera/internal/v4l2_device.h\n> > > +++ b/include/libcamera/internal/v4l2_device.h\n> > > @@ -13,11 +13,11 @@\n> > >\n> > > #include \n> > >\n> > > +#include \n> > > #include \n> > > #include \n> > >\n> > > #include \"libcamera/internal/log.h\"\n> > > -#include \"libcamera/internal/v4l2_controls.h\"\n> > >\n> > > namespace libcamera {\n> > >\n> > > @@ -61,7 +61,7 @@ private:\n> > > void eventAvailable(EventNotifier *notifier);\n> > >\n> > > std::map controlInfo_;\n> > > - std::vector> controlIds_;\n> > > + std::vector> controlIds_;\n> > > ControlInfoMap controls_;\n> > > std::string deviceNode_;\n> > > int fd_;\n> > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > > index e0a48aa2..62251a64 100644\n> > > --- a/src/libcamera/meson.build\n> > > +++ b/src/libcamera/meson.build\n> > > @@ -51,7 +51,6 @@ libcamera_sources = files([\n> > > 'timer.cpp',\n> > > 'transform.cpp',\n> > > 'utils.cpp',\n> > > - 'v4l2_controls.cpp',\n> > > 'v4l2_device.cpp',\n> > > 'v4l2_pixelformat.cpp',\n> > > 'v4l2_subdevice.cpp',\n> > > diff --git a/src/libcamera/v4l2_controls.cpp\n> b/src/libcamera/v4l2_controls.cpp\n> > > deleted file mode 100644\n> > > index 3f8ec6ca..00000000\n> > > --- a/src/libcamera/v4l2_controls.cpp\n> > > +++ /dev/null\n> > > @@ -1,151 +0,0 @@\n> > > -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > -/*\n> > > - * Copyright (C) 2019, Google Inc.\n> > > - *\n> > > - * v4l2_controls.cpp - V4L2 Controls Support\n> > > - */\n> > > -\n> > > -#include \"libcamera/internal/v4l2_controls.h\"\n> > > -\n> > > -#include \n> > > -\n> > > -/**\n> > > - * \\file v4l2_controls.h\n> > > - * \\brief Support for V4L2 Controls using the V4L2 Extended Controls\n> APIs\n> > > - *\n> > > - * The V4L2 Control API allows application to inspect and modify sets\n> of\n> > > - * configurable parameters on a video device or subdevice. The nature\n> of the\n> > > - * parameters an application can modify using the control framework\n> depends on\n> > > - * what the driver implements support for, and on the characteristics\n> of the\n> > > - * underlying hardware platform. Generally controls are used to\n> modify user\n> > > - * visible settings, such as the image brightness and exposure time,\n> or\n> > > - * non-standard parameters which cannot be controlled through the\n> V4L2 format\n> > > - * negotiation API.\n> > > - *\n> > > - * Controls are identified by a numerical ID, defined by the V4L2\n> kernel headers\n> > > - * and have an associated type. Each control has a value, which is\n> the data that\n> > > - * can be modified with V4L2Device::setControls() or retrieved with\n> > > - * V4L2Device::getControls().\n> > > - *\n> > > - * The control's type along with the control's flags define the type\n> of the\n> > > - * control's value content. Controls can transport a single data\n> value stored in\n> > > - * variable inside the control, or they might as well deal with more\n> complex\n> > > - * data types, such as arrays of matrices, stored in a contiguous\n> memory\n> > > - * locations associated with the control and called 'the payload'.\n> Such controls\n> > > - * are called 'compound controls' and are currently not supported by\n> the\n> > > - * libcamera V4L2 control framework.\n> > > - *\n> > > - * libcamera implements support for controls using the V4L2 Extended\n> Control\n> > > - * API, which allows future handling of controls with payloads of\n> arbitrary\n> > > - * sizes.\n> > > - *\n> > > - * The libcamera V4L2 Controls framework operates on lists of\n> controls, wrapped\n> > > - * by the ControlList class, to match the V4L2 extended controls API.\n> The\n> > > - * interface to set and get control is implemented by the V4L2Device\n> class, and\n> > > - * this file only provides the data type definitions.\n> > > - *\n> > > - * \\todo Add support for compound controls\n> > > - */\n> > > -\n> > > -namespace libcamera {\n> > > -\n> > > -namespace {\n> > > -\n> > > -std::string v4l2_ctrl_name(const struct v4l2_query_ext_ctrl &ctrl)\n> > > -{\n> > > - size_t len = strnlen(ctrl.name, sizeof(ctrl.name));\n> > > - return std::string(static_cast(ctrl.name), len);\n> > > -}\n> > > -\n> > > -ControlType v4l2_ctrl_type(const struct v4l2_query_ext_ctrl &ctrl)\n> > > -{\n> > > - switch (ctrl.type) {\n> > > - case V4L2_CTRL_TYPE_U8:\n> > > - return ControlTypeByte;\n> > > -\n> > > - case V4L2_CTRL_TYPE_BOOLEAN:\n> > > - return ControlTypeBool;\n> > > -\n> > > - case V4L2_CTRL_TYPE_INTEGER:\n> > > - return ControlTypeInteger32;\n> > > -\n> > > - case V4L2_CTRL_TYPE_INTEGER64:\n> > > - return ControlTypeInteger64;\n> > > -\n> > > - case V4L2_CTRL_TYPE_MENU:\n> > > - case V4L2_CTRL_TYPE_BUTTON:\n> > > - case V4L2_CTRL_TYPE_BITMASK:\n> > > - case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > > - /*\n> > > - * More precise types may be needed, for now use a 32-bit\n> > > - * integer type.\n> > > - */\n> > > - return ControlTypeInteger32;\n> > > -\n> > > - default:\n> > > - return ControlTypeNone;\n> > > - }\n> > > -}\n> > > -\n> > > -} /* namespace */\n> > > -\n> > > -/**\n> > > - * \\class V4L2ControlId\n> > > - * \\brief V4L2 control static metadata\n> > > - *\n> > > - * The V4L2ControlId class is a specialisation of the ControlId for\n> V4L2\n> > > - * controls.\n> > > - */\n> > > -\n> > > -/**\n> > > - * \\brief Construct a V4L2ControlId from a struct v4l2_query_ext_ctrl\n> > > - * \\param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the\n> kernel\n> > > - */\n> > > -V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl)\n> > > - : ControlId(ctrl.id, v4l2_ctrl_name(ctrl), v4l2_ctrl_type(ctrl))\n> > > -{\n> > > -}\n> > > -\n> > > -/**\n> > > - * \\class V4L2ControlInfo\n> > > - * \\brief Convenience specialisation of ControlInfo for V4L2 controls\n> > > - *\n> > > - * The V4L2ControlInfo class is a specialisation of the ControlInfo\n> for V4L2\n> > > - * controls. It offers a convenience constructor from a struct\n> > > - * v4l2_query_ext_ctrl, and is otherwise equivalent to the\n> ControlInfo class.\n> > > - */\n> > > -\n> > > -/**\n> > > - * \\brief Construct a V4L2ControlInfo from a struct\n> v4l2_query_ext_ctrl\n> > > - * \\param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the\n> kernel\n> > > - */\n> > > -V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl\n> &ctrl)\n> > > -{\n> > > - switch (ctrl.type) {\n> > > - case V4L2_CTRL_TYPE_U8:\n> > > -\n> ControlInfo::operator=(ControlInfo(static_cast(ctrl.minimum),\n> > > -\n> static_cast(ctrl.maximum),\n> > > -\n> static_cast(ctrl.default_value)));\n> > > - break;\n> > > -\n> > > - case V4L2_CTRL_TYPE_BOOLEAN:\n> > > -\n> ControlInfo::operator=(ControlInfo(static_cast(ctrl.minimum),\n> > > -\n> static_cast(ctrl.maximum),\n> > > -\n> static_cast(ctrl.default_value)));\n> > > - break;\n> > > -\n> > > - case V4L2_CTRL_TYPE_INTEGER64:\n> > > -\n> ControlInfo::operator=(ControlInfo(static_cast(ctrl.minimum),\n> > > -\n> static_cast(ctrl.maximum),\n> > > -\n> static_cast(ctrl.default_value)));\n> > > - break;\n> > > -\n> > > - default:\n> > > -\n> ControlInfo::operator=(ControlInfo(static_cast(ctrl.minimum),\n> > > -\n> static_cast(ctrl.maximum),\n> > > -\n> static_cast(ctrl.default_value)));\n> > > - break;\n> > > - }\n> > > -}\n> > > -\n> > > -} /* namespace libcamera */\n> > > diff --git a/src/libcamera/v4l2_device.cpp\n> b/src/libcamera/v4l2_device.cpp\n> > > index 397029ac..f9414043 100644\n> > > --- a/src/libcamera/v4l2_device.cpp\n> > > +++ b/src/libcamera/v4l2_device.cpp\n> > > @@ -20,7 +20,6 @@\n> > > #include \"libcamera/internal/log.h\"\n> > > #include \"libcamera/internal/sysfs.h\"\n> > > #include \"libcamera/internal/utils.h\"\n> > > -#include \"libcamera/internal/v4l2_controls.h\"\n> > >\n> > > /**\n> > > * \\file v4l2_device.h\n> > > @@ -31,6 +30,73 @@ namespace libcamera {\n> > >\n> > > LOG_DEFINE_CATEGORY(V4L2)\n> > >\n> > > +namespace {\n> > > +\n> > > +ControlType v4l2CtrlType(uint32_t ctrlType)\n> > > +{\n> > > + switch (ctrlType) {\n> > > + case V4L2_CTRL_TYPE_U8:\n> > > + return ControlTypeByte;\n> > > +\n> > > + case V4L2_CTRL_TYPE_BOOLEAN:\n> > > + return ControlTypeBool;\n> > > +\n> > > + case V4L2_CTRL_TYPE_INTEGER:\n> > > + return ControlTypeInteger32;\n> > > +\n> > > + case V4L2_CTRL_TYPE_INTEGER64:\n> > > + return ControlTypeInteger64;\n> > > +\n> > > + case V4L2_CTRL_TYPE_MENU:\n> > > + case V4L2_CTRL_TYPE_BUTTON:\n> > > + case V4L2_CTRL_TYPE_BITMASK:\n> > > + case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > > + /*\n> > > + * More precise types may be needed, for now use a 32-bit\n> > > + * integer type.\n> > > + */\n> > > + return ControlTypeInteger32;\n> > > +\n> > > + default:\n> > > + return ControlTypeNone;\n> > > + }\n> > > +}\n> > > +\n> > > +std::unique_ptr v4l2ControlId(const v4l2_query_ext_ctrl\n> &ctrl)\n> > > +{\n> > > + const size_t len = strnlen(ctrl.name, sizeof(ctrl.name));\n> > > + const std::string name(static_cast(ctrl.name), len);\n> > > + const ControlType type = v4l2CtrlType(ctrl.type);\n> > > +\n> > > + return std::make_unique(ctrl.id, name, type);\n> > > +}\n> > > +\n> > > +ControlInfo v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl)\n> > > +{\n> > > + switch (ctrl.type) {\n> > > + case V4L2_CTRL_TYPE_U8:\n> > > + return ControlInfo(static_cast(ctrl.minimum),\n> > > + static_cast(ctrl.maximum),\n> > > +\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> > > + static_cast(ctrl.default_value));\n> > > +\n> > > + case V4L2_CTRL_TYPE_INTEGER64:\n> > > + return ControlInfo(static_cast(ctrl.minimum),\n> > > + static_cast(ctrl.maximum),\n> > > +\n> static_cast(ctrl.default_value));\n> > > +\n> > > + default:\n> > > + return ControlInfo(static_cast(ctrl.minimum),\n> > > + static_cast(ctrl.maximum),\n> > > +\n> static_cast(ctrl.default_value));\n> > > + }\n> > > +}\n> > > +} /* namespace */\n> > > +\n> > > /**\n> > > * \\class V4L2Device\n> > > * \\brief Base class for V4L2VideoDevice and V4L2Subdevice\n> > > @@ -502,10 +568,10 @@ void V4L2Device::listControls()\n> > > continue;\n> > > }\n> > >\n> > > -\n> controlIds_.emplace_back(std::make_unique(ctrl));\n> > > + controlIds_.emplace_back(v4l2ControlId(ctrl));\n> > > controlInfo_.emplace(ctrl.id, ctrl);\n> > >\n> > > - ctrls.emplace(controlIds_.back().get(),\n> V4L2ControlInfo(ctrl));\n>\n> There's another instance of V4L2ControlInfo in\n> V4L2Device::updateControlInfo(). I think it can just be replace with\n> v4l2ControlInfo(). If that's fine with you, I can handle this when\n> applying.\n>\n>\nIndeed. Thanks in advance. I have no idea why I haven't caught it when\ncreating this pacth. :(\n-Hiro\n\n\n> > > + ctrls.emplace(controlIds_.back().get(),\n> v4l2ControlInfo(ctrl));\n> > > }\n> > >\n> > > controls_ = std::move(ctrls);\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 B39C2C3200\n\tfor ;\n\tMon, 24 May 2021 04:48:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1BE076891E;\n\tMon, 24 May 2021 06:48:26 +0200 (CEST)","from mail-ej1-x636.google.com (mail-ej1-x636.google.com\n\t[IPv6:2a00:1450:4864:20::636])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C6A75602AD\n\tfor ;\n\tMon, 24 May 2021 06:48:24 +0200 (CEST)","by mail-ej1-x636.google.com with SMTP id et19so32654269ejc.4\n\tfor ;\n\tSun, 23 May 2021 21:48: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=\"MpvY7znL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=2i077/rqtLUlvOhjDEnS3k18n4dFPNKHyDohZKeUJp4=;\n\tb=MpvY7znLVcfLMuSUW7ticwidDAVTwLY0TmgSM4XsFp70KSRHlheLlKX4u9FzpVUiGf\n\t4j2vpTsoAM3LYfDxmGkMXg5oPwthMCkrlu+6pbquExQJ1JZTxtlAUhinHIUC+KjASSAl\n\t8iDW2cHtqQ317oblYvKSg6RB5YtmcOI4Z4Xps=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=2i077/rqtLUlvOhjDEnS3k18n4dFPNKHyDohZKeUJp4=;\n\tb=KwHzgyA17gWZeJ85lBFsEGWZMMTjfxR2aeZkBrtnyqig50+zozo0fSSz/p5DgeiGRU\n\tXzfexzuzZH/KO7bKN6GnXkfCsdc+A2WMGtlAcFKEv4ynjo8PK5s4VOBRXNMLRIpI7gm4\n\t7fVHq0DJA1hbtm+zsI18LrVTFQx55eLcddutI2cO9lU19D0dOeKWr7lh4t/4RdSCHaGJ\n\tsxA8DtIH+1feSN2yudt8bFOdMCFSBkoSOJDicxKJupdU6vQJAGK751wJMoSzeKKLH6VW\n\tVK4UWndPSrFeQswo74ePc8RmxXGpCEQEUpubrbsn/FXe+1RSSq7BSleyvDnk5wQtXxKe\n\t19RQ==","X-Gm-Message-State":"AOAM533RJMMS13o55k+eqqK+a3fjOTx7/xAx7hlhBAbe6IghrYaGeJ3p\n\tjSzUOQG3Qk4wPkTOCJaJALl3ekxIlGHqb9yM3kHIKDMQLcc=","X-Google-Smtp-Source":"ABdhPJzBQ2F1LZGwJfJO/skXiKMMbtuHW09f6ceeahdTJnSUkplRTIcQx6+5utRtpD8Jp8wsb4Zz4K+rtv+GrvAjWLc=","X-Received":"by 2002:a17:906:4d44:: with SMTP id\n\tb4mr20803657ejv.306.1621831704419; \n\tSun, 23 May 2021 21:48:24 -0700 (PDT)","MIME-Version":"1.0","References":"<20210510054242.2735473-1-hiroh@chromium.org>\n\t<20210510054242.2735473-2-hiroh@chromium.org>\n\t\n\t","In-Reply-To":"","From":"Hirokazu Honda ","Date":"Mon, 24 May 2021 13:48:13 +0900","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000a0b0ba05c30c1e18\"","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: V4L2Control: remove\n\tV4L2Control classes","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]