[libcamera-devel] [PATCH v4 2/6] libcamera: camera_manager: Refactor pipelines creation into separate function

[Laurent Pinchart]

Hi Umang,

Thank you for the patch.

On Thu, Jun 11, 2020 at 05:16:04PM +0000, Umang Jain wrote:
> This commit introduces no functional changes.
> Split pipelines creation code into a separate function,
> so that the function can be re-used for upcoming hotplug
> functionality in subsequent commits.
> 
> Also, fixup correct tag for \todo.
> 
> Signed-off-by: Umang Jain <email at uajain.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/libcamera/camera_manager.cpp | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index b8128df..aba5a0c 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -56,6 +56,7 @@ protected:
>  private:
>  	int init();
>  	void cleanup();
> +	void createPipelineHandlers();

We try to declare functions in the same order in the .h and .cpp files
(the rule is a bit more complex, we can interleave public and private
functions in the .cpp file, but within a group of public or private
functions, we retain the order).

You can swap cleanup() and createPipelineHandlers() here, or in the .cpp
file, up to you.

>  
>  	CameraManager *cm_;
>  
> @@ -123,12 +124,20 @@ int CameraManager::Private::init()
>  	if (!enumerator_ || enumerator_->enumerate())
>  		return -ENODEV;
>  
> +	createPipelineHandlers();
> +
> +	return 0;
> +}
> +
> +void CameraManager::Private::createPipelineHandlers()
> +{
>  	/*
> -	 * TODO: Try to read handlers and order from configuration
> +	 * \todo Try to read handlers and order from configuration
>  	 * file and only fallback on all handlers if there is no
>  	 * configuration file.
>  	 */
> -	std::vector<PipelineHandlerFactory *> &factories = PipelineHandlerFactory::factories();
> +	std::vector<PipelineHandlerFactory *> &factories =
> +		PipelineHandlerFactory::factories();
>  
>  	for (PipelineHandlerFactory *factory : factories) {
>  		/*
> @@ -146,14 +155,12 @@ int CameraManager::Private::init()
>  		}
>  	}
>  
> -	/* TODO: register hot-plug callback here */
> -
> -	return 0;
> +	/* \todo Register hot-plug callback here */
>  }
>  
>  void CameraManager::Private::cleanup()
>  {
> -	/* TODO: unregister hot-plug callback here */
> +	/* \todo Unregister hot-plug callback here */
>  
>  	/*
>  	 * Release all references to cameras and pipeline handlers to ensure

-- 
Regards,

Laurent Pinchart