[libcamera-devel,05/10] libcamera: pipeline_handler: Store pipe and camera in CameraData

Message ID 20190228162913.6508-6-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • Rework request completion handling
Related show

Commit Message

Laurent Pinchart Feb. 28, 2019, 4:29 p.m. UTC
Extend the CameraData class with two member variables pipe_ and camera_
that store pointers to the pipeline handler and camera that the
CameraData instance is related to. This will be used by pipeline
handlers to access the camera and the pipeline in member methods of
their CameraData derived classes.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/include/pipeline_handler.h |  9 ++++-
 src/libcamera/pipeline/ipu3/ipu3.cpp     |  9 +++--
 src/libcamera/pipeline/uvcvideo.cpp      |  5 ++-
 src/libcamera/pipeline/vimc.cpp          |  5 ++-
 src/libcamera/pipeline_handler.cpp       | 49 +++++++++++++++++++++---
 5 files changed, 63 insertions(+), 14 deletions(-)

Comments

Niklas Söderlund Feb. 28, 2019, 5:28 p.m. UTC | #1
Hi Laurent,

Thanks for your work.

On 2019-02-28 18:29:08 +0200, Laurent Pinchart wrote:
> Extend the CameraData class with two member variables pipe_ and camera_
> that store pointers to the pipeline handler and camera that the
> CameraData instance is related to. This will be used by pipeline
> handlers to access the camera and the pipeline in member methods of
> their CameraData derived classes.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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

