From patchwork Sat Oct 12 18:43: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: 2162 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 3AE9061563 for ; Sat, 12 Oct 2019 20:44:15 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 84F0D54C for ; Sat, 12 Oct 2019 20:44:14 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1570905854; bh=rUJFRV8cl37hkAEqYF6hFVVdtfsUwULZC17nOZieKxQ=; h=From:To:Subject:Date:In-Reply-To:References:From; b=Hb8yOGkRrwmsXAgxX1JixPwGVK6PFiyiG+hOVEvA4GlsPD6Ivm40FwX9owJZNgJpv 5KezhTjavyAJHSb7IP5KQngy/jDj+Sj4OuchYn0f2n9dgOZvzvAgykmeCpadI2CVPU 3A3+MOulwXM98WR4pdmb1zB4+O3jg1yVE77mWpyM= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Oct 2019 21:43:54 +0300 Message-Id: <20191012184407.31684-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> References: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 01/14] libcamera: pipeline: rkisp1: Avoid copy assignment of V4L2 control map 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: Sat, 12 Oct 2019 18:44:15 -0000 Use the std::map::emplace() method to avoid unnecessary creation of an empty V4L2ControlInfoMap folled by a copy assignment. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 029d5868d11f..32b023730009 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -777,7 +777,7 @@ int PipelineHandlerRkISP1::start(Camera *camera) }; std::map entityControls; - entityControls[0] = data->sensor_->controls(); + entityControls.emplace(0, data->sensor_->controls()); data->ipa_->configure(streamConfig, entityControls); From patchwork Sat Oct 12 18:43:55 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2163 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1192B6196E for ; Sat, 12 Oct 2019 20:44:16 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6D9BA33A for ; Sat, 12 Oct 2019 20:44:15 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1570905855; bh=SsVqKXaIVcdlWRXP2hsy95tJFefEHTZPpJr/BetEt8w=; h=From:To:Subject:Date:In-Reply-To:References:From; b=bFTGZU01ICwYWQFqcMoVsGuQbVKUwHfDC+NcD01OSIp7G7DFvKDtk2V/9AjoBYlDy nxK830fxvvQnaU6GK3XKNB041DC7vfAJnOp5qTAh4IaY7J1GwCSfgerlUgRvY3zDdW u0myajHAG6gd02K6GbDwkt3wSoypNZLJ/MYaJ+kY= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Oct 2019 21:43:55 +0300 Message-Id: <20191012184407.31684-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> References: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 02/14] libcamera: control_ids: Fix documentation for controls namespace 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: Sat, 12 Oct 2019 18:44:16 -0000 The controls namespace isn't documenting, making it impossible to reference the variables it contains. Furthermore, within the documentation page for the control_ids.h file, links from overview to detailed variable documentation are broken for the same reason. Both issues can be fixed by documenting the controls namespace. Unfortunately doxygen then fails to parse the initialisers for the controls global variables correctly and considers them as functions. To work around this, modify the control_ids.cpp generation script to hide the variables from doxygen, but still expose their documentation. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/control_ids.cpp.in | 13 ++++++++++++- src/libcamera/gen-controls.py | 18 ++++++++++++------ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in index f699ac9eea54..dd5433820a8b 100644 --- a/src/libcamera/control_ids.cpp.in +++ b/src/libcamera/control_ids.cpp.in @@ -16,9 +16,20 @@ namespace libcamera { +/** + * \brief Namespace for libcamera controls + */ namespace controls { -${controls} +${controls_doc} + +#ifndef __DOXYGEN__ +/* + * Keep the controls definitions hidden from doxygen as it incorrectly parses + * them as functions. + */ +${controls_def} +#endif } /* namespace controls */ diff --git a/src/libcamera/gen-controls.py b/src/libcamera/gen-controls.py index 0899e40b4080..a3e52fb36f7a 100755 --- a/src/libcamera/gen-controls.py +++ b/src/libcamera/gen-controls.py @@ -17,12 +17,14 @@ def snake_case(s): def generate_cpp(controls): - template = string.Template('''/** + doc_template = string.Template('''/** + * \\var extern const Control<${type}> ${name} ${description} - */ -extern const Control<${type}> ${name}(${id_name}, "${name}");''') + */''') + def_template = string.Template('extern const Control<${type}> ${name}(${id_name}, "${name}");') - ctrls = [] + ctrls_doc = [] + ctrls_def = [] for ctrl in controls: name, ctrl = ctrl.popitem() @@ -39,9 +41,13 @@ extern const Control<${type}> ${name}(${id_name}, "${name}");''') 'id_name': id_name, } - ctrls.append(template.substitute(info)) + ctrls_doc.append(doc_template.substitute(info)) + ctrls_def.append(def_template.substitute(info)) - return {'controls': '\n\n'.join(ctrls)} + return { + 'controls_doc': '\n\n'.join(ctrls_doc), + 'controls_def': '\n'.join(ctrls_def), + } def generate_h(controls): From patchwork Sat Oct 12 18:43:56 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2164 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 8A20361563 for ; Sat, 12 Oct 2019 20:44:17 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id AD08B33A for ; Sat, 12 Oct 2019 20:44:16 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1570905857; bh=SafN5xzragEoQRgOVMscoYT6s79il8oVw21JGF3HcSY=; h=From:To:Subject:Date:In-Reply-To:References:From; b=W7g+oTO6IHv7ijteDLVqjOqvVG6srNnUBP1zCHYX6YIH+YqFo0NbB7nc9T2nMD+nA ECjBGCiKofqtgwIIlab0dEgnTnEMT1fyHDTxI3ufVB8RAq6oauqKgvD30ybEZ87npX XTXiWQIfrajK+Y01HZ/LoeDBnK5Q78/AxgZCf+zQ= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Oct 2019 21:43:56 +0300 Message-Id: <20191012184407.31684-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> References: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 03/14] libcamera: control_ids: Generate map of all supported controls 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: Sat, 12 Oct 2019 18:44:17 -0000 In order to deserialise a ControlList, we will need to lookup ControlId instances based on their numerical ID. Generate a global map from the controls definitions to support such a lookup. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- Changes since v1: - Add ControlIdMap type --- include/libcamera/control_ids.h.in | 2 ++ include/libcamera/controls.h | 1 + src/libcamera/control_ids.cpp.in | 7 +++++++ src/libcamera/controls.cpp | 9 +++++++++ src/libcamera/gen-controls.py | 3 +++ 5 files changed, 22 insertions(+) diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in index 1d0bc791e559..6ff0e4186983 100644 --- a/include/libcamera/control_ids.h.in +++ b/include/libcamera/control_ids.h.in @@ -24,6 +24,8 @@ ${ids} ${controls} +extern const ControlIdMap controls; + } /* namespace controls */ } /* namespace libcamera */ diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 90426753f40f..d8acd800b143 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -111,6 +111,7 @@ private: ControlValue max_; }; +using ControlIdMap = std::unordered_map; using ControlInfoMap = std::unordered_map; class ControlList diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in index dd5433820a8b..99c511d0e712 100644 --- a/src/libcamera/control_ids.cpp.in +++ b/src/libcamera/control_ids.cpp.in @@ -31,6 +31,13 @@ ${controls_doc} ${controls_def} #endif +/** + * \brief List of all supported libcamera controls + */ +extern const ControlIdMap controls { +${controls_map} +}; + } /* namespace controls */ } /* namespace libcamera */ diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index f3260edce0bc..70c1af481af2 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -355,6 +355,15 @@ std::string ControlRange::toString() const return ss.str(); } +/** + * \typedef ControlIdMap + * \brief A map of numerical control ID to ControlId + * + * The map is used by ControlList instances to access controls by numerical + * IDs. A global map of all libcamera controls is provided by + * controls::controls. + */ + /** * \typedef ControlInfoMap * \brief A map of ControlId to ControlRange diff --git a/src/libcamera/gen-controls.py b/src/libcamera/gen-controls.py index a3e52fb36f7a..940386cc68c8 100755 --- a/src/libcamera/gen-controls.py +++ b/src/libcamera/gen-controls.py @@ -25,6 +25,7 @@ ${description} ctrls_doc = [] ctrls_def = [] + ctrls_map = [] for ctrl in controls: name, ctrl = ctrl.popitem() @@ -43,10 +44,12 @@ ${description} ctrls_doc.append(doc_template.substitute(info)) ctrls_def.append(def_template.substitute(info)) + ctrls_map.append('\t{ ' + id_name + ', &' + name + ' },') return { 'controls_doc': '\n\n'.join(ctrls_doc), 'controls_def': '\n'.join(ctrls_def), + 'controls_map': '\n'.join(ctrls_map), } From patchwork Sat Oct 12 18:43:57 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2165 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F7BB61563 for ; Sat, 12 Oct 2019 20:44:18 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id B473B33A for ; Sat, 12 Oct 2019 20:44:17 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1570905858; bh=w8a1lB+CzP86yPSLqH/maQu2rcKF90DtfQQx6a9ulnA=; h=From:To:Subject:Date:In-Reply-To:References:From; b=cRnxiTMZoG1jr/rVa30G1Fw74FvdlSSacrechawZjfPspWU6FwoZBeEqVZ/kMRXX5 l+k5oF26OSU9bqhsu2lmTkgCmSxgyGj4J0My7Ts7m9ZQhC8mbyqaf8CWrAFt0EMwYo p0HWGWuLf3nphhHY869HOMGjmrjF4ddDRGlJiXeE= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Oct 2019 21:43:57 +0300 Message-Id: <20191012184407.31684-5-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> References: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 04/14] libcamera: controls: Add comparison operators for ControlValue 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: Sat, 12 Oct 2019 18:44:18 -0000 Add equality and non equality comparison operators for the ControlValue class. This simplifies code that deals with control values. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/controls.h | 6 ++++++ src/libcamera/controls.cpp | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index d8acd800b143..342251c21018 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -40,6 +40,12 @@ public: std::string toString() const; + bool operator==(const ControlValue &other) const; + bool operator!=(const ControlValue &other) const + { + return !(*this == other); + } + private: ControlType type_; diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 70c1af481af2..bfab177fc508 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -194,6 +194,33 @@ std::string ControlValue::toString() const return ""; } +/** + * \brief Compare ControlValue instances for equality + * \return True if the values have identical types and values, false otherwise + */ +bool ControlValue::operator==(const ControlValue &other) const +{ + if (type_ != other.type_) + return false; + + switch (type_) { + case ControlTypeBool: + return bool_ == other.bool_; + case ControlTypeInteger32: + return integer32_ == other.integer32_; + case ControlTypeInteger64: + return integer64_ == other.integer64_; + default: + return false; + } +} + +/** + * \fn bool ControlValue::operator!=() + * \brief Compare ControlValue instances for non equality + * \return False if the values have identical types and values, true otherwise + */ + /** * \class ControlId * \brief Control static metadata From patchwork Sat Oct 12 18:43:58 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2166 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9B1CD6196D for ; Sat, 12 Oct 2019 20:44:19 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BC3A133A for ; Sat, 12 Oct 2019 20:44:18 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1570905859; bh=vIQdsnSW8EKMO3MIx8ujNjFnihSBMuxBWiqf/RBv70M=; h=From:To:Subject:Date:In-Reply-To:References:From; b=GlnC1GMsaamdQol8aXOPq5+tk3GvpxVvENtZMdpxAAkBwkjBN9SDXMW8v6FZ/B5Qs kawLTYzyu8dFIcboAT2GeyZTlu4lH7m6lCtUvyFPZVUUm3Hr0JURbGMIxSPGD+LLq7 /grs97AVJbsEhl9um4lGpRZwOe2nkmE9/jepVZ6o= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Oct 2019 21:43:58 +0300 Message-Id: <20191012184407.31684-6-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> References: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 05/14] libcamera: controls: Default ControlList validator argument to nullptr 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: Sat, 12 Oct 2019 18:44:19 -0000 The ControlList constructor takes a validator pointer that can be null. Set its default value to nullptr to simplify code in users of ControlList. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/controls.h | 2 +- src/ipa/rkisp1/rkisp1.cpp | 2 +- src/libcamera/request.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 342251c21018..12a13aacb198 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -126,7 +126,7 @@ private: using ControlListMap = std::unordered_map; public: - ControlList(ControlValidator *validator); + ControlList(ControlValidator *validator = nullptr); using iterator = ControlListMap::iterator; using const_iterator = ControlListMap::const_iterator; diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 02419e254f2d..80138f196184 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -220,7 +220,7 @@ void IPARkISP1::setControls(unsigned int frame) void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState) { - ControlList ctrls(nullptr); + ControlList ctrls; if (aeState) ctrls.set(controls::AeLocked, aeState == 2); diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 23d3ab6f422c..e800f1449888 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -69,7 +69,7 @@ Request::Request(Camera *camera, uint64_t cookie) /** * \todo: Add a validator for metadata controls. */ - metadata_ = new ControlList(nullptr); + metadata_ = new ControlList(); } Request::~Request() From patchwork Sat Oct 12 18:43:59 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2167 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 9FE1361978 for ; Sat, 12 Oct 2019 20:44:20 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id C527933A for ; Sat, 12 Oct 2019 20:44:19 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1570905860; bh=IA9VBgPUp8yRKmdQjnufsiRaJSC3uSX3RWPnb4Z3Y8U=; h=From:To:Subject:Date:In-Reply-To:References:From; b=CIhr+Qr3AYmprIMfS35pvzLj+9bTrX7l+XFT5Z45oRfOzxezRhY+X9f+gkhXF6IV5 odWZRt+95wFmf7BkcRz1bTLyLh4SiO21XqTHM1PbL6CAiFWef7vVwnHPYAS/lHZ0HA DLxMYwtSg8bmnGMFGPITBjWJanLhP5o5HzguSRMo= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Oct 2019 21:43:59 +0300 Message-Id: <20191012184407.31684-7-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> References: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 06/14] libcamera: controls: Store control name in ControlId 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: Sat, 12 Oct 2019 18:44:20 -0000 The ControlId class stores a pointer to the control name. This works fine for statically-defined controls, but requires code that allocates controls dynamically (for instance based on control discovery on a V4L2 device) to keep a list of control names in separate storage. To ease usage of dynamically allocated controls, store a copy of the control name string in the ControlId class. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/libcamera/controls.h | 6 +++--- src/libcamera/controls.cpp | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 12a13aacb198..999fcf7a3a62 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -60,11 +60,11 @@ class ControlId { public: unsigned int id() const { return id_; } - const char *name() const { return name_; } + const std::string &name() const { return name_; } ControlType type() const { return type_; } protected: - ControlId(unsigned int id, const char *name, ControlType type) + ControlId(unsigned int id, const std::string &name, ControlType type) : id_(id), name_(name), type_(type) { } @@ -74,7 +74,7 @@ private: ControlId &operator=(const ControlId &) = delete; unsigned int id_; - const char *name_; + std::string name_; ControlType type_; }; diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index bfab177fc508..292e48cd6d25 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -232,7 +232,7 @@ bool ControlValue::operator==(const ControlValue &other) const */ /** - * \fn ControlId::ControlId(unsigned int id, const char *name, ControlType type) + * \fn ControlId::ControlId(unsigned int id, const std::string &name, ControlType type) * \brief Construct a ControlId instance * \param[in] id The control numerical ID * \param[in] name The control name From patchwork Sat Oct 12 18:44:00 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2168 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 0E54161978 for ; Sat, 12 Oct 2019 20:44:21 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 9728B9C0 for ; Sat, 12 Oct 2019 20:44:20 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1570905860; bh=d0p6DvIdtTbSRMT48oSbkp/Qaj5k5oL64j2QfRmA5RQ=; h=From:To:Subject:Date:In-Reply-To:References:From; b=cAD3EgUjHiE4hUWB7OfDyKNG65EyX0ZQ4D5/JL2SyQF7b58+mNT90qNmfojvM6vQb gDCZzhhTUt1ose6X5beR9vGDMMotkHHH4L5bcH/CFv+F2hmCvDuXb6c9fFEERjq25P 9h+sQae2T20/9MARK03XTbWdiH0kSox2hVmix0ZA= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Oct 2019 21:44:00 +0300 Message-Id: <20191012184407.31684-8-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> References: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 07/14] libcamera: controls: Support accessing controls by numerical 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: Sat, 12 Oct 2019 18:44:21 -0000 The ControlList class has template get() and set() methods to get and set control values. The methods require a reference to a Control instance, which is only available when calling them with a hardcoded control. In order to support usage of ControlList for V4L2 controls, as well as serialisation and deserialisation of ControlList, we need a way to get and set control values based on a control numerical ID. Add new contains(), get() and set() overload methods to do so. As this change prepares the ControlList to be used for other objects than camera, update its documentation accordingly. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/libcamera/controls.h | 10 ++- src/ipa/rkisp1/rkisp1.cpp | 2 +- src/libcamera/controls.cpp | 143 ++++++++++++++++++++++++++------- src/libcamera/request.cpp | 5 +- test/controls/control_list.cpp | 2 +- 5 files changed, 125 insertions(+), 37 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 999fcf7a3a62..5e6708fe570b 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -126,7 +126,7 @@ private: using ControlListMap = std::unordered_map; public: - ControlList(ControlValidator *validator = nullptr); + ControlList(const ControlIdMap &idmap, ControlValidator *validator = nullptr); using iterator = ControlListMap::iterator; using const_iterator = ControlListMap::const_iterator; @@ -136,11 +136,13 @@ public: const_iterator begin() const { return controls_.begin(); } const_iterator end() const { return controls_.end(); } - bool contains(const ControlId &id) const; bool empty() const { return controls_.empty(); } std::size_t size() const { return controls_.size(); } void clear() { controls_.clear(); } + bool contains(const ControlId &id) const; + bool contains(unsigned int id) const; + template const T &get(const Control &ctrl) const { @@ -163,11 +165,15 @@ public: val->set(value); } + const ControlValue &get(unsigned int id) const; + void set(unsigned int id, const ControlValue &value); + private: const ControlValue *find(const ControlId &id) const; ControlValue *find(const ControlId &id); ControlValidator *validator_; + const ControlIdMap *idmap_; ControlListMap controls_; }; diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 80138f196184..b0d23dd154be 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -220,7 +220,7 @@ void IPARkISP1::setControls(unsigned int frame) void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState) { - ControlList ctrls; + ControlList ctrls(controls::controls); if (aeState) ctrls.set(controls::AeLocked, aeState == 2); diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 292e48cd6d25..ddd4e6680ce2 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -7,6 +7,7 @@ #include +#include #include #include @@ -16,13 +17,13 @@ /** * \file controls.h - * \brief Describes control framework and controls supported by a camera + * \brief Framework to handle manage controls related to an object * - * 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. + * A control is a mean to govern or influence the operation of an object, and in + * particular 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: @@ -274,7 +275,7 @@ bool ControlValue::operator==(const ControlValue &other) const * \class Control * \brief Describe a control and its intrinsic properties * - * The Control class models a control exposed by a camera. Its template type + * The Control class models a control exposed by an object. 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 @@ -293,8 +294,8 @@ bool ControlValue::operator==(const ControlValue &other) const * 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. + * the Control class to be created with the same ID for the same object, doing + * so may cause undefined behaviour. */ /** @@ -398,18 +399,28 @@ std::string ControlRange::toString() const /** * \class ControlList - * \brief Associate a list of ControlId with their values for a camera + * \brief Associate a list of ControlId with their values for an object * - * A ControlList wraps a map of ControlId to ControlValue and optionally - * validates controls against a ControlValidator. + * The ControlList class stores values of controls exposed by an object. The + * lists returned by the Request::controls() and Request::metadata() methods + * refer to the camera that the request belongs to. + * + * Control lists are constructed with a map of all the controls supported by + * their object, and an optional ControlValidator to further validate the + * controls. */ /** * \brief Construct a ControlList with an optional control validator + * \param[in] idmap The ControlId map for the control list target object * \param[in] validator The validator (may be null) + * + * For ControlList containing libcamera controls, a global map of all libcamera + * controls is provided by controls::controls and can be used as the \a idmap + * argument. */ -ControlList::ControlList(ControlValidator *validator) - : validator_(validator) +ControlList::ControlList(const ControlIdMap &idmap, ControlValidator *validator) + : validator_(validator), idmap_(&idmap) { } @@ -449,20 +460,6 @@ ControlList::ControlList(ControlValidator *validator) * list */ -/** - * \brief Check if the list contains a control with the specified \a id - * \param[in] id The control ID - * - * The behaviour is undefined if the control \a id is not supported by the - * camera that the ControlList refers to. - * - * \return True if the list contains a matching control, false otherwise - */ -bool ControlList::contains(const ControlId &id) const -{ - return controls_.find(&id) != controls_.end(); -} - /** * \fn ControlList::empty() * \brief Identify if the list is empty @@ -481,7 +478,33 @@ bool ControlList::contains(const ControlId &id) const */ /** - * \fn template const T &ControlList::get() const + * \brief Check if the list contains a control with the specified \a id + * \param[in] id The control ID + * + * \return True if the list contains a matching control, false otherwise + */ +bool ControlList::contains(const ControlId &id) const +{ + return controls_.find(&id) != controls_.end(); +} + +/** + * \brief Check if the list contains a control with the specified \a id + * \param[in] id The control numerical ID + * + * \return True if the list contains a matching control, false otherwise + */ +bool ControlList::contains(unsigned int id) const +{ + const auto iter = idmap_->find(id); + if (iter == idmap_->end()) + return false; + + return contains(*iter->second); +} + +/** + * \fn template const T &ControlList::get(const Control &ctrl) const * \brief Get the value of a control * \param[in] ctrl The control * @@ -496,7 +519,7 @@ bool ControlList::contains(const ControlId &id) const */ /** - * \fn template void ControlList::set() + * \fn template void ControlList::set(const Control &ctrl, const T &value) * \brief Set the control value to \a value * \param[in] ctrl The control * \param[in] value The control value @@ -506,9 +529,67 @@ bool ControlList::contains(const ControlId &id) const * to the list. * * The behaviour is undefined if the control \a ctrl is not supported by the - * camera that the list refers to. + * object that the list refers to. */ +/** + * \brief Get the value of control \a id + * \param[in] id The control numerical ID + * + * The behaviour is undefined if the control \a id is not present in the list. + * Use ControlList::contains() to test for the presence of a control in the + * list before retrieving its value. + * + * \return The control value + */ +const ControlValue &ControlList::get(unsigned int id) const +{ + static ControlValue zero; + + const auto ctrl = idmap_->find(id); + if (ctrl == idmap_->end()) { + LOG(Controls, Error) + << std::hex << std::setfill('0') + << "Control 0x" << std::setw(8) << id << " is not valid"; + return zero; + } + + const ControlValue *val = find(*ctrl->second); + if (!val) + return zero; + + return *val; +} + +/** + * \brief Set the value of control \a id to \a value + * \param[in] id The control ID + * \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 id is not supported by the + * object that the list refers to. + */ +void ControlList::set(unsigned int id, const ControlValue &value) +{ + const auto ctrl = idmap_->find(id); + if (ctrl == idmap_->end()) { + LOG(Controls, Error) + << std::hex << std::setfill('0') + << "Control 0x" << std::setw(8) << id << " is not valid"; + return; + } + + ControlValue *val = find(*ctrl->second); + if (!val) + return; + + *val = value; +} + const ControlValue *ControlList::find(const ControlId &id) const { const auto iter = controls_.find(&id); diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index e800f1449888..c14ed1a4d3ce 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include "camera_controls.h" @@ -64,12 +65,12 @@ Request::Request(Camera *camera, uint64_t cookie) * creating a new instance for each request? */ validator_ = new CameraControlValidator(camera); - controls_ = new ControlList(validator_); + controls_ = new ControlList(controls::controls, validator_); /** * \todo: Add a validator for metadata controls. */ - metadata_ = new ControlList(); + metadata_ = new ControlList(controls::controls); } Request::~Request() diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp index 1bcfecc467b5..5af53f64bb6c 100644 --- a/test/controls/control_list.cpp +++ b/test/controls/control_list.cpp @@ -42,7 +42,7 @@ protected: int run() { CameraControlValidator validator(camera_.get()); - ControlList list(&validator); + ControlList list(controls::controls, &validator); /* Test that the list is initially empty. */ if (!list.empty()) { From patchwork Sat Oct 12 18:44:01 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2169 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5E7E361986 for ; Sat, 12 Oct 2019 20:44:21 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id F228C33A for ; Sat, 12 Oct 2019 20:44:20 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1570905861; bh=sG7Ifl84SxV6o5oFM/Kb4BddvDTqdPqV0WkoKLzi/8E=; h=From:To:Subject:Date:In-Reply-To:References:From; b=YvZqbOQo6x9OKjCQhUVxvV/E33yZZsEW8nn0CrvWfPpRKS1wjnYNFA4k1J/do49q1 OalPgxql7qsU0jY+muIigfHpEaws/cQsDf8HgsDkujdJ1TFO2ZEGWsJ3W7aUl/hUDF V5zRf9yID98YiLfaH3ESpfMOsWDr4vvhRm1nrDR8= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Oct 2019 21:44:01 +0300 Message-Id: <20191012184407.31684-9-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> References: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 08/14] test: v4l2_videodevice: Add V4L2 control test 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: Sat, 12 Oct 2019 18:44:21 -0000 Add a test that exercises the control enumeration, get and set APIs on a V4L2Device. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- test/v4l2_videodevice/controls.cpp | 98 ++++++++++++++++++++++++++++++ test/v4l2_videodevice/meson.build | 1 + 2 files changed, 99 insertions(+) create mode 100644 test/v4l2_videodevice/controls.cpp diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp new file mode 100644 index 000000000000..4a7535245c00 --- /dev/null +++ b/test/v4l2_videodevice/controls.cpp @@ -0,0 +1,98 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * controls.cpp - V4L2 device controls handling test + */ + +#include +#include + +#include "v4l2_videodevice.h" + +#include "v4l2_videodevice_test.h" + +using namespace std; +using namespace libcamera; + +class V4L2ControlTest : public V4L2VideoDeviceTest +{ +public: + V4L2ControlTest() + : V4L2VideoDeviceTest("vivid", "vivid-000-vid-cap") {} + +protected: + int run() + { + const V4L2ControlInfoMap &info = capture_->controls(); + + /* Test control enumeration. */ + if (info.empty()) { + cerr << "Failed to enumerate controls" << endl; + return TestFail; + } + + if (info.find(V4L2_CID_BRIGHTNESS) == info.end() || + info.find(V4L2_CID_CONTRAST) == info.end() || + info.find(V4L2_CID_SATURATION) == info.end()) { + cerr << "Missing controls" << endl; + return TestFail; + } + + const V4L2ControlInfo &brightness = info.find(V4L2_CID_BRIGHTNESS)->second; + const V4L2ControlInfo &contrast = info.find(V4L2_CID_CONTRAST)->second; + const V4L2ControlInfo &saturation = info.find(V4L2_CID_SATURATION)->second; + + /* Test getting controls. */ + V4L2ControlList ctrls; + ctrls.add(V4L2_CID_BRIGHTNESS); + ctrls.add(V4L2_CID_CONTRAST); + ctrls.add(V4L2_CID_SATURATION); + + int ret = capture_->getControls(&ctrls); + if (ret != 0) { + cerr << "Failed to get controls" << endl; + return TestFail; + } + + if (ctrls[V4L2_CID_BRIGHTNESS]->value().get() == -1 || + ctrls[V4L2_CID_CONTRAST]->value().get() == -1 || + ctrls[V4L2_CID_SATURATION]->value().get() == -1) { + cerr << "Incorrect value for retrieved controls" << endl; + return TestFail; + } + + /* Test setting controls. */ + ctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min(); + ctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max(); + ctrls[V4L2_CID_SATURATION]->value() = saturation.range().min(); + + ret = capture_->setControls(&ctrls); + if (ret != 0) { + cerr << "Failed to set controls" << endl; + return TestFail; + } + + /* Test setting controls outside of range. */ + ctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min().get() - 1; + ctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max().get() + 1; + ctrls[V4L2_CID_SATURATION]->value() = saturation.range().min().get() + 1; + + ret = capture_->setControls(&ctrls); + if (ret != 0) { + cerr << "Failed to set controls (out of range)" << endl; + return TestFail; + } + + if (ctrls[V4L2_CID_BRIGHTNESS]->value() != brightness.range().min() || + ctrls[V4L2_CID_CONTRAST]->value() != brightness.range().max() || + ctrls[V4L2_CID_SATURATION]->value() != saturation.range().min().get() + 1) { + cerr << "Controls not updated when set" << endl; + return TestFail; + } + + return TestPass; + } +}; + +TEST_REGISTER(V4L2ControlTest); diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build index ad41898b5f8b..5c52da7219c2 100644 --- a/test/v4l2_videodevice/meson.build +++ b/test/v4l2_videodevice/meson.build @@ -2,6 +2,7 @@ # They are not alphabetically sorted. v4l2_videodevice_tests = [ [ 'double_open', 'double_open.cpp' ], + [ 'controls', 'controls.cpp' ], [ 'formats', 'formats.cpp' ], [ 'request_buffers', 'request_buffers.cpp' ], [ 'stream_on_off', 'stream_on_off.cpp' ], From patchwork Sat Oct 12 18:44:02 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2170 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 ABCBA61986 for ; Sat, 12 Oct 2019 20:44:21 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 4BDF254C for ; Sat, 12 Oct 2019 20:44:21 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1570905861; bh=PHukfiupzGP473rBSTqF5QnoGbfpdJ2UyRUkSw3CLyE=; h=From:To:Subject:Date:In-Reply-To:References:From; b=XngmN1uW8S1A4dATT71Rwpxhu7SfKpd3J0VOMD+y3h0siQNc1BzdLNl2JRCeULix/ 3BTkAS9KDMEHTBQ3vzd/dEaACOPCQqRZIawDmJ73fcrRYF9At24NtfzDgYHOcD6m7u 2J+Odn9P4uikEtBn3yEJIDl3tWiSSFKCJozNzJPc= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Oct 2019 21:44:02 +0300 Message-Id: <20191012184407.31684-10-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> References: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 09/14] libcamera: v4l2_device: Avoid copy of V4L2ControlInfo 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: Sat, 12 Oct 2019 18:44:21 -0000 The V4L2Device::listControls() method creates an instance of V4L2ControlInfo and then inserts it in the device's list of controls. The insertion uses std::map::emplace(), but passes the V4L2ControlInfo, resulting in a copy being performed (using the copy constructor of V4L2ControlInfo). Optimise this by really constructing the V4L2ControlInfo in-place. The use of std::piecewise_construct is required for gcc-5 that seems to have trouble with std::map::emplace() on non-copyable values in this case. Signed-off-by: Laurent Pinchart Acked-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- src/libcamera/v4l2_device.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index fd4b9c6d5c62..3bd82ff23212 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -352,8 +352,7 @@ void V4L2Device::listControls() ctrl.flags & V4L2_CTRL_FLAG_DISABLED) continue; - V4L2ControlInfo info(ctrl); - switch (info.type()) { + switch (ctrl.type) { case V4L2_CTRL_TYPE_INTEGER: case V4L2_CTRL_TYPE_BOOLEAN: case V4L2_CTRL_TYPE_MENU: @@ -364,12 +363,14 @@ void V4L2Device::listControls() break; /* \todo Support compound controls. */ default: - LOG(V4L2, Debug) << "Control type '" << info.type() + LOG(V4L2, Debug) << "Control type '" << ctrl.type << "' not supported"; continue; } - controls_.emplace(ctrl.id, info); + controls_.emplace(std::piecewise_construct, + std::forward_as_tuple(ctrl.id), + std::forward_as_tuple(ctrl)); } } From patchwork Sat Oct 12 18:44:03 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2171 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 0850D61985 for ; Sat, 12 Oct 2019 20:44:22 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 99CC69C0 for ; Sat, 12 Oct 2019 20:44:21 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1570905861; bh=KdlihWjUEp+qWzvK6f4WsvK6IQKlXK8MZTC9Vo9754g=; h=From:To:Subject:Date:In-Reply-To:References:From; b=iSVrpoB1CAnyjah2NwK7OJHGmh1WSSX8oxtvJokSWKk++DLW2+7WnmkuOlPt5/ixm Bym6c/rOSLRYMqJiFMsJ4lGcnHa8uZorHPVeMm2QLW4Nih8E1t8tarUU57EavEjhtx w0fKFS/e/qniRHRNqUWhQKGOQQYA8ORr5hZyh+qU= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Oct 2019 21:44:03 +0300 Message-Id: <20191012184407.31684-11-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> References: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 10/14] libcamera: v4l2_controls: Add V4L2ControlId 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: Sat, 12 Oct 2019 18:44:23 -0000 Add a V4L2 specialisation of the ControlId class, in order to construct a ControlId from a v4l2_query_ext_ctrl. The V4L2ControlId is embedded in V4L2ControlInfo, and thus needs to be copyable to allow for V4L2ControlInfo to be passed to IPAs. The ControlId copy constructor and assignment operators are thus restored, but made protected to avoid the Control class being copyable. This is needed in order to use ControlList for V4L2 controls, as ControlList requires ControlId instances for all controls. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/libcamera/controls.h | 7 +-- src/libcamera/include/v4l2_controls.h | 12 +++-- src/libcamera/v4l2_controls.cpp | 67 +++++++++++++++++++++++---- src/libcamera/v4l2_device.cpp | 4 +- 4 files changed, 72 insertions(+), 18 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 5e6708fe570b..ebc4204f98fd 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -68,11 +68,12 @@ protected: : id_(id), name_(name), type_(type) { } +#ifndef __DOXYGEN__ + ControlId &operator=(const ControlId &) = default; + ControlId(const ControlId &) = default; +#endif private: - ControlId(const ControlId &) = delete; - ControlId &operator=(const ControlId &) = delete; - unsigned int id_; std::string name_; ControlType type_; diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h index b39370b2e90e..71ce377fe4c5 100644 --- a/src/libcamera/include/v4l2_controls.h +++ b/src/libcamera/include/v4l2_controls.h @@ -20,23 +20,27 @@ namespace libcamera { +class V4L2ControlId : public ControlId +{ +public: + V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl); +}; + class V4L2ControlInfo { public: V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl); - unsigned int id() const { return id_; } + const ControlId &id() const { return id_; } unsigned int type() const { return type_; } size_t size() const { return size_; } - const std::string &name() const { return name_; } const ControlRange &range() const { return range_; } private: - unsigned int id_; + V4L2ControlId id_; unsigned int type_; size_t size_; - std::string name_; ControlRange range_; }; diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp index 6f5f1578b139..a630a2583d33 100644 --- a/src/libcamera/v4l2_controls.cpp +++ b/src/libcamera/v4l2_controls.cpp @@ -7,6 +7,8 @@ #include "v4l2_controls.h" +#include + /** * \file v4l2_controls.h * \brief Support for V4L2 Controls using the V4L2 Extended Controls APIs @@ -47,6 +49,60 @@ namespace libcamera { +namespace { + +std::string v4l2_ctrl_name(const struct v4l2_query_ext_ctrl &ctrl) +{ + size_t len = strnlen(ctrl.name, sizeof(ctrl.name)); + return std::string(static_cast(ctrl.name), len); +} + +ControlType v4l2_ctrl_type(const struct v4l2_query_ext_ctrl &ctrl) +{ + switch (ctrl.type) { + case V4L2_CTRL_TYPE_BOOLEAN: + return ControlTypeBool; + + case V4L2_CTRL_TYPE_INTEGER: + return ControlTypeInteger32; + + case V4L2_CTRL_TYPE_INTEGER64: + return ControlTypeInteger64; + + case V4L2_CTRL_TYPE_MENU: + case V4L2_CTRL_TYPE_BUTTON: + case V4L2_CTRL_TYPE_BITMASK: + case V4L2_CTRL_TYPE_INTEGER_MENU: + /* + * More precise types may be needed, for now use a 32-bit + * integer type. + */ + return ControlTypeInteger32; + + default: + return ControlTypeNone; + } +} + +} /* namespace */ + +/** + * \class V4L2ControlId + * \brief V4L2 control static metadata + * + * The V4L2ControlId class is a specialisation of the ControlId for V4L2 + * controls. + */ + +/** + * \brief Construct a V4L2ControlId from a struct v4l2_query_ext_ctrl + * \param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the kernel + */ +V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl) + : ControlId(ctrl.id, v4l2_ctrl_name(ctrl), v4l2_ctrl_type(ctrl)) +{ +} + /** * \class V4L2ControlInfo * \brief Information on a V4L2 control @@ -66,13 +122,12 @@ namespace libcamera { /** * \brief Construct a V4L2ControlInfo from a struct v4l2_query_ext_ctrl - * \param ctrl The struct v4l2_query_ext_ctrl as returned by the kernel + * \param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the kernel */ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl) + : id_(ctrl) { - id_ = ctrl.id; type_ = ctrl.type; - name_ = static_cast(ctrl.name); size_ = ctrl.elem_size * ctrl.elems; if (ctrl.type == V4L2_CTRL_TYPE_INTEGER64) @@ -101,12 +156,6 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl) * \return The V4L2 control value data size */ -/** - * \fn V4L2ControlInfo::name() - * \brief Retrieve the control user readable name - * \return The V4L2 control user readable name - */ - /** * \fn V4L2ControlInfo::range() * \brief Retrieve the control value range diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 3bd82ff23212..466c3d41f6e3 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -184,7 +184,7 @@ int V4L2Device::getControls(V4L2ControlList *ctrls) const V4L2ControlInfo *info = &iter->second; controlInfo[i] = info; - v4l2Ctrls[i].id = info->id(); + v4l2Ctrls[i].id = ctrl->id(); } struct v4l2_ext_controls v4l2ExtCtrls = {}; @@ -259,7 +259,7 @@ int V4L2Device::setControls(V4L2ControlList *ctrls) const V4L2ControlInfo *info = &iter->second; controlInfo[i] = info; - v4l2Ctrls[i].id = info->id(); + v4l2Ctrls[i].id = ctrl->id(); /* Set the v4l2_ext_control value for the write operation. */ switch (info->type()) { From patchwork Sat Oct 12 18:44:04 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2172 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 494B761986 for ; Sat, 12 Oct 2019 20:44:22 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id E8D4654C for ; Sat, 12 Oct 2019 20:44:21 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1570905862; bh=hSbqtCCCi0RBDE7TPZXAWt6zgI4a/8yeeuEK9NkvFYQ=; h=From:To:Subject:Date:In-Reply-To:References:From; b=o6FVWSNCJ4JqH6oM6rL+tie+WHDZrfgUHOxpVM8rTvySmIe6/cuIq7/5Bg5kBsIJd prZTwhfBaMlXgLo/xrbld8fye4A/M+MtxSZevLRpFQVHnTXM6i0JQ0e2Xg1XJXipL9 PpHeCLKaJZQyjAxao6FDqUGBMD50RONGWdphQk5s= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Oct 2019 21:44:04 +0300 Message-Id: <20191012184407.31684-12-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> References: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 11/14] libcamera: v4l2_controls: Remove V4L2ControlInfo type field 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: Sat, 12 Oct 2019 18:44:23 -0000 The V4L2ControlInfo type field stores the V4L2 control type. It partly duplicates the V4L2ControlInfo id().type() that stores the corresponding libcamera control type. The two fields are not strictly identical, but having two types doesn't provide us with any extra value. As this is confusing, remove the V4L2ControlInfo type field. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- src/libcamera/include/v4l2_controls.h | 2 -- src/libcamera/v4l2_controls.cpp | 7 ------- src/libcamera/v4l2_device.cpp | 8 ++++---- 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h index 71ce377fe4c5..949ca21cb863 100644 --- a/src/libcamera/include/v4l2_controls.h +++ b/src/libcamera/include/v4l2_controls.h @@ -32,14 +32,12 @@ public: V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl); const ControlId &id() const { return id_; } - unsigned int type() const { return type_; } size_t size() const { return size_; } const ControlRange &range() const { return range_; } private: V4L2ControlId id_; - unsigned int type_; size_t size_; ControlRange range_; diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp index a630a2583d33..3f5f3ff10880 100644 --- a/src/libcamera/v4l2_controls.cpp +++ b/src/libcamera/v4l2_controls.cpp @@ -127,7 +127,6 @@ V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl) V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl) : id_(ctrl) { - type_ = ctrl.type; size_ = ctrl.elem_size * ctrl.elems; if (ctrl.type == V4L2_CTRL_TYPE_INTEGER64) @@ -144,12 +143,6 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl) * \return The V4L2 control ID */ -/** - * \fn V4L2ControlInfo::type() - * \brief Retrieve the control type as defined by V4L2_CTRL_TYPE_* - * \return The V4L2 control type - */ - /** * \fn V4L2ControlInfo::size() * \brief Retrieve the control value data size (in bytes) diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 466c3d41f6e3..8c5998435020 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -262,8 +262,8 @@ int V4L2Device::setControls(V4L2ControlList *ctrls) v4l2Ctrls[i].id = ctrl->id(); /* Set the v4l2_ext_control value for the write operation. */ - switch (info->type()) { - case V4L2_CTRL_TYPE_INTEGER64: + switch (info->id().type()) { + case ControlTypeInteger64: v4l2Ctrls[i].value64 = ctrl->value().get(); break; default: @@ -392,8 +392,8 @@ void V4L2Device::updateControls(V4L2ControlList *ctrls, const V4L2ControlInfo *info = controlInfo[i]; V4L2Control *ctrl = ctrls->getByIndex(i); - switch (info->type()) { - case V4L2_CTRL_TYPE_INTEGER64: + switch (info->id().type()) { + case ControlTypeInteger64: ctrl->value().set(v4l2Ctrl->value64); break; default: From patchwork Sat Oct 12 18:44:05 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2173 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 AAD5D61998 for ; Sat, 12 Oct 2019 20:44:22 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 42CA733A for ; Sat, 12 Oct 2019 20:44:22 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1570905862; bh=G6sXZFVt53xbTzzhk7rk5JduO2MXjDbfRqpb6Tkj+RU=; h=From:To:Subject:Date:In-Reply-To:References:From; b=t8RX2KaFQwanihACJLHsHhCX45d+0UrGYPR6uxAFGywdVqOAwPrHSLeznJWg/Ekas vC2HD9wyBg39Ya1FxmLMIhSXLspiYCz59Mbv1T0r753fZaDNFou2S7nUsAzro/u37L PvUB+CogZ20tb/I10eYYazwlmc5oBrCDTKwzTO2c= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Oct 2019 21:44:05 +0300 Message-Id: <20191012184407.31684-13-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> References: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 12/14] libcamera: v4l2_controls: Turn V4L2ControlInfoMap into a class 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: Sat, 12 Oct 2019 18:44:23 -0000 In preparation for extending V4L2ControlInfoMap with control idmap support, turn it into a real class. Make it inherit from std::map<> instead of wrapping it to keep the API simple. V4L2ControlInfoMap is meant to be constructed with a set of control info, and never modified afterwards. To ensure this, inherit from std::map<> with private access specifier, and explicitly expose the std::map<> methods that do not allow insertion or removal of elements. A custom move assignment operator is implemented to allow efficient construction of the V4L2ControlInfoMap. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- src/libcamera/include/v4l2_controls.h | 16 +++++++++++++++- src/libcamera/v4l2_controls.cpp | 17 ++++++++++++++++- src/libcamera/v4l2_device.cpp | 9 ++++++--- 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h index 949ca21cb863..8990e755a385 100644 --- a/src/libcamera/include/v4l2_controls.h +++ b/src/libcamera/include/v4l2_controls.h @@ -43,7 +43,21 @@ private: ControlRange range_; }; -using V4L2ControlInfoMap = std::map; +class V4L2ControlInfoMap : private std::map +{ +public: + V4L2ControlInfoMap &operator=(std::map &&info); + + using std::map::begin; + using std::map::cbegin; + using std::map::end; + using std::map::cend; + using std::map::at; + using std::map::empty; + using std::map::size; + using std::map::count; + using std::map::find; +}; class V4L2Control { diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp index 3f5f3ff10880..438e8d8bdb99 100644 --- a/src/libcamera/v4l2_controls.cpp +++ b/src/libcamera/v4l2_controls.cpp @@ -156,10 +156,25 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl) */ /** - * \typedef V4L2ControlInfoMap + * \class V4L2ControlInfoMap * \brief A map of control ID to V4L2ControlInfo */ +/** + * \brief Move assignment operator from plain map + * + * Populate the map by replacing its contents with those of \a info using move + * semantics. This is the only supported + * + * \return *this + */ +V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(std::map &&info) +{ + std::map::operator=(std::move(info)); + + return *this; +} + /** * \class V4L2Control * \brief A V4L2 control value diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 8c5998435020..1f755f0f3ef6 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -340,6 +340,7 @@ int V4L2Device::ioctl(unsigned long request, void *argp) */ void V4L2Device::listControls() { + std::map ctrls; struct v4l2_query_ext_ctrl ctrl = {}; /* \todo Add support for menu and compound controls. */ @@ -368,10 +369,12 @@ void V4L2Device::listControls() continue; } - controls_.emplace(std::piecewise_construct, - std::forward_as_tuple(ctrl.id), - std::forward_as_tuple(ctrl)); + ctrls.emplace(std::piecewise_construct, + std::forward_as_tuple(ctrl.id), + std::forward_as_tuple(ctrl)); } + + controls_ = std::move(ctrls); } /* From patchwork Sat Oct 12 18:44:06 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2174 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 1D21761992 for ; Sat, 12 Oct 2019 20:44:23 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 98E7F9C0 for ; Sat, 12 Oct 2019 20:44:22 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1570905862; bh=oEl0RtMWRwCO1T25+14EclV/mIo5AIWgKBa4j/ltJPo=; h=From:To:Subject:Date:In-Reply-To:References:From; b=ViFT4Rpgp5VEgiBPQzoDHK7+QUzudP3sd1jjVw8WUjRV+/36opqmITV3izHonuTAv FJ2g0naPG33HcJpzzD8fKlrkYth6IpFK+Mvka4fLPUkTyN/Kkizo4GuUpf3387Xa68 nVpkuNzIcYIR4qFn/oBhKrjA6epPsGTiUX2y7BUs= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Oct 2019 21:44:06 +0300 Message-Id: <20191012184407.31684-14-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> References: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 13/14] libcamera: v4l2_device: Replace V4L2ControlList with 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: Sat, 12 Oct 2019 18:44:24 -0000 The V4L2Device class uses V4L2ControlList as a controls container for the getControls() and setControls() operations. Having a distinct container from ControlList will makes the IPA API more complex, as it needs to explicitly transport both types of lists. This will become even more painful when implementing serialisation and deserialisation. To simplify the IPA API and ease the implementation of serialisation and deserialisation, replace usage of V4L2ControlList with ControlList in the V4L2Device (and thus CameraSensor) API. The V4L2ControlList class becomes a thin wrapper around ControlList that slightly simplifies the creation of control lists for V4L2 controls, and may be removed in the future. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- Changes since v1: - Fix typo in comment - Use the v4l2_ext_control::value field instead of value64 where appropriate - Break loop in updateControls() when reaching count - Rename controls_ to info_ (and related variables) - Add reference to V4L2Device::[gs]etControls() to V4L2ControlList class documentation - Update the rkisp1 IPA - Add V4L2ControlInfoMap class - Remove ID-based accessors from V4L2ControlList as they are now present in the base ControlList class - Don't include v4l2_controls.h in camera_sensor.h --- src/ipa/rkisp1/rkisp1.cpp | 18 +-- src/libcamera/camera_sensor.cpp | 4 +- src/libcamera/include/camera_sensor.h | 7 +- src/libcamera/include/v4l2_controls.h | 44 ++---- src/libcamera/include/v4l2_device.h | 6 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +- src/libcamera/pipeline/uvcvideo.cpp | 24 ++-- src/libcamera/pipeline/vimc.cpp | 25 ++-- src/libcamera/v4l2_controls.cpp | 196 +++----------------------- src/libcamera/v4l2_device.cpp | 51 ++++--- test/v4l2_videodevice/controls.cpp | 32 ++--- 11 files changed, 124 insertions(+), 292 deletions(-) diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index b0d23dd154be..69ced840585f 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -49,6 +49,8 @@ private: std::map bufferInfo_; + V4L2ControlInfoMap ctrls_; + /* Camera sensor controls. */ bool autoExposure_; uint32_t exposure_; @@ -65,16 +67,16 @@ void IPARkISP1::configure(const std::map &streamConfig, if (entityControls.empty()) return; - const V4L2ControlInfoMap &ctrls = entityControls.at(0); + ctrls_ = entityControls.at(0); - const auto itExp = ctrls.find(V4L2_CID_EXPOSURE); - if (itExp == ctrls.end()) { + const auto itExp = ctrls_.find(V4L2_CID_EXPOSURE); + if (itExp == ctrls_.end()) { LOG(IPARkISP1, Error) << "Can't find exposure control"; return; } - const auto itGain = ctrls.find(V4L2_CID_ANALOGUE_GAIN); - if (itGain == ctrls.end()) { + const auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN); + if (itGain == ctrls_.end()) { LOG(IPARkISP1, Error) << "Can't find gain control"; return; } @@ -210,9 +212,9 @@ void IPARkISP1::setControls(unsigned int frame) IPAOperationData op; op.operation = RKISP1_IPA_ACTION_V4L2_SET; - V4L2ControlList ctrls; - ctrls.add(V4L2_CID_EXPOSURE, exposure_); - ctrls.add(V4L2_CID_ANALOGUE_GAIN, gain_); + V4L2ControlList ctrls(ctrls_); + ctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure_)); + ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast(gain_)); op.v4l2controls.push_back(ctrls); queueFrameAction.emit(frame, op); diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index a7670b449b31..9e8b44a23850 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -279,7 +279,7 @@ const V4L2ControlInfoMap &CameraSensor::controls() const * \retval -EINVAL One of the control is not supported or not accessible * \retval i The index of the control that failed */ -int CameraSensor::getControls(V4L2ControlList *ctrls) +int CameraSensor::getControls(ControlList *ctrls) { return subdev_->getControls(ctrls); } @@ -309,7 +309,7 @@ int CameraSensor::getControls(V4L2ControlList *ctrls) * \retval -EINVAL One of the control is not supported or not accessible * \retval i The index of the control that failed */ -int CameraSensor::setControls(V4L2ControlList *ctrls) +int CameraSensor::setControls(ControlList *ctrls) { return subdev_->setControls(ctrls); } diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h index fe033fb374c1..f426e29b84bf 100644 --- a/src/libcamera/include/camera_sensor.h +++ b/src/libcamera/include/camera_sensor.h @@ -13,11 +13,12 @@ #include #include "log.h" -#include "v4l2_controls.h" namespace libcamera { +class ControlList; class MediaEntity; +class V4L2ControlInfoMap; class V4L2Subdevice; struct V4L2SubdeviceFormat; @@ -43,8 +44,8 @@ public: int setFormat(V4L2SubdeviceFormat *format); const V4L2ControlInfoMap &controls() const; - int getControls(V4L2ControlList *ctrls); - int setControls(V4L2ControlList *ctrls); + int getControls(ControlList *ctrls); + int setControls(ControlList *ctrls); protected: std::string logPrefix() const; diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h index 8990e755a385..89cc74485e6f 100644 --- a/src/libcamera/include/v4l2_controls.h +++ b/src/libcamera/include/v4l2_controls.h @@ -13,7 +13,6 @@ #include #include -#include #include #include @@ -57,47 +56,20 @@ public: using std::map::size; using std::map::count; using std::map::find; + + const ControlIdMap &idmap() const { return idmap_; } + +private: + ControlIdMap idmap_; }; -class V4L2Control +class V4L2ControlList : public ControlList { public: - V4L2Control(unsigned int id, const ControlValue &value = ControlValue()) - : id_(id), value_(value) + V4L2ControlList(const V4L2ControlInfoMap &info) + : ControlList(info.idmap()) { } - - unsigned int id() const { return id_; } - const ControlValue &value() const { return value_; } - ControlValue &value() { return value_; } - -private: - unsigned int id_; - ControlValue value_; -}; - -class V4L2ControlList -{ -public: - using iterator = std::vector::iterator; - using const_iterator = std::vector::const_iterator; - - iterator begin() { return controls_.begin(); } - const_iterator begin() const { return controls_.begin(); } - iterator end() { return controls_.end(); } - const_iterator end() const { return controls_.end(); } - - bool empty() const { return controls_.empty(); } - std::size_t size() const { return controls_.size(); } - - void clear() { controls_.clear(); } - void add(unsigned int id, int64_t value = 0); - - V4L2Control *getByIndex(unsigned int index); - V4L2Control *operator[](unsigned int id); - -private: - std::vector controls_; }; } /* namespace libcamera */ diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h index 75a52c33d821..daa762d58d2b 100644 --- a/src/libcamera/include/v4l2_device.h +++ b/src/libcamera/include/v4l2_device.h @@ -25,8 +25,8 @@ public: const V4L2ControlInfoMap &controls() const { return controls_; } - int getControls(V4L2ControlList *ctrls); - int setControls(V4L2ControlList *ctrls); + int getControls(ControlList *ctrls); + int setControls(ControlList *ctrls); const std::string &deviceNode() const { return deviceNode_; } @@ -43,7 +43,7 @@ protected: private: void listControls(); - void updateControls(V4L2ControlList *ctrls, + void updateControls(ControlList *ctrls, const V4L2ControlInfo **controlInfo, const struct v4l2_ext_control *v4l2Ctrls, unsigned int count); diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 827906d5cd2e..9776b36b88cd 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -199,6 +199,7 @@ class PipelineHandlerIPU3 : public PipelineHandler { public: static constexpr unsigned int V4L2_CID_IPU3_PIPE_MODE = 0x009819c1; + enum IPU3PipeModes { IPU3PipeModeVideo = 0, IPU3PipeModeStillCapture = 1, @@ -603,10 +604,10 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) return ret; /* Apply the "pipe_mode" control to the ImgU subdevice. */ - V4L2ControlList ctrls; - ctrls.add(V4L2_CID_IPU3_PIPE_MODE, vfStream->active_ ? - IPU3PipeModeVideo : - IPU3PipeModeStillCapture); + V4L2ControlList ctrls(imgu->imgu_->controls()); + ctrls.set(V4L2_CID_IPU3_PIPE_MODE, + static_cast(vfStream->active_ ? IPU3PipeModeVideo : + IPU3PipeModeStillCapture)); ret = imgu->imgu_->setControls(&ctrls); if (ret) { LOG(IPU3, Error) << "Unable to set pipe_mode control"; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 88f7fb9bc568..547ad5ca4e55 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -228,31 +228,30 @@ void PipelineHandlerUVC::stop(Camera *camera) int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) { - V4L2ControlList controls; + V4L2ControlList controls(data->video_->controls()); for (auto it : request->controls()) { const ControlId &id = *it.first; ControlValue &value = it.second; if (id == controls::Brightness) { - controls.add(V4L2_CID_BRIGHTNESS, value.get()); + controls.set(V4L2_CID_BRIGHTNESS, value); } else if (id == controls::Contrast) { - controls.add(V4L2_CID_CONTRAST, value.get()); + controls.set(V4L2_CID_CONTRAST, value); } else if (id == controls::Saturation) { - controls.add(V4L2_CID_SATURATION, value.get()); + controls.set(V4L2_CID_SATURATION, value); } else if (id == controls::ManualExposure) { - controls.add(V4L2_CID_EXPOSURE_AUTO, 1); - controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get()); + controls.set(V4L2_CID_EXPOSURE_AUTO, static_cast(1)); + controls.set(V4L2_CID_EXPOSURE_ABSOLUTE, value); } else if (id == controls::ManualGain) { - controls.add(V4L2_CID_GAIN, value.get()); + controls.set(V4L2_CID_GAIN, value); } } - for (const V4L2Control &ctrl : controls) + for (const auto &ctrl : controls) LOG(UVC, Debug) - << "Setting control 0x" - << std::hex << std::setw(8) << ctrl.id() << std::dec - << " to " << ctrl.value().toString(); + << "Setting control " << ctrl.first->name() + << " to " << ctrl.second.toString(); int ret = data->video_->setControls(&controls); if (ret) { @@ -338,11 +337,10 @@ int UVCCameraData::init(MediaEntity *entity) /* Initialise the supported controls. */ const V4L2ControlInfoMap &controls = video_->controls(); for (const auto &ctrl : controls) { - unsigned int v4l2Id = ctrl.first; const V4L2ControlInfo &info = ctrl.second; const ControlId *id; - switch (v4l2Id) { + switch (info.id().id()) { case V4L2_CID_BRIGHTNESS: id = &controls::Brightness; break; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index f1cfd0ed35cf..53d00360eb6e 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -279,26 +279,24 @@ void PipelineHandlerVimc::stop(Camera *camera) int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) { - V4L2ControlList controls; + V4L2ControlList controls(data->sensor_->controls()); for (auto it : request->controls()) { const ControlId &id = *it.first; ControlValue &value = it.second; - if (id == controls::Brightness) { - controls.add(V4L2_CID_BRIGHTNESS, value.get()); - } else if (id == controls::Contrast) { - controls.add(V4L2_CID_CONTRAST, value.get()); - } else if (id == controls::Saturation) { - controls.add(V4L2_CID_SATURATION, value.get()); - } + if (id == controls::Brightness) + controls.set(V4L2_CID_BRIGHTNESS, value); + else if (id == controls::Contrast) + controls.set(V4L2_CID_CONTRAST, value); + else if (id == controls::Saturation) + controls.set(V4L2_CID_SATURATION, value); } - for (const V4L2Control &ctrl : controls) + for (const auto &ctrl : controls) LOG(VIMC, Debug) - << "Setting control 0x" - << std::hex << std::setw(8) << ctrl.id() << std::dec - << " to " << ctrl.value().toString(); + << "Setting control " << ctrl.first->name() + << " to " << ctrl.second.toString(); int ret = data->sensor_->setControls(&controls); if (ret) { @@ -415,11 +413,10 @@ int VimcCameraData::init(MediaDevice *media) /* Initialise the supported controls. */ const V4L2ControlInfoMap &controls = sensor_->controls(); for (const auto &ctrl : controls) { - unsigned int v4l2Id = ctrl.first; const V4L2ControlInfo &info = ctrl.second; const ControlId *id; - switch (v4l2Id) { + switch (info.id().id()) { case V4L2_CID_BRIGHTNESS: id = &controls::Brightness; break; diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp index 438e8d8bdb99..b4bb7e791491 100644 --- a/src/libcamera/v4l2_controls.cpp +++ b/src/libcamera/v4l2_controls.cpp @@ -40,7 +40,7 @@ * sizes. * * The libcamera V4L2 Controls framework operates on lists of controls, wrapped - * by the V4L2ControlList class, to match the V4L2 extended controls API. The + * by the ControlList class, to match the V4L2 extended controls API. The * interface to set and get control is implemented by the V4L2Device class, and * this file only provides the data type definitions. * @@ -172,194 +172,42 @@ V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(std::map::operator=(std::move(info)); + idmap_.clear(); + for (const auto &ctrl : *this) + idmap_[ctrl.first] = &ctrl.second.id(); + return *this; } /** - * \class V4L2Control - * \brief A V4L2 control value + * \fn const ControlIdMap &V4L2ControlInfoMap::idmap() const + * \brief Retrieve the ControlId map * - * The V4L2Control class represent the value of a V4L2 control. The class - * stores values that have been read from or will be applied to a V4L2 device. + * Constructing ControlList instances for V4L2 controls requires a ControlIdMap + * for the V4L2 device that the control list targets. This helper method + * returns a suitable idmap for that purpose. * - * The value stored in the class instances does not reflect what is actually - * applied to the hardware but is a pure software cache optionally initialized - * at control creation time and modified by a control read or write operation. - * - * The write and read controls the V4L2Control class instances are not meant - * to be directly used but are instead intended to be grouped in - * V4L2ControlList instances, which are then passed as parameters to - * V4L2Device::setControls() and V4L2Device::getControls() operations. - */ - -/** - * \fn V4L2Control::V4L2Control - * \brief Construct a V4L2 control with \a id and \a value - * \param id The V4L2 control ID - * \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 - * - * This method returns the cached control value, initially set by - * V4L2ControlList::add() and then updated when the controls are read or - * written with V4L2Device::getControls() and V4L2Device::setControls(). - * - * \return The V4L2 control value - */ - -/** - * \fn V4L2Control::id() - * \brief Retrieve the control ID this instance refers to - * \return The V4L2Control ID + * \return The ControlId map */ /** * \class V4L2ControlList - * \brief Container of V4L2Control instances + * \brief A list of controls for a V4L2 device * - * The V4L2ControlList class works as a container for a list of V4L2Control - * instances. The class provides operations to add a new control to the list, - * get back a control value, and reset the list of controls it contains. + * This class specialises the ControList class for use with V4L2 devices. It + * offers a convenience API to create a ControlList from a V4L2ControlInfoMap. * - * In order to set and get controls, user of the libcamera V4L2 control - * framework should operate on instances of the V4L2ControlList class, and use - * them as argument for the V4L2Device::setControls() and - * V4L2Device::getControls() operations, which write and read a list of - * controls to or from a V4L2 device (a video device or a subdevice). - * - * Controls are added to a V4L2ControlList instance with the add() method, with - * or without a value. - * - * To write controls to a device, the controls of interest shall be added with - * an initial value by calling V4L2ControlList::add(unsigned int id, int64_t - * value) to prepare for a write operation. Once the values of all controls of - * interest have been added, the V4L2ControlList instance is passed to the - * V4L2Device::setControls(), which sets the controls on the device. - * - * To read controls from a device, the desired controls are added with - * V4L2ControlList::add(unsigned int id) to prepare for a read operation. The - * V4L2ControlList instance is then passed to V4L2Device::getControls(), which - * reads the controls from the device and updates the values stored in - * V4L2ControlList. - * - * V4L2ControlList instances can be reset to remove all controls they contain - * and prepare to be re-used for a new control write/read sequence. - */ - -/** - * \typedef V4L2ControlList::iterator - * \brief Iterator on the V4L2 controls contained in the instance - */ - -/** - * \typedef V4L2ControlList::const_iterator - * \brief Const iterator on the V4L2 controls contained in the instance - */ - -/** - * \fn iterator V4L2ControlList::begin() - * \brief Retrieve an iterator to the first V4L2Control in the instance - * \return An iterator to the first V4L2 control - */ - -/** - * \fn const_iterator V4L2ControlList::begin() const - * \brief Retrieve a constant iterator to the first V4L2Control in the instance - * \return A constant iterator to the first V4L2 control - */ - -/** - * \fn iterator V4L2ControlList::end() - * \brief Retrieve an iterator pointing to the past-the-end V4L2Control in the - * instance - * \return An iterator to the element following the last V4L2 control in the - * instance - */ - -/** - * \fn const_iterator V4L2ControlList::end() const - * \brief Retrieve a constant iterator pointing to the past-the-end V4L2Control - * in the instance - * \return A constant iterator to the element following the last V4L2 control - * in the instance - */ - -/** - * \fn V4L2ControlList::empty() - * \brief Verify if the instance does not contain any control - * \return True if the instance does not contain any control, false otherwise + * V4L2ControlList allows easy construction of a ControlList containing V4L2 + * controls for a device. It can be used to construct the list of controls + * passed to the V4L2Device::getControls() and V4L2Device::setControls() + * methods. The class should however not be used in place of ControlList in + * APIs. */ /** - * \fn V4L2ControlList::size() - * \brief Retrieve the number on controls in the instance - * \return The number of V4L2Control stored in the instance - */ - -/** - * \fn V4L2ControlList::clear() - * \brief Remove all controls in the instance - */ - -/** - * \brief Add control with \a id and optional \a value to the instance - * \param id The V4L2 control ID (V4L2_CID_*) - * \param value The V4L2 control value - * - * This method adds a new V4L2 control to the V4L2ControlList. The newly - * inserted control shall not already be present in the control lists, otherwise - * this method, and further use of the control list lead to undefined behaviour. + * \fn V4L2ControlList::V4L2ControlList(const V4L2ControlInfoMap &info) + * \brief Construct a V4L2ControlList associated with a V4L2 device + * \param[in] info The V4L2 device control info map */ -void V4L2ControlList::add(unsigned int id, int64_t value) -{ - controls_.emplace_back(id, value); -} - -/** - * \brief Retrieve the control at \a index - * \param[in] index The control index - * - * Controls are stored in a V4L2ControlList in order of insertion and this - * method retrieves the control at \a index. - * - * \return A pointer to the V4L2Control at \a index or nullptr if the - * index is larger than the number of controls - */ -V4L2Control *V4L2ControlList::getByIndex(unsigned int index) -{ - if (index >= controls_.size()) - return nullptr; - - return &controls_[index]; -} - -/** - * \brief Retrieve the control with \a id - * \param[in] id The V4L2 control ID (V4L2_CID_xx) - * \return A pointer to the V4L2Control with \a id or nullptr if the - * control ID is not part of this instance. - */ -V4L2Control *V4L2ControlList::operator[](unsigned int id) -{ - for (V4L2Control &ctrl : controls_) { - if (ctrl.id() == id) - return &ctrl; - } - - return nullptr; -} } /* namespace libcamera */ diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 1f755f0f3ef6..b47ba448f354 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -163,7 +163,7 @@ void V4L2Device::close() * \retval -EINVAL One of the control is not supported or not accessible * \retval i The index of the control that failed */ -int V4L2Device::getControls(V4L2ControlList *ctrls) +int V4L2Device::getControls(ControlList *ctrls) { unsigned int count = ctrls->size(); if (count == 0) @@ -173,18 +173,21 @@ int V4L2Device::getControls(V4L2ControlList *ctrls) struct v4l2_ext_control v4l2Ctrls[count]; memset(v4l2Ctrls, 0, sizeof(v4l2Ctrls)); - for (unsigned int i = 0; i < count; ++i) { - const V4L2Control *ctrl = ctrls->getByIndex(i); - const auto iter = controls_.find(ctrl->id()); + unsigned int i = 0; + for (const auto &ctrl : *ctrls) { + const ControlId *id = ctrl.first; + const auto iter = controls_.find(id->id()); if (iter == controls_.end()) { LOG(V4L2, Error) - << "Control '" << ctrl->id() << "' not found"; + << "Control '" << id->name() << "' not found"; return -EINVAL; } const V4L2ControlInfo *info = &iter->second; controlInfo[i] = info; - v4l2Ctrls[i].id = ctrl->id(); + v4l2Ctrls[i].id = id->id(); + + i++; } struct v4l2_ext_controls v4l2ExtCtrls = {}; @@ -238,7 +241,7 @@ int V4L2Device::getControls(V4L2ControlList *ctrls) * \retval -EINVAL One of the control is not supported or not accessible * \retval i The index of the control that failed */ -int V4L2Device::setControls(V4L2ControlList *ctrls) +int V4L2Device::setControls(ControlList *ctrls) { unsigned int count = ctrls->size(); if (count == 0) @@ -248,32 +251,36 @@ int V4L2Device::setControls(V4L2ControlList *ctrls) struct v4l2_ext_control v4l2Ctrls[count]; memset(v4l2Ctrls, 0, sizeof(v4l2Ctrls)); - for (unsigned int i = 0; i < count; ++i) { - const V4L2Control *ctrl = ctrls->getByIndex(i); - const auto iter = controls_.find(ctrl->id()); + unsigned int i = 0; + for (const auto &ctrl : *ctrls) { + const ControlId *id = ctrl.first; + const auto iter = controls_.find(id->id()); if (iter == controls_.end()) { LOG(V4L2, Error) - << "Control '" << ctrl->id() << "' not found"; + << "Control '" << id->name() << "' not found"; return -EINVAL; } const V4L2ControlInfo *info = &iter->second; controlInfo[i] = info; - v4l2Ctrls[i].id = ctrl->id(); + v4l2Ctrls[i].id = id->id(); /* Set the v4l2_ext_control value for the write operation. */ + const ControlValue &value = ctrl.second; switch (info->id().type()) { case ControlTypeInteger64: - v4l2Ctrls[i].value64 = ctrl->value().get(); + v4l2Ctrls[i].value64 = value.get(); break; default: /* * \todo To be changed when support for string and * compound controls will be added. */ - v4l2Ctrls[i].value = ctrl->value().get(); + v4l2Ctrls[i].value = value.get(); break; } + + i++; } struct v4l2_ext_controls v4l2ExtCtrls = {}; @@ -385,28 +392,34 @@ void V4L2Device::listControls() * \param[in] v4l2Ctrls List of V4L2 extended controls as returned by the driver * \param[in] count The number of controls to update */ -void V4L2Device::updateControls(V4L2ControlList *ctrls, +void V4L2Device::updateControls(ControlList *ctrls, const V4L2ControlInfo **controlInfo, const struct v4l2_ext_control *v4l2Ctrls, unsigned int count) { - for (unsigned int i = 0; i < count; ++i) { + unsigned int i = 0; + for (auto &ctrl : *ctrls) { + if (i == count) + break; + const struct v4l2_ext_control *v4l2Ctrl = &v4l2Ctrls[i]; const V4L2ControlInfo *info = controlInfo[i]; - V4L2Control *ctrl = ctrls->getByIndex(i); + ControlValue &value = ctrl.second; switch (info->id().type()) { case ControlTypeInteger64: - ctrl->value().set(v4l2Ctrl->value64); + value.set(v4l2Ctrl->value64); break; default: /* * \todo To be changed when support for string and * compound controls will be added. */ - ctrl->value().set(v4l2Ctrl->value); + value.set(v4l2Ctrl->value); break; } + + i++; } } diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp index 4a7535245c00..783b695b8cbf 100644 --- a/test/v4l2_videodevice/controls.cpp +++ b/test/v4l2_videodevice/controls.cpp @@ -44,10 +44,10 @@ protected: const V4L2ControlInfo &saturation = info.find(V4L2_CID_SATURATION)->second; /* Test getting controls. */ - V4L2ControlList ctrls; - ctrls.add(V4L2_CID_BRIGHTNESS); - ctrls.add(V4L2_CID_CONTRAST); - ctrls.add(V4L2_CID_SATURATION); + V4L2ControlList ctrls(info); + ctrls.set(V4L2_CID_BRIGHTNESS, -1); + ctrls.set(V4L2_CID_CONTRAST, -1); + ctrls.set(V4L2_CID_SATURATION, -1); int ret = capture_->getControls(&ctrls); if (ret != 0) { @@ -55,17 +55,17 @@ protected: return TestFail; } - if (ctrls[V4L2_CID_BRIGHTNESS]->value().get() == -1 || - ctrls[V4L2_CID_CONTRAST]->value().get() == -1 || - ctrls[V4L2_CID_SATURATION]->value().get() == -1) { + if (ctrls.get(V4L2_CID_BRIGHTNESS).get() == -1 || + ctrls.get(V4L2_CID_CONTRAST).get() == -1 || + ctrls.get(V4L2_CID_SATURATION).get() == -1) { cerr << "Incorrect value for retrieved controls" << endl; return TestFail; } /* Test setting controls. */ - ctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min(); - ctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max(); - ctrls[V4L2_CID_SATURATION]->value() = saturation.range().min(); + ctrls.set(V4L2_CID_BRIGHTNESS, brightness.range().min()); + ctrls.set(V4L2_CID_CONTRAST, contrast.range().max()); + ctrls.set(V4L2_CID_SATURATION, saturation.range().min()); ret = capture_->setControls(&ctrls); if (ret != 0) { @@ -74,9 +74,9 @@ protected: } /* Test setting controls outside of range. */ - ctrls[V4L2_CID_BRIGHTNESS]->value() = brightness.range().min().get() - 1; - ctrls[V4L2_CID_CONTRAST]->value() = contrast.range().max().get() + 1; - ctrls[V4L2_CID_SATURATION]->value() = saturation.range().min().get() + 1; + ctrls.set(V4L2_CID_BRIGHTNESS, brightness.range().min().get() - 1); + ctrls.set(V4L2_CID_CONTRAST, contrast.range().max().get() + 1); + ctrls.set(V4L2_CID_SATURATION, saturation.range().min().get() + 1); ret = capture_->setControls(&ctrls); if (ret != 0) { @@ -84,9 +84,9 @@ protected: return TestFail; } - if (ctrls[V4L2_CID_BRIGHTNESS]->value() != brightness.range().min() || - ctrls[V4L2_CID_CONTRAST]->value() != brightness.range().max() || - ctrls[V4L2_CID_SATURATION]->value() != saturation.range().min().get() + 1) { + if (ctrls.get(V4L2_CID_BRIGHTNESS) != brightness.range().min() || + ctrls.get(V4L2_CID_CONTRAST) != brightness.range().max() || + ctrls.get(V4L2_CID_SATURATION) != saturation.range().min().get() + 1) { cerr << "Controls not updated when set" << endl; return TestFail; } From patchwork Sat Oct 12 18:44:07 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2175 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7DE7761985 for ; Sat, 12 Oct 2019 20:44:23 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 204BF54C for ; Sat, 12 Oct 2019 20:44:23 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1570905863; bh=tFI3g3TcoFavZvD5c9z6NKP/c9qf+W3af6KFvQJ5VcQ=; h=From:To:Subject:Date:In-Reply-To:References:From; b=X2J2Lmp94/9MpqQcgDCIXT/vp7imgjVOg3Oc7cDwyiJBHGzOY5DCY+ZqnDR8VhSYi vJfZfVqcK4sSiroT9fN0Q+GsRlLEmztxWd2bhGAKm9zChYq9k5o0IsenAQuUQEXuh6 sK5jxP54BueC1w3h3TDiFPV6ulMRdgsX7puZfpyo= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Oct 2019 21:44:07 +0300 Message-Id: <20191012184407.31684-15-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> References: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 14/14] libcamera: ipa: Merge controls and v4l2controls in IPAOperationData 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: Sat, 12 Oct 2019 18:44:24 -0000 Now that the V4L2ControlList is merely a helper to construct a ControlList for V4L2 controls, without any data member, all controls can be transferred between pipeline handlers and IPAs using ControlList only. Remove the v4l2controls member for IPAOperationData and use the control member instead. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/ipa/ipa_interface.h | 1 - src/ipa/rkisp1/rkisp1.cpp | 2 +- src/libcamera/ipa_interface.cpp | 8 -------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 +++--- 4 files changed, 4 insertions(+), 13 deletions(-) diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h index c393b64f6aa1..dfb1bcbee516 100644 --- a/include/ipa/ipa_interface.h +++ b/include/ipa/ipa_interface.h @@ -33,7 +33,6 @@ struct IPAOperationData { unsigned int operation; std::vector data; std::vector controls; - std::vector v4l2controls; }; class IPAInterface diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 69ced840585f..13059d99036c 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -215,7 +215,7 @@ void IPARkISP1::setControls(unsigned int frame) V4L2ControlList ctrls(ctrls_); ctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure_)); ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast(gain_)); - op.v4l2controls.push_back(ctrls); + op.controls.push_back(ctrls); queueFrameAction.emit(frame, op); } diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp index 4fb178298f4c..0571b8e6bf8b 100644 --- a/src/libcamera/ipa_interface.cpp +++ b/src/libcamera/ipa_interface.cpp @@ -120,14 +120,6 @@ namespace libcamera { * by the IPA protocol. */ -/** - * \var IPAOperationData::v4l2controls - * \brief Operation V4L2 controls data - * - * The interpretation and position of different values in the array are defined - * by the IPA protocol. - */ - /** * \class IPAInterface * \brief Interface for IPA implementation diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 32b023730009..9b19bde8a274 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -324,7 +324,7 @@ RkISP1FrameInfo *RkISP1Frames::find(Request *request) class RkISP1ActionSetSensor : public FrameAction { public: - RkISP1ActionSetSensor(unsigned int frame, CameraSensor *sensor, V4L2ControlList controls) + RkISP1ActionSetSensor(unsigned int frame, CameraSensor *sensor, const ControlList &controls) : FrameAction(frame, SetSensor), sensor_(sensor), controls_(controls) {} protected: @@ -335,7 +335,7 @@ protected: private: CameraSensor *sensor_; - V4L2ControlList controls_; + ControlList controls_; }; class RkISP1ActionQueueBuffers : public FrameAction @@ -387,7 +387,7 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame, { switch (action.operation) { case RKISP1_IPA_ACTION_V4L2_SET: { - V4L2ControlList controls = action.v4l2controls[0]; + const ControlList &controls = action.controls[0]; timeline_.scheduleAction(utils::make_unique(frame, sensor_, controls));