[libcamera-devel] libcamera: media_device: Skip all non-data links during enumeration

15054 diff mbox series

The MediaDevice::populateLinks() function iterates over data links by
skipping interface links. This isn't very future-proof, it will break if
the kernel adds new types of links. Fix it by only considering data
links instead of blacklisting interface links.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Let's merge this ASAP to avoid bug reports when using an older libcamera
version with a kernel that supports the soon-to-be-added ancillary
links.
---
 src/libcamera/media_device.cpp | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)


base-commit: 294663eece8c067d268442724b969c9dfa081b0a

Hi Laurent,

On 06/12/2021 19:50, Laurent Pinchart wrote:
> The MediaDevice::populateLinks() function iterates over data links by
> skipping interface links. This isn't very future-proof, it will break if
> the kernel adds new types of links. Fix it by only considering data
> links instead of blacklisting interface links.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks, it works like a charm :-).
Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>

> ---
> Let's merge this ASAP to avoid bug reports when using an older libcamera
> version with a kernel that supports the soon-to-be-added ancillary
> links.
> ---
>   src/libcamera/media_device.cpp | 9 +++------
>   1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> index 4df0a27fe193..0b7940182d0c 100644
> --- a/src/libcamera/media_device.cpp
> +++ b/src/libcamera/media_device.cpp
> @@ -695,12 +695,9 @@ bool MediaDevice::populateLinks(const struct media_v2_topology &topology)
>   					   (topology.ptr_links);
>   
>   	for (unsigned int i = 0; i < topology.num_links; ++i) {
> -		/*
> -		 * Skip links between entities and interfaces: we only care
> -		 * about pad-2-pad links here.
> -		 */
> -		if ((mediaLinks[i].flags & MEDIA_LNK_FL_LINK_TYPE) ==
> -		    MEDIA_LNK_FL_INTERFACE_LINK)
> +		/* We only care about pad-2-pad links here. */
> +		if ((mediaLinks[i].flags & MEDIA_LNK_FL_LINK_TYPE) !=
> +		    MEDIA_LNK_FL_DATA_LINK)
>   			continue;
>   
>   		/* Store references to source and sink pads in the link. */
> 
> base-commit: 294663eece8c067d268442724b969c9dfa081b0a
>

Quoting Laurent Pinchart (2021-12-06 18:50:47)
> The MediaDevice::populateLinks() function iterates over data links by
> skipping interface links. This isn't very future-proof, it will break if
> the kernel adds new types of links. Fix it by only considering data
> links instead of blacklisting interface links.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Let's merge this ASAP to avoid bug reports when using an older libcamera
> version with a kernel that supports the soon-to-be-added ancillary
> links.

Looks fine to me.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/libcamera/media_device.cpp | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> index 4df0a27fe193..0b7940182d0c 100644
> --- a/src/libcamera/media_device.cpp
> +++ b/src/libcamera/media_device.cpp
> @@ -695,12 +695,9 @@ bool MediaDevice::populateLinks(const struct media_v2_topology &topology)
>                                            (topology.ptr_links);
>  
>         for (unsigned int i = 0; i < topology.num_links; ++i) {
> -               /*
> -                * Skip links between entities and interfaces: we only care
> -                * about pad-2-pad links here.
> -                */
> -               if ((mediaLinks[i].flags & MEDIA_LNK_FL_LINK_TYPE) ==
> -                   MEDIA_LNK_FL_INTERFACE_LINK)
> +               /* We only care about pad-2-pad links here. */
> +               if ((mediaLinks[i].flags & MEDIA_LNK_FL_LINK_TYPE) !=
> +                   MEDIA_LNK_FL_DATA_LINK)
>                         continue;
>  
>                 /* Store references to source and sink pads in the link. */
> 
> base-commit: 294663eece8c067d268442724b969c9dfa081b0a
> -- 
> Regards,
> 
> Laurent Pinchart
>