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

[Daniel Scally]

Hi Laurent

On 08/12/2021 07:51, Laurent Pinchart wrote:
> 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.


Sure, I'll start doing that

>
>>  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().


Ordering may or may not be my strong suit...

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


Thanks!

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