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

Message ID 20220131223330.61563-4-djrscally@gmail.com
State Superseded
Headers show
Series
  • Enumerate CameraLens by following sensor's ancillary links
Related show

Commit Message

Daniel Scally Jan. 31, 2022, 10:33 p.m. UTC
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(-)

Comments

Laurent Pinchart Feb. 3, 2022, 12:18 a.m. UTC | #1
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;

Patch
diff mbox series

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
+		 * sink pointers would never 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: "
 				<< 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;
+		}
+		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;
 		}
+		default:
+			LOG(MediaDevice, Warning)
+				<< "Unknown media link type";
 
-		source->addLink(link);
-		sink->addLink(link);
+			break;
+		}
 	}
 
 	return true;