[libcamera-devel] [PATCH v3 1/9] libcamera: Add members to MediaEntity to support ancillary entities

[Laurent Pinchart]

Hi Daniel,

Thank you for the patch.

On Tue, Dec 07, 2021 at 10:45:04PM +0000, Daniel Scally wrote:
> With kernel support for ancillary links, we can describe the
> relationship between two devices represented individually as instances
> of MediaEntity. As the only property of that relationship is its
> existence, describe those relationships in libcamera simply as a
> vector of MediaEntity pointers to the ancillary devices.
> 
> Signed-off-by: Daniel Scally <djrscally at gmail.com>
> ---
> Changes in v3:
> 
> 	- Fixed some style issues
> 	- Made addAncillaryEntity() private

General note, you could keep the changelog for previous versions. I
usually include them in my commit message (below ---) when I develop,
and only strip them off when upstreaming.

>  include/libcamera/internal/media_object.h |  4 ++++
>  src/libcamera/media_object.cpp            | 15 +++++++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h
> index 90c63598..b1572968 100644
> --- a/include/libcamera/internal/media_object.h
> +++ b/include/libcamera/internal/media_object.h
> @@ -104,6 +104,7 @@ public:
>  	unsigned int deviceMinor() const { return minor_; }
>  
>  	const std::vector<MediaPad *> &pads() const { return pads_; }
> +	const std::vector<MediaEntity *> ancillaryEntities() const { return ancillaryEntities_; }
>  
>  	const MediaPad *getPadByIndex(unsigned int index) const;
>  	const MediaPad *getPadById(unsigned int id) const;
> @@ -120,6 +121,8 @@ private:
>  
>  	void addPad(MediaPad *pad);
>  
> +	void addAncillaryEntity(MediaEntity *ancillaryEntity);
> +
>  	std::string name_;
>  	unsigned int function_;
>  	unsigned int flags_;
> @@ -129,6 +132,7 @@ private:
>  	unsigned int minor_;
>  
>  	std::vector<MediaPad *> pads_;
> +	std::vector<MediaEntity *> ancillaryEntities_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp
> index f425d044..dbcf10e2 100644
> --- a/src/libcamera/media_object.cpp
> +++ b/src/libcamera/media_object.cpp
> @@ -326,6 +326,21 @@ void MediaPad::addLink(MediaLink *link)
>   * \return The list of the entity's pads
>   */
>  
> +/**
> + * \fn MediaEntity::ancillaryEntities()
> + * \brief Retrieve all ancillary entities of the entity
> + * \return The list of the entity's ancillary entities
> + */
> +
> +/**
> + * \brief Add a MediaEntity to the list of ancillary entities
> + * \param[in] ancillaryEntity The instance of MediaEntity to add
> + */
> +void MediaEntity::addAncillaryEntity(MediaEntity *ancillaryEntity)
> +{
> +	ancillaryEntities_.push_back(ancillaryEntity);
> +}
> +

We try to match the order of functions in the .h and .cpp file (at least
within the public, protected and private categories, the categories
themselves can be interleaved). This function should thus be moved right
after addPad().

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

>  /**
>   * \brief Get a pad in this entity by its index
>   * \param[in] index The 0-based pad index

-- 
Regards,

Laurent Pinchart