[v1,2/3] pipeline: rpi: Move media device configuration into its own function
diff mbox series

Message ID 20250625084212.858487-3-naush@raspberrypi.com
State New
Headers show
Series
  • Raspberry Pi: Media graph enumeration updates
Related show

Commit Message

Naushir Patuck June 25, 2025, 8:41 a.m. UTC
Add a new function configureMediaDevices() used to configure the
media links and pads for all known bridge/mux entities in the media
graph.

No functional change, this simply moves existing code into a new
function in prepartion for further changes in a subsequent commit.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
 .../pipeline/rpi/common/pipeline_base.cpp     | 84 ++++++++++---------
 1 file changed, 46 insertions(+), 38 deletions(-)

Comments

Umang Jain June 26, 2025, 6:55 a.m. UTC | #1
Hi Naush,

Thank you for the patch.

On 6/25/25 2:11 PM, Naushir Patuck wrote:
> Add a new function configureMediaDevices() used to configure the
> media links and pads for all known bridge/mux entities in the media
> graph.
>
> No functional change, this simply moves existing code into a new
> function in prepartion for further changes in a subsequent commit.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>   .../pipeline/rpi/common/pipeline_base.cpp     | 84 ++++++++++---------
>   1 file changed, 46 insertions(+), 38 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index eafe94427dbc..219bd81cc63a 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -85,6 +85,51 @@ std::optional<ColorSpace> findValidColorSpace(const ColorSpace &colourSpace)
>   	return std::nullopt;
>   }
>   
> +int configureMediaDevices(std::vector<std::pair<std::unique_ptr<V4L2Subdevice>, MediaLink *>> &bridgeDevices,
> +			  V4L2SubdeviceFormat *sensorFormat)
> +{
> +	int ret = 0;
> +
> +	/* Setup the Video Mux/Bridge entities. */
> +	for (auto &[device, link] : bridgeDevices) {
> +		/*
> +		 * Start by disabling all the sink pad links on the devices in the
> +		 * cascade, with the exception of the link connecting the device.
> +		 */
> +		for (const MediaPad *p : device->entity()->pads()) {
> +			if (!(p->flags() & MEDIA_PAD_FL_SINK))
> +				continue;
> +
> +			for (MediaLink *l : p->links()) {
> +				if (l != link)
> +					l->setEnabled(false);
> +			}
> +		}
> +
> +		/*
> +		 * Next, enable the entity -> entity links, and setup the pad format.
> +		 *
> +		 * \todo Some bridge devices may chainge the media bus code, so we

In this code move, the spell error can be corrected,

s/chainge/change

Reviewed-by: Umang Jain <uajain@igalia.com>


> +		 * ought to read the source pad format and propagate it to the sink pad.
> +		 */
> +		link->setEnabled(true);
> +		const MediaPad *sinkPad = link->sink();
> +		ret = device->setFormat(sinkPad->index(), sensorFormat);
> +		if (ret) {
> +			LOG(RPI, Error) << "Failed to set format on " << device->entity()->name()
> +					<< " pad " << sinkPad->index()
> +					<< " with format  " << *sensorFormat
> +					<< ": " << ret;
> +			return ret;
> +		}
> +
> +		LOG(RPI, Debug) << "Configured media link on device " << device->entity()->name()
> +				<< " on pad " << sinkPad->index();
> +	}
> +
> +	return ret;
> +}
> +
>   } /* namespace */
>   
>   /*
> @@ -589,44 +634,7 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)
>   
>   	data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap());
>   
> -	/* Setup the Video Mux/Bridge entities. */
> -	for (auto &[device, link] : data->bridgeDevices_) {
> -		/*
> -		 * Start by disabling all the sink pad links on the devices in the
> -		 * cascade, with the exception of the link connecting the device.
> -		 */
> -		for (const MediaPad *p : device->entity()->pads()) {
> -			if (!(p->flags() & MEDIA_PAD_FL_SINK))
> -				continue;
> -
> -			for (MediaLink *l : p->links()) {
> -				if (l != link)
> -					l->setEnabled(false);
> -			}
> -		}
> -
> -		/*
> -		 * Next, enable the entity -> entity links, and setup the pad format.
> -		 *
> -		 * \todo Some bridge devices may chainge the media bus code, so we
> -		 * ought to read the source pad format and propagate it to the sink pad.
> -		 */
> -		link->setEnabled(true);
> -		const MediaPad *sinkPad = link->sink();
> -		ret = device->setFormat(sinkPad->index(), sensorFormat);
> -		if (ret) {
> -			LOG(RPI, Error) << "Failed to set format on " << device->entity()->name()
> -					<< " pad " << sinkPad->index()
> -					<< " with format  " << *sensorFormat
> -					<< ": " << ret;
> -			return ret;
> -		}
> -
> -		LOG(RPI, Debug) << "Configured media link on device " << device->entity()->name()
> -				<< " on pad " << sinkPad->index();
> -	}
> -
> -	return 0;
> +	return configureMediaDevices(data->bridgeDevices_, sensorFormat);
>   }
>   
>   int PipelineHandlerBase::exportFrameBuffers([[maybe_unused]] Camera *camera, libcamera::Stream *stream,
Laurent Pinchart June 26, 2025, 12:56 p.m. UTC | #2
On Wed, Jun 25, 2025 at 09:41:17AM +0100, Naushir Patuck wrote:
> Add a new function configureMediaDevices() used to configure the
> media links and pads for all known bridge/mux entities in the media
> graph.
> 
> No functional change, this simply moves existing code into a new
> function in prepartion for further changes in a subsequent commit.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>  .../pipeline/rpi/common/pipeline_base.cpp     | 84 ++++++++++---------
>  1 file changed, 46 insertions(+), 38 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index eafe94427dbc..219bd81cc63a 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -85,6 +85,51 @@ std::optional<ColorSpace> findValidColorSpace(const ColorSpace &colourSpace)
>  	return std::nullopt;
>  }
>  
> +int configureMediaDevices(std::vector<std::pair<std::unique_ptr<V4L2Subdevice>, MediaLink *>> &bridgeDevices,

The vector should be const, it's not modified by this function.

> +			  V4L2SubdeviceFormat *sensorFormat)
> +{
> +	int ret = 0;
> +
> +	/* Setup the Video Mux/Bridge entities. */
> +	for (auto &[device, link] : bridgeDevices) {
> +		/*
> +		 * Start by disabling all the sink pad links on the devices in the
> +		 * cascade, with the exception of the link connecting the device.
> +		 */
> +		for (const MediaPad *p : device->entity()->pads()) {
> +			if (!(p->flags() & MEDIA_PAD_FL_SINK))
> +				continue;
> +
> +			for (MediaLink *l : p->links()) {
> +				if (l != link)
> +					l->setEnabled(false);
> +			}
> +		}
> +
> +		/*
> +		 * Next, enable the entity -> entity links, and setup the pad format.
> +		 *
> +		 * \todo Some bridge devices may chainge the media bus code, so we
> +		 * ought to read the source pad format and propagate it to the sink pad.
> +		 */
> +		link->setEnabled(true);
> +		const MediaPad *sinkPad = link->sink();
> +		ret = device->setFormat(sinkPad->index(), sensorFormat);
> +		if (ret) {
> +			LOG(RPI, Error) << "Failed to set format on " << device->entity()->name()
> +					<< " pad " << sinkPad->index()
> +					<< " with format  " << *sensorFormat
> +					<< ": " << ret;
> +			return ret;
> +		}
> +
> +		LOG(RPI, Debug) << "Configured media link on device " << device->entity()->name()
> +				<< " on pad " << sinkPad->index();
> +	}
> +
> +	return ret;
> +}
> +
>  } /* namespace */
>  
>  /*
> @@ -589,44 +634,7 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)
>  
>  	data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap());
>  
> -	/* Setup the Video Mux/Bridge entities. */
> -	for (auto &[device, link] : data->bridgeDevices_) {
> -		/*
> -		 * Start by disabling all the sink pad links on the devices in the
> -		 * cascade, with the exception of the link connecting the device.
> -		 */
> -		for (const MediaPad *p : device->entity()->pads()) {
> -			if (!(p->flags() & MEDIA_PAD_FL_SINK))
> -				continue;
> -
> -			for (MediaLink *l : p->links()) {
> -				if (l != link)
> -					l->setEnabled(false);
> -			}
> -		}
> -
> -		/*
> -		 * Next, enable the entity -> entity links, and setup the pad format.
> -		 *
> -		 * \todo Some bridge devices may chainge the media bus code, so we
> -		 * ought to read the source pad format and propagate it to the sink pad.
> -		 */
> -		link->setEnabled(true);
> -		const MediaPad *sinkPad = link->sink();
> -		ret = device->setFormat(sinkPad->index(), sensorFormat);
> -		if (ret) {
> -			LOG(RPI, Error) << "Failed to set format on " << device->entity()->name()
> -					<< " pad " << sinkPad->index()
> -					<< " with format  " << *sensorFormat
> -					<< ": " << ret;
> -			return ret;
> -		}
> -
> -		LOG(RPI, Debug) << "Configured media link on device " << device->entity()->name()
> -				<< " on pad " << sinkPad->index();
> -	}
> -
> -	return 0;
> +	return configureMediaDevices(data->bridgeDevices_, sensorFormat);
>  }
>  
>  int PipelineHandlerBase::exportFrameBuffers([[maybe_unused]] Camera *camera, libcamera::Stream *stream,

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
index eafe94427dbc..219bd81cc63a 100644
--- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
@@ -85,6 +85,51 @@  std::optional<ColorSpace> findValidColorSpace(const ColorSpace &colourSpace)
 	return std::nullopt;
 }
 