> ---
>  src/libcamera/include/pipeline_handler.h |  9 ++++-
>  src/libcamera/pipeline/ipu3/ipu3.cpp     |  9 +++--
>  src/libcamera/pipeline/uvcvideo.cpp      |  5 ++-
>  src/libcamera/pipeline/vimc.cpp          |  5 ++-
>  src/libcamera/pipeline_handler.cpp       | 49 +++++++++++++++++++++---
>  5 files changed, 63 insertions(+), 14 deletions(-)
> 
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index 45e1f07ffca9..bc4da5820ac4 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -19,6 +19,7 @@ class Camera;
>  class CameraManager;
>  class DeviceEnumerator;
>  class MediaDevice;
> +class PipelineHandler;
>  class Request;
>  class Stream;
>  class StreamConfiguration;
> @@ -26,10 +27,14 @@ class StreamConfiguration;
>  class CameraData
>  {
>  public:
> +	explicit CameraData(PipelineHandler *pipe)
> +		: pipe_(pipe)
> +	{
> +	}
>  	virtual ~CameraData() {}
>  
> -protected:
> -	CameraData() {}
> +	Camera *camera_;
> +	PipelineHandler *pipe_;
>  
>  private:
>  	CameraData(const CameraData &) = delete;
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 248e921117f4..347ee657fddf 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -50,8 +50,11 @@ private:
>  	class IPU3CameraData : public CameraData
>  	{
>  	public:
> -		IPU3CameraData()
> -			: cio2_(nullptr), csi2_(nullptr), sensor_(nullptr) {}
> +		IPU3CameraData(PipelineHandler *pipe)
> +			: CameraData(pipe), cio2_(nullptr), csi2_(nullptr),
> +			  sensor_(nullptr)
> +		{
> +		}
>  
>  		~IPU3CameraData()
>  		{
> @@ -365,7 +368,7 @@ void PipelineHandlerIPU3::registerCameras()
>  		if (link->setEnabled(true))
>  			continue;
>  
> -		std::unique_ptr<IPU3CameraData> data = utils::make_unique<IPU3CameraData>();
> +		std::unique_ptr<IPU3CameraData> data = utils::make_unique<IPU3CameraData>(this);
>  
>  		std::string cameraName = sensor->name() + " " + std::to_string(id);
>  		std::vector<Stream *> streams{ &data->stream_ };
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index cc513513cb34..9af4838891f4 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -46,7 +46,8 @@ private:
>  	class UVCCameraData : public CameraData
>  	{
>  	public:
> -		UVCCameraData()
> +		UVCCameraData(PipelineHandler *pipe)
> +			: CameraData(pipe)
>  		{
>  		}
>  
> @@ -180,7 +181,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
>  
>  	media_->acquire();
>  
> -	std::unique_ptr<UVCCameraData> data = utils::make_unique<UVCCameraData>();
> +	std::unique_ptr<UVCCameraData> data = utils::make_unique<UVCCameraData>(this);
>  
>  	/* Locate and open the default video node. */
>  	for (MediaEntity *entity : media_->entities()) {
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 1d01fa80f8d5..c4c1eb0dc19f 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -46,7 +46,8 @@ private:
>  	class VimcCameraData : public CameraData
>  	{
>  	public:
> -		VimcCameraData()
> +		VimcCameraData(PipelineHandler *pipe)
> +			: CameraData(pipe)
>  		{
>  		}
>  
> @@ -190,7 +191,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
>  
>  	media_->acquire();
>  
> -	std::unique_ptr<VimcCameraData> data = utils::make_unique<VimcCameraData>();
> +	std::unique_ptr<VimcCameraData> data = utils::make_unique<VimcCameraData>(this);
>  
>  	/* Locate and open the capture video node. */
>  	MediaEntity *entity = media_->getEntityByName("Raw Capture 1");
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index c440aa8382ff..ac1acea5a318 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -11,6 +11,7 @@
>  #include "log.h"
>  #include "media_device.h"
>  #include "pipeline_handler.h"
> +#include "utils.h"
>  
>  /**
>   * \file pipeline_handler.h
> @@ -44,6 +45,35 @@ LOG_DEFINE_CATEGORY(Pipeline)
>   * with cameraData().
>   */
>  
> +/**
> + * \fn CameraData::CameraData(PipelineHandler *pipe)
> + * \brief Construct a CameraData instance for the given pipeline handler
> + * \param[in] pipe The pipeline handler
> + *
> + * The reference to the pipeline handler is stored internally, the caller shall
> + * guarantee that the pointer remains valid as long as the CameraData instance
> + * exists.
> + */
> +
> +/**
> + * \var CameraData::camera_
> + * \brief The camera related to this CameraData instance
> + *
> + * The camera_ pointer provides access to the Camera object that this instance
> + * is related to. It is set when the Camera is registered with
> + * PipelineHandler::registerCamera() and remains valid until the CameraData
> + * instance is destroyed.
> + */
> +
> +/**
> + * \var CameraData::pipe_
> + * \brief The pipeline handler related to this CameraData instance
> + *
> + * The pipe_ pointer provides access to the PipelineHandler object that this
> + * instance is related to. It is set when the CameraData instance is created
> + * and remains valid until the instance is destroyed.
> + */
> +
>  /**
>   * \class PipelineHandler
>   * \brief Create and manage cameras based on a set of media devices
> @@ -218,11 +248,19 @@ PipelineHandler::~PipelineHandler()
>   * \brief Register a camera to the camera manager and pipeline handler
>   * \param[in] camera The camera to be added
>   *
> - * This function is called by pipeline handlers to register the cameras they
> - * handle with the camera manager.
> + * This method is called by pipeline handlers to register the cameras they
> + * handle with the camera manager. If no CameraData has been associated with
> + * the camera with setCameraData() by the pipeline handler, the method creates
> + * a default CameraData instance for the \a camera.
>   */
>  void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)
>  {
> +	if (!cameraData_.count(camera.get())) {
> +		std::unique_ptr<CameraData> data = utils::make_unique<CameraData>(this);
> +		setCameraData(camera.get(), std::move(data));
> +	}
> +
> +	cameraData(camera.get())->camera_ = camera.get();
>  	cameras_.push_back(camera);
>  	manager_->addCamera(std::move(camera));
>  }
> @@ -313,9 +351,10 @@ CameraData *PipelineHandler::cameraData(const Camera *camera)
>   * information with \a camera. Ownership of \a data is transferred to
>   * the PipelineHandler.
>   *
> - * Pipeline-specific data can only be set once. Any attempt to call
> - * this method after the first one with the same camera won't change
> - * the pipeline-specific data.
> + * Pipeline-specific data can only be set once, and shall be set before
> + * registering the camera with registerCamera(). Any attempt to call this
> + * method more than once for the same camera, or to call it after registering
> + * the camera, will not change the pipeline-specific data.
>   *
>   * The data can be retrieved by pipeline handlers using the cameraData() method.
>   */
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch

diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
index 45e1f07ffca9..bc4da5820ac4 100644
--- a/src/libcamera/include/pipeline_handler.h
+++ b/src/libcamera/include/pipeline_handler.h
@@ -19,6 +19,7 @@  class Camera;
 class CameraManager;
 class DeviceEnumerator;
 class MediaDevice;
+class PipelineHandler;
 class Request;
 class Stream;
 class StreamConfiguration;
@@ -26,10 +27,14 @@  class StreamConfiguration;
 class CameraData
 {
 public:
+	explicit CameraData(PipelineHandler *pipe)
+		: pipe_(pipe)
+	{
+	}
 	virtual ~CameraData() {}
 
-protected:
-	CameraData() {}
+	Camera *camera_;
+	PipelineHandler *pipe_;
 
 private:
 	CameraData(const CameraData &) = delete;
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 248e921117f4..347ee657fddf 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -50,8 +50,11 @@  private:
 	class IPU3CameraData : public CameraData
 	{
 	public:
-		IPU3CameraData()
-			: cio2_(nullptr), csi2_(nullptr), sensor_(nullptr) {}
+		IPU3CameraData(PipelineHandler *pipe)
+			: CameraData(pipe), cio2_(nullptr), csi2_(nullptr),
+			  sensor_(nullptr)
+		{
+		}
 
 		~IPU3CameraData()
 		{
@@ -365,7 +368,7 @@  void PipelineHandlerIPU3::registerCameras()
 		if (link->setEnabled(true))
 			continue;
 
-		std::unique_ptr<IPU3CameraData> data = utils::make_unique<IPU3CameraData>();
+		std::unique_ptr<IPU3CameraData> data = utils::make_unique<IPU3CameraData>(this);
 
 		std::string cameraName = sensor->name() + " " + std::to_string(id);
 		std::vector<Stream *> streams{ &data->stream_ };
diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
index cc513513cb34..9af4838891f4 100644
--- a/src/libcamera/pipeline/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo.cpp
@@ -46,7 +46,8 @@  private:
 	class UVCCameraData : public CameraData
 	{
 	public:
-		UVCCameraData()
+		UVCCameraData(PipelineHandler *pipe)
+			: CameraData(pipe)
 		{
 		}
 
@@ -180,7 +181,7 @@  bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
 
 	media_->acquire();
 
-	std::unique_ptr<UVCCameraData> data = utils::make_unique<UVCCameraData>();
+	std::unique_ptr<UVCCameraData> data = utils::make_unique<UVCCameraData>(this);
 
 	/* Locate and open the default video node. */
 	for (MediaEntity *entity : media_->entities()) {
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index 1d01fa80f8d5..c4c1eb0dc19f 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -46,7 +46,8 @@  private:
 	class VimcCameraData : public CameraData
 	{
 	public:
-		VimcCameraData()
+		VimcCameraData(PipelineHandler *pipe)
+			: CameraData(pipe)
 		{
 		}
 
@@ -190,7 +191,7 @@  bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
 
 	media_->acquire();
 
-	std::unique_ptr<VimcCameraData> data = utils::make_unique<VimcCameraData>();
+	std::unique_ptr<VimcCameraData> data = utils::make_unique<VimcCameraData>(this);
 
 	/* Locate and open the capture video node. */
 	MediaEntity *entity = media_->getEntityByName("Raw Capture 1");
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index c440aa8382ff..ac1acea5a318 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -11,6 +11,7 @@ 
 #include "log.h"
 #include "media_device.h"
 #include "pipeline_handler.h"
+#include "utils.h"
 
 /**
  * \file pipeline_handler.h
@@ -44,6 +45,35 @@  LOG_DEFINE_CATEGORY(Pipeline)
  * with cameraData().
  */
 
+/**
+ * \fn CameraData::CameraData(PipelineHandler *pipe)
+ * \brief Construct a CameraData instance for the given pipeline handler
+ * \param[in] pipe The pipeline handler
+ *
+ * The reference to the pipeline handler is stored internally, the caller shall
+ * guarantee that the pointer remains valid as long as the CameraData instance
+ * exists.
+ */
+
+/**
+ * \var CameraData::camera_
+ * \brief The camera related to this CameraData instance
+ *
+ * The camera_ pointer provides access to the Camera object that this instance
+ * is related to. It is set when the Camera is registered with
+ * PipelineHandler::registerCamera() and remains valid until the CameraData
+ * instance is destroyed.
+ */
+
+/**
+ * \var CameraData::pipe_
+ * \brief The pipeline handler related to this CameraData instance
+ *
+ * The pipe_ pointer provides access to the PipelineHandler object that this
+ * instance is related to. It is set when the CameraData instance is created
+ * and remains valid until the instance is destroyed.
+ */
+
 /**
  * \class PipelineHandler
  * \brief Create and manage cameras based on a set of media devices
@@ -218,11 +248,19 @@  PipelineHandler::~PipelineHandler()
  * \brief Register a camera to the camera manager and pipeline handler
  * \param[in] camera The camera to be added
  *
- * This function is called by pipeline handlers to register the cameras they
- * handle with the camera manager.
+ * This method is called by pipeline handlers to register the cameras they
+ * handle with the camera manager. If no CameraData has been associated with
+ * the camera with setCameraData() by the pipeline handler, the method creates
+ * a default CameraData instance for the \a camera.
  */
 void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)
 {
+	if (!cameraData_.count(camera.get())) {
+		std::unique_ptr<CameraData> data = utils::make_unique<CameraData>(this);
+		setCameraData(camera.get(), std::move(data));
+	}
+
+	cameraData(camera.get())->camera_ = camera.get();
 	cameras_.push_back(camera);
 	manager_->addCamera(std::move(camera));
 }
@@ -313,9 +351,10 @@  CameraData *PipelineHandler::cameraData(const Camera *camera)
  * information with \a camera. Ownership of \a data is transferred to
  * the PipelineHandler.
  *
- * Pipeline-specific data can only be set once. Any attempt to call
- * this method after the first one with the same camera won't change
- * the pipeline-specific data.
+ * Pipeline-specific data can only be set once, and shall be set before
+ * registering the camera with registerCamera(). Any attempt to call this
+ * method more than once for the same camera, or to call it after registering
+ * the camera, will not change the pipeline-specific data.
  *
  * The data can be retrieved by pipeline handlers using the cameraData() method.
  */