[libcamera-devel,11/13] libcamera: pipeline: simple: Walk pipeline using subdev internal routing
diff mbox series

Message ID 20220801000543.3501-12-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: pipeline: simple: Support the NXP i.MX8 ISI
Related show

Commit Message

Laurent Pinchart Aug. 1, 2022, 12:05 a.m. UTC 
From: Phi-Bang Nguyen <pnguyen@baylibre.com>

When traversing the media graph to discover a pipeline from the camera
sensor to a video node, all sink-to-source paths inside subdevs are
considered. This can lead to invalid paths being followed, when a subdev
has restrictions on its internal routing.

The V4L2 API supports exposing subdev internal routing to userspace.
Make use if this feature, when implemented by a subdev, to restrict the
internal paths to the currently active routes. If a subdev doesn't
implement the internal routing operations, all source pads are
considered, as done today.

This change is needed to properly support multiple sensors with devices
such as the NXP i.MX8 ISI or the MediaTek i350 and i500 SENINF. Support
for changing routes dynamically will be added later when required.

Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/pipeline/simple/simple.cpp | 73 ++++++++++++++++++++++--
 1 file changed, 67 insertions(+), 6 deletions(-)

Comments

Jacopo Mondi Aug. 1, 2022, 4:16 p.m. UTC | #1 
On Mon, Aug 01, 2022 at 03:05:41AM +0300, Laurent Pinchart via libcamera-devel wrote:
> From: Phi-Bang Nguyen <pnguyen@baylibre.com>
>
> When traversing the media graph to discover a pipeline from the camera
> sensor to a video node, all sink-to-source paths inside subdevs are
> considered. This can lead to invalid paths being followed, when a subdev
> has restrictions on its internal routing.
>
> The V4L2 API supports exposing subdev internal routing to userspace.
> Make use if this feature, when implemented by a subdev, to restrict the

Make use "of"

