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;