[libcamera-devel,v2,10/13] libcamera: v4l2-subdevice: Add method to retrieve the media entity

Message ID 20190416220839.1577-11-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • Rockchip ISP pipeline handler
Related show

Commit Message

Laurent Pinchart April 16, 2019, 10:08 p.m. UTC
Add a method to retrieve the media entity associated with a subdevice.
The entityName() and deviceNode() methods are not needed anymore as they
can be accessed through the media entity, remove them.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/libcamera/include/v4l2_subdevice.h      |  3 +--
 src/libcamera/pipeline/ipu3/ipu3.cpp        |  4 ++--
 src/libcamera/v4l2_subdevice.cpp            | 22 +++++++--------------
 test/v4l2_subdevice/list_formats.cpp        |  6 +++---
 test/v4l2_subdevice/v4l2_subdevice_test.cpp |  2 +-
 5 files changed, 14 insertions(+), 23 deletions(-)

Comments

Jacopo Mondi April 17, 2019, 11:57 a.m. UTC | #1
Hi Laurent,

On Wed, Apr 17, 2019 at 01:08:36AM +0300, Laurent Pinchart wrote:
> Add a method to retrieve the media entity associated with a subdevice.
> The entityName() and deviceNode() methods are not needed anymore as they
> can be accessed through the media entity, remove them.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

As the purpose of the patch has been clarified in a previous answer
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

