[libcamera-devel,RFC,1/7] libcamera: camera_sensor: Introduce CameraSensorFactory
diff mbox series

Message ID 20191218145001.22283-2-jacopo@jmondi.org
State Superseded
Delegated to: Jacopo Mondi
Headers show
Series
  • Define and register 'sensor' and 'lens' properties
Related show

Commit Message

Jacopo Mondi Dec. 18, 2019, 2:49 p.m. UTC 
Introduce a factory to create CameraSensor derived classes instances by
inspecting the sensor media entity name.

For now, unconditionally create and return an instance of the only
existing CameraSensor class in CameraSensorFactory::create()

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/camera_sensor.cpp               | 22 +++++++++++++++++++
 src/libcamera/include/camera_sensor.h         | 10 ++++++++-
 src/libcamera/pipeline/ipu3/ipu3.cpp          |  2 +-
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  2 +-
 src/libcamera/pipeline/vimc.cpp               |  2 +-
 test/camera-sensor.cpp                        |  2 +-
 .../v4l2_videodevice_test.cpp                 |  2 +-
 7 files changed, 36 insertions(+), 6 deletions(-)

Comments

Niklas Söderlund Jan. 15, 2020, 7:53 p.m. UTC | #1 
Hi Jacopo,

Thanks for your work.

