[libcamera-devel,v4,3/9] libcamera: media_device: Handle ancillary links in populateLinks()

15308 diff mbox series

The populateLinks() function can't currently handle ancillary links
which causes an error to be thrown in process() when the MediaObject
cannot be cast to a MediaPad.

Add explicit handling for the different link types, creating either
pad-2-pad links or else storing the pointer to the ancillary device
MediaEntity in the ancillaryEntities_ member of the primary.

Signed-off-by: Daniel Scally <djrscally@gmail.com>
---

Changes in v4:

	- Added some error checking to confirm that the casts to pad or entity
	return valid pointers (Laurent)

Changes in v3:

	- Split out the new macro
	- Fixed some style errors and comments
	- Added a default case

 src/libcamera/media_device.cpp | 67 ++++++++++++++++++++++++++--------
 1 file changed, 51 insertions(+), 16 deletions(-)

Hi Dan,

Thank you for the patch.

On Mon, Jan 31, 2022 at 10:33:24PM +0000, Daniel Scally wrote:
> The populateLinks() function can't currently handle ancillary links
> which causes an error to be thrown in process() when the MediaObject
> cannot be cast to a MediaPad.
> 
> Add explicit handling for the different link types, creating either
> pad-2-pad links or else storing the pointer to the ancillary device
> MediaEntity in the ancillaryEntities_ member of the primary.
> 
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> 
> Changes in v4:
> 
> 	- Added some error checking to confirm that the casts to pad or entity
> 	return valid pointers (Laurent)
> 
> Changes in v3:
> 
> 	- Split out the new macro
> 	- Fixed some style errors and comments
> 	- Added a default case
> 
>  src/libcamera/media_device.cpp | 67 ++++++++++++++++++++++++++--------
>  1 file changed, 51 insertions(+), 16 deletions(-)
> 
> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> index 941f86c2..a008383a 100644
> --- a/src/libcamera/media_device.cpp
> +++ b/src/libcamera/media_device.cpp
> @@ -683,40 +683,75 @@ bool MediaDevice::populateLinks(const struct media_v2_topology &topology)
>  					   (topology.ptr_links);
>  
>  	for (unsigned int i = 0; i < topology.num_links; ++i) {
> -		/* We only care about pad-2-pad links here. */
> -		if ((mediaLinks[i].flags & MEDIA_LNK_FL_LINK_TYPE) !=
> -		    MEDIA_LNK_FL_DATA_LINK)
> +		/*
> +		 * Skip links between entities and interfaces: interfaces are
> +		 * not created as MediaObjects at this time, so the source and

s/MediaObjects/MediaObject instances/

> +		 * sink pointers would never be found.

Isn't it only the sink pointer that can't be found ?

> +		 */
> +		if ((mediaLinks[i].flags & MEDIA_LNK_FL_LINK_TYPE) ==
> +		    MEDIA_LNK_FL_INTERFACE_LINK)
>  			continue;
>  
> -		/* Store references to source and sink pads in the link. */
> +		/* Look up the source and sink objects. */
>  		unsigned int source_id = mediaLinks[i].source_id;
> -		MediaPad *source = dynamic_cast<MediaPad *>
> -				   (object(source_id));
> +		MediaObject *source = object(source_id);
>  		if (!source) {
>  			LOG(MediaDevice, Error)
> -				<< "Failed to find pad with id: "
> +				<< "Failed to find MediaObject with id: "

While at it, s/://. Same below.

>  				<< source_id;
>  			return false;
>  		}
>  
>  		unsigned int sink_id = mediaLinks[i].sink_id;
> -		MediaPad *sink = dynamic_cast<MediaPad *>
> -				 (object(sink_id));
> +		MediaObject *sink = object(sink_id);
>  		if (!sink) {
>  			LOG(MediaDevice, Error)
> -				<< "Failed to find pad with id: "
> +				<< "Failed to find MediaObject with id: "
>  				<< sink_id;
>  			return false;
>  		}
>  
> -		MediaLink *link = new MediaLink(&mediaLinks[i], source, sink);
> -		if (!addObject(link)) {
> -			delete link;
> -			return false;
> +		switch (mediaLinks[i].flags & MEDIA_LNK_FL_LINK_TYPE) {
> +		case MEDIA_LNK_FL_DATA_LINK: {
> +			MediaPad *sourcePad = dynamic_cast<MediaPad *>(source);
> +			MediaPad *sinkPad = dynamic_cast<MediaPad *>(sink);
> +			if (!source || !sink) {
> +				LOG(MediaDevice, Error)
> +					<< "Source or sink is not a pad";
> +				return false;
> +			}
> +
> +			MediaLink *link = new MediaLink(&mediaLinks[i],
> +							sourcePad, sinkPad);
> +			if (!addObject(link)) {
> +				delete link;
> +				return false;
> +			}
> +
> +			link->source()->addLink(link);
> +			link->sink()->addLink(link);
> +
> +			break;
> +		}

Blank line here.

> +		case MEDIA_LNK_FL_ANCILLARY_LINK: {
> +			MediaEntity *primary = dynamic_cast<MediaEntity *>(source);
> +			MediaEntity *ancillary = dynamic_cast<MediaEntity *>(sink);
> +			if (!primary || !ancillary) {
> +				LOG(MediaDevice, Error)
> +					<< "Source or sink is not an entity";
> +				return false;
> +			}
> +
> +			primary->addAncillaryEntity(ancillary);
> +
> +			break;
>  		}

Here too.

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

> +		default:
> +			LOG(MediaDevice, Warning)
> +				<< "Unknown media link type";
>  
> -		source->addLink(link);
> -		sink->addLink(link);
> +			break;
> +		}
>  	}
>  
>  	return true;