> ---
>  src/libcamera/include/v4l2_subdevice.h      |  3 +--
>  src/libcamera/pipeline/ipu3/ipu3.cpp        |  4 ++--
>  src/libcamera/v4l2_subdevice.cpp            | 22 +++++++--------------
>  test/v4l2_subdevice/list_formats.cpp        |  6 +++---
>  test/v4l2_subdevice/v4l2_subdevice_test.cpp |  2 +-
>  5 files changed, 14 insertions(+), 23 deletions(-)
>
> diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
> index e592d67dd043..c7b0b4cb76ea 100644
> --- a/src/libcamera/include/v4l2_subdevice.h
> +++ b/src/libcamera/include/v4l2_subdevice.h
> @@ -41,8 +41,7 @@ public:
>  	bool isOpen() const;
>  	void close();
>
> -	const std::string &deviceNode() const { return entity_->deviceNode(); }
> -	const std::string &entityName() const { return entity_->name(); }
> +	const MediaEntity *entity() const { return entity_; }
>
>  	int setCrop(unsigned int pad, Rectangle *rect);
>  	int setCompose(unsigned int pad, Rectangle *rect);
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 4ddd1ede1c81..e3c79f93963e 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -579,7 +579,7 @@ int PipelineHandlerIPU3::registerCameras()
>  					&IPU3CameraData::imguOutputBufferReady);
>
>  		/* Create and register the Camera instance. */
> -		std::string cameraName = cio2->sensor_->entityName() + " "
> +		std::string cameraName = cio2->sensor_->entity()->name() + " "
>  				       + std::to_string(id);
>  		std::shared_ptr<Camera> camera = Camera::create(this,
>  								cameraName,
> @@ -1055,7 +1055,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)
>  		}
>  	}
>  	if (maxSize_.width == 0) {
> -		LOG(IPU3, Info) << "Sensor '" << sensor_->entityName()
> +		LOG(IPU3, Info) << "Sensor '" << sensor_->entity()->name()
>  				<< "' detected, but no supported image format "
>  				<< " found: skip camera creation";
>  		return -ENODEV;
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index a03fadfd579e..5d5b168f98a2 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -131,12 +131,12 @@ int V4L2Subdevice::open()
>  		return -EBUSY;
>  	}
>
> -	ret = ::open(deviceNode().c_str(), O_RDWR);
> +	ret = ::open(entity_->deviceNode().c_str(), O_RDWR);
>  	if (ret < 0) {
>  		ret = -errno;
>  		LOG(V4L2Subdev, Error)
> -			<< "Failed to open V4L2 subdevice '" << deviceNode()
> -			<< "': " << strerror(-ret);
> +			<< "Failed to open V4L2 subdevice '"
> +			<< entity_->deviceNode() << "': " << strerror(-ret);
>  		return ret;
>  	}
>  	fd_ = ret;
> @@ -166,17 +166,9 @@ void V4L2Subdevice::close()
>  }
>
>  /**
> - * \fn V4L2Subdevice::deviceNode()
> - * \brief Retrieve the path of the device node associated with the subdevice
> - *
> - * \return The subdevice's device node system path
> - */
> -
> -/**
> - * \fn V4L2Subdevice::entityName()
> - * \brief Retrieve the name of the media entity associated with the subdevice
> - *
> - * \return The name of the media entity the subdevice is associated to
> + * \fn V4L2Subdevice::entity()
> + * \brief Retrieve the media entity associated with the subdevice
> + * \return The subdevice's associated media entity.
>   */
>
>  /**
> @@ -343,7 +335,7 @@ V4L2Subdevice *V4L2Subdevice::fromEntityName(const MediaDevice *media,
>
>  std::string V4L2Subdevice::logPrefix() const
>  {
> -	return "'" + entityName() + "'";
> +	return "'" + entity_->name() + "'";
>  }
>
>  int V4L2Subdevice::enumPadSizes(unsigned int pad,unsigned int code,
> diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp
> index 09dec9abe854..18dd8761a8ab 100644
> --- a/test/v4l2_subdevice/list_formats.cpp
> +++ b/test/v4l2_subdevice/list_formats.cpp
> @@ -52,7 +52,7 @@ int ListFormatsTest::run()
>  	formats = scaler_->formats(0);
>  	if (formats.empty()) {
>  		cerr << "Failed to list formats on pad 0 of subdevice "
> -		     << scaler_->entityName() << endl;
> +		     << scaler_->entity()->name() << endl;
>  		return TestFail;
>  	}
>  	for (auto it = formats.begin(); it != formats.end(); ++it)
> @@ -61,7 +61,7 @@ int ListFormatsTest::run()
>  	formats = scaler_->formats(1);
>  	if (formats.empty()) {
>  		cerr << "Failed to list formats on pad 1 of subdevice "
> -		     << scaler_->entityName() << endl;
> +		     << scaler_->entity()->name() << endl;
>  		return TestFail;
>  	}
>  	for (auto it = formats.begin(); it != formats.end(); ++it)
> @@ -71,7 +71,7 @@ int ListFormatsTest::run()
>  	formats = scaler_->formats(2);
>  	if (!formats.empty()) {
>  		cerr << "Listing formats on non-existing pad 2 of subdevice "
> -		     << scaler_->entityName()
> +		     << scaler_->entity()->name()
>  		     << " should return an empty format list" << endl;
>  		return TestFail;
>  	}
> diff --git a/test/v4l2_subdevice/v4l2_subdevice_test.cpp b/test/v4l2_subdevice/v4l2_subdevice_test.cpp
> index 06582969eb45..dfcf779af95e 100644
> --- a/test/v4l2_subdevice/v4l2_subdevice_test.cpp
> +++ b/test/v4l2_subdevice/v4l2_subdevice_test.cpp
> @@ -66,7 +66,7 @@ int V4L2SubdeviceTest::init()
>  	ret = scaler_->open();
>  	if (ret) {
>  		cerr << "Unable to open video subdevice "
> -		     << scaler_->deviceNode() << endl;
> +		     << scaler_->entity()->deviceNode() << endl;
>  		media_->release();
>  		return TestSkip;
>  	}
> --
> 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/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
index e592d67dd043..c7b0b4cb76ea 100644
--- a/src/libcamera/include/v4l2_subdevice.h
+++ b/src/libcamera/include/v4l2_subdevice.h
@@ -41,8 +41,7 @@  public:
 	bool isOpen() const;
 	void close();
 
-	const std::string &deviceNode() const { return entity_->deviceNode(); }
-	const std::string &entityName() const { return entity_->name(); }
+	const MediaEntity *entity() const { return entity_; }
 
 	int setCrop(unsigned int pad, Rectangle *rect);
 	int setCompose(unsigned int pad, Rectangle *rect);
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 4ddd1ede1c81..e3c79f93963e 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -579,7 +579,7 @@  int PipelineHandlerIPU3::registerCameras()
 					&IPU3CameraData::imguOutputBufferReady);
 
 		/* Create and register the Camera instance. */