On 2019-12-18 15:49:55 +0100, Jacopo Mondi wrote:
> Introduce a factory to create CameraSensor derived classes instances by
> inspecting the sensor media entity name.
> 
> For now, unconditionally create and return an instance of the only
> existing CameraSensor class in CameraSensorFactory::create()
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/camera_sensor.cpp               | 22 +++++++++++++++++++
>  src/libcamera/include/camera_sensor.h         | 10 ++++++++-
>  src/libcamera/pipeline/ipu3/ipu3.cpp          |  2 +-
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  2 +-
>  src/libcamera/pipeline/vimc.cpp               |  2 +-
>  test/camera-sensor.cpp                        |  2 +-
>  .../v4l2_videodevice_test.cpp                 |  2 +-
>  7 files changed, 36 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index bc8b9e78bc42..ac8878fe336e 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -28,6 +28,28 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(CameraSensor);
>  
> +/**
> + * \class CameraSensorFactory
> + * \brief Factory of camera sensor
> + *
> + * The CameraSensorFactory instantiate and return the opportune CameraSensor
> + * sub-class by inspecting the name of the sensor entity to handle.
> + */
> +
> +/**
> + * \brief Create and return a CameraSensor
> + * \param[in] entity The media entity that represents the sensor to handle
> + *
> + * Instantiate and return the opportune CameraSensor subclass inspecting the
> + * \a entity name.
> + *
> + * \return A pointer to a CameraSensor derived class instance
> + */
> +CameraSensor *CameraSensorFactory::create(const MediaEntity *entity)
> +{
> +	return new CameraSensor(entity);
> +}
> +
>  /**
>   * \class CameraSensor
>   * \brief A camera sensor based on V4L2 subdevices
> diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h
> index 99cff98128dc..2c09b1bdb61b 100644
> --- a/src/libcamera/include/camera_sensor.h
> +++ b/src/libcamera/include/camera_sensor.h
> @@ -22,10 +22,16 @@ class V4L2Subdevice;
>  
>  struct V4L2SubdeviceFormat;
>  
> +class CameraSensor;
> +class CameraSensorFactory final
> +{
> +public:
> +	static CameraSensor *create(const MediaEntity *entity);
> +};

I Wonder if it would not make more sens to move this this to 
CameraSensor::create(). Looking at our code this seems to be the pattern 
used in most places.

> +
>  class CameraSensor : protected Loggable
>  {
>  public:
> -	explicit CameraSensor(const MediaEntity *entity);
>  	~CameraSensor();
>  
>  	CameraSensor(const CameraSensor &) = delete;
> @@ -49,6 +55,8 @@ public:
>  	const ControlList &properties() const { return properties_; }
>  
>  protected:
> +	friend class CameraSensorFactory;
> +	explicit CameraSensor(const MediaEntity *entity);

Do we need to keep explicit here?

>  	std::string logPrefix() const;
>  
>  private:
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 536a63a30018..6de2f548fe1c 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -1362,7 +1362,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)
>  
>  	MediaLink *link = links[0];
>  	MediaEntity *sensorEntity = link->source()->entity();
> -	sensor_ = new CameraSensor(sensorEntity);
> +	sensor_ = CameraSensorFactory::create(sensorEntity);
>  	ret = sensor_->init();
>  	if (ret)
>  		return ret;
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index e9a70755f4c5..276a12755f59 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -883,7 +883,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>  
>  	data->controlInfo_ = std::move(ctrls);
>  
> -	data->sensor_ = new CameraSensor(sensor);
> +	data->sensor_ = CameraSensorFactory::create(sensor);
>  	ret = data->sensor_->init();
>  	if (ret)
>  		return ret;
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 2bfab35314c4..5d6baa56ba8b 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -403,7 +403,7 @@ int VimcCameraData::init(MediaDevice *media)
>  		return ret;
>  
>  	/* Create and open the camera sensor, debayer, scaler and video device. */
> -	sensor_ = new CameraSensor(media->getEntityByName("Sensor B"));
> +	sensor_ = CameraSensorFactory::create(media->getEntityByName("Sensor B"));
>  	ret = sensor_->init();
>  	if (ret)
>  		return ret;
> diff --git a/test/camera-sensor.cpp b/test/camera-sensor.cpp
> index 27c190fe7ace..0e7e3f5585d2 100644
> --- a/test/camera-sensor.cpp
> +++ b/test/camera-sensor.cpp
> @@ -50,7 +50,7 @@ protected:
>  			return TestFail;
>  		}
>  
> -		sensor_ = new CameraSensor(entity);
> +		sensor_ = CameraSensorFactory::create(entity);
>  		if (sensor_->init() < 0) {
>  			cerr << "Unable to initialise camera sensor" << endl;
>  			return TestFail;
> diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp
> index 577da4cb601c..3c4ca60b9076 100644
> --- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp
> +++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp
> @@ -61,7 +61,7 @@ int V4L2VideoDeviceTest::init()
>  		return TestFail;
>  
>  	if (driver_ == "vimc") {
> -		sensor_ = new CameraSensor(media_->getEntityByName("Sensor A"));
> +		sensor_ = CameraSensorFactory::create(media_->getEntityByName("Sensor A"));
>  		if (sensor_->init())
>  			return TestFail;
>  
> -- 
> 2.24.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
index bc8b9e78bc42..ac8878fe336e 100644
--- a/src/libcamera/camera_sensor.cpp
+++ b/src/libcamera/camera_sensor.cpp
@@ -28,6 +28,28 @@  namespace libcamera {
 
 LOG_DEFINE_CATEGORY(CameraSensor);
 
+/**
+ * \class CameraSensorFactory
+ * \brief Factory of camera sensor
+ *
+ * The CameraSensorFactory instantiate and return the opportune CameraSensor
+ * sub-class by inspecting the name of the sensor entity to handle.
+ */
+
+/**
+ * \brief Create and return a CameraSensor
+ * \param[in] entity The media entity that represents the sensor to handle
+ *
+ * Instantiate and return the opportune CameraSensor subclass inspecting the
+ * \a entity name.
+ *
+ * \return A pointer to a CameraSensor derived class instance
+ */
+CameraSensor *CameraSensorFactory::create(const MediaEntity *entity)
+{
+	return new CameraSensor(entity);
+}
+
 /**
  * \class CameraSensor
  * \brief A camera sensor based on V4L2 subdevices
diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h
index 99cff98128dc..2c09b1bdb61b 100644
--- a/src/libcamera/include/camera_sensor.h
+++ b/src/libcamera/include/camera_sensor.h
@@ -22,10 +22,16 @@  class V4L2Subdevice;
 
 struct V4L2SubdeviceFormat;
 
+class CameraSensor;
+class CameraSensorFactory final
+{
+public:
+	static CameraSensor *create(const MediaEntity *entity);
+};
+
 class CameraSensor : protected Loggable
 {
 public:
-	explicit CameraSensor(const MediaEntity *entity);
 	~CameraSensor();
 
 	CameraSensor(const CameraSensor &) = delete;
@@ -49,6 +55,8 @@  public:
 	const ControlList &properties() const { return properties_; }
 
 protected:
+	friend class CameraSensorFactory;
+	explicit CameraSensor(const MediaEntity *entity);
 	std::string logPrefix() const;
 
 private:
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 536a63a30018..6de2f548fe1c 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -1362,7 +1362,7 @@  int CIO2Device::init(const MediaDevice *media, unsigned int index)
 
 	MediaLink *link = links[0];
 	MediaEntity *sensorEntity = link->source()->entity();
-	sensor_ = new CameraSensor(sensorEntity);
+	sensor_ = CameraSensorFactory::create(sensorEntity);
 	ret = sensor_->init();
 	if (ret)
 		return ret;
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index e9a70755f4c5..276a12755f59 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -883,7 +883,7 @@  int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
 
 	data->controlInfo_ = std::move(ctrls);
 
-	data->sensor_ = new CameraSensor(sensor);
+	data->sensor_ = CameraSensorFactory::create(sensor);
 	ret = data->sensor_->init();
 	if (ret)
 		return ret;
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index 2bfab35314c4..5d6baa56ba8b 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -403,7 +403,7 @@  int VimcCameraData::init(MediaDevice *media)
 		return ret;
 
 	/* Create and open the camera sensor, debayer, scaler and video device. */
-	sensor_ = new CameraSensor(media->getEntityByName("Sensor B"));
+	sensor_ = CameraSensorFactory::create(media->getEntityByName("Sensor B"));
 	ret = sensor_->init();
 	if (ret)
 		return ret;
diff --git a/test/camera-sensor.cpp b/test/camera-sensor.cpp
index 27c190fe7ace..0e7e3f5585d2 100644
--- a/test/camera-sensor.cpp
+++ b/test/camera-sensor.cpp
@@ -50,7 +50,7 @@  protected:
 			return TestFail;
 		}
 
-		sensor_ = new CameraSensor(entity);
+		sensor_ = CameraSensorFactory::create(entity);
 		if (sensor_->init() < 0) {
 			cerr << "Unable to initialise camera sensor" << endl;
 			return TestFail;
diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp
index 577da4cb601c..3c4ca60b9076 100644
--- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp
+++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp
@@ -61,7 +61,7 @@  int V4L2VideoDeviceTest::init()
 		return TestFail;
 
 	if (driver_ == "vimc") {
-		sensor_ = new CameraSensor(media_->getEntityByName("Sensor A"));
+		sensor_ = CameraSensorFactory::create(media_->getEntityByName("Sensor A"));
 		if (sensor_->init())
 			return TestFail;