[libcamera-devel,5/8] libcamera: pipeline_handler: Make factory create() function const
diff mbox series

Message ID 20221003212128.32429-6-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: Use class templates for auto-registration
Related show

Commit Message

Laurent Pinchart Oct. 3, 2022, 9:21 p.m. UTC 
The PipelineHandlerFactory::create() function has no need to modify the
factory instance. Make it const, as well as the createInstance()
function.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/internal/pipeline_handler.h | 6 +++---
 src/libcamera/pipeline_handler.cpp            | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

Comments

Xavier Roumegue Oct. 5, 2022, 11:09 a.m. UTC | #1 
Hi Laurent,

Thanks for the patch.

On 10/3/22 23:21, Laurent Pinchart via libcamera-devel wrote:
> The PipelineHandlerFactory::create() function has no need to modify the
> factory instance. Make it const, as well as the createInstance()
> function.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>   include/libcamera/internal/pipeline_handler.h | 6 +++---
>   src/libcamera/pipeline_handler.cpp            | 4 ++--
>   2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index 20f1cbb07fea..46df69dc789a 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -104,7 +104,7 @@ public:
>   	PipelineHandlerFactory(const char *name);
>   	virtual ~PipelineHandlerFactory() = default;
>   
> -	std::shared_ptr<PipelineHandler> create(CameraManager *manager);
> +	std::shared_ptr<PipelineHandler> create(CameraManager *manager) const;
>   
>   	const std::string &name() const { return name_; }
>   
> @@ -112,7 +112,7 @@ public:
>   	static std::vector<PipelineHandlerFactory *> &factories();
>   
>   private:
> -	virtual PipelineHandler *createInstance(CameraManager *manager) = 0;
> +	virtual PipelineHandler *createInstance(CameraManager *manager) const = 0;
>   
>   	std::string name_;
>   };
> @@ -124,7 +124,7 @@ public:									\
>   	handler##Factory() : PipelineHandlerFactory(#handler) {}	\
>   									\
>   private:								\
> -	PipelineHandler *createInstance(CameraManager *manager)		\
> +	PipelineHandler *createInstance(CameraManager *manager) const	\
>   	{								\
>   		return new handler(manager);				\
>   	}								\
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index e5cb751cc743..4470e9041e8e 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -676,7 +676,7 @@ PipelineHandlerFactory::PipelineHandlerFactory(const char *name)
>    * \return A shared pointer to a new instance of the PipelineHandler subclass
>    * corresponding to the factory
>    */
> -std::shared_ptr<PipelineHandler> PipelineHandlerFactory::create(CameraManager *manager)
> +std::shared_ptr<PipelineHandler> PipelineHandlerFactory::create(CameraManager *manager) const
>   {
>   	PipelineHandler *handler = createInstance(manager);
>   	handler->name_ = name_.c_str();
> @@ -719,7 +719,7 @@ std::vector<PipelineHandlerFactory *> &PipelineHandlerFactory::factories()
>   }
>   
>   /**
> - * \fn PipelineHandlerFactory::createInstance()
> + * \fn PipelineHandlerFactory::createInstance() const
>    * \brief Create an instance of the PipelineHandler corresponding to the factory
>    * \param[in] manager The camera manager
>    *


Reviewed-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>
Jacopo Mondi Oct. 7, 2022, 2:10 p.m. UTC | #2 
Hi Laurent

On Tue, Oct 04, 2022 at 12:21:25AM +0300, Laurent Pinchart via libcamera-devel wrote:
> The PipelineHandlerFactory::create() function has no need to modify the
> factory instance. Make it const, as well as the createInstance()
> function.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Looks legit
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> ---
>  include/libcamera/internal/pipeline_handler.h | 6 +++---
>  src/libcamera/pipeline_handler.cpp            | 4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index 20f1cbb07fea..46df69dc789a 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -104,7 +104,7 @@ public:
>  	PipelineHandlerFactory(const char *name);
>  	virtual ~PipelineHandlerFactory() = default;
>
> -	std::shared_ptr<PipelineHandler> create(CameraManager *manager);
> +	std::shared_ptr<PipelineHandler> create(CameraManager *manager) const;
>
>  	const std::string &name() const { return name_; }
>
> @@ -112,7 +112,7 @@ public:
>  	static std::vector<PipelineHandlerFactory *> &factories();
>
>  private:
> -	virtual PipelineHandler *createInstance(CameraManager *manager) = 0;
> +	virtual PipelineHandler *createInstance(CameraManager *manager) const = 0;
>
>  	std::string name_;
>  };
> @@ -124,7 +124,7 @@ public:									\
>  	handler##Factory() : PipelineHandlerFactory(#handler) {}	\
>  									\
>  private:								\
> -	PipelineHandler *createInstance(CameraManager *manager)		\
> +	PipelineHandler *createInstance(CameraManager *manager) const	\
>  	{								\
>  		return new handler(manager);				\
>  	}								\
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index e5cb751cc743..4470e9041e8e 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -676,7 +676,7 @@ PipelineHandlerFactory::PipelineHandlerFactory(const char *name)
>   * \return A shared pointer to a new instance of the PipelineHandler subclass
>   * corresponding to the factory
>   */
> -std::shared_ptr<PipelineHandler> PipelineHandlerFactory::create(CameraManager *manager)
> +std::shared_ptr<PipelineHandler> PipelineHandlerFactory::create(CameraManager *manager) const
>  {
>  	PipelineHandler *handler = createInstance(manager);
>  	handler->name_ = name_.c_str();
> @@ -719,7 +719,7 @@ std::vector<PipelineHandlerFactory *> &PipelineHandlerFactory::factories()
>  }
>
>  /**
> - * \fn PipelineHandlerFactory::createInstance()
> + * \fn PipelineHandlerFactory::createInstance() const
>   * \brief Create an instance of the PipelineHandler corresponding to the factory
>   * \param[in] manager The camera manager
>   *
> --
> Regards,
>
> Laurent Pinchart
>

Patch
diff mbox series 

diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
index 20f1cbb07fea..46df69dc789a 100644
--- a/include/libcamera/internal/pipeline_handler.h
+++ b/include/libcamera/internal/pipeline_handler.h
@@ -104,7 +104,7 @@  public:
 	PipelineHandlerFactory(const char *name);
 	virtual ~PipelineHandlerFactory() = default;
 
-	std::shared_ptr<PipelineHandler> create(CameraManager *manager);
+	std::shared_ptr<PipelineHandler> create(CameraManager *manager) const;
 
 	const std::string &name() const { return name_; }
 
@@ -112,7 +112,7 @@  public:
 	static std::vector<PipelineHandlerFactory *> &factories();
 
 private:
-	virtual PipelineHandler *createInstance(CameraManager *manager) = 0;
+	virtual PipelineHandler *createInstance(CameraManager *manager) const = 0;
 
 	std::string name_;
 };
@@ -124,7 +124,7 @@  public:									\
 	handler##Factory() : PipelineHandlerFactory(#handler) {}	\
 									\
 private:								\
-	PipelineHandler *createInstance(CameraManager *manager)		\
+	PipelineHandler *createInstance(CameraManager *manager) const	\
 	{								\
 		return new handler(manager);				\
 	}								\
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index e5cb751cc743..4470e9041e8e 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -676,7 +676,7 @@  PipelineHandlerFactory::PipelineHandlerFactory(const char *name)
  * \return A shared pointer to a new instance of the PipelineHandler subclass
  * corresponding to the factory
  */
-std::shared_ptr<PipelineHandler> PipelineHandlerFactory::create(CameraManager *manager)
+std::shared_ptr<PipelineHandler> PipelineHandlerFactory::create(CameraManager *manager) const
 {
 	PipelineHandler *handler = createInstance(manager);
 	handler->name_ = name_.c_str();
@@ -719,7 +719,7 @@  std::vector<PipelineHandlerFactory *> &PipelineHandlerFactory::factories()
 }
 
 /**
- * \fn PipelineHandlerFactory::createInstance()
+ * \fn PipelineHandlerFactory::createInstance() const
  * \brief Create an instance of the PipelineHandler corresponding to the factory
  * \param[in] manager The camera manager
  *