[v2,1/2] libcamera: pipeline: Add a get factory by name helper
diff mbox series

Message ID 20240308110056.453320-2-julien.vuillaumier@nxp.com
State New
Headers show
Series
  • Add environment variable to order pipelines match
Related show

Commit Message

Julien Vuillaumier March 8, 2024, 11 a.m. UTC 
Signed-off-by: Julien Vuillaumier <julien.vuillaumier@nxp.com>
---
 include/libcamera/internal/pipeline_handler.h |  1 +
 src/libcamera/pipeline_handler.cpp            | 22 +++++++++++++++++++
 2 files changed, 23 insertions(+)

Comments

Jacopo Mondi March 20, 2024, 12:40 p.m. UTC | #1 
Hi Julien

On Fri, Mar 08, 2024 at 12:00:55PM +0100, Julien Vuillaumier wrote:

A commit message is required.

What about something alone the lines of

Add a static helper to the PipelineHandlerFactoryBase class to
allow retrieving a pipeline by name.

> Signed-off-by: Julien Vuillaumier <julien.vuillaumier@nxp.com>
> ---
>  include/libcamera/internal/pipeline_handler.h |  1 +
>  src/libcamera/pipeline_handler.cpp            | 22 +++++++++++++++++++
>  2 files changed, 23 insertions(+)
>
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index c96944f4..19361a40 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -114,6 +114,7 @@ public:
>  	const std::string &name() const { return name_; }
>
>  	static std::vector<PipelineHandlerFactoryBase *> &factories();
> +	static const PipelineHandlerFactoryBase *getFactoryByName(const std::string &name);
>
>  private:
>  	static void registerType(PipelineHandlerFactoryBase *factory);
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 29e0c98a..a821ab84 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -794,6 +794,28 @@ std::vector<PipelineHandlerFactoryBase *> &PipelineHandlerFactoryBase::factories
>  	return factories;
>  }
>
> +/**
> + * \brief Return the factory for the pipeline handler with name \a name
> + * \param[in] name The pipeline handler name
> + * \return The factory of the pipeline with name \a name, or nullptr if not found
> + */
> +const PipelineHandlerFactoryBase *PipelineHandlerFactoryBase::getFactoryByName(const std::string &name)
> +{
> +	const std::vector<PipelineHandlerFactoryBase *> &factories =
> +		PipelineHandlerFactoryBase::factories();
> +
> +	auto iter = std::find_if(factories.begin(),
> +				 factories.end(),
> +				 [&name](const PipelineHandlerFactoryBase *f) {
> +					return f->name() == name;
> +				 });
> +
> +	if (iter != factories.end())
> +		return *iter;
> +	else

No need for an else here, as you've return here above


> +		return nullptr;
> +}
> +

With the above proposed fixes:
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

>  /**
>   * \class PipelineHandlerFactory
>   * \brief Registration of PipelineHandler classes and creation of instances
> --
> 2.34.1
>
Julien Vuillaumier March 21, 2024, 7:26 p.m. UTC | #2 
Hi Jacopo,

I will integrate those fixes in the v3.
Thanks,

Julien

On 20/03/2024 13:40, Jacopo Mondi wrote:
> 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
> 
> 
> Hi Julien
> 
> On Fri, Mar 08, 2024 at 12:00:55PM +0100, Julien Vuillaumier wrote:
> 
> A commit message is required.
> 
> What about something alone the lines of
> 
> Add a static helper to the PipelineHandlerFactoryBase class to
> allow retrieving a pipeline by name.
> 
>> Signed-off-by: Julien Vuillaumier <julien.vuillaumier@nxp.com>
>> ---
>>   include/libcamera/internal/pipeline_handler.h |  1 +
>>   src/libcamera/pipeline_handler.cpp            | 22 +++++++++++++++++++
>>   2 files changed, 23 insertions(+)
>>
>> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
>> index c96944f4..19361a40 100644
>> --- a/include/libcamera/internal/pipeline_handler.h
>> +++ b/include/libcamera/internal/pipeline_handler.h
>> @@ -114,6 +114,7 @@ public:
>>        const std::string &name() const { return name_; }
>>
>>        static std::vector<PipelineHandlerFactoryBase *> &factories();
>> +     static const PipelineHandlerFactoryBase *getFactoryByName(const std::string &name);
>>
>>   private:
>>        static void registerType(PipelineHandlerFactoryBase *factory);
>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
>> index 29e0c98a..a821ab84 100644
>> --- a/src/libcamera/pipeline_handler.cpp
>> +++ b/src/libcamera/pipeline_handler.cpp
>> @@ -794,6 +794,28 @@ std::vector<PipelineHandlerFactoryBase *> &PipelineHandlerFactoryBase::factories
>>        return factories;
>>   }
>>
>> +/**
>> + * \brief Return the factory for the pipeline handler with name \a name
>> + * \param[in] name The pipeline handler name
>> + * \return The factory of the pipeline with name \a name, or nullptr if not found
>> + */
>> +const PipelineHandlerFactoryBase *PipelineHandlerFactoryBase::getFactoryByName(const std::string &name)
>> +{
>> +     const std::vector<PipelineHandlerFactoryBase *> &factories =
>> +             PipelineHandlerFactoryBase::factories();
>> +
>> +     auto iter = std::find_if(factories.begin(),
>> +                              factories.end(),
>> +                              [&name](const PipelineHandlerFactoryBase *f) {
>> +                                     return f->name() == name;
>> +                              });
>> +
>> +     if (iter != factories.end())
>> +             return *iter;
>> +     else
> 
> No need for an else here, as you've return here above
> 
> 
>> +             return nullptr;
>> +}
>> +
> 
> With the above proposed fixes:
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
>>   /**
>>    * \class PipelineHandlerFactory
>>    * \brief Registration of PipelineHandler classes and creation of instances
>> --
>> 2.34.1
>>

Patch
diff mbox series 

diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
index c96944f4..19361a40 100644
--- a/include/libcamera/internal/pipeline_handler.h
+++ b/include/libcamera/internal/pipeline_handler.h
@@ -114,6 +114,7 @@  public:
 	const std::string &name() const { return name_; }
 
 	static std::vector<PipelineHandlerFactoryBase *> &factories();
+	static const PipelineHandlerFactoryBase *getFactoryByName(const std::string &name);
 
 private:
 	static void registerType(PipelineHandlerFactoryBase *factory);
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index 29e0c98a..a821ab84 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -794,6 +794,28 @@  std::vector<PipelineHandlerFactoryBase *> &PipelineHandlerFactoryBase::factories
 	return factories;
 }
 
+/**
+ * \brief Return the factory for the pipeline handler with name \a name
+ * \param[in] name The pipeline handler name
+ * \return The factory of the pipeline with name \a name, or nullptr if not found
+ */
+const PipelineHandlerFactoryBase *PipelineHandlerFactoryBase::getFactoryByName(const std::string &name)
+{
+	const std::vector<PipelineHandlerFactoryBase *> &factories =
+		PipelineHandlerFactoryBase::factories();
+
+	auto iter = std::find_if(factories.begin(),
+				 factories.end(),
+				 [&name](const PipelineHandlerFactoryBase *f) {
+					return f->name() == name;
+				 });
+
+	if (iter != factories.end())
+		return *iter;
+	else
+		return nullptr;
+}
+
 /**
  * \class PipelineHandlerFactory
  * \brief Registration of PipelineHandler classes and creation of instances