[{"id":29308,"web_url":"https://patchwork.libcamera.org/comment/29308/","msgid":"<171386336704.1272602.18378385220870805500@ping.linuxembedded.co.uk>","date":"2024-04-23T09:09:27","subject":"Re: [PATCH v3 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Julien Vuillaumier (2024-03-27 17:27:31)\n> To match the enumerated media devices, each registered pipeline handler\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 to give the option to define an ordered list of pipelines to\n> match on.\n> \n> LIBCAMERA_PIPELINES_MATCH_LIST=\"[,[,...]]]\"\n> \n> Example:\n> LIBCAMERA_PIPELINES_MATCH_LIST=\"PipelineHandlerRkISP1,SimplePipelineHandler\"\n\nI still think this env variable (and presumably also possible to set in\na configuration file with Milan's configuration file work) makes sense.\n\nI do wish the match list used the short names that are equivalent to how\nthe meson configuration would name them though. Perhaps as you suggested\nby using a parameter to the pipeline handler registration instead so\nthat a more suitable name is used for the references.\n\nThat would still then work with the previous patch I believe.\n\nSo ... I think it's worth making the naming easier for users if we set\nthis ... but I also think this patch is ok.\n\n\nReviewed-by: Kieran Bingham \n\nSo the question is - if we merge this - does it become some sort of\nexpected ABI and mean we /shouldn't/ then change the name to the more\nuser-friendly ones? And therefore we should do that first? or just do it\non top ?\n\n--\nKieran\n\n\n> \n> Signed-off-by: Julien Vuillaumier \n> Reviewed-by: Jacopo Mondi \n> ---\n> Documentation/environment_variables.rst | 8 ++++\n> include/libcamera/internal/camera_manager.h | 1 +\n> src/libcamera/camera_manager.cpp | 53 ++++++++++++++++-----\n> 3 files changed, 50 insertions(+), 12 deletions(-)\n> \n> diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n> index a9b230bc..c763435c 100644\n> --- a/Documentation/environment_variables.rst\n> +++ b/Documentation/environment_variables.rst\n> @@ -37,6 +37,14 @@ 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\n> + devices in the system. The pipeline handler names used to populate the\n> + variable are the ones passed to the REGISTER_PIPELINE_HANDLER() macro in the\n> + source code.\n> +\n> + Example value: ``PipelineHandlerRkISP1,SimplePipelineHandler``\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..5694d40d 100644\n> --- a/include/libcamera/internal/camera_manager.h\n> +++ b/include/libcamera/internal/camera_manager.h\n> @@ -44,6 +44,7 @@ protected:\n> private:\n> int init();\n> void createPipelineHandlers();\n> + void pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory);\n> void cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);\n> \n> /*\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index 355f3ada..ce6607e1 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -99,16 +99,37 @@ 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> + const char *pipesList =\n> + utils::secure_getenv(\"LIBCAMERA_PIPELINES_MATCH_LIST\");\n> + if (pipesList) {\n> + /*\n> + * When a list of preferred pipelines is defined, iterate\n> + * through the ordered list to match the enumerated devices.\n> + */\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> + /* Match all the registered pipeline handlers. */\n> for (const PipelineHandlerFactoryBase *factory : factories) {\n> LOG(Camera, Debug)\n> << \"Found registered pipeline handler '\"\n> @@ -117,15 +138,23 @@ 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> + /* Provide as many matching pipelines as possible. */\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> -- \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 93381C3200\n\tfor ;\n\tTue, 23 Apr 2024 09:09:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8C9F66341C;\n\tTue, 23 Apr 2024 11:09:31 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BD20463408\n\tfor ;\n\tTue, 23 Apr 2024 11:09:29 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 935FE65D;\n\tTue, 23 Apr 2024 11:08:38 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"vTRbBMcV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713863318;\n\tbh=m18fJu4Q6FoaglCO6f99bB7Xgzo/Wj7iLnteXHwrdq8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=vTRbBMcV7S+JHmAs8mUkxlcy/EZq5O+LdpuiJhasuLcWtPztIQl9cgoGxvfkuXt78\n\txrdKFchQExhL5RyuDcd6+5OxG5VrJskNZQZ2gss+gLjFX4S2cpMKxrM1fGPaMPsEiA\n\teVihIGXpJ8wTF10MRHqzNAxGWTjqT1B/T/VSCNWs=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240327172731.434048-3-julien.vuillaumier@nxp.com>","References":"<20240327172731.434048-1-julien.vuillaumier@nxp.com>\n\t<20240327172731.434048-3-julien.vuillaumier@nxp.com>","Subject":"Re: [PATCH v3 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","From":"Kieran Bingham ","Cc":"julien.vuillaumier@nxp.com, Jacopo Mondi ","To":"Julien Vuillaumier ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 23 Apr 2024 10:09:27 +0100","Message-ID":"<171386336704.1272602.18378385220870805500@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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":29313,"web_url":"https://patchwork.libcamera.org/comment/29313/","msgid":"<87bk60cqpb.fsf@redhat.com>","date":"2024-04-23T10:34:56","subject":"Re: [PATCH v3 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Kieran Bingham writes:\n\n> Quoting Julien Vuillaumier (2024-03-27 17:27:31)\n>> To match the enumerated media devices, each registered pipeline handler\n>> is used in no specific order. It is a limitation when several pipelines\n>\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 to give the option to define an ordered list of pipelines to\n>> match on.\n>> \n>> LIBCAMERA_PIPELINES_MATCH_LIST=\"[,[,...]]]\"\n>> \n>> Example:\n>> LIBCAMERA_PIPELINES_MATCH_LIST=\"PipelineHandlerRkISP1,SimplePipelineHandler\"\n>\n> I still think this env variable (and presumably also possible to set in\n> a configuration file with Milan's configuration file work) makes sense.\n\nAs for the configuration file, I just posted \"Add global configuration file\"\npatch series. I haven't tried to include these patches there but feel free to\ncomment on / use / ignore the configuration file patches.\n\nRegards,\nMilan\n\n> I do wish the match list used the short names that are equivalent to how\n> the meson configuration would name them though. Perhaps as you suggested\n> by using a parameter to the pipeline handler registration instead so\n> that a more suitable name is used for the references.\n>\n> That would still then work with the previous patch I believe.\n>\n> So ... I think it's worth making the naming easier for users if we set\n> this ... but I also think this patch is ok.\n>\n>\n> Reviewed-by: Kieran Bingham \n>\n> So the question is - if we merge this - does it become some sort of\n> expected ABI and mean we /shouldn't/ then change the name to the more\n> user-friendly ones? And therefore we should do that first? or just do it\n> on top ?\n>\n> --\n> Kieran\n>\n>\n>> \n>> Signed-off-by: Julien Vuillaumier \n>> Reviewed-by: Jacopo Mondi \n>> ---\n>> Documentation/environment_variables.rst | 8 ++++\n>> include/libcamera/internal/camera_manager.h | 1 +\n>> src/libcamera/camera_manager.cpp | 53 ++++++++++++++++-----\n>> 3 files changed, 50 insertions(+), 12 deletions(-)\n>> \n>> diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n>> index a9b230bc..c763435c 100644\n>> --- a/Documentation/environment_variables.rst\n>> +++ b/Documentation/environment_variables.rst\n>> @@ -37,6 +37,14 @@ 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\n>> + devices in the system. The pipeline handler names used to populate the\n>> + variable are the ones passed to the REGISTER_PIPELINE_HANDLER() macro in the\n>> + source code.\n>> +\n>> + Example value: ``PipelineHandlerRkISP1,SimplePipelineHandler``\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..5694d40d 100644\n>> --- a/include/libcamera/internal/camera_manager.h\n>> +++ b/include/libcamera/internal/camera_manager.h\n>> @@ -44,6 +44,7 @@ protected:\n>> private:\n>> int init();\n>> void createPipelineHandlers();\n>> + void pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory);\n>> void cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);\n>> \n>> /*\n>> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n>> index 355f3ada..ce6607e1 100644\n>> --- a/src/libcamera/camera_manager.cpp\n>> +++ b/src/libcamera/camera_manager.cpp\n>> @@ -99,16 +99,37 @@ 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>> + const char *pipesList =\n>> + utils::secure_getenv(\"LIBCAMERA_PIPELINES_MATCH_LIST\");\n>> + if (pipesList) {\n>> + /*\n>> + * When a list of preferred pipelines is defined, iterate\n>> + * through the ordered list to match the enumerated devices.\n>> + */\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>> + /* Match all the registered pipeline handlers. */\n>> for (const PipelineHandlerFactoryBase *factory : factories) {\n>> LOG(Camera, Debug)\n>> << \"Found registered pipeline handler '\"\n>> @@ -117,15 +138,23 @@ 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>> + /* Provide as many matching pipelines as possible. */\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>> -- \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 E39A2BE08B\n\tfor ;\n\tTue, 23 Apr 2024 10:35:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 60D6961B28;\n\tTue, 23 Apr 2024 12:35:03 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7A14861AC9\n\tfor ;\n\tTue, 23 Apr 2024 12:35:01 +0200 (CEST)","from mail-wm1-f69.google.com (mail-wm1-f69.google.com\n\t[209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-471-FCu88jfbO6ijXjJiLT3ssg-1; Tue, 23 Apr 2024 06:34:59 -0400","by mail-wm1-f69.google.com with SMTP id\n\t5b1f17b1804b1-419ec098d81so13381065e9.2\n\tfor ;\n\tTue, 23 Apr 2024 03:34:58 -0700 (PDT)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\thg16-20020a05600c539000b0041aa8ad46d6sm2437853wmb.16.2024.04.23.03.34.56\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 23 Apr 2024 03:34:56 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"HSxToz9T\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1713868500;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=XC7wJyySvIsabo9ZXbivRDRMsG2DJT+olB7qxCZEKu0=;\n\tb=HSxToz9T6W5HNKiYSCdzg2/nRwWESs82s5teN9K+1cIcCSihmrViEWjuX8r+CMCM03lZ0d\n\t2XmTkKI8sjd7wCT/rLKIsK6qNmxzm+VK2Z8qtYxlJrgKIeeS1eO9M9o2Tlo+mNLaFEA6yQ\n\t8mt0tzYXZygiJLFa08D8qbxdY86+ppw=","X-MC-Unique":"FCu88jfbO6ijXjJiLT3ssg-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1713868498; x=1714473298;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=XC7wJyySvIsabo9ZXbivRDRMsG2DJT+olB7qxCZEKu0=;\n\tb=ChdjD9EeaDBeFc8XvL582HKFkTlY+cgJ0q6dxloYj7YlH3Eb0LhGtcHDBp+KBRC4M7\n\tSDvHx19imv67XwSmGNjJe29ux8RmbCjH4sq4Ir/37N+U+G0fyQU8c/cm4bdZJHmSt78O\n\timsS11SEBqo1xRxvhexaqfqG1YbfiNbHXEUR/yf377UvTrdiuXBaotDWeeKKkLwseTFA\n\tiFc3LmCe/CBu2h3/oxBqTCm8rPD6sLiYwsMSUIhulPyOvrX0ug9MDRT8P5c2IjGwQ53o\n\tvrwxhVOnqGjmkbtYoWVkQhcW1fJdRAksUVDfFCyKe9dFYcTOvsxfQSx2gOmfF69T0BOt\n\tA+7Q==","X-Gm-Message-State":"AOJu0YyuHp4BsN/kg7Y+l4wwyDh+ZYCuJ4WQWCpYIVEFSX9ZASdrLCSP\n\tIwvAdqrSM24X2FTg4cqp8CUgB2MVpmtNtfX9oOMNE73FC17HYvQ15qzA3EieksA1RC1gp3WXAlb\n\tvUbY7AlU+4I+iGwx/CmLUHO7G7e5ZuL9jm/p++xZaKbqb7tAWwgfK+OWohDCDJXw9Sh1XwNk=","X-Received":["by 2002:a05:600c:1913:b0:418:2b26:47a3 with SMTP id\n\tj19-20020a05600c191300b004182b2647a3mr8355894wmq.32.1713868497992; \n\tTue, 23 Apr 2024 03:34:57 -0700 (PDT)","by 2002:a05:600c:1913:b0:418:2b26:47a3 with SMTP id\n\tj19-20020a05600c191300b004182b2647a3mr8355885wmq.32.1713868497568; \n\tTue, 23 Apr 2024 03:34:57 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IG9BmwIdTubr4hR1c4FEGTCpYKS9ZRidUI7ndKpxwzu62B5kQr6Oy4I+McGLRnnNSquEbHixQ==","From":"Milan Zamazal ","To":"Julien Vuillaumier ","Cc":"libcamera-devel@lists.libcamera.org, Kieran Bingham\n\t, Jacopo Mondi\n\t","Subject":"Re: [PATCH v3 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","In-Reply-To":"<171386336704.1272602.18378385220870805500@ping.linuxembedded.co.uk>\n\t(Kieran Bingham's message of \"Tue, 23 Apr 2024 10:09:27 +0100\")","References":"<20240327172731.434048-1-julien.vuillaumier@nxp.com>\n\t<20240327172731.434048-3-julien.vuillaumier@nxp.com>\n\t<171386336704.1272602.18378385220870805500@ping.linuxembedded.co.uk>","Date":"Tue, 23 Apr 2024 12:34:56 +0200","Message-ID":"<87bk60cqpb.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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":29354,"web_url":"https://patchwork.libcamera.org/comment/29354/","msgid":"<4ecb306d-5cac-42f2-8c43-dfb763b563fb@nxp.com>","date":"2024-04-26T15:13:43","subject":"Re: [PATCH v3 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 Kieran,\n\nOn 23/04/2024 11:09, Kieran Bingham wrote:\n\n> \n> \n> Quoting Julien Vuillaumier (2024-03-27 17:27:31)\n>> To match the enumerated media devices, each registered pipeline handler\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 to give the option to define an ordered list of pipelines to\n>> match on.\n>>\n>> LIBCAMERA_PIPELINES_MATCH_LIST=\"[,[,...]]]\"\n>>\n>> Example:\n>> LIBCAMERA_PIPELINES_MATCH_LIST=\"PipelineHandlerRkISP1,SimplePipelineHandler\"\n> \n> I still think this env variable (and presumably also possible to set in\n> a configuration file with Milan's configuration file work) makes sense.\n\nDefinitely, adding the option to configure the pipelines list using \nMilan's global configuration file would be useful.\n\n> I do wish the match list used the short names that are equivalent to how\n> the meson configuration would name them though. Perhaps as you suggested\n> by using a parameter to the pipeline handler registration instead so\n> that a more suitable name is used for the references.\n> \n> That would still then work with the previous patch I believe.\n> \n> So ... I think it's worth making the naming easier for users if we set\n> this ... but I also think this patch is ok.\n> \n> \n> Reviewed-by: Kieran Bingham \n> \n> So the question is - if we merge this - does it become some sort of\n> expected ABI and mean we /shouldn't/ then change the name to the more\n> user-friendly ones? And therefore we should do that first? or just do it\n> on top ?\n\nIf using the short names is the preferred way to go, then I suppose it \nis better adding that change first. It should work the same with the \nother patches.\nSo, if nobody objects, I will add to those patches the change in the \npipeline registration macro, in order to assign to each pipeline the \nshort name used in meson files.\n\nThanks,\nJulien\n\n> \n> --\n> Kieran\n> \n> \n>>\n>> Signed-off-by: Julien Vuillaumier \n>> Reviewed-by: Jacopo Mondi \n>> ---\n>> Documentation/environment_variables.rst | 8 ++++\n>> include/libcamera/internal/camera_manager.h | 1 +\n>> src/libcamera/camera_manager.cpp | 53 ++++++++++++++++-----\n>> 3 files changed, 50 insertions(+), 12 deletions(-)\n>>\n>> diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n>> index a9b230bc..c763435c 100644\n>> --- a/Documentation/environment_variables.rst\n>> +++ b/Documentation/environment_variables.rst\n>> @@ -37,6 +37,14 @@ 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\n>> + devices in the system. The pipeline handler names used to populate the\n>> + variable are the ones passed to the REGISTER_PIPELINE_HANDLER() macro in the\n>> + source code.\n>> +\n>> + Example value: ``PipelineHandlerRkISP1,SimplePipelineHandler``\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..5694d40d 100644\n>> --- a/include/libcamera/internal/camera_manager.h\n>> +++ b/include/libcamera/internal/camera_manager.h\n>> @@ -44,6 +44,7 @@ protected:\n>> private:\n>> int init();\n>> void createPipelineHandlers();\n>> + void pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory);\n>> void cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);\n>>\n>> /*\n>> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n>> index 355f3ada..ce6607e1 100644\n>> --- a/src/libcamera/camera_manager.cpp\n>> +++ b/src/libcamera/camera_manager.cpp\n>> @@ -99,16 +99,37 @@ 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>> + const char *pipesList =\n>> + utils::secure_getenv(\"LIBCAMERA_PIPELINES_MATCH_LIST\");\n>> + if (pipesList) {\n>> + /*\n>> + * When a list of preferred pipelines is defined, iterate\n>> + * through the ordered list to match the enumerated devices.\n>> + */\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>> + /* Match all the registered pipeline handlers. */\n>> for (const PipelineHandlerFactoryBase *factory : factories) {\n>> LOG(Camera, Debug)\n>> << \"Found registered pipeline handler '\"\n>> @@ -117,15 +138,23 @@ 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>> + /* Provide as many matching pipelines as possible. */\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>> --\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 CFCB4C3220\n\tfor ;\n\tFri, 26 Apr 2024 15:13:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 88BFD63416;\n\tFri, 26 Apr 2024 17:13:16 +0200 (CEST)","from EUR04-DB3-obe.outbound.protection.outlook.com\n\t(mail-db3eur04on0610.outbound.protection.outlook.com\n\t[IPv6:2a01:111:f400:fe0c::610])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6FD4861A9B\n\tfor ;\n\tFri, 26 Apr 2024 17:13:14 +0200 (CEST)","from AM9PR04MB8147.eurprd04.prod.outlook.com\n\t(2603:10a6:20b:3e0::22)\n\tby AM9PR04MB8131.eurprd04.prod.outlook.com (2603:10a6:20b:3ed::21)\n\twith Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7472.44;\n\tFri, 26 Apr 2024 15:13:13 +0000","from AM9PR04MB8147.eurprd04.prod.outlook.com\n\t([fe80::eace:e980:28a4:ef8a]) by\n\tAM9PR04MB8147.eurprd04.prod.outlook.com\n\t([fe80::eace:e980:28a4:ef8a%2]) with mapi id 15.20.7472.044;\n\tFri, 26 Apr 2024 15:13:13 +0000"],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"VF31SuNE\";\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=d0XQWUOC/7YNF9FNvjoz6demF54bZlmmGWL7vDeTZitV/jkDr4wXse8Uwzxb6gpDaCE/p9nHFXm1uBGCGzIpG6T7/xccE139qDj+Mo4VheDSEncPE7qHZFJgERLHvwa7EgR/YMkyXsWebya33FTmWEGtQzpariN2Pjvu6K6tNjQIa4LbxrTzadMtMr5VysmaQXu2vVmCT7/UFwRNgp7cpU7iLV1GVcfbLmjgKVj4SwN4zcdKkwHlq4Xyq7QBHV8VA/SgVkUjprhvkKngCILZr1/FqpEDX9Vq5Qem28+4lkcrjflJHt2dTb+Mm6aSgVVndJodn/0ucpjYAkoBDm2iNA==","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=SGm8Yp+W7zWESu5D39zBl3zSLG3ps4GVbT+aMAG6iS0=;\n\tb=YEhGZglLpRSs8FSyAMeZxUR7/3yEvwsu6HMCFCwbUg4jPiaHOPX2EgzQlT4eQ0TVx1WneL9Mqznz1gKP1zwUX/r1tCeUUK/XEKvLQlt/j8cYSMxefvcgSrXoJe+xXzSV9f5nzZEPaSdzZoxCA4gnBM+aK0E1bpLZEc1BwzTz5gqH//06dfbEW18IHy20lHAabfuT2RInwvYXFXtk2QJ2wn8YEzPVzvc/7HviUzMVsvWq+MMFIjZRhwLY2BiEqVrkLeAn8C7xI40F90hJVZMU9DgsG9nzylg0YqOADWYAdU0V4p8YHeXl4T8F2Fk8lT2unL9b7E2dw0rq+mRyGZe1HA==","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=SGm8Yp+W7zWESu5D39zBl3zSLG3ps4GVbT+aMAG6iS0=;\n\tb=VF31SuNEDyCStTvljjlVlmv1uFMflSGZNFi+fqfjvbev48Tbj1rFwv2BeUgkF+ndUIHqII3EehTK3yUl8ZsAVFC7sgXE9ycjnLL9o7FtYIyq9ysXPRFVb8FZyLHhL4yWxw9bTviYpDoo3Ek2+c6s4A7DwqNtDT45onLZLKZwMa0=","Message-ID":"<4ecb306d-5cac-42f2-8c43-dfb763b563fb@nxp.com>","Date":"Fri, 26 Apr 2024 17:13:43 +0200","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","To":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Cc":"Jacopo Mondi , mzamazal@redhat.com","References":"<20240327172731.434048-1-julien.vuillaumier@nxp.com>\n\t<20240327172731.434048-3-julien.vuillaumier@nxp.com>\n\t<171386336704.1272602.18378385220870805500@ping.linuxembedded.co.uk>","Content-Language":"en-US","From":"Julien Vuillaumier ","In-Reply-To":"<171386336704.1272602.18378385220870805500@ping.linuxembedded.co.uk>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-ClientProxiedBy":"AM0PR02CA0187.eurprd02.prod.outlook.com\n\t(2603:10a6:20b:28e::24) 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_|AM9PR04MB8131:EE_","X-MS-Office365-Filtering-Correlation-Id":"206ae8e4-875a-4fdc-b0a2-08dc6603640a","X-MS-Exchange-SenderADCheck":"1","X-MS-Exchange-AntiSpam-Relay":"0","X-Microsoft-Antispam":"BCL:0;","X-Microsoft-Antispam-Message-Info":"=?utf-8?q?9EEqkT0xtjiEThXc5JjgTHL++fex?=\n\t=?utf-8?q?3tQCyWEScuGMQtGBIYT/HFXjXxO3urkVCsVE3SjEzPg/rLEaA1TXjSFl?=\n\t=?utf-8?q?8mJxYD6cHUEkgSyFEonicPu7zS8kHHNLkCo5EGK7bUJuh95KqLsXVM8U?=\n\t=?utf-8?q?Jr51Z+Qy1VV68qNLIC9ghpd5fEz23I4OOi+fLqx015Ver1SNz7DOUkT/?=\n\t=?utf-8?q?HkzJXU3cNibiZs2n6vcIsb+nhJg7RaGklZwgLKXbUfNFYB81or3SBIMd?=\n\t=?utf-8?q?p3YhP8EQ1xiBqMTK8zHGKB4ooue0dF8PhWC7nnEGVNc2+tvGHDV1nA1p?=\n\t=?utf-8?q?tmB0cfUhNiAjZsRLOEZiTK7SNaw4J5nfA+cr5YigrNWERtQwjh5K5OQ4?=\n\t=?utf-8?q?Mv0AVh2UtTG1GN3ZqKEL1RRMHwtm8iibqI95gREgb5PHyxNio9TUGi/x?=\n\t=?utf-8?q?+2V6RAV9ocR/3K6xhR1f25Gc12RTnDSCEZ9Uroj2OATklzhyj56HrcyH?=\n\t=?utf-8?q?yTasTtIfR8j8stAXNWFZmDG6wpRLXhmmF9S+OJxiUyPLSGzjp0KdR3IU?=\n\t=?utf-8?q?1H3/jAbnjjliaMunnSFg0Pqy47o/AFsJMGCP9rAlRT1xEA74GbPX2SBk?=\n\t=?utf-8?q?ZWcnQrwhKXD264oEtoRZgRmOGT4qHHCRZ8r7mcVi7K5Sxm46TOf/BYce?=\n\t=?utf-8?q?AXoQvoIr332tgWEEAfIKMFHTegVrC+54x7KuaZwQNm4Zx+OkUPdA0KBg?=\n\t=?utf-8?q?LHIlziJ6CeoBqzxH+PLaXskO7+lNVm2V9znQzsg+q9CaiO7pU7ddfTON?=\n\t=?utf-8?q?CzE5PiWjtnVUpaSjwXsQPXYMpABAoHsexPBP6Gx9cL4N+lzAaX2/Qfsr?=\n\t=?utf-8?q?3X3CCDefnpDsiky2xZyYUul+22ZaHDW0l+8SdXL2uBufdhKQuTAKGy7v?=\n\t=?utf-8?q?EdfDWxawGeTZ+TJhdmucvwO19VKoXU0SXNHJyO2YGwpf1tEZsutci8hj?=\n\t=?utf-8?q?Nw5UmXG/+AaB2Wnehi6G3u2P+DA1vvMb6PeiCOI5msPhSEpa5Bf6Mrcp?=\n\t=?utf-8?q?HrcQIuf7RTtTVLVPT3kOOHJDVld4F2NxYAGkCvvmirfjWpaQOw7vZq4y?=\n\t=?utf-8?q?EnfkxZypoTmf4r+PKd++hHk7XCMFyf6gjkrYGP1wjyZodZry+IkfXYl8?=\n\t=?utf-8?q?/tQyRHS5/3p2TqB+lxfqxoXI2lrJpE3vVgADMlDo4W/ByRSzSUrhApvd?=\n\t=?utf-8?q?58v2k+Uk6mxPIG4B7GfxigXMUfUeiUMXnjQxa2/bqaDk+96yj2WkkADU?=\n\t=?utf-8?q?5Ha1oQdOL3gydcZaI7BRKm9i8U0UeH7H9xn6vayXRHBdAWtXkYs4J7WJ?=\n\t=?utf-8?q?7vl/1VzH7uAQHvOo5C2xyvKaqd8KgyUE5eVaaLdWSg=3D=3D?=","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?s7fBi97YRCH/cxr0NcrSAPhgi?=\n\t=?utf-8?q?m93pkpH9P0/YNGTUDHXelsrTOJsfacgm1VYh7/v/JAODZEr1K8tG1S3y?=\n\t=?utf-8?q?lH/nyPJRk0arNuRZWYFIXbDue2iEcRdmuyXXNYgfuhIR4HMt+MzaZEOm?=\n\t=?utf-8?q?lEPKGcr/EnY4TApEkyz8aLhDxum0E/+CuqdpyLVh3DdYd7nwayguMx0j?=\n\t=?utf-8?q?77J+SLdeYj6h90i5S3Xf3v3X1ho6kQT3qUGA6WzyUyGrhqcAXA9jf5pK?=\n\t=?utf-8?q?b43yJwsaUDucuzM6Lx+F6Ehb35C0QkSDAg86vQIwY0oEgJtP3fvIiIF2?=\n\t=?utf-8?q?U2v5HSgn6Te5TUP4J9YqG9admU6dSjhSY3/aJeema8iTDzeeaGq8CZ2D?=\n\t=?utf-8?q?BoBMsnhcLkGlsvBSe4YyWtIhbGIXoNl4YSjg+8FnuOqPVtDDOIozagIN?=\n\t=?utf-8?q?SZyoAHb5b61psPL7Rz2iylzT+Q4q/j+ngaqCsz8CdiRqadIR/zTuY67C?=\n\t=?utf-8?q?kh9Axal9m0ODyG49oRjiUspGAyuvhy1KaIuvECyfgeNcBJ5w9/cnykt4?=\n\t=?utf-8?q?CReQKDnrd/ppijD9QIOkuJdNGLvVS1cMaPEz4vUkXkXsh9pcTbIw/b5g?=\n\t=?utf-8?q?c12KNxvehHLYk+iJyaSqf1HMoMkpEubkjRKAvUNX5KYHVKK/LY9PDr9P?=\n\t=?utf-8?q?fiCYTanSXxUhMa0kyxE1AxLL6C4qKoWxxzHZqtHJYqh5bA2hwdtFahxh?=\n\t=?utf-8?q?HzeLKf9FIDAqhgTpenP0nMVzM26rY54ww2zgZEA/qxoVJvjjiFoWzruJ?=\n\t=?utf-8?q?W1iW6475FEhyNgiIYP9Sf8MYzZYiwT0GNzPdk9MUUFAGYWuxlCRR1Pl1?=\n\t=?utf-8?q?SFODCAwKcMqeH6TyM4xnv0OPIgKQNLVBe0DQoPNVJCMFqUSAfyarqZiX?=\n\t=?utf-8?q?35OOkDmo7oJvUrGjkohmnYmqyvzVyOS9WFrx0rpYdibBbm9wrvye3PCK?=\n\t=?utf-8?q?z++ro1yQd5XzRosE1oMzhI9G/UTEqef74lJbPEjdcN7iYoqARjzc66yx?=\n\t=?utf-8?q?PFJ5RboRpHMahBWijyTGwQ58krc23J9HtuwijR3/Dk0dCSB0sC6Ze6Sc?=\n\t=?utf-8?q?f82UqwCSeWheogN1wKtya4Qsdkm2gxT4wcuwiQfVpudUmRV3gE6IaWl/?=\n\t=?utf-8?q?elrIuKl+cEkR2plZNMK0sANP07176mcD370dkzL4e4yJPm5G8LspF2zf?=\n\t=?utf-8?q?qh/nm6p6rW0adkYNpBIXtDU63YuMkzDgYXCe97yNr8zCoiJd8NjccAik?=\n\t=?utf-8?q?Cyp/1+Jk+Y6ywIU9Znl74JfWchjeZn3H+Np4QL/LqdNTtnuCZy6mvGXX?=\n\t=?utf-8?q?WMwMbRTT5mYrxyHSKYuy5CsxmAxw4QPdoJNP0v11cyR4Cz8DuBjpo1HN?=\n\t=?utf-8?q?cM6tuyc21VJYM9KwM1OzI4aafdin+nMCEeWBRiRsQanb0VuyDC27RCiO?=\n\t=?utf-8?q?QaYE6dWgYEbBsn3LbBklquWeSzx7+gjcT8NfVDd1VXW0BSGQRXNqDHh+?=\n\t=?utf-8?q?pjtUvhyFnWgW8c/Iag69MzuWFUkmOrVS5fnNdksdlMtIjDOpu5jzBJyG?=\n\t=?utf-8?q?/vdKvqKlBDYNyDrdEwPJurLiWzcHSzqzMBCLuyCpkOXnTx293KC9y+Qt?=\n\t=?utf-8?q?AOswqcaYjHyoikWyaCLdPBBz18ITFnZfMbDgwjWoG9ASaPJmT9fF4qte?=\n\t=?utf-8?q?LeVAS/7zUUbSE4zTT6Hge5VEdb4jw=3D=3D?=","X-OriginatorOrg":"nxp.com","X-MS-Exchange-CrossTenant-Network-Message-Id":"206ae8e4-875a-4fdc-b0a2-08dc6603640a","X-MS-Exchange-CrossTenant-AuthSource":"AM9PR04MB8147.eurprd04.prod.outlook.com","X-MS-Exchange-CrossTenant-AuthAs":"Internal","X-MS-Exchange-CrossTenant-OriginalArrivalTime":"26 Apr 2024 15:13:13.0154\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":"S40ifXtU50EuYPZW5uQqZ61Yo3eh3ev/WUIiFGHvue0UCI5XGNaZthzH6iiucnrG6Yx3nybVa5bQ8F6RHyPxdZdd5eSdNfu2cC16Svd2rRs=","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"AM9PR04MB8131","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":29357,"web_url":"https://patchwork.libcamera.org/comment/29357/","msgid":"<171416630199.3237008.16626145635985878311@ping.linuxembedded.co.uk>","date":"2024-04-26T21:18:21","subject":"Re: [PATCH v3 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Julien Vuillaumier (2024-04-26 16:13:43)\n> Hi Kieran,\n> \n> On 23/04/2024 11:09, Kieran Bingham wrote:\n> \n> > Quoting Julien Vuillaumier (2024-03-27 17:27:31)\n> >> To match the enumerated media devices, each registered pipeline handler\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 to give the option to define an ordered list of pipelines to\n> >> match on.\n> >>\n> >> LIBCAMERA_PIPELINES_MATCH_LIST=\"[,[,...]]]\"\n> >>\n> >> Example:\n> >> LIBCAMERA_PIPELINES_MATCH_LIST=\"PipelineHandlerRkISP1,SimplePipelineHandler\"\n> > \n> > I still think this env variable (and presumably also possible to set in\n> > a configuration file with Milan's configuration file work) makes sense.\n> \n> Definitely, adding the option to configure the pipelines list using \n> Milan's global configuration file would be useful.\n> \n> > I do wish the match list used the short names that are equivalent to how\n> > the meson configuration would name them though. Perhaps as you suggested\n> > by using a parameter to the pipeline handler registration instead so\n> > that a more suitable name is used for the references.\n> > \n> > That would still then work with the previous patch I believe.\n> > \n> > So ... I think it's worth making the naming easier for users if we set\n> > this ... but I also think this patch is ok.\n> > \n> > \n> > Reviewed-by: Kieran Bingham \n> > \n> > So the question is - if we merge this - does it become some sort of\n> > expected ABI and mean we /shouldn't/ then change the name to the more\n> > user-friendly ones? And therefore we should do that first? or just do it\n> > on top ?\n> \n> If using the short names is the preferred way to go, then I suppose it \n> is better adding that change first. It should work the same with the \n> other patches.\n> So, if nobody objects, I will add to those patches the change in the \n> pipeline registration macro, in order to assign to each pipeline the \n> short name used in meson files.\n\nI think the short names are more likely to be the better reference for\nusers indeed. Maybe even something to use in the logging mechanism.\n\nIf you're happy to handle the update to the Pipeline Registration Macro,\nthat makes merging the topic easier I believe, so yes please!\n\n--\nKieran\n\n\n> \n> Thanks,\n> Julien\n> \n> > \n> > --\n> > Kieran\n> > \n> > \n> >>\n> >> Signed-off-by: Julien Vuillaumier \n> >> Reviewed-by: Jacopo Mondi \n> >> ---\n> >> Documentation/environment_variables.rst | 8 ++++\n> >> include/libcamera/internal/camera_manager.h | 1 +\n> >> src/libcamera/camera_manager.cpp | 53 ++++++++++++++++-----\n> >> 3 files changed, 50 insertions(+), 12 deletions(-)\n> >>\n> >> diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n> >> index a9b230bc..c763435c 100644\n> >> --- a/Documentation/environment_variables.rst\n> >> +++ b/Documentation/environment_variables.rst\n> >> @@ -37,6 +37,14 @@ 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\n> >> + devices in the system. The pipeline handler names used to populate the\n> >> + variable are the ones passed to the REGISTER_PIPELINE_HANDLER() macro in the\n> >> + source code.\n> >> +\n> >> + Example value: ``PipelineHandlerRkISP1,SimplePipelineHandler``\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..5694d40d 100644\n> >> --- a/include/libcamera/internal/camera_manager.h\n> >> +++ b/include/libcamera/internal/camera_manager.h\n> >> @@ -44,6 +44,7 @@ protected:\n> >> private:\n> >> int init();\n> >> void createPipelineHandlers();\n> >> + void pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory);\n> >> void cleanup() LIBCAMERA_TSA_EXCLUDES(mutex_);\n> >>\n> >> /*\n> >> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> >> index 355f3ada..ce6607e1 100644\n> >> --- a/src/libcamera/camera_manager.cpp\n> >> +++ b/src/libcamera/camera_manager.cpp\n> >> @@ -99,16 +99,37 @@ 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> >> + const char *pipesList =\n> >> + utils::secure_getenv(\"LIBCAMERA_PIPELINES_MATCH_LIST\");\n> >> + if (pipesList) {\n> >> + /*\n> >> + * When a list of preferred pipelines is defined, iterate\n> >> + * through the ordered list to match the enumerated devices.\n> >> + */\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> >> + /* Match all the registered pipeline handlers. */\n> >> for (const PipelineHandlerFactoryBase *factory : factories) {\n> >> LOG(Camera, Debug)\n> >> << \"Found registered pipeline handler '\"\n> >> @@ -117,15 +138,23 @@ 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> >> + /* Provide as many matching pipelines as possible. */\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> >> --\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 61810BE08B\n\tfor ;\n\tFri, 26 Apr 2024 21:18:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4DE0463416;\n\tFri, 26 Apr 2024 23:18:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8462761A9B\n\tfor ;\n\tFri, 26 Apr 2024 23:18:24 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E0AED66B;\n\tFri, 26 Apr 2024 23:17:30 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"aTMXxyOH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1714166250;\n\tbh=RVkungrCFv4kLyZ6OqbfyVOKZNdQ5ihqZN7+YKQWkxI=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=aTMXxyOHQBjugybydtsT883ujn56tPYb8GLFWKzqIsp4iDXFUcG+iDITzTw/1PeNQ\n\tTukWuOG3EuADAOBjidFiXpxCaCbMYbZBTEfu0yuJ089ivrV3yZ/On49wqx//VDqXM2\n\t8Dgn8EUAR6ajUdpUbiiny5MUbPZSD/fQhewstfCA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<4ecb306d-5cac-42f2-8c43-dfb763b563fb@nxp.com>","References":"<20240327172731.434048-1-julien.vuillaumier@nxp.com>\n\t<20240327172731.434048-3-julien.vuillaumier@nxp.com>\n\t<171386336704.1272602.18378385220870805500@ping.linuxembedded.co.uk>\n\t<4ecb306d-5cac-42f2-8c43-dfb763b563fb@nxp.com>","Subject":"Re: [PATCH v3 2/2] libcamera: camera_manager: Add environment\n\tvariable to order pipelines match","From":"Kieran Bingham ","Cc":"Jacopo Mondi , mzamazal@redhat.com","To":"Julien Vuillaumier ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 26 Apr 2024 22:18:21 +0100","Message-ID":"<171416630199.3237008.16626145635985878311@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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\" "}}]