[libcamera-devel] [PATCH v3 02/10] libcamera: pipeline: add name

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Tue, Jun 04, 2019 at 08:53:08PM -0400, Paul Elder wrote:
> In order to match an IPA module with a pipeline handler, the pipeline
> handler must have a name. Add a name attribute and getter to
> PipelineHandler such that it can automatically be defined by
> REGISTER_PIPELINE_HANDLER.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
> Changes in v3:
> - remove version completely from the pipeline hander's side, as the
>   pipeline handler will now specify its acceptable version range when it
>   asks the IPAManager for an IPAInterface (so IPAModules still need a
>   pipeline version)
> - nicer, less intrusive way of setting the pipeline names by making the
>   pipeline handler factory a friend class of pipeline handler
> 
> Changes in v2:
> - make the name and version getters into methods of the base PipelineHandler
>   class, so that the specialized pipeline handlers no longer have to
>   specify overriding
> - add PIPELINE_VERSION macro to create one version number from major and
>   minor pair
> 
>  src/libcamera/include/pipeline_handler.h | 14 +++++++++++++-
>  src/libcamera/pipeline_handler.cpp       | 17 +++++++++++++++++
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index 7da6df1..84307e4 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -77,6 +77,8 @@ public:
>  	bool completeBuffer(Camera *camera, Request *request, Buffer *buffer);
>  	void completeRequest(Camera *camera, Request *request);
>  
> +	const char *name() const { return name_; }
> +
>  protected:
>  	void registerCamera(std::shared_ptr<Camera> camera,
>  			    std::unique_ptr<CameraData> data);
> @@ -93,6 +95,10 @@ private:
>  	std::vector<std::shared_ptr<MediaDevice>> mediaDevices_;
>  	std::vector<std::weak_ptr<Camera>> cameras_;
>  	std::map<const Camera *, std::unique_ptr<CameraData>> cameraData_;
> +
> +	const char *name_;
> +
> +	friend class PipelineHandlerFactory;
>  };
>  
>  class PipelineHandlerFactory
> @@ -108,6 +114,9 @@ public:
>  	static void registerType(PipelineHandlerFactory *factory);
>  	static std::vector<PipelineHandlerFactory *> &factories();
>  
> +protected:
> +	void setInfo(PipelineHandler *handler, const char *name);
> +
>  private:
>  	std::string name_;
>  };
> @@ -119,7 +128,10 @@ public:									\
>  	handler##Factory() : PipelineHandlerFactory(#handler) {}	\
>  	std::shared_ptr<PipelineHandler> create(CameraManager *manager)	\
>  	{								\
> -		return std::make_shared<handler>(manager);		\
> +		std::shared_ptr<handler> h =				\
> +			std::make_shared<handler>(manager);		\
> +		setInfo(h.get(), #handler);				\
> +		return h;						\
>  	}								\
>  };									\
>  static handler##Factory global_##handler##Factory;
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index dd56907..800931d 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -505,6 +505,12 @@ CameraData *PipelineHandler::cameraData(const Camera *camera)
>   * constant for the whole lifetime of the pipeline handler.
>   */
>  
> +/**
> + * \fn PipelineHandler::name()
> + * \brief Retrieve the pipeline handler name
> + * \return The pipeline handler name
> + */
> +
>  /**
>   * \class PipelineHandlerFactory
>   * \brief Registration of PipelineHandler classes and creation of instances
> @@ -582,6 +588,17 @@ std::vector<PipelineHandlerFactory *> &PipelineHandlerFactory::factories()
>  	return factories;
>  }
>  
> +/**
> + * \brief Set the information of a given pipeline handler
> + * \param[in] handler The handler whose info is to be set
> + * \param[in] name The name of the pipeline handler
> + */
> +void PipelineHandlerFactory::setInfo(PipelineHandler *handler,
> +				     const char *name)
> +{
> +	handler->name_ = name;
> +}
> +
>  /**
>   * \def REGISTER_PIPELINE_HANDLER
>   * \brief Register a pipeline handler with the pipeline handler factory

-- 
Regards,

Laurent Pinchart