[libcamera-devel,v4,1/2] pipeline: raspberrypi: Move sensor entity detection out of registerCamera()
diff mbox series

Message ID 20211214140002.3552445-2-naush@raspberrypi.com
State Accepted
Headers show
Series
  • Raspberry Pi: Add video mux and bridge support
Related show

Commit Message

Naushir Patuck Dec. 14, 2021, 2 p.m. UTC 
Enumerate the sensor device entities in PipelineHandlerRPi::match() and loop
over PipelineHandlerRPi::registerCamera() for each sensor found. This will
allow the pipeline handler to register multiple cameras attached to a single
Unicam instance with a Video Mux device.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 .../pipeline/raspberrypi/raspberrypi.cpp      | 35 +++++++++++--------
 1 file changed, 20 insertions(+), 15 deletions(-)

Comments

David Plowman Dec. 21, 2021, 10:34 a.m. UTC | #1 
Hi Naush

Thanks for this patch!

On Tue, 14 Dec 2021 at 14:00, Naushir Patuck <naush@raspberrypi.com> wrote:
>
> Enumerate the sensor device entities in PipelineHandlerRPi::match() and loop
> over PipelineHandlerRPi::registerCamera() for each sensor found. This will
> allow the pipeline handler to register multiple cameras attached to a single
> Unicam instance with a Video Mux device.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 35 +++++++++++--------
>  1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 86851ac467ad..2a2fb5273eb8 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -310,7 +310,7 @@ private:
>                 return static_cast<RPiCameraData *>(camera->_d());
>         }
>
> -       int registerCamera(MediaDevice *unicam, MediaDevice *isp);
> +       int registerCamera(MediaDevice *unicam, MediaDevice *isp, MediaEntity *sensorEntity);
>         int queueAllBuffers(Camera *camera);
>         int prepareBuffers(Camera *camera);
>         void freeBuffers(Camera *camera);
> @@ -1029,16 +1029,28 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
>                 return false;
>         }
>
> -       int ret = registerCamera(unicamDevice, ispDevice);
> -       if (ret) {
> -               LOG(RPI, Error) << "Failed to register camera: " << ret;
> -               return false;
> +       /*
> +        * The loop below is used to register multiple cameras behind one or more
> +        * video mux devices that are attached to a particular Unicam instance.
> +        * Obviously these cameras cannot be used simultaneously.
> +        */
> +       unsigned int numCameras = 0;
> +       for (MediaEntity *entity : unicamDevice->entities()) {
> +               if (entity->function() != MEDIA_ENT_F_CAM_SENSOR)
> +                       continue;
> +
> +               int ret = registerCamera(unicamDevice, ispDevice, entity);
> +               if (ret)
> +                       LOG(RPI, Error) << "Failed to register camera "
> +                                       << entity->name() << ": " << ret;
> +               else
> +                       numCameras++;
>         }
>
> -       return true;
> +       return !!numCameras;

I slightly wonder if the "!!" is necessary, or perhaps it reminds you
that you're returning a bool?

But I have nothing to add above the pointless/trivial, so:

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Thanks!
David

>  }
>
> -int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp)
> +int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, MediaEntity *sensorEntity)
>  {
>         std::unique_ptr<RPiCameraData> data = std::make_unique<RPiCameraData>(this);
>
> @@ -1079,14 +1091,7 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp)
>         data->isp_[Isp::Output1].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispOutputDequeue);
>         data->isp_[Isp::Stats].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispOutputDequeue);
>
> -       /* Identify the sensor. */
> -       for (MediaEntity *entity : unicam->entities()) {
> -               if (entity->function() == MEDIA_ENT_F_CAM_SENSOR) {
> -                       data->sensor_ = std::make_unique<CameraSensor>(entity);
> -                       break;
> -               }
> -       }
> -
> +       data->sensor_ = std::make_unique<CameraSensor>(sensorEntity);
>         if (!data->sensor_)
>                 return -EINVAL;
>
> --
> 2.25.1
>
Jacopo Mondi Dec. 21, 2021, 5:45 p.m. UTC | #2 
Hi Naush

On Tue, Dec 14, 2021 at 02:00:01PM +0000, Naushir Patuck wrote:
> Enumerate the sensor device entities in PipelineHandlerRPi::match() and loop
> over PipelineHandlerRPi::registerCamera() for each sensor found. This will
> allow the pipeline handler to register multiple cameras attached to a single
> Unicam instance with a Video Mux device.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

