[libcamera-devel,v2,1/7] libcamera: Add members to MediaEntity to support ancillary entities

15031 diff mbox series

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@gmail.com>
---
Changes in v2:

	- Switched from additional members against MediaLink pointing to the
	primary and ancillary entity to a vector against MediaEntity holding
	pointers to the ancillary entities linked to that MediaEntity (Laurent)

 include/libcamera/internal/media_object.h |  4 ++++
 src/libcamera/media_object.cpp            | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)

Hi Daniel,

Thank you for the patch.

On Fri, Dec 03, 2021 at 10:42:24PM +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@gmail.com>
> ---
> Changes in v2:
> 
> 	- Switched from additional members against MediaLink pointing to the
> 	primary and ancillary entity to a vector against MediaEntity holding
> 	pointers to the ancillary entities linked to that MediaEntity (Laurent)
> 
>  include/libcamera/internal/media_object.h |  4 ++++
>  src/libcamera/media_object.cpp            | 16 ++++++++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h
> index 90c63598..48616a43 100644
> --- a/include/libcamera/internal/media_object.h
> +++ b/include/libcamera/internal/media_object.h
> @@ -104,6 +104,9 @@ public:
>  	unsigned int deviceMinor() const { return minor_; }
>  
>  	const std::vector<MediaPad *> &pads() const { return pads_; }
> +	const std::vector<MediaEntity *> ancillaryEntities() const {return ancillaryEntities_; }

Missing space before return. checkstyle.py should have warned you (see
Documentation/coding-style.rst for instructions to automate style
checking with a git commit hook).

> +
> +	void addAncillaryEntity(MediaEntity *ancillaryEntity);

As this is called by MediaDevice only, which is a friend of this class,
you can make the function private.

>  
>  	const MediaPad *getPadByIndex(unsigned int index) const;
>  	const MediaPad *getPadById(unsigned int id) const;
> @@ -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..42b26b07 100644
> --- a/src/libcamera/media_object.cpp
> +++ b/src/libcamera/media_object.cpp
> @@ -326,6 +326,22 @@ 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

s/the/The/

> + * \return void

No need for \return for functions that don't return anything.

> + */
> +void MediaEntity::addAncillaryEntity(MediaEntity *ancillaryEntity)
> +{
> +	ancillaryEntities_.push_back(ancillaryEntity);
> +}
> +
>  /**
>   * \brief Get a pad in this entity by its index
>   * \param[in] index The 0-based pad index

Hi Laurent

On 04/12/2021 00:03, Laurent Pinchart wrote:
> Hi Daniel,
> 
> Thank you for the patch.
> 
> On Fri, Dec 03, 2021 at 10:42:24PM +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@gmail.com>
>> ---
>> Changes in v2:
>>
>> 	- Switched from additional members against MediaLink pointing to the
>> 	primary and ancillary entity to a vector against MediaEntity holding
>> 	pointers to the ancillary entities linked to that MediaEntity (Laurent)
>>
>>  include/libcamera/internal/media_object.h |  4 ++++
>>  src/libcamera/media_object.cpp            | 16 ++++++++++++++++
>>  2 files changed, 20 insertions(+)
>>
>> diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h
>> index 90c63598..48616a43 100644
>> --- a/include/libcamera/internal/media_object.h
>> +++ b/include/libcamera/internal/media_object.h
>> @@ -104,6 +104,9 @@ public:
>>  	unsigned int deviceMinor() const { return minor_; }
>>  
>>  	const std::vector<MediaPad *> &pads() const { return pads_; }
>> +	const std::vector<MediaEntity *> ancillaryEntities() const {return ancillaryEntities_; }
> 
> Missing space before return. checkstyle.py should have warned you (see
> Documentation/coding-style.rst for instructions to automate style
> checking with a git commit hook).

Ooh, that's quite a neat idea. Thanks

> 
>> +
>> +	void addAncillaryEntity(MediaEntity *ancillaryEntity);
> 
> As this is called by MediaDevice only, which is a friend of this class,
> you can make the function private.
> 

TIL of the friend concept - thanks. Still trying to get up to speed with
this whole cpp thing.

>>  
>>  	const MediaPad *getPadByIndex(unsigned int index) const;
>>  	const MediaPad *getPadById(unsigned int id) const;
>> @@ -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..42b26b07 100644
>> --- a/src/libcamera/media_object.cpp
>> +++ b/src/libcamera/media_object.cpp
>> @@ -326,6 +326,22 @@ 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
> 
> s/the/The/
> 
>> + * \return void
> 
> No need for \return for functions that don't return anything.
> 
>> + */
>> +void MediaEntity::addAncillaryEntity(MediaEntity *ancillaryEntity)
>> +{
>> +	ancillaryEntities_.push_back(ancillaryEntity);
>> +}
>> +
>>  /**
>>   * \brief Get a pad in this entity by its index
>>   * \param[in] index The 0-based pad index
>