[libcamera-devel] [PATCH 1/5] libcamera: Add support for ancillary links to MediaLink

[Laurent Pinchart]

Hi Daniel,

Thank you for the patch.

On Fri, Nov 26, 2021 at 12:31:14AM +0000, Daniel Scally wrote:
> Update the MediaLink class to include members suitable for the new
> type of media_v2_link, connecting two instances of MediaEntity
> rather than MediaPads
> 
> Signed-off-by: Daniel Scally <djrscally at gmail.com>
> ---
> 
> Adding new members and a new constructor here seemed in the end like the least
> impactful and probably cleanest method of doing this, as otherwise the source
> and sink would need to become MediaObjects and be cast everywhere.

I'm not really thrilled by this. Given that the purpose of ancillary
links is to expose the relationship between a primary device and its
ancillary devices, and given that those relationships don't carry any
other property than their existence, I think adding a

	const std::vector<MediaEntity *> ancillaryEntities() const;

(naming bikeshedding accepted) function to the MediaEntity class should
be enough, without a need to extend the MediaLink API.

>  include/libcamera/internal/media_object.h | 10 ++++++++++
>  src/libcamera/media_object.cpp            | 24 ++++++++++++++++++++++-
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h
> index 6ae22c67..79c71325 100644
> --- a/include/libcamera/internal/media_object.h
> +++ b/include/libcamera/internal/media_object.h
> @@ -45,6 +45,8 @@ class MediaLink : public MediaObject
>  public:
>  	MediaPad *source() const { return source_; }
>  	MediaPad *sink() const { return sink_; }
> +	MediaEntity *primary() const { return primary_; };
> +	MediaEntity *ancillary() const { return ancillary_; };
>  	unsigned int flags() const { return flags_; }
>  	int setEnabled(bool enable);
>  
> @@ -55,9 +57,13 @@ private:
>  
>  	MediaLink(const struct media_v2_link *link,
>  		  MediaPad *source, MediaPad *sink);
> +	MediaLink(const struct media_v2_link *link,
> +		  MediaEntity *primary, MediaEntity *ancillary);
>  
>  	MediaPad *source_;
>  	MediaPad *sink_;
> +	MediaEntity *primary_;
> +	MediaEntity *ancillary_;
>  	unsigned int flags_;
>  };
>  
> @@ -104,12 +110,15 @@ public:
>  	unsigned int deviceMinor() const { return minor_; }
>  
>  	const std::vector<MediaPad *> &pads() const { return pads_; }
> +	const std::vector<MediaLink *> &ancillary_links() const { return ancillary_links_; }
>  
>  	const MediaPad *getPadByIndex(unsigned int index) const;
>  	const MediaPad *getPadById(unsigned int id) const;
>  
>  	int setDeviceNode(const std::string &deviceNode);
>  
> +	void addLink(MediaLink *link);
> +
>  private:
>  	LIBCAMERA_DISABLE_COPY_AND_MOVE(MediaEntity)
>  
> @@ -129,6 +138,7 @@ private:
>  	unsigned int minor_;
>  
>  	std::vector<MediaPad *> pads_;
> +	std::vector<MediaLink *> ancillary_links_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp
> index f425d044..e903d5ef 100644
> --- a/src/libcamera/media_object.cpp
> +++ b/src/libcamera/media_object.cpp
> @@ -134,7 +134,7 @@ int MediaLink::setEnabled(bool enable)
>  }
>  
>  /**
> - * \brief Construct a MediaLink
> + * \brief Construct a MediaLink between two pads
>   * \param[in] link The media link kernel data
>   * \param[in] source The source pad at the origin of the link
>   * \param[in] sink The sink pad at the destination of the link
> @@ -146,6 +146,19 @@ MediaLink::MediaLink(const struct media_v2_link *link, MediaPad *source,
>  {
>  }
>  
> +/**
> + * \brief Construct a MediaLink between two entities
> + * \param[in] link The media link kernel data
> + * \param[in] primary The primary entity at the origin of the link
> + * \param[in] ancillary The ancillary entity at the destination of the link
> + */
> +MediaLink::MediaLink(const struct media_v2_link *link, MediaEntity *primary,
> +		     MediaEntity *ancillary)
> +	: MediaObject(primary->device(), link->id), primary_(primary),
> +	  ancillary_(ancillary), flags_(link->flags)
> +{
> +}
> +
>  /**
>   * \fn MediaLink::source()
>   * \brief Retrieve the link's source pad
> @@ -378,6 +391,15 @@ int MediaEntity::setDeviceNode(const std::string &deviceNode)
>  	return 0;
>  }
>  
> +/**
> + * \brief Add an ancillary link to the MediaEntity
> + * \param[in] link Pointer to the MediaLink class
> + */
> +void MediaEntity::addLink(MediaLink *link)
> +{
> +	ancillary_links_.push_back(link);
> +}
> +
>  /**
>   * \brief Construct a MediaEntity
>   * \param[in] dev The media device this entity belongs to

-- 
Regards,

Laurent Pinchart