[1/2] pipeline: simple: Remove media member variable
diff mbox series

Message ID 20240808173921.2519957-2-kieran.bingham@ideasonboard.com
State Accepted
Headers show
Series
  • pipeline: simple: Fix matching with an empty media graph
Related show

Commit Message

Kieran Bingham Aug. 8, 2024, 5:39 p.m. UTC
From: Paul Elder <paul.elder@ideasonboard.com>

There is no need for the simple pipeline handler to save the media
device. Remove it.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/pipeline/simple/simple.cpp | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

Comments

Laurent Pinchart Aug. 10, 2024, 12:29 p.m. UTC | #1
hi Kieran and Paul,

Thank you for the patch.

On Thu, Aug 08, 2024 at 06:39:20PM +0100, Kieran Bingham wrote:
> From: Paul Elder <paul.elder@ideasonboard.com>
> 
> There is no need for the simple pipeline handler to save the media
> device. Remove it.

I suspect there's a hidden reason behind this change. Well, maybe not
that hidden, given there's a second patch posted in the same series :-)

> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/libcamera/pipeline/simple/simple.cpp | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 9910900e0e5e..222052ce02f8 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -363,13 +363,12 @@ private:
>  		return static_cast<SimpleCameraData *>(camera->_d());
>  	}
>  
> -	std::vector<MediaEntity *> locateSensors();
> +	std::vector<MediaEntity *> locateSensors(MediaDevice *media);
>  	static int resetRoutingTable(V4L2Subdevice *subdev);
>  
>  	const MediaPad *acquirePipeline(SimpleCameraData *data);
>  	void releasePipeline(SimpleCameraData *data);
>  
> -	MediaDevice *media_;
>  	std::map<const MediaEntity *, EntityData> entities_;
>  
>  	MediaDevice *converter_;
> @@ -1424,7 +1423,8 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)
>   * Match and Setup
>   */
>  
> -std::vector<MediaEntity *> SimplePipelineHandler::locateSensors()
> +std::vector<MediaEntity *>
> +SimplePipelineHandler::locateSensors(MediaDevice *media)
>  {
>  	std::vector<MediaEntity *> entities;
>  
> @@ -1432,7 +1432,7 @@ std::vector<MediaEntity *> SimplePipelineHandler::locateSensors()
>  	 * Gather all the camera sensor entities based on the function they
>  	 * expose.
>  	 */
> -	for (MediaEntity *entity : media_->entities()) {
> +	for (MediaEntity *entity : media->entities()) {
>  		if (entity->function() == MEDIA_ENT_F_CAM_SENSOR)
>  			entities.push_back(entity);
>  	}
> @@ -1520,17 +1520,18 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
>  {
>  	const SimplePipelineInfo *info = nullptr;
>  	unsigned int numStreams = 1;
> +	MediaDevice *media;
>  
>  	for (const SimplePipelineInfo &inf : supportedDevices) {
>  		DeviceMatch dm(inf.driver);
> -		media_ = acquireMediaDevice(enumerator, dm);
> -		if (media_) {
> +		media = acquireMediaDevice(enumerator, dm);
> +		if (media) {
>  			info = &inf;
>  			break;
>  		}
>  	}
>  
> -	if (!media_)
> +	if (!media)
>  		return false;
>  
>  	for (const auto &[name, streams] : info->converters) {
> @@ -1545,13 +1546,13 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
>  	swIspEnabled_ = info->swIspEnabled;
>  
>  	/* Locate the sensors. */
> -	std::vector<MediaEntity *> sensors = locateSensors();
> +	std::vector<MediaEntity *> sensors = locateSensors(media);
>  	if (sensors.empty()) {
> -		LOG(SimplePipeline, Info) << "No sensor found for " << media_->deviceNode();
> +		LOG(SimplePipeline, Info) << "No sensor found for " << media->deviceNode();
>  		return false;
>  	}
>  
> -	LOG(SimplePipeline, Debug) << "Sensor found for " << media_->deviceNode();
> +	LOG(SimplePipeline, Debug) << "Sensor found for " << media->deviceNode();
>  
>  	/*
>  	 * Create one camera data instance for each sensor and gather all

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 9910900e0e5e..222052ce02f8 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -363,13 +363,12 @@  private:
 		return static_cast<SimpleCameraData *>(camera->_d());
 	}
 
-	std::vector<MediaEntity *> locateSensors();
+	std::vector<MediaEntity *> locateSensors(MediaDevice *media);
 	static int resetRoutingTable(V4L2Subdevice *subdev);
 
 	const MediaPad *acquirePipeline(SimpleCameraData *data);
 	void releasePipeline(SimpleCameraData *data);
 
-	MediaDevice *media_;
 	std::map<const MediaEntity *, EntityData> entities_;
 
 	MediaDevice *converter_;
@@ -1424,7 +1423,8 @@  int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)
  * Match and Setup
  */
 
-std::vector<MediaEntity *> SimplePipelineHandler::locateSensors()
+std::vector<MediaEntity *>
+SimplePipelineHandler::locateSensors(MediaDevice *media)
 {
 	std::vector<MediaEntity *> entities;
 
@@ -1432,7 +1432,7 @@  std::vector<MediaEntity *> SimplePipelineHandler::locateSensors()
 	 * Gather all the camera sensor entities based on the function they
 	 * expose.
 	 */
-	for (MediaEntity *entity : media_->entities()) {
+	for (MediaEntity *entity : media->entities()) {
 		if (entity->function() == MEDIA_ENT_F_CAM_SENSOR)
 			entities.push_back(entity);
 	}
@@ -1520,17 +1520,18 @@  bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
 {
 	const SimplePipelineInfo *info = nullptr;
 	unsigned int numStreams = 1;
+	MediaDevice *media;
 
 	for (const SimplePipelineInfo &inf : supportedDevices) {
 		DeviceMatch dm(inf.driver);
-		media_ = acquireMediaDevice(enumerator, dm);
-		if (media_) {
+		media = acquireMediaDevice(enumerator, dm);
+		if (media) {
 			info = &inf;
 			break;
 		}
 	}
 
-	if (!media_)
+	if (!media)
 		return false;
 
 	for (const auto &[name, streams] : info->converters) {
@@ -1545,13 +1546,13 @@  bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
 	swIspEnabled_ = info->swIspEnabled;
 
 	/* Locate the sensors. */
-	std::vector<MediaEntity *> sensors = locateSensors();
+	std::vector<MediaEntity *> sensors = locateSensors(media);
 	if (sensors.empty()) {
-		LOG(SimplePipeline, Info) << "No sensor found for " << media_->deviceNode();
+		LOG(SimplePipeline, Info) << "No sensor found for " << media->deviceNode();
 		return false;
 	}
 
-	LOG(SimplePipeline, Debug) << "Sensor found for " << media_->deviceNode();
+	LOG(SimplePipeline, Debug) << "Sensor found for " << media->deviceNode();
 
 	/*
 	 * Create one camera data instance for each sensor and gather all