From patchwork Thu Nov 2 17:24:38 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 19188 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 5634CC327C for ; Thu, 2 Nov 2023 17:24:49 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DE9E561DC8; Thu, 2 Nov 2023 18:24:48 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1698945888; bh=Cvj0hzouLJ5CCqPMyLcqmu0yrX3vm66hpydwrZl15FE=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=XcgG37zqf/HUEh+oXPxtxifnx8d2cSOXAh1nK/STDNdLuBclxFfsqltD448B4J7Ss X+KKfQLVWybJSquLGCu10r4m6siVGmxkPuy1ei3Gv3eM+UDnEcDw6bnNUQOdujug4r nXN8188DPM2/bIQy7nbcfChQUpUK+XVXPp8K6gsPa8qeT7hE0zxkrdO6+VbKCDRyiY b7GUPcWitpNyzqUs9S7oWSVZL6ll7FQkeHqaiYnKF8/S2WGKbrpDH1simeuN7K3H+/ 9hqdNdGYGRh4Evj5q/2N0As4J0vaP1GttRwpsMepdj5B/xvTtH0sgQg+PFsaq95tpg muoMynRIu4Mcg== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1284D61DC8 for ; Thu, 2 Nov 2023 18:24:45 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Ax3sCIR9"; dkim-atps=neutral Received: from Monstersaurus.local (aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net [82.37.23.78]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 531E38C2; Thu, 2 Nov 2023 18:24:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1698945867; bh=Cvj0hzouLJ5CCqPMyLcqmu0yrX3vm66hpydwrZl15FE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Ax3sCIR9tWeSdoH0avBXdH807RR1L+8jSJaEH3KGpXu1oMWqaWHx2b3nF41rY1X5I W/93SiPEWBZT+qB8x+LAc/FjvZCr99RFInZzmLq0HewMP1SIFXKH4I4wd394+feSNF cjmZ1jUp69kvymzUEidG9+dT8I/OY/dSYtLK9KLA= To: libcamera devel Date: Thu, 2 Nov 2023 17:24:38 +0000 Message-Id: <20231102172439.3543870-2-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231102172439.3543870-1-kieran.bingham@ideasonboard.com> References: <20231102172439.3543870-1-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 1/2] libcamera: controls: Add read-only flag to ControlInfo 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-Patchwork-Original-From: Kieran Bingham via libcamera-devel From: Kieran Bingham Reply-To: Kieran Bingham Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Naushir Patuck Add a read-only flag to the ControlInfo flag to indicate whether a V4L2 control is read-only or not. This flag only makes sense for V2L2 controls at preset, so only update the ControlInfo constructor signature used by the V4L2Device class to set the value of the flag. Add a ControlInfo::readOnly() member function to retrive this flag. Signed-off-by: Naushir Patuck Signed-off-by: Kieran Bingham --- v2: - Initialised readOnly_ for ControlInfo(Span<>) - Improved documentation include/libcamera/controls.h | 5 ++++- src/libcamera/controls.cpp | 22 ++++++++++++++++++---- src/libcamera/v4l2_device.cpp | 12 ++++++++---- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index cf94205577a5..488663a7ba04 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -270,7 +270,8 @@ class ControlInfo public: explicit ControlInfo(const ControlValue &min = {}, const ControlValue &max = {}, - const ControlValue &def = {}); + const ControlValue &def = {}, + bool readOnly = false); explicit ControlInfo(Span values, const ControlValue &def = {}); explicit ControlInfo(std::set values, bool def); @@ -279,6 +280,7 @@ public: const ControlValue &min() const { return min_; } const ControlValue &max() const { return max_; } const ControlValue &def() const { return def_; } + bool readOnly() const { return readOnly_; } const std::vector &values() const { return values_; } std::string toString() const; @@ -297,6 +299,7 @@ private: ControlValue min_; ControlValue max_; ControlValue def_; + bool readOnly_; std::vector values_; }; diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index b808116c01e5..8af304bdeac9 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -484,11 +484,13 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen * \param[in] min The control minimum value * \param[in] max The control maximum value * \param[in] def The control default value + * \param[in] readOnly True if the control reports a dynamic property */ ControlInfo::ControlInfo(const ControlValue &min, const ControlValue &max, - const ControlValue &def) - : min_(min), max_(max), def_(def) + const ControlValue &def, + bool readOnly) + : min_(min), max_(max), def_(def), readOnly_(readOnly) { } @@ -508,6 +510,7 @@ ControlInfo::ControlInfo(Span values, min_ = values.front(); max_ = values.back(); def_ = !def.isNone() ? def : values.front(); + readOnly_ = false; values_.reserve(values.size()); for (const ControlValue &value : values) @@ -525,7 +528,8 @@ ControlInfo::ControlInfo(Span values, * default value is \a def. */ ControlInfo::ControlInfo(std::set values, bool def) - : min_(false), max_(true), def_(def), values_({ false, true }) + : min_(false), max_(true), def_(def), readOnly_(false), + values_({ false, true }) { ASSERT(values.count(def) && values.size() == 2); } @@ -538,7 +542,7 @@ ControlInfo::ControlInfo(std::set values, bool def) * value. The minimum, maximum, and default values will all be \a value. */ ControlInfo::ControlInfo(bool value) - : min_(value), max_(value), def_(value) + : min_(value), max_(value), def_(value), readOnly_(true) { values_ = { value }; } @@ -571,6 +575,16 @@ ControlInfo::ControlInfo(bool value) * \return A ControlValue with the default value for the control */ +/** + * \fn ControlInfo::readOnly() + * \brief Identifies if a control is flagged as read-only + * \return True if the control is read-only, false otherwise + * + * Read-only controls may signify that the control is a volatile property and + * can not be set. Adding a read-only control to a control list may cause the + * list to fail when the list is submitted. + */ + /** * \fn ControlInfo::values() * \brief Retrieve the list of valid values diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 24d208ef77dc..8563bbd2595f 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -536,17 +536,20 @@ std::optional V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl case V4L2_CTRL_TYPE_U8: return ControlInfo(static_cast(ctrl.minimum), static_cast(ctrl.maximum), - static_cast(ctrl.default_value)); + static_cast(ctrl.default_value), + !!(ctrl.flags & V4L2_CTRL_FLAG_READ_ONLY)); case V4L2_CTRL_TYPE_BOOLEAN: return ControlInfo(static_cast(ctrl.minimum), static_cast(ctrl.maximum), - static_cast(ctrl.default_value)); + static_cast(ctrl.default_value), + !!(ctrl.flags & V4L2_CTRL_FLAG_READ_ONLY)); case V4L2_CTRL_TYPE_INTEGER64: return ControlInfo(static_cast(ctrl.minimum), static_cast(ctrl.maximum), - static_cast(ctrl.default_value)); + static_cast(ctrl.default_value), + !!(ctrl.flags & V4L2_CTRL_FLAG_READ_ONLY)); case V4L2_CTRL_TYPE_INTEGER_MENU: case V4L2_CTRL_TYPE_MENU: @@ -555,7 +558,8 @@ std::optional V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl default: return ControlInfo(static_cast(ctrl.minimum), static_cast(ctrl.maximum), - static_cast(ctrl.default_value)); + static_cast(ctrl.default_value), + !!(ctrl.flags & V4L2_CTRL_FLAG_READ_ONLY)); } } From patchwork Thu Nov 2 17:24:39 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 19189 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 9004CC32B6 for ; Thu, 2 Nov 2023 17:24:50 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 62C1861DC6; Thu, 2 Nov 2023 18:24:49 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1698945889; bh=+bbm4psm5cdiQCmfBl2gZxOBC9GXr3hgFzDEb8VJY7g=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=kUWNzDAsJlA51pzskVweJujcThEC9N8kVdiG6l1SbDgaLDdLiXaNqOTxSKhmtFPv4 VBL+ds9ThbtChOdi4NGO7VR7LhT3LG9SNQo6WdO7AcovV0Tq5PNFpreuu9t8//3H6V v4kxh3n+/OELMsvLXzjbEZJIGtNIqYnFsUCa+xh+myUo6RY/ZVGDrAkux930XDk+uV +T4d31ovO//WOz/eZHUg1Smzj79XR0CCPum5vJlrIJIsolMyegsyyjli8CxON4E0dA 9NH4iizpV67aR8lKduLP4DREy2K1RCNtTYnRCyONcyEXc/Mk5onSpaNRJyIKqiwLJH 0Q49tBgD7HvqQ== 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 485D061DC6 for ; Thu, 2 Nov 2023 18:24:45 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="sHBcdCEx"; dkim-atps=neutral Received: from Monstersaurus.local (aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net [82.37.23.78]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 90A2D1B44; Thu, 2 Nov 2023 18:24:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1698945867; bh=+bbm4psm5cdiQCmfBl2gZxOBC9GXr3hgFzDEb8VJY7g=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=sHBcdCEx8e3485IrF697GpBJ+9ckznps88A0UxsHHxNjl69C1DES1hFSY0FRKeApH vfnpQ1qpWrYSyIs9+g/zdCyjFT1iMKVZQSwOrFPo/u2nH/Zv7WMMDa1zS5A88uWEOi wnmPVvAbuVb0h4mShD44Z79n0A7fkmqP+t0TW+ks= To: libcamera devel Date: Thu, 2 Nov 2023 17:24:39 +0000 Message-Id: <20231102172439.3543870-3-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231102172439.3543870-1-kieran.bingham@ideasonboard.com> References: <20231102172439.3543870-1-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 2/2] libcamera:camera_sensor, ipa: raspberrypi: Test readOnly() for HBLANK control 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-Patchwork-Original-From: Kieran Bingham via libcamera-devel From: Kieran Bingham Reply-To: Kieran Bingham Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Naushir Patuck Use ControlInfo::readOnly() to test if the camera sensor accepts V4L2_CID_HBLANK control changes. This replaces the current workaround where we test if the V4L2_CID_HBLANK min and max values are the same to determine if a control is read-only. Signed-off-by: Naushir Patuck Reviewed-by: Jacopo Mondi Reviewed-by: Kieran Bingham Signed-off-by: Kieran Bingham --- src/ipa/rpi/common/ipa_base.cpp | 12 +----------- src/libcamera/camera_sensor.cpp | 14 ++------------ 2 files changed, 3 insertions(+), 23 deletions(-) diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp index a1fec3aa3dd1..a623ad1bae66 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -1346,17 +1346,7 @@ void IpaBase::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls) ctrls.set(V4L2_CID_EXPOSURE, exposureLines); ctrls.set(V4L2_CID_ANALOGUE_GAIN, gainCode); - /* - * At present, there is no way of knowing if a control is read-only. - * As a workaround, assume that if the minimum and maximum values of - * the V4L2_CID_HBLANK control are the same, it implies the control - * is read-only. This seems to be the case for all the cameras our IPA - * works with. - * - * \todo The control API ought to have a flag to specify if a control - * is read-only which could be used below. - */ - if (mode_.minLineLength != mode_.maxLineLength) + if (!sensorCtrls_.at(V4L2_CID_HBLANK).readOnly()) ctrls.set(V4L2_CID_HBLANK, static_cast(hblank)); /* diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index ee1066c680ff..12512341feb6 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -188,20 +188,10 @@ int CameraSensor::init() * Set HBLANK to the minimum to start with a well-defined line length, * allowing IPA modules that do not modify HBLANK to use the sensor * minimum line length in their calculations. - * - * At present, there is no way of knowing if a control is read-only. - * As a workaround, assume that if the minimum and maximum values of - * the V4L2_CID_HBLANK control are the same, it implies the control - * is read-only. - * - * \todo The control API ought to have a flag to specify if a control - * is read-only which could be used below. */ const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK); - const int32_t hblankMin = hblank.min().get(); - const int32_t hblankMax = hblank.max().get(); - - if (hblankMin != hblankMax) { + if (!hblank.readOnly()) { + const int32_t hblankMin = hblank.min().get(); ControlList ctrl(subdev_->controls()); ctrl.set(V4L2_CID_HBLANK, hblankMin);