[{"id":29014,"web_url":"https://patchwork.libcamera.org/comment/29014/","msgid":"","date":"2024-03-20T12:53:22","subject":"Re: [PATCH v2 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Julien\n\nOn Fri, Mar 08, 2024 at 12:00:56PM +0100, Julien Vuillaumier wrote:\n> To match the enumerated media devices, each pipeline handler registered\n\ns/pipeline handler registered/registered pipeline handler/\n\n> is used in no specific order. It is a limitation when several pipelines\n> can match the devices, and user has to select a specific pipeline.\n>\n> For this purpose, environment variable LIBCAMERA_PIPELINES_MATCH_LIST is\n> created that gives the option to define an ordered list of pipelines\n\ns/that gives/to give/\n\n> to invoke during the match process.\n\nOr just: \"to match on.\"\n\n>\n> LIBCAMERA_PIPELINES_MATCH_LIST=\"[,[,...]]]\"\n>\n> Example:\n> LIBCAMERA_PIPELINES_MATCH_LIST=\"PipelineHandlerRkISP1,SimplePipelineHandler\"\n>\n> Signed-off-by: Julien Vuillaumier \n> ---\n> Documentation/environment_variables.rst | 5 ++\n> include/libcamera/internal/camera_manager.h | 1 +\n> src/libcamera/camera_manager.cpp | 57 ++++++++++++++++-----\n> 3 files changed, 51 insertions(+), 12 deletions(-)\n>\n> diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n> index a9b230bc..ea4da3c9 100644\n> --- a/Documentation/environment_variables.rst\n> +++ b/Documentation/environment_variables.rst\n> @@ -37,6 +37,11 @@ LIBCAMERA_IPA_MODULE_PATH\n>\n> Example value: ``${HOME}/.libcamera/lib:/opt/libcamera/vendor/lib``\n>\n> +LIBCAMERA_PIPELINES_MATCH_LIST\n> + Define an ordered list of pipeline names to be used to match the media devices in the system.\n\nlong line over 80 cols which could eaily be broken in 2 lines\n\n> +\n> + Example value: ``PipelineHandlerRkISP1,SimplePipelineHandler``\n\nThese are the names of the PipelineHandlers classes, which is fine,\nbut maybe for users it is better to use something they can more easily\nreference, like the meson option associated with the pipeline ?\n\nThis will require a map though...\n\n> +\n> LIBCAMERA_RPI_CONFIG_FILE\n> Define a custom configuration file to use in the Raspberry Pi pipeline handler.\n>\n> diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n> index 33ebe069..c57e509a 100644\n> --- a/include/libcamera/internal/camera_manager.h\n> +++ b/include/libcamera/internal/camera_manager.h\n> @@ -45,6 +45,7 @@ private:\n> \tint init();\n> \tvoid createPipelineHandlers();\n> \tvoid cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);\n> +\tvoid pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory);\n\nPlease move this one line up to match the function definition order in\nthe .cpp file\n\n>\n> \t/*\n> \t * This mutex protects\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index 355f3ada..620be1c8 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -99,13 +99,36 @@ int CameraManager::Private::init()\n>\n> void CameraManager::Private::createPipelineHandlers()\n> {\n> -\tCameraManager *const o = LIBCAMERA_O_PTR();\n> -\n> \t/*\n> \t * \\todo Try to read handlers and order from configuration\n> -\t * file and only fallback on all handlers if there is no\n> -\t * configuration file.\n> +\t * file and only fallback on environment variable or all handlers, if\n> +\t * there is no configuration file.\n> +\t */\n> +\n> +\t/*\n> +\t * When a list of preferred pipelines is defined, iterate through the\n> +\t * ordered list to match the devices enumerated.\n\ns/devices enumerated/enumerated devices/\n\n> +\t * Otherwise, device matching is done in no specific order with each\n> +\t * registered pipeline handler.\n> \t */\n\nWhat about moving this comment block in the below if (pipesList)\nbranch ?\n\n> +\tconst char *pipesList =\n> +\t\tutils::secure_getenv(\"LIBCAMERA_PIPELINES_MATCH_LIST\");\n> +\tif (pipesList) {\n> +\t\tfor (const auto &pipeName : utils::split(pipesList, \",\")) {\n> +\t\t\tconst PipelineHandlerFactoryBase *factory;\n> +\t\t\tfactory = PipelineHandlerFactoryBase::getFactoryByName(pipeName);\n> +\t\t\tif (!factory)\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\tLOG(Camera, Debug)\n> +\t\t\t\t<< \"Found listed pipeline handler '\"\n> +\t\t\t\t<< pipeName << \"'\";\n> +\t\t\tpipelineFactoryMatch(factory);\n> +\t\t}\n> +\n> +\t\treturn;\n> +\t}\n> +\n> \tconst std::vector &factories =\n> \t\tPipelineHandlerFactoryBase::factories();\n>\n> @@ -117,15 +140,25 @@ void CameraManager::Private::createPipelineHandlers()\n> \t\t * Try each pipeline handler until it exhaust\n> \t\t * all pipelines it can provide.\n> \t\t */\n> -\t\twhile (1) {\n> -\t\t\tstd::shared_ptr pipe = factory->create(o);\n> -\t\t\tif (!pipe->match(enumerator_.get()))\n> -\t\t\t\tbreak;\n> +\t\tpipelineFactoryMatch(factory);\n> +\t}\n> +}\n>\n> -\t\t\tLOG(Camera, Debug)\n> -\t\t\t\t<< \"Pipeline handler \\\"\" << factory->name()\n> -\t\t\t\t<< \"\\\" matched\";\n> -\t\t}\n> +void CameraManager::Private::pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory)\n> +{\n> +\tCameraManager *const o = LIBCAMERA_O_PTR();\n> +\n> +\t/*\n> +\t * Provide as many matching pipelines as possible\n> +\t */\n\nFits on a single line\n\nAlso, I would\n /* Match all the registed pipeline handlers. */\n\n> +\twhile (1) {\n> +\t\tstd::shared_ptr pipe = factory->create(o);\n> +\t\tif (!pipe->match(enumerator_.get()))\n> +\t\t\tbreak;\n> +\n> +\t\tLOG(Camera, Debug)\n> +\t\t\t<< \"Pipeline handler \\\"\" << factory->name()\n> +\t\t\t<< \"\\\" matched\";\n> \t}\n> }\n\nOverall, I concur this is an useful addition. I know there are ideas\nabout assigning a priority to pipeline handlers at creation time as\ncurrently the only conflict is on the ISI which can be\nmatched both by the imx8-isi pipeline and the simple pipeline.\n\nFor the time being, with the above issues fixed\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n>\n> --\n> 2.34.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A0B79BD160\n\tfor ;\n\tWed, 20 Mar 2024 12:53:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3A78B62822;\n\tWed, 20 Mar 2024 13:53:28 +0100 (CET)","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 AE52C62828\n\tfor ;\n\tWed, 20 Mar 2024 13:53:25 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:cc1e:e404:491f:e6ea])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 24545B1;\n\tWed, 20 Mar 2024 13:52:58 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kH/2f58a\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1710939178;\n\tbh=/zU49vAc8jxXo7amB+n0vrcGRsDhbGjudDNM/5xmJ6k=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kH/2f58aqTy1xN5zc+OywIHRsdIIFoSlmAyXSD89iuzFfULDSN2fD1rtvQoNvh9BH\n\tzHzk4d96M74HQcDPdKPd/0JQc+PfsZpGA0lPhqY3PJ/aMCODYrAJr1lEjFjwkW1xvr\n\t6Qb5IsCG5lltNbz4wRlsjV8K4dchfond4eJB4IsQ=","Date":"Wed, 20 Mar 2024 13:53:22 +0100","From":"Jacopo Mondi ","To":"Julien Vuillaumier ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","Message-ID":"","References":"<20240308110056.453320-1-julien.vuillaumier@nxp.com>\n\t<20240308110056.453320-3-julien.vuillaumier@nxp.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240308110056.453320-3-julien.vuillaumier@nxp.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29032,"web_url":"https://patchwork.libcamera.org/comment/29032/","msgid":"","date":"2024-03-21T19:26:06","subject":"Re: [PATCH v2 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","submitter":{"id":190,"url":"https://patchwork.libcamera.org/api/people/190/","name":"Julien Vuillaumier","email":"julien.vuillaumier@nxp.com"},"content":"Hi Jacopo,\n\nYour comments will be integrated in v3.\n\nBut I have a couple of questions to confirm I have the correct \nunderstanding.\n\nThank you,\nJulien\n\nOn 20/03/2024 13:53, Jacopo Mondi wrote:\n> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button\n> \n> \n> Hi Julien\n> \n> On Fri, Mar 08, 2024 at 12:00:56PM +0100, Julien Vuillaumier wrote:\n>> To match the enumerated media devices, each pipeline handler registered\n> \n> s/pipeline handler registered/registered pipeline handler/\n> \n>> is used in no specific order. It is a limitation when several pipelines\n>> can match the devices, and user has to select a specific pipeline.\n>>\n>> For this purpose, environment variable LIBCAMERA_PIPELINES_MATCH_LIST is\n>> created that gives the option to define an ordered list of pipelines\n> \n> s/that gives/to give/\n> \n>> to invoke during the match process.\n> \n> Or just: \"to match on.\"\n> \n>>\n>> LIBCAMERA_PIPELINES_MATCH_LIST=\"[,[,...]]]\"\n>>\n>> Example:\n>> LIBCAMERA_PIPELINES_MATCH_LIST=\"PipelineHandlerRkISP1,SimplePipelineHandler\"\n>>\n>> Signed-off-by: Julien Vuillaumier \n>> ---\n>> Documentation/environment_variables.rst | 5 ++\n>> include/libcamera/internal/camera_manager.h | 1 +\n>> src/libcamera/camera_manager.cpp | 57 ++++++++++++++++-----\n>> 3 files changed, 51 insertions(+), 12 deletions(-)\n>>\n>> diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n>> index a9b230bc..ea4da3c9 100644\n>> --- a/Documentation/environment_variables.rst\n>> +++ b/Documentation/environment_variables.rst\n>> @@ -37,6 +37,11 @@ LIBCAMERA_IPA_MODULE_PATH\n>>\n>> Example value: ``${HOME}/.libcamera/lib:/opt/libcamera/vendor/lib``\n>>\n>> +LIBCAMERA_PIPELINES_MATCH_LIST\n>> + Define an ordered list of pipeline names to be used to match the media devices in the system.\n> \n> long line over 80 cols which could eaily be broken in 2 lines\n> \n>> +\n>> + Example value: ``PipelineHandlerRkISP1,SimplePipelineHandler``\n> \n> These are the names of the PipelineHandlers classes, which is fine,\n> but maybe for users it is better to use something they can more easily\n> reference, like the meson option associated with the pipeline ?\n> \n> This will require a map though...\n\nAs of today the pipeline name, defined as PipelineHandler class name, is \npresented to the user (log=DEBUG), during the match procedure:\n\"Found registered pipeline handler \"\n\"Pipeline handler matched\"\n\nThus, reusing that same name to define the ordered list of pipelines for \nthe match seemed fairly consistent.\n\nBut agreed, PipelineHandler names could be simpler. Currently, apart \nfrom those logs during match, pipeline name looks to be used only in an \nipa interface test.\nIf deemed useful, we could change the current names of each \nPipelineHandler to replace the class name, as you suggested, by the \nmeson option name associated with that pipeline.\n\nA simple way of doing that would be adding to the existing \nREGISTER_PIPELINE_HANDLER(handler) a parameter 'name' to be passed to \nPipelineHandlerFactory constructor, instead of the stringified class name.\nRegistration of each existing pipelines would need to be updated \naccordingly.\n\nDo you recommend changing PipelineHandler name used in the context of \nthat change?\nIn that case, does the above proposal (add a short name to the register \nmacro) makes sense?\n\n>> +\n>> LIBCAMERA_RPI_CONFIG_FILE\n>> Define a custom configuration file to use in the Raspberry Pi pipeline handler.\n>>\n>> diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n>> index 33ebe069..c57e509a 100644\n>> --- a/include/libcamera/internal/camera_manager.h\n>> +++ b/include/libcamera/internal/camera_manager.h\n>> @@ -45,6 +45,7 @@ private:\n>> int init();\n>> void createPipelineHandlers();\n>> void cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);\n>> + void pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory);\n> \n> Please move this one line up to match the function definition order in\n> the .cpp file\n> \n>>\n>> /*\n>> * This mutex protects\n>> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n>> index 355f3ada..620be1c8 100644\n>> --- a/src/libcamera/camera_manager.cpp\n>> +++ b/src/libcamera/camera_manager.cpp\n>> @@ -99,13 +99,36 @@ int CameraManager::Private::init()\n>>\n>> void CameraManager::Private::createPipelineHandlers()\n>> {\n>> - CameraManager *const o = LIBCAMERA_O_PTR();\n>> -\n>> /*\n>> * \\todo Try to read handlers and order from configuration\n>> - * file and only fallback on all handlers if there is no\n>> - * configuration file.\n>> + * file and only fallback on environment variable or all handlers, if\n>> + * there is no configuration file.\n>> + */\n>> +\n>> + /*\n>> + * When a list of preferred pipelines is defined, iterate through the\n>> + * ordered list to match the devices enumerated.\n> \n> s/devices enumerated/enumerated devices/\n> \n>> + * Otherwise, device matching is done in no specific order with each\n>> + * registered pipeline handler.\n>> */\n> \n> What about moving this comment block in the below if (pipesList)\n> branch ?\n> \n>> + const char *pipesList =\n>> + utils::secure_getenv(\"LIBCAMERA_PIPELINES_MATCH_LIST\");\n>> + if (pipesList) {\n>> + for (const auto &pipeName : utils::split(pipesList, \",\")) {\n>> + const PipelineHandlerFactoryBase *factory;\n>> + factory = PipelineHandlerFactoryBase::getFactoryByName(pipeName);\n>> + if (!factory)\n>> + continue;\n>> +\n>> + LOG(Camera, Debug)\n>> + << \"Found listed pipeline handler '\"\n>> + << pipeName << \"'\";\n>> + pipelineFactoryMatch(factory);\n>> + }\n>> +\n>> + return;\n>> + }\n>> +\n>> const std::vector &factories =\n>> PipelineHandlerFactoryBase::factories();\n>>\n>> @@ -117,15 +140,25 @@ void CameraManager::Private::createPipelineHandlers()\n>> * Try each pipeline handler until it exhaust\n>> * all pipelines it can provide.\n>> */\n>> - while (1) {\n>> - std::shared_ptr pipe = factory->create(o);\n>> - if (!pipe->match(enumerator_.get()))\n>> - break;\n>> + pipelineFactoryMatch(factory);\n>> + }\n>> +}\n>>\n>> - LOG(Camera, Debug)\n>> - << \"Pipeline handler \\\"\" << factory->name()\n>> - << \"\\\" matched\";\n>> - }\n>> +void CameraManager::Private::pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory)\n>> +{\n>> + CameraManager *const o = LIBCAMERA_O_PTR();\n>> +\n>> + /*\n>> + * Provide as many matching pipelines as possible\n>> + */\n> \n> Fits on a single line\n> \n> Also, I would\n> /* Match all the registed pipeline handlers. */\n> \n>> + while (1) {\n>> + std::shared_ptr pipe = factory->create(o);\n>> + if (!pipe->match(enumerator_.get()))\n>> + break;\n>> +\n>> + LOG(Camera, Debug)\n>> + << \"Pipeline handler \\\"\" << factory->name()\n>> + << \"\\\" matched\";\n>> }\n>> }\n> \n> Overall, I concur this is an useful addition. I know there are ideas\n> about assigning a priority to pipeline handlers at creation time as\n> currently the only conflict is on the ISI which can be\n> matched both by the imx8-isi pipeline and the simple pipeline.\n\nAs you mentioned, imx8-isi and simple pipelines are concurrent. With \nimx9 family, there is an additional cause of concurrency as platforms \ncan/will use topologies with both ISI + ISP. And depending on the use \ncase, user may want to use either simple, or imx8-isi or the isi+isp \npipeline.\n\nThis change is a basic way to dynamically assign relative priorities to \nthe pipelines. It was suggested in v1 review comments to also have a \nsimilar configuration possible from a global config file.\n\n> \n> For the time being, with the above issues fixed\n> Reviewed-by: Jacopo Mondi \n> \n> Thanks\n> j\n> \n>>\n>> --\n>> 2.34.1\n>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 385D5C3272\n\tfor ;\n\tThu, 21 Mar 2024 19:26:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2809B63055;\n\tThu, 21 Mar 2024 20:26:08 +0100 (CET)","from EUR04-VI1-obe.outbound.protection.outlook.com\n\t(mail-vi1eur04on20600.outbound.protection.outlook.com\n\t[IPv6:2a01:111:f403:2611::600])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8622362827\n\tfor ;\n\tThu, 21 Mar 2024 20:26:05 +0100 (CET)","from AM9PR04MB8147.eurprd04.prod.outlook.com\n\t(2603:10a6:20b:3e0::22)\n\tby AS8PR04MB9127.eurprd04.prod.outlook.com (2603:10a6:20b:44a::15)\n\twith Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7386.29;\n\tThu, 21 Mar 2024 19:26:03 +0000","from AM9PR04MB8147.eurprd04.prod.outlook.com\n\t([fe80::2208:ff47:a8fe:dea6]) by\n\tAM9PR04MB8147.eurprd04.prod.outlook.com\n\t([fe80::2208:ff47:a8fe:dea6%4]) with mapi id 15.20.7386.030;\n\tThu, 21 Mar 2024 19:26:03 +0000"],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"NxCTgiCs\";\n\tdkim-atps=neutral","dkim=none (message not signed)\n\theader.d=none;dmarc=none action=none header.from=nxp.com;"],"ARC-Seal":"i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;\n\tb=jI4rHqfbhSA3caBdHml3RTrdQWiB30Qw5bAEaxa+Iubim7EXK7/73YPfiaGWY56OuFsIRE5dm6/TXWpHSbSihE0L6juxMHiI7D6q1kIubofqKn7JNi8lfBlD0SsucwxZ2FEdMiylomIE+RFpp9KyTFBMbMlf/4DvVDe+5WxoB2rc1zCqSwiH3hPvfA+6yi9agJu7J1qTzKG7tUIBgMuyfgSh0yo7ugUoWeXzNG+KZ25pmB2tMLzP2f0zkaQNtSeoXpsNCzzGyfubrwAPFTT7b/N9dRNuhe+qxApDR8eHnO8hv26Eqx0I/KF7uA3WFwu0eanjKMXxw32oXo7Q9B0fKg==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;\n\ts=arcselector9901;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;\n\tbh=6vb2EDOWRuLf7yQmiemjB5kOpOYukH2ckjXVZWddTbI=;\n\tb=SoTXDlHwqkcJNRSPZnKZO7/MnJbzgkXu886uVbUKxg1jV1TRz6XRmaBFc6LzRLPdblsCEUV7dyzMSkqN4AX155acyM/rqhGgGVmvqtexoPJl2zhEXwmMEdmXyPIl7g9sSTwdU4jEGBqFb0t9tz62rZVnkI+K5F7dp0TnOlfL1N/noGoX8tbLIHUNLYgZVJxpOZeq8nPdBrYFm8HMVtXtJWd5BTa2Gkp2CCMLsu56tkPYPNgmozxUEIKl4CsA2cNiu2wLmbwnFTC3p4XolpTSHYfQf0Y8LHo1L+rypra0+AKseVqpIHfrRv3eP2pIazrIsLA0zT4c/ih4pDnXXxhB7g==","ARC-Authentication-Results":"i=1; mx.microsoft.com 1; spf=pass\n\tsmtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com;\n\tdkim=pass header.d=nxp.com; arc=none","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector2;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;\n\tbh=6vb2EDOWRuLf7yQmiemjB5kOpOYukH2ckjXVZWddTbI=;\n\tb=NxCTgiCsuyu6SStSGU2tsh8vfMtTruO5Hyu5lXxxmXWZVrAp7c1vJ+ogaBNay9FvGMR1fPdl7qLoE53ziBPe6XvfRDm6Epn183NTjfCniY8pZ5Ffgp+jwGM4KntEDfalP0vxk/fuL4Amu5rJ/pTCYrQbgf7g14ME4zwT+TKbZtU=","Message-ID":"","Date":"Thu, 21 Mar 2024 20:26:06 +0100","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","Content-Language":"en-US","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20240308110056.453320-1-julien.vuillaumier@nxp.com>\n\t<20240308110056.453320-3-julien.vuillaumier@nxp.com>\n\t","From":"Julien Vuillaumier ","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-ClientProxiedBy":"AS4P191CA0020.EURP191.PROD.OUTLOOK.COM\n\t(2603:10a6:20b:5d9::8) To AM9PR04MB8147.eurprd04.prod.outlook.com\n\t(2603:10a6:20b:3e0::22)","MIME-Version":"1.0","X-MS-PublicTrafficType":"Email","X-MS-TrafficTypeDiagnostic":"AM9PR04MB8147:EE_|AS8PR04MB9127:EE_","X-MS-Office365-Filtering-Correlation-Id":"90e71be7-0a0d-4f89-efd9-08dc49dcbf46","X-MS-Exchange-SenderADCheck":"1","X-MS-Exchange-AntiSpam-Relay":"0","X-Microsoft-Antispam":"BCL:0;","X-Microsoft-Antispam-Message-Info":"mV0dH0E+1Zpykvio79l1zQ6MBC1C4FTzzrByf+J1vKOWhgzkSHhLNwyVfKTX5tuOmBcy8BDKIlqmpXJl78oZZUo96dE5O8ClBds2my1hdSYi5yYAxDTRUguNlQjsVB7l9EN21r6GB0O0PfgeFtEeWue3+m8uSLCBqMwibtIBaM6Cl3olIuErMLBTGjupylOYuVmtEiq9xPo9kzRgPLv2oqfQALd9nVubCoLsBSKxPSbyAkm+YqfbsJ0y8/PDMGJU4EVNOrwvCgOjCFfjeIe7xT0yy8co9wz1jP4VSad9HNYOzyAfHg4yeqYg34yElzVnJM/a+VnVvnaJtltfaZtBJEUsNdlnQVuGM8JtGlPUsZpNUJdc7mftTH7kS2sxxcZC03E6nSXF6fd18OLlOXXUjVol89QZ5DAj1ogtV46ar1tGmV63ICyP8DszMxnDGrOvHHIDe1RNZ3AhS9eRfH70tVvk7BV3Tp+tpRNR3fx4T/ZlbOmvqNPPhZimdo0q8uzaKNTMQZfbGlfX2d+29cvgqu8vieoGErSXUdxK/6lsqQlQ3MgJMkjg1ZSzlTnBSOzAOSBJbIAkoB/vwMvhyjT5NTUwvc4Niaj9qLhxVfVGe7zVF1j8o2rNfXND8IpHfLTqUXeKDnTqsRL/+xyYqw1JP/2z9PoREVAg36r6+ft/LQc=","X-Forefront-Antispam-Report":"CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;\n\tIPV:NLI; SFV:NSPM; H:AM9PR04MB8147.eurprd04.prod.outlook.com; PTR:;\n\tCAT:NONE; \n\tSFS:(13230031)(1800799015)(376005)(366007); DIR:OUT; SFP:1101; ","X-MS-Exchange-AntiSpam-MessageData-ChunkCount":"1","X-MS-Exchange-AntiSpam-MessageData-0":"=?utf-8?q?Dvo+L07GrC8SdZtUdpYlrhlrC?=\n\t=?utf-8?q?oPrDVpD936sGISsewjufOMXMRG67hFnqPGV743gsNmiFYPtwY72yb6ev?=\n\t=?utf-8?q?XvODQb05IfLaMY+1ZDZljCPFoWtwPw+kUjUNUtAbykek82TCON6LyyeL?=\n\t=?utf-8?q?dPP1Tk6XSdhV0D8RnemQB6GvyYgVu5ZxcqdRF0LrWClotaykK8mMZNsX?=\n\t=?utf-8?q?vWi9PeakJqKJN3SedYRfOKq0L8YELjlinEN2C3Jxy7XRbalylnxeY/Fl?=\n\t=?utf-8?q?p3zvkq2Gem+sV8WqEmz1bqZwV2uNbvdmfV5Sew7TAt1oG1UWaACMogcC?=\n\t=?utf-8?q?gXUdEHs7PcRmPKNNylWrojTSRit0IRMl7vFcBilwWR8GEyfjDwgv8oiu?=\n\t=?utf-8?q?IQvy1ZkYRBhQ/pslDbwKAxd8PYNwOg33sr9HBFdLmw3sOmuRhRu+4ij3?=\n\t=?utf-8?q?Mruf+TPUrwENXuWgbHAJmXy36ZtDRfm9ee0MsOBkTJFpayN1/EaPbGTs?=\n\t=?utf-8?q?TKicEQHzOIeJoKdOCEtZynr6u2mhJpqk3igulq9jMpu4he+YkgU6r5Jv?=\n\t=?utf-8?q?tzIG/gfq9/f5SXyt+gKlEUYQ5QqQWbKnWwsIl35SWgCPT1j9F9VzShp3?=\n\t=?utf-8?q?9Or1ukAdzyplI55IvfrQEpm/uyBfvBK8XJVM12vuBoksKH2Wr0O3UaNU?=\n\t=?utf-8?q?BITa0eC3q7egrzhbM+a3Q0uKxL7ckNq6evn38K/2J84lzviqYddFMNbn?=\n\t=?utf-8?q?8EvOLpzlO4SQpm2Ff8DBBYADTcuejG1F4aO4WQxabHhLq9qn02Rtofbq?=\n\t=?utf-8?q?NCP5L5+cO15IQpTqzwO9CZ3Z0y3b6/ED42/Tp8MD2ZH8K8KP/jfy9ynw?=\n\t=?utf-8?q?AJP/Q/ZsVtrvikM58F4btRyI332Q92rBvbyxLa56yoCDNdEcpJYGK+qG?=\n\t=?utf-8?q?iUayX3DWxe3gPVSRgKDOuUTKyc2nUzZ08WUG4dM3AgBDGnvz53btA/PZ?=\n\t=?utf-8?q?6eV0+oQYm9sjubmt8tXKO4G7Prs8YqO/XT3CQwD/LG277Ln3+99FVVNP?=\n\t=?utf-8?q?N35KxAU6QUJ/wKy9z7awKkaO7V2V2v6rd+mEe22UOG68kcDsLuhvt1Le?=\n\t=?utf-8?q?aOHtOoGsBNim4ox5F/ONzKofZiAX8Nuu9wxYlJBCfQ/fZBNgRRLNWTAY?=\n\t=?utf-8?q?KDhqs4SmjBGeHonl6zTSOWnDBX7H5y9H78x7o8+FTOTxqI9BqTJCjoBV?=\n\t=?utf-8?q?wwgDc18arZ7nxQo+v8RYQOkKDhyymPkn3zVFWZihzpGaiRq7PsxOxfHY?=\n\t=?utf-8?q?vu4scZsm6vFYG49w2r9znmfMKEQ/Wuon58udSEPqzFva8ucWHTWyntcB?=\n\t=?utf-8?q?e/KZ893Zap7owRYYEAPpoTkFAGvHwJj90XOeG3tQGt6CGYPjjZZT5vib?=\n\t=?utf-8?q?MNh2MV7mtQIHfotsoRsHLkg2ZKWGFtx/S7zE7W2MXhkUiZCE5i0S0qTJ?=\n\t=?utf-8?q?5zqv0jkovUAGok85AsR1moVvAsZ3//l6ANx1abehYAl6r2E/C5OKxnD9?=\n\t=?utf-8?q?gZWsQQdd9MFQq/A+OJguq4b0mHgc5TLyOqAoMaRIohnWDTyUTSxfmo/+?=\n\t=?utf-8?q?etuor9F33HRMgcbGpA64EEAXsb6hzW2XWYMFCWcFYxQac0xRmkxNgWqv?=\n\t=?utf-8?q?zFu9AvFqwQlkLXzAExHtBHT2mZVzAWipIAHZGsGhN2PAhMY78m5KG1xB?=\n\t=?utf-8?q?c8ob7VyZidTNlNdOqVjpWXwHee9Vg=3D=3D?=","X-OriginatorOrg":"nxp.com","X-MS-Exchange-CrossTenant-Network-Message-Id":"90e71be7-0a0d-4f89-efd9-08dc49dcbf46","X-MS-Exchange-CrossTenant-AuthSource":"AM9PR04MB8147.eurprd04.prod.outlook.com","X-MS-Exchange-CrossTenant-AuthAs":"Internal","X-MS-Exchange-CrossTenant-OriginalArrivalTime":"21 Mar 2024 19:26:03.1840\n\t(UTC)","X-MS-Exchange-CrossTenant-FromEntityHeader":"Hosted","X-MS-Exchange-CrossTenant-Id":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635","X-MS-Exchange-CrossTenant-MailboxType":"HOSTED","X-MS-Exchange-CrossTenant-UserPrincipalName":"SDelcojbHrmQ1H5H+1wmrZ94/nVGJOL/zyoiBa0+h6ZNR3uX2v8+FRxGazcjdMglTor0f2RxRJ7rYsArrVw3yftQmJYz4kMXx7L6bIyAegQ=","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"AS8PR04MB9127","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29034,"web_url":"https://patchwork.libcamera.org/comment/29034/","msgid":"","date":"2024-03-22T08:50:17","subject":"Re: [PATCH v2 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Julien\n\nOn Thu, Mar 21, 2024 at 08:26:06PM +0100, Julien Vuillaumier wrote:\n> Hi Jacopo,\n>\n> Your comments will be integrated in v3.\n>\n> But I have a couple of questions to confirm I have the correct\n> understanding.\n>\n> Thank you,\n> Julien\n>\n> On 20/03/2024 13:53, Jacopo Mondi wrote:\n> > Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button\n> >\n> >\n> > Hi Julien\n> >\n> > On Fri, Mar 08, 2024 at 12:00:56PM +0100, Julien Vuillaumier wrote:\n> > > To match the enumerated media devices, each pipeline handler registered\n> >\n> > s/pipeline handler registered/registered pipeline handler/\n> >\n> > > is used in no specific order. It is a limitation when several pipelines\n> > > can match the devices, and user has to select a specific pipeline.\n> > >\n> > > For this purpose, environment variable LIBCAMERA_PIPELINES_MATCH_LIST is\n> > > created that gives the option to define an ordered list of pipelines\n> >\n> > s/that gives/to give/\n> >\n> > > to invoke during the match process.\n> >\n> > Or just: \"to match on.\"\n> >\n> > >\n> > > LIBCAMERA_PIPELINES_MATCH_LIST=\"[,[,...]]]\"\n> > >\n> > > Example:\n> > > LIBCAMERA_PIPELINES_MATCH_LIST=\"PipelineHandlerRkISP1,SimplePipelineHandler\"\n> > >\n> > > Signed-off-by: Julien Vuillaumier \n> > > ---\n> > > Documentation/environment_variables.rst | 5 ++\n> > > include/libcamera/internal/camera_manager.h | 1 +\n> > > src/libcamera/camera_manager.cpp | 57 ++++++++++++++++-----\n> > > 3 files changed, 51 insertions(+), 12 deletions(-)\n> > >\n> > > diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n> > > index a9b230bc..ea4da3c9 100644\n> > > --- a/Documentation/environment_variables.rst\n> > > +++ b/Documentation/environment_variables.rst\n> > > @@ -37,6 +37,11 @@ LIBCAMERA_IPA_MODULE_PATH\n> > >\n> > > Example value: ``${HOME}/.libcamera/lib:/opt/libcamera/vendor/lib``\n> > >\n> > > +LIBCAMERA_PIPELINES_MATCH_LIST\n> > > + Define an ordered list of pipeline names to be used to match the media devices in the system.\n> >\n> > long line over 80 cols which could eaily be broken in 2 lines\n> >\n> > > +\n> > > + Example value: ``PipelineHandlerRkISP1,SimplePipelineHandler``\n> >\n> > These are the names of the PipelineHandlers classes, which is fine,\n> > but maybe for users it is better to use something they can more easily\n> > reference, like the meson option associated with the pipeline ?\n> >\n> > This will require a map though...\n>\n> As of today the pipeline name, defined as PipelineHandler class name, is\n> presented to the user (log=DEBUG), during the match procedure:\n> \"Found registered pipeline handler \"\n> \"Pipeline handler matched\"\n>\n> Thus, reusing that same name to define the ordered list of pipelines for the\n> match seemed fairly consistent.\n>\n\nYes, it's consistent indeed\n\n> But agreed, PipelineHandler names could be simpler. Currently, apart from\n\nI'm just a bit concerned about the fact users should either inspect\nthe code or debug logs, while using something like the meson option is\neasier to access ?\n\n> those logs during match, pipeline name looks to be used only in an ipa\n> interface test.\n> If deemed useful, we could change the current names of each PipelineHandler\n> to replace the class name, as you suggested, by the meson option name\n> associated with that pipeline.\n\nNo wait, I was suggesting using the meson option to populate the\nLIBCAMERA_PIPELINES_MATCH_LIST env variable.\n\n>\n> A simple way of doing that would be adding to the existing\n> REGISTER_PIPELINE_HANDLER(handler) a parameter 'name' to be passed to\n> PipelineHandlerFactory constructor, instead of the stringified class name.\n> Registration of each existing pipelines would need to be updated\n> accordingly.\n>\n> Do you recommend changing PipelineHandler name used in the context of that\n> change?\n> In that case, does the above proposal (add a short name to the register\n> macro) makes sense?\n>\n\nMaybe I'm just over-concerned. A way out could be to expand the env\nvariable description to clearly indicate what identifiers to use as\npipeline handler names. Something like:\n\nLIBCAMERA_PIPELINES_MATCH_LIST\n Define an ordered list of pipeline handlers to be used to match the media\n devices in the system. The pipeline handler names used to populate\n the variable are the ones passed to the the\n REGISTER_PIPELINE_HANDLER() macro in the source code.\n\n Example value: ``PipelineHandlerRkISP1,SimplePipelineHandler``\n\nAlternatively what I was proposing was to use the identifiers for\nthe 'pipelines' meson option\n\n option('pipelines',\n type : 'array',\n value : ['auto'],\n choices : [\n 'all',\n 'auto',\n 'imx8-isi',\n 'ipu3',\n 'rkisp1',\n 'rpi/vc4',\n 'simple',\n 'uvcvideo',\n 'vimc'\n ],\n\n\nLIBCAMERA_PIPELINES_MATCH_LIST=``rkisp1,simple,imx8-isi``\n\nand keep a map in CameraManager class, but this indeed requires more\nmaintainership.\n\nNow that I wrote this, isn't it easier to just compile only the\npipeline handler you need ? I presume this doesn't apply to generic\ndistro though\n\n\n> > > +\n> > > LIBCAMERA_RPI_CONFIG_FILE\n> > > Define a custom configuration file to use in the Raspberry Pi pipeline handler.\n> > >\n> > > diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n> > > index 33ebe069..c57e509a 100644\n> > > --- a/include/libcamera/internal/camera_manager.h\n> > > +++ b/include/libcamera/internal/camera_manager.h\n> > > @@ -45,6 +45,7 @@ private:\n> > > int init();\n> > > void createPipelineHandlers();\n> > > void cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > > + void pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory);\n> >\n> > Please move this one line up to match the function definition order in\n> > the .cpp file\n> >\n> > >\n> > > /*\n> > > * This mutex protects\n> > > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > > index 355f3ada..620be1c8 100644\n> > > --- a/src/libcamera/camera_manager.cpp\n> > > +++ b/src/libcamera/camera_manager.cpp\n> > > @@ -99,13 +99,36 @@ int CameraManager::Private::init()\n> > >\n> > > void CameraManager::Private::createPipelineHandlers()\n> > > {\n> > > - CameraManager *const o = LIBCAMERA_O_PTR();\n> > > -\n> > > /*\n> > > * \\todo Try to read handlers and order from configuration\n> > > - * file and only fallback on all handlers if there is no\n> > > - * configuration file.\n> > > + * file and only fallback on environment variable or all handlers, if\n> > > + * there is no configuration file.\n> > > + */\n> > > +\n> > > + /*\n> > > + * When a list of preferred pipelines is defined, iterate through the\n> > > + * ordered list to match the devices enumerated.\n> >\n> > s/devices enumerated/enumerated devices/\n> >\n> > > + * Otherwise, device matching is done in no specific order with each\n> > > + * registered pipeline handler.\n> > > */\n> >\n> > What about moving this comment block in the below if (pipesList)\n> > branch ?\n> >\n> > > + const char *pipesList =\n> > > + utils::secure_getenv(\"LIBCAMERA_PIPELINES_MATCH_LIST\");\n> > > + if (pipesList) {\n> > > + for (const auto &pipeName : utils::split(pipesList, \",\")) {\n> > > + const PipelineHandlerFactoryBase *factory;\n> > > + factory = PipelineHandlerFactoryBase::getFactoryByName(pipeName);\n> > > + if (!factory)\n> > > + continue;\n> > > +\n> > > + LOG(Camera, Debug)\n> > > + << \"Found listed pipeline handler '\"\n> > > + << pipeName << \"'\";\n> > > + pipelineFactoryMatch(factory);\n> > > + }\n> > > +\n> > > + return;\n> > > + }\n> > > +\n> > > const std::vector &factories =\n> > > PipelineHandlerFactoryBase::factories();\n> > >\n> > > @@ -117,15 +140,25 @@ void CameraManager::Private::createPipelineHandlers()\n> > > * Try each pipeline handler until it exhaust\n> > > * all pipelines it can provide.\n> > > */\n> > > - while (1) {\n> > > - std::shared_ptr pipe = factory->create(o);\n> > > - if (!pipe->match(enumerator_.get()))\n> > > - break;\n> > > + pipelineFactoryMatch(factory);\n> > > + }\n> > > +}\n> > >\n> > > - LOG(Camera, Debug)\n> > > - << \"Pipeline handler \\\"\" << factory->name()\n> > > - << \"\\\" matched\";\n> > > - }\n> > > +void CameraManager::Private::pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory)\n> > > +{\n> > > + CameraManager *const o = LIBCAMERA_O_PTR();\n> > > +\n> > > + /*\n> > > + * Provide as many matching pipelines as possible\n> > > + */\n> >\n> > Fits on a single line\n> >\n> > Also, I would\n> > /* Match all the registed pipeline handlers. */\n> >\n> > > + while (1) {\n> > > + std::shared_ptr pipe = factory->create(o);\n> > > + if (!pipe->match(enumerator_.get()))\n> > > + break;\n> > > +\n> > > + LOG(Camera, Debug)\n> > > + << \"Pipeline handler \\\"\" << factory->name()\n> > > + << \"\\\" matched\";\n> > > }\n> > > }\n> >\n> > Overall, I concur this is an useful addition. I know there are ideas\n> > about assigning a priority to pipeline handlers at creation time as\n> > currently the only conflict is on the ISI which can be\n> > matched both by the imx8-isi pipeline and the simple pipeline.\n>\n> As you mentioned, imx8-isi and simple pipelines are concurrent. With imx9\n> family, there is an additional cause of concurrency as platforms can/will\n> use topologies with both ISI + ISP. And depending on the use case, user may\n> want to use either simple, or imx8-isi or the isi+isp pipeline.\n>\n> This change is a basic way to dynamically assign relative priorities to the\n> pipelines. It was suggested in v1 review comments to also have a similar\n> configuration possible from a global config file.\n>\n> >\n> > For the time being, with the above issues fixed\n> > Reviewed-by: Jacopo Mondi \n> >\n> > Thanks\n> > j\n> >\n> > >\n> > > --\n> > > 2.34.1\n> > >\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4C18FBD160\n\tfor ;\n\tFri, 22 Mar 2024 08:50:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 501DA63055;\n\tFri, 22 Mar 2024 09:50:23 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C476962CA3\n\tfor ;\n\tFri, 22 Mar 2024 09:50:20 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:cc1e:e404:491f:e6ea])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DDF79BEB;\n\tFri, 22 Mar 2024 09:49:51 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"E5fMP2DX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1711097392;\n\tbh=fO33+jtdODZrm+Iwh2B3lvg6dlIDH/qCdh3fayA51wc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=E5fMP2DXq/Y/uisyIKYCJAWSzNhbvPtEYmJrA/ItGRaTCXfHE1bVx/+c4WakdGnfb\n\tOmn8oi8v9a6PXDxJ0YgISk8nfuaftcQBsl09Vhlio54vXHAyd+MCve9XMj6OS4Bkdz\n\t0KLY0jpVRKM0n4PPRsrlW7uVumd9tk+TVz/oY8q8=","Date":"Fri, 22 Mar 2024 09:50:17 +0100","From":"Jacopo Mondi ","To":"Julien Vuillaumier ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","Message-ID":"","References":"<20240308110056.453320-1-julien.vuillaumier@nxp.com>\n\t<20240308110056.453320-3-julien.vuillaumier@nxp.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29040,"web_url":"https://patchwork.libcamera.org/comment/29040/","msgid":"<72347124-7275-4305-ab3b-d458abc0e940@nxp.com>","date":"2024-03-22T14:15:50","subject":"Re: [PATCH v2 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","submitter":{"id":190,"url":"https://patchwork.libcamera.org/api/people/190/","name":"Julien Vuillaumier","email":"julien.vuillaumier@nxp.com"},"content":"Hi Jacopo,\n\nOn 22/03/2024 09:50, Jacopo Mondi wrote:\n> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button\n> \n> \n> Hi Julien\n> \n> On Thu, Mar 21, 2024 at 08:26:06PM +0100, Julien Vuillaumier wrote:\n>> Hi Jacopo,\n>>\n>> Your comments will be integrated in v3.\n>>\n>> But I have a couple of questions to confirm I have the correct\n>> understanding.\n>>\n>> Thank you,\n>> Julien\n>>\n>> On 20/03/2024 13:53, Jacopo Mondi wrote:\n>>> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button\n>>>\n>>>\n>>> Hi Julien\n>>>\n>>> On Fri, Mar 08, 2024 at 12:00:56PM +0100, Julien Vuillaumier wrote:\n>>>> To match the enumerated media devices, each pipeline handler registered\n>>>\n>>> s/pipeline handler registered/registered pipeline handler/\n>>>\n>>>> is used in no specific order. It is a limitation when several pipelines\n>>>> can match the devices, and user has to select a specific pipeline.\n>>>>\n>>>> For this purpose, environment variable LIBCAMERA_PIPELINES_MATCH_LIST is\n>>>> created that gives the option to define an ordered list of pipelines\n>>>\n>>> s/that gives/to give/\n>>>\n>>>> to invoke during the match process.\n>>>\n>>> Or just: \"to match on.\"\n>>>\n>>>>\n>>>> LIBCAMERA_PIPELINES_MATCH_LIST=\"[,[,...]]]\"\n>>>>\n>>>> Example:\n>>>> LIBCAMERA_PIPELINES_MATCH_LIST=\"PipelineHandlerRkISP1,SimplePipelineHandler\"\n>>>>\n>>>> Signed-off-by: Julien Vuillaumier \n>>>> ---\n>>>> Documentation/environment_variables.rst | 5 ++\n>>>> include/libcamera/internal/camera_manager.h | 1 +\n>>>> src/libcamera/camera_manager.cpp | 57 ++++++++++++++++-----\n>>>> 3 files changed, 51 insertions(+), 12 deletions(-)\n>>>>\n>>>> diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n>>>> index a9b230bc..ea4da3c9 100644\n>>>> --- a/Documentation/environment_variables.rst\n>>>> +++ b/Documentation/environment_variables.rst\n>>>> @@ -37,6 +37,11 @@ LIBCAMERA_IPA_MODULE_PATH\n>>>>\n>>>> Example value: ``${HOME}/.libcamera/lib:/opt/libcamera/vendor/lib``\n>>>>\n>>>> +LIBCAMERA_PIPELINES_MATCH_LIST\n>>>> + Define an ordered list of pipeline names to be used to match the media devices in the system.\n>>>\n>>> long line over 80 cols which could eaily be broken in 2 lines\n>>>\n>>>> +\n>>>> + Example value: ``PipelineHandlerRkISP1,SimplePipelineHandler``\n>>>\n>>> These are the names of the PipelineHandlers classes, which is fine,\n>>> but maybe for users it is better to use something they can more easily\n>>> reference, like the meson option associated with the pipeline ?\n>>>\n>>> This will require a map though...\n>>\n>> As of today the pipeline name, defined as PipelineHandler class name, is\n>> presented to the user (log=DEBUG), during the match procedure:\n>> \"Found registered pipeline handler \"\n>> \"Pipeline handler matched\"\n>>\n>> Thus, reusing that same name to define the ordered list of pipelines for the\n>> match seemed fairly consistent.\n>>\n> \n> Yes, it's consistent indeed\n> \n>> But agreed, PipelineHandler names could be simpler. Currently, apart from\n> \n> I'm just a bit concerned about the fact users should either inspect\n> the code or debug logs, while using something like the meson option is\n> easier to access ?\n> \n>> those logs during match, pipeline name looks to be used only in an ipa\n>> interface test.\n>> If deemed useful, we could change the current names of each PipelineHandler\n>> to replace the class name, as you suggested, by the meson option name\n>> associated with that pipeline.\n> \n> No wait, I was suggesting using the meson option to populate the\n> LIBCAMERA_PIPELINES_MATCH_LIST env variable.\n> \n>>\n>> A simple way of doing that would be adding to the existing\n>> REGISTER_PIPELINE_HANDLER(handler) a parameter 'name' to be passed to\n>> PipelineHandlerFactory constructor, instead of the stringified class name.\n>> Registration of each existing pipelines would need to be updated\n>> accordingly.\n>>\n>> Do you recommend changing PipelineHandler name used in the context of that\n>> change?\n>> In that case, does the above proposal (add a short name to the register\n>> macro) makes sense?\n>>\n> \n> Maybe I'm just over-concerned. A way out could be to expand the env\n> variable description to clearly indicate what identifiers to use as\n> pipeline handler names. Something like:\n> \n> LIBCAMERA_PIPELINES_MATCH_LIST\n> Define an ordered list of pipeline handlers to be used to match the media\n> devices in the system. The pipeline handler names used to populate\n> the variable are the ones passed to the the\n> REGISTER_PIPELINE_HANDLER() macro in the source code.\n> \n> Example value: ``PipelineHandlerRkISP1,SimplePipelineHandler``\n> \n> Alternatively what I was proposing was to use the identifiers for\n> the 'pipelines' meson option\n> \n> option('pipelines',\n> type : 'array',\n> value : ['auto'],\n> choices : [\n> 'all',\n> 'auto',\n> 'imx8-isi',\n> 'ipu3',\n> 'rkisp1',\n> 'rpi/vc4',\n> 'simple',\n> 'uvcvideo',\n> 'vimc'\n> ],\n> \n> \n> LIBCAMERA_PIPELINES_MATCH_LIST=``rkisp1,simple,imx8-isi``\n> \n> and keep a map in CameraManager class, but this indeed requires more\n> maintainership.\n\nI share the concern that adding a map of nicknames in CameraManager \nclass would add some extra maintenance.\nIn that respect, the option of adding an explicit name parameter to \nREGISTER_PIPELINE_HANDLER() macro in order to assign to each pipeline \nits meson option name, was looking simpler to me.\n\nNow, your proposal to expand the env variable description, documenting \nhow the pipeline handler names are assigned with the register macro, \nlooks perfectly fine to me.\nIf that option makes it acceptable to keep the current pipeline handler \nnames, and nobody objects against the principle, I propose to move \nforward with that for the v3.\n\n> \n> Now that I wrote this, isn't it easier to just compile only the\n> pipeline handler you need ? I presume this doesn't apply to generic\n> distro though\n\nIt does not apply to generic distro, indeed.\n\nBut also when using a custom libcamera package, one may want to change \npipeline handler at runtime because of different use case, camera \n(raw/smart), test purpose... Reconfiguring and rebuilding libcamera with \nthe unique targeted pipeline is not a very practical option here.\n\n> \n> \n>>>> +\n>>>> LIBCAMERA_RPI_CONFIG_FILE\n>>>> Define a custom configuration file to use in the Raspberry Pi pipeline handler.\n>>>>\n>>>> diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n>>>> index 33ebe069..c57e509a 100644\n>>>> --- a/include/libcamera/internal/camera_manager.h\n>>>> +++ b/include/libcamera/internal/camera_manager.h\n>>>> @@ -45,6 +45,7 @@ private:\n>>>> int init();\n>>>> void createPipelineHandlers();\n>>>> void cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);\n>>>> + void pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory);\n>>>\n>>> Please move this one line up to match the function definition order in\n>>> the .cpp file\n>>>\n>>>>\n>>>> /*\n>>>> * This mutex protects\n>>>> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n>>>> index 355f3ada..620be1c8 100644\n>>>> --- a/src/libcamera/camera_manager.cpp\n>>>> +++ b/src/libcamera/camera_manager.cpp\n>>>> @@ -99,13 +99,36 @@ int CameraManager::Private::init()\n>>>>\n>>>> void CameraManager::Private::createPipelineHandlers()\n>>>> {\n>>>> - CameraManager *const o = LIBCAMERA_O_PTR();\n>>>> -\n>>>> /*\n>>>> * \\todo Try to read handlers and order from configuration\n>>>> - * file and only fallback on all handlers if there is no\n>>>> - * configuration file.\n>>>> + * file and only fallback on environment variable or all handlers, if\n>>>> + * there is no configuration file.\n>>>> + */\n>>>> +\n>>>> + /*\n>>>> + * When a list of preferred pipelines is defined, iterate through the\n>>>> + * ordered list to match the devices enumerated.\n>>>\n>>> s/devices enumerated/enumerated devices/\n>>>\n>>>> + * Otherwise, device matching is done in no specific order with each\n>>>> + * registered pipeline handler.\n>>>> */\n>>>\n>>> What about moving this comment block in the below if (pipesList)\n>>> branch ?\n>>>\n>>>> + const char *pipesList =\n>>>> + utils::secure_getenv(\"LIBCAMERA_PIPELINES_MATCH_LIST\");\n>>>> + if (pipesList) {\n>>>> + for (const auto &pipeName : utils::split(pipesList, \",\")) {\n>>>> + const PipelineHandlerFactoryBase *factory;\n>>>> + factory = PipelineHandlerFactoryBase::getFactoryByName(pipeName);\n>>>> + if (!factory)\n>>>> + continue;\n>>>> +\n>>>> + LOG(Camera, Debug)\n>>>> + << \"Found listed pipeline handler '\"\n>>>> + << pipeName << \"'\";\n>>>> + pipelineFactoryMatch(factory);\n>>>> + }\n>>>> +\n>>>> + return;\n>>>> + }\n>>>> +\n>>>> const std::vector &factories =\n>>>> PipelineHandlerFactoryBase::factories();\n>>>>\n>>>> @@ -117,15 +140,25 @@ void CameraManager::Private::createPipelineHandlers()\n>>>> * Try each pipeline handler until it exhaust\n>>>> * all pipelines it can provide.\n>>>> */\n>>>> - while (1) {\n>>>> - std::shared_ptr pipe = factory->create(o);\n>>>> - if (!pipe->match(enumerator_.get()))\n>>>> - break;\n>>>> + pipelineFactoryMatch(factory);\n>>>> + }\n>>>> +}\n>>>>\n>>>> - LOG(Camera, Debug)\n>>>> - << \"Pipeline handler \\\"\" << factory->name()\n>>>> - << \"\\\" matched\";\n>>>> - }\n>>>> +void CameraManager::Private::pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory)\n>>>> +{\n>>>> + CameraManager *const o = LIBCAMERA_O_PTR();\n>>>> +\n>>>> + /*\n>>>> + * Provide as many matching pipelines as possible\n>>>> + */\n>>>\n>>> Fits on a single line\n>>>\n>>> Also, I would\n>>> /* Match all the registed pipeline handlers. */\n>>>\n>>>> + while (1) {\n>>>> + std::shared_ptr pipe = factory->create(o);\n>>>> + if (!pipe->match(enumerator_.get()))\n>>>> + break;\n>>>> +\n>>>> + LOG(Camera, Debug)\n>>>> + << \"Pipeline handler \\\"\" << factory->name()\n>>>> + << \"\\\" matched\";\n>>>> }\n>>>> }\n>>>\n>>> Overall, I concur this is an useful addition. I know there are ideas\n>>> about assigning a priority to pipeline handlers at creation time as\n>>> currently the only conflict is on the ISI which can be\n>>> matched both by the imx8-isi pipeline and the simple pipeline.\n>>\n>> As you mentioned, imx8-isi and simple pipelines are concurrent. With imx9\n>> family, there is an additional cause of concurrency as platforms can/will\n>> use topologies with both ISI + ISP. And depending on the use case, user may\n>> want to use either simple, or imx8-isi or the isi+isp pipeline.\n>>\n>> This change is a basic way to dynamically assign relative priorities to the\n>> pipelines. It was suggested in v1 review comments to also have a similar\n>> configuration possible from a global config file.\n>>\n>>>\n>>> For the time being, with the above issues fixed\n>>> Reviewed-by: Jacopo Mondi \n>>>\n>>> Thanks\n>>> j\n>>>\n>>>>\n>>>> --\n>>>> 2.34.1\n>>>>\n>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id EA8B6BD160\n\tfor ;\n\tFri, 22 Mar 2024 14:15:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 07F6B63055;\n\tFri, 22 Mar 2024 15:15:50 +0100 (CET)","from EUR03-DBA-obe.outbound.protection.outlook.com\n\t(mail-dbaeur03on20601.outbound.protection.outlook.com\n\t[IPv6:2a01:111:f403:260d::601])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 626B161C45\n\tfor ;\n\tFri, 22 Mar 2024 15:15:48 +0100 (CET)","from AM9PR04MB8147.eurprd04.prod.outlook.com\n\t(2603:10a6:20b:3e0::22)\n\tby PAXPR04MB8638.eurprd04.prod.outlook.com (2603:10a6:102:21d::14)\n\twith Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7386.30;\n\tFri, 22 Mar 2024 14:15:46 +0000","from AM9PR04MB8147.eurprd04.prod.outlook.com\n\t([fe80::2208:ff47:a8fe:dea6]) by\n\tAM9PR04MB8147.eurprd04.prod.outlook.com\n\t([fe80::2208:ff47:a8fe:dea6%5]) with mapi id 15.20.7409.023;\n\tFri, 22 Mar 2024 14:15:46 +0000"],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"PDTgf0i8\";\n\tdkim-atps=neutral","dkim=none (message not signed)\n\theader.d=none;dmarc=none action=none header.from=nxp.com;"],"ARC-Seal":"i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;\n\tb=HQXS8z58Tqye83/rPzggUGYv0FlrSHSzXdw80muTtZs5G0Vset0sX5ummVJT34yp3680EhIw0tM+EO4uBZPRb2+UhVRDAwhG9r0tRjNM+YQeaqUeg7fKzxobDszipCIfuJoNIE1Dv6KyY/KJneo0130Qzctcas+R3ER02o4FkhxR6S/NU+VK/qTaU7jDUTuu811cGFjLV30MA15Ifvc7y3g90JPzZOlWGGiry+XdLGzUZwB10sNNsAIhmmXrMhXMdy/P6emMFh7yx3bCw6L53PqfdgTxxq17hce00hxT2EBC93u4TPpPgNhvgTXYgcfuPHeO1YIIiiFLlnZM5yLFSA==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;\n\ts=arcselector9901;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;\n\tbh=YqC2Q5yLlVwChKHWNGVXNA2ResY4clr1K1qVxk28F/8=;\n\tb=ekPiZZ6nypq9Cqq9IImzluM+o0+/8bUuDPAA3UgM/DzR/1WhHEmz6ub0CAdyIf0pR5PUXW5B9ocQW2a/FP7FZfJRuktP+k8TDzl64gNic/VA0M9ofhiweXXCX5oQW3EudHLv0AA964Z4qX9poS21sImgaKxEF/23UPNyXPQ8leXol59Tymt/AcJ6ASTMpIbX2vQHJIXmFGmO5KCKn6xA9RFt4RXYXq+IiD8lNgYgRnczW6Jp6aa5OUmXHpluDApSNIxkkBWOU40uYFiIqeG8OvGGkLxU6YctGHRxxMXBkLeWn9NDQXd7/5tmhvtwI27YxyXREWkWGZzlLpomwXzyfA==","ARC-Authentication-Results":"i=1; mx.microsoft.com 1; spf=pass\n\tsmtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com;\n\tdkim=pass header.d=nxp.com; arc=none","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector2;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;\n\tbh=YqC2Q5yLlVwChKHWNGVXNA2ResY4clr1K1qVxk28F/8=;\n\tb=PDTgf0i8bxJSkpT4lyQeOGQHrk8O8ndwehkQ9gwjR7eXU7kA+NuRzBGchUFjXBKqef1uncE+503oqGXqYcnMRMiehhDjbwrK73IF/pFPdSepml6qUTHS8vliky6ZLPHWE9dnvW8GvGMeSzrDUSrQV9UMBqVlTZl+NYySZqf9Tl8=","Message-ID":"<72347124-7275-4305-ab3b-d458abc0e940@nxp.com>","Date":"Fri, 22 Mar 2024 15:15:50 +0100","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20240308110056.453320-1-julien.vuillaumier@nxp.com>\n\t<20240308110056.453320-3-julien.vuillaumier@nxp.com>\n\t\n\t\n\t","Content-Language":"en-US","From":"Julien Vuillaumier ","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-ClientProxiedBy":"AS4P190CA0024.EURP190.PROD.OUTLOOK.COM\n\t(2603:10a6:20b:5d0::15) To AM9PR04MB8147.eurprd04.prod.outlook.com\n\t(2603:10a6:20b:3e0::22)","MIME-Version":"1.0","X-MS-PublicTrafficType":"Email","X-MS-TrafficTypeDiagnostic":"AM9PR04MB8147:EE_|PAXPR04MB8638:EE_","X-MS-Office365-Filtering-Correlation-Id":"9423993e-3d2d-4fa1-ebe6-08dc4a7a917c","X-MS-Exchange-SenderADCheck":"1","X-MS-Exchange-AntiSpam-Relay":"0","X-Microsoft-Antispam":"BCL:0;","X-Microsoft-Antispam-Message-Info":"JZCe+ag6qiLeyg8EOIpIEdnVDbznMqzk6LzhbH06jE9UjK6ybWIRRHy76Wacy+WwaVcBCF4sWRhOsQluRKUoBlV0sF8bd6XzeSEj+SDd79HjzyH/3MAPIuNigTYuGm/+HsA8ng2j1ha9+gecjXCcgVwqClOVQdLw2+eFvJDS4/8YgOAVELHM2L12unUzT0UNfCzyvhM1ZZM3PVNQ/qDvfszp/jgtp9cQBPZ7PQS+jv8cgOMwfju//i2loUT3CF2aAl84WPuWp3CPuCuXpfO7+9jpHPQV6ScUELl4XwZ8I197DniycFXgv18kETLmFe51Mksk8lB+iMITn4/IsUPciXAoveDA+4yQyvo1eFU279rDQp9YWLmKelkFjRqAZBKn/HpcHlc2zUvD1n9KWIu5VG/OS4IX+bN4qIsQSlFXAizUC/y1fiqdiLkM0KCoJj9LZZI62KCXdi16E3zVPei3WMAvXvEj2RYqOoDkdsYHHBONSz1N/rt212cQSrdZcsKSRCM8s2CvKYrjZNqM+cnr+37glmUBdkKBv62Hp6dzrZdYbGAE3qYHv13U5ZpE4yVlHB9OA8XYri81uK1r7eWR4+vpvS1uSPYaexQWw1Nzzn+82Fg6l+Ul0RVnGyR9n71nBitsVuR++uos3/e8dT90Wsv3gcctEp/ugTmIekn87K8=","X-Forefront-Antispam-Report":"CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;\n\tIPV:NLI; SFV:NSPM; H:AM9PR04MB8147.eurprd04.prod.outlook.com; PTR:;\n\tCAT:NONE; \n\tSFS:(13230031)(376005)(1800799015)(366007); DIR:OUT; SFP:1101; ","X-MS-Exchange-AntiSpam-MessageData-ChunkCount":"1","X-MS-Exchange-AntiSpam-MessageData-0":"=?utf-8?q?mDpwuYkJaN2MYgsiUomg9tUJx?=\n\t=?utf-8?q?GPY8LupC8yxaFILUvofY9puDrhM3tsyNQ6FCZBUvFL1jYL0lkot/JgKe?=\n\t=?utf-8?q?QTMtSO6N5tt4vt+tB57dsWaDjZ9OLpL5jGhrwHzyKBOlOgB7rX6dV7XZ?=\n\t=?utf-8?q?YK3+ENm5Vrw2JRKzHvN95o4QUHtMFxlfETMLsRbM3LOXzE3wVreNcbpw?=\n\t=?utf-8?q?SVBpHYrSOjvxnkDZ1cb0lp4OrlOoJY+Um5RbDtbmvu7XIlBqMncoqdne?=\n\t=?utf-8?q?QFo7lYylR4+622lZmQFRJrMwSNMsXaGZsfVmJYC0/BAP7qBqL9JqAopr?=\n\t=?utf-8?q?huBS7uk/2bGCBXSKWkQvB3Z4/nCJ6zyx9IPp7+f9WTnvpK6Tm34zi9HI?=\n\t=?utf-8?q?owcl1mFqNbuDkomyOzNOKweB6hZSJ0A2BRb+UEUOsHHuNzh3ISnc1PDC?=\n\t=?utf-8?q?9nAGDP2LMhjZ6CDWz+q04+FMBhyXwNfVkEaZJymsVMk4mOi0ClyEOh8q?=\n\t=?utf-8?q?cdJI/jaG7IHuNmJX3AiUtectIfe6Fd6tlhrPHqcw6TL2BGiNU9E/cW/Q?=\n\t=?utf-8?q?vISRMYjTZyvi7CEHtGw0iL2US4QITWIoIqrhUEpmfRqBj5PtxEioRuLc?=\n\t=?utf-8?q?NjU2BWc4OqYspxVUW86ZnzfrYdJIjWsmE1unW7RPmfTmfinxaNoQsjRN?=\n\t=?utf-8?q?WI+0z/aScW49S7YHcBNefo27nSmgUZecNNL2kTUW6Z5aqz4cZB0ono4M?=\n\t=?utf-8?q?LVoZ46/9YSCqX2xxJasEPh3EUuClH4n3z+DVDRitQkPkIPiOuQaBoYN0?=\n\t=?utf-8?q?DQGWRGvm6gO2+glPEuoVGrhh8GqajQR9HBa6zFPvEApnTZhDJuvS9h5B?=\n\t=?utf-8?q?Z3iRCn0b/kJM+TcS8BbRu/cYKRQNoHrzuYzr7w2dg52H9X6BGtB6ASQR?=\n\t=?utf-8?q?j1Zn34/7SipMTRWTWaTrz5eZ6gxquTacfhSj+bju8obQEwbASDV0H3YK?=\n\t=?utf-8?q?luWq1eDxdfs5Me7w+NabANO+yMxUK4bQj1Z4DseQBRBvkqdlhpaNqgip?=\n\t=?utf-8?q?JrGdlNdfggb4eY63rhKmRFhJe2Cg93vZQpxz+hQ9AnWqdOdfb24/G9VP?=\n\t=?utf-8?q?X4jJsYcCzPfzaGk/860ag85wx1m0evwlGycphQ0cSu5r9tyhvLeI8hJX?=\n\t=?utf-8?q?Bs6Xnn3IAws7FBrAU9wpPzZBTB/T3VUOzmvygGS5iERy1qtiALhHo+BL?=\n\t=?utf-8?q?WYKV6T4uZF+UpkeWU5xyf4DOUof+KnPZIlQSXt0m/CMHBTZEIAHRp5dG?=\n\t=?utf-8?q?p5mbut5CcBrLW8neoSd2xiBzoQhuCk+q7ZpEiJlaR8IgvjYEWqY59f89?=\n\t=?utf-8?q?cpgUAOm+JGJLbhlee5FCZeDwzIF/xqq4DZTB5TaTHm0eNe8HALObXPGf?=\n\t=?utf-8?q?7GdFGN3tcxEDGdLEEveKEPFfIhdGgCK1SnccRq/WFcR5zuXcZMiCdpKO?=\n\t=?utf-8?q?XYnkngYM88JeVEyPPTkh+MrVDdJIqLXeOmTmAx0y/W8TeOcqGKPxdGUs?=\n\t=?utf-8?q?DzRot+7m/RBXC0EszIR+mS6j9uP60AmsFUbv/XB9j/hynNsvPEpWy9Sn?=\n\t=?utf-8?q?XSEoSxSyGb+YMsrPiaRf9LC1/cOyH0fkY3DitrtIsH/+lGssA9tzaCnK?=\n\t=?utf-8?q?oEcqUozrGn814bZz8ZNOMwpuFplHGXubY21Aeg/FaU7/2azznUzb6aNX?=\n\t=?utf-8?q?Ws8vwRAPHvQvyWQXZo1/9Ioaag6Fg=3D=3D?=","X-OriginatorOrg":"nxp.com","X-MS-Exchange-CrossTenant-Network-Message-Id":"9423993e-3d2d-4fa1-ebe6-08dc4a7a917c","X-MS-Exchange-CrossTenant-AuthSource":"AM9PR04MB8147.eurprd04.prod.outlook.com","X-MS-Exchange-CrossTenant-AuthAs":"Internal","X-MS-Exchange-CrossTenant-OriginalArrivalTime":"22 Mar 2024 14:15:46.8848\n\t(UTC)","X-MS-Exchange-CrossTenant-FromEntityHeader":"Hosted","X-MS-Exchange-CrossTenant-Id":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635","X-MS-Exchange-CrossTenant-MailboxType":"HOSTED","X-MS-Exchange-CrossTenant-UserPrincipalName":"jPXGbSTXUJKN40B7Y9dNnJl+VJULLSTGjL4yYPIz+L7wjWRS6AtQbDmFpNiDIw1vLlJ/dl18A1VfHZMoznvvSgpICCgn7JnTfHQZgeJMTMA=","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"PAXPR04MB8638","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29114,"web_url":"https://patchwork.libcamera.org/comment/29114/","msgid":"<20240327203550.GT4721@pendragon.ideasonboard.com>","date":"2024-03-27T20:35:50","subject":"Re: [PATCH v2 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Julien,\n\nOn Fri, Mar 22, 2024 at 03:15:50PM +0100, Julien Vuillaumier wrote:\n> On 22/03/2024 09:50, Jacopo Mondi wrote:\n> > On Thu, Mar 21, 2024 at 08:26:06PM +0100, Julien Vuillaumier wrote:\n> >> On 20/03/2024 13:53, Jacopo Mondi wrote:\n> >>> On Fri, Mar 08, 2024 at 12:00:56PM +0100, Julien Vuillaumier wrote:\n> >>>> To match the enumerated media devices, each pipeline handler registered\n> >>>\n> >>> s/pipeline handler registered/registered pipeline handler/\n> >>>\n> >>>> is used in no specific order. It is a limitation when several pipelines\n> >>>> can match the devices, and user has to select a specific pipeline.\n> >>>>\n> >>>> For this purpose, environment variable LIBCAMERA_PIPELINES_MATCH_LIST is\n> >>>> created that gives the option to define an ordered list of pipelines\n> >>>\n> >>> s/that gives/to give/\n> >>>\n> >>>> to invoke during the match process.\n> >>>\n> >>> Or just: \"to match on.\"\n> >>>\n> >>>> LIBCAMERA_PIPELINES_MATCH_LIST=\"[,[,...]]]\"\n> >>>>\n> >>>> Example:\n> >>>> LIBCAMERA_PIPELINES_MATCH_LIST=\"PipelineHandlerRkISP1,SimplePipelineHandler\"\n> >>>>\n> >>>> Signed-off-by: Julien Vuillaumier \n> >>>> ---\n> >>>> Documentation/environment_variables.rst | 5 ++\n> >>>> include/libcamera/internal/camera_manager.h | 1 +\n> >>>> src/libcamera/camera_manager.cpp | 57 ++++++++++++++++-----\n> >>>> 3 files changed, 51 insertions(+), 12 deletions(-)\n> >>>>\n> >>>> diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n> >>>> index a9b230bc..ea4da3c9 100644\n> >>>> --- a/Documentation/environment_variables.rst\n> >>>> +++ b/Documentation/environment_variables.rst\n> >>>> @@ -37,6 +37,11 @@ LIBCAMERA_IPA_MODULE_PATH\n> >>>>\n> >>>> Example value: ``${HOME}/.libcamera/lib:/opt/libcamera/vendor/lib``\n> >>>>\n> >>>> +LIBCAMERA_PIPELINES_MATCH_LIST\n> >>>> + Define an ordered list of pipeline names to be used to match the media devices in the system.\n> >>>\n> >>> long line over 80 cols which could eaily be broken in 2 lines\n> >>>\n> >>>> +\n> >>>> + Example value: ``PipelineHandlerRkISP1,SimplePipelineHandler``\n> >>>\n> >>> These are the names of the PipelineHandlers classes, which is fine,\n> >>> but maybe for users it is better to use something they can more easily\n> >>> reference, like the meson option associated with the pipeline ?\n> >>>\n> >>> This will require a map though...\n> >>\n> >> As of today the pipeline name, defined as PipelineHandler class name, is\n> >> presented to the user (log=DEBUG), during the match procedure:\n> >> \"Found registered pipeline handler \"\n> >> \"Pipeline handler matched\"\n> >>\n> >> Thus, reusing that same name to define the ordered list of pipelines for the\n> >> match seemed fairly consistent.\n> > \n> > Yes, it's consistent indeed\n> > \n> >> But agreed, PipelineHandler names could be simpler. Currently, apart from\n> > \n> > I'm just a bit concerned about the fact users should either inspect\n> > the code or debug logs, while using something like the meson option is\n> > easier to access ?\n> > \n> >> those logs during match, pipeline name looks to be used only in an ipa\n> >> interface test.\n> >> If deemed useful, we could change the current names of each PipelineHandler\n> >> to replace the class name, as you suggested, by the meson option name\n> >> associated with that pipeline.\n> > \n> > No wait, I was suggesting using the meson option to populate the\n> > LIBCAMERA_PIPELINES_MATCH_LIST env variable.\n> > \n> >> A simple way of doing that would be adding to the existing\n> >> REGISTER_PIPELINE_HANDLER(handler) a parameter 'name' to be passed to\n> >> PipelineHandlerFactory constructor, instead of the stringified class name.\n> >> Registration of each existing pipelines would need to be updated\n> >> accordingly.\n> >>\n> >> Do you recommend changing PipelineHandler name used in the context of that\n> >> change?\n> >> In that case, does the above proposal (add a short name to the register\n> >> macro) makes sense?\n> > \n> > Maybe I'm just over-concerned. A way out could be to expand the env\n> > variable description to clearly indicate what identifiers to use as\n> > pipeline handler names. Something like:\n> > \n> > LIBCAMERA_PIPELINES_MATCH_LIST\n> > Define an ordered list of pipeline handlers to be used to match the media\n> > devices in the system. The pipeline handler names used to populate\n> > the variable are the ones passed to the the\n> > REGISTER_PIPELINE_HANDLER() macro in the source code.\n> > \n> > Example value: ``PipelineHandlerRkISP1,SimplePipelineHandler``\n> > \n> > Alternatively what I was proposing was to use the identifiers for\n> > the 'pipelines' meson option\n> > \n> > option('pipelines',\n> > type : 'array',\n> > value : ['auto'],\n> > choices : [\n> > 'all',\n> > 'auto',\n> > 'imx8-isi',\n> > 'ipu3',\n> > 'rkisp1',\n> > 'rpi/vc4',\n> > 'simple',\n> > 'uvcvideo',\n> > 'vimc'\n> > ],\n> > \n> > \n> > LIBCAMERA_PIPELINES_MATCH_LIST=``rkisp1,simple,imx8-isi``\n> > \n> > and keep a map in CameraManager class, but this indeed requires more\n> > maintainership.\n> \n> I share the concern that adding a map of nicknames in CameraManager \n> class would add some extra maintenance.\n> In that respect, the option of adding an explicit name parameter to \n> REGISTER_PIPELINE_HANDLER() macro in order to assign to each pipeline \n> its meson option name, was looking simpler to me.\n\nIf we want shorter names, that's the way to go indeed.\n\n> Now, your proposal to expand the env variable description, documenting \n> how the pipeline handler names are assigned with the register macro, \n> looks perfectly fine to me.\n> If that option makes it acceptable to keep the current pipeline handler \n> names, and nobody objects against the principle, I propose to move \n> forward with that for the v3.\n> \n> > Now that I wrote this, isn't it easier to just compile only the\n> > pipeline handler you need ? I presume this doesn't apply to generic\n> > distro though\n> \n> It does not apply to generic distro, indeed.\n> \n> But also when using a custom libcamera package, one may want to change \n> pipeline handler at runtime because of different use case, camera \n> (raw/smart), test purpose... Reconfiguring and rebuilding libcamera with \n> the unique targeted pipeline is not a very practical option here.\n\nI'm still struggling a bit to understand the use cases (besiding testing\nduring development). Please see below.\n\n> >>>> +\n> >>>> LIBCAMERA_RPI_CONFIG_FILE\n> >>>> Define a custom configuration file to use in the Raspberry Pi pipeline handler.\n> >>>>\n> >>>> diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n> >>>> index 33ebe069..c57e509a 100644\n> >>>> --- a/include/libcamera/internal/camera_manager.h\n> >>>> +++ b/include/libcamera/internal/camera_manager.h\n> >>>> @@ -45,6 +45,7 @@ private:\n> >>>> int init();\n> >>>> void createPipelineHandlers();\n> >>>> void cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);\n> >>>> + void pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory);\n> >>>\n> >>> Please move this one line up to match the function definition order in\n> >>> the .cpp file\n> >>>\n> >>>>\n> >>>> /*\n> >>>> * This mutex protects\n> >>>> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> >>>> index 355f3ada..620be1c8 100644\n> >>>> --- a/src/libcamera/camera_manager.cpp\n> >>>> +++ b/src/libcamera/camera_manager.cpp\n> >>>> @@ -99,13 +99,36 @@ int CameraManager::Private::init()\n> >>>>\n> >>>> void CameraManager::Private::createPipelineHandlers()\n> >>>> {\n> >>>> - CameraManager *const o = LIBCAMERA_O_PTR();\n> >>>> -\n> >>>> /*\n> >>>> * \\todo Try to read handlers and order from configuration\n> >>>> - * file and only fallback on all handlers if there is no\n> >>>> - * configuration file.\n> >>>> + * file and only fallback on environment variable or all handlers, if\n> >>>> + * there is no configuration file.\n> >>>> + */\n> >>>> +\n> >>>> + /*\n> >>>> + * When a list of preferred pipelines is defined, iterate through the\n> >>>> + * ordered list to match the devices enumerated.\n> >>>\n> >>> s/devices enumerated/enumerated devices/\n> >>>\n> >>>> + * Otherwise, device matching is done in no specific order with each\n> >>>> + * registered pipeline handler.\n> >>>> */\n> >>>\n> >>> What about moving this comment block in the below if (pipesList)\n> >>> branch ?\n> >>>\n> >>>> + const char *pipesList =\n> >>>> + utils::secure_getenv(\"LIBCAMERA_PIPELINES_MATCH_LIST\");\n> >>>> + if (pipesList) {\n> >>>> + for (const auto &pipeName : utils::split(pipesList, \",\")) {\n> >>>> + const PipelineHandlerFactoryBase *factory;\n> >>>> + factory = PipelineHandlerFactoryBase::getFactoryByName(pipeName);\n> >>>> + if (!factory)\n> >>>> + continue;\n> >>>> +\n> >>>> + LOG(Camera, Debug)\n> >>>> + << \"Found listed pipeline handler '\"\n> >>>> + << pipeName << \"'\";\n> >>>> + pipelineFactoryMatch(factory);\n> >>>> + }\n> >>>> +\n> >>>> + return;\n> >>>> + }\n> >>>> +\n> >>>> const std::vector &factories =\n> >>>> PipelineHandlerFactoryBase::factories();\n> >>>>\n> >>>> @@ -117,15 +140,25 @@ void CameraManager::Private::createPipelineHandlers()\n> >>>> * Try each pipeline handler until it exhaust\n> >>>> * all pipelines it can provide.\n> >>>> */\n> >>>> - while (1) {\n> >>>> - std::shared_ptr pipe = factory->create(o);\n> >>>> - if (!pipe->match(enumerator_.get()))\n> >>>> - break;\n> >>>> + pipelineFactoryMatch(factory);\n> >>>> + }\n> >>>> +}\n> >>>>\n> >>>> - LOG(Camera, Debug)\n> >>>> - << \"Pipeline handler \\\"\" << factory->name()\n> >>>> - << \"\\\" matched\";\n> >>>> - }\n> >>>> +void CameraManager::Private::pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory)\n> >>>> +{\n> >>>> + CameraManager *const o = LIBCAMERA_O_PTR();\n> >>>> +\n> >>>> + /*\n> >>>> + * Provide as many matching pipelines as possible\n> >>>> + */\n> >>>\n> >>> Fits on a single line\n> >>>\n> >>> Also, I would\n> >>> /* Match all the registed pipeline handlers. */\n> >>>\n> >>>> + while (1) {\n> >>>> + std::shared_ptr pipe = factory->create(o);\n> >>>> + if (!pipe->match(enumerator_.get()))\n> >>>> + break;\n> >>>> +\n> >>>> + LOG(Camera, Debug)\n> >>>> + << \"Pipeline handler \\\"\" << factory->name()\n> >>>> + << \"\\\" matched\";\n> >>>> }\n> >>>> }\n> >>>\n> >>> Overall, I concur this is an useful addition. I know there are ideas\n> >>> about assigning a priority to pipeline handlers at creation time as\n> >>> currently the only conflict is on the ISI which can be\n> >>> matched both by the imx8-isi pipeline and the simple pipeline.\n> >>\n> >> As you mentioned, imx8-isi and simple pipelines are concurrent.\n\nFor this very specific problem, shouldn't we simply drop imx8-isi\nsupport from the simple pipeline handler ? It was added there before we\nhad a dedicated pipeline handler for the imx8-isi. Now that we have one,\nall the use cases that the simple pipeline handler could cover should be\nhandled by the imx8-isi pipeline handler. If some are missing, that's a\nbug, and the imx8-isi pipeline handler should be extended.\n\n> >> With imx9\n> >> family, there is an additional cause of concurrency as platforms can/will\n> >> use topologies with both ISI + ISP. And depending on the use case, user may\n> >> want to use either simple, or imx8-isi or the isi+isp pipeline.\n\nNote that the i.MX8MP also has an ISI and 2 ISP instances. Currently, we\nneed to select what to route the CSIS output to in DT, which is not a\ngood solution, but from a libcamera point of view, it means that only\none of the imx8-isi and rkisp1 pipeline handler can ever match at\nruntime.\n\nThis should be fixed in the kernel, to enable dynamic routing\nconfiguration from userspace. That's no simple work, and I'm not aware\nof anyone actively working on it, but should this be completed at some\npoint, we would need to also improve libcamera to cover all the use\ncases with a single pipeline handler. I don't really see why users would\nwant to select between multiple pipeline handlers.\n\nHow does the situation differ on the i.MX9 ?\n\n> >> This change is a basic way to dynamically assign relative priorities to the\n> >> pipelines. It was suggested in v1 review comments to also have a similar\n> >> configuration possible from a global config file.\n> >>\n> >>> For the time being, with the above issues fixed\n> >>> Reviewed-by: Jacopo Mondi ","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id EB4DFBD160\n\tfor ;\n\tWed, 27 Mar 2024 20:36:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DFF2F63339;\n\tWed, 27 Mar 2024 21:36:00 +0100 (CET)","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 869E661C41\n\tfor ;\n\tWed, 27 Mar 2024 21:35:59 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BCC3C675;\n\tWed, 27 Mar 2024 21:35:26 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"H+BdrR62\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1711571726;\n\tbh=fZHJ7fG24+P6RQ8sTy61Pp8sfrM3Fq+Guv7Jfn+3X4k=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=H+BdrR620dEAbUln8Rcgcj/1N6TlaC5NwVnXpgtitTbZCQESv1bCjk2TP83/lsEhQ\n\t8bERut7rhcOAoAAeLU6O261859AzKIquAUW1tCfXPkOa08/9/95w8AKeUKeeil4L7F\n\tMdHFQzKQoNMSCeAjCBoDVSPqm8ceC/3+n+eKVqVY=","Date":"Wed, 27 Mar 2024 22:35:50 +0200","From":"Laurent Pinchart ","To":"Julien Vuillaumier ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","Message-ID":"<20240327203550.GT4721@pendragon.ideasonboard.com>","References":"<20240308110056.453320-1-julien.vuillaumier@nxp.com>\n\t<20240308110056.453320-3-julien.vuillaumier@nxp.com>\n\t\n\t\n\t\n\t<72347124-7275-4305-ab3b-d458abc0e940@nxp.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<72347124-7275-4305-ab3b-d458abc0e940@nxp.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29125,"web_url":"https://patchwork.libcamera.org/comment/29125/","msgid":"<278f4389-0cc6-462e-8205-c132b4345f26@nxp.com>","date":"2024-03-28T15:54:46","subject":"Re: [PATCH v2 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","submitter":{"id":190,"url":"https://patchwork.libcamera.org/api/people/190/","name":"Julien Vuillaumier","email":"julien.vuillaumier@nxp.com"},"content":"Hi Laurent,\n\nOn 27/03/2024 21:35, Laurent Pinchart wrote:\n> \n> Hi Julien,\n> \n> On Fri, Mar 22, 2024 at 03:15:50PM +0100, Julien Vuillaumier wrote:\n>> On 22/03/2024 09:50, Jacopo Mondi wrote:\n>>> On Thu, Mar 21, 2024 at 08:26:06PM +0100, Julien Vuillaumier wrote:\n>>>> On 20/03/2024 13:53, Jacopo Mondi wrote:\n>>>>> On Fri, Mar 08, 2024 at 12:00:56PM +0100, Julien Vuillaumier wrote:\n>>>>>> To match the enumerated media devices, each pipeline handler registered\n>>>>>\n>>>>> s/pipeline handler registered/registered pipeline handler/\n>>>>>\n>>>>>> is used in no specific order. It is a limitation when several pipelines\n>>>>>> can match the devices, and user has to select a specific pipeline.\n>>>>>>\n>>>>>> For this purpose, environment variable LIBCAMERA_PIPELINES_MATCH_LIST is\n>>>>>> created that gives the option to define an ordered list of pipelines\n>>>>>\n>>>>> s/that gives/to give/\n>>>>>\n>>>>>> to invoke during the match process.\n>>>>>\n>>>>> Or just: \"to match on.\"\n>>>>>\n>>>>>> LIBCAMERA_PIPELINES_MATCH_LIST=\"[,[,...]]]\"\n>>>>>>\n>>>>>> Example:\n>>>>>> LIBCAMERA_PIPELINES_MATCH_LIST=\"PipelineHandlerRkISP1,SimplePipelineHandler\"\n>>>>>>\n>>>>>> Signed-off-by: Julien Vuillaumier \n>>>>>> ---\n>>>>>> Documentation/environment_variables.rst | 5 ++\n>>>>>> include/libcamera/internal/camera_manager.h | 1 +\n>>>>>> src/libcamera/camera_manager.cpp | 57 ++++++++++++++++-----\n>>>>>> 3 files changed, 51 insertions(+), 12 deletions(-)\n>>>>>>\n>>>>>> diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n>>>>>> index a9b230bc..ea4da3c9 100644\n>>>>>> --- a/Documentation/environment_variables.rst\n>>>>>> +++ b/Documentation/environment_variables.rst\n>>>>>> @@ -37,6 +37,11 @@ LIBCAMERA_IPA_MODULE_PATH\n>>>>>>\n>>>>>> Example value: ``${HOME}/.libcamera/lib:/opt/libcamera/vendor/lib``\n>>>>>>\n>>>>>> +LIBCAMERA_PIPELINES_MATCH_LIST\n>>>>>> + Define an ordered list of pipeline names to be used to match the media devices in the system.\n>>>>>\n>>>>> long line over 80 cols which could eaily be broken in 2 lines\n>>>>>\n>>>>>> +\n>>>>>> + Example value: ``PipelineHandlerRkISP1,SimplePipelineHandler``\n>>>>>\n>>>>> These are the names of the PipelineHandlers classes, which is fine,\n>>>>> but maybe for users it is better to use something they can more easily\n>>>>> reference, like the meson option associated with the pipeline ?\n>>>>>\n>>>>> This will require a map though...\n>>>>\n>>>> As of today the pipeline name, defined as PipelineHandler class name, is\n>>>> presented to the user (log=DEBUG), during the match procedure:\n>>>> \"Found registered pipeline handler \"\n>>>> \"Pipeline handler matched\"\n>>>>\n>>>> Thus, reusing that same name to define the ordered list of pipelines for the\n>>>> match seemed fairly consistent.\n>>>\n>>> Yes, it's consistent indeed\n>>>\n>>>> But agreed, PipelineHandler names could be simpler. Currently, apart from\n>>>\n>>> I'm just a bit concerned about the fact users should either inspect\n>>> the code or debug logs, while using something like the meson option is\n>>> easier to access ?\n>>>\n>>>> those logs during match, pipeline name looks to be used only in an ipa\n>>>> interface test.\n>>>> If deemed useful, we could change the current names of each PipelineHandler\n>>>> to replace the class name, as you suggested, by the meson option name\n>>>> associated with that pipeline.\n>>>\n>>> No wait, I was suggesting using the meson option to populate the\n>>> LIBCAMERA_PIPELINES_MATCH_LIST env variable.\n>>>\n>>>> A simple way of doing that would be adding to the existing\n>>>> REGISTER_PIPELINE_HANDLER(handler) a parameter 'name' to be passed to\n>>>> PipelineHandlerFactory constructor, instead of the stringified class name.\n>>>> Registration of each existing pipelines would need to be updated\n>>>> accordingly.\n>>>>\n>>>> Do you recommend changing PipelineHandler name used in the context of that\n>>>> change?\n>>>> In that case, does the above proposal (add a short name to the register\n>>>> macro) makes sense?\n>>>\n>>> Maybe I'm just over-concerned. A way out could be to expand the env\n>>> variable description to clearly indicate what identifiers to use as\n>>> pipeline handler names. Something like:\n>>>\n>>> LIBCAMERA_PIPELINES_MATCH_LIST\n>>> Define an ordered list of pipeline handlers to be used to match the media\n>>> devices in the system. The pipeline handler names used to populate\n>>> the variable are the ones passed to the the\n>>> REGISTER_PIPELINE_HANDLER() macro in the source code.\n>>>\n>>> Example value: ``PipelineHandlerRkISP1,SimplePipelineHandler``\n>>>\n>>> Alternatively what I was proposing was to use the identifiers for\n>>> the 'pipelines' meson option\n>>>\n>>> option('pipelines',\n>>> type : 'array',\n>>> value : ['auto'],\n>>> choices : [\n>>> 'all',\n>>> 'auto',\n>>> 'imx8-isi',\n>>> 'ipu3',\n>>> 'rkisp1',\n>>> 'rpi/vc4',\n>>> 'simple',\n>>> 'uvcvideo',\n>>> 'vimc'\n>>> ],\n>>>\n>>>\n>>> LIBCAMERA_PIPELINES_MATCH_LIST=``rkisp1,simple,imx8-isi``\n>>>\n>>> and keep a map in CameraManager class, but this indeed requires more\n>>> maintainership.\n>>\n>> I share the concern that adding a map of nicknames in CameraManager\n>> class would add some extra maintenance.\n>> In that respect, the option of adding an explicit name parameter to\n>> REGISTER_PIPELINE_HANDLER() macro in order to assign to each pipeline\n>> its meson option name, was looking simpler to me.\n> \n> If we want shorter names, that's the way to go indeed.\n> \n>> Now, your proposal to expand the env variable description, documenting\n>> how the pipeline handler names are assigned with the register macro,\n>> looks perfectly fine to me.\n>> If that option makes it acceptable to keep the current pipeline handler\n>> names, and nobody objects against the principle, I propose to move\n>> forward with that for the v3.\n>>\n>>> Now that I wrote this, isn't it easier to just compile only the\n>>> pipeline handler you need ? I presume this doesn't apply to generic\n>>> distro though\n>>\n>> It does not apply to generic distro, indeed.\n>>\n>> But also when using a custom libcamera package, one may want to change\n>> pipeline handler at runtime because of different use case, camera\n>> (raw/smart), test purpose... Reconfiguring and rebuilding libcamera with\n>> the unique targeted pipeline is not a very practical option here.\n> \n> I'm still struggling a bit to understand the use cases (besiding testing\n> during development). Please see below.\n> \n>>>>>> +\n>>>>>> LIBCAMERA_RPI_CONFIG_FILE\n>>>>>> Define a custom configuration file to use in the Raspberry Pi pipeline handler.\n>>>>>>\n>>>>>> diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n>>>>>> index 33ebe069..c57e509a 100644\n>>>>>> --- a/include/libcamera/internal/camera_manager.h\n>>>>>> +++ b/include/libcamera/internal/camera_manager.h\n>>>>>> @@ -45,6 +45,7 @@ private:\n>>>>>> int init();\n>>>>>> void createPipelineHandlers();\n>>>>>> void cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);\n>>>>>> + void pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory);\n>>>>>\n>>>>> Please move this one line up to match the function definition order in\n>>>>> the .cpp file\n>>>>>\n>>>>>>\n>>>>>> /*\n>>>>>> * This mutex protects\n>>>>>> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n>>>>>> index 355f3ada..620be1c8 100644\n>>>>>> --- a/src/libcamera/camera_manager.cpp\n>>>>>> +++ b/src/libcamera/camera_manager.cpp\n>>>>>> @@ -99,13 +99,36 @@ int CameraManager::Private::init()\n>>>>>>\n>>>>>> void CameraManager::Private::createPipelineHandlers()\n>>>>>> {\n>>>>>> - CameraManager *const o = LIBCAMERA_O_PTR();\n>>>>>> -\n>>>>>> /*\n>>>>>> * \\todo Try to read handlers and order from configuration\n>>>>>> - * file and only fallback on all handlers if there is no\n>>>>>> - * configuration file.\n>>>>>> + * file and only fallback on environment variable or all handlers, if\n>>>>>> + * there is no configuration file.\n>>>>>> + */\n>>>>>> +\n>>>>>> + /*\n>>>>>> + * When a list of preferred pipelines is defined, iterate through the\n>>>>>> + * ordered list to match the devices enumerated.\n>>>>>\n>>>>> s/devices enumerated/enumerated devices/\n>>>>>\n>>>>>> + * Otherwise, device matching is done in no specific order with each\n>>>>>> + * registered pipeline handler.\n>>>>>> */\n>>>>>\n>>>>> What about moving this comment block in the below if (pipesList)\n>>>>> branch ?\n>>>>>\n>>>>>> + const char *pipesList =\n>>>>>> + utils::secure_getenv(\"LIBCAMERA_PIPELINES_MATCH_LIST\");\n>>>>>> + if (pipesList) {\n>>>>>> + for (const auto &pipeName : utils::split(pipesList, \",\")) {\n>>>>>> + const PipelineHandlerFactoryBase *factory;\n>>>>>> + factory = PipelineHandlerFactoryBase::getFactoryByName(pipeName);\n>>>>>> + if (!factory)\n>>>>>> + continue;\n>>>>>> +\n>>>>>> + LOG(Camera, Debug)\n>>>>>> + << \"Found listed pipeline handler '\"\n>>>>>> + << pipeName << \"'\";\n>>>>>> + pipelineFactoryMatch(factory);\n>>>>>> + }\n>>>>>> +\n>>>>>> + return;\n>>>>>> + }\n>>>>>> +\n>>>>>> const std::vector &factories =\n>>>>>> PipelineHandlerFactoryBase::factories();\n>>>>>>\n>>>>>> @@ -117,15 +140,25 @@ void CameraManager::Private::createPipelineHandlers()\n>>>>>> * Try each pipeline handler until it exhaust\n>>>>>> * all pipelines it can provide.\n>>>>>> */\n>>>>>> - while (1) {\n>>>>>> - std::shared_ptr pipe = factory->create(o);\n>>>>>> - if (!pipe->match(enumerator_.get()))\n>>>>>> - break;\n>>>>>> + pipelineFactoryMatch(factory);\n>>>>>> + }\n>>>>>> +}\n>>>>>>\n>>>>>> - LOG(Camera, Debug)\n>>>>>> - << \"Pipeline handler \\\"\" << factory->name()\n>>>>>> - << \"\\\" matched\";\n>>>>>> - }\n>>>>>> +void CameraManager::Private::pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory)\n>>>>>> +{\n>>>>>> + CameraManager *const o = LIBCAMERA_O_PTR();\n>>>>>> +\n>>>>>> + /*\n>>>>>> + * Provide as many matching pipelines as possible\n>>>>>> + */\n>>>>>\n>>>>> Fits on a single line\n>>>>>\n>>>>> Also, I would\n>>>>> /* Match all the registed pipeline handlers. */\n>>>>>\n>>>>>> + while (1) {\n>>>>>> + std::shared_ptr pipe = factory->create(o);\n>>>>>> + if (!pipe->match(enumerator_.get()))\n>>>>>> + break;\n>>>>>> +\n>>>>>> + LOG(Camera, Debug)\n>>>>>> + << \"Pipeline handler \\\"\" << factory->name()\n>>>>>> + << \"\\\" matched\";\n>>>>>> }\n>>>>>> }\n>>>>>\n>>>>> Overall, I concur this is an useful addition. I know there are ideas\n>>>>> about assigning a priority to pipeline handlers at creation time as\n>>>>> currently the only conflict is on the ISI which can be\n>>>>> matched both by the imx8-isi pipeline and the simple pipeline.\n>>>>\n>>>> As you mentioned, imx8-isi and simple pipelines are concurrent.\n> \n> For this very specific problem, shouldn't we simply drop imx8-isi\n> support from the simple pipeline handler ? It was added there before we\n> had a dedicated pipeline handler for the imx8-isi. Now that we have one,\n> all the use cases that the simple pipeline handler could cover should be\n> handled by the imx8-isi pipeline handler. If some are missing, that's a\n> bug, and the imx8-isi pipeline handler should be extended.\n\nLibcamera users are likely familiar with simple pipeline and probably \nexpect it to work on i.MX8/9 - as I understand it seems commonly used.\nAlso, there are features from simple pipeline not available in imx8-isi \npipeline, for instance support for more complex topologies - agreed, \nimx8-isi could be extended to integrate that. Another example could be \nSoftware ISP in simple pipeline - that may be valuable for i.MX \nplatforms having ISI device but no hardware ISP (i.MX93). On those \nplatforms I presume that Software ISP + IPA support using simple \npipeline could come almost for free - but not sure that extending \nimx8-isi to add the same support would be worth the effort and the extra \nmaintenance cost.\n\nImx8-isi pipeline presumably brings value compared to simple pipeline on \nsystems using a smart sensor, when the use case requires duplicating \nthe stream with rescaling / CSC.\nThat said, the simple pipeline supports an optional scaler. So, an ISI \nchannel could possibly be setup in M2M mode to act as the scaler and \nachieve a similar functionality (though less efficient with regard to \nmemory bandwidth usage).\n\nThus, I think we should keep support for mxc.isi device in the simple \npipeline as it is well-known by libcamera users, and also to benefit \nfrom all its features.\n\n> \n>>>> With imx9\n>>>> family, there is an additional cause of concurrency as platforms can/will\n>>>> use topologies with both ISI + ISP. And depending on the use case, user may\n>>>> want to use either simple, or imx8-isi or the isi+isp pipeline.\n> \n> Note that the i.MX8MP also has an ISI and 2 ISP instances. Currently, we\n> need to select what to route the CSIS output to in DT, which is not a\n> good solution, but from a libcamera point of view, it means that only\n> one of the imx8-isi and rkisp1 pipeline handler can ever match at\n> runtime.\n\nOn i.MX8MP indeed, depending on DT configuration, one can configure the \ntopology as:\nCSI->ISP (no ISI): rkisp1 pipeline match\nCSI->ISI (no ISP): imx8-isi or single pipeline match\nConcurrency is only about imx8-isi vs single pipeline, for the ISI case.\n\n> \n> This should be fixed in the kernel, to enable dynamic routing\n> configuration from userspace. That's no simple work, and I'm not aware\n> of anyone actively working on it, but should this be completed at some\n> point, we would need to also improve libcamera to cover all the use\n> cases with a single pipeline handler. I don't really see why users would\n> want to select between multiple pipeline handlers.\n\nOn i.MX8MP, even with dynamic routing in the kernel, I am not sure which \nunique pipeline could handle the 2 uses cases :\nISP case: would presumably remain something for rkisp1 pipeline\nISI / no-ISP case: the 2 options of simple or imx8-isi pipelines would \nstill apply. Rkisp1 pipeline looks out of scope here.\n\n> How does the situation differ on the i.MX9 ?\n\ni.MX9 SoCs have an ISI device. Some of them also have a (new) ISP. That \nISP has a single hardware/context instance, and multi-camera use case is \noperated via memory-to-memory operation. Camera raw frames are captured \nfrom ISI channels video devices, and associated buffers passed to a \nsoftware instance of the ISP device. On the principle, the flow of \nbuffers handled by the pipeline is similar to the one done by IPU3 \npipeline.\n\nA dedicated pipeline is needed to support this ISP and its associated \nIPA. ISI device usage is a common factor between that pipeline and \nimx8-isi pipeline. However, usage and operation of the 2 pipelines being \nvery different, there is very limited implementation that can be shared \nbetween them. Therefore, working assumption has been that evolution and \nmaintenance would be simpler by keeping the 2 separated.\nIt may be a bit premature to discuss the details of this ISP pipeline \nright now: the ISP driver has not been posted to linux-media yet, which \nis presumably a prerequisite before addressing the libcamera pipeline \npart.\n\nTo come back to the original question, i.MX9 user may want to use simple \nor imx8-isi pipeline with a smart camera, or the ISP pipeline with a raw \ncamera. But to pick the ISP pipeline, its match() procedure has to be \ninvoked first so that ISI devices are not acquired by simple or imx8-isi \npipelines.\n\n>>>> This change is a basic way to dynamically assign relative priorities to the\n>>>> pipelines. It was suggested in v1 review comments to also have a similar\n>>>> configuration possible from a global config file.\n>>>>\n>>>>> For the time being, with the above issues fixed\n>>>>> Reviewed-by: Jacopo Mondi \n> \n> --\n> Regards,\n> \n> Laurent Pinchart\n\nThanks,\nJulien","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 11CDDC0DA4\n\tfor ;\n\tThu, 28 Mar 2024 15:54:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AFC936333B;\n\tThu, 28 Mar 2024 16:54:26 +0100 (CET)","from EUR04-DB3-obe.outbound.protection.outlook.com\n\t(mail-db3eur04on061d.outbound.protection.outlook.com\n\t[IPv6:2a01:111:f400:fe0c::61d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 553D2632EA\n\tfor ;\n\tThu, 28 Mar 2024 16:54:25 +0100 (CET)","from AS8PR04MB8150.eurprd04.prod.outlook.com\n\t(2603:10a6:20b:3f0::12)\n\tby AS5PR04MB9856.eurprd04.prod.outlook.com (2603:10a6:20b:678::13)\n\twith Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7409.32;\n\tThu, 28 Mar 2024 15:54:19 +0000","from AS8PR04MB8150.eurprd04.prod.outlook.com\n\t([fe80::d380:a139:d82f:24fe]) by\n\tAS8PR04MB8150.eurprd04.prod.outlook.com\n\t([fe80::d380:a139:d82f:24fe%4]) with mapi id 15.20.7409.038;\n\tThu, 28 Mar 2024 15:54:19 +0000"],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"iFAxPXpQ\";\n\tdkim-atps=neutral","dkim=none (message not signed)\n\theader.d=none;dmarc=none action=none header.from=nxp.com;"],"ARC-Seal":"i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;\n\tb=OfYsp4QTQ34mJsWOmQRetB+GTr/le3seXj0yeYHeJw6vi8yOwF96OC44KE9p3RdFe4V1Fv/0WejhkyklVKcFl/qJDuVqltqvc0lTeEf0vJNGUx28/IYXjgHrDoApRlGGR8v1middqGYD7Tg6zwYEqLOruMj7u8aITpF7YLjEpwLxxoqWNwp9x1bNbofUrX15lxNPDql7BU+l2ctb1/ccogodFE135GYohgntdmiKrLGoDxNi1o4NnG9718FF5f+sWLc9LP/RpWxeZWsv5sWMvBJdp5mg0WKbcD7wuPGSk1lyBX9DbnFk9pazm+oWSb9EdHURxE1hGXOb2ZBBLqmn0g==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;\n\ts=arcselector9901;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;\n\tbh=pdRmSmkdpkuDJPvkcW3LrOJj2EcP/xDqG5pzhtj5y6M=;\n\tb=h0VSjUjEUY+PfEP4EzkmbSkyWtu4fjwtR73rlLAl9X+25sbCd+9DRpp+9i47+Su/x+dsqa6G7oHtpneWbagN1hQk4h1Veda45UwYoD+6RujSBqvQKsdeHyzCeSaie2pLmhmRL9WA8QGBcTPWxuidNoYaI3UVfpWLCJLmeHpZ3THjyBWfj578ieqtT5K+oNHDqelDNrYsXxFU8bvHb4vOUnIKkM3svLJgBVgVtpGvAZ12tlSO9C+JdqSaAT3cC75xO2OBJSAfLG7KQ+HGUOx9AQ2NqNBLVBYVXzBG148cDkfmJjODqM1eDynaHVK+indgvMwARPmkOokOSHxjptKQJg==","ARC-Authentication-Results":"i=1; mx.microsoft.com 1; spf=pass\n\tsmtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com;\n\tdkim=pass header.d=nxp.com; arc=none","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector2;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;\n\tbh=pdRmSmkdpkuDJPvkcW3LrOJj2EcP/xDqG5pzhtj5y6M=;\n\tb=iFAxPXpQEgYpGAGeIBE7yDAVu4Q/3nLua+5i/8C47Q1Smt4Mahxpa0tioMVxn19kS4rI+1T0MPx8ZBPrsd61C09mw90leS3a7WEddD3+28lOO8c+jPLeLMrZDjEaDrs2HAE2/hhRLsMFsOPDZdC6z28WCXoURvQrmaMj+V3Mtr0=","Message-ID":"<278f4389-0cc6-462e-8205-c132b4345f26@nxp.com>","Date":"Thu, 28 Mar 2024 16:54:46 +0100","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","To":"Laurent Pinchart ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20240308110056.453320-1-julien.vuillaumier@nxp.com>\n\t<20240308110056.453320-3-julien.vuillaumier@nxp.com>\n\t\n\t\n\t\n\t<72347124-7275-4305-ab3b-d458abc0e940@nxp.com>\n\t<20240327203550.GT4721@pendragon.ideasonboard.com>","Content-Language":"en-US","From":"Julien Vuillaumier ","In-Reply-To":"<20240327203550.GT4721@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-ClientProxiedBy":"AM9P250CA0021.EURP250.PROD.OUTLOOK.COM\n\t(2603:10a6:20b:21c::26) To AS8PR04MB8150.eurprd04.prod.outlook.com\n\t(2603:10a6:20b:3f0::12)","MIME-Version":"1.0","X-MS-PublicTrafficType":"Email","X-MS-TrafficTypeDiagnostic":"AS8PR04MB8150:EE_|AS5PR04MB9856:EE_","X-MS-Office365-Filtering-Correlation-Id":"062490d0-508d-4865-4e62-08dc4f3f5439","X-LD-Processed":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635,ExtAddr","X-MS-Exchange-SenderADCheck":"1","X-MS-Exchange-AntiSpam-Relay":"0","X-Microsoft-Antispam":"BCL:0;","X-Microsoft-Antispam-Message-Info":"zukPf5CwfKT99tXRV9HNawQ1VjwhFaLNxI0jyOku64k1f4grF/fFQogKOi4WHdOUQm5TepcTVtc+IKfsURO/8D0OOoDsbKREodqYkmqDhs+ffqstQemwJae+R6sp7tjiukw017VeQIr9JA5CS65uKUh0m4M8IOs3qgz0J+n9n4fwn/JPaxUCK4Hx8Fdl5LL18sWrJeGx271Gx29gACmt3m5wDKaSCNiXpJ+RksKR1HUJ0rEzUSlFzVgF0CfGMm/nEa1bh85+skmHBFnRPbCgIYFD954QRSVx4dUb7sTWnTuDgkGlNHtWdLiLM0DdD0GrqoqgHhqvXlOGCvDV0F7FNP3Hn7OqrrahKZrPNOxe7HaoTh02CFAUevBsGSfVVw51w8NwqrER7Q9pFJJYcvSVMHRuzLUcnhTjhG3bjyTdTcT6Z27m9JYe+HBfYVaihlnuZQfigxuoAV94/ULYmo3Ot3j8Sd0OIgyp5SRvqUMXvj9XFwfD9IR91VEf4CgdM8+rbGrt0A+jzTQn8jNGQBD8CCrTwpF640JN3Kz7swnEASL0aeplNRBalVRjClxYxnoLjF6xPG8khfc9sbX/B4rURLWWocC1wZi6HosQzRb2W5pNc4/RmEDulvhteMQpyy8iY7BHcsqTnhk1jzOPUfeGx0QOFEwSAWdnBxN+OyCcz3U=","X-Forefront-Antispam-Report":"CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;\n\tIPV:NLI; SFV:NSPM; H:AS8PR04MB8150.eurprd04.prod.outlook.com; PTR:;\n\tCAT:NONE; \n\tSFS:(13230031)(1800799015)(376005)(366007); DIR:OUT; SFP:1101; ","X-MS-Exchange-AntiSpam-MessageData-ChunkCount":"1","X-MS-Exchange-AntiSpam-MessageData-0":"=?utf-8?q?CMJ+3+gPQ0YJTHc436OxZ7u4B?=\n\t=?utf-8?q?Z9kxVLK312mg+RaqjNwi2HjU/x4rrSMp4e0SUlijsFWkN0r94RBBG/Ed?=\n\t=?utf-8?q?d7DCbAvWZF6vV4mT0IkFaQ7Q78WoSK9WhsWw3EzlJRFP5wGcbImOxB2j?=\n\t=?utf-8?q?UY0SygKj5NWEKNBP6newxkh3bQv4vaeaq377d4+2hSnlzz8COoFf9hwV?=\n\t=?utf-8?q?kjuF9WCm/+ePN1lRVFu0WwKUX4MDazdiOXQwn6DmYeubwtOQqWLOFpBL?=\n\t=?utf-8?q?jOUg27alFiVZM1aV+6zftfVekYbzY99ynT2Hy6SRFoaNwu3VEl++7hoN?=\n\t=?utf-8?q?MZ2sKdsBfcfS5f51NturdTdBCkEq6C0eFWou+85LX7cTnwM9YUayboxG?=\n\t=?utf-8?q?dCWkPcWdIYbMX6oc2PDn2wXTnEsODG4W+GggqWI9di85vts9LeKjR/Iq?=\n\t=?utf-8?q?YBBdqWPxt6D1mo21KD1rSoX3lnPM3aWJGsu6q4suhvlDIbOOKj21rPZL?=\n\t=?utf-8?q?cU8avKrJ8Fg4sDwuGt0i8LnmDZmw4QCGvBrwl40RlKCwLt6sWphBRRYD?=\n\t=?utf-8?q?50gP1jhVgWblNVP5PgQkH5+SC6itLi2uxUB1MheUhDVuUBaqkVdN8+53?=\n\t=?utf-8?q?CTLMjd9AXEqaaMW67NRVzVcH9zcMuGW4AOO+KayNCSUeBhFJ2Xt2VegE?=\n\t=?utf-8?q?EChYY2rIuie52UreSw6shM22cOBEtH6AWZ1vGBayiupih19fnfMWxWl3?=\n\t=?utf-8?q?YWBmSFDssssdlSrGAVjKBxdaJsDLfsxiyp7nfa7dmkgi4nDffGuJA8hC?=\n\t=?utf-8?q?Xwr0e7D65s7k+ArZS2Ti1+NOXC5eKG2gDtsUL0zKp9GBcsTPjDZOJ4Fi?=\n\t=?utf-8?q?N1cm3UKLd4dbFbOe1cSBgVpMX7peTcuUwRJxQ1VFA7sbvQVpjmZllUPV?=\n\t=?utf-8?q?In82dBt77FxPpIzv58RMqGJgxA78oQypCZjSPpgZObsNP9vr/gGgGMOc?=\n\t=?utf-8?q?bO1b1YQ6qqcqZMbP16nY/oxgccp0zH41Pc+CCM4kRdiBn3flOw0+Issh?=\n\t=?utf-8?q?ICuh+eI1jaB4ZgBKenoXjLM+YhvodunXWpMOJEKnUHBH4o5OxGaepSxy?=\n\t=?utf-8?q?cpjE8hIf2lcYvHKgO9zvS1yX4IRnhkTeTpjupvu1Yu3H1/C4WjTY7iKX?=\n\t=?utf-8?q?Fs9IAV8FOT4KZ6ANE5z1hjsFLCritP59f5hrj1p3f9oTd/ZbfV4A0z8n?=\n\t=?utf-8?q?IVxZ4AqfM1RWvYEwfvA9fC4IPKQJun6ZGvcC8QTurYqiEzrfX3gRvu8P?=\n\t=?utf-8?q?v1WWrdv8kQ1nzM24WAq32sIxy3bm03MCwgIvINgmBLJzNIRjThvZqBAy?=\n\t=?utf-8?q?BZPVi7KDhKD5EXRhwC/6ycrPMnaMha8SMJi17toYoDrVNhXDQdpapIw1?=\n\t=?utf-8?q?JAwAjOWH26nUl5Cjn5f1z1kqAgpCRkEJNYTuwpAuSPUwYIUy8F4Z+icQ?=\n\t=?utf-8?q?AB03IopWOIvQBPZK8s3ErqhQLbW6Q5xgJIR/FSEuubcbZlP11QjEc2Ny?=\n\t=?utf-8?q?iL/uTyD732ALeGiBnE9DnjsIWLyi55/Zoekw8sBCG0uV9wOkhLrGYf1T?=\n\t=?utf-8?q?qty37fYMT38sXmEW9jdCC3xsLjQJQdFMc9Ql95rvL3c80pIRboVPOdMy?=\n\t=?utf-8?q?1lo/kmh+7C269qTGt1AgPHNZr8XfbGIbcF/K21mlyS+a+H7jJC8Am/2s?=\n\t=?utf-8?q?qkQPgadIiWj9If6dY1xJhVFRCduFQ=3D=3D?=","X-OriginatorOrg":"nxp.com","X-MS-Exchange-CrossTenant-Network-Message-Id":"062490d0-508d-4865-4e62-08dc4f3f5439","X-MS-Exchange-CrossTenant-AuthSource":"AS8PR04MB8150.eurprd04.prod.outlook.com","X-MS-Exchange-CrossTenant-AuthAs":"Internal","X-MS-Exchange-CrossTenant-OriginalArrivalTime":"28 Mar 2024 15:54:19.6114\n\t(UTC)","X-MS-Exchange-CrossTenant-FromEntityHeader":"Hosted","X-MS-Exchange-CrossTenant-Id":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635","X-MS-Exchange-CrossTenant-MailboxType":"HOSTED","X-MS-Exchange-CrossTenant-UserPrincipalName":"fZvU+jvQDEZO/kLriDA0/ztmh6hkQq8wOIRQp78EEyiPZGwUyyk8FWU2RkvLBqal0RbQYwtL9MeBBlfd8ZY9ON5uQGjV4Mm1W1wlESvib2s=","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"AS5PR04MB9856","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]