> internal paths to the currently active routes. If a subdev doesn't
> implement the internal routing operations, all source pads are
> considered, as done today.
>
> This change is needed to properly support multiple sensors with devices
> such as the NXP i.MX8 ISI or the MediaTek i350 and i500 SENINF. Support
> for changing routes dynamically will be added later when required.
>
> Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/pipeline/simple/simple.cpp | 73 ++++++++++++++++++++++--
>  1 file changed, 67 insertions(+), 6 deletions(-)
>
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 4bde9caa7254..2a8811183907 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -100,8 +100,14 @@ LOG_DEFINE_CATEGORY(SimplePipeline)
>   *
>   * During the breadth-first search, the pipeline is traversed from entity to
>   * entity, by following media graph links from source to sink, starting at the
> - * camera sensor. When reaching an entity (on its sink side), all its source
> - * pads are considered to continue the graph traversal.
> + * camera sensor.
> + *
> + * When reaching an entity (on its sink side), if the entity is a V4L2 subdev
> + * that supports the streams API, the subdev internal routes are followed to
> + * find the connected source pads. Otherwise all of the entity's source pads
> + * are considered to continue the graph traversal. The pipeline handler
> + * currently considers the default internal routes only and doesn't attempt to
> + * setup custom routes. This can be extended if needed.
>   *
>   * The shortest path between the camera sensor and a video node is stored in
>   * SimpleCameraData::entities_ as a list of SimpleCameraData::Entity structures,
> @@ -261,6 +267,7 @@ public:
>
>  private:
>  	void tryPipeline(unsigned int code, const Size &size);
> +	static std::vector<const MediaPad *> routedSourcePads(MediaPad *sink);
>
>  	void converterInputDone(FrameBuffer *buffer);
>  	void converterOutputDone(FrameBuffer *buffer);
> @@ -387,12 +394,29 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
>  			break;
>  		}
>
> -		/* The actual breadth-first search algorithm. */
>  		visited.insert(entity);
> -		for (MediaPad *pad : entity->pads()) {
> -			if (!(pad->flags() & MEDIA_PAD_FL_SOURCE))
> -				continue;
>
> +		/*
> +		 * Add direct downstream entities to the search queue. If the
> +		 * current entity supports the subdev internal routing API,
> +		 * restrict the search to downstream entities reachable through
> +		 * active routes.
> +		 */
> +

Is the emtpy line necessary ?

> +		std::vector<const MediaPad *> pads;
> +
> +		if (sinkPad)
> +			pads = routedSourcePads(sinkPad);
> +
> +		if (pads.empty()) {

pads is empty also in case of errors, like failures to open the
subdev.

Isn't it better to consider pads.emtpy and error condition, and handle
the !sinkPad cases in the "routedSourcePads()" function ?


                std::vector<> pads = connectedSources(entity, sinkPad);
                if (pads.empty()) {
                        LOG(Error) ...
                        return -EINVAL;
                }

}

std::vector<> SimpleCameraData::nextSources(entity)
{
        vector<> pads;

        for (pad : entity->pads()) {
                if (pad->flags & SINK)
                        continue;

                pads.push_back(pad);
        }

        return pads;
}

std::vector<> SimpleCameraData::connectedSources(entity, sinkPad)
{
        if (!sinkPad)
                return nextSources(entity);

        ret = subdev->open();
        if (ret)
                return {};

        ret = subdev->getRouting();
        if (!routing)
                return nextSources(entity);

        ....
}

> +			for (const MediaPad *pad : entity->pads()) {
> +				if (!(pad->flags() & MEDIA_PAD_FL_SOURCE))
> +					continue;
> +				pads.push_back(pad);
> +			}
> +		}
> +
> +		for (const MediaPad *pad : pads) {
>  			for (MediaLink *link : pad->links()) {
>  				MediaEntity *next = link->sink()->entity();
>  				if (visited.find(next) == visited.end()) {
> @@ -782,6 +806,43 @@ void SimpleCameraData::converterOutputDone(FrameBuffer *buffer)
>  		pipe->completeRequest(request);
>  }
>
> +/* Retrieve all source pads connected to a sink pad through active routes. */
> +std::vector<const MediaPad *> SimpleCameraData::routedSourcePads(MediaPad *sink)
> +{
> +	MediaEntity *entity = sink->entity();
> +	std::unique_ptr<V4L2Subdevice> subdev =
> +		std::make_unique<V4L2Subdevice>(entity);
> +
> +	int ret = subdev->open();
> +	if (ret < 0)
> +		return {};
> +
> +	V4L2Subdevice::Routing routing = {};
> +	ret = subdev->getRouting(&routing, V4L2Subdevice::ActiveFormat);
> +	if (ret < 0)
> +		return {};
> +
> +	std::vector<const MediaPad *> pads;
> +
> +	for (const struct v4l2_subdev_route &route : routing) {
> +		if (sink->index() != route.sink_pad ||
> +		    !(route.flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
> +			continue;
> +
> +		const MediaPad *pad = entity->getPadByIndex(route.source_pad);
> +		if (!pad) {
> +			LOG(SimplePipeline, Warning)
> +				<< "Entity " << entity->name()
> +				<< " has invalid route source pad "
> +				<< route.source_pad;
> +		}
> +
> +		pads.push_back(pad);
> +	}
> +
> +	return pads;
> +}
> +
>  /* -----------------------------------------------------------------------------
>   * Camera Configuration
>   */
> --
> Regards,
>
> Laurent Pinchart
>
Laurent Pinchart Aug. 1, 2022, 8:36 p.m. UTC | #2 
Hi Jacopo,

On Mon, Aug 01, 2022 at 06:16:03PM +0200, Jacopo Mondi wrote:
> On Mon, Aug 01, 2022 at 03:05:41AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > From: Phi-Bang Nguyen <pnguyen@baylibre.com>
> >
> > When traversing the media graph to discover a pipeline from the camera
> > sensor to a video node, all sink-to-source paths inside subdevs are
> > considered. This can lead to invalid paths being followed, when a subdev
> > has restrictions on its internal routing.
> >
> > The V4L2 API supports exposing subdev internal routing to userspace.
> > Make use if this feature, when implemented by a subdev, to restrict the
> 
> Make use "of"
> 
> > internal paths to the currently active routes. If a subdev doesn't
> > implement the internal routing operations, all source pads are
> > considered, as done today.
> >
> > This change is needed to properly support multiple sensors with devices
> > such as the NXP i.MX8 ISI or the MediaTek i350 and i500 SENINF. Support
> > for changing routes dynamically will be added later when required.
> >
> > Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/simple/simple.cpp | 73 ++++++++++++++++++++++--
> >  1 file changed, 67 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index 4bde9caa7254..2a8811183907 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -100,8 +100,14 @@ LOG_DEFINE_CATEGORY(SimplePipeline)
> >   *
> >   * During the breadth-first search, the pipeline is traversed from entity to
> >   * entity, by following media graph links from source to sink, starting at the
> > - * camera sensor. When reaching an entity (on its sink side), all its source
> > - * pads are considered to continue the graph traversal.
> > + * camera sensor.
> > + *
> > + * When reaching an entity (on its sink side), if the entity is a V4L2 subdev
> > + * that supports the streams API, the subdev internal routes are followed to
> > + * find the connected source pads. Otherwise all of the entity's source pads
> > + * are considered to continue the graph traversal. The pipeline handler
> > + * currently considers the default internal routes only and doesn't attempt to
> > + * setup custom routes. This can be extended if needed.
> >   *
> >   * The shortest path between the camera sensor and a video node is stored in
> >   * SimpleCameraData::entities_ as a list of SimpleCameraData::Entity structures,
> > @@ -261,6 +267,7 @@ public:
> >
> >  private:
> >  	void tryPipeline(unsigned int code, const Size &size);
> > +	static std::vector<const MediaPad *> routedSourcePads(MediaPad *sink);
> >
> >  	void converterInputDone(FrameBuffer *buffer);
> >  	void converterOutputDone(FrameBuffer *buffer);
> > @@ -387,12 +394,29 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
> >  			break;
> >  		}
> >
> > -		/* The actual breadth-first search algorithm. */
> >  		visited.insert(entity);
> > -		for (MediaPad *pad : entity->pads()) {
> > -			if (!(pad->flags() & MEDIA_PAD_FL_SOURCE))
> > -				continue;
> >
> > +		/*
> > +		 * Add direct downstream entities to the search queue. If the
> > +		 * current entity supports the subdev internal routing API,
> > +		 * restrict the search to downstream entities reachable through
> > +		 * active routes.
> > +		 */
> > +
> 
> Is the emtpy line necessary ?

The code seems to still run fine without it ;-) I'll drop it.

> > +		std::vector<const MediaPad *> pads;
> > +
> > +		if (sinkPad)
> > +			pads = routedSourcePads(sinkPad);
> > +
> > +		if (pads.empty()) {
> 
> pads is empty also in case of errors, like failures to open the
> subdev.

That's right. That's actually the only failure unrelated to routing (the
other failure would come from subdev->getRouting(), which would just
indicate that the API isn't supported). We could handle the open failure
here, but the subdevs are all opened in SimplePipelineHandler::match()
just after constructing the SimpleCameraData instances, so we'll also
catch errors there. Failures to open subdevs are really not supposed to
happen, so as long as we handle them correctly somewhere, that's good
enough for me I think.

> Isn't it better to consider pads.emtpy and error condition, and handle
> the !sinkPad cases in the "routedSourcePads()" function ?

These are two different issues. The sinkPad check is only meant to skip
the routedSourcePads() call for the camera sensor entity (that's the
only one pushed to the queue with a nullptr for the MediaPad pointer).
Then, pads can be empty in error cases indeed, but also if the entity
doesn't support the routing API (not all entities do, and certainly not
on kernels that don't have the stream series applied :-)). In that case
the code defaults to exploring all the source pads of the entity, like
it did before this patch.

And now that I've written that, I see it matches the code you wrote
below :-)

> 
> 
>                 std::vector<> pads = connectedSources(entity, sinkPad);
>                 if (pads.empty()) {
>                         LOG(Error) ...
>                         return -EINVAL;
>                 }
> 
> }
> 
> std::vector<> SimpleCameraData::nextSources(entity)
> {
>         vector<> pads;
> 
>         for (pad : entity->pads()) {
>                 if (pad->flags & SINK)
>                         continue;
> 
>                 pads.push_back(pad);
>         }
> 
>         return pads;
> }
> 
> std::vector<> SimpleCameraData::connectedSources(entity, sinkPad)
> {
>         if (!sinkPad)
>                 return nextSources(entity);
> 
>         ret = subdev->open();
>         if (ret)
>                 return {};
> 
>         ret = subdev->getRouting();
>         if (!routing)
>                 return nextSources(entity);
> 
>         ....
> }

I was going to push back a bit because I'm lazy, but that looks cleaner
:-) I'll give it a try.

> > +			for (const MediaPad *pad : entity->pads()) {
> > +				if (!(pad->flags() & MEDIA_PAD_FL_SOURCE))
> > +					continue;
> > +				pads.push_back(pad);
> > +			}
> > +		}
> > +
> > +		for (const MediaPad *pad : pads) {
> >  			for (MediaLink *link : pad->links()) {
> >  				MediaEntity *next = link->sink()->entity();
> >  				if (visited.find(next) == visited.end()) {
> > @@ -782,6 +806,43 @@ void SimpleCameraData::converterOutputDone(FrameBuffer *buffer)
> >  		pipe->completeRequest(request);
> >  }
> >
> > +/* Retrieve all source pads connected to a sink pad through active routes. */
> > +std::vector<const MediaPad *> SimpleCameraData::routedSourcePads(MediaPad *sink)
> > +{
> > +	MediaEntity *entity = sink->entity();
> > +	std::unique_ptr<V4L2Subdevice> subdev =
> > +		std::make_unique<V4L2Subdevice>(entity);
> > +
> > +	int ret = subdev->open();
> > +	if (ret < 0)
> > +		return {};
> > +
> > +	V4L2Subdevice::Routing routing = {};
> > +	ret = subdev->getRouting(&routing, V4L2Subdevice::ActiveFormat);
> > +	if (ret < 0)
> > +		return {};
> > +
> > +	std::vector<const MediaPad *> pads;
> > +
> > +	for (const struct v4l2_subdev_route &route : routing) {
> > +		if (sink->index() != route.sink_pad ||
> > +		    !(route.flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
> > +			continue;
> > +
> > +		const MediaPad *pad = entity->getPadByIndex(route.source_pad);
> > +		if (!pad) {
> > +			LOG(SimplePipeline, Warning)
> > +				<< "Entity " << entity->name()
> > +				<< " has invalid route source pad "
> > +				<< route.source_pad;
> > +		}
> > +
> > +		pads.push_back(pad);
> > +	}
> > +
> > +	return pads;
> > +}
> > +
> >  /* -----------------------------------------------------------------------------
> >   * Camera Configuration
> >   */
Laurent Pinchart Aug. 1, 2022, 8:46 p.m. UTC | #3 
Hi Jacopo,

On Mon, Aug 01, 2022 at 11:36:19PM +0300, Laurent Pinchart via libcamera-devel wrote:
> On Mon, Aug 01, 2022 at 06:16:03PM +0200, Jacopo Mondi wrote:
> > On Mon, Aug 01, 2022 at 03:05:41AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > > From: Phi-Bang Nguyen <pnguyen@baylibre.com>
> > >
> > > When traversing the media graph to discover a pipeline from the camera
> > > sensor to a video node, all sink-to-source paths inside subdevs are
> > > considered. This can lead to invalid paths being followed, when a subdev
> > > has restrictions on its internal routing.
> > >
> > > The V4L2 API supports exposing subdev internal routing to userspace.
> > > Make use if this feature, when implemented by a subdev, to restrict the
> > 
> > Make use "of"
> > 
> > > internal paths to the currently active routes. If a subdev doesn't
> > > implement the internal routing operations, all source pads are
> > > considered, as done today.
> > >
> > > This change is needed to properly support multiple sensors with devices
> > > such as the NXP i.MX8 ISI or the MediaTek i350 and i500 SENINF. Support
> > > for changing routes dynamically will be added later when required.
> > >
> > > Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >  src/libcamera/pipeline/simple/simple.cpp | 73 ++++++++++++++++++++++--
> > >  1 file changed, 67 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > > index 4bde9caa7254..2a8811183907 100644
> > > --- a/src/libcamera/pipeline/simple/simple.cpp
> > > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > > @@ -100,8 +100,14 @@ LOG_DEFINE_CATEGORY(SimplePipeline)
> > >   *
> > >   * During the breadth-first search, the pipeline is traversed from entity to
> > >   * entity, by following media graph links from source to sink, starting at the
> > > - * camera sensor. When reaching an entity (on its sink side), all its source
> > > - * pads are considered to continue the graph traversal.
> > > + * camera sensor.
> > > + *
> > > + * When reaching an entity (on its sink side), if the entity is a V4L2 subdev
> > > + * that supports the streams API, the subdev internal routes are followed to
> > > + * find the connected source pads. Otherwise all of the entity's source pads
> > > + * are considered to continue the graph traversal. The pipeline handler
> > > + * currently considers the default internal routes only and doesn't attempt to
> > > + * setup custom routes. This can be extended if needed.
> > >   *
> > >   * The shortest path between the camera sensor and a video node is stored in
> > >   * SimpleCameraData::entities_ as a list of SimpleCameraData::Entity structures,
> > > @@ -261,6 +267,7 @@ public:
> > >
> > >  private:
> > >  	void tryPipeline(unsigned int code, const Size &size);
> > > +	static std::vector<const MediaPad *> routedSourcePads(MediaPad *sink);
> > >
> > >  	void converterInputDone(FrameBuffer *buffer);
> > >  	void converterOutputDone(FrameBuffer *buffer);
> > > @@ -387,12 +394,29 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
> > >  			break;
> > >  		}
> > >
> > > -		/* The actual breadth-first search algorithm. */
> > >  		visited.insert(entity);
> > > -		for (MediaPad *pad : entity->pads()) {
> > > -			if (!(pad->flags() & MEDIA_PAD_FL_SOURCE))
> > > -				continue;
> > >
> > > +		/*
> > > +		 * Add direct downstream entities to the search queue. If the
> > > +		 * current entity supports the subdev internal routing API,
> > > +		 * restrict the search to downstream entities reachable through
> > > +		 * active routes.
> > > +		 */
> > > +
> > 
> > Is the emtpy line necessary ?
> 
> The code seems to still run fine without it ;-) I'll drop it.
> 
> > > +		std::vector<const MediaPad *> pads;
> > > +
> > > +		if (sinkPad)
> > > +			pads = routedSourcePads(sinkPad);
> > > +
> > > +		if (pads.empty()) {
> > 
> > pads is empty also in case of errors, like failures to open the
> > subdev.
> 
> That's right. That's actually the only failure unrelated to routing (the
> other failure would come from subdev->getRouting(), which would just
> indicate that the API isn't supported). We could handle the open failure
> here, but the subdevs are all opened in SimplePipelineHandler::match()
> just after constructing the SimpleCameraData instances, so we'll also
> catch errors there. Failures to open subdevs are really not supposed to
> happen, so as long as we handle them correctly somewhere, that's good
> enough for me I think.
> 
> > Isn't it better to consider pads.emtpy and error condition, and handle
> > the !sinkPad cases in the "routedSourcePads()" function ?
> 
> These are two different issues. The sinkPad check is only meant to skip
> the routedSourcePads() call for the camera sensor entity (that's the
> only one pushed to the queue with a nullptr for the MediaPad pointer).
> Then, pads can be empty in error cases indeed, but also if the entity
> doesn't support the routing API (not all entities do, and certainly not
> on kernels that don't have the stream series applied :-)). In that case
> the code defaults to exploring all the source pads of the entity, like
> it did before this patch.
> 
> And now that I've written that, I see it matches the code you wrote
> below :-)
> 
> > 
> > 
> >                 std::vector<> pads = connectedSources(entity, sinkPad);
> >                 if (pads.empty()) {
> >                         LOG(Error) ...
> >                         return -EINVAL;
> >                 }
> > 
> > }
> > 
> > std::vector<> SimpleCameraData::nextSources(entity)
> > {
> >         vector<> pads;
> > 
> >         for (pad : entity->pads()) {
> >                 if (pad->flags & SINK)
> >                         continue;
> > 
> >                 pads.push_back(pad);
> >         }
> > 
> >         return pads;
> > }
> > 
> > std::vector<> SimpleCameraData::connectedSources(entity, sinkPad)
> > {
> >         if (!sinkPad)
> >                 return nextSources(entity);
> > 
> >         ret = subdev->open();
> >         if (ret)
> >                 return {};
> > 
> >         ret = subdev->getRouting();
> >         if (!routing)
> >                 return nextSources(entity);
> > 
> >         ....
> > }
> 
> I was going to push back a bit because I'm lazy, but that looks cleaner
> :-) I'll give it a try.

That's actually not going to work nicely. If for any reason there's no
connected source pad, we shouldn't fail, but keep walking the graph
through all the other links to find a suitable path.

Would you be OK keeping this patch as-is ?

> > > +			for (const MediaPad *pad : entity->pads()) {
> > > +				if (!(pad->flags() & MEDIA_PAD_FL_SOURCE))
> > > +					continue;
> > > +				pads.push_back(pad);
> > > +			}
> > > +		}
> > > +
> > > +		for (const MediaPad *pad : pads) {
> > >  			for (MediaLink *link : pad->links()) {
> > >  				MediaEntity *next = link->sink()->entity();
> > >  				if (visited.find(next) == visited.end()) {
> > > @@ -782,6 +806,43 @@ void SimpleCameraData::converterOutputDone(FrameBuffer *buffer)
> > >  		pipe->completeRequest(request);
> > >  }
> > >
> > > +/* Retrieve all source pads connected to a sink pad through active routes. */
> > > +std::vector<const MediaPad *> SimpleCameraData::routedSourcePads(MediaPad *sink)
> > > +{
> > > +	MediaEntity *entity = sink->entity();
> > > +	std::unique_ptr<V4L2Subdevice> subdev =
> > > +		std::make_unique<V4L2Subdevice>(entity);
> > > +
> > > +	int ret = subdev->open();
> > > +	if (ret < 0)
> > > +		return {};
> > > +
> > > +	V4L2Subdevice::Routing routing = {};
> > > +	ret = subdev->getRouting(&routing, V4L2Subdevice::ActiveFormat);
> > > +	if (ret < 0)
> > > +		return {};
> > > +
> > > +	std::vector<const MediaPad *> pads;
> > > +
> > > +	for (const struct v4l2_subdev_route &route : routing) {
> > > +		if (sink->index() != route.sink_pad ||
> > > +		    !(route.flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
> > > +			continue;
> > > +
> > > +		const MediaPad *pad = entity->getPadByIndex(route.source_pad);
> > > +		if (!pad) {
> > > +			LOG(SimplePipeline, Warning)
> > > +				<< "Entity " << entity->name()
> > > +				<< " has invalid route source pad "
> > > +				<< route.source_pad;
> > > +		}
> > > +
> > > +		pads.push_back(pad);
> > > +	}
> > > +
> > > +	return pads;
> > > +}
> > > +
> > >  /* -----------------------------------------------------------------------------
> > >   * Camera Configuration
> > >   */
Jacopo Mondi Aug. 2, 2022, 7:48 a.m. UTC | #4 
Hi Laurent

On Mon, Aug 01, 2022 at 11:46:23PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Mon, Aug 01, 2022 at 11:36:19PM +0300, Laurent Pinchart via libcamera-devel wrote:
> > On Mon, Aug 01, 2022 at 06:16:03PM +0200, Jacopo Mondi wrote:
> > > On Mon, Aug 01, 2022 at 03:05:41AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > > > From: Phi-Bang Nguyen <pnguyen@baylibre.com>
> > > >
> > > > When traversing the media graph to discover a pipeline from the camera
> > > > sensor to a video node, all sink-to-source paths inside subdevs are
> > > > considered. This can lead to invalid paths being followed, when a subdev
> > > > has restrictions on its internal routing.
> > > >
> > > > The V4L2 API supports exposing subdev internal routing to userspace.
> > > > Make use if this feature, when implemented by a subdev, to restrict the
> > >
> > > Make use "of"
> > >
> > > > internal paths to the currently active routes. If a subdev doesn't
> > > > implement the internal routing operations, all source pads are
> > > > considered, as done today.
> > > >
> > > > This change is needed to properly support multiple sensors with devices
> > > > such as the NXP i.MX8 ISI or the MediaTek i350 and i500 SENINF. Support
> > > > for changing routes dynamically will be added later when required.
> > > >
> > > > Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > ---
> > > >  src/libcamera/pipeline/simple/simple.cpp | 73 ++++++++++++++++++++++--
> > > >  1 file changed, 67 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > > > index 4bde9caa7254..2a8811183907 100644
> > > > --- a/src/libcamera/pipeline/simple/simple.cpp
> > > > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > > > @@ -100,8 +100,14 @@ LOG_DEFINE_CATEGORY(SimplePipeline)
> > > >   *
> > > >   * During the breadth-first search, the pipeline is traversed from entity to
> > > >   * entity, by following media graph links from source to sink, starting at the
> > > > - * camera sensor. When reaching an entity (on its sink side), all its source
> > > > - * pads are considered to continue the graph traversal.
> > > > + * camera sensor.
> > > > + *
> > > > + * When reaching an entity (on its sink side), if the entity is a V4L2 subdev
> > > > + * that supports the streams API, the subdev internal routes are followed to
> > > > + * find the connected source pads. Otherwise all of the entity's source pads
> > > > + * are considered to continue the graph traversal. The pipeline handler
> > > > + * currently considers the default internal routes only and doesn't attempt to
> > > > + * setup custom routes. This can be extended if needed.
> > > >   *
> > > >   * The shortest path between the camera sensor and a video node is stored in
> > > >   * SimpleCameraData::entities_ as a list of SimpleCameraData::Entity structures,
> > > > @@ -261,6 +267,7 @@ public:
> > > >
> > > >  private:
> > > >  	void tryPipeline(unsigned int code, const Size &size);
> > > > +	static std::vector<const MediaPad *> routedSourcePads(MediaPad *sink);
> > > >
> > > >  	void converterInputDone(FrameBuffer *buffer);
> > > >  	void converterOutputDone(FrameBuffer *buffer);
> > > > @@ -387,12 +394,29 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
> > > >  			break;
> > > >  		}
> > > >
> > > > -		/* The actual breadth-first search algorithm. */
> > > >  		visited.insert(entity);
> > > > -		for (MediaPad *pad : entity->pads()) {
> > > > -			if (!(pad->flags() & MEDIA_PAD_FL_SOURCE))
> > > > -				continue;
> > > >
> > > > +		/*
> > > > +		 * Add direct downstream entities to the search queue. If the
> > > > +		 * current entity supports the subdev internal routing API,
> > > > +		 * restrict the search to downstream entities reachable through
> > > > +		 * active routes.
> > > > +		 */
> > > > +
> > >
> > > Is the emtpy line necessary ?
> >
> > The code seems to still run fine without it ;-) I'll drop it.
> >
> > > > +		std::vector<const MediaPad *> pads;
> > > > +
> > > > +		if (sinkPad)
> > > > +			pads = routedSourcePads(sinkPad);
> > > > +
> > > > +		if (pads.empty()) {
> > >
> > > pads is empty also in case of errors, like failures to open the
> > > subdev.
> >
> > That's right. That's actually the only failure unrelated to routing (the
> > other failure would come from subdev->getRouting(), which would just
> > indicate that the API isn't supported). We could handle the open failure
> > here, but the subdevs are all opened in SimplePipelineHandler::match()
> > just after constructing the SimpleCameraData instances, so we'll also
> > catch errors there. Failures to open subdevs are really not supposed to
> > happen, so as long as we handle them correctly somewhere, that's good
> > enough for me I think.
> >
> > > Isn't it better to consider pads.emtpy and error condition, and handle
> > > the !sinkPad cases in the "routedSourcePads()" function ?
> >
> > These are two different issues. The sinkPad check is only meant to skip
> > the routedSourcePads() call for the camera sensor entity (that's the
> > only one pushed to the queue with a nullptr for the MediaPad pointer).
> > Then, pads can be empty in error cases indeed, but also if the entity
> > doesn't support the routing API (not all entities do, and certainly not
> > on kernels that don't have the stream series applied :-)). In that case
> > the code defaults to exploring all the source pads of the entity, like
> > it did before this patch.
> >
> > And now that I've written that, I see it matches the code you wrote
> > below :-)
> >
> > >
> > >
> > >                 std::vector<> pads = connectedSources(entity, sinkPad);
> > >                 if (pads.empty()) {
> > >                         LOG(Error) ...
> > >                         return -EINVAL;
> > >                 }
> > >
> > > }
> > >
> > > std::vector<> SimpleCameraData::nextSources(entity)
> > > {
> > >         vector<> pads;
> > >
> > >         for (pad : entity->pads()) {
> > >                 if (pad->flags & SINK)
> > >                         continue;
> > >
> > >                 pads.push_back(pad);
> > >         }
> > >
> > >         return pads;
> > > }
> > >
> > > std::vector<> SimpleCameraData::connectedSources(entity, sinkPad)
> > > {
> > >         if (!sinkPad)
> > >                 return nextSources(entity);
> > >
> > >         ret = subdev->open();
> > >         if (ret)
> > >                 return {};
> > >
> > >         ret = subdev->getRouting();
> > >         if (!routing)
> > >                 return nextSources(entity);
> > >
> > >         ....
> > > }
> >
> > I was going to push back a bit because I'm lazy, but that looks cleaner
> > :-) I'll give it a try.
>
> That's actually not going to work nicely. If for any reason there's no
> connected source pad, we shouldn't fail, but keep walking the graph
> through all the other links to find a suitable path.
>

connected or routed ?
Anyway, I see your point, it would add yet another case to handle and
the function would grow a bit too much ?

> Would you be OK keeping this patch as-is ?
>

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

> > > > +			for (const MediaPad *pad : entity->pads()) {
> > > > +				if (!(pad->flags() & MEDIA_PAD_FL_SOURCE))
> > > > +					continue;
> > > > +				pads.push_back(pad);
> > > > +			}
> > > > +		}
> > > > +
> > > > +		for (const MediaPad *pad : pads) {
> > > >  			for (MediaLink *link : pad->links()) {
> > > >  				MediaEntity *next = link->sink()->entity();
> > > >  				if (visited.find(next) == visited.end()) {
> > > > @@ -782,6 +806,43 @@ void SimpleCameraData::converterOutputDone(FrameBuffer *buffer)
> > > >  		pipe->completeRequest(request);
> > > >  }
> > > >
> > > > +/* Retrieve all source pads connected to a sink pad through active routes. */
> > > > +std::vector<const MediaPad *> SimpleCameraData::routedSourcePads(MediaPad *sink)
> > > > +{
> > > > +	MediaEntity *entity = sink->entity();
> > > > +	std::unique_ptr<V4L2Subdevice> subdev =
> > > > +		std::make_unique<V4L2Subdevice>(entity);
> > > > +
> > > > +	int ret = subdev->open();
> > > > +	if (ret < 0)
> > > > +		return {};
> > > > +
> > > > +	V4L2Subdevice::Routing routing = {};
> > > > +	ret = subdev->getRouting(&routing, V4L2Subdevice::ActiveFormat);
> > > > +	if (ret < 0)
> > > > +		return {};
> > > > +
> > > > +	std::vector<const MediaPad *> pads;
> > > > +
> > > > +	for (const struct v4l2_subdev_route &route : routing) {
> > > > +		if (sink->index() != route.sink_pad ||
> > > > +		    !(route.flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
> > > > +			continue;
> > > > +
> > > > +		const MediaPad *pad = entity->getPadByIndex(route.source_pad);
> > > > +		if (!pad) {
> > > > +			LOG(SimplePipeline, Warning)
> > > > +				<< "Entity " << entity->name()
> > > > +				<< " has invalid route source pad "
> > > > +				<< route.source_pad;
> > > > +		}
> > > > +
> > > > +		pads.push_back(pad);
> > > > +	}
> > > > +
> > > > +	return pads;
> > > > +}
> > > > +
> > > >  /* -----------------------------------------------------------------------------
> > > >   * Camera Configuration
> > > >   */
>
> --
> Regards,
>
> Laurent Pinchart
Laurent Pinchart Aug. 2, 2022, 10:24 a.m. UTC | #5 
Hi Jacopo,

On Tue, Aug 02, 2022 at 09:48:30AM +0200, Jacopo Mondi wrote:
> On Mon, Aug 01, 2022 at 11:46:23PM +0300, Laurent Pinchart wrote:
> > On Mon, Aug 01, 2022 at 11:36:19PM +0300, Laurent Pinchart via libcamera-devel wrote:
> > > On Mon, Aug 01, 2022 at 06:16:03PM +0200, Jacopo Mondi wrote:
> > > > On Mon, Aug 01, 2022 at 03:05:41AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > > > > From: Phi-Bang Nguyen <pnguyen@baylibre.com>
> > > > >
> > > > > When traversing the media graph to discover a pipeline from the camera
> > > > > sensor to a video node, all sink-to-source paths inside subdevs are
> > > > > considered. This can lead to invalid paths being followed, when a subdev
> > > > > has restrictions on its internal routing.
> > > > >
> > > > > The V4L2 API supports exposing subdev internal routing to userspace.
> > > > > Make use if this feature, when implemented by a subdev, to restrict the
> > > >
> > > > Make use "of"
> > > >
> > > > > internal paths to the currently active routes. If a subdev doesn't
> > > > > implement the internal routing operations, all source pads are
> > > > > considered, as done today.
> > > > >
> > > > > This change is needed to properly support multiple sensors with devices
> > > > > such as the NXP i.MX8 ISI or the MediaTek i350 and i500 SENINF. Support
> > > > > for changing routes dynamically will be added later when required.
> > > > >
> > > > > Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>
> > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > > ---
> > > > >  src/libcamera/pipeline/simple/simple.cpp | 73 ++++++++++++++++++++++--
> > > > >  1 file changed, 67 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > > > > index 4bde9caa7254..2a8811183907 100644
> > > > > --- a/src/libcamera/pipeline/simple/simple.cpp
> > > > > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > > > > @@ -100,8 +100,14 @@ LOG_DEFINE_CATEGORY(SimplePipeline)
> > > > >   *
> > > > >   * During the breadth-first search, the pipeline is traversed from entity to
> > > > >   * entity, by following media graph links from source to sink, starting at the
> > > > > - * camera sensor. When reaching an entity (on its sink side), all its source
> > > > > - * pads are considered to continue the graph traversal.
> > > > > + * camera sensor.
> > > > > + *
> > > > > + * When reaching an entity (on its sink side), if the entity is a V4L2 subdev
> > > > > + * that supports the streams API, the subdev internal routes are followed to
> > > > > + * find the connected source pads. Otherwise all of the entity's source pads
> > > > > + * are considered to continue the graph traversal. The pipeline handler
> > > > > + * currently considers the default internal routes only and doesn't attempt to
> > > > > + * setup custom routes. This can be extended if needed.
> > > > >   *
> > > > >   * The shortest path between the camera sensor and a video node is stored in
> > > > >   * SimpleCameraData::entities_ as a list of SimpleCameraData::Entity structures,
> > > > > @@ -261,6 +267,7 @@ public:
> > > > >
> > > > >  private:
> > > > >  	void tryPipeline(unsigned int code, const Size &size);
> > > > > +	static std::vector<const MediaPad *> routedSourcePads(MediaPad *sink);
> > > > >
> > > > >  	void converterInputDone(FrameBuffer *buffer);
> > > > >  	void converterOutputDone(FrameBuffer *buffer);
> > > > > @@ -387,12 +394,29 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
> > > > >  			break;
> > > > >  		}
> > > > >
> > > > > -		/* The actual breadth-first search algorithm. */
> > > > >  		visited.insert(entity);
> > > > > -		for (MediaPad *pad : entity->pads()) {
> > > > > -			if (!(pad->flags() & MEDIA_PAD_FL_SOURCE))
> > > > > -				continue;
> > > > >
> > > > > +		/*
> > > > > +		 * Add direct downstream entities to the search queue. If the
> > > > > +		 * current entity supports the subdev internal routing API,
> > > > > +		 * restrict the search to downstream entities reachable through
> > > > > +		 * active routes.
> > > > > +		 */
> > > > > +
> > > >
> > > > Is the emtpy line necessary ?
> > >
> > > The code seems to still run fine without it ;-) I'll drop it.
> > >
> > > > > +		std::vector<const MediaPad *> pads;
> > > > > +
> > > > > +		if (sinkPad)
> > > > > +			pads = routedSourcePads(sinkPad);
> > > > > +
> > > > > +		if (pads.empty()) {
> > > >
> > > > pads is empty also in case of errors, like failures to open the
> > > > subdev.
> > >
> > > That's right. That's actually the only failure unrelated to routing (the
> > > other failure would come from subdev->getRouting(), which would just
> > > indicate that the API isn't supported). We could handle the open failure
> > > here, but the subdevs are all opened in SimplePipelineHandler::match()
> > > just after constructing the SimpleCameraData instances, so we'll also
> > > catch errors there. Failures to open subdevs are really not supposed to
> > > happen, so as long as we handle them correctly somewhere, that's good
> > > enough for me I think.
> > >
> > > > Isn't it better to consider pads.emtpy and error condition, and handle
> > > > the !sinkPad cases in the "routedSourcePads()" function ?
> > >
> > > These are two different issues. The sinkPad check is only meant to skip
> > > the routedSourcePads() call for the camera sensor entity (that's the
> > > only one pushed to the queue with a nullptr for the MediaPad pointer).
> > > Then, pads can be empty in error cases indeed, but also if the entity
> > > doesn't support the routing API (not all entities do, and certainly not
> > > on kernels that don't have the stream series applied :-)). In that case
> > > the code defaults to exploring all the source pads of the entity, like
> > > it did before this patch.
> > >
> > > And now that I've written that, I see it matches the code you wrote
> > > below :-)
> > >
> > > >
> > > >
> > > >                 std::vector<> pads = connectedSources(entity, sinkPad);
> > > >                 if (pads.empty()) {
> > > >                         LOG(Error) ...
> > > >                         return -EINVAL;
> > > >                 }
> > > >
> > > > }
> > > >
> > > > std::vector<> SimpleCameraData::nextSources(entity)
> > > > {
> > > >         vector<> pads;
> > > >
> > > >         for (pad : entity->pads()) {
> > > >                 if (pad->flags & SINK)
> > > >                         continue;
> > > >
> > > >                 pads.push_back(pad);
> > > >         }
> > > >
> > > >         return pads;
> > > > }
> > > >
> > > > std::vector<> SimpleCameraData::connectedSources(entity, sinkPad)
> > > > {
> > > >         if (!sinkPad)
> > > >                 return nextSources(entity);
> > > >
> > > >         ret = subdev->open();
> > > >         if (ret)
> > > >                 return {};
> > > >
> > > >         ret = subdev->getRouting();
> > > >         if (!routing)
> > > >                 return nextSources(entity);
> > > >
> > > >         ....
> > > > }
> > >
> > > I was going to push back a bit because I'm lazy, but that looks cleaner
> > > :-) I'll give it a try.
> >
> > That's actually not going to work nicely. If for any reason there's no
> > connected source pad, we shouldn't fail, but keep walking the graph
> > through all the other links to find a suitable path.
> 
> connected or routed ?

I meant routed, sorry.

> Anyway, I see your point, it would add yet another case to handle and
> the function would grow a bit too much ?

My point was that when there's no connected pad, the function can only
return an empty vector, and that's not an error. If we wanted to add
error handling here, we would need to return both an error code and a
vector (with std::tuple, std::variant, ...).

> > Would you be OK keeping this patch as-is ?
> 
> OK
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> > > > > +			for (const MediaPad *pad : entity->pads()) {
> > > > > +				if (!(pad->flags() & MEDIA_PAD_FL_SOURCE))
> > > > > +					continue;
> > > > > +				pads.push_back(pad);
> > > > > +			}
> > > > > +		}
> > > > > +
> > > > > +		for (const MediaPad *pad : pads) {
> > > > >  			for (MediaLink *link : pad->links()) {
> > > > >  				MediaEntity *next = link->sink()->entity();
> > > > >  				if (visited.find(next) == visited.end()) {
> > > > > @@ -782,6 +806,43 @@ void SimpleCameraData::converterOutputDone(FrameBuffer *buffer)
> > > > >  		pipe->completeRequest(request);
> > > > >  }
> > > > >
> > > > > +/* Retrieve all source pads connected to a sink pad through active routes. */
> > > > > +std::vector<const MediaPad *> SimpleCameraData::routedSourcePads(MediaPad *sink)
> > > > > +{
> > > > > +	MediaEntity *entity = sink->entity();
> > > > > +	std::unique_ptr<V4L2Subdevice> subdev =
> > > > > +		std::make_unique<V4L2Subdevice>(entity);
> > > > > +
> > > > > +	int ret = subdev->open();
> > > > > +	if (ret < 0)
> > > > > +		return {};
> > > > > +
> > > > > +	V4L2Subdevice::Routing routing = {};
> > > > > +	ret = subdev->getRouting(&routing, V4L2Subdevice::ActiveFormat);
> > > > > +	if (ret < 0)
> > > > > +		return {};
> > > > > +
> > > > > +	std::vector<const MediaPad *> pads;
> > > > > +
> > > > > +	for (const struct v4l2_subdev_route &route : routing) {
> > > > > +		if (sink->index() != route.sink_pad ||
> > > > > +		    !(route.flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
> > > > > +			continue;
> > > > > +
> > > > > +		const MediaPad *pad = entity->getPadByIndex(route.source_pad);
> > > > > +		if (!pad) {
> > > > > +			LOG(SimplePipeline, Warning)
> > > > > +				<< "Entity " << entity->name()
> > > > > +				<< " has invalid route source pad "
> > > > > +				<< route.source_pad;
> > > > > +		}
> > > > > +
> > > > > +		pads.push_back(pad);
> > > > > +	}
> > > > > +
> > > > > +	return pads;
> > > > > +}
> > > > > +
> > > > >  /* -----------------------------------------------------------------------------
> > > > >   * Camera Configuration
> > > > >   */

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 4bde9caa7254..2a8811183907 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -100,8 +100,14 @@  LOG_DEFINE_CATEGORY(SimplePipeline)
  *
  * During the breadth-first search, the pipeline is traversed from entity to
  * entity, by following media graph links from source to sink, starting at the
- * camera sensor. When reaching an entity (on its sink side), all its source
- * pads are considered to continue the graph traversal.
+ * camera sensor.
+ *
+ * When reaching an entity (on its sink side), if the entity is a V4L2 subdev
+ * that supports the streams API, the subdev internal routes are followed to
+ * find the connected source pads. Otherwise all of the entity's source pads
+ * are considered to continue the graph traversal. The pipeline handler
+ * currently considers the default internal routes only and doesn't attempt to
+ * setup custom routes. This can be extended if needed.
  *
  * The shortest path between the camera sensor and a video node is stored in
  * SimpleCameraData::entities_ as a list of SimpleCameraData::Entity structures,
@@ -261,6 +267,7 @@  public:
 
 private:
 	void tryPipeline(unsigned int code, const Size &size);
+	static std::vector<const MediaPad *> routedSourcePads(MediaPad *sink);
 
 	void converterInputDone(FrameBuffer *buffer);
 	void converterOutputDone(FrameBuffer *buffer);
@@ -387,12 +394,29 @@  SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
 			break;
 		}
 
-		/* The actual breadth-first search algorithm. */
 		visited.insert(entity);
-		for (MediaPad *pad : entity->pads()) {
-			if (!(pad->flags() & MEDIA_PAD_FL_SOURCE))
-				continue;
 
+		/*
+		 * Add direct downstream entities to the search queue. If the
+		 * current entity supports the subdev internal routing API,
+		 * restrict the search to downstream entities reachable through
+		 * active routes.
+		 */
+
+		std::vector<const MediaPad *> pads;
+
+		if (sinkPad)
+			pads = routedSourcePads(sinkPad);
+
+		if (pads.empty()) {
+			for (const MediaPad *pad : entity->pads()) {
+				if (!(pad->flags() & MEDIA_PAD_FL_SOURCE))
+					continue;
+				pads.push_back(pad);
+			}
+		}
+
+		for (const MediaPad *pad : pads) {
 			for (MediaLink *link : pad->links()) {
 				MediaEntity *next = link->sink()->entity();
 				if (visited.find(next) == visited.end()) {
@@ -782,6 +806,43 @@  void SimpleCameraData::converterOutputDone(FrameBuffer *buffer)
 		pipe->completeRequest(request);
 }
 
+/* Retrieve all source pads connected to a sink pad through active routes. */
+std::vector<const MediaPad *> SimpleCameraData::routedSourcePads(MediaPad *sink)
+{
+	MediaEntity *entity = sink->entity();
+	std::unique_ptr<V4L2Subdevice> subdev =
+		std::make_unique<V4L2Subdevice>(entity);
+
+	int ret = subdev->open();
+	if (ret < 0)
+		return {};
+
+	V4L2Subdevice::Routing routing = {};
+	ret = subdev->getRouting(&routing, V4L2Subdevice::ActiveFormat);
+	if (ret < 0)
+		return {};
+
+	std::vector<const MediaPad *> pads;
+
+	for (const struct v4l2_subdev_route &route : routing) {
+		if (sink->index() != route.sink_pad ||
+		    !(route.flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
+			continue;
+
+		const MediaPad *pad = entity->getPadByIndex(route.source_pad);
+		if (!pad) {
+			LOG(SimplePipeline, Warning)
+				<< "Entity " << entity->name()
+				<< " has invalid route source pad "
+				<< route.source_pad;
+		}
+
+		pads.push_back(pad);
+	}
+
+	return pads;
+}
+
 /* -----------------------------------------------------------------------------
  * Camera Configuration
  */