> ---
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 35 +++++++++++--------
>  1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 86851ac467ad..2a2fb5273eb8 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -310,7 +310,7 @@ private:
>  		return static_cast<RPiCameraData *>(camera->_d());
>  	}
>
> -	int registerCamera(MediaDevice *unicam, MediaDevice *isp);
> +	int registerCamera(MediaDevice *unicam, MediaDevice *isp, MediaEntity *sensorEntity);
>  	int queueAllBuffers(Camera *camera);
>  	int prepareBuffers(Camera *camera);
>  	void freeBuffers(Camera *camera);
> @@ -1029,16 +1029,28 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
>  		return false;
>  	}
>
> -	int ret = registerCamera(unicamDevice, ispDevice);
> -	if (ret) {
> -		LOG(RPI, Error) << "Failed to register camera: " << ret;
> -		return false;
> +	/*
> +	 * The loop below is used to register multiple cameras behind one or more
> +	 * video mux devices that are attached to a particular Unicam instance.
> +	 * Obviously these cameras cannot be used simultaneously.
> +	 */
> +	unsigned int numCameras = 0;
> +	for (MediaEntity *entity : unicamDevice->entities()) {
> +		if (entity->function() != MEDIA_ENT_F_CAM_SENSOR)
> +			continue;
> +
> +		int ret = registerCamera(unicamDevice, ispDevice, entity);
> +		if (ret)
> +			LOG(RPI, Error) << "Failed to register camera "
> +					<< entity->name() << ": " << ret;
> +		else
> +			numCameras++;
>  	}
>
> -	return true;
> +	return !!numCameras;
>  }
>
> -int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp)
> +int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, MediaEntity *sensorEntity)
>  {
>  	std::unique_ptr<RPiCameraData> data = std::make_unique<RPiCameraData>(this);
>
> @@ -1079,14 +1091,7 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp)
>  	data->isp_[Isp::Output1].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispOutputDequeue);
>  	data->isp_[Isp::Stats].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispOutputDequeue);
>
> -	/* Identify the sensor. */
> -	for (MediaEntity *entity : unicam->entities()) {
> -		if (entity->function() == MEDIA_ENT_F_CAM_SENSOR) {
> -			data->sensor_ = std::make_unique<CameraSensor>(entity);
> -			break;
> -		}
> -	}
> -
> +	data->sensor_ = std::make_unique<CameraSensor>(sensorEntity);
>  	if (!data->sensor_)
>  		return -EINVAL;
>
> --
> 2.25.1
>

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 86851ac467ad..2a2fb5273eb8 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -310,7 +310,7 @@  private:
 		return static_cast<RPiCameraData *>(camera->_d());
 	}
 
-	int registerCamera(MediaDevice *unicam, MediaDevice *isp);
+	int registerCamera(MediaDevice *unicam, MediaDevice *isp, MediaEntity *sensorEntity);
 	int queueAllBuffers(Camera *camera);
 	int prepareBuffers(Camera *camera);
 	void freeBuffers(Camera *camera);
@@ -1029,16 +1029,28 @@  bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
 		return false;
 	}
 
-	int ret = registerCamera(unicamDevice, ispDevice);
-	if (ret) {
-		LOG(RPI, Error) << "Failed to register camera: " << ret;
-		return false;
+	/*
+	 * The loop below is used to register multiple cameras behind one or more
+	 * video mux devices that are attached to a particular Unicam instance.
+	 * Obviously these cameras cannot be used simultaneously.
+	 */
+	unsigned int numCameras = 0;
+	for (MediaEntity *entity : unicamDevice->entities()) {
+		if (entity->function() != MEDIA_ENT_F_CAM_SENSOR)
+			continue;
+
+		int ret = registerCamera(unicamDevice, ispDevice, entity);
+		if (ret)
+			LOG(RPI, Error) << "Failed to register camera "
+					<< entity->name() << ": " << ret;
+		else
+			numCameras++;
 	}
 
-	return true;
+	return !!numCameras;
 }
 
-int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp)
+int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, MediaEntity *sensorEntity)
 {
 	std::unique_ptr<RPiCameraData> data = std::make_unique<RPiCameraData>(this);
 
@@ -1079,14 +1091,7 @@  int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp)
 	data->isp_[Isp::Output1].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispOutputDequeue);
 	data->isp_[Isp::Stats].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispOutputDequeue);
 
-	/* Identify the sensor. */
-	for (MediaEntity *entity : unicam->entities()) {
-		if (entity->function() == MEDIA_ENT_F_CAM_SENSOR) {
-			data->sensor_ = std::make_unique<CameraSensor>(entity);
-			break;
-		}
-	}
-
+	data->sensor_ = std::make_unique<CameraSensor>(sensorEntity);
 	if (!data->sensor_)
 		return -EINVAL;