+int configureMediaDevices(std::vector<std::pair<std::unique_ptr<V4L2Subdevice>, MediaLink *>> &bridgeDevices,
+			  V4L2SubdeviceFormat *sensorFormat)
+{
+	int ret = 0;
+
+	/* Setup the Video Mux/Bridge entities. */
+	for (auto &[device, link] : bridgeDevices) {
+		/*
+		 * Start by disabling all the sink pad links on the devices in the
+		 * cascade, with the exception of the link connecting the device.
+		 */
+		for (const MediaPad *p : device->entity()->pads()) {
+			if (!(p->flags() & MEDIA_PAD_FL_SINK))
+				continue;
+
+			for (MediaLink *l : p->links()) {
+				if (l != link)
+					l->setEnabled(false);
+			}
+		}
+
+		/*
+		 * Next, enable the entity -> entity links, and setup the pad format.
+		 *
+		 * \todo Some bridge devices may chainge the media bus code, so we
+		 * ought to read the source pad format and propagate it to the sink pad.
+		 */
+		link->setEnabled(true);
+		const MediaPad *sinkPad = link->sink();
+		ret = device->setFormat(sinkPad->index(), sensorFormat);
+		if (ret) {
+			LOG(RPI, Error) << "Failed to set format on " << device->entity()->name()
+					<< " pad " << sinkPad->index()
+					<< " with format  " << *sensorFormat
+					<< ": " << ret;
+			return ret;
+		}
+
+		LOG(RPI, Debug) << "Configured media link on device " << device->entity()->name()
+				<< " on pad " << sinkPad->index();
+	}
+
+	return ret;
+}
+
 } /* namespace */
 
 /*
@@ -589,44 +634,7 @@  int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)
 
 	data->controlInfo_ = ControlInfoMap(std::move(ctrlMap), result.controlInfo.idmap());
 
-	/* Setup the Video Mux/Bridge entities. */
-	for (auto &[device, link] : data->bridgeDevices_) {
-		/*
-		 * Start by disabling all the sink pad links on the devices in the
-		 * cascade, with the exception of the link connecting the device.
-		 */
-		for (const MediaPad *p : device->entity()->pads()) {
-			if (!(p->flags() & MEDIA_PAD_FL_SINK))
-				continue;
-
-			for (MediaLink *l : p->links()) {
-				if (l != link)
-					l->setEnabled(false);
-			}
-		}
-
-		/*
-		 * Next, enable the entity -> entity links, and setup the pad format.
-		 *
-		 * \todo Some bridge devices may chainge the media bus code, so we
-		 * ought to read the source pad format and propagate it to the sink pad.
-		 */
-		link->setEnabled(true);
-		const MediaPad *sinkPad = link->sink();
-		ret = device->setFormat(sinkPad->index(), sensorFormat);
-		if (ret) {
-			LOG(RPI, Error) << "Failed to set format on " << device->entity()->name()
-					<< " pad " << sinkPad->index()
-					<< " with format  " << *sensorFormat
-					<< ": " << ret;
-			return ret;
-		}
-
-		LOG(RPI, Debug) << "Configured media link on device " << device->entity()->name()
-				<< " on pad " << sinkPad->index();
-	}
-
-	return 0;
+	return configureMediaDevices(data->bridgeDevices_, sensorFormat);
 }
 
 int PipelineHandlerBase::exportFrameBuffers([[maybe_unused]] Camera *camera, libcamera::Stream *stream,