[{"id":3821,"web_url":"https://patchwork.libcamera.org/comment/3821/","msgid":"<20200220202748.GL4998@pendragon.ideasonboard.com>","date":"2020-02-20T20:27:48","subject":"Re: [libcamera-devel] [PATCH v3 2/6] libcamera: ipa_manager: Split\n\tpath handling","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Thu, Feb 20, 2020 at 04:57:00PM +0000, Kieran Bingham wrote:\n> Move the iteration of a colon-separated path to its own function.\n> This prepares for parsing extra path variables.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> src/libcamera/include/ipa_manager.h | 1 +\n> src/libcamera/ipa_manager.cpp | 35 ++++++++++++++++++++++-------\n> 2 files changed, 28 insertions(+), 8 deletions(-)\n> \n> diff --git a/src/libcamera/include/ipa_manager.h b/src/libcamera/include/ipa_manager.h\n> index 126f8babbc8f..94d35d9062e4 100644\n> --- a/src/libcamera/include/ipa_manager.h\n> +++ b/src/libcamera/include/ipa_manager.h\n> @@ -33,6 +33,7 @@ private:\n> \t~IPAManager();\n> \n> \tint addDir(const char *libDir);\n> +\tint addPath(const char *path);\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> index 4ffbdd712ac2..d87f2221b00b 100644\n> --- a/src/libcamera/ipa_manager.cpp\n> +++ b/src/libcamera/ipa_manager.cpp\n> @@ -110,14 +110,7 @@ IPAManager::IPAManager()\n> \t\treturn;\n> \t}\n> \n> -\tfor (const auto &dir : utils::split(modulePaths, \":\")) {\n> -\t\tif (dir.empty())\n> -\t\t\tcontinue;\n> -\n> -\t\tint ret = addDir(dir.c_str());\n> -\t\tif (ret > 0)\n> -\t\t\tipaCount += ret;\n> -\t}\n> +\tipaCount += addPath(modulePaths);\n> \n> \tif (!ipaCount)\n> \t\tLOG(IPAManager, Warning)\n> @@ -197,6 +190,32 @@ int IPAManager::addDir(const char *libDir)\n> \treturn count;\n> }\n> \n> +/**\n> + * \\brief Load IPA modules from a search path\n> + * \\param[in] path The colon-separated list of directories to load IPA modules from\n> + *\n> + * This method tries to create an IPAModule instance for every shared object\n> + * found in the directories listed in \\a path.\n> + *\n> + * \\return Number of modules loaded by this call, or a negative error code\n> + * otherwise\n\nThe function never returns a negative error code. I would drop the part\nafter the comma, and change the function return type and the ipaCount to\nbe unsigned int.\n\nReviewed-by: Laurent Pinchart \n\n> + */\n> +int IPAManager::addPath(const char *path)\n> +{\n> +\tint ipaCount = 0;\n> +\n> +\tfor (const auto &dir : utils::split(path, \":\")) {\n> +\t\tif (dir.empty())\n> +\t\t\tcontinue;\n> +\n> +\t\tint ret = addDir(dir.c_str());\n> +\t\tif (ret > 0)\n> +\t\t\tipaCount += ret;\n> +\t}\n> +\n> +\treturn ipaCount;\n> +}\n> +\n> /**\n> * \\brief Create an IPA interface that matches a given pipeline handler\n> * \\param[in] pipe The pipeline handler that wants a matching IPA interface","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E84CE60434\n\tfor ;\n\tThu, 20 Feb 2020 21:28:07 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 730B1563;\n\tThu, 20 Feb 2020 21:28:07 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1582230487;\n\tbh=cdgb45PZI1cycK1qBH44mxgS/97FCkPvWmBwzwSHrWs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LGe08/KW3hYTocy5158kP+IigqPG8RtpvUzCcv3wv31r7wf4yq2s7fSgft/FJKVcx\n\tqQeUWngix042JI/4Wjrw4cn2EPlZ4BkJTqadIJsoZy6Ivrj3LiK2Lvc1wjjq/entoI\n\tza5p+Rm/7M5gnj0UX3YeM8OaXBdu4uvtOY4R5U5o=","Date":"Thu, 20 Feb 2020 22:27:48 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera devel ","Message-ID":"<20200220202748.GL4998@pendragon.ideasonboard.com>","References":"<20200220165704.23600-1-kieran.bingham@ideasonboard.com>\n\t<20200220165704.23600-3-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200220165704.23600-3-kieran.bingham@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 2/6] libcamera: ipa_manager: Split\n\tpath handling","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Thu, 20 Feb 2020 20:28:08 -0000"}},{"id":3827,"web_url":"https://patchwork.libcamera.org/comment/3827/","msgid":"<793a2cfe-db4e-fffa-ed56-cd3b2f555095@ideasonboard.com>","date":"2020-02-21T11:52:55","subject":"Re: [libcamera-devel] [PATCH v3 2/6] libcamera: ipa_manager: Split\n\tpath handling","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 20/02/2020 20:27, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Thu, Feb 20, 2020 at 04:57:00PM +0000, Kieran Bingham wrote:\n>> Move the iteration of a colon-separated path to its own function.\n>> This prepares for parsing extra path variables.\n>>\n\nIn fact, we no longer parse 'extra' path variables.\n\nAlso - once addDir is changed to not return errors, this functionality\nreally is just simple enough to be inlined and avoid all the extra\ndocumentation and layer required for little gain.\n\nVery likely dropping this patch.\n\n--\n\nKieran\n\n>> Signed-off-by: Kieran Bingham \n>> ---\n>> src/libcamera/include/ipa_manager.h | 1 +\n>> src/libcamera/ipa_manager.cpp | 35 ++++++++++++++++++++++-------\n>> 2 files changed, 28 insertions(+), 8 deletions(-)\n>>\n>> diff --git a/src/libcamera/include/ipa_manager.h b/src/libcamera/include/ipa_manager.h\n>> index 126f8babbc8f..94d35d9062e4 100644\n>> --- a/src/libcamera/include/ipa_manager.h\n>> +++ b/src/libcamera/include/ipa_manager.h\n>> @@ -33,6 +33,7 @@ private:\n>> \t~IPAManager();\n>> \n>> \tint addDir(const char *libDir);\n>> +\tint addPath(const char *path);\n>> };\n>> \n>> } /* namespace libcamera */\n>> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n>> index 4ffbdd712ac2..d87f2221b00b 100644\n>> --- a/src/libcamera/ipa_manager.cpp\n>> +++ b/src/libcamera/ipa_manager.cpp\n>> @@ -110,14 +110,7 @@ IPAManager::IPAManager()\n>> \t\treturn;\n>> \t}\n>> \n>> -\tfor (const auto &dir : utils::split(modulePaths, \":\")) {\n>> -\t\tif (dir.empty())\n>> -\t\t\tcontinue;\n>> -\n>> -\t\tint ret = addDir(dir.c_str());\n>> -\t\tif (ret > 0)\n>> -\t\t\tipaCount += ret;\n>> -\t}\n>> +\tipaCount += addPath(modulePaths);\n>> \n>> \tif (!ipaCount)\n>> \t\tLOG(IPAManager, Warning)\n>> @@ -197,6 +190,32 @@ int IPAManager::addDir(const char *libDir)\n>> \treturn count;\n>> }\n>> \n>> +/**\n>> + * \\brief Load IPA modules from a search path\n>> + * \\param[in] path The colon-separated list of directories to load IPA modules from\n>> + *\n>> + * This method tries to create an IPAModule instance for every shared object\n>> + * found in the directories listed in \\a path.\n>> + *\n>> + * \\return Number of modules loaded by this call, or a negative error code\n>> + * otherwise\n> \n> The function never returns a negative error code. I would drop the part\n> after the comma, and change the function return type and the ipaCount to\n> be unsigned int.\n> \n> Reviewed-by: Laurent Pinchart \n> \n>> + */\n>> +int IPAManager::addPath(const char *path)\n>> +{\n>> +\tint ipaCount = 0;\n>> +\n>> +\tfor (const auto &dir : utils::split(path, \":\")) {\n>> +\t\tif (dir.empty())\n>> +\t\t\tcontinue;\n>> +\n>> +\t\tint ret = addDir(dir.c_str());\n>> +\t\tif (ret > 0)\n>> +\t\t\tipaCount += ret;\n>> +\t}\n>> +\n>> +\treturn ipaCount;\n>> +}\n>> +\n>> /**\n>> * \\brief Create an IPA interface that matches a given pipeline handler\n>> * \\param[in] pipe The pipeline handler that wants a matching IPA interface\n>","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7DE9F60438\n\tfor ;\n\tFri, 21 Feb 2020 12:52:59 +0100 (CET)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D4371563;\n\tFri, 21 Feb 2020 12:52:58 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1582285979;\n\tbh=Q7B+RtwUUIz3j7GLwYCO9/rk2SXi6jl3okkxz+lG2R8=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=BUH8sPBUdYFP2FfNh+Q1soq/fIUoC/haU4g30YKwLoFeAnRb1irFceQVl1vIOsewL\n\tcQcuUb0PJqsChvYMkV8HoATN8YVxTXPwkF9FtpS2SEI/+OPcJq4vLvWchNvGypIKCo\n\tyEnb4TwZeGsEC1Xn7zyjxeYIJE1gTpZuO9/DTBdU=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart ","Cc":"libcamera devel ","References":"<20200220165704.23600-1-kieran.bingham@ideasonboard.com>\n\t<20200220165704.23600-3-kieran.bingham@ideasonboard.com>\n\t<20200220202748.GL4998@pendragon.ideasonboard.com>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<793a2cfe-db4e-fffa-ed56-cd3b2f555095@ideasonboard.com>","Date":"Fri, 21 Feb 2020 11:52:55 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.9.1","MIME-Version":"1.0","In-Reply-To":"<20200220202748.GL4998@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 2/6] libcamera: ipa_manager: Split\n\tpath handling","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 21 Feb 2020 11:52:59 -0000"}}]