From patchwork Tue Aug 16 01:54:06 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 17128 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 A61B9C3272 for ; Tue, 16 Aug 2022 01:54:32 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 4FD2461FBC; Tue, 16 Aug 2022 03:54:32 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1660614872; bh=hHJclU+jgLIrh4iJSwZNCcWf+UXGK/ilaMiZ2D55/rk=; 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=kHAvwdI+MmMS884763plKq6gVUNdP05R0hkjKXH1OneIuGSb+WRiY/KXdMoJy6i1G zF4rQn8ogQO2MfB33fEY1NhT8OeB6ufgXN6Z6Pre0OajFEfVBeNokcPs6SMQNXtHlS sHBKs3M2NYdf3D13Nyv0nBvFeJk3M8vKoEMRh2ifDWaYSAJTb1/IHwvJY8BYmqEo1i eU+Ia90vGD76aIh5yGxbFvpWC/uXPPTr3aKRJap4hwGc+06MqwrLoalHaxlY8fLQw7 i6EGHiKXwEG3LuUAkbuS4zebOIMFUJSp0fHHAuRIWy3ygmDjkIYKs+W1+ZNJ+XLw4m 54Xi8vG8r7bUA== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B985D61FBC for ; Tue, 16 Aug 2022 03:54:29 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="u9F4jHEp"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 57E5A496; Tue, 16 Aug 2022 03:54:29 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1660614869; bh=hHJclU+jgLIrh4iJSwZNCcWf+UXGK/ilaMiZ2D55/rk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=u9F4jHEp6melVzXOHNS19HKZ3nsrMyGpUJTtyQ7As43CPIn3ff1PGxXtXTsnxCqC6 9ZXiyFjkdRhCgZToID+/+it1uti3Mr/UmwW6cfTfI9HOjPjJuLKLdgcDaSKTjtddsm qqLAPtQgHZKRXgsO8NAK8h8tI1ZK65IuQdV4Tw5s= To: libcamera-devel@lists.libcamera.org Date: Tue, 16 Aug 2022 04:54:06 +0300 Message-Id: <20220816015414.7462-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> References: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 1/9] test: yaml-parser: Simplify code by centralizing parse error checks 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: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Centralize most parse failure checks in a single function to avoid a larger number of copies of nearly identical checks. Signed-off-by: Laurent Pinchart Reviewed-by: Paul Elder Reviewed-by: Jacopo Mondi --- test/yaml-parser.cpp | 309 +++++++++++++------------------------------ 1 file changed, 95 insertions(+), 214 deletions(-) diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp index 93ba88b8bcd5..6729e1bd430e 100644 --- a/test/yaml-parser.cpp +++ b/test/yaml-parser.cpp @@ -72,6 +72,83 @@ protected: return TestPass; } + enum class Type { + String, + Int32, + UInt32, + Double, + Size, + List, + Dictionary, + }; + + int testObjectType(const YamlObject &obj, const char *name, Type type) + { + bool isList = type == Type::List || type == Type::Size; + bool isScalar = !isList && type != Type::Dictionary; + bool isInteger = type == Type::Int32 || type == Type::UInt32; + bool isSigned = type == Type::Int32; + + if ((isScalar && !obj.isValue()) || (!isScalar && obj.isValue())) { + std::cerr + << "Object " << name << " type mismatch when compared to " + << "value" << std::endl; + return TestFail; + } + + if ((isList && !obj.isList()) || (!isList && obj.isList())) { + std::cerr + << "Object " << name << " type mismatch when compared to " + << "list" << std::endl; + return TestFail; + } + + if ((type == Type::Dictionary && !obj.isDictionary()) || + (type != Type::Dictionary && obj.isDictionary())) { + std::cerr + << "Object " << name << " type mismatch when compared to " + << "dictionary" << std::endl; + return TestFail; + } + + if (!isScalar && obj.get()) { + std::cerr + << "Object " << name << " didn't fail to parse as " + << "string" << std::endl; + return TestFail; + } + + if (!isInteger && obj.get()) { + std::cerr + << "Object " << name << " didn't fail to parse as " + << "int32_t" << std::endl; + return TestFail; + } + + if ((!isInteger || isSigned) && obj.get()) { + std::cerr + << "Object " << name << " didn't fail to parse as " + << "uint32_t" << std::endl; + return TestFail; + } + + if (!isInteger && type != Type::Double && obj.get()) { + std::cerr + << "Object " << name << " didn't fail to parse as " + << "double" << std::endl; + return TestFail; + } + + if (type != Type::Size && obj.get()) { + std::cerr + << "Object " << name << " didn't fail to parse as " + << "Size" << std::endl; + return TestFail; + } + + return TestPass; + } + int run() { /* Test invalid YAML file */ @@ -107,58 +184,24 @@ protected: return TestFail; } - if (!root->contains("string")) { - cerr << "Missing string object in YAML root" << std::endl; - return TestFail; - } - - if (!root->contains("double")) { - cerr << "Missing double object in YAML root" << std::endl; - return TestFail; - } - - if (!root->contains("int32_t")) { - cerr << "Missing int32_t object in YAML root" << std::endl; - return TestFail; - } - - if (!root->contains("uint32_t")) { - cerr << "Missing uint32_t object in YAML root" << std::endl; - return TestFail; - } - - if (!root->contains("size")) { - cerr << "Missing Size object in YAML root" << std::endl; - return TestFail; - } - - if (!root->contains("list")) { - cerr << "Missing list object in YAML root" << std::endl; - return TestFail; - } - - if (!root->contains("dictionary")) { - cerr << "Missing dictionary object in YAML root" << std::endl; - return TestFail; - } - - if (!root->contains("level1")) { - cerr << "Missing leveled object in YAML root" << std::endl; - return TestFail; + std::vector rootElemNames = { + "string", "double", "int32_t", "uint32_t", "size", + "list", "dictionary", "level1", + }; + + for (const char *name : rootElemNames) { + if (!root->contains(name)) { + cerr << "Missing " << name << " object in YAML root" + << std::endl; + return TestFail; + } } /* Test string object */ auto &strObj = (*root)["string"]; - if (strObj.isDictionary()) { - cerr << "String object parse as Dictionary" << std::endl; + if (testObjectType(strObj, "string", Type::String) != TestPass) return TestFail; - } - - if (strObj.isList()) { - cerr << "String object parse as List" << std::endl; - return TestFail; - } if (strObj.get().value_or("") != "libcamera" || strObj.get("") != "libcamera") { @@ -166,38 +209,11 @@ protected: return TestFail; } - if (strObj.get()) { - cerr << "String object parse as integer" << std::endl; - return TestFail; - } - - if (strObj.get()) { - cerr << "String object parse as unsigned integer" << std::endl; - return TestFail; - } - - if (strObj.get()) { - cerr << "String object parse as double" << std::endl; - return TestFail; - } - - if (strObj.get()) { - cerr << "String object parse as Size" << std::endl; - return TestFail; - } - /* Test int32_t object */ auto &int32Obj = (*root)["int32_t"]; - if (int32Obj.isDictionary()) { - cerr << "Integer object parse as Dictionary" << std::endl; + if (testObjectType(int32Obj, "int32_t", Type::Int32) != TestPass) return TestFail; - } - - if (int32Obj.isList()) { - cerr << "Integer object parse as Integer" << std::endl; - return TestFail; - } if (int32Obj.get().value_or(0) != -100 || int32Obj.get(0) != -100) { @@ -217,28 +233,11 @@ protected: return TestFail; } - if (int32Obj.get()) { - cerr << "Negative integer object parse as unsigned integer" << std::endl; - return TestFail; - } - - if (int32Obj.get()) { - cerr << "Integer object parse as Size" << std::endl; - return TestFail; - } - /* Test uint32_t object */ auto &uint32Obj = (*root)["uint32_t"]; - if (uint32Obj.isDictionary()) { - cerr << "Unsigned integer object parse as Dictionary" << std::endl; + if (testObjectType(uint32Obj, "uint32_t", Type::UInt32) != TestPass) return TestFail; - } - - if (uint32Obj.isList()) { - cerr << "Unsigned integer object parse as List" << std::endl; - return TestFail; - } if (uint32Obj.get().value_or(0) != 100 || uint32Obj.get(0) != 100) { @@ -264,23 +263,11 @@ protected: return TestFail; } - if (uint32Obj.get()) { - cerr << "Unsigned integer object parsed as Size" << std::endl; - return TestFail; - } - /* Test double value */ auto &doubleObj = (*root)["double"]; - if (doubleObj.isDictionary()) { - cerr << "Double object parse as Dictionary" << std::endl; + if (testObjectType(doubleObj, "double", Type::Double) != TestPass) return TestFail; - } - - if (doubleObj.isList()) { - cerr << "Double object parse as List" << std::endl; - return TestFail; - } if (doubleObj.get().value_or("") != "3.14159" || doubleObj.get("") != "3.14159") { @@ -294,53 +281,11 @@ protected: return TestFail; } - if (doubleObj.get()) { - cerr << "Double object parse as integer" << std::endl; - return TestFail; - } - - if (doubleObj.get()) { - cerr << "Double object parse as unsigned integer" << std::endl; - return TestFail; - } - - if (doubleObj.get()) { - cerr << "Double object parse as Size" << std::endl; - return TestFail; - } - /* Test Size value */ auto &sizeObj = (*root)["size"]; - if (sizeObj.isDictionary()) { - cerr << "Size object parse as Dictionary" << std::endl; + if (testObjectType(sizeObj, "size", Type::Size) != TestPass) return TestFail; - } - - if (!sizeObj.isList()) { - cerr << "Size object parse as List" << std::endl; - return TestFail; - } - - if (sizeObj.get()) { - cerr << "Size object parse as string" << std::endl; - return TestFail; - } - - if (sizeObj.get()) { - cerr << "Size object parse as double" << std::endl; - return TestFail; - } - - if (sizeObj.get()) { - cerr << "Size object parse as integer" << std::endl; - return TestFail; - } - - if (sizeObj.get()) { - cerr << "Size object parse as unsigned integer" << std::endl; - return TestFail; - } if (sizeObj.get().value_or(Size(0, 0)) != Size(1920, 1080) || sizeObj.get(Size(0, 0)) != Size(1920, 1080)) { @@ -351,40 +296,8 @@ protected: /* Test list object */ auto &listObj = (*root)["list"]; - if (listObj.isDictionary()) { - cerr << "List object parse as Dictionary" << std::endl; + if (testObjectType(listObj, "list", Type::List) != TestPass) return TestFail; - } - - if (!listObj.isList()) { - cerr << "List object fail to parse as List" << std::endl; - return TestFail; - } - - if (listObj.get()) { - cerr << "List object parse as string" << std::endl; - return TestFail; - } - - if (listObj.get()) { - cerr << "List object parse as double" << std::endl; - return TestFail; - } - - if (listObj.get()) { - cerr << "List object parse as integer" << std::endl; - return TestFail; - } - - if (listObj.get()) { - cerr << "List object parse as unsigne integer" << std::endl; - return TestFail; - } - - if (listObj.get()) { - cerr << "String list object parse as Size" << std::endl; - return TestFail; - } static constexpr std::array listValues{ "James", @@ -424,40 +337,8 @@ protected: /* Test dictionary object */ auto &dictObj = (*root)["dictionary"]; - if (!dictObj.isDictionary()) { - cerr << "Dictionary object fail to parse as Dictionary" << std::endl; + if (testObjectType(dictObj, "dictionary", Type::Dictionary) != TestPass) return TestFail; - } - - if (dictObj.isList()) { - cerr << "Dictionary object parse as List" << std::endl; - return TestFail; - } - - if (dictObj.get()) { - cerr << "Dictionary object parse as string" << std::endl; - return TestFail; - } - - if (dictObj.get()) { - cerr << "Dictionary object parse as double" << std::endl; - return TestFail; - } - - if (dictObj.get()) { - cerr << "Dictionary object parse as integer" << std::endl; - return TestFail; - } - - if (dictObj.get()) { - cerr << "Dictionary object parse as unsigned integer" << std::endl; - return TestFail; - } - - if (dictObj.get()) { - cerr << "Dictionary object parse as Size" << std::endl; - return TestFail; - } static constexpr std::array, 3> dictValues{ { { "a", 1 }, From patchwork Tue Aug 16 01:54:07 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 17129 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 DE876C3272 for ; Tue, 16 Aug 2022 01:54:33 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D443261FC8; Tue, 16 Aug 2022 03:54:32 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1660614872; bh=RFaj1YfbNtPs/J89bCrjLlmBZYKCilVYZMH7MRWeKW0=; 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=d/XAWfWBe2eKCh4kQ6vj3jQX6+Ey+sbtCdssR+ZLKuO0hwpUIhsXAopZJ8ZMZLo01 JQ33JdmyHKH2hr1iiS9GHxbG+p6sbzfYNVR3tTMlZg3tBlcMQNpl8h2MAdWoEN9Ajn ws3WdXkJbkSOFvW0WMOdj3Z6reITwUWrWdm+kCdoK+V5kDC1So1liZHLiTZZ7gKV52 RUuYjf6hRCpLt+mWkdI+0kv1+/0ptFn7hxujvvSeAo4cZfTg7WJ6xTJRtpNY7XnaIN tFn6Xml0JVXUk8TXVBPF0Yh2Sel/W6/glbonbcmERnAZX4o3SyFFpsSUy6a6uHPCe2 qzZ7y5jtLSARQ== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 520FE603E3 for ; Tue, 16 Aug 2022 03:54:31 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="UzdhceGp"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D7A65496; Tue, 16 Aug 2022 03:54:30 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1660614871; bh=RFaj1YfbNtPs/J89bCrjLlmBZYKCilVYZMH7MRWeKW0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=UzdhceGpW9lpmc71NLZCHnxBmNsVMfKlqlHscKTZtQzBaP5t7HgzNviIAH78MUt8G KFf8c/ZMinsDPsjLqlpVk4rAY370dC2UPSQ0O+qM9XDaur6P+iZQyPfkZRLVTuGF+I 5bBziINfsLGFto9M8b7iTMW0uTuKBO67t9xCDORw= To: libcamera-devel@lists.libcamera.org Date: Tue, 16 Aug 2022 04:54:07 +0300 Message-Id: <20220816015414.7462-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> References: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 2/9] test: yaml-parser: Centralize integer parse checks 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: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Centralize the signed and unsigned integer parse checks to avoid code duplication. The diffstat isn't very impressive at this point, but this will help more when adding 8-bit and 16-bit integer tests. Signed-off-by: Laurent Pinchart Reviewed-by: Paul Elder --- test/yaml-parser.cpp | 86 ++++++++++++++++++++++++-------------------- 1 file changed, 48 insertions(+), 38 deletions(-) diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp index 6729e1bd430e..803e70beb782 100644 --- a/test/yaml-parser.cpp +++ b/test/yaml-parser.cpp @@ -149,6 +149,52 @@ protected: return TestPass; } + int testIntegerObject(const YamlObject &obj, const char *name, Type type, + int64_t value) + { + uint64_t unsignedValue = static_cast(value); + std::string strValue = std::to_string(value); + bool isSigned = type == Type::Int32; + + /* All integers can be parsed as strings or double. */ + + if (obj.get().value_or("") != strValue || + obj.get("") != strValue) { + std::cerr + << "Object " << name << " failed to parse as " + << "string" << std::endl; + return TestFail; + } + + if (obj.get().value_or(0.0) != value || + obj.get(0.0) != value) { + std::cerr + << "Object " << name << " failed to parse as " + << "double" << std::endl; + return TestFail; + } + + if (obj.get().value_or(0) != value || + obj.get(0) != value) { + std::cerr + << "Object " << name << " failed to parse as " + << "int32_t" << std::endl; + return TestFail; + } + + if (!isSigned) { + if (obj.get().value_or(0) != unsignedValue || + obj.get(0) != unsignedValue) { + std::cerr + << "Object " << name << " failed to parse as " + << "uint32_t" << std::endl; + return TestFail; + } + } + + return TestPass; + } + int run() { /* Test invalid YAML file */ @@ -215,23 +261,8 @@ protected: if (testObjectType(int32Obj, "int32_t", Type::Int32) != TestPass) return TestFail; - if (int32Obj.get().value_or(0) != -100 || - int32Obj.get(0) != -100) { - cerr << "Integer object parse as wrong value" << std::endl; + if (testIntegerObject(int32Obj, "int32_t", Type::Int32, -100) != TestPass) return TestFail; - } - - if (int32Obj.get().value_or("") != "-100" || - int32Obj.get("") != "-100") { - cerr << "Integer object fail to parse as string" << std::endl; - return TestFail; - } - - if (int32Obj.get().value_or(0.0) != -100.0 || - int32Obj.get(0.0) != -100.0) { - cerr << "Integer object fail to parse as double" << std::endl; - return TestFail; - } /* Test uint32_t object */ auto &uint32Obj = (*root)["uint32_t"]; @@ -239,29 +270,8 @@ protected: if (testObjectType(uint32Obj, "uint32_t", Type::UInt32) != TestPass) return TestFail; - if (uint32Obj.get().value_or(0) != 100 || - uint32Obj.get(0) != 100) { - cerr << "Unsigned integer object fail to parse as integer" << std::endl; + if (testIntegerObject(uint32Obj, "uint32_t", Type::UInt32, 100) != TestPass) return TestFail; - } - - if (uint32Obj.get().value_or("") != "100" || - uint32Obj.get("") != "100") { - cerr << "Unsigned integer object fail to parse as string" << std::endl; - return TestFail; - } - - if (uint32Obj.get().value_or(0.0) != 100.0 || - uint32Obj.get(0.0) != 100.0) { - cerr << "Unsigned integer object fail to parse as double" << std::endl; - return TestFail; - } - - if (uint32Obj.get().value_or(0) != 100 || - uint32Obj.get(0) != 100) { - cerr << "Unsigned integer object parsed as wrong value" << std::endl; - return TestFail; - } /* Test double value */ auto &doubleObj = (*root)["double"]; From patchwork Tue Aug 16 01:54:08 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 17130 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 9893CC3272 for ; Tue, 16 Aug 2022 01:54:36 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 4FED161FCC; Tue, 16 Aug 2022 03:54:36 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1660614876; bh=2yct2hbDhZ7Y5+s+AVYn0T62Z6ztIKO9p9BNEyV/fAA=; 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=3cjkh3uH2zoUJZBdjYcvVS1MYXLZi8gr8uWay7rgwQbxSTO0Hzqw5vf/F/iDfjrXM iz7OqnVVqVLizaIrWIQfzZZKyWdeVBrXM5rnasv1V2ROF1+xrpJ220EcgGrEgAFXbH My3x34C1hF+r6n3dXlLOaJkT7tpxpuQ7JsA+w2+arVNu1mZppvB+PfJKSV3pgilHSI PlxbIDiutnRQ5UFU9IYfsfC91JP0CSEuSavp+7FU1SwG2wNZkOZ+Bidb/tAgks68Gj 6EEntPS9GliMjQ+Z9PW245gmP232QIy0ZMuVpaKoesr7Q7VqA2dqx17an6N3dDK/Gl sL8ye3xyojltA== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 912B161FC7 for ; Tue, 16 Aug 2022 03:54:32 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="dqYJtJDr"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2E7D0496; Tue, 16 Aug 2022 03:54:32 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1660614872; bh=2yct2hbDhZ7Y5+s+AVYn0T62Z6ztIKO9p9BNEyV/fAA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=dqYJtJDrFeTI+YXaKGBp48hEDwwSQ1VgfWDreroBdzPALLNoeNIbUaQgbudujY5ni JXVJuRpjLR+YYDTK6ep0NVn4Wj7MSXjcdlno4mjNs5er3sfrrXiyukbWdnTwOOIBdI cpaCJqvd9M3adDSKOJQEGTDoQlD6EtSS0Hqm/2AE= To: libcamera-devel@lists.libcamera.org Date: Tue, 16 Aug 2022 04:54:08 +0300 Message-Id: <20220816015414.7462-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> References: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 3/9] test: yaml-parser: Test out-of-range checks on integer parsing 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: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Add 16-bit integer parsing tests, including a test to verify the out-of-range checks when parsing 32-bit integers as 16-bit values. That test currently fails. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Paul Elder --- test/yaml-parser.cpp | 77 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 68 insertions(+), 9 deletions(-) diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp index 803e70beb782..28f8cc8822b3 100644 --- a/test/yaml-parser.cpp +++ b/test/yaml-parser.cpp @@ -24,8 +24,10 @@ using namespace std; static const string testYaml = "string: libcamera\n" "double: 3.14159\n" - "uint32_t: 100\n" - "int32_t: -100\n" + "int16_t: -1000\n" + "uint16_t: 1000\n" + "int32_t: -100000\n" + "uint32_t: 100000\n" "size: [1920, 1080]\n" "list:\n" " - James\n" @@ -74,6 +76,8 @@ protected: enum class Type { String, + Int16, + UInt16, Int32, UInt32, Double, @@ -86,8 +90,10 @@ protected: { bool isList = type == Type::List || type == Type::Size; bool isScalar = !isList && type != Type::Dictionary; - bool isInteger = type == Type::Int32 || type == Type::UInt32; - bool isSigned = type == Type::Int32; + bool isInteger16 = type == Type::Int16 || type == Type::UInt16; + bool isInteger32 = type == Type::Int32 || type == Type::UInt32; + bool isInteger = isInteger16 || isInteger32; + bool isSigned = type == Type::Int16 || type == Type::Int32; if ((isScalar && !obj.isValue()) || (!isScalar && obj.isValue())) { std::cerr @@ -118,6 +124,20 @@ protected: return TestFail; } + if (!isInteger16 && obj.get()) { + std::cerr + << "Object " << name << " didn't fail to parse as " + << "int16_t" << std::endl; + return TestFail; + } + + if ((!isInteger16 || isSigned) && obj.get()) { + std::cerr + << "Object " << name << " didn't fail to parse as " + << "uint16_t" << std::endl; + return TestFail; + } + if (!isInteger && obj.get()) { std::cerr << "Object " << name << " didn't fail to parse as " @@ -154,7 +174,8 @@ protected: { uint64_t unsignedValue = static_cast(value); std::string strValue = std::to_string(value); - bool isSigned = type == Type::Int32; + bool isInteger16 = type == Type::Int16 || type == Type::UInt16; + bool isSigned = type == Type::Int16 || type == Type::Int32; /* All integers can be parsed as strings or double. */ @@ -174,6 +195,26 @@ protected: return TestFail; } + if (isInteger16) { + if (obj.get().value_or(0) != value || + obj.get(0) != value) { + std::cerr + << "Object " << name << " failed to parse as " + << "int16_t" << std::endl; + return TestFail; + } + } + + if (isInteger16 && !isSigned) { + if (obj.get().value_or(0) != unsignedValue || + obj.get(0) != unsignedValue) { + std::cerr + << "Object " << name << " failed to parse as " + << "uint16_t" << std::endl; + return TestFail; + } + } + if (obj.get().value_or(0) != value || obj.get(0) != value) { std::cerr @@ -231,8 +272,8 @@ protected: } std::vector rootElemNames = { - "string", "double", "int32_t", "uint32_t", "size", - "list", "dictionary", "level1", + "string", "double", "int16_t", "uint16_t", "int32_t", + "uint32_t", "size", "list", "dictionary", "level1", }; for (const char *name : rootElemNames) { @@ -255,13 +296,31 @@ protected: return TestFail; } + /* Test int16_t object */ + auto &int16Obj = (*root)["int16_t"]; + + if (testObjectType(int16Obj, "int16_t", Type::Int16) != TestPass) + return TestFail; + + if (testIntegerObject(int16Obj, "int16_t", Type::Int16, -1000) != TestPass) + return TestFail; + + /* Test uint16_t object */ + auto &uint16Obj = (*root)["uint16_t"]; + + if (testObjectType(uint16Obj, "uint16_t", Type::UInt16) != TestPass) + return TestFail; + + if (testIntegerObject(uint16Obj, "uint16_t", Type::UInt16, 1000) != TestPass) + return TestFail; + /* Test int32_t object */ auto &int32Obj = (*root)["int32_t"]; if (testObjectType(int32Obj, "int32_t", Type::Int32) != TestPass) return TestFail; - if (testIntegerObject(int32Obj, "int32_t", Type::Int32, -100) != TestPass) + if (testIntegerObject(int32Obj, "int32_t", Type::Int32, -100000) != TestPass) return TestFail; /* Test uint32_t object */ @@ -270,7 +329,7 @@ protected: if (testObjectType(uint32Obj, "uint32_t", Type::UInt32) != TestPass) return TestFail; - if (testIntegerObject(uint32Obj, "uint32_t", Type::UInt32, 100) != TestPass) + if (testIntegerObject(uint32Obj, "uint32_t", Type::UInt32, 100000) != TestPass) return TestFail; /* Test double value */ From patchwork Tue Aug 16 01:54:09 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 17131 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 76FDFC3275 for ; Tue, 16 Aug 2022 01:54:37 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0801B61FC5; Tue, 16 Aug 2022 03:54:37 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1660614877; bh=IkepI4OpLEdCsj0gxeEvWVugmg3kTJKvXZusdraEcb4=; 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=d143DgPRRDdof+tY9ESvUH/nXkyUHBA98Q8QNdba5lA/afXv9beaoHLrgYwLSXQwT treFykTd+vBue2l+0KBeXWVbA1adOt4cXlufxUgI/SF0GDyKrvWwYUDyVwMi3wrUgx W2wo3/1m+X3peRVIL2kH5b2cSUSPkAB2Nsk/vsbKld7OZ2qYfMHopvdHgglDRPlHBn d2OGil1V6lMR4Uq8jiX42rWgg1HHtMB8ueUjgYJ86EFm67iddLM8M6f56lj6HGxDja paptPNWP14BcjONoJ/iigxTj2oRdLDoD56icd5jse5V7JhvoniudcwgofZ7KcRAPNW IjqgtipCgWy7Q== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id F404861FBD for ; Tue, 16 Aug 2022 03:54:33 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="ae3M7qwl"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 912D251C; Tue, 16 Aug 2022 03:54:33 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1660614873; bh=IkepI4OpLEdCsj0gxeEvWVugmg3kTJKvXZusdraEcb4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ae3M7qwlVOQ1L/hVEqdbFTQ+vyhdPOyJJn0hTZJ/FxxHckiYkPpi8b4ux0kFYzPC+ Z/ECz8vAJGMxJ02cmshvUwcF5hVyeV8FepXE79Ka+MS805b2bsFxQKCMBQoxoRN3RT mCMcF/0D1ssZ5fNnQbNyzXBOs/wxNZyzJBoIFfm0= To: libcamera-devel@lists.libcamera.org Date: Tue, 16 Aug 2022 04:54:09 +0300 Message-Id: <20220816015414.7462-5-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> References: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 4/9] libcamera: yaml_parser: Fix bounds checking for 16-bit YamlObject::get() 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: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The YamlObject::get() function specializations for 16-bit integers cast the return value of strto(u)l() to a 16-bit integer, rendering the bounds checking useless. Fix them. Fixes: c7d260c03abd ("libcamera: yaml_parser: Add get() specializations for 16-bit integers") Signed-off-by: Laurent Pinchart Reviewed-by: Paul Elder Reviewed-by: Jacopo Mondi --- src/libcamera/yaml_parser.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp index 9162e2250ed4..f928b7238a19 100644 --- a/src/libcamera/yaml_parser.cpp +++ b/src/libcamera/yaml_parser.cpp @@ -143,7 +143,7 @@ std::optional YamlObject::get() const char *end; errno = 0; - int16_t value = std::strtol(value_.c_str(), &end, 10); + long value = std::strtol(value_.c_str(), &end, 10); if ('\0' != *end || errno == ERANGE || value < std::numeric_limits::min() || @@ -176,7 +176,7 @@ std::optional YamlObject::get() const char *end; errno = 0; - uint16_t value = std::strtoul(value_.c_str(), &end, 10); + unsigned long value = std::strtoul(value_.c_str(), &end, 10); if ('\0' != *end || errno == ERANGE || value < std::numeric_limits::min() || From patchwork Tue Aug 16 01:54:10 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 17132 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 2CA35C3272 for ; Tue, 16 Aug 2022 01:54:39 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id EBC2E61FD2; Tue, 16 Aug 2022 03:54:38 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1660614878; bh=MnNYIntuyymR7unE1fXXD+PG9BX+3m1CqgAiFujFWp0=; 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=g0ih25sGKpz4xFlkF3LOVhhS0jkd6xcx+h1CYmEc9gD8I2ZW3XYbhfJHM9cOwD2wg IdR+j5OTybHnYRgc9D0P7adwoKRTornTir8xxa5UwRj7Gfea9UfyijI9xF5P7FsmXu 83szzY1uo7cTnNkpdtXtf89GLkw3DSipv0u9c1deLK1cMKv+ZPqmR0l3ITONgF0Uyz MHVjHjalaH8o31abRYflLJZi2+Oh9MCjWXcSNnI7BmWY/kRZVP2iveczbX6418fi4I oOGvH38YkbvvWqTcJh3g/C8Micqe5fNlTWBucYfSe5lUtZlzKftXfyfFOvj4GVY7q7 /bhhjYcfyGOCw== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5DB0E61FC4 for ; Tue, 16 Aug 2022 03:54:35 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="S1OJDx29"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id F1CB9496; Tue, 16 Aug 2022 03:54:34 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1660614875; bh=MnNYIntuyymR7unE1fXXD+PG9BX+3m1CqgAiFujFWp0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=S1OJDx29WJsjxCN2rDZIsbL2yFxiuNzGFaJXfRZDbWFIRp39+Fzq5UPFzNI4ZSHk0 E1v9xmfpYUIs1/+vAwrH2NAPY8iAprMxyBGu9HM4CYahD44bPzOarHSXVw03UMXMpw adTPAEHQXd9swXQYdwqAsZDh9hXmnppRtb0lYLVQ= To: libcamera-devel@lists.libcamera.org Date: Tue, 16 Aug 2022 04:54:10 +0300 Message-Id: <20220816015414.7462-6-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> References: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 5/9] libcamera: yaml_parser: Enable YamlObject::get() for int8_t and uint8_t 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: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The YamlObject::get() function template is implemented for 16-bit and 32-bit integers. Add an 8-bit specialization that will be used in the rkisp1 IPA module, and extend the unit tests accordingly. Signed-off-by: Laurent Pinchart Reviewed-by: Paul Elder Reviewed-by: Jacopo Mondi --- include/libcamera/internal/yaml_parser.h | 4 ++ src/libcamera/yaml_parser.cpp | 59 ++++++++++++++++ test/yaml-parser.cpp | 86 ++++++++++++++++++++---- 3 files changed, 137 insertions(+), 12 deletions(-) diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h index 5ba777d364fa..8ca71df867ab 100644 --- a/include/libcamera/internal/yaml_parser.h +++ b/include/libcamera/internal/yaml_parser.h @@ -166,6 +166,8 @@ public: std::enable_if_t< std::is_same_v || std::is_same_v || + std::is_same_v || + std::is_same_v || std::is_same_v || std::is_same_v || std::is_same_v || @@ -188,6 +190,8 @@ public: std::enable_if_t< std::is_same_v || std::is_same_v || + std::is_same_v || + std::is_same_v || std::is_same_v || std::is_same_v || std::is_same_v || diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp index f928b7238a19..85a52c05e682 100644 --- a/src/libcamera/yaml_parser.cpp +++ b/src/libcamera/yaml_parser.cpp @@ -131,6 +131,61 @@ std::optional YamlObject::get() const return std::nullopt; } +template<> +std::optional YamlObject::get() const +{ + if (type_ != Type::Value) + return std::nullopt; + + if (value_ == "") + return std::nullopt; + + char *end; + + errno = 0; + long value = std::strtol(value_.c_str(), &end, 10); + + if ('\0' != *end || errno == ERANGE || + value < std::numeric_limits::min() || + value > std::numeric_limits::max()) + return std::nullopt; + + return value; +} + +template<> +std::optional YamlObject::get() const +{ + if (type_ != Type::Value) + return std::nullopt; + + if (value_ == "") + return std::nullopt; + + /* + * libyaml parses all scalar values as strings. When a string has + * leading spaces before a minus sign, for example " -10", strtoul + * skips leading spaces, accepts the leading minus sign, and the + * calculated digits are negated as if by unary minus. Rule it out in + * case the user gets a large number when the value is negative. + */ + std::size_t found = value_.find_first_not_of(" \t"); + if (found != std::string::npos && value_[found] == '-') + return std::nullopt; + + char *end; + + errno = 0; + unsigned long value = std::strtoul(value_.c_str(), &end, 10); + + if ('\0' != *end || errno == ERANGE || + value < std::numeric_limits::min() || + value > std::numeric_limits::max()) + return std::nullopt; + + return value; +} + template<> std::optional YamlObject::get() const { @@ -310,6 +365,8 @@ template || std::is_same_v || + std::is_same_v || + std::is_same_v || std::is_same_v || std::is_same_v || std::is_same_v || @@ -336,6 +393,8 @@ std::optional> YamlObject::getList() const template std::optional> YamlObject::getList() const; template std::optional> YamlObject::getList() const; +template std::optional> YamlObject::getList() const; +template std::optional> YamlObject::getList() const; template std::optional> YamlObject::getList() const; template std::optional> YamlObject::getList() const; template std::optional> YamlObject::getList() const; diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp index 28f8cc8822b3..2d92463aed98 100644 --- a/test/yaml-parser.cpp +++ b/test/yaml-parser.cpp @@ -24,6 +24,8 @@ using namespace std; static const string testYaml = "string: libcamera\n" "double: 3.14159\n" + "int8_t: -100\n" + "uint8_t: 100\n" "int16_t: -1000\n" "uint16_t: 1000\n" "int32_t: -100000\n" @@ -76,6 +78,8 @@ protected: enum class Type { String, + Int8, + UInt8, Int16, UInt16, Int32, @@ -90,10 +94,13 @@ protected: { bool isList = type == Type::List || type == Type::Size; bool isScalar = !isList && type != Type::Dictionary; + bool isInteger8 = type == Type::Int8 || type == Type::UInt8; bool isInteger16 = type == Type::Int16 || type == Type::UInt16; bool isInteger32 = type == Type::Int32 || type == Type::UInt32; - bool isInteger = isInteger16 || isInteger32; - bool isSigned = type == Type::Int16 || type == Type::Int32; + bool isIntegerUpTo16 = isInteger8 || isInteger16; + bool isIntegerUpTo32 = isIntegerUpTo16 || isInteger32; + bool isSigned = type == Type::Int8 || type == Type::Int16 || + type == Type::Int32; if ((isScalar && !obj.isValue()) || (!isScalar && obj.isValue())) { std::cerr @@ -124,35 +131,49 @@ protected: return TestFail; } - if (!isInteger16 && obj.get()) { + if (!isInteger8 && obj.get()) { + std::cerr + << "Object " << name << " didn't fail to parse as " + << "int8_t" << std::endl; + return TestFail; + } + + if ((!isInteger8 || isSigned) && obj.get()) { + std::cerr + << "Object " << name << " didn't fail to parse as " + << "uint8_t" << std::endl; + return TestFail; + } + + if (!isIntegerUpTo16 && obj.get()) { std::cerr << "Object " << name << " didn't fail to parse as " << "int16_t" << std::endl; return TestFail; } - if ((!isInteger16 || isSigned) && obj.get()) { + if ((!isIntegerUpTo16 || isSigned) && obj.get()) { std::cerr << "Object " << name << " didn't fail to parse as " << "uint16_t" << std::endl; return TestFail; } - if (!isInteger && obj.get()) { + if (!isIntegerUpTo32 && obj.get()) { std::cerr << "Object " << name << " didn't fail to parse as " << "int32_t" << std::endl; return TestFail; } - if ((!isInteger || isSigned) && obj.get()) { + if ((!isIntegerUpTo32 || isSigned) && obj.get()) { std::cerr << "Object " << name << " didn't fail to parse as " << "uint32_t" << std::endl; return TestFail; } - if (!isInteger && type != Type::Double && obj.get()) { + if (!isIntegerUpTo32 && type != Type::Double && obj.get()) { std::cerr << "Object " << name << " didn't fail to parse as " << "double" << std::endl; @@ -174,8 +195,10 @@ protected: { uint64_t unsignedValue = static_cast(value); std::string strValue = std::to_string(value); + bool isInteger8 = type == Type::Int8 || type == Type::UInt8; bool isInteger16 = type == Type::Int16 || type == Type::UInt16; - bool isSigned = type == Type::Int16 || type == Type::Int32; + bool isSigned = type == Type::Int8 || type == Type::Int16 || + type == Type::Int32; /* All integers can be parsed as strings or double. */ @@ -195,7 +218,27 @@ protected: return TestFail; } - if (isInteger16) { + if (isInteger8) { + if (obj.get().value_or(0) != value || + obj.get(0) != value) { + std::cerr + << "Object " << name << " failed to parse as " + << "int8_t" << std::endl; + return TestFail; + } + } + + if (isInteger8 && !isSigned) { + if (obj.get().value_or(0) != unsignedValue || + obj.get(0) != unsignedValue) { + std::cerr + << "Object " << name << " failed to parse as " + << "uint8_t" << std::endl; + return TestFail; + } + } + + if (isInteger8 || isInteger16) { if (obj.get().value_or(0) != value || obj.get(0) != value) { std::cerr @@ -205,7 +248,7 @@ protected: } } - if (isInteger16 && !isSigned) { + if ((isInteger8 || isInteger16) && !isSigned) { if (obj.get().value_or(0) != unsignedValue || obj.get(0) != unsignedValue) { std::cerr @@ -272,8 +315,9 @@ protected: } std::vector rootElemNames = { - "string", "double", "int16_t", "uint16_t", "int32_t", - "uint32_t", "size", "list", "dictionary", "level1", + "string", "double", "int8_t", "uint8_t", "int16_t", + "uint16_t", "int32_t", "uint32_t", "size", "list", + "dictionary", "level1", }; for (const char *name : rootElemNames) { @@ -296,6 +340,24 @@ protected: return TestFail; } + /* Test int8_t object */ + auto &int8Obj = (*root)["int8_t"]; + + if (testObjectType(int8Obj, "int8_t", Type::Int8) != TestPass) + return TestFail; + + if (testIntegerObject(int8Obj, "int8_t", Type::Int8, -100) != TestPass) + return TestFail; + + /* Test uint8_t object */ + auto &uint8Obj = (*root)["uint8_t"]; + + if (testObjectType(uint8Obj, "uint8_t", Type::UInt8) != TestPass) + return TestFail; + + if (testIntegerObject(uint8Obj, "uint8_t", Type::UInt8, 100) != TestPass) + return TestFail; + /* Test int16_t object */ auto &int16Obj = (*root)["int16_t"]; From patchwork Tue Aug 16 01:54:11 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 17133 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 0D8CAC3275 for ; Tue, 16 Aug 2022 01:54:40 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 914CE61FD4; Tue, 16 Aug 2022 03:54:39 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1660614879; bh=JZ9nVNOV0g5kaaCyiDfyY+ChH3PMrrXzF4SuNMylqAc=; 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=PnUqg76Uy9mS/SuqLOiRFUiZD3RHLVdVqf7JFFc7hWJyNu7O4n2L92RTPq2GoKZaK dk6VxaNqCru+kxonmkmIJrYf1LccuCQZym9ADoJAU2AfO0938XWSnCQv564TbXFgl4 RUa1fO20LvJ9MrjBH+tBew31HBmS3KB+JNdfdZYBqbWqDWXQ9UVtnBkhEnEHxhdqCF AE7hfWkQxA3oE9Iu/xRS/EuV7cjm3PEmUNUW6+khBoDfpWEAC08DarCu7AqemWsUGr BUzd9Vovpxmqct0nPIqkNIYh+SMy0lNimxS9aH8HK61ENacjLUs5epdmpqm5JWSVEb DLTV9Y899PyWQ== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B676A61FBD for ; Tue, 16 Aug 2022 03:54:36 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="JYA3IGPe"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 56598496; Tue, 16 Aug 2022 03:54:36 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1660614876; bh=JZ9nVNOV0g5kaaCyiDfyY+ChH3PMrrXzF4SuNMylqAc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=JYA3IGPeetLW6XUKvQwvv+kgjaNiUOcZfwBLnXGEXDWhLHcF3DTKJcQhUhNAN2Feq fYnitQOEUTc7cl/W6RsK3nSWSLLZkvpYvsst+5yAg1clL9nuqPfREGHlhPy7207Klk 5si4vrzvodYqz84tzlr4zlvDzpKMZqweDyEO7yQM= To: libcamera-devel@lists.libcamera.org Date: Tue, 16 Aug 2022 04:54:11 +0300 Message-Id: <20220816015414.7462-7-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> References: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 6/9] libcamera: yaml_parser: De-duplicate common code in YamlObject::get() 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: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The specializations of the YamlObject::get() function template for integer types duplicate code that doesn't directly depend on the template type argument. Move it to separate helper functions to reduce the object size. While at it, rephrase the comment about unsigned integer parsing. Signed-off-by: Laurent Pinchart Reviewed-by: Paul Elder Reviewed-by: Jacopo Mondi --- src/libcamera/yaml_parser.cpp | 160 ++++++++++++++-------------------- 1 file changed, 67 insertions(+), 93 deletions(-) diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp index 85a52c05e682..3d5efdc4419b 100644 --- a/src/libcamera/yaml_parser.cpp +++ b/src/libcamera/yaml_parser.cpp @@ -131,23 +131,65 @@ std::optional YamlObject::get() const return std::nullopt; } +namespace { + +bool parseSignedInteger(const std::string &str, int32_t min, int32_t max, + int32_t *result) +{ + if (str == "") + return false; + + char *end; + + errno = 0; + long value = std::strtol(str.c_str(), &end, 10); + + if ('\0' != *end || errno == ERANGE || value < min || value > max) + return false; + + *result = value; + return true; +} + +bool parseUnsignedInteger(const std::string &str, uint32_t max, uint32_t *result) +{ + if (str == "") + return false; + + /* + * strtoul() accepts strings representing a negative number, in which + * case it negates the converted value. We don't want to silently accept + * negative values and return a large positive number, so check for a + * minus sign (after optional whitespace) and return an error. + */ + std::size_t found = str.find_first_not_of(" \t"); + if (found != std::string::npos && str[found] == '-') + return false; + + char *end; + + errno = 0; + unsigned long value = std::strtoul(str.c_str(), &end, 10); + + if ('\0' != *end || errno == ERANGE || value > max) + return false; + + *result = value; + return true; +} + +} /* namespace */ + template<> std::optional YamlObject::get() const { if (type_ != Type::Value) return std::nullopt; - if (value_ == "") - return std::nullopt; + int32_t value; - char *end; - - errno = 0; - long value = std::strtol(value_.c_str(), &end, 10); - - if ('\0' != *end || errno == ERANGE || - value < std::numeric_limits::min() || - value > std::numeric_limits::max()) + if (!parseSignedInteger(value_, std::numeric_limits::min(), + std::numeric_limits::max(), &value)) return std::nullopt; return value; @@ -159,28 +201,10 @@ std::optional YamlObject::get() const if (type_ != Type::Value) return std::nullopt; - if (value_ == "") - return std::nullopt; + uint32_t value; - /* - * libyaml parses all scalar values as strings. When a string has - * leading spaces before a minus sign, for example " -10", strtoul - * skips leading spaces, accepts the leading minus sign, and the - * calculated digits are negated as if by unary minus. Rule it out in - * case the user gets a large number when the value is negative. - */ - std::size_t found = value_.find_first_not_of(" \t"); - if (found != std::string::npos && value_[found] == '-') - return std::nullopt; - - char *end; - - errno = 0; - unsigned long value = std::strtoul(value_.c_str(), &end, 10); - - if ('\0' != *end || errno == ERANGE || - value < std::numeric_limits::min() || - value > std::numeric_limits::max()) + if (!parseUnsignedInteger(value_, std::numeric_limits::max(), + &value)) return std::nullopt; return value; @@ -192,17 +216,10 @@ std::optional YamlObject::get() const if (type_ != Type::Value) return std::nullopt; - if (value_ == "") - return std::nullopt; + int32_t value; - char *end; - - errno = 0; - long value = std::strtol(value_.c_str(), &end, 10); - - if ('\0' != *end || errno == ERANGE || - value < std::numeric_limits::min() || - value > std::numeric_limits::max()) + if (!parseSignedInteger(value_, std::numeric_limits::min(), + std::numeric_limits::max(), &value)) return std::nullopt; return value; @@ -214,28 +231,10 @@ std::optional YamlObject::get() const if (type_ != Type::Value) return std::nullopt; - if (value_ == "") - return std::nullopt; + uint32_t value; - /* - * libyaml parses all scalar values as strings. When a string has - * leading spaces before a minus sign, for example " -10", strtoul - * skips leading spaces, accepts the leading minus sign, and the - * calculated digits are negated as if by unary minus. Rule it out in - * case the user gets a large number when the value is negative. - */ - std::size_t found = value_.find_first_not_of(" \t"); - if (found != std::string::npos && value_[found] == '-') - return std::nullopt; - - char *end; - - errno = 0; - unsigned long value = std::strtoul(value_.c_str(), &end, 10); - - if ('\0' != *end || errno == ERANGE || - value < std::numeric_limits::min() || - value > std::numeric_limits::max()) + if (!parseUnsignedInteger(value_, std::numeric_limits::max(), + &value)) return std::nullopt; return value; @@ -247,17 +246,10 @@ std::optional YamlObject::get() const if (type_ != Type::Value) return std::nullopt; - if (value_ == "") - return std::nullopt; + int32_t value; - char *end; - - errno = 0; - long value = std::strtol(value_.c_str(), &end, 10); - - if ('\0' != *end || errno == ERANGE || - value < std::numeric_limits::min() || - value > std::numeric_limits::max()) + if (!parseSignedInteger(value_, std::numeric_limits::min(), + std::numeric_limits::max(), &value)) return std::nullopt; return value; @@ -269,28 +261,10 @@ std::optional YamlObject::get() const if (type_ != Type::Value) return std::nullopt; - if (value_ == "") - return std::nullopt; + uint32_t value; - /* - * libyaml parses all scalar values as strings. When a string has - * leading spaces before a minus sign, for example " -10", strtoul - * skips leading spaces, accepts the leading minus sign, and the - * calculated digits are negated as if by unary minus. Rule it out in - * case the user gets a large number when the value is negative. - */ - std::size_t found = value_.find_first_not_of(" \t"); - if (found != std::string::npos && value_[found] == '-') - return std::nullopt; - - char *end; - - errno = 0; - unsigned long value = std::strtoul(value_.c_str(), &end, 10); - - if ('\0' != *end || errno == ERANGE || - value < std::numeric_limits::min() || - value > std::numeric_limits::max()) + if (!parseUnsignedInteger(value_, std::numeric_limits::max(), + &value)) return std::nullopt; return value; From patchwork Tue Aug 16 01:54:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 17134 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 968ABC3272 for ; Tue, 16 Aug 2022 01:54:42 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 41E6861FD8; Tue, 16 Aug 2022 03:54:42 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1660614882; bh=K2EuVIZJxLrW4GpeyRSiCauCGy7HYgzJMlbdxtqHtzo=; 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=HNnhyuiJmTXstIdN9qeoVWSDug9zUvgZVyjsX+V+gE2OHKB+oqOkyOCB8TkAUQyzj +BP7QgPwp6SWcPLmsWyJvGA9yEBzhwwX+fV05INaLJ0zMSjiWsON5KqfodUeL+MmWT 0SE0jLDHejwBsGaYrxeoSl5PpTZnCEwPEa0HJIh4eO+6KnweO5vF6wTdCx+7ga+AvA HT2YJG+NaWi7G8mn6aVKjjCZG23LH4fPXF9zDUQPI2wozdmviJutZLtQvah3bWJwDu l3YTa+/BpG9zcJ44mlSIFjC7WimR3W+ZN2oIp+0KPoYKd7DZEhL+8XPh0vH3f+Gpa9 zguccq9aL001Q== 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 F275D603E3 for ; Tue, 16 Aug 2022 03:54:37 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="c1RukeYK"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 953A9496; Tue, 16 Aug 2022 03:54:37 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1660614877; bh=K2EuVIZJxLrW4GpeyRSiCauCGy7HYgzJMlbdxtqHtzo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=c1RukeYKvt63k6Z5osnWsy2reDV2WjPCjqUxOhnpvrKEwPF5w0Bs8CCnAQ3mG2ivX AQwB1x3je+xkq4HszS/P6OxDu0nSKGxSkYKq7kqQqTUWdBEiGf9+e1jv/dPVqHGKGm 4wwhJM3Rxwsl+c9P+SaUMeHvOxv5ldY8UbdiEDzY= To: libcamera-devel@lists.libcamera.org Date: Tue, 16 Aug 2022 04:54:12 +0300 Message-Id: <20220816015414.7462-8-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> References: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 7/9] ipa: rkisp1: Add enable field for AWB algorithm in IPA context 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: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Florian Sylvestre Add an enable variable in the awb struct in IPASessionConfiguration which indicates if the awb algorithm has been configured. This will allow other algorithms to retrieve this information. Signed-off-by: Florian Sylvestre Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart Reviewed-by: Umang Jain Reviewed-by: Paul Elder --- src/ipa/rkisp1/algorithms/awb.cpp | 2 ++ src/ipa/rkisp1/ipa_context.cpp | 3 +++ src/ipa/rkisp1/ipa_context.h | 1 + 3 files changed, 6 insertions(+) diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index 9f00364d12b1..d1328f011081 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -48,6 +48,8 @@ int Awb::configure(IPAContext &context, context.configuration.awb.measureWindow.h_size = 3 * configInfo.outputSize.width / 4; context.configuration.awb.measureWindow.v_size = 3 * configInfo.outputSize.height / 4; + context.configuration.awb.enabled = true; + return 0; } diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp index ef8bb8e931c8..23a63f8c6e25 100644 --- a/src/ipa/rkisp1/ipa_context.cpp +++ b/src/ipa/rkisp1/ipa_context.cpp @@ -87,6 +87,9 @@ namespace libcamera::ipa::rkisp1 { * * \var IPASessionConfiguration::awb.measureWindow * \brief AWB measure window + * + * \var IPASessionConfiguration::awb.enabled + * \brief Indicates if the AWB hardware is enabled to apply colour gains */ /** diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index 2bdb6a81d7c9..7f7b3e4d88fa 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -29,6 +29,7 @@ struct IPASessionConfiguration { struct { struct rkisp1_cif_isp_window measureWindow; + bool enabled; } awb; struct { From patchwork Tue Aug 16 01:54:13 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 17135 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 2D76DC3275 for ; Tue, 16 Aug 2022 01:54:43 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DF7FC61FD0; Tue, 16 Aug 2022 03:54:42 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1660614882; bh=sDaSbu1INieInB7Qjnm73vNt08k2g+vxXl857Mh5A0M=; 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=dJFpEEx5rEgJM9Q4H3IUjGZaswc4oYX7Xk/jVd8xZiWGBJKOJF4LnNQg/GuM7oLKy 2sDCfli7VwHvVlTSoLuoutBSZdRQqXSAhymhBqfsqzJ25kTOoPUNA17riSu8DyHi1X YO3qU4CoWKqmDPpZuCft8cIiZ8UFC7Itzq97McJ6xDPB8IepbpUA8iHQHvP+8aMpkv I1UKs1UkVlQ0EHOrQkzut70fA1ro2RL3LljxmzOHatDJSjDm01sfmxnFX9MQIfiBIE hIxSdqhyUPS1B3Fq/FJpwZOnQ2lYxPVObLNHxA6kaWo08Q1VpOiwIMQQOy6u5cScA9 mzeSI+Uw5fzDA== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7783661FC7 for ; Tue, 16 Aug 2022 03:54:39 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Da9HOl7R"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 12A4D496; Tue, 16 Aug 2022 03:54:39 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1660614879; bh=sDaSbu1INieInB7Qjnm73vNt08k2g+vxXl857Mh5A0M=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Da9HOl7RgSoYsXg4Xf8p1A9I9fuurruzRtDxqaFDGW2q4V+zqgX//WNXacCz4XUrp dUrorjagz531x56A6I2jS2QAE8h0D+ZwEfaSMDBm4TaGKK9ZASer65tLxO5qVC6mfn W8T6oxJaLtCFeUBNZTBNwemhNfkr4F1N4ZJp10Dw= To: libcamera-devel@lists.libcamera.org Date: Tue, 16 Aug 2022 04:54:13 +0300 Message-Id: <20220816015414.7462-9-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> References: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 8/9] ipa: rkisp1: Add enable field for LSC algorithm in IPA context 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: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Florian Sylvestre Add an enable variable in the lsc struct in IPASessionConfiguration which indicates if the lsc algorithm has been configured. This will allow other algorithms to retrieve this information. Signed-off-by: Florian Sylvestre Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart Reviewed-by: Umang Jain Reviewed-by: Paul Elder --- Changes since v1: - Improve documentation --- src/ipa/rkisp1/algorithms/lsc.cpp | 10 ++++++++++ src/ipa/rkisp1/algorithms/lsc.h | 1 + src/ipa/rkisp1/ipa_context.cpp | 8 ++++++++ src/ipa/rkisp1/ipa_context.h | 4 ++++ 4 files changed, 23 insertions(+) diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp index 05c8c0dab5c8..da287ac7af75 100644 --- a/src/ipa/rkisp1/algorithms/lsc.cpp +++ b/src/ipa/rkisp1/algorithms/lsc.cpp @@ -119,6 +119,16 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context, return 0; } +/** + * \copydoc libcamera::ipa::Algorithm::configure + */ +int LensShadingCorrection::configure(IPAContext &context, + [[maybe_unused]] const IPACameraSensorInfo &configInfo) +{ + context.configuration.lsc.enabled = initialized_; + return 0; +} + /** * \copydoc libcamera::ipa::Algorithm::prepare */ diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h index fdb2ec1dd27d..f68602c005c4 100644 --- a/src/ipa/rkisp1/algorithms/lsc.h +++ b/src/ipa/rkisp1/algorithms/lsc.h @@ -20,6 +20,7 @@ public: ~LensShadingCorrection() = default; int init(IPAContext &context, const YamlObject &tuningData) override; + int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; void prepare(IPAContext &context, rkisp1_params_cfg *params) override; private: diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp index 23a63f8c6e25..1a549c092d73 100644 --- a/src/ipa/rkisp1/ipa_context.cpp +++ b/src/ipa/rkisp1/ipa_context.cpp @@ -92,6 +92,14 @@ namespace libcamera::ipa::rkisp1 { * \brief Indicates if the AWB hardware is enabled to apply colour gains */ +/** + * \var IPASessionConfiguration::lsc + * \brief Lens Shading Correction configuration of the IPA + * + * \var IPASessionConfiguration::lsc.enabled + * \brief Indicates if the LSC hardware is enabled + */ + /** * \var IPASessionConfiguration::sensor * \brief Sensor-specific configuration of the IPA diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index 7f7b3e4d88fa..0cd6aadb83ed 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -32,6 +32,10 @@ struct IPASessionConfiguration { bool enabled; } awb; + struct { + bool enabled; + } lsc; + struct { utils::Duration lineDuration; Size size; From patchwork Tue Aug 16 01:54:14 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 17136 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 26815C3272 for ; Tue, 16 Aug 2022 01:54:44 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B8D8961FCD; Tue, 16 Aug 2022 03:54:43 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1660614883; bh=ey0GQPUNYpQh9XI+sF9qAbTHtublHV1xoXSfKxR/WWo=; 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=Z5+4zobm4kb6KYo61ZRlUv5PAISX9aymXhArLswjRpCmR4wS+jeLRieXOgWetpOrA 5sz6cSYXqTMnUlmPqIqriLd26jr8cKec3ozSFN9uZYnnad/50AltJD/Je33uDPtknP XYw08GvggJUpqUyW3dC25ClZy43FNDfw0V9NBdUwj3D7dK3dafnD+gRqXC9g7fXu3F 7dOi4WFmCNohhG4YY8FXiA5vBVlQItcZCR3BQCO81N0V0iQQxuMKVDPyrr0TF9bh/o 5GEpKIPzdis4UOabBMNny12L3UA5YbjTz6EGbmxmkHAspNUkhG9o+wAOgI5lsSsS6I 5laJG8USl5FCQ== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D9BE261FC7 for ; Tue, 16 Aug 2022 03:54:40 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="tEV3iiXG"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7739051C; Tue, 16 Aug 2022 03:54:40 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1660614880; bh=ey0GQPUNYpQh9XI+sF9qAbTHtublHV1xoXSfKxR/WWo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=tEV3iiXGosTWaegSTplPt0SgYxmjw6dVcw48pNAlHjKRL9yNBsUBU+ygk540vDpBX t/DZFGpKSBZ85PCWF/3wXKkXnS5SD0MO/qm6joc4lNpD9zLvjZK3LZCOaDzzfBk5Fm uN4xoqirPvZgKiVTVAL9U2ZvnfULD8YjMgaxrznU= To: libcamera-devel@lists.libcamera.org Date: Tue, 16 Aug 2022 04:54:14 +0300 Message-Id: <20220816015414.7462-10-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> References: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 9/9] ipa: rkisp1: Add support of Denoise Pre-Filter 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: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Florian Sylvestre The denoise pre-filter algorithm is a bilateral filter which combines a range filter and a domain filter. The denoise pre-filter is applied before demosaicing. Signed-off-by: Florian Sylvestre Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart Reviewed-by: Umang Jain Reviewed-by: Paul Elder --- Changes since v3: - Use YamlObject::getList for domain filter coefficients - Add comment to explain gain handling Changes since v2: - Change the scope of some variables in Dpf::prepare() - Line-wrap coefficients in tuning file --- src/ipa/rkisp1/algorithms/dpf.cpp | 258 ++++++++++++++++++++++++++ src/ipa/rkisp1/algorithms/dpf.h | 36 ++++ src/ipa/rkisp1/algorithms/meson.build | 1 + src/ipa/rkisp1/data/ov5640.yaml | 15 ++ src/ipa/rkisp1/ipa_context.cpp | 11 ++ src/ipa/rkisp1/ipa_context.h | 5 + 6 files changed, 326 insertions(+) create mode 100644 src/ipa/rkisp1/algorithms/dpf.cpp create mode 100644 src/ipa/rkisp1/algorithms/dpf.h diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp new file mode 100644 index 000000000000..c93c8361b5da --- /dev/null +++ b/src/ipa/rkisp1/algorithms/dpf.cpp @@ -0,0 +1,258 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2021-2022, Ideas On Board + * + * dpf.cpp - RkISP1 Denoise Pre-Filter control + */ + +#include "dpf.h" + +#include + +#include + +#include + +#include "linux/rkisp1-config.h" + +/** + * \file dpf.h + */ + +namespace libcamera { + +namespace ipa::rkisp1::algorithms { + +/** + * \class Dpf + * \brief RkISP1 Denoise Pre-Filter control + * + * The denoise pre-filter algorithm is a bilateral filter which combines a + * range filter and a domain filter. The denoise pre-filter is applied before + * demosaicing. + */ + +LOG_DEFINE_CATEGORY(RkISP1Dpf) + +Dpf::Dpf() + : initialized_(false), config_({}), strengthConfig_({}) +{ +} + +/** + * \copydoc libcamera::ipa::Algorithm::init + */ +int Dpf::init([[maybe_unused]] IPAContext &context, + const YamlObject &tuningData) +{ + std::vector values; + + /* + * The domain kernel is configured with a 9x9 kernel for the green + * pixels, and a 13x9 or 9x9 kernel for red and blue pixels. + */ + const YamlObject &dFObject = tuningData["DomainFilter"]; + + /* + * For the green component, we have the 9x9 kernel specified + * as 6 coefficients: + * Y + * ^ + * 4 | 6 5 4 5 6 + * 3 | 5 3 3 5 + * 2 | 5 3 2 3 5 + * 1 | 3 1 1 3 + * 0 - 4 2 0 2 4 + * -1 | 3 1 1 3 + * -2 | 5 3 2 3 5 + * -3 | 5 3 3 5 + * -4 | 6 5 4 5 6 + * +---------|--------> X + * -4....-1 0 1 2 3 4 + */ + values = dFObject["g"].getList().value_or(utils::defopt); + if (values.size() != RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS) { + LOG(RkISP1Dpf, Error) + << "Invalid 'DomainFilter:g': expected " + << RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS + << " elements, got " << values.size(); + return -EINVAL; + } + + std::copy_n(values.begin(), values.size(), + std::begin(config_.g_flt.spatial_coeff)); + + config_.g_flt.gr_enable = true; + config_.g_flt.gb_enable = true; + + /* + * For the red and blue components, we have the 13x9 kernel specified + * as 6 coefficients: + * + * Y + * ^ + * 4 | 6 5 4 3 4 5 6 + * | + * 2 | 5 4 2 1 2 4 5 + * | + * 0 - 5 3 1 0 1 3 5 + * | + * -2 | 5 4 2 1 2 4 5 + * | + * -4 | 6 5 4 3 4 5 6 + * +-------------|------------> X + * -6 -4 -2 0 2 4 6 + * + * For a 9x9 kernel, columns -6 and 6 are dropped, so coefficient + * number 6 is not used. + */ + values = dFObject["rb"].getList().value_or(utils::defopt); + if (values.size() != RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS && + values.size() != RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS - 1) { + LOG(RkISP1Dpf, Error) + << "Invalid 'DomainFilter:rb': expected " + << RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS - 1 + << " or " << RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS + << " elements, got " << values.size(); + return -EINVAL; + } + + config_.rb_flt.fltsize = values.size() == RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS + ? RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9 + : RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_9x9; + + std::copy_n(values.begin(), values.size(), + std::begin(config_.rb_flt.spatial_coeff)); + + config_.rb_flt.r_enable = true; + config_.rb_flt.b_enable = true; + + /* + * The range kernel is configured with a noise level lookup table (NLL) + * which stores a piecewise linear function that characterizes the + * sensor noise profile as a noise level function curve (NLF). + */ + const YamlObject &rFObject = tuningData["NoiseLevelFunction"]; + + std::vector nllValues; + nllValues = rFObject["coeff"].getList().value_or(utils::defopt); + if (nllValues.size() != RKISP1_CIF_ISP_DPF_MAX_NLF_COEFFS) { + LOG(RkISP1Dpf, Error) + << "Invalid 'RangeFilter:coeff': expected " + << RKISP1_CIF_ISP_DPF_MAX_NLF_COEFFS + << " elements, got " << nllValues.size(); + return -EINVAL; + } + + std::copy_n(nllValues.begin(), nllValues.size(), + std::begin(config_.nll.coeff)); + + std::string scaleMode = rFObject["scale-mode"].get(""); + if (scaleMode == "linear") { + config_.nll.scale_mode = RKISP1_CIF_ISP_NLL_SCALE_LINEAR; + } else if (scaleMode == "logarithmic") { + config_.nll.scale_mode = RKISP1_CIF_ISP_NLL_SCALE_LOGARITHMIC; + } else { + LOG(RkISP1Dpf, Error) + << "Invalid 'RangeFilter:scale-mode': expected " + << "'linear' or 'logarithmic' value, got " + << scaleMode; + return -EINVAL; + } + + const YamlObject &fSObject = tuningData["FilterStrength"]; + + strengthConfig_.r = fSObject["r"].get(64); + strengthConfig_.g = fSObject["g"].get(64); + strengthConfig_.b = fSObject["b"].get(64); + + initialized_ = true; + + return 0; +} + +/** + * \copydoc libcamera::ipa::Algorithm::queueRequest + */ +void Dpf::queueRequest(IPAContext &context, + [[maybe_unused]] const uint32_t frame, + const ControlList &controls) +{ + auto &dpf = context.frameContext.dpf; + + const auto &denoise = controls.get(controls::draft::NoiseReductionMode); + if (denoise) { + LOG(RkISP1Dpf, Debug) << "Set denoise to " << *denoise; + + switch (*denoise) { + case controls::draft::NoiseReductionModeOff: + dpf.denoise = false; + dpf.updateParams = true; + break; + case controls::draft::NoiseReductionModeMinimal: + case controls::draft::NoiseReductionModeHighQuality: + case controls::draft::NoiseReductionModeFast: + dpf.denoise = true; + dpf.updateParams = true; + break; + default: + LOG(RkISP1Dpf, Error) + << "Unsupported denoise value " + << *denoise; + } + } +} + +/** + * \copydoc libcamera::ipa::Algorithm::prepare + */ +void Dpf::prepare(IPAContext &context, rkisp1_params_cfg *params) +{ + if (!initialized_) + return; + + auto &dpf = context.frameContext.dpf; + + if (context.frameContext.frameCount == 0) { + params->others.dpf_config = config_; + params->others.dpf_strength_config = strengthConfig_; + + const auto &awb = context.configuration.awb; + const auto &lsc = context.configuration.lsc; + auto &mode = params->others.dpf_config.gain.mode; + + /* + * The DPF needs to take into account the total amount of + * digital gain, which comes from the AWB and LSC modules. The + * DPF hardware can be programmed with a digital gain value + * manually, but can also use the gains supplied by the AWB and + * LSC modules automatically when they are enabled. Use that + * mode of operation as it simplifies control of the DPF. + */ + if (awb.enabled && lsc.enabled) + mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_LSC_GAINS; + else if (awb.enabled) + mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_GAINS; + else if (lsc.enabled) + mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_LSC_GAINS; + else + mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED; + + params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_DPF | + RKISP1_CIF_ISP_MODULE_DPF_STRENGTH; + } + + if (dpf.updateParams) { + params->module_en_update |= RKISP1_CIF_ISP_MODULE_DPF; + if (dpf.denoise) + params->module_ens |= RKISP1_CIF_ISP_MODULE_DPF; + + dpf.updateParams = false; + } +} + +REGISTER_IPA_ALGORITHM(Dpf, "Dpf") + +} /* namespace ipa::rkisp1::algorithms */ + +} /* namespace libcamera */ diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h new file mode 100644 index 000000000000..128ebd5e02e3 --- /dev/null +++ b/src/ipa/rkisp1/algorithms/dpf.h @@ -0,0 +1,36 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2021-2022, Ideas On Board + * + * dpf.h - RkISP1 Denoise Pre-Filter control + */ + +#pragma once + +#include + +#include "algorithm.h" + +namespace libcamera { + +namespace ipa::rkisp1::algorithms { + +class Dpf : public Algorithm +{ +public: + Dpf(); + ~Dpf() = default; + + int init(IPAContext &context, const YamlObject &tuningData) override; + void queueRequest(IPAContext &context, const uint32_t frame, + const ControlList &controls) override; + void prepare(IPAContext &context, rkisp1_params_cfg *params) override; + +private: + bool initialized_; + struct rkisp1_cif_isp_dpf_config config_; + struct rkisp1_cif_isp_dpf_strength_config strengthConfig_; +}; + +} /* namespace ipa::rkisp1::algorithms */ +} /* namespace libcamera */ diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build index e48974b454b5..93a483292753 100644 --- a/src/ipa/rkisp1/algorithms/meson.build +++ b/src/ipa/rkisp1/algorithms/meson.build @@ -6,6 +6,7 @@ rkisp1_ipa_algorithms = files([ 'blc.cpp', 'cproc.cpp', 'dpcc.cpp', + 'dpf.cpp', 'filter.cpp', 'gsl.cpp', 'lsc.cpp', diff --git a/src/ipa/rkisp1/data/ov5640.yaml b/src/ipa/rkisp1/data/ov5640.yaml index 45d4bb77f8ca..3dc369ac3681 100644 --- a/src/ipa/rkisp1/data/ov5640.yaml +++ b/src/ipa/rkisp1/data/ov5640.yaml @@ -156,5 +156,20 @@ algorithms: rnd-offsets: green: 2 red-blue: 2 + - Dpf: + DomainFilter: + g: [ 16, 16, 16, 16, 16, 16 ] + rb: [ 16, 16, 16, 16, 16, 16 ] + NoiseLevelFunction: + coeff: [ + 1023, 1023, 1023, 1023, 1023, 1023, 1023, 1023, + 1023, 1023, 1023, 1023, 1023, 1023, 1023, 1023, + 1023 + ] + scale-mode: "linear" + FilterStrength: + r: 64 + g: 64 + b: 64 - Filter: ... diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp index 1a549c092d73..17f15fd4e6b5 100644 --- a/src/ipa/rkisp1/ipa_context.cpp +++ b/src/ipa/rkisp1/ipa_context.cpp @@ -164,6 +164,17 @@ namespace libcamera::ipa::rkisp1 { * \brief Indicates if ISP parameters need to be updated */ +/** + * \var IPAFrameContext::dpf + * \brief Context for the Denoise Pre-Filter algorithm + * + * \var IPAFrameContext::dpf.denoise + * \brief Indicates if denoise is activated + * + * \var IPAFrameContext::dpf.updateParams + * \brief Indicates if ISP parameters need to be updated + */ + /** * \var IPAFrameContext::filter * \brief Context for the Filter algorithm diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index 0cd6aadb83ed..3a743ac325da 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -69,6 +69,11 @@ struct IPAFrameContext { bool updateParams; } cproc; + struct { + bool denoise; + bool updateParams; + } dpf; + struct { uint8_t denoise; uint8_t sharpness;