-		std::string cameraName = cio2->sensor_->entityName() + " "
+		std::string cameraName = cio2->sensor_->entity()->name() + " "
 				       + std::to_string(id);
 		std::shared_ptr<Camera> camera = Camera::create(this,
 								cameraName,
@@ -1055,7 +1055,7 @@  int CIO2Device::init(const MediaDevice *media, unsigned int index)
 		}
 	}
 	if (maxSize_.width == 0) {
-		LOG(IPU3, Info) << "Sensor '" << sensor_->entityName()
+		LOG(IPU3, Info) << "Sensor '" << sensor_->entity()->name()
 				<< "' detected, but no supported image format "
 				<< " found: skip camera creation";
 		return -ENODEV;
diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
index a03fadfd579e..5d5b168f98a2 100644
--- a/src/libcamera/v4l2_subdevice.cpp
+++ b/src/libcamera/v4l2_subdevice.cpp
@@ -131,12 +131,12 @@  int V4L2Subdevice::open()
 		return -EBUSY;
 	}
 
-	ret = ::open(deviceNode().c_str(), O_RDWR);
+	ret = ::open(entity_->deviceNode().c_str(), O_RDWR);
 	if (ret < 0) {
 		ret = -errno;
 		LOG(V4L2Subdev, Error)
-			<< "Failed to open V4L2 subdevice '" << deviceNode()
-			<< "': " << strerror(-ret);
+			<< "Failed to open V4L2 subdevice '"
+			<< entity_->deviceNode() << "': " << strerror(-ret);
 		return ret;
 	}
 	fd_ = ret;
@@ -166,17 +166,9 @@  void V4L2Subdevice::close()
 }
 
 /**
- * \fn V4L2Subdevice::deviceNode()
- * \brief Retrieve the path of the device node associated with the subdevice
- *
- * \return The subdevice's device node system path
- */
-
-/**
- * \fn V4L2Subdevice::entityName()
- * \brief Retrieve the name of the media entity associated with the subdevice
- *
- * \return The name of the media entity the subdevice is associated to
+ * \fn V4L2Subdevice::entity()
+ * \brief Retrieve the media entity associated with the subdevice
+ * \return The subdevice's associated media entity.
  */
 
 /**
@@ -343,7 +335,7 @@  V4L2Subdevice *V4L2Subdevice::fromEntityName(const MediaDevice *media,
 
 std::string V4L2Subdevice::logPrefix() const
 {
-	return "'" + entityName() + "'";
+	return "'" + entity_->name() + "'";
 }
 
 int V4L2Subdevice::enumPadSizes(unsigned int pad,unsigned int code,
diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp
index 09dec9abe854..18dd8761a8ab 100644
--- a/test/v4l2_subdevice/list_formats.cpp
+++ b/test/v4l2_subdevice/list_formats.cpp
@@ -52,7 +52,7 @@  int ListFormatsTest::run()
 	formats = scaler_->formats(0);
 	if (formats.empty()) {
 		cerr << "Failed to list formats on pad 0 of subdevice "
-		     << scaler_->entityName() << endl;
+		     << scaler_->entity()->name() << endl;
 		return TestFail;
 	}
 	for (auto it = formats.begin(); it != formats.end(); ++it)
@@ -61,7 +61,7 @@  int ListFormatsTest::run()
 	formats = scaler_->formats(1);
 	if (formats.empty()) {
 		cerr << "Failed to list formats on pad 1 of subdevice "
-		     << scaler_->entityName() << endl;
+		     << scaler_->entity()->name() << endl;
 		return TestFail;
 	}
 	for (auto it = formats.begin(); it != formats.end(); ++it)
@@ -71,7 +71,7 @@  int ListFormatsTest::run()
 	formats = scaler_->formats(2);
 	if (!formats.empty()) {
 		cerr << "Listing formats on non-existing pad 2 of subdevice "
-		     << scaler_->entityName()
+		     << scaler_->entity()->name()
 		     << " should return an empty format list" << endl;
 		return TestFail;
 	}
diff --git a/test/v4l2_subdevice/v4l2_subdevice_test.cpp b/test/v4l2_subdevice/v4l2_subdevice_test.cpp
index 06582969eb45..dfcf779af95e 100644
--- a/test/v4l2_subdevice/v4l2_subdevice_test.cpp
+++ b/test/v4l2_subdevice/v4l2_subdevice_test.cpp
@@ -66,7 +66,7 @@  int V4L2SubdeviceTest::init()
 	ret = scaler_->open();
 	if (ret) {
 		cerr << "Unable to open video subdevice "
-		     << scaler_->deviceNode() << endl;
+		     << scaler_->entity()->deviceNode() << endl;
 		media_->release();
 		return TestSkip;
 	}