[libcamera-devel] [PATCH] libcamera: pipeline: simple: walk the pipline by following the first link

[Laurent Pinchart]

Hi Dafna,

Thank you for the patch.

s/pipline/pipeline/ on the subject line.

On Wed, Mar 03, 2021 at 03:16:44PM +0100, Dafna Hirschfeld wrote:
> When walking the pipeline, follow the first link of each
> source pad.
> This patch removes a redundant condition for choosing the link:
> 
> "(link->flags() & MEDIA_LNK_FL_ENABLED) ||
>  !(link->flags() & MEDIA_LNK_FL_IMMUTABLE)"
> 
> since it always returns true.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld at collabora.com>

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

> ---
> 
> Note, I could only make sure that the patch compiles, but I don't have
> the hardware to test it.

Tested-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

:-)

> 
>  src/libcamera/pipeline/simple/simple.cpp | 23 ++++++-----------------
>  1 file changed, 6 insertions(+), 17 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 7fba495c..1aeabcf7 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -279,12 +279,17 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
>  		if (source->function() == MEDIA_ENT_F_IO_V4L)
>  			break;
>  
> -		/* Use the first output pad that has links. */
> +		/*
> +		 * Use the first output pad that has links and follow its first
> +		 * link.
> +		 */
>  		MediaPad *sourcePad = nullptr;
> +		MediaLink *sourceLink = nullptr;
>  		for (MediaPad *pad : source->pads()) {
>  			if ((pad->flags() & MEDIA_PAD_FL_SOURCE) &&
>  			    !pad->links().empty()) {
>  				sourcePad = pad;
> +				sourceLink = pad->links().front();
>  				break;
>  			}
>  		}
> @@ -292,22 +297,6 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
>  		if (!sourcePad)
>  			return;
>  
> -		/*
> -		 * Use the first link that is enabled or can be enabled (not
> -		 * immutable).
> -		 */
> -		MediaLink *sourceLink = nullptr;
> -		for (MediaLink *link : sourcePad->links()) {
> -			if ((link->flags() & MEDIA_LNK_FL_ENABLED) ||
> -			    !(link->flags() & MEDIA_LNK_FL_IMMUTABLE)) {
> -				sourceLink = link;
> -				break;
> -			}
> -		}
> -
> -		if (!sourceLink)
> -			return;
> -
>  		entities_.push_back({ source, sourceLink });
>  
>  		source = sourceLink->sink()->entity();

-- 
Regards,

Laurent Pinchart