| Message ID | 20211207224512.753979-2-djrscally@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@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@ideasonboard.com> > /** > * \brief Get a pad in this entity by its index > * \param[in] index The 0-based pad index
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@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@ideasonboard.com> Thanks! > >> /** >> * \brief Get a pad in this entity by its index >> * \param[in] index The 0-based pad index
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); +} + /** * \brief Get a pad in this entity by its index * \param[in] index The 0-based pad index
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 v3: - Fixed some style issues - Made addAncillaryEntity() private include/libcamera/internal/media_object.h | 4 ++++ src/libcamera/media_object.cpp | 15 +++++++++++++++ 2 files changed, 19 insertions(+)