From patchwork Sun Sep 29 19:02:42 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2056 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id ABC8461658 for ; Sun, 29 Sep 2019 21:03:11 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 3FDC9813 for ; Sun, 29 Sep 2019 21:03:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1569783791; bh=KK+0eSmP/pMHFJ6+5V3Be0Nf9i8zT7umKsLfBsu3y3s=; h=From:To:Subject:Date:In-Reply-To:References:From; b=SFLp6HMHPjXtIQNVq97FdzqVUeaWcSdE+3E974HMYXudZAlD2KJM0237Hrj/wRF2j LSvBX2JZHyl/5zKEXOf2cgauiLcR3c0xjFO1mB3ry2LnNMoh6dKu6PZrcdCFWQzi93 BaEBrQu2AkDg+6X203NxzY5sCEtTR231zx6C6puw= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 29 Sep 2019 22:02:42 +0300 Message-Id: <20190929190254.18920-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> References: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 01/13] libcamera: controls: Rename ControlValueType to ControlType X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Sep 2019 19:03:11 -0000 The type of a control value is also the type of the control. Shorten the ControlValueType enumeration to ControlType, and rename ControlValue* to ControlType* for better clarity. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/libcamera/controls.h | 20 +++++++------- src/libcamera/controls.cpp | 50 +++++++++++++++++----------------- src/libcamera/gen-controls.awk | 2 +- test/controls/control_info.cpp | 4 +-- 4 files changed, 38 insertions(+), 38 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index fbb3a62274c6..ffba880a66ff 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -18,11 +18,11 @@ namespace libcamera { class Camera; -enum ControlValueType { - ControlValueNone, - ControlValueBool, - ControlValueInteger, - ControlValueInteger64, +enum ControlType { + ControlTypeNone, + ControlTypeBool, + ControlTypeInteger, + ControlTypeInteger64, }; class ControlValue @@ -33,8 +33,8 @@ public: ControlValue(int value); ControlValue(int64_t value); - ControlValueType type() const { return type_; }; - bool isNone() const { return type_ == ControlValueNone; }; + ControlType type() const { return type_; }; + bool isNone() const { return type_ == ControlTypeNone; }; void set(bool value); void set(int value); @@ -47,7 +47,7 @@ public: std::string toString() const; private: - ControlValueType type_; + ControlType type_; union { bool bool_; @@ -59,7 +59,7 @@ private: struct ControlIdentifier { ControlId id; const char *name; - ControlValueType type; + ControlType type; }; class ControlInfo @@ -70,7 +70,7 @@ public: ControlId id() const { return ident_->id; } const char *name() const { return ident_->name; } - ControlValueType type() const { return ident_->type; } + ControlType type() const { return ident_->type; } const ControlValue &min() const { return min_; } const ControlValue &max() const { return max_; } diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 727fdbd9450d..9960a30dfa03 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -25,16 +25,16 @@ namespace libcamera { LOG_DEFINE_CATEGORY(Controls) /** - * \enum ControlValueType - * \brief Define the data type of value represented by a ControlValue - * \var ControlValueNone - * Identifies an unset control value - * \var ControlValueBool - * Identifies controls storing a boolean value - * \var ControlValueInteger - * Identifies controls storing an integer value - * \var ControlValueInteger64 - * Identifies controls storing a 64-bit integer value + * \enum ControlType + * \brief Define the data type of a Control + * \var ControlTypeNone + * Invalid type, for empty values + * \var ControlTypeBool + * The control stores a boolean value + * \var ControlTypeInteger + * The control stores an integer value + * \var ControlTypeInteger64 + * The control stores a 64-bit integer value */ /** @@ -46,7 +46,7 @@ LOG_DEFINE_CATEGORY(Controls) * \brief Construct an empty ControlValue. */ ControlValue::ControlValue() - : type_(ControlValueNone) + : type_(ControlTypeNone) { } @@ -55,7 +55,7 @@ ControlValue::ControlValue() * \param[in] value Boolean value to store */ ControlValue::ControlValue(bool value) - : type_(ControlValueBool), bool_(value) + : type_(ControlTypeBool), bool_(value) { } @@ -64,7 +64,7 @@ ControlValue::ControlValue(bool value) * \param[in] value Integer value to store */ ControlValue::ControlValue(int value) - : type_(ControlValueInteger), integer_(value) + : type_(ControlTypeInteger), integer_(value) { } @@ -73,7 +73,7 @@ ControlValue::ControlValue(int value) * \param[in] value Integer value to store */ ControlValue::ControlValue(int64_t value) - : type_(ControlValueInteger64), integer64_(value) + : type_(ControlTypeInteger64), integer64_(value) { } @@ -86,7 +86,7 @@ ControlValue::ControlValue(int64_t value) /** * \fn ControlValue::isNone() * \brief Determine if the value is not initialised - * \return True if the value type is ControlValueNone, false otherwise + * \return True if the value type is ControlTypeNone, false otherwise */ /** @@ -95,7 +95,7 @@ ControlValue::ControlValue(int64_t value) */ void ControlValue::set(bool value) { - type_ = ControlValueBool; + type_ = ControlTypeBool; bool_ = value; } @@ -105,7 +105,7 @@ void ControlValue::set(bool value) */ void ControlValue::set(int value) { - type_ = ControlValueInteger; + type_ = ControlTypeInteger; integer_ = value; } @@ -115,7 +115,7 @@ void ControlValue::set(int value) */ void ControlValue::set(int64_t value) { - type_ = ControlValueInteger64; + type_ = ControlTypeInteger64; integer64_ = value; } @@ -128,7 +128,7 @@ void ControlValue::set(int64_t value) */ bool ControlValue::getBool() const { - ASSERT(type_ == ControlValueBool); + ASSERT(type_ == ControlTypeBool); return bool_; } @@ -142,7 +142,7 @@ bool ControlValue::getBool() const */ int ControlValue::getInt() const { - ASSERT(type_ == ControlValueInteger || type_ == ControlValueInteger64); + ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64); return integer_; } @@ -156,7 +156,7 @@ int ControlValue::getInt() const */ int64_t ControlValue::getInt64() const { - ASSERT(type_ == ControlValueInteger || type_ == ControlValueInteger64); + ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64); return integer64_; } @@ -168,13 +168,13 @@ int64_t ControlValue::getInt64() const std::string ControlValue::toString() const { switch (type_) { - case ControlValueNone: + case ControlTypeNone: return ""; - case ControlValueBool: + case ControlTypeBool: return bool_ ? "True" : "False"; - case ControlValueInteger: + case ControlTypeInteger: return std::to_string(integer_); - case ControlValueInteger64: + case ControlTypeInteger64: return std::to_string(integer64_); } diff --git a/src/libcamera/gen-controls.awk b/src/libcamera/gen-controls.awk index f3d068123012..a3f291e7071c 100755 --- a/src/libcamera/gen-controls.awk +++ b/src/libcamera/gen-controls.awk @@ -92,7 +92,7 @@ function GenerateTable(file) { print "extern const std::unordered_map" > file print "controlTypes {" > file for (i=1; i <= id; ++i) { - printf "\t{ %s, { %s, \"%s\", ControlValue%s } },\n", names[i], names[i], names[i], types[i] > file + printf "\t{ %s, { %s, \"%s\", ControlType%s } },\n", names[i], names[i], names[i], types[i] > file } print "};" > file ExitNameSpace(file) diff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp index aa3a65b1e5ef..8cda860b9fe9 100644 --- a/test/controls/control_info.cpp +++ b/test/controls/control_info.cpp @@ -26,7 +26,7 @@ protected: ControlInfo info(Brightness); if (info.id() != Brightness || - info.type() != ControlValueInteger || + info.type() != ControlTypeInteger || info.name() != std::string("Brightness")) { cout << "Invalid control identification for Brightness" << endl; return TestFail; @@ -44,7 +44,7 @@ protected: info = ControlInfo(Contrast, 10, 200); if (info.id() != Contrast || - info.type() != ControlValueInteger || + info.type() != ControlTypeInteger || info.name() != std::string("Contrast")) { cout << "Invalid control identification for Contrast" << endl; return TestFail; From patchwork Sun Sep 29 19:02:43 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2057 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E5C4061658 for ; Sun, 29 Sep 2019 21:03:11 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8EF80320 for ; Sun, 29 Sep 2019 21:03:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1569783791; bh=IS8zhd8EgSCr/AQhO9yBmZh03KUS4S/4OXtbxmdAjrM=; h=From:To:Subject:Date:In-Reply-To:References:From; b=gzVS+qMksNHg52S5vFTApg25sK06u1JBS4Aiu92rveFLHMPDjdWo1yw0KQLEtXzbd KZwJGC1FtMjO15yJy/WtIkiPd1fMy9nkr4sEtyrhJa88zfcETs3UNn3yICVwcoQ310 1A9WDvjCtSrwxGHCympkI/caJKGA/y8/Z92Z33pQ= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 29 Sep 2019 22:02:43 +0300 Message-Id: <20190929190254.18920-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> References: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 02/13] libcamera: controls: Make ControlValue get/set accessors template methods X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Sep 2019 19:03:12 -0000 The ControlValue get accessors are implemented with functions of different names, whlie the set accessors use polymorphism to support different control types. This isn't very consistent and intuitive. Make the API clearer by using template methods. This will also have the added advantage that support for the new types will only require adding template specialisations, without adding new methods. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/libcamera/controls.h | 11 ++- src/libcamera/controls.cpp | 101 +++++++++++++--------------- src/libcamera/pipeline/uvcvideo.cpp | 10 +-- src/libcamera/pipeline/vimc.cpp | 6 +- test/controls/control_info.cpp | 4 +- test/controls/control_list.cpp | 16 ++--- test/controls/control_value.cpp | 12 ++-- 7 files changed, 74 insertions(+), 86 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index ffba880a66ff..0886370f0901 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -36,13 +36,10 @@ public: ControlType type() const { return type_; }; bool isNone() const { return type_ == ControlTypeNone; }; - void set(bool value); - void set(int value); - void set(int64_t value); - - bool getBool() const; - int getInt() const; - int64_t getInt64() const; + template + const T &get() const; + template + void set(const T &value); std::string toString() const; diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 9960a30dfa03..88aab88db327 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -90,76 +90,67 @@ ControlValue::ControlValue(int64_t value) */ /** - * \brief Set the value with a boolean - * \param[in] value Boolean value to store + * \fn template const T &ControlValue::get() const + * \brief Get the control value + * + * The control value type shall match the type T, otherwise the behaviour is + * undefined. + * + * \return The control value */ -void ControlValue::set(bool value) + +/** + * \fn template void ControlValue::set(const T &value) + * \brief Set the control value to \a value + * \param[in] value The control value + */ + +#ifndef __DOXYGEN__ +template<> +const bool &ControlValue::get() const +{ + ASSERT(type_ == ControlTypeBool); + + return bool_; +} + +template<> +const int &ControlValue::get() const +{ + ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64); + + return integer_; +} + +template<> +const int64_t &ControlValue::get() const +{ + ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64); + + return integer64_; +} + +template<> +void ControlValue::set(const bool &value) { type_ = ControlTypeBool; bool_ = value; } -/** - * \brief Set the value with an integer - * \param[in] value Integer value to store - */ -void ControlValue::set(int value) +template<> +void ControlValue::set(const int &value) { type_ = ControlTypeInteger; integer_ = value; } -/** - * \brief Set the value with a 64 bit integer - * \param[in] value 64 bit integer value to store - */ -void ControlValue::set(int64_t value) +template<> +void ControlValue::set(const int64_t &value) { type_ = ControlTypeInteger64; integer64_ = value; } - -/** - * \brief Get the boolean value - * - * The value type must be Boolean. - * - * \return The boolean value - */ -bool ControlValue::getBool() const -{ - ASSERT(type_ == ControlTypeBool); - - return bool_; -} - -/** - * \brief Get the integer value - * - * The value type must be Integer or Integer64. - * - * \return The integer value - */ -int ControlValue::getInt() const -{ - ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64); - - return integer_; -} - -/** - * \brief Get the 64-bit integer value - * - * The value type must be Integer or Integer64. - * - * \return The 64-bit integer value - */ -int64_t ControlValue::getInt64() const -{ - ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64); - - return integer64_; -} +#endif /* __DOXYGEN__ */ /** * \brief Assemble and return a string describing the value diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 8965210550d2..81c548af2c64 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -235,24 +235,24 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) switch (ci->id()) { case Brightness: - controls.add(V4L2_CID_BRIGHTNESS, value.getInt()); + controls.add(V4L2_CID_BRIGHTNESS, value.get()); break; case Contrast: - controls.add(V4L2_CID_CONTRAST, value.getInt()); + controls.add(V4L2_CID_CONTRAST, value.get()); break; case Saturation: - controls.add(V4L2_CID_SATURATION, value.getInt()); + controls.add(V4L2_CID_SATURATION, value.get()); break; case ManualExposure: controls.add(V4L2_CID_EXPOSURE_AUTO, 1); - controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.getInt()); + controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get()); break; case ManualGain: - controls.add(V4L2_CID_GAIN, value.getInt()); + controls.add(V4L2_CID_GAIN, value.get()); break; default: diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index f26a91f86ec1..3e34e7a0edbf 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -288,15 +288,15 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) switch (ci->id()) { case Brightness: - controls.add(V4L2_CID_BRIGHTNESS, value.getInt()); + controls.add(V4L2_CID_BRIGHTNESS, value.get()); break; case Contrast: - controls.add(V4L2_CID_CONTRAST, value.getInt()); + controls.add(V4L2_CID_CONTRAST, value.get()); break; case Saturation: - controls.add(V4L2_CID_SATURATION, value.getInt()); + controls.add(V4L2_CID_SATURATION, value.get()); break; default: diff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp index 8cda860b9fe9..faefaaa444d9 100644 --- a/test/controls/control_info.cpp +++ b/test/controls/control_info.cpp @@ -32,7 +32,7 @@ protected: return TestFail; } - if (info.min().getInt() != 0 || info.max().getInt() != 0) { + if (info.min().get() != 0 || info.max().get() != 0) { cout << "Invalid control range for Brightness" << endl; return TestFail; } @@ -50,7 +50,7 @@ protected: return TestFail; } - if (info.min().getInt() != 10 || info.max().getInt() != 200) { + if (info.min().get() != 10 || info.max().get() != 200) { cout << "Invalid control range for Contrast" << endl; return TestFail; } diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp index f1d79ff8fcfd..0402e7c23dba 100644 --- a/test/controls/control_list.cpp +++ b/test/controls/control_list.cpp @@ -96,7 +96,7 @@ protected: return TestFail; } - if (list[Brightness].getInt() != 255) { + if (list[Brightness].get() != 255) { cout << "Incorrest Brightness control value" << endl; return TestFail; } @@ -125,8 +125,8 @@ protected: /* * Test control value retrieval and update through ControlInfo. */ - if (list[brightness].getInt() != 64 || - list[contrast].getInt() != 128) { + if (list[brightness].get() != 64 || + list[contrast].get() != 128) { cout << "Failed to retrieve control value" << endl; return TestFail; } @@ -134,8 +134,8 @@ protected: list[brightness] = 10; list[contrast] = 20; - if (list[brightness].getInt() != 10 || - list[contrast].getInt() != 20) { + if (list[brightness].get() != 10 || + list[contrast].get() != 20) { cout << "Failed to update control value" << endl; return TestFail; } @@ -185,9 +185,9 @@ protected: return TestFail; } - if (newList[Brightness].getInt() != 10 || - newList[Contrast].getInt() != 20 || - newList[Saturation].getInt() != 255) { + if (newList[Brightness].get() != 10 || + newList[Contrast].get() != 20 || + newList[Saturation].get() != 255) { cout << "New list contains incorrect values" << endl; return TestFail; } diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp index 778efe5c115f..397c43f799ad 100644 --- a/test/controls/control_value.cpp +++ b/test/controls/control_value.cpp @@ -27,12 +27,12 @@ protected: << " Bool: " << boolean.toString() << endl; - if (integer.getInt() != 1234) { + if (integer.get() != 1234) { cerr << "Failed to get Integer" << endl; return TestFail; } - if (boolean.getBool() != true) { + if (boolean.get() != true) { cerr << "Failed to get Boolean" << endl; return TestFail; } @@ -45,19 +45,19 @@ protected: return TestFail; } - value.set(true); + value.set(true); if (value.isNone()) { cerr << "Failed to set an empty object" << endl; return TestFail; } - if (value.getBool() != true) { + if (value.get() != true) { cerr << "Failed to get Booleans" << endl; return TestFail; } - value.set(10); - if (value.getInt() != 10) { + value.set(10); + if (value.get() != 10) { cerr << "Failed to get Integer" << endl; return TestFail; } From patchwork Sun Sep 29 19:02:44 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2058 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 49FFF6165A for ; Sun, 29 Sep 2019 21:03:12 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id DEA13813 for ; Sun, 29 Sep 2019 21:03:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1569783792; bh=icoU5fJP8tsRcqI8mMUhffNRrhHR6pzlmD3k1yu5z9I=; h=From:To:Subject:Date:In-Reply-To:References:From; b=wgXPzB3wl9QmBnbZZ4IHxu64UjqRShFXLdV1zyhRX9gnqQQcrZfNcDk6qCBrUo/fm lqq/AHLcSYjGFgUmnAusazcBXQ1+zA+4YxqRqXg1ZFiq+/6nilcDj/wtVFN0W7Xxbf xvWAoEoNHr8TpwkX/Clt+1C+cJFV8jVR2s5VTkos= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 29 Sep 2019 22:02:44 +0300 Message-Id: <20190929190254.18920-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> References: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 03/13] libcamera: controls: Use explicit 32-bit integer types X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Sep 2019 19:03:12 -0000 Make the control API more explicit when dealing with integer controls by specifying the size. We already do so for 64-bit integers, using int64_t and ControlTypeInteger64, do the same for 32-bit integers. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/libcamera/controls.h | 6 ++--- src/libcamera/controls.cpp | 36 ++++++++++++++--------------- src/libcamera/pipeline/uvcvideo.cpp | 10 ++++---- src/libcamera/pipeline/vimc.cpp | 6 ++--- test/controls/control_info.cpp | 10 ++++---- test/controls/control_list.cpp | 16 ++++++------- test/controls/control_value.cpp | 6 ++--- 7 files changed, 46 insertions(+), 44 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 0886370f0901..a3089064c4b5 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -21,7 +21,7 @@ class Camera; enum ControlType { ControlTypeNone, ControlTypeBool, - ControlTypeInteger, + ControlTypeInteger32, ControlTypeInteger64, }; @@ -30,7 +30,7 @@ class ControlValue public: ControlValue(); ControlValue(bool value); - ControlValue(int value); + ControlValue(int32_t value); ControlValue(int64_t value); ControlType type() const { return type_; }; @@ -48,7 +48,7 @@ private: union { bool bool_; - int integer_; + int32_t integer32_; int64_t integer64_; }; }; diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 88aab88db327..295ccd55a622 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -31,8 +31,8 @@ LOG_DEFINE_CATEGORY(Controls) * Invalid type, for empty values * \var ControlTypeBool * The control stores a boolean value - * \var ControlTypeInteger - * The control stores an integer value + * \var ControlTypeInteger32 + * The control stores a 32-bit integer value * \var ControlTypeInteger64 * The control stores a 64-bit integer value */ @@ -63,8 +63,8 @@ ControlValue::ControlValue(bool value) * \brief Construct an integer ControlValue * \param[in] value Integer value to store */ -ControlValue::ControlValue(int value) - : type_(ControlTypeInteger), integer_(value) +ControlValue::ControlValue(int32_t value) + : type_(ControlTypeInteger32), integer32_(value) { } @@ -115,17 +115,17 @@ const bool &ControlValue::get() const } template<> -const int &ControlValue::get() const +const int32_t &ControlValue::get() const { - ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64); + ASSERT(type_ == ControlTypeInteger32 || type_ == ControlTypeInteger64); - return integer_; + return integer32_; } template<> const int64_t &ControlValue::get() const { - ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64); + ASSERT(type_ == ControlTypeInteger32 || type_ == ControlTypeInteger64); return integer64_; } @@ -138,10 +138,10 @@ void ControlValue::set(const bool &value) } template<> -void ControlValue::set(const int &value) +void ControlValue::set(const int32_t &value) { - type_ = ControlTypeInteger; - integer_ = value; + type_ = ControlTypeInteger32; + integer32_ = value; } template<> @@ -163,8 +163,8 @@ std::string ControlValue::toString() const return ""; case ControlTypeBool: return bool_ ? "True" : "False"; - case ControlTypeInteger: - return std::to_string(integer_); + case ControlTypeInteger32: + return std::to_string(integer32_); case ControlTypeInteger64: return std::to_string(integer64_); } @@ -186,35 +186,35 @@ std::string ControlValue::toString() const /** * \var Brightness - * ControlType: Integer + * ControlType: Integer32 * * Specify a fixed brightness parameter. */ /** * \var Contrast - * ControlType: Integer + * ControlType: Integer32 * * Specify a fixed contrast parameter. */ /** * \var Saturation - * ControlType: Integer + * ControlType: Integer32 * * Specify a fixed saturation parameter. */ /** * \var ManualExposure - * ControlType: Integer + * ControlType: Integer32 * * Specify a fixed exposure time in milli-seconds */ /** * \var ManualGain - * ControlType: Integer + * ControlType: Integer32 * * Specify a fixed gain parameter */ diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 81c548af2c64..0d56758e3e1d 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -235,24 +235,24 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) switch (ci->id()) { case Brightness: - controls.add(V4L2_CID_BRIGHTNESS, value.get()); + controls.add(V4L2_CID_BRIGHTNESS, value.get()); break; case Contrast: - controls.add(V4L2_CID_CONTRAST, value.get()); + controls.add(V4L2_CID_CONTRAST, value.get()); break; case Saturation: - controls.add(V4L2_CID_SATURATION, value.get()); + controls.add(V4L2_CID_SATURATION, value.get()); break; case ManualExposure: controls.add(V4L2_CID_EXPOSURE_AUTO, 1); - controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get()); + controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get()); break; case ManualGain: - controls.add(V4L2_CID_GAIN, value.get()); + controls.add(V4L2_CID_GAIN, value.get()); break; default: diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 3e34e7a0edbf..e549dc64b996 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -288,15 +288,15 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) switch (ci->id()) { case Brightness: - controls.add(V4L2_CID_BRIGHTNESS, value.get()); + controls.add(V4L2_CID_BRIGHTNESS, value.get()); break; case Contrast: - controls.add(V4L2_CID_CONTRAST, value.get()); + controls.add(V4L2_CID_CONTRAST, value.get()); break; case Saturation: - controls.add(V4L2_CID_SATURATION, value.get()); + controls.add(V4L2_CID_SATURATION, value.get()); break; default: diff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp index faefaaa444d9..2aba568a302e 100644 --- a/test/controls/control_info.cpp +++ b/test/controls/control_info.cpp @@ -26,13 +26,14 @@ protected: ControlInfo info(Brightness); if (info.id() != Brightness || - info.type() != ControlTypeInteger || + info.type() != ControlTypeInteger32 || info.name() != std::string("Brightness")) { cout << "Invalid control identification for Brightness" << endl; return TestFail; } - if (info.min().get() != 0 || info.max().get() != 0) { + if (info.min().get() != 0 || + info.max().get() != 0) { cout << "Invalid control range for Brightness" << endl; return TestFail; } @@ -44,13 +45,14 @@ protected: info = ControlInfo(Contrast, 10, 200); if (info.id() != Contrast || - info.type() != ControlTypeInteger || + info.type() != ControlTypeInteger32 || info.name() != std::string("Contrast")) { cout << "Invalid control identification for Contrast" << endl; return TestFail; } - if (info.min().get() != 10 || info.max().get() != 200) { + if (info.min().get() != 10 || + info.max().get() != 200) { cout << "Invalid control range for Contrast" << endl; return TestFail; } diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp index 0402e7c23dba..5215840b1c4e 100644 --- a/test/controls/control_list.cpp +++ b/test/controls/control_list.cpp @@ -96,7 +96,7 @@ protected: return TestFail; } - if (list[Brightness].get() != 255) { + if (list[Brightness].get() != 255) { cout << "Incorrest Brightness control value" << endl; return TestFail; } @@ -125,8 +125,8 @@ protected: /* * Test control value retrieval and update through ControlInfo. */ - if (list[brightness].get() != 64 || - list[contrast].get() != 128) { + if (list[brightness].get() != 64 || + list[contrast].get() != 128) { cout << "Failed to retrieve control value" << endl; return TestFail; } @@ -134,8 +134,8 @@ protected: list[brightness] = 10; list[contrast] = 20; - if (list[brightness].get() != 10 || - list[contrast].get() != 20) { + if (list[brightness].get() != 10 || + list[contrast].get() != 20) { cout << "Failed to update control value" << endl; return TestFail; } @@ -185,9 +185,9 @@ protected: return TestFail; } - if (newList[Brightness].get() != 10 || - newList[Contrast].get() != 20 || - newList[Saturation].get() != 255) { + if (newList[Brightness].get() != 10 || + newList[Contrast].get() != 20 || + newList[Saturation].get() != 255) { cout << "New list contains incorrect values" << endl; return TestFail; } diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp index 397c43f799ad..a1ffa842f29e 100644 --- a/test/controls/control_value.cpp +++ b/test/controls/control_value.cpp @@ -27,7 +27,7 @@ protected: << " Bool: " << boolean.toString() << endl; - if (integer.get() != 1234) { + if (integer.get() != 1234) { cerr << "Failed to get Integer" << endl; return TestFail; } @@ -56,8 +56,8 @@ protected: return TestFail; } - value.set(10); - if (value.get() != 10) { + value.set(10); + if (value.get() != 10) { cerr << "Failed to get Integer" << endl; return TestFail; } From patchwork Sun Sep 29 19:02:45 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2059 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A7FAE6165A for ; Sun, 29 Sep 2019 21:03:12 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 3EBE4320 for ; Sun, 29 Sep 2019 21:03:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1569783792; bh=OpQLvcExfyicIRpUJ6VVfy0IN3Qx2QFI4jq5s89AAVg=; h=From:To:Subject:Date:In-Reply-To:References:From; b=uBr+08HKz01FUu0WAsLKVOg3N+T2ec8G+uHgOyox0EUzWfI28yRAXNIQDZsPOL5Fo olqies/b6LSPLmjcjpI8d9Y60c1JPJCTkS3DLwcn+5laq0Py8nCzQ3NJGnfzuJpkad jYkRkFSNXAcVISg/5qsILeTt6k9IV6KC7AW+n3/8= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 29 Sep 2019 22:02:45 +0300 Message-Id: <20190929190254.18920-5-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> References: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 04/13] libcamera: controls: Improve the API towards applications X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Sep 2019 19:03:13 -0000 Rework the control-related classes to improve the API towards applications. The goal is to enable writing code similar to Request *req = ...; ControlList &controls = req->controls(); controls->set(controls::AwbEnable, false); controls->set(controls::ManualExposure, 1000); ... int32_t exposure = controls->get(controls::ManualExposure); with the get and set operations ensuring type safety for the control values. This is achieved by creating the following classes: - Control defines controls and is the main way to reference a control. It is a template class to allow methods using it to refer to the control type. - ControlId is the base class of Control. It stores the control ID, name and type, and can be used in contexts where a control needs to be referenced regardless of its type (for instance in lists of controls). This class replaces ControlIdentifier. - ControlValue is kept as-is. The ControlList class now exposes two template get() and set() methods that replace the operator[]. They ensure type safety by infering the value type from the Control reference that they receive. The main way to refer to a control is now through the Control class, and optionally through its base ControlId class. The ControlId enumeration is removed, replaced by a list of global Control instances. Numerical control IDs are turned into macros, and are still exposed as they are required to communicate with IPAs (especially to deserialise control lists). They should however not be used by applications. Auto-generation of header and source files is removed for now to keep the change simple. It will be added back in the future in a more elaborate form. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- Changes since v1: - Reword comments - Remove libcamera::controls::None - Replace macros with an enum for control numerical IDs --- include/libcamera/control_ids.h | 44 ++-- include/libcamera/controls.h | 108 +++++++--- src/libcamera/control_ids.cpp | 52 +++++ src/libcamera/controls.cpp | 318 ++++++++++++++-------------- src/libcamera/gen-controls.awk | 106 ---------- src/libcamera/meson.build | 11 +- src/libcamera/pipeline/uvcvideo.cpp | 40 ++-- src/libcamera/pipeline/vimc.cpp | 28 +-- test/controls/control_info.cpp | 25 +-- test/controls/control_list.cpp | 66 +++--- 10 files changed, 372 insertions(+), 426 deletions(-) create mode 100644 src/libcamera/control_ids.cpp delete mode 100755 src/libcamera/gen-controls.awk diff --git a/include/libcamera/control_ids.h b/include/libcamera/control_ids.h index 75b6a2d5cafe..54235f1aea95 100644 --- a/include/libcamera/control_ids.h +++ b/include/libcamera/control_ids.h @@ -8,34 +8,32 @@ #ifndef __LIBCAMERA_CONTROL_IDS_H__ #define __LIBCAMERA_CONTROL_IDS_H__ -#include +#include + +#include namespace libcamera { -enum ControlId { - AwbEnable, - Brightness, - Contrast, - Saturation, - ManualExposure, - ManualGain, +namespace controls { + +enum { + AWB_ENABLE = 1, + BRIGHTNESS = 2, + CONTRAST = 3, + SATURATION = 4, + MANUAL_EXPOSURE = 5, + MANUAL_GAIN = 6, }; +extern const Control AwbEnable; +extern const Control Brightness; +extern const Control Contrast; +extern const Control Saturation; +extern const Control ManualExposure; +extern const Control ManualGain; + +} /* namespace controls */ + } /* namespace libcamera */ -namespace std { - -template<> -struct hash { - using argument_type = libcamera::ControlId; - using result_type = std::size_t; - - result_type operator()(const argument_type &key) const noexcept - { - return std::hash::type>()(key); - } -}; - -} /* namespace std */ - #endif // __LIBCAMERA_CONTROL_IDS_H__ diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index a3089064c4b5..9698bd1dd383 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -8,12 +8,9 @@ #ifndef __LIBCAMERA_CONTROLS_H__ #define __LIBCAMERA_CONTROLS_H__ -#include #include #include -#include - namespace libcamera { class Camera; @@ -53,55 +50,75 @@ private: }; }; -struct ControlIdentifier { - ControlId id; - const char *name; - ControlType type; +class ControlId +{ +public: + unsigned int id() const { return id_; } + const char *name() const { return name_; } + ControlType type() const { return type_; } + +protected: + ControlId(unsigned int id, const char *name, ControlType type) + : id_(id), name_(name), type_(type) + { + } + +private: + ControlId(const ControlId &) = delete; + ControlId &operator=(const ControlId &) = delete; + + unsigned int id_; + const char *name_; + ControlType type_; +}; + +static inline bool operator==(const ControlId &lhs, const ControlId &rhs) +{ + return lhs.id() == rhs.id(); +} + +static inline bool operator!=(const ControlId &lhs, const ControlId &rhs) +{ + return !(lhs == rhs); +} + +template +class Control : public ControlId +{ +public: + using type = T; + + Control(unsigned int id, const char *name); + +private: + Control(const Control &) = delete; + Control &operator=(const Control &) = delete; }; class ControlInfo { public: - explicit ControlInfo(ControlId id, const ControlValue &min = 0, + explicit ControlInfo(const ControlId &id, const ControlValue &min = 0, const ControlValue &max = 0); - ControlId id() const { return ident_->id; } - const char *name() const { return ident_->name; } - ControlType type() const { return ident_->type; } - + const ControlId &id() const { return id_; } const ControlValue &min() const { return min_; } const ControlValue &max() const { return max_; } std::string toString() const; private: - const struct ControlIdentifier *ident_; + const ControlId &id_; ControlValue min_; ControlValue max_; }; -bool operator==(const ControlInfo &lhs, const ControlInfo &rhs); -bool operator==(const ControlId &lhs, const ControlInfo &rhs); -bool operator==(const ControlInfo &lhs, const ControlId &rhs); -static inline bool operator!=(const ControlInfo &lhs, const ControlInfo &rhs) -{ - return !(lhs == rhs); -} -static inline bool operator!=(const ControlId &lhs, const ControlInfo &rhs) -{ - return !(lhs == rhs); -} -static inline bool operator!=(const ControlInfo &lhs, const ControlId &rhs) -{ - return !(lhs == rhs); -} - -using ControlInfoMap = std::unordered_map; +using ControlInfoMap = std::unordered_map; class ControlList { private: - using ControlListMap = std::unordered_map; + using ControlListMap = std::unordered_map; public: ControlList(Camera *camera); @@ -114,18 +131,39 @@ public: const_iterator begin() const { return controls_.begin(); } const_iterator end() const { return controls_.end(); } - bool contains(ControlId id) const; - bool contains(const ControlInfo *info) const; + bool contains(const ControlId &id) const; bool empty() const { return controls_.empty(); } std::size_t size() const { return controls_.size(); } void clear() { controls_.clear(); } - ControlValue &operator[](ControlId id); - ControlValue &operator[](const ControlInfo *info) { return controls_[info]; } + template + const T &get(const Control &ctrl) const + { + const ControlValue *val = find(ctrl); + if (!val) { + static T t(0); + return t; + } + + return val->get(); + } + + template + void set(const Control &ctrl, const T &value) + { + ControlValue *val = find(ctrl); + if (!val) + return; + + val->set(value); + } void update(const ControlList &list); private: + const ControlValue *find(const ControlId &id) const; + ControlValue *find(const ControlId &id); + Camera *camera_; ControlListMap controls_; }; diff --git a/src/libcamera/control_ids.cpp b/src/libcamera/control_ids.cpp new file mode 100644 index 000000000000..3af23a458862 --- /dev/null +++ b/src/libcamera/control_ids.cpp @@ -0,0 +1,52 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * control_ids.cpp : Control ID list + */ + +#include + +/** + * \file control_ids.h + * \brief Camera control identifiers + */ + +namespace libcamera { + +namespace controls { + +/** + * \brief Enables or disables the AWB. + * \sa ManualGain + */ +extern const Control AwbEnable(AWB_ENABLE, "AwbEnable"); + +/** + * \brief Specify a fixed brightness parameter. + */ +extern const Control Brightness(BRIGHTNESS, "Brightness"); + +/** + * \brief Specify a fixed contrast parameter. + */ +extern const Control Contrast(CONTRAST, "Contrast"); + +/** + * \brief Specify a fixed saturation parameter. + */ +extern const Control Saturation(SATURATION, "Saturation"); + +/** + * \brief Specify a fixed exposure time in milli-seconds + */ +extern const Control ManualExposure(MANUAL_EXPOSURE, "ManualExposure"); + +/** + * \brief Specify a fixed gain parameter + */ +extern const Control ManualGain(MANUAL_GAIN, "ManualGain"); + +} /* namespace controls */ + +} /* namespace libcamera */ diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 295ccd55a622..a34af588fc7e 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -18,6 +18,29 @@ /** * \file controls.h * \brief Describes control framework and controls supported by a camera + * + * A control is a mean to govern or influence the operation of a camera. Every + * control is defined by a unique numerical ID, a name string and the data type + * of the value it stores. The libcamera API defines a set of standard controls + * in the libcamera::controls namespace, as a set of instances of the Control + * class. + * + * The main way for applications to interact with controls is through the + * ControlList stored in the Request class: + * + * \code{.cpp} + * Request *req = ...; + * ControlList &controls = req->controls(); + * controls->set(controls::AwbEnable, false); + * controls->set(controls::ManualExposure, 1000); + * + * ... + * + * int32_t exposure = controls->get(controls::ManualExposure); + * \endcode + * + * The ControlList::get() and ControlList::set() methods automatically deduce + * the data type based on the control. */ namespace libcamera { @@ -173,76 +196,120 @@ std::string ControlValue::toString() const } /** - * \enum ControlId - * \brief Numerical control ID + * \class ControlId + * \brief Control static metadata + * + * The ControlId class stores a control ID, name and data type. It provides + * unique identification of a control, but without support for compile-time + * type deduction that the derived template Control class supports. See the + * Control class for more information. */ /** - * \var AwbEnable - * ControlType: Bool - * - * Enables or disables the AWB. See also \a libcamera::ControlId::ManualGain + * \fn ControlId::ControlId(unsigned int id, const char *name, ControlType type) + * \brief Construct a ControlId instance + * \param[in] id The control numerical ID + * \param[in] name The control name + * \param[in] type The control data type */ /** - * \var Brightness - * ControlType: Integer32 - * - * Specify a fixed brightness parameter. + * \fn unsigned int ControlId::id() const + * \brief Retrieve the control numerical ID + * \return The control numerical ID */ /** - * \var Contrast - * ControlType: Integer32 - * - * Specify a fixed contrast parameter. + * \fn const char *ControlId::name() const + * \brief Retrieve the control name + * \return The control name */ /** - * \var Saturation - * ControlType: Integer32 - * - * Specify a fixed saturation parameter. + * \fn ControlType ControlId::type() const + * \brief Retrieve the control data type + * \return The control data type */ /** - * \var ManualExposure - * ControlType: Integer32 + * \fn bool operator==(const ControlId &lhs, const ControlId &rhs) + * \brief Compare two ControlId instances for equality + * \param[in] lhs Left-hand side ControlId + * \param[in] rhs Right-hand side ControlId + * + * ControlId instances are compared based on the numerical ControlId::id() + * only, as an object may not have two separate controls with the same + * numerical ID. * - * Specify a fixed exposure time in milli-seconds + * \return True if \a lhs and \a rhs have equal control IDs, false otherwise */ /** - * \var ManualGain - * ControlType: Integer32 + * \class Control + * \brief Describe a control and its intrinsic properties + * + * The Control class models a control exposed by a camera. Its template type + * name T refers to the control data type, and allows methods that operate on + * control values to be defined as template methods using the same type T for + * the control value. See for instance how the ControlList::get() method + * returns a value corresponding to the type of the requested control. + * + * While this class is the main mean to refer to a control, the control + * identifying information are stored in the non-template base ControlId class. + * This allows code that operates on a set of controls of different types to + * reference those controls through a ControlId instead of a Control. For + * instance, the list of controls supported by a camera is exposed as ControlId + * instead of Control. * - * Specify a fixed gain parameter + * Controls of any type can be defined through template specialisation, but + * libcamera only supports the bool, int32_t and int64_t types natively (this + * includes types that are equivalent to the supported types, such as int and + * long int). + * + * Controls IDs shall be unique. While nothing prevents multiple instances of + * the Control class to be created with the same ID, this may lead to undefined + * behaviour. */ /** - * \struct ControlIdentifier - * \brief Describe a ControlId with control specific constant meta-data - * - * Defines a Control with a unique ID, a name, and a type. - * This structure is used as static part of the auto-generated control - * definitions, which are generated from the ControlId documentation. + * \fn Control::Control(unsigned int id, const char *name) + * \brief Construct a Control instance + * \param[in] id The control numerical ID + * \param[in] name The control name * - * \var ControlIdentifier::id - * The unique ID for a control - * \var ControlIdentifier::name - * The string representation of the control - * \var ControlIdentifier::type - * The ValueType required to represent the control value + * The control data type is automatically deduced from the template type T. */ -/* - * The controlTypes are automatically generated to produce a control_types.cpp - * output. This file is not for public use, and so no suitable header exists - * for this sole usage of the controlTypes reference. As such the extern is - * only defined here for use during the ControlInfo constructor and should not - * be referenced directly elsewhere. +/** + * \typedef Control::type + * \brief The Control template type T */ -extern const std::unordered_map controlTypes; + +#ifndef __DOXYGEN__ +template<> +Control::Control(unsigned int id, const char *name) + : ControlId(id, name, ControlTypeNone) +{ +} + +template<> +Control::Control(unsigned int id, const char *name) + : ControlId(id, name, ControlTypeBool) +{ +} + +template<> +Control::Control(unsigned int id, const char *name) + : ControlId(id, name, ControlTypeInteger32) +{ +} + +template<> +Control::Control(unsigned int id, const char *name) + : ControlId(id, name, ControlTypeInteger64) +{ +} +#endif /* __DOXYGEN__ */ /** * \class ControlInfo @@ -260,17 +327,10 @@ extern const std::unordered_map controlTypes; * \param[in] min The control minimum value * \param[in] max The control maximum value */ -ControlInfo::ControlInfo(ControlId id, const ControlValue &min, +ControlInfo::ControlInfo(const ControlId &id, const ControlValue &min, const ControlValue &max) - : min_(min), max_(max) + : id_(id), min_(min), max_(max) { - auto iter = controlTypes.find(id); - if (iter == controlTypes.end()) { - LOG(Controls, Fatal) << "Attempt to create invalid ControlInfo"; - return; - } - - ident_ = &iter->second; } /** @@ -279,18 +339,6 @@ ControlInfo::ControlInfo(ControlId id, const ControlValue &min, * \return The control ID */ -/** - * \fn ControlInfo::name() - * \brief Retrieve the control name string - * \return The control name string - */ - -/** - * \fn ControlInfo::type() - * \brief Retrieve the control data type - * \return The control data type - */ - /** * \fn ControlInfo::min() * \brief Retrieve the minimum value of the control @@ -310,56 +358,11 @@ std::string ControlInfo::toString() const { std::stringstream ss; - ss << name() << "[" << min_.toString() << ".." << max_.toString() << "]"; + ss << id_.name() << "[" << min_.toString() << ".." << max_.toString() << "]"; return ss.str(); } -/** - * \brief Compare control information for equality - * \param[in] lhs Left-hand side control information - * \param[in] rhs Right-hand side control information - * - * Control information is compared based on the ID only, as a camera may not - * have two separate controls with the same ID. - * - * \return True if \a lhs and \a rhs are equal, false otherwise - */ -bool operator==(const ControlInfo &lhs, const ControlInfo &rhs) -{ - return lhs.id() == rhs.id(); -} - -/** - * \brief Compare control ID and information for equality - * \param[in] lhs Left-hand side control identifier - * \param[in] rhs Right-hand side control information - * - * Control information is compared based on the ID only, as a camera may not - * have two separate controls with the same ID. - * - * \return True if \a lhs and \a rhs are equal, false otherwise - */ -bool operator==(const ControlId &lhs, const ControlInfo &rhs) -{ - return lhs == rhs.id(); -} - -/** - * \brief Compare control information and ID for equality - * \param[in] lhs Left-hand side control information - * \param[in] rhs Right-hand side control identifier - * - * Control information is compared based on the ID only, as a camera may not - * have two separate controls with the same ID. - * - * \return True if \a lhs and \a rhs are equal, false otherwise - */ -bool operator==(const ControlInfo &lhs, const ControlId &rhs) -{ - return lhs.id() == rhs; -} - /** * \typedef ControlInfoMap * \brief A map of ControlId to ControlInfo @@ -431,29 +434,9 @@ ControlList::ControlList(Camera *camera) * * \return True if the list contains a matching control, false otherwise */ -bool ControlList::contains(ControlId id) const +bool ControlList::contains(const ControlId &id) const { - const ControlInfoMap &controls = camera_->controls(); - const auto iter = controls.find(id); - if (iter == controls.end()) { - LOG(Controls, Error) - << "Camera " << camera_->name() - << " does not support control " << id; - - return false; - } - - return controls_.find(&iter->second) != controls_.end(); -} - -/** - * \brief Check if the list contains a control with the specified \a info - * \param[in] info The control info - * \return True if the list contains a matching control, false otherwise - */ -bool ControlList::contains(const ControlInfo *info) const -{ - return controls_.find(info) != controls_.end(); + return controls_.find(&id) != controls_.end(); } /** @@ -474,44 +457,61 @@ bool ControlList::contains(const ControlInfo *info) const */ /** - * \brief Access or insert the control specified by \a id - * \param[in] id The control ID + * \fn template const T &ControlList::get() const + * \brief Get the value of a control + * \param[in] ctrl The control * - * This method returns a reference to the control identified by \a id, inserting - * it in the list if the ID is not already present. + * The behaviour is undefined if the control \a ctrl is not present in the + * list. Use ControlList::contains() to test for the presence of a control in + * the list before retrieving its value. * - * The behaviour is undefined if the control \a id is not supported by the - * camera that the ControlList refers to. + * The control value type shall match the type T, otherwise the behaviour is + * undefined. * - * \return A reference to the value of the control identified by \a id + * \return The control value */ -ControlValue &ControlList::operator[](ControlId id) + +/** + * \fn template void ControlList::set() + * \brief Set the control value to \a value + * \param[in] ctrl The control + * \param[in] value The control value + * + * This method sets the value of a control in the control list. If the control + * is already present in the list, its value is updated, otherwise it is added + * to the list. + * + * The behaviour is undefined if the control \a ctrl is not supported by the + * camera that the list refers to. + */ + +const ControlValue *ControlList::find(const ControlId &id) const +{ + const auto iter = controls_.find(&id); + if (iter == controls_.end()) { + LOG(Controls, Error) + << "Control " << id.name() << " not found"; + + return nullptr; + } + + return &iter->second; +} + +ControlValue *ControlList::find(const ControlId &id) { const ControlInfoMap &controls = camera_->controls(); - const auto iter = controls.find(id); + const auto iter = controls.find(&id); if (iter == controls.end()) { LOG(Controls, Error) << "Camera " << camera_->name() - << " does not support control " << id; - - static ControlValue empty; - return empty; + << " does not support control " << id.name(); + return nullptr; } - return controls_[&iter->second]; + return &controls_[&id]; } -/** - * \fn ControlList::operator[](const ControlInfo *info) - * \brief Access or insert the control specified by \a info - * \param[in] info The control info - * - * This method returns a reference to the control identified by \a info, - * inserting it in the list if the info is not already present. - * - * \return A reference to the value of the control identified by \a info - */ - /** * \brief Update the list with a union of itself and \a other * \param other The other list @@ -533,10 +533,10 @@ void ControlList::update(const ControlList &other) } for (auto it : other) { - const ControlInfo *info = it.first; + const ControlId *id = it.first; const ControlValue &value = it.second; - controls_[info] = value; + controls_[id] = value; } } diff --git a/src/libcamera/gen-controls.awk b/src/libcamera/gen-controls.awk deleted file mode 100755 index a3f291e7071c..000000000000 --- a/src/libcamera/gen-controls.awk +++ /dev/null @@ -1,106 +0,0 @@ -#!/usr/bin/awk -f - -# SPDX-License-Identifier: LGPL-2.1-or-later - -# Controls are documented using Doxygen in the main controls.cpp source. -# -# Generate control tables directly from the documentation, creating enumerations -# to support the IDs and static type information regarding each control. - -BEGIN { - id=0 - input=ARGV[1] - mode=ARGV[2] - output=ARGV[3] -} - -# Detect Doxygen style comment blocks and ignore other lines -/^\/\*\*$/ { in_doxygen=1; first_line=1; next } -// { if (!in_doxygen) next } - -# Entry point for the Control Documentation -/ * \\enum ControlId$/ { in_controls=1; first_line=0; next } -// { if (!in_controls) next } - -# Extract control information -/ \* \\var/ { names[++id]=$3; first_line=0; next } -/ \* ControlType:/ { types[id] = $3 } - -# End of comment blocks -/^ \*\// { in_doxygen=0 } - -# Identify the end of controls -/^ \* \\/ { if (first_line) exit } -// { first_line=0 } - -################################ -# Support output file generation - -function basename(file) { - sub(".*/", "", file) - return file -} - -function Header(file, description) { - print "/* SPDX-License-Identifier: LGPL-2.1-or-later */" > file - print "/*" > file - print " * Copyright (C) 2019, Google Inc." > file - print " *" > file - print " * " basename(file) " - " description > file - print " *" > file - print " * This file is auto-generated. Do not edit." > file - print " */" > file - print "" > file -} - -function EnterNameSpace(file) { - print "namespace libcamera {" > file - print "" > file -} - -function ExitNameSpace(file) { - print "" > file - print "} /* namespace libcamera */" > file -} - -function GenerateHeader(file) { - Header(file, "Control ID list") - - print "#ifndef __LIBCAMERA_CONTROL_IDS_H__" > file - print "#define __LIBCAMERA_CONTROL_IDS_H__" > file - print "" > file - - EnterNameSpace(file) - print "enum ControlId {" > file - for (i=1; i <= id; ++i) { - printf "\t%s,\n", names[i] > file - } - print "};" > file - ExitNameSpace(file) - - print "" > file - print "#endif // __LIBCAMERA_CONTROL_IDS_H__" > file -} - -function GenerateTable(file) { - Header(file, "Control types") - print "#include " > file - print "" > file - - EnterNameSpace(file) - - print "extern const std::unordered_map" > file - print "controlTypes {" > file - for (i=1; i <= id; ++i) { - printf "\t{ %s, { %s, \"%s\", ControlType%s } },\n", names[i], names[i], names[i], types[i] > file - } - print "};" > file - ExitNameSpace(file) -} - -END { - if (mode == "--header") - GenerateHeader(output) - else - GenerateTable(output) -} diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 755149c55c7b..8123d1d5bee9 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -5,6 +5,7 @@ libcamera_sources = files([ 'camera_manager.cpp', 'camera_sensor.cpp', 'controls.cpp', + 'control_ids.cpp', 'device_enumerator.cpp', 'device_enumerator_sysfs.cpp', 'event_dispatcher.cpp', @@ -57,16 +58,6 @@ if libudev.found() ]) endif -gen_controls = files('gen-controls.awk') - -control_types_cpp = custom_target('control_types_cpp', - input : files('controls.cpp'), - output : 'control_types.cpp', - depend_files : gen_controls, - command : [gen_controls, '@INPUT@', '--code', '@OUTPUT@']) - -libcamera_sources += control_types_cpp - gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh') version_cpp = vcs_tag(command : [gen_version, meson.build_root()], diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 0d56758e3e1d..d5d30932870a 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -230,33 +231,20 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) V4L2ControlList controls; for (auto it : request->controls()) { - const ControlInfo *ci = it.first; + const ControlId &id = *it.first; ControlValue &value = it.second; - switch (ci->id()) { - case Brightness: + if (id == controls::Brightness) { controls.add(V4L2_CID_BRIGHTNESS, value.get()); - break; - - case Contrast: + } else if (id == controls::Contrast) { controls.add(V4L2_CID_CONTRAST, value.get()); - break; - - case Saturation: + } else if (id == controls::Saturation) { controls.add(V4L2_CID_SATURATION, value.get()); - break; - - case ManualExposure: + } else if (id == controls::ManualExposure) { controls.add(V4L2_CID_EXPOSURE_AUTO, 1); controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get()); - break; - - case ManualGain: + } else if (id == controls::ManualGain) { controls.add(V4L2_CID_GAIN, value.get()); - break; - - default: - break; } } @@ -352,23 +340,23 @@ int UVCCameraData::init(MediaEntity *entity) for (const auto &ctrl : controls) { unsigned int v4l2Id = ctrl.first; const V4L2ControlInfo &info = ctrl.second; - ControlId id; + const ControlId *id; switch (v4l2Id) { case V4L2_CID_BRIGHTNESS: - id = Brightness; + id = &controls::Brightness; break; case V4L2_CID_CONTRAST: - id = Contrast; + id = &controls::Contrast; break; case V4L2_CID_SATURATION: - id = Saturation; + id = &controls::Saturation; break; case V4L2_CID_EXPOSURE_ABSOLUTE: - id = ManualExposure; + id = &controls::ManualExposure; break; case V4L2_CID_GAIN: - id = ManualGain; + id = &controls::ManualGain; break; default: continue; @@ -376,7 +364,7 @@ int UVCCameraData::init(MediaEntity *entity) controlInfo_.emplace(std::piecewise_construct, std::forward_as_tuple(id), - std::forward_as_tuple(id, info.min(), info.max())); + std::forward_as_tuple(*id, info.min(), info.max())); } return 0; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index e549dc64b996..608a47aecf76 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -283,24 +284,15 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) V4L2ControlList controls; for (auto it : request->controls()) { - const ControlInfo *ci = it.first; + const ControlId &id = *it.first; ControlValue &value = it.second; - switch (ci->id()) { - case Brightness: + if (id == controls::Brightness) { controls.add(V4L2_CID_BRIGHTNESS, value.get()); - break; - - case Contrast: + } else if (id == controls::Contrast) { controls.add(V4L2_CID_CONTRAST, value.get()); - break; - - case Saturation: + } else if (id == controls::Saturation) { controls.add(V4L2_CID_SATURATION, value.get()); - break; - - default: - break; } } @@ -427,17 +419,17 @@ int VimcCameraData::init(MediaDevice *media) for (const auto &ctrl : controls) { unsigned int v4l2Id = ctrl.first; const V4L2ControlInfo &info = ctrl.second; - ControlId id; + const ControlId *id; switch (v4l2Id) { case V4L2_CID_BRIGHTNESS: - id = Brightness; + id = &controls::Brightness; break; case V4L2_CID_CONTRAST: - id = Contrast; + id = &controls::Contrast; break; case V4L2_CID_SATURATION: - id = Saturation; + id = &controls::Saturation; break; default: continue; @@ -445,7 +437,7 @@ int VimcCameraData::init(MediaDevice *media) controlInfo_.emplace(std::piecewise_construct, std::forward_as_tuple(id), - std::forward_as_tuple(id, info.min(), info.max())); + std::forward_as_tuple(*id, info.min(), info.max())); } return 0; diff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp index 2aba568a302e..dbc43df8e3d3 100644 --- a/test/controls/control_info.cpp +++ b/test/controls/control_info.cpp @@ -7,6 +7,7 @@ #include +#include #include #include "test.h" @@ -23,17 +24,17 @@ protected: * Test information retrieval from a control with no minimum * and maximum. */ - ControlInfo info(Brightness); + ControlInfo brightness(controls::Brightness); - if (info.id() != Brightness || - info.type() != ControlTypeInteger32 || - info.name() != std::string("Brightness")) { + if (brightness.id() != controls::Brightness || + brightness.id().type() != ControlTypeInteger32 || + brightness.id().name() != std::string("Brightness")) { cout << "Invalid control identification for Brightness" << endl; return TestFail; } - if (info.min().get() != 0 || - info.max().get() != 0) { + if (brightness.min().get() != 0 || + brightness.max().get() != 0) { cout << "Invalid control range for Brightness" << endl; return TestFail; } @@ -42,17 +43,17 @@ protected: * Test information retrieval from a control with a minimum and * a maximum value. */ - info = ControlInfo(Contrast, 10, 200); + ControlInfo contrast(controls::Contrast, 10, 200); - if (info.id() != Contrast || - info.type() != ControlTypeInteger32 || - info.name() != std::string("Contrast")) { + if (contrast.id() != controls::Contrast || + contrast.id().type() != ControlTypeInteger32 || + contrast.id().name() != std::string("Contrast")) { cout << "Invalid control identification for Contrast" << endl; return TestFail; } - if (info.min().get() != 10 || - info.max().get() != 200) { + if (contrast.min().get() != 10 || + contrast.max().get() != 200) { cout << "Invalid control range for Contrast" << endl; return TestFail; } diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp index 5215840b1c4e..053696814b67 100644 --- a/test/controls/control_list.cpp +++ b/test/controls/control_list.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include "test.h" @@ -52,7 +53,7 @@ protected: return TestFail; } - if (list.contains(Brightness)) { + if (list.contains(controls::Brightness)) { cout << "List should not contain Brightness control" << endl; return TestFail; } @@ -70,7 +71,7 @@ protected: * Set a control, and verify that the list now contains it, and * nothing else. */ - list[Brightness] = 255; + list.set(controls::Brightness, 255); if (list.empty()) { cout << "List should not be empty" << endl; @@ -82,7 +83,7 @@ protected: return TestFail; } - if (!list.contains(Brightness)) { + if (!list.contains(controls::Brightness)) { cout << "List should contain Brightness control" << endl; return TestFail; } @@ -96,54 +97,45 @@ protected: return TestFail; } - if (list[Brightness].get() != 255) { + if (list.get(controls::Brightness) != 255) { cout << "Incorrest Brightness control value" << endl; return TestFail; } - if (list.contains(Contrast)) { + if (list.contains(controls::Contrast)) { cout << "List should not contain Contract control" << endl; return TestFail; } - /* - * Set a second control through ControlInfo and retrieve it - * through both controlId and ControlInfo. - */ - const ControlInfoMap &controls = camera_->controls(); - const ControlInfo *brightness = &controls.find(Brightness)->second; - const ControlInfo *contrast = &controls.find(Contrast)->second; + /* Update the first control and set a second one. */ + list.set(controls::Brightness, 64); + list.set(controls::Contrast, 128); - list[brightness] = 64; - list[contrast] = 128; - - if (!list.contains(Contrast) || !list.contains(contrast)) { + if (!list.contains(controls::Contrast) || + !list.contains(controls::Contrast)) { cout << "List should contain Contrast control" << endl; return TestFail; } - /* - * Test control value retrieval and update through ControlInfo. - */ - if (list[brightness].get() != 64 || - list[contrast].get() != 128) { + if (list.get(controls::Brightness) != 64 || + list.get(controls::Contrast) != 128) { cout << "Failed to retrieve control value" << endl; return TestFail; } - list[brightness] = 10; - list[contrast] = 20; + /* + * Update both controls and verify that the container doesn't + * grow. + */ + list.set(controls::Brightness, 10); + list.set(controls::Contrast, 20); - if (list[brightness].get() != 10 || - list[contrast].get() != 20) { + if (list.get(controls::Brightness) != 10 || + list.get(controls::Contrast) != 20) { cout << "Failed to update control value" << endl; return TestFail; } - /* - * Assert that the container has not grown with the control - * updated. - */ if (list.size() != 2) { cout << "List should contain two elements" << endl; return TestFail; @@ -157,8 +149,8 @@ protected: */ ControlList newList(camera_.get()); - newList[Brightness] = 128; - newList[Saturation] = 255; + newList.set(controls::Brightness, 128); + newList.set(controls::Saturation, 255); newList.update(list); list.clear(); @@ -178,16 +170,16 @@ protected: return TestFail; } - if (!newList.contains(Brightness) || - !newList.contains(Contrast) || - !newList.contains(Saturation)) { + if (!newList.contains(controls::Brightness) || + !newList.contains(controls::Contrast) || + !newList.contains(controls::Saturation)) { cout << "New list contains incorrect items" << endl; return TestFail; } - if (newList[Brightness].get() != 10 || - newList[Contrast].get() != 20 || - newList[Saturation].get() != 255) { + if (newList.get(controls::Brightness) != 10 || + newList.get(controls::Contrast) != 20 || + newList.get(controls::Saturation) != 255) { cout << "New list contains incorrect values" << endl; return TestFail; } From patchwork Sun Sep 29 19:02:46 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2060 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 130E76170D for ; Sun, 29 Sep 2019 21:03:13 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A283A813 for ; Sun, 29 Sep 2019 21:03:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1569783792; bh=QP82eA1FSJEOJh66iNGvuR7L6ETSutojYIEf77g0Qrg=; h=From:To:Subject:Date:In-Reply-To:References:From; b=GYkNPu5h8rHtQzfUdYTI88qMq9KBs1hEcQmSM+x9oWthfAlBePQv3h2TfaD2ddv9F B8zhY1EILf58cwesB4anjcsonLmcOoGLzNpFxi89ufztEzFAu9n6KIlTqj4Xc1z+ea Aak3ZyxbrVfYqTBT9PTeHgFT0qduWnLyZUMbotzs= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 29 Sep 2019 22:02:46 +0300 Message-Id: <20190929190254.18920-6-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> References: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 05/13] libcamera: controls: Auto-generate control_ids.h and control_ids.cpp X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Sep 2019 19:03:14 -0000 Bring back auto-generation of control ids. In this version, both the header and the source files are generated from a single YAML file that stores all control definitions. This allows centralising controls in a single file, while the previous version required keeping both declarations (in a header) and documentation (in a the source) in sync manually. Using YAML as a format to store control definitions is a trade-off between ease of use (there are many YAML parsers available) and simplicity (XML was considered, but would have lead to more complex processing). A new build time dependency is added on python3-yaml, which should be available as a package in all distributions and build environments. The YAML format is likely to change over time as we improve documentation of controls, the first version simply copies the information currently available. Future improvements should also include a YAML schema to validate the YAML source file. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- Documentation/Doxyfile.in | 4 +- README.rst | 2 +- .../{control_ids.h => control_ids.h.in} | 16 +-- include/libcamera/gen-header.sh | 2 +- include/libcamera/meson.build | 18 ++- .../libcamera/libcamera-9999.ebuild | 9 +- src/libcamera/control_ids.cpp | 52 -------- src/libcamera/control_ids.cpp.in | 25 ++++ src/libcamera/control_ids.yaml | 35 ++++++ src/libcamera/gen-controls.py | 114 ++++++++++++++++++ src/libcamera/meson.build | 12 +- 11 files changed, 215 insertions(+), 74 deletions(-) rename include/libcamera/{control_ids.h => control_ids.h.in} (53%) delete mode 100644 src/libcamera/control_ids.cpp create mode 100644 src/libcamera/control_ids.cpp.in create mode 100644 src/libcamera/control_ids.yaml create mode 100755 src/libcamera/gen-controls.py diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in index 28a9c2da1ad4..5237cf60854f 100644 --- a/Documentation/Doxyfile.in +++ b/Documentation/Doxyfile.in @@ -793,7 +793,9 @@ WARN_LOGFILE = INPUT = "@TOP_SRCDIR@/include/ipa" \ "@TOP_SRCDIR@/include/libcamera" \ - "@TOP_SRCDIR@/src/libcamera" + "@TOP_SRCDIR@/src/libcamera" \ + "@TOP_BUILDDIR@/include/libcamera" \ + "@TOP_BUILDDIR@/src/libcamera" # This tag can be used to specify the character encoding of the source files # that doxygen parses. Internally doxygen uses the UTF-8 encoding. Doxygen uses diff --git a/README.rst b/README.rst index 169837e41a4e..2ccf7cbec40a 100644 --- a/README.rst +++ b/README.rst @@ -40,7 +40,7 @@ A C++ toolchain: [required] Either {g++, clang} for libcamera: [required] - meson ninja-build + meson ninja-build python3-yaml for device hotplug enumeration: [optional] pkg-config libudev-dev diff --git a/include/libcamera/control_ids.h b/include/libcamera/control_ids.h.in similarity index 53% rename from include/libcamera/control_ids.h rename to include/libcamera/control_ids.h.in index 54235f1aea95..1d0bc791e559 100644 --- a/include/libcamera/control_ids.h +++ b/include/libcamera/control_ids.h.in @@ -3,6 +3,8 @@ * Copyright (C) 2019, Google Inc. * * control_ids.h : Control ID list + * + * This file is auto-generated. Do not edit. */ #ifndef __LIBCAMERA_CONTROL_IDS_H__ @@ -17,20 +19,10 @@ namespace libcamera { namespace controls { enum { - AWB_ENABLE = 1, - BRIGHTNESS = 2, - CONTRAST = 3, - SATURATION = 4, - MANUAL_EXPOSURE = 5, - MANUAL_GAIN = 6, +${ids} }; -extern const Control AwbEnable; -extern const Control Brightness; -extern const Control Contrast; -extern const Control Saturation; -extern const Control ManualExposure; -extern const Control ManualGain; +${controls} } /* namespace controls */ diff --git a/include/libcamera/gen-header.sh b/include/libcamera/gen-header.sh index a69fe8e982a1..7f7816c9f879 100755 --- a/include/libcamera/gen-header.sh +++ b/include/libcamera/gen-header.sh @@ -19,7 +19,7 @@ EOF headers=$(for header in "$src_dir"/*.h ; do header=$(basename "$header") echo "$header" -done ; echo "version.h" | sort) +done ; echo "control_ids.h" ; echo "version.h" | sort) for header in $headers ; do echo "#include " >> "$dst_file" diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build index 868f1a6bf1ab..4ffbdab3b173 100644 --- a/include/libcamera/meson.build +++ b/include/libcamera/meson.build @@ -3,7 +3,6 @@ libcamera_api = files([ 'buffer.h', 'camera.h', 'camera_manager.h', - 'control_ids.h', 'controls.h', 'event_dispatcher.h', 'event_notifier.h', @@ -18,6 +17,20 @@ libcamera_api = files([ include_dir = join_paths(libcamera_include_dir, 'libcamera') +install_headers(libcamera_api, + subdir : include_dir) + +gen_controls = files('../../src/libcamera/gen-controls.py') + +control_ids_h = custom_target('control_ids_h', + input : files('../../src/libcamera/control_ids.yaml', 'control_ids.h.in'), + output : 'control_ids.h', + depend_files : gen_controls, + command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'], + install_dir : join_paths('include', include_dir)) + +libcamera_api += control_ids_h + gen_header = files('gen-header.sh') libcamera_h = custom_target('gen-header', @@ -37,6 +50,3 @@ configure_file(input : 'version.h.in', output : 'version.h', configuration : libcamera_version_config, install_dir : join_paths('include', include_dir)) - -install_headers(libcamera_api, - subdir : include_dir) diff --git a/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild b/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild index fed2b409a91b..fc241b1f5584 100644 --- a/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild +++ b/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild @@ -2,7 +2,9 @@ # Distributed under the terms of the GNU General Public License v2 EAPI=6 -inherit git-r3 meson +PYTHON_COMPAT=( python3_{5,6,7} ) + +inherit git-r3 meson python-any-r1 DESCRIPTION="Camera support library for Linux" HOMEPAGE="http://libcamera.org" @@ -15,7 +17,10 @@ KEYWORDS="*" IUSE="udev" RDEPEND="udev? ( virtual/libudev )" -DEPEND="${RDEPEND}" +DEPEND=" + ${RDEPEND} + $(python_gen_any_dep 'dev-python/pyyaml[${PYTHON_USEDEP}]') +" src_configure() { local emesonargs=( diff --git a/src/libcamera/control_ids.cpp b/src/libcamera/control_ids.cpp deleted file mode 100644 index 3af23a458862..000000000000 --- a/src/libcamera/control_ids.cpp +++ /dev/null @@ -1,52 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2019, Google Inc. - * - * control_ids.cpp : Control ID list - */ - -#include - -/** - * \file control_ids.h - * \brief Camera control identifiers - */ - -namespace libcamera { - -namespace controls { - -/** - * \brief Enables or disables the AWB. - * \sa ManualGain - */ -extern const Control AwbEnable(AWB_ENABLE, "AwbEnable"); - -/** - * \brief Specify a fixed brightness parameter. - */ -extern const Control Brightness(BRIGHTNESS, "Brightness"); - -/** - * \brief Specify a fixed contrast parameter. - */ -extern const Control Contrast(CONTRAST, "Contrast"); - -/** - * \brief Specify a fixed saturation parameter. - */ -extern const Control Saturation(SATURATION, "Saturation"); - -/** - * \brief Specify a fixed exposure time in milli-seconds - */ -extern const Control ManualExposure(MANUAL_EXPOSURE, "ManualExposure"); - -/** - * \brief Specify a fixed gain parameter - */ -extern const Control ManualGain(MANUAL_GAIN, "ManualGain"); - -} /* namespace controls */ - -} /* namespace libcamera */ diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in new file mode 100644 index 000000000000..f699ac9eea54 --- /dev/null +++ b/src/libcamera/control_ids.cpp.in @@ -0,0 +1,25 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * control_ids.cpp : Control ID list + * + * This file is auto-generated. Do not edit. + */ + +#include + +/** + * \file control_ids.h + * \brief Camera control identifiers + */ + +namespace libcamera { + +namespace controls { + +${controls} + +} /* namespace controls */ + +} /* namespace libcamera */ diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml new file mode 100644 index 000000000000..819a5967a2fc --- /dev/null +++ b/src/libcamera/control_ids.yaml @@ -0,0 +1,35 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# Copyright (C) 2019, Google Inc. +# +%YAML 1.2 +--- +controls: + - AwbEnable: + type: bool + description: | + Enables or disables the AWB. + + \sa ManualGain + + - Brightness: + type: int32_t + description: Specify a fixed brightness parameter + + - Contrast: + type: int32_t + description: Specify a fixed contrast parameter + + - Saturation: + type: int32_t + description: Specify a fixed saturation parameter + + - ManualExposure: + type: int32_t + description: Specify a fixed exposure time in milli-seconds + + - ManualGain: + type: int32_t + description: Specify a fixed gain parameter + +... diff --git a/src/libcamera/gen-controls.py b/src/libcamera/gen-controls.py new file mode 100755 index 000000000000..0899e40b4080 --- /dev/null +++ b/src/libcamera/gen-controls.py @@ -0,0 +1,114 @@ +#!/usr/bin/python3 +# SPDX-License-Identifier: GPL-2.0-or-later +# Copyright (C) 2019, Google Inc. +# +# Author: Laurent Pinchart +# +# gen-controls.py - Generate control definitions from YAML + +import argparse +import string +import sys +import yaml + + +def snake_case(s): + return ''.join([c.isupper() and ('_' + c) or c for c in s]).strip('_') + + +def generate_cpp(controls): + template = string.Template('''/** +${description} + */ +extern const Control<${type}> ${name}(${id_name}, "${name}");''') + + ctrls = [] + + for ctrl in controls: + name, ctrl = ctrl.popitem() + id_name = snake_case(name).upper() + + description = ctrl['description'].strip('\n').split('\n') + description[0] = '\\brief ' + description[0] + description = '\n'.join([(line and ' * ' or ' *') + line for line in description]) + + info = { + 'name': name, + 'type': ctrl['type'], + 'description': description, + 'id_name': id_name, + } + + ctrls.append(template.substitute(info)) + + return {'controls': '\n\n'.join(ctrls)} + + +def generate_h(controls): + template = string.Template('''extern const Control<${type}> ${name};''') + + ctrls = [] + ids = [] + id_value = 1 + + for ctrl in controls: + name, ctrl = ctrl.popitem() + id_name = snake_case(name).upper() + + ids.append('\t' + id_name + ' = ' + str(id_value) + ',') + + info = { + 'name': name, + 'type': ctrl['type'], + } + + ctrls.append(template.substitute(info)) + id_value += 1 + + return {'ids': '\n'.join(ids), 'controls': '\n'.join(ctrls)} + + +def fill_template(template, data): + + template = open(template, 'rb').read() + template = template.decode('utf-8') + template = string.Template(template) + return template.substitute(data) + + +def main(argv): + + # Parse command line arguments + parser = argparse.ArgumentParser() + parser.add_argument('-o', dest='output', metavar='file', type=str, + help='Output file name. Defaults to standard output if not specified.') + parser.add_argument('input', type=str, + help='Input file name.') + parser.add_argument('template', type=str, + help='Template file name.') + args = parser.parse_args(argv[1:]) + + data = open(args.input, 'rb').read() + controls = yaml.safe_load(data)['controls'] + + if args.template.endswith('.cpp.in'): + data = generate_cpp(controls) + elif args.template.endswith('.h.in'): + data = generate_h(controls) + else: + raise RuntimeError('Unknown template type') + + data = fill_template(args.template, data) + + if args.output: + output = open(args.output, 'wb') + output.write(data.encode('utf-8')) + output.close() + else: + sys.stdout.write(data) + + return 0 + + +if __name__ == '__main__': + sys.exit(main(sys.argv)) diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 8123d1d5bee9..6df48365266d 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -5,7 +5,6 @@ libcamera_sources = files([ 'camera_manager.cpp', 'camera_sensor.cpp', 'controls.cpp', - 'control_ids.cpp', 'device_enumerator.cpp', 'device_enumerator_sysfs.cpp', 'event_dispatcher.cpp', @@ -58,6 +57,17 @@ if libudev.found() ]) endif +gen_controls = files('gen-controls.py') + +control_ids_cpp = custom_target('control_ids_cpp', + input : files('control_ids.yaml', 'control_ids.cpp.in'), + output : 'control_ids.cpp', + depend_files : gen_controls, + command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@']) + +libcamera_sources += control_ids_cpp +#libcamera_sources += control_ids_h + gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh') version_cpp = vcs_tag(command : [gen_version, meson.build_root()], From patchwork Sun Sep 29 19:02:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2061 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5DB0B6191C for ; Sun, 29 Sep 2019 21:03:13 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id F040F320 for ; Sun, 29 Sep 2019 21:03:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1569783793; bh=2NrNo8KQ7Wfni7wSOayjhItbOtGwdOr4gfL9KiZ8m9E=; h=From:To:Subject:Date:In-Reply-To:References:From; b=Yk82cAQ5Cl5f3AQH/0A6torZC1j8A44wmw3HM6aRCRF8gI195OIlOixu75fmvSpcM xW0djX/CUuIzMQe/uekzBcOoioS40Fj/n6SxxrpMiEtA+MQUDSADIZxI/Yrfr0BSRG xXfdxgsTWajtEVhGpG3HsZpPEIHhOWcjM9Q7H2oc= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 29 Sep 2019 22:02:47 +0300 Message-Id: <20190929190254.18920-7-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> References: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 06/13] libcamera: controls: Remove the unused ControlList::update() method X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Sep 2019 19:03:15 -0000 The ControlList::update() method is unused. While it is meant to fulfil a need of applications, having no user means that it is most probably not correctly designed. Remove the method, we will add it back later if needed. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/libcamera/controls.h | 2 -- src/libcamera/controls.cpp | 28 ---------------------- test/controls/control_list.cpp | 43 ---------------------------------- 3 files changed, 73 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 9698bd1dd383..d4a74ada1b6a 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -158,8 +158,6 @@ public: val->set(value); } - void update(const ControlList &list); - private: const ControlValue *find(const ControlId &id) const; ControlValue *find(const ControlId &id); diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index a34af588fc7e..5e8b3a9b5184 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -512,32 +512,4 @@ ControlValue *ControlList::find(const ControlId &id) return &controls_[&id]; } -/** - * \brief Update the list with a union of itself and \a other - * \param other The other list - * - * Update the control list to include all values from the \a other list. - * Elements in the list whose control IDs are contained in \a other are updated - * with the value from \a other. Elements in the \a other list that have no - * corresponding element in the list are added to the list with their value. - * - * The behaviour is undefined if the two lists refer to different Camera - * instances. - */ -void ControlList::update(const ControlList &other) -{ - if (other.camera_ != camera_) { - LOG(Controls, Error) - << "Can't update ControlList from a different camera"; - return; - } - - for (auto it : other) { - const ControlId *id = it.first; - const ControlValue &value = it.second; - - controls_[id] = value; - } -} - } /* namespace libcamera */ diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp index 053696814b67..8469ecf09439 100644 --- a/test/controls/control_list.cpp +++ b/test/controls/control_list.cpp @@ -141,49 +141,6 @@ protected: return TestFail; } - /* - * Test list merging. Create a new list, add two controls with - * one overlapping the existing list, merge the lists and clear - * the old list. Verify that the new list is empty and that the - * new list contains the expected items and values. - */ - ControlList newList(camera_.get()); - - newList.set(controls::Brightness, 128); - newList.set(controls::Saturation, 255); - newList.update(list); - - list.clear(); - - if (list.size() != 0) { - cout << "Old List should contain zero items" << endl; - return TestFail; - } - - if (!list.empty()) { - cout << "Old List should be empty" << endl; - return TestFail; - } - - if (newList.size() != 3) { - cout << "New list has incorrect size" << endl; - return TestFail; - } - - if (!newList.contains(controls::Brightness) || - !newList.contains(controls::Contrast) || - !newList.contains(controls::Saturation)) { - cout << "New list contains incorrect items" << endl; - return TestFail; - } - - if (newList.get(controls::Brightness) != 10 || - newList.get(controls::Contrast) != 20 || - newList.get(controls::Saturation) != 255) { - cout << "New list contains incorrect values" << endl; - return TestFail; - } - return TestPass; } From patchwork Sun Sep 29 19:02:48 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2062 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A1E086165A for ; Sun, 29 Sep 2019 21:03:13 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 4CC9F813 for ; Sun, 29 Sep 2019 21:03:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1569783793; bh=GhSM4t5wSxG2WSohu5R5J4gcN4xOlJpdKYlGXScBytE=; h=From:To:Subject:Date:In-Reply-To:References:From; b=lXVymg0/3wcKU0YZnTyEFg2GWWtX+daupFCyt/m5N71VkWCeX1pCtLlH5usf9fI9S XQ8UkeqtlZp46rC7weKIFih8b7esol6B9J/PI7IweutwDUTIoWaWmnNRJC+Lv90qMa M+0hjIsI/13fHo3jYbWQvvhpYHjdP1NyIhxAvfnk= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 29 Sep 2019 22:02:48 +0300 Message-Id: <20190929190254.18920-8-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> References: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 07/13] libcamera: controls: Remove ControlInfo::id X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Sep 2019 19:03:15 -0000 The ControlInfo id member is only used in the toString() method of the class, and nowhere else externally. The same way that ControlValue doesn't store a ControlId, ControlInfo shouldn't. Remove it. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/libcamera/controls.h | 4 +--- src/libcamera/controls.cpp | 13 +++---------- src/libcamera/pipeline/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc.cpp | 2 +- test/controls/control_info.cpp | 18 ++---------------- 5 files changed, 8 insertions(+), 31 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index d4a74ada1b6a..854ea3b84267 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -98,17 +98,15 @@ private: class ControlInfo { public: - explicit ControlInfo(const ControlId &id, const ControlValue &min = 0, + explicit ControlInfo(const ControlValue &min = 0, const ControlValue &max = 0); - const ControlId &id() const { return id_; } const ControlValue &min() const { return min_; } const ControlValue &max() const { return max_; } std::string toString() const; private: - const ControlId &id_; ControlValue min_; ControlValue max_; }; diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 5e8b3a9b5184..526b7755b390 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -323,22 +323,15 @@ Control::Control(unsigned int id, const char *name) /** * \brief Construct a ControlInfo with minimum and maximum range parameters - * \param[in] id The control ID * \param[in] min The control minimum value * \param[in] max The control maximum value */ -ControlInfo::ControlInfo(const ControlId &id, const ControlValue &min, +ControlInfo::ControlInfo(const ControlValue &min, const ControlValue &max) - : id_(id), min_(min), max_(max) + : min_(min), max_(max) { } -/** - * \fn ControlInfo::id() - * \brief Retrieve the control ID - * \return The control ID - */ - /** * \fn ControlInfo::min() * \brief Retrieve the minimum value of the control @@ -358,7 +351,7 @@ std::string ControlInfo::toString() const { std::stringstream ss; - ss << id_.name() << "[" << min_.toString() << ".." << max_.toString() << "]"; + ss << "[" << min_.toString() << ".." << max_.toString() << "]"; return ss.str(); } diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index d5d30932870a..2ac582d77801 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -364,7 +364,7 @@ int UVCCameraData::init(MediaEntity *entity) controlInfo_.emplace(std::piecewise_construct, std::forward_as_tuple(id), - std::forward_as_tuple(*id, info.min(), info.max())); + std::forward_as_tuple(info.min(), info.max())); } return 0; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 608a47aecf76..ec9c1cd2d484 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -437,7 +437,7 @@ int VimcCameraData::init(MediaDevice *media) controlInfo_.emplace(std::piecewise_construct, std::forward_as_tuple(id), - std::forward_as_tuple(*id, info.min(), info.max())); + std::forward_as_tuple(info.min(), info.max())); } return 0; diff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp index dbc43df8e3d3..9cf59185e459 100644 --- a/test/controls/control_info.cpp +++ b/test/controls/control_info.cpp @@ -24,14 +24,7 @@ protected: * Test information retrieval from a control with no minimum * and maximum. */ - ControlInfo brightness(controls::Brightness); - - if (brightness.id() != controls::Brightness || - brightness.id().type() != ControlTypeInteger32 || - brightness.id().name() != std::string("Brightness")) { - cout << "Invalid control identification for Brightness" << endl; - return TestFail; - } + ControlInfo brightness; if (brightness.min().get() != 0 || brightness.max().get() != 0) { @@ -43,14 +36,7 @@ protected: * Test information retrieval from a control with a minimum and * a maximum value. */ - ControlInfo contrast(controls::Contrast, 10, 200); - - if (contrast.id() != controls::Contrast || - contrast.id().type() != ControlTypeInteger32 || - contrast.id().name() != std::string("Contrast")) { - cout << "Invalid control identification for Contrast" << endl; - return TestFail; - } + ControlInfo contrast(10, 200); if (contrast.min().get() != 10 || contrast.max().get() != 200) { From patchwork Sun Sep 29 19:02:49 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2063 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 075E861658 for ; Sun, 29 Sep 2019 21:03:14 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 9ABB3A28 for ; Sun, 29 Sep 2019 21:03:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1569783793; bh=g5Vj+UMl6l7CQJLTD1xZ1CK/ATT83WQIXj4fyWMCptg=; h=From:To:Subject:Date:In-Reply-To:References:From; b=KV3JONIPAD8+iOKv+F4OEbnPx3c7cSymWuxMUaBu4RBJeBQdkteUG/NVZ8Jbb/O+k l0wJ6vlbkE20EKv1ehJmFmZ5FI2UjLOu0rqmz9siTGBQbla2O9n2s7x/C7YcrmJIex N7q+QaRIBzMF8VIrTGMhm6H8FIYEN1tGaRLf/5TE= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 29 Sep 2019 22:02:49 +0300 Message-Id: <20190929190254.18920-9-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> References: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 08/13] libcamera: controls: Rename ControlInfo to ControlRange X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Sep 2019 19:03:15 -0000 The ControlInfo class stores a range of valid values for a control. Its name is vague, as "info" has multiple meanings. Rename it to ControlRange. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/libcamera/controls.h | 8 +++--- src/libcamera/controls.cpp | 28 +++++++++---------- .../{control_info.cpp => control_range.cpp} | 14 +++++----- test/controls/meson.build | 2 +- 4 files changed, 26 insertions(+), 26 deletions(-) rename test/controls/{control_info.cpp => control_range.cpp} (75%) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 854ea3b84267..d3eea643c0ec 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -95,11 +95,11 @@ private: Control &operator=(const Control &) = delete; }; -class ControlInfo +class ControlRange { public: - explicit ControlInfo(const ControlValue &min = 0, - const ControlValue &max = 0); + explicit ControlRange(const ControlValue &min = 0, + const ControlValue &max = 0); const ControlValue &min() const { return min_; } const ControlValue &max() const { return max_; } @@ -111,7 +111,7 @@ private: ControlValue max_; }; -using ControlInfoMap = std::unordered_map; +using ControlInfoMap = std::unordered_map; class ControlList { diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 526b7755b390..a7e9d069b31a 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -312,42 +312,42 @@ Control::Control(unsigned int id, const char *name) #endif /* __DOXYGEN__ */ /** - * \class ControlInfo - * \brief Describe the information and capabilities of a Control + * \class ControlRange + * \brief Describe the limits of valid values for a Control * - * The ControlInfo represents control specific meta-data which is constant on a - * per camera basis. ControlInfo classes are constructed by pipeline handlers - * to expose the controls they support and the metadata needed to utilise those - * controls. + * The ControlRange expresses the constraints on valid values for a control. + * The constraints depend on the object the control applies to, and are + * constant for the lifetime of that object. They are typically constructed by + * pipeline handlers to describe the controls they support. */ /** - * \brief Construct a ControlInfo with minimum and maximum range parameters + * \brief Construct a ControlRange with minimum and maximum range parameters * \param[in] min The control minimum value * \param[in] max The control maximum value */ -ControlInfo::ControlInfo(const ControlValue &min, - const ControlValue &max) +ControlRange::ControlRange(const ControlValue &min, + const ControlValue &max) : min_(min), max_(max) { } /** - * \fn ControlInfo::min() + * \fn ControlRange::min() * \brief Retrieve the minimum value of the control * \return A ControlValue with the minimum value for the control */ /** - * \fn ControlInfo::max() + * \fn ControlRange::max() * \brief Retrieve the maximum value of the control * \return A ControlValue with the maximum value for the control */ /** - * \brief Provide a string representation of the ControlInfo + * \brief Provide a string representation of the ControlRange */ -std::string ControlInfo::toString() const +std::string ControlRange::toString() const { std::stringstream ss; @@ -358,7 +358,7 @@ std::string ControlInfo::toString() const /** * \typedef ControlInfoMap - * \brief A map of ControlId to ControlInfo + * \brief A map of ControlId to ControlRange */ /** diff --git a/test/controls/control_info.cpp b/test/controls/control_range.cpp similarity index 75% rename from test/controls/control_info.cpp rename to test/controls/control_range.cpp index 9cf59185e459..06ec3506ee62 100644 --- a/test/controls/control_info.cpp +++ b/test/controls/control_range.cpp @@ -2,7 +2,7 @@ /* * Copyright (C) 2019, Google Inc. * - * control_info.cpp - ControlInfo tests + * control_range.cpp - ControlRange tests */ #include @@ -15,16 +15,16 @@ using namespace std; using namespace libcamera; -class ControlInfoTest : public Test +class ControlRangeTest : public Test { protected: int run() { /* - * Test information retrieval from a control with no minimum - * and maximum. + * Test information retrieval from a range with no minimum and + * maximum. */ - ControlInfo brightness; + ControlRange brightness; if (brightness.min().get() != 0 || brightness.max().get() != 0) { @@ -36,7 +36,7 @@ protected: * Test information retrieval from a control with a minimum and * a maximum value. */ - ControlInfo contrast(10, 200); + ControlRange contrast(10, 200); if (contrast.min().get() != 10 || contrast.max().get() != 200) { @@ -48,4 +48,4 @@ protected: } }; -TEST_REGISTER(ControlInfoTest) +TEST_REGISTER(ControlRangeTest) diff --git a/test/controls/meson.build b/test/controls/meson.build index f4fc7b947dd9..9f0f005a2759 100644 --- a/test/controls/meson.build +++ b/test/controls/meson.build @@ -1,6 +1,6 @@ control_tests = [ - [ 'control_info', 'control_info.cpp' ], [ 'control_list', 'control_list.cpp' ], + [ 'control_range', 'control_range.cpp' ], [ 'control_value', 'control_value.cpp' ], ] From patchwork Sun Sep 29 19:02:50 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2064 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5CFFF6191E for ; Sun, 29 Sep 2019 21:03:14 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id EA689813 for ; Sun, 29 Sep 2019 21:03:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1569783794; bh=djiY8n788vm61+jwm/2NZt++j1AV044iIyQGJaLNe/E=; h=From:To:Subject:Date:In-Reply-To:References:From; b=k9x9U7/nmCY8Ua+7GngGcxP67fqEk67WjNNC04LeZoNV34hNwwsVurfwzAYZSWCTi 5QOfWvPfAJreu97gVs7mZhCSjjc0WuKz4G5mSt+tDsooFDzMvNryH3FozHIAEh/01n /SBuIZrOPEfqXSIyQBHdF7W+8SuS8D7Ww3E8cfW8= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 29 Sep 2019 22:02:50 +0300 Message-Id: <20190929190254.18920-10-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> References: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 09/13] libcamera: v4l2_controls: Use the ControlValue class for data storage X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Sep 2019 19:03:15 -0000 Use the ControlValue class to replace the manually crafted data storage in V4L2Control. This will help sharing code when more data types will be supported. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- src/libcamera/include/v4l2_controls.h | 15 +++++++++------ src/libcamera/pipeline/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc.cpp | 2 +- src/libcamera/v4l2_controls.cpp | 19 ++++++++++--------- src/libcamera/v4l2_device.cpp | 8 ++++---- 5 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h index 10b726504951..f2b67c5d44e1 100644 --- a/src/libcamera/include/v4l2_controls.h +++ b/src/libcamera/include/v4l2_controls.h @@ -16,6 +16,8 @@ #include #include +#include + namespace libcamera { class V4L2ControlInfo @@ -46,17 +48,18 @@ using V4L2ControlInfoMap = std::map; class V4L2Control { public: - V4L2Control(unsigned int id, int value = 0) - : id_(id), value_(value) {} - - int64_t value() const { return value_; } - void setValue(int64_t value) { value_ = value; } + V4L2Control(unsigned int id, const ControlValue &value = ControlValue()) + : id_(id), value_(value) + { + } unsigned int id() const { return id_; } + const ControlValue &value() const { return value_; } + ControlValue &value() { return value_; } private: unsigned int id_; - int64_t value_; + ControlValue value_; }; class V4L2ControlList diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 2ac582d77801..860578d41875 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -252,7 +252,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) LOG(UVC, Debug) << "Setting control 0x" << std::hex << std::setw(8) << ctrl.id() << std::dec - << " to " << ctrl.value(); + << " to " << ctrl.value().toString(); int ret = data->video_->setControls(&controls); if (ret) { diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index ec9c1cd2d484..b2b36b238809 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -300,7 +300,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) LOG(VIMC, Debug) << "Setting control 0x" << std::hex << std::setw(8) << ctrl.id() << std::dec - << " to " << ctrl.value(); + << " to " << ctrl.value().toString(); int ret = data->sensor_->setControls(&controls); if (ret) { diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp index 84258d9954d0..64f0555fff7c 100644 --- a/src/libcamera/v4l2_controls.cpp +++ b/src/libcamera/v4l2_controls.cpp @@ -143,6 +143,16 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl) * \param value The control value */ +/** + * \fn V4L2Control::value() const + * \brief Retrieve the value of the control + * + * This method is a const version of V4L2Control::value(), returning a const + * reference to the value. + * + * \return The V4L2 control value + */ + /** * \fn V4L2Control::value() * \brief Retrieve the value of the control @@ -154,15 +164,6 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl) * \return The V4L2 control value */ -/** - * \fn V4L2Control::setValue() - * \brief Set the value of the control - * \param value The new V4L2 control value - * - * This method stores the control value, which will be applied to the - * device when calling V4L2Device::setControls(). - */ - /** * \fn V4L2Control::id() * \brief Retrieve the control ID this instance refers to diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 349bf2d29704..fd4b9c6d5c62 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -264,14 +264,14 @@ int V4L2Device::setControls(V4L2ControlList *ctrls) /* Set the v4l2_ext_control value for the write operation. */ switch (info->type()) { case V4L2_CTRL_TYPE_INTEGER64: - v4l2Ctrls[i].value64 = ctrl->value(); + v4l2Ctrls[i].value64 = ctrl->value().get(); break; default: /* * \todo To be changed when support for string and * compound controls will be added. */ - v4l2Ctrls[i].value = ctrl->value(); + v4l2Ctrls[i].value = ctrl->value().get(); break; } } @@ -393,14 +393,14 @@ void V4L2Device::updateControls(V4L2ControlList *ctrls, switch (info->type()) { case V4L2_CTRL_TYPE_INTEGER64: - ctrl->setValue(v4l2Ctrl->value64); + ctrl->value().set(v4l2Ctrl->value64); break; default: /* * \todo To be changed when support for string and * compound controls will be added. */ - ctrl->setValue(v4l2Ctrl->value); + ctrl->value().set(v4l2Ctrl->value); break; } } From patchwork Sun Sep 29 19:02:51 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2065 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AE47761656 for ; Sun, 29 Sep 2019 21:03:14 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 4A3F1A28 for ; Sun, 29 Sep 2019 21:03:14 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1569783794; bh=1JBHVC3zgWeqhxg+KpD/3fDP3piOGg+VVu2VfNKAqgg=; h=From:To:Subject:Date:In-Reply-To:References:From; b=tn7mv2m9DNfXvohzPHtSqUe+BbA/YE/3dCTaP6OW6QYo1VRqC6dQNHL3ll64ygOtC TywjmUdO4ZCFCh3oRu3v2AtaCftf/VP1vmYguOt1Mu+DOI9u7WtdcB5vdIywbFoiSy 8BWb3pduPkOnds+GjtDyOxpo1LMW2prkoG0+iaLM= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 29 Sep 2019 22:02:51 +0300 Message-Id: <20190929190254.18920-11-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> References: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 10/13] libcamera: v4l2_controls: Use the ControlRange class for control info X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Sep 2019 19:03:15 -0000 Use the ControlRange class to express the range of a V4L2 control, replacing the open-coded minimum and maximum fields in the V4L2ControlInfo class. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- src/libcamera/include/v4l2_controls.h | 6 ++---- src/libcamera/pipeline/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc.cpp | 2 +- src/libcamera/v4l2_controls.cpp | 21 ++++++++++----------- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h index f2b67c5d44e1..b39370b2e90e 100644 --- a/src/libcamera/include/v4l2_controls.h +++ b/src/libcamera/include/v4l2_controls.h @@ -30,8 +30,7 @@ public: size_t size() const { return size_; } const std::string &name() const { return name_; } - int64_t min() const { return min_; } - int64_t max() const { return max_; } + const ControlRange &range() const { return range_; } private: unsigned int id_; @@ -39,8 +38,7 @@ private: size_t size_; std::string name_; - int64_t min_; - int64_t max_; + ControlRange range_; }; using V4L2ControlInfoMap = std::map; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 860578d41875..88f7fb9bc568 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -364,7 +364,7 @@ int UVCCameraData::init(MediaEntity *entity) controlInfo_.emplace(std::piecewise_construct, std::forward_as_tuple(id), - std::forward_as_tuple(info.min(), info.max())); + std::forward_as_tuple(info.range())); } return 0; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index b2b36b238809..26477dccbb8f 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -437,7 +437,7 @@ int VimcCameraData::init(MediaDevice *media) controlInfo_.emplace(std::piecewise_construct, std::forward_as_tuple(id), - std::forward_as_tuple(info.min(), info.max())); + std::forward_as_tuple(info.range())); } return 0; diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp index 64f0555fff7c..6f5f1578b139 100644 --- a/src/libcamera/v4l2_controls.cpp +++ b/src/libcamera/v4l2_controls.cpp @@ -74,8 +74,13 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl) type_ = ctrl.type; name_ = static_cast(ctrl.name); size_ = ctrl.elem_size * ctrl.elems; - min_ = ctrl.minimum; - max_ = ctrl.maximum; + + if (ctrl.type == V4L2_CTRL_TYPE_INTEGER64) + range_ = ControlRange(static_cast(ctrl.minimum), + static_cast(ctrl.maximum)); + else + range_ = ControlRange(static_cast(ctrl.minimum), + static_cast(ctrl.maximum)); } /** @@ -103,15 +108,9 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl) */ /** - * \fn V4L2ControlInfo::min() - * \brief Retrieve the control minimum value - * \return The V4L2 control minimum value - */ - -/** - * \fn V4L2ControlInfo::max() - * \brief Retrieve the control maximum value - * \return The V4L2 control maximum value + * \fn V4L2ControlInfo::range() + * \brief Retrieve the control value range + * \return The V4L2 control value range */ /** From patchwork Sun Sep 29 19:02:52 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2066 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 033456170D for ; Sun, 29 Sep 2019 21:03:15 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 9E1ED813 for ; Sun, 29 Sep 2019 21:03:14 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1569783794; bh=SB2SIV2WuUsC4640PIGf1O2GfLA95GL8Lp8db7LO8IM=; h=From:To:Subject:Date:In-Reply-To:References:From; b=i+aHl8S8hlpNJ2l7mv0/I4MrChXQvELzZcWYEmi7G5cO/Fn8dssBnUwEAZO9elIMy EdLpW+10G/Z5eRi4ndklxYGRTufx/+fa3/8gTq4D36hrtHA75dEaNC0pB2cUKn+Oh4 +TPLkXsfaCOqWGUoY0KsvUfhW0B9hXG6mj8NC+0Q= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 29 Sep 2019 22:02:52 +0300 Message-Id: <20190929190254.18920-12-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> References: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 11/13] libcamera: Add ControlValidator X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Sep 2019 19:03:15 -0000 The new abstract ControlValidator class defines an interface that will be used by the ControlList class to validate controls. This will allow controls to the validated against different object types, such as Camera and V4L2Device. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/control_validator.cpp | 45 +++++++++++++++++++++++ src/libcamera/include/control_validator.h | 27 ++++++++++++++ src/libcamera/include/meson.build | 1 + src/libcamera/meson.build | 1 + 4 files changed, 74 insertions(+) create mode 100644 src/libcamera/control_validator.cpp create mode 100644 src/libcamera/include/control_validator.h diff --git a/src/libcamera/control_validator.cpp b/src/libcamera/control_validator.cpp new file mode 100644 index 000000000000..8e5cf3c3e3ee --- /dev/null +++ b/src/libcamera/control_validator.cpp @@ -0,0 +1,45 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * control_validator.cpp - Control validator + */ + +#include "control_validator.h" + +/** + * \file control_validator.h + * \brief Abstract control validator + */ + +namespace libcamera { + +/** + * \class ControlValidator + * \brief Interface for the control validator + * + * The ControlValidator class is used by the ControlList class to validate + * controls added to the list. It is an abstract class providing an interface + * for object-specific control validation, such a Camera controls and V4L2 + * controls. + */ + +/** + * \fn ControlValidator::name() + * \brief Retrieve the name of the object associated with the validator + * \return The name of the object associated with the validator + */ + +/** + * \fn ControlValidator::validate() + * \brief Validate a control + * \param[in] id The control ID + * + * This method validates the control \a id against the object corresponding to + * the validator. It shall at least validate that the control is applicable to + * the object instance, and may perform additional checks. + * + * \return True if the control is valid, false otherwise + */ + +} /* namespace libcamera */ diff --git a/src/libcamera/include/control_validator.h b/src/libcamera/include/control_validator.h new file mode 100644 index 000000000000..3598b18f2f26 --- /dev/null +++ b/src/libcamera/include/control_validator.h @@ -0,0 +1,27 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * control_validator.h - Control validator + */ +#ifndef __LIBCAMERA_CONTROL_VALIDATOR_H__ +#define __LIBCAMERA_CONTROL_VALIDATOR_H__ + +#include + +namespace libcamera { + +class ControlId; + +class ControlValidator +{ +public: + virtual ~ControlValidator() {} + + virtual const std::string &name() const = 0; + virtual bool validate(const ControlId &id) const = 0; +}; + +} /* namespace libcamera */ + +#endif /* __LIBCAMERA_CONTROL_VALIDATOR_H__ */ diff --git a/src/libcamera/include/meson.build b/src/libcamera/include/meson.build index 933be8543a8d..1cf47204f2b5 100644 --- a/src/libcamera/include/meson.build +++ b/src/libcamera/include/meson.build @@ -1,5 +1,6 @@ libcamera_headers = files([ 'camera_sensor.h', + 'control_validator.h', 'device_enumerator.h', 'device_enumerator_sysfs.h', 'device_enumerator_udev.h', diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 6df48365266d..c8a66cfc93d5 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -5,6 +5,7 @@ libcamera_sources = files([ 'camera_manager.cpp', 'camera_sensor.cpp', 'controls.cpp', + 'control_validator.cpp', 'device_enumerator.cpp', 'device_enumerator_sysfs.cpp', 'event_dispatcher.cpp', From patchwork Sun Sep 29 19:02:53 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2067 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5116C6165A for ; Sun, 29 Sep 2019 21:03:15 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id EEC7D320 for ; Sun, 29 Sep 2019 21:03:14 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1569783795; bh=/v+F4BkbSs2L888fNEvyizetOU7Xyh5VfkYLkMZpepU=; h=From:To:Subject:Date:In-Reply-To:References:From; b=hhrKocVMRIQveAhM+G0kLxBC8w2aYm2VM9K9iEMEg5662bU1KMKg2xlTNJUn9cRhq a8quyL4zqoubicLRFPjH5ukmhku3bWbEKrxx5kibFVIEK+xNeyJ1uKP8SdDWNqt8E7 72KYedj7MkWsQ6BHkxxnha1pHg+QeOx2NF/vKTyo= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 29 Sep 2019 22:02:53 +0300 Message-Id: <20190929190254.18920-13-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> References: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 12/13] libcamera: Add ControlValidator implementation for Camera X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Sep 2019 19:03:15 -0000 Add a new CameraControlValidator class that implements the ControlValidator interface for a Camera object. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/camera_controls.cpp | 53 +++++++++++++++++++++++++ src/libcamera/include/camera_controls.h | 30 ++++++++++++++ src/libcamera/include/meson.build | 1 + src/libcamera/meson.build | 1 + 4 files changed, 85 insertions(+) create mode 100644 src/libcamera/camera_controls.cpp create mode 100644 src/libcamera/include/camera_controls.h diff --git a/src/libcamera/camera_controls.cpp b/src/libcamera/camera_controls.cpp new file mode 100644 index 000000000000..341da56019f7 --- /dev/null +++ b/src/libcamera/camera_controls.cpp @@ -0,0 +1,53 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * camera_controls.cpp - Camera controls + */ + +#include "camera_controls.h" + +#include +#include + +/** + * \file camera_controls.h + * \brief Controls for Camera instances + */ + +namespace libcamera { + +/** + * \class CameraControlValidator + * \brief A control validator for Camera instances + * + * This ControlValidator specialisation validates that controls exist in the + * Camera associated with the validator. + */ + +/** + * \brief Construst a CameraControlValidator for the \a camera + * \param[in] camera The camera + */ +CameraControlValidator::CameraControlValidator(Camera *camera) + : camera_(camera) +{ +} + +const std::string &CameraControlValidator::name() const +{ + return camera_->name(); +} + +/** + * \brief Validate a control + * \param[in] id The control ID + * \return True if the control is valid, false otherwise + */ +bool CameraControlValidator::validate(const ControlId &id) const +{ + const ControlInfoMap &controls = camera_->controls(); + return controls.find(&id) != controls.end(); +} + +} /* namespace libcamera */ diff --git a/src/libcamera/include/camera_controls.h b/src/libcamera/include/camera_controls.h new file mode 100644 index 000000000000..998a2d155a44 --- /dev/null +++ b/src/libcamera/include/camera_controls.h @@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * camera_controls.h - Camera controls + */ +#ifndef __LIBCAMERA_CAMERA_CONTROLS_H__ +#define __LIBCAMERA_CAMERA_CONTROLS_H__ + +#include "control_validator.h" + +namespace libcamera { + +class Camera; + +class CameraControlValidator final : public ControlValidator +{ +public: + CameraControlValidator(Camera *camera); + + const std::string &name() const override; + bool validate(const ControlId &id) const override; + +private: + Camera *camera_; +}; + +} /* namespace libcamera */ + +#endif /* __LIBCAMERA_CAMERA_CONTROLS_H__ */ diff --git a/src/libcamera/include/meson.build b/src/libcamera/include/meson.build index 1cf47204f2b5..2c74d29bd925 100644 --- a/src/libcamera/include/meson.build +++ b/src/libcamera/include/meson.build @@ -1,4 +1,5 @@ libcamera_headers = files([ + 'camera_controls.h', 'camera_sensor.h', 'control_validator.h', 'device_enumerator.h', diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index c8a66cfc93d5..3a3b388a6a34 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -2,6 +2,7 @@ libcamera_sources = files([ 'bound_method.cpp', 'buffer.cpp', 'camera.cpp', + 'camera_controls.cpp', 'camera_manager.cpp', 'camera_sensor.cpp', 'controls.cpp', From patchwork Sun Sep 29 19:02:54 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2068 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B033C61964 for ; Sun, 29 Sep 2019 21:03:15 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 49A55813 for ; Sun, 29 Sep 2019 21:03:15 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1569783795; bh=ZrkAjAayFSXg8FArGn7fWuf5BhX8/3kODKypo7oG+bQ=; h=From:To:Subject:Date:In-Reply-To:References:From; b=D216+Wy2orli0UwsaevH/0BPyQUlsd1K8Myb6Swau3rk6hzCxd6suDFuxjxwOmZe1 /xFH3Rr8XqF9XJuF+i/mCWOS1rS46EHjhse8Di8USsTOPO0tCjJ4zHvfVjCHTdreb0 erJXPc8UEtdYlZayRLyGnVHd9Rqy++8z+Gd+WgtY= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 29 Sep 2019 22:02:54 +0300 Message-Id: <20190929190254.18920-14-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> References: <20190929190254.18920-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 13/13] libcamera: controls: Use ControlValidator to validate ControlList X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Sep 2019 19:03:16 -0000 Replace the manual validation of controls against a Camera with usage of the new ControlValidator interface. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/controls.h | 6 +++--- include/libcamera/request.h | 7 ++++--- src/libcamera/controls.cpp | 27 ++++++++++----------------- src/libcamera/request.cpp | 14 ++++++++++++-- test/controls/control_list.cpp | 15 ++++++++++++++- 5 files changed, 43 insertions(+), 26 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index d3eea643c0ec..90426753f40f 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -13,7 +13,7 @@ namespace libcamera { -class Camera; +class ControlValidator; enum ControlType { ControlTypeNone, @@ -119,7 +119,7 @@ private: using ControlListMap = std::unordered_map; public: - ControlList(Camera *camera); + ControlList(ControlValidator *validator); using iterator = ControlListMap::iterator; using const_iterator = ControlListMap::const_iterator; @@ -160,7 +160,7 @@ private: const ControlValue *find(const ControlId &id) const; ControlValue *find(const ControlId &id); - Camera *camera_; + ControlValidator *validator_; ControlListMap controls_; }; diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 9edf1cedc054..e3db5243aaf3 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -19,9 +19,9 @@ namespace libcamera { class Buffer; class Camera; +class CameraControlValidator; class Stream; - class Request { public: @@ -36,7 +36,7 @@ public: Request &operator=(const Request &) = delete; ~Request(); - ControlList &controls() { return controls_; } + ControlList &controls() { return *controls_; } const std::map &buffers() const { return bufferMap_; } int addBuffer(std::unique_ptr buffer); Buffer *findBuffer(Stream *stream) const; @@ -56,7 +56,8 @@ private: bool completeBuffer(Buffer *buffer); Camera *camera_; - ControlList controls_; + CameraControlValidator *validator_; + ControlList *controls_; std::map bufferMap_; std::unordered_set pending_; diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index a7e9d069b31a..f3260edce0bc 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -10,8 +10,7 @@ #include #include -#include - +#include "control_validator.h" #include "log.h" #include "utils.h" @@ -365,20 +364,16 @@ std::string ControlRange::toString() const * \class ControlList * \brief Associate a list of ControlId with their values for a camera * - * A ControlList wraps a map of ControlId to ControlValue and provide - * additional validation against the control information exposed by a Camera. - * - * A list is only valid for as long as the camera it refers to is valid. After - * that calling any method of the ControlList class other than its destructor - * will cause undefined behaviour. + * A ControlList wraps a map of ControlId to ControlValue and optionally + * validates controls against a ControlValidator. */ /** - * \brief Construct a ControlList with a reference to the Camera it applies on - * \param[in] camera The camera + * \brief Construct a ControlList with an optional control validator + * \param[in] validator The validator (may be null) */ -ControlList::ControlList(Camera *camera) - : camera_(camera) +ControlList::ControlList(ControlValidator *validator) + : validator_(validator) { } @@ -493,12 +488,10 @@ const ControlValue *ControlList::find(const ControlId &id) const ControlValue *ControlList::find(const ControlId &id) { - const ControlInfoMap &controls = camera_->controls(); - const auto iter = controls.find(&id); - if (iter == controls.end()) { + if (validator_ && !validator_->validate(id)) { LOG(Controls, Error) - << "Camera " << camera_->name() - << " does not support control " << id.name(); + << "Control " << id.name() + << " is not valid for " << validator_->name(); return nullptr; } diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index ee2158fc7a9c..19f6d0b9a0ae 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -13,6 +13,7 @@ #include #include +#include "camera_controls.h" #include "log.h" /** @@ -55,9 +56,15 @@ LOG_DEFINE_CATEGORY(Request) * */ Request::Request(Camera *camera, uint64_t cookie) - : camera_(camera), controls_(camera), cookie_(cookie), - status_(RequestPending), cancelled_(false) + : camera_(camera), cookie_(cookie), status_(RequestPending), + cancelled_(false) { + /** + * \todo Should the Camera expose a validator instance, to avoid + * creating a new instance for each request? + */ + validator_ = new CameraControlValidator(camera); + controls_ = new ControlList(validator_); } Request::~Request() @@ -66,6 +73,9 @@ Request::~Request() Buffer *buffer = it.second; delete buffer; } + + delete controls_; + delete validator_; } /** diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp index 8469ecf09439..1bcfecc467b5 100644 --- a/test/controls/control_list.cpp +++ b/test/controls/control_list.cpp @@ -12,6 +12,7 @@ #include #include +#include "camera_controls.h" #include "test.h" using namespace std; @@ -40,7 +41,8 @@ protected: int run() { - ControlList list(camera_.get()); + CameraControlValidator validator(camera_.get()); + ControlList list(&validator); /* Test that the list is initially empty. */ if (!list.empty()) { @@ -141,6 +143,17 @@ protected: return TestFail; } + /* + * Attempt to set an invalid control and verify that the + * operation failed. + */ + list.set(controls::AwbEnable, true); + + if (list.contains(controls::AwbEnable)) { + cout << "List shouldn't contain AwbEnable control" << endl; + return TestFail; + } + return TestPass; }