[libcamera-devel] [PATCH] libcamera: pipeline_handler: Reorder member declaration order

[Niklas Söderlund]

Hi Laurent,

Thanks for your work.

On 2019-02-09 01:01:38 +0200, Laurent Pinchart wrote:
> Reorder the member declaration order in the PipelineHandler class to
> match the control flow order, and to declare variables after methods
> according to the coding style. Update the documentation accordingly,
> preserving the order within the public, protected and private sections,
> but grouping related methods together between the sections.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> ---
>  src/libcamera/include/pipeline_handler.h |  10 +--
>  src/libcamera/pipeline_handler.cpp       | 101 +++++++++++------------
>  2 files changed, 55 insertions(+), 56 deletions(-)
> 
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index 7f2ec2975db0..4363dcd8ed8e 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -42,6 +42,8 @@ public:
>  	PipelineHandler(CameraManager *manager);
>  	virtual ~PipelineHandler();
>  
> +	virtual bool match(DeviceEnumerator *enumerator) = 0;
> +
>  	virtual std::map<Stream *, StreamConfiguration>
>  	streamConfiguration(Camera *camera, std::vector<Stream *> &streams) = 0;
>  	virtual int configureStreams(Camera *camera,
> @@ -55,20 +57,18 @@ public:
>  
>  	virtual int queueRequest(const Camera *camera, Request *request) = 0;
>  
> -	virtual bool match(DeviceEnumerator *enumerator) = 0;
> -
>  protected:
> -	CameraManager *manager_;
> -
>  	void registerCamera(std::shared_ptr<Camera> camera);
>  	void hotplugMediaDevice(MediaDevice *media);
>  
>  	CameraData *cameraData(const Camera *camera);
>  	void setCameraData(const Camera *camera, std::unique_ptr<CameraData> data);
>  
> +	CameraManager *manager_;
> +
>  private:
> -	virtual void disconnect();
>  	void mediaDeviceDisconnected(MediaDevice *media);
> +	virtual void disconnect();
>  
>  	std::vector<std::weak_ptr<Camera>> cameras_;
>  	std::map<const Camera *, std::unique_ptr<CameraData>> cameraData_;
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index cc2d4643ec2f..4e111d6d2f55 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -75,6 +75,36 @@ PipelineHandler::~PipelineHandler()
>  {
>  };
>  
> +/**
> + * \fn PipelineHandler::match(DeviceEnumerator *enumerator)
> + * \brief Match media devices and create camera instances
> + * \param enumerator The enumerator providing all media devices found in the
> + * system
> + *
> + * This function is the main entry point of the pipeline handler. It is called
> + * by the camera manager with the \a enumerator passed as an argument. It shall
> + * acquire from the \a enumerator all the media devices it needs for a single
> + * pipeline, create one or multiple Camera instances and register them with the
> + * camera manager.
> + *
> + * If all media devices needed by the pipeline handler are found, they must all
> + * be acquired by a call to MediaDevice::acquire(). This function shall then
> + * create the corresponding Camera instances, store them internally, and return
> + * true. Otherwise it shall not acquire any media device (or shall release all
> + * the media devices is has acquired by calling MediaDevice::release()) and
> + * return false.
> + *
> + * If multiple instances of a pipeline are available in the system, the
> + * PipelineHandler class will be instanciated once per instance, and its match()
> + * function called for every instance. Each call shall acquire media devices for
> + * one pipeline instance, until all compatible media devices are exhausted.
> + *
> + * If this function returns true, a new instance of the pipeline handler will
> + * be created and its match() function called.
> + *
> + * \return true if media devices have been acquired and camera instances
> + * created, or false otherwise
> + */
>  
>  /**
>   * \fn PipelineHandler::streamConfiguration()
> @@ -176,46 +206,6 @@ PipelineHandler::~PipelineHandler()
>   * \return 0 on success or a negative error code on error
>   */
>  
> -/**
> - * \fn PipelineHandler::match(DeviceEnumerator *enumerator)
> - * \brief Match media devices and create camera instances
> - * \param enumerator The enumerator providing all media devices found in the
> - * system
> - *
> - * This function is the main entry point of the pipeline handler. It is called
> - * by the camera manager with the \a enumerator passed as an argument. It shall
> - * acquire from the \a enumerator all the media devices it needs for a single
> - * pipeline, create one or multiple Camera instances and register them with the
> - * camera manager.
> - *
> - * If all media devices needed by the pipeline handler are found, they must all
> - * be acquired by a call to MediaDevice::acquire(). This function shall then
> - * create the corresponding Camera instances, store them internally, and return
> - * true. Otherwise it shall not acquire any media device (or shall release all
> - * the media devices is has acquired by calling MediaDevice::release()) and
> - * return false.
> - *
> - * If multiple instances of a pipeline are available in the system, the
> - * PipelineHandler class will be instanciated once per instance, and its match()
> - * function called for every instance. Each call shall acquire media devices for
> - * one pipeline instance, until all compatible media devices are exhausted.
> - *
> - * If this function returns true, a new instance of the pipeline handler will
> - * be created and its match() function called.
> - *
> - * \return true if media devices have been acquired and camera instances
> - * created, or false otherwise
> - */
> -
> -/**
> - * \var PipelineHandler::manager_
> - * \brief The Camera manager associated with the pipeline handler
> - *
> - * The camera manager pointer is stored in the pipeline handler for the
> - * convenience of pipeline handler implementations. It remains valid and
> - * constant for the whole lifetime of the pipeline handler.
> - */
> -
>  /**
>   * \brief Register a camera to the camera manager and pipeline handler
>   * \param[in] camera The camera to be added
> @@ -246,6 +236,17 @@ void PipelineHandler::hotplugMediaDevice(MediaDevice *media)
>  	media->disconnected.connect(this, &PipelineHandler::mediaDeviceDisconnected);
>  }
>  
> +/**
> + * \brief Slot for the MediaDevice disconnected signal
> + */
> +void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media)
> +{
> +	if (cameras_.empty())
> +		return;
> +
> +	disconnect();
> +}
> +
>  /**
>   * \brief Device disconnection handler
>   *
> @@ -272,17 +273,6 @@ void PipelineHandler::disconnect()
>  	cameras_.clear();
>  }
>  
> -/**
> - * \brief Slot for the MediaDevice disconnected signal
> - */
> -void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media)
> -{
> -	if (cameras_.empty())
> -		return;
> -
> -	disconnect();
> -}
> -
>  /**
>   * \brief Retrieve the pipeline-specific data associated with a Camera
>   * \param camera The camera whose data to retrieve
> @@ -331,6 +321,15 @@ void PipelineHandler::setCameraData(const Camera *camera,
>  	cameraData_[camera] = std::move(data);
>  }
>  
> +/**
> + * \var PipelineHandler::manager_
> + * \brief The Camera manager associated with the pipeline handler
> + *
> + * The camera manager pointer is stored in the pipeline handler for the
> + * convenience of pipeline handler implementations. It remains valid and
> + * constant for the whole lifetime of the pipeline handler.
> + */
> +
>  /**
>   * \class PipelineHandlerFactory
>   * \brief Registration of PipelineHandler classes and creation of instances
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund