[libcamera-devel] [PATCH 3/5] libcamera: ipu3: Break-out ipu3 header file

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2019-02-20 14:17:55 +0100, Jacopo Mondi wrote:
> As the class grows, break out the class definitions in a separate header
> file, which can be used by other ipu3-related cpp files that will be
> added in next commits.

I would mention that there is no other change in this commit then moving 
the class definition to a separate header file.

> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 56 +-----------------
>  src/libcamera/pipeline/ipu3/ipu3.h   | 85 ++++++++++++++++++++++++++++
>  2 files changed, 86 insertions(+), 55 deletions(-)
>  create mode 100644 src/libcamera/pipeline/ipu3/ipu3.h
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 9065073913a2..07029dd763c9 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -14,6 +14,7 @@
>  
>  #include "device_enumerator.h"
>  #include "log.h"
> +#include "ipu3.h"
>  #include "media_device.h"
>  #include "pipeline_handler.h"
>  #include "utils.h"
> @@ -24,61 +25,6 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(IPU3)
>  
> -class PipelineHandlerIPU3 : public PipelineHandler
> -{
> -public:
> -	PipelineHandlerIPU3(CameraManager *manager);
> -	~PipelineHandlerIPU3();
> -
> -	std::map<Stream *, StreamConfiguration>
> -	streamConfiguration(Camera *camera,
> -			    std::vector<Stream *> &streams) override;
> -	int configureStreams(Camera *camera,
> -			     std::map<Stream *, StreamConfiguration> &config) override;
> -
> -	int allocateBuffers(Camera *camera, Stream *stream) override;
> -	int freeBuffers(Camera *camera, Stream *stream) override;
> -
> -	int start(const Camera *camera) override;
> -	void stop(const Camera *camera) override;
> -
> -	int queueRequest(const Camera *camera, Request *request) override;
> -
> -	bool match(DeviceEnumerator *enumerator);
> -
> -private:
> -	class IPU3CameraData : public CameraData
> -	{
> -	public:
> -		IPU3CameraData()
> -			: cio2_(nullptr), csi2_(nullptr), sensor_(nullptr) {}
> -
> -		~IPU3CameraData()
> -		{
> -			delete cio2_;
> -			delete csi2_;
> -			delete sensor_;
> -		}
> -
> -		V4L2Device *cio2_;
> -		V4L2Subdevice *csi2_;
> -		V4L2Subdevice *sensor_;
> -
> -		Stream stream_;
> -	};
> -
> -	IPU3CameraData *cameraData(const Camera *camera)
> -	{
> -		return static_cast<IPU3CameraData *>(
> -			PipelineHandler::cameraData(camera));
> -	}
> -
> -	void registerCameras();
> -
> -	std::shared_ptr<MediaDevice> cio2_;
> -	std::shared_ptr<MediaDevice> imgu_;
> -};
> -
>  PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager)
>  	: PipelineHandler(manager), cio2_(nullptr), imgu_(nullptr)
>  {
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.h b/src/libcamera/pipeline/ipu3/ipu3.h
> new file mode 100644
> index 000000000000..48c2a3e16980
> --- /dev/null
> +++ b/src/libcamera/pipeline/ipu3/ipu3.h
> @@ -0,0 +1,85 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * ipu3.h - Pipeline handler for Intel IPU3
> + */
> +
> +#ifndef __LIBCAMERA_PIPELINE_IPU3_H__
> +#define __LIBCAMERA_PIPELINE_IPU3_H__
> +
> +#include <memory>
> +#include <vector>
> +
> +#include <libcamera/camera.h>
> +#include <libcamera/request.h>
> +#include <libcamera/stream.h>
> +
> +#include "device_enumerator.h"
> +#include "media_device.h"
> +#include "pipeline_handler.h"
> +#include "v4l2_device.h"
> +#include "v4l2_subdevice.h"

Do you really need to include all these headers here? Is it not enough 
to include the bulk of them in the cpp file?

> +
> +namespace libcamera {
> +
> +class PipelineHandlerIPU3 : public PipelineHandler
> +{
> +public:
> +	PipelineHandlerIPU3(CameraManager *manager);
> +	~PipelineHandlerIPU3();
> +
> +	std::map<Stream *, StreamConfiguration>
> +	streamConfiguration(Camera *camera,
> +			    std::vector<Stream *> &streams) override;
> +	int configureStreams(Camera *camera,
> +			     std::map<Stream *, StreamConfiguration> &config) override;
> +
> +	int allocateBuffers(Camera *camera, Stream *stream) override;
> +	int freeBuffers(Camera *camera, Stream *stream) override;
> +
> +	int start(const Camera *camera) override;
> +	void stop(const Camera *camera) override;
> +
> +	int queueRequest(const Camera *camera, Request *request) override;
> +
> +	bool match(DeviceEnumerator *enumerator);
> +
> +private:
> +	class IPU3CameraData : public CameraData
> +	{
> +	public:
> +		IPU3CameraData()
> +			: cio2_(nullptr), csi2_(nullptr), sensor_(nullptr) {}
> +
> +		~IPU3CameraData()
> +		{
> +			delete cio2_;
> +			delete csi2_;
> +			delete sensor_;
> +		}
> +
> +		V4L2Device *cio2_;
> +		V4L2Subdevice *csi2_;
> +		V4L2Subdevice *sensor_;
> +
> +		Stream stream_;
> +	};
> +
> +	IPU3CameraData *cameraData(const Camera *camera)
> +	{
> +		return static_cast<IPU3CameraData *>(
> +			PipelineHandler::cameraData(camera));
> +	}
> +
> +	void registerCameras();
> +
> +	std::shared_ptr<MediaDevice> cio2_;
> +	std::shared_ptr<MediaDevice> imgu_;
> +	IMGUDevice imgu0_;
> +	IMGUDevice imgu1_;
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_PIPELINE_IPU3_H__ */
> -- 
> 2.20.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund