[libcamera-devel] [PATCH 1/3] libcamera: camera: create a association with the responsible pipeline handler

[Jacopo Mondi]

Hi Niklas,
   thanks for this, very welcome

On Wed, Jan 23, 2019 at 12:29:53AM +0100, Niklas Söderlund wrote:
> The PipelineHandler which creates a Camera is responsible for serving
> any operation requested by the user. In order to do so the Camera needs
> to store a reference to its creator.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  include/libcamera/camera.h           |  8 ++++++--
>  src/libcamera/camera.cpp             | 15 +++++++++------
>  src/libcamera/pipeline/ipu3/ipu3.cpp |  3 ++-
>  src/libcamera/pipeline/uvcvideo.cpp  |  2 +-
>  src/libcamera/pipeline/vimc.cpp      | 10 ++--------
>  5 files changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 2ea1a6883311cf9f..d3bae4cbee1e0cea 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -12,10 +12,13 @@
>
>  namespace libcamera {
>
> +class PipelineHandler;
> +
>  class Camera final
>  {
>  public:
> -	static std::shared_ptr<Camera> create(const std::string &name);
> +	static std::shared_ptr<Camera> create(const std::string &name,
> +					      class PipelineHandler *pipe);
>
>  	Camera(const Camera &) = delete;
>  	void operator=(const Camera &) = delete;
> @@ -23,10 +26,11 @@ public:
>  	const std::string &name() const;
>
>  private:
> -	explicit Camera(const std::string &name);
> +	explicit Camera(const std::string &name, class PipelineHandler *pipe);

You can drop the 'explicit' here, as it avoid copy-construction and
conversion-contruction, which can't happen with 2 parameters (if I got
this right :)

I'm fine with rest, with the minor thing that I feel more natural to
have 'this' passed as first parameter of the Camera constructor.

Thanks
  j

>  	~Camera();
>
>  	std::string name_;
> +	class PipelineHandler *pipe_;
>  };
>
>  } /* namespace libcamera */
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index acf912bee95cbec4..f198eb4978b12239 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -52,17 +52,20 @@ namespace libcamera {
>  /**
>   * \brief Create a camera instance
>   * \param[in] name The name of the camera device
> + * \param[in] pipe The pipeline handler responsible for the camera device
>   *
>   * The caller is responsible for guaranteeing unicity of the camera name.
>   *
>   * \return A shared pointer to the newly created camera object
>   */
> -std::shared_ptr<Camera> Camera::create(const std::string &name)
> +std::shared_ptr<Camera> Camera::create(const std::string &name,
> +				       class PipelineHandler *pipe)
>  {
>  	struct Allocator : std::allocator<Camera> {
> -		void construct(void *p, const std::string &name)
> +		void construct(void *p, const std::string &name,
> +			       class PipelineHandler *pipe)
>  		{
> -			::new(p) Camera(name);
> +			::new(p) Camera(name, pipe);
>  		}
>  		void destroy(Camera *p)
>  		{
> @@ -70,7 +73,7 @@ std::shared_ptr<Camera> Camera::create(const std::string &name)
>  		}
>  	};
>
> -	return std::allocate_shared<Camera>(Allocator(), name);
> +	return std::allocate_shared<Camera>(Allocator(), name, pipe);
>  }
>
>  /**
> @@ -83,8 +86,8 @@ const std::string &Camera::name() const
>  	return name_;
>  }
>
> -Camera::Camera(const std::string &name)
> -	: name_(name)
> +Camera::Camera(const std::string &name, class PipelineHandler *pipe)
> +	: name_(name), pipe_(pipe)
>  {
>  }
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 8cbc10acfbb571fd..48d028f7e6cd9b4d 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -171,7 +171,8 @@ void PipelineHandlerIPU3::registerCameras(CameraManager *manager)
>  			continue;
>
>  		std::string cameraName = sensor->name() + " " + std::to_string(id);
> -		std::shared_ptr<Camera> camera = Camera::create(cameraName);
> +		std::shared_ptr<Camera> camera = Camera::create(cameraName,
> +								this);
>  		manager->addCamera(std::move(camera));
>
>  		LOG(IPU3, Info)
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 3ba69da8b77586e3..3651250b683e7810 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -48,7 +48,7 @@ bool PipelineHandlerUVC::match(CameraManager *manager, DeviceEnumerator *enumera
>
>  	dev_->acquire();
>
> -	std::shared_ptr<Camera> camera = Camera::create(dev_->model());
> +	std::shared_ptr<Camera> camera = Camera::create(dev_->model(), this);
>  	manager->addCamera(std::move(camera));
>
>  	return true;
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 82b9237a3d7d93e5..81d8319eb88e06d2 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -57,14 +57,8 @@ bool PipeHandlerVimc::match(CameraManager *manager, DeviceEnumerator *enumerator
>
>  	dev_->acquire();
>
> -	/*
> -	 * NOTE: A more complete Camera implementation could
> -	 * be passed the MediaDevice(s) it controls here or
> -	 * a reference to the PipelineHandler. Which method
> -	 * will be chosen depends on how the Camera
> -	 * object is modeled.
> -	 */
> -	std::shared_ptr<Camera> camera = Camera::create("Dummy VIMC Camera");
> +	std::shared_ptr<Camera> camera = Camera::create("Dummy VIMC Camera",
> +							this);
>  	manager->addCamera(std::move(camera));
>
>  	return true;
> --
> 2.20.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190123/d6044842/attachment.sig>