From patchwork Sat Apr 25 23:16:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3544 X-Patchwork-Delegate: laurent.pinchart@ideasonboard.com Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id CEDDB603FD for ; Sun, 26 Apr 2020 01:16:41 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="qZ7xzRTw"; dkim-atps=neutral Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 3DFB84F7; Sun, 26 Apr 2020 01:16:41 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1587856601; bh=UkjB9ANiqnfRJ3FzWVHqjroZgqqC4m7kiUIXDtrFLfs=; h=From:To:Cc:Subject:Date:From; b=qZ7xzRTw7dujmrPfznPDw8WiZ8/A2AGkhC/Jeq81G+Z0pVO8+f9YDc8qwqNiIbadH l3+bfG6Kh7hdY3a2UBUe4cmXtW28PF7ddhO0O/q4WTBDhhWyXus1mDhCIqQE0UNw5w 1zJSXOQfI8TtyohO0c6fzCG10oOepsyIPHsUnYm0= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 26 Apr 2020 02:16:22 +0300 Message-Id: <20200425231623.6505-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.25.3 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/2] libcamera: controls: Return ControlValue reference from ControlList::set() 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, 25 Apr 2020 23:16:42 -0000 It is useful for the caller of ControlList::set() to get a reference to the ControlValue that stores the control in the control list, in order to then modify that value directly. This allows creating an entry in the ControlList and then reserving memory for the control when getting V4L2 controls from a device. This is already possible by calling the get() function right after set(), but results in two lookups. Extend the id-based set() function to return the reference to the inserted ControlValue to avoid the second lookup. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- include/libcamera/controls.h | 2 +- src/libcamera/controls.cpp | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 80944efc133a..5a5cfc3e52ca 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -394,7 +394,7 @@ public: } const ControlValue &get(unsigned int id) const; - void set(unsigned int id, const ControlValue &value); + ControlValue &set(unsigned int id, const ControlValue &value); const ControlInfoMap *infoMap() const { return infoMap_; } diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 08df7f29e938..12822e87a4d7 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -930,14 +930,17 @@ const ControlValue &ControlList::get(unsigned int id) const * * The behaviour is undefined if the control \a id is not supported by the * object that the list refers to. + * + * \return A reference to the ControlValue stored in the control list */ -void ControlList::set(unsigned int id, const ControlValue &value) +ControlValue &ControlList::set(unsigned int id, const ControlValue &value) { ControlValue *val = find(id); if (!val) - return; + return *val; *val = value; + return *val; } /** From patchwork Sat Apr 25 23:16:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3545 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 21A93603FD for ; Sun, 26 Apr 2020 01:16:42 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="UkCGD9Gj"; dkim-atps=neutral Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A3D0B564; Sun, 26 Apr 2020 01:16:41 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1587856601; bh=DpQsZoJDyr+ksVcMup3zK59+RrOfxnGAEz1BJfVPhA4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=UkCGD9Gjzq2X1DY4aHT/kFdCulCmrqDQZVEZN5jsN6zhSiMpMOfX5kEKNjuedur0x 8QOHpywXSxv3lXfBKiPlI1XNkWSL2ZG4bTG69/kateh95J0ukGTX/ziAtj/UcQAnuA h7YUyAw1C5VokzDaMNgyNX+4CGLD7tfC5qGN28Zc= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sun, 26 Apr 2020 02:16:23 +0300 Message-Id: <20200425231623.6505-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.25.3 In-Reply-To: <20200425231623.6505-1-laurent.pinchart@ideasonboard.com> References: <20200425231623.6505-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 2/2] libcamera: v4l2_device: Simplify usage of getControls() 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, 25 Apr 2020 23:16:42 -0000 The V4L2Device::getControls() function takes a ControlList that needs to be pre-populated with dummy entries for the controls that need to be read. This is a cumbersome API, especially when reading a single control. Make it nicer by passing the list of V4L2 controls as a vector of control IDs, and returning a ControlList. Signed-off-by: Laurent Pinchart --- src/libcamera/camera_sensor.cpp | 23 ++++++--------- src/libcamera/include/camera_sensor.h | 2 +- src/libcamera/include/v4l2_device.h | 2 +- src/libcamera/v4l2_device.cpp | 42 ++++++++++++--------------- test/v4l2_videodevice/controls.cpp | 20 +++++++------ 5 files changed, 41 insertions(+), 48 deletions(-) diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index 2219a4307436..ce585cab1a4f 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -299,30 +299,25 @@ const ControlInfoMap &CameraSensor::controls() const /** * \brief Read controls from the sensor - * \param[inout] ctrls The list of controls to read + * \param[in] ids The list of control to read, specified by their ID * - * This method reads the value of all controls contained in \a ctrls, and stores - * their values in the corresponding \a ctrls entry. + * This method reads the value of all controls contained in \a ids, and returns + * their values as a ControlList. * - * If any control in \a ctrls is not supported by the device, is disabled (i.e. + * If any control in \a ids is not supported by the device, is disabled (i.e. * has the V4L2_CTRL_FLAG_DISABLED flag set), is a compound control, or if any * other error occurs during validation of the requested controls, no control is - * read and this method returns -EINVAL. + * read and this method returns an empty control list. * - * If an error occurs while reading the controls, the index of the first control - * that couldn't be read is returned. The value of all controls below that index - * are updated in \a ctrls, while the value of all the other controls are not - * changed. + * If an error occurs while reading the controls, an empty list is returned. * * \sa V4L2Device::getControls() * - * \return 0 on success or an error code otherwise - * \retval -EINVAL One of the control is not supported or not accessible - * \retval i The index of the control that failed + * \return The control values in a ControlList */ -int CameraSensor::getControls(ControlList *ctrls) +ControlList CameraSensor::getControls(const std::vector &ids) { - return subdev_->getControls(ctrls); + return subdev_->getControls(ids); } /** diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h index 99cff98128dc..5277f7f7fe87 100644 --- a/src/libcamera/include/camera_sensor.h +++ b/src/libcamera/include/camera_sensor.h @@ -43,7 +43,7 @@ public: int setFormat(V4L2SubdeviceFormat *format); const ControlInfoMap &controls() const; - int getControls(ControlList *ctrls); + ControlList getControls(const std::vector &ids); int setControls(ControlList *ctrls); const ControlList &properties() const { return properties_; } diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h index ce8edd98a01d..e604a40df4c9 100644 --- a/src/libcamera/include/v4l2_device.h +++ b/src/libcamera/include/v4l2_device.h @@ -26,7 +26,7 @@ public: const ControlInfoMap &controls() const { return controls_; } - int getControls(ControlList *ctrls); + ControlList getControls(const std::vector &ids); int setControls(ControlList *ctrls); const std::string &deviceNode() const { return deviceNode_; } diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 03e305165096..989ed05c9692 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -147,46 +147,42 @@ void V4L2Device::close() /** * \brief Read controls from the device - * \param[inout] ctrls The list of controls to read + * \param[in] ids The list of control to read, specified by their ID * - * This method reads the value of all controls contained in \a ctrls, and stores - * their values in the corresponding \a ctrls entry. + * This method reads the value of all controls contained in \a ids, and returns + * their values as a ControlList. * - * If any control in \a ctrls is not supported by the device, is disabled (i.e. + * If any control in \a ids is not supported by the device, is disabled (i.e. * has the V4L2_CTRL_FLAG_DISABLED flag set), is a compound control, or if any * other error occurs during validation of the requested controls, no control is - * read and this method returns -EINVAL. + * read and this method returns an empty control list. * - * If an error occurs while reading the controls, the index of the first control - * that couldn't be read is returned. The value of all controls below that index - * are updated in \a ctrls, while the value of all the other controls are not - * changed. + * If an error occurs while reading the controls, an empty list is returned. * - * \return 0 on success or an error code otherwise - * \retval -EINVAL One of the control is not supported or not accessible - * \retval i The index of the control that failed + * \return The control values in a ControlList */ -int V4L2Device::getControls(ControlList *ctrls) +ControlList V4L2Device::getControls(const std::vector &ids) { - unsigned int count = ctrls->size(); + unsigned int count = ids.size(); if (count == 0) - return 0; + return {}; + + ControlList ctrls{ controls_ }; struct v4l2_ext_control v4l2Ctrls[count]; memset(v4l2Ctrls, 0, sizeof(v4l2Ctrls)); unsigned int i = 0; - for (auto &ctrl : *ctrls) { - unsigned int id = ctrl.first; + for (uint32_t id : ids) { const auto iter = controls_.find(id); if (iter == controls_.end()) { LOG(V4L2, Error) << "Control " << utils::hex(id) << " not found"; - return -EINVAL; + return {}; } const struct v4l2_query_ext_ctrl &info = controlInfo_[id]; - ControlValue &value = ctrl.second; + ControlValue &value = ctrls.set(id, {}); if (info.flags & V4L2_CTRL_FLAG_HAS_PAYLOAD) { ControlType type; @@ -200,7 +196,7 @@ int V4L2Device::getControls(ControlList *ctrls) LOG(V4L2, Error) << "Unsupported payload control type " << info.type; - return -EINVAL; + return {}; } value.reserve(type, true, info.elems); @@ -227,7 +223,7 @@ int V4L2Device::getControls(ControlList *ctrls) if (errorIdx == 0 || errorIdx >= count) { LOG(V4L2, Error) << "Unable to read controls: " << strerror(-ret); - return -EINVAL; + return {}; } /* A specific control failed. */ @@ -237,9 +233,9 @@ int V4L2Device::getControls(ControlList *ctrls) ret = errorIdx; } - updateControls(ctrls, v4l2Ctrls, count); + updateControls(&ctrls, v4l2Ctrls, count); - return ret; + return ctrls; } /** diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp index da9e0111e221..347af2112f1a 100644 --- a/test/v4l2_videodevice/controls.cpp +++ b/test/v4l2_videodevice/controls.cpp @@ -57,18 +57,20 @@ protected: const ControlInfo &u8 = infoMap.find(VIVID_CID_U8_4D_ARRAY)->second; /* Test getting controls. */ - ControlList ctrls(infoMap); - ctrls.set(V4L2_CID_BRIGHTNESS, -1); - ctrls.set(V4L2_CID_CONTRAST, -1); - ctrls.set(V4L2_CID_SATURATION, -1); - ctrls.set(VIVID_CID_U8_4D_ARRAY, 0); - - int ret = capture_->getControls(&ctrls); - if (ret) { + ControlList ctrls = capture_->getControls({ V4L2_CID_BRIGHTNESS, + V4L2_CID_CONTRAST, + V4L2_CID_SATURATION, + VIVID_CID_U8_4D_ARRAY }); + if (ctrls.empty()) { cerr << "Failed to get controls" << endl; return TestFail; } + if (ctrls.infoMap() != &infoMap) { + cerr << "Incorrect infoMap for retrieved controls" << endl; + return TestFail; + } + if (ctrls.get(V4L2_CID_BRIGHTNESS).get() == -1 || ctrls.get(V4L2_CID_CONTRAST).get() == -1 || ctrls.get(V4L2_CID_SATURATION).get() == -1) { @@ -97,7 +99,7 @@ protected: std::fill(u8Values.begin(), u8Values.end(), u8.min().get()); ctrls.set(VIVID_CID_U8_4D_ARRAY, Span(u8Values)); - ret = capture_->setControls(&ctrls); + int ret = capture_->setControls(&ctrls); if (ret) { cerr << "Failed to set controls" << endl; return TestFail;