| Message ID | 20240815210925.4172287-2-kieran.bingham@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Thu, Aug 15, 2024 at 10:09:23PM +0100, Kieran Bingham wrote: > Various parts of libcamera print the representation of a MediaLink by > inline joining the parts to make a string representation. > > This repeated use case can be supported with a common helper to print > the MediaLink in a common manner using the existing toString() and > operator<< overload style to make it easier to report on MediaLink > types. > > This implementation will report in the following style: > > 'imx283 1-001a'[0] -> 'video-mux'[0] > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > include/libcamera/internal/media_object.h | 4 ++++ > src/libcamera/media_object.cpp | 25 +++++++++++++++++++++++ > 2 files changed, 29 insertions(+) > > diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h > index c9d77511a991..3ebef938bc49 100644 > --- a/include/libcamera/internal/media_object.h > +++ b/include/libcamera/internal/media_object.h > @@ -48,6 +48,8 @@ public: > unsigned int flags() const { return flags_; } > int setEnabled(bool enable); > > + std::string toString() const; > + > private: > LIBCAMERA_DISABLE_COPY_AND_MOVE(MediaLink) > > @@ -61,6 +63,8 @@ private: > unsigned int flags_; > }; > > +std::ostream &operator<<(std::ostream &out, const MediaLink &link); > + > class MediaPad : public MediaObject > { > public: > diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp > index 1b191a1e4df8..fa2c44b0abb6 100644 > --- a/src/libcamera/media_object.cpp > +++ b/src/libcamera/media_object.cpp > @@ -146,6 +146,31 @@ MediaLink::MediaLink(const struct media_v2_link *link, MediaPad *source, > { > } > > +std::string MediaLink::toString() const No warning from doxygen about missing documentation ? > +{ > + std::stringstream ss; > + ss << *this; > + > + return ss.str(); > +} > + > +/** > + * \brief Insert a text representation of a Link into an output stream > + * \param[in] out The output stream > + * \param[in] r The MediaLink > + * \return The output stream \a out > + */ > +std::ostream &operator<<(std::ostream &out, const MediaLink &link) > +{ > + out << "'" > + << link.source()->entity()->name() << "'[" > + << link.source()->index() << "] -> '" > + << link.sink()->entity()->name() << "'[" > + << link.sink()->index() << "]"; How about also adding the same two helpers for MediaPad ? Then you could write out << link.source() << " -> " << link->sink(); Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + > + return out; > +} > + > /** > * \fn MediaLink::source() > * \brief Retrieve the link's source pad
Quoting Laurent Pinchart (2024-08-15 22:17:45) > Hi Kieran, > > Thank you for the patch. > > On Thu, Aug 15, 2024 at 10:09:23PM +0100, Kieran Bingham wrote: > > Various parts of libcamera print the representation of a MediaLink by > > inline joining the parts to make a string representation. > > > > This repeated use case can be supported with a common helper to print > > the MediaLink in a common manner using the existing toString() and > > operator<< overload style to make it easier to report on MediaLink > > types. > > > > This implementation will report in the following style: > > > > 'imx283 1-001a'[0] -> 'video-mux'[0] > > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > --- > > include/libcamera/internal/media_object.h | 4 ++++ > > src/libcamera/media_object.cpp | 25 +++++++++++++++++++++++ > > 2 files changed, 29 insertions(+) > > > > diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h > > index c9d77511a991..3ebef938bc49 100644 > > --- a/include/libcamera/internal/media_object.h > > +++ b/include/libcamera/internal/media_object.h > > @@ -48,6 +48,8 @@ public: > > unsigned int flags() const { return flags_; } > > int setEnabled(bool enable); > > > > + std::string toString() const; > > + > > private: > > LIBCAMERA_DISABLE_COPY_AND_MOVE(MediaLink) > > > > @@ -61,6 +63,8 @@ private: > > unsigned int flags_; > > }; > > > > +std::ostream &operator<<(std::ostream &out, const MediaLink &link); > > + > > class MediaPad : public MediaObject > > { > > public: > > diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp > > index 1b191a1e4df8..fa2c44b0abb6 100644 > > --- a/src/libcamera/media_object.cpp > > +++ b/src/libcamera/media_object.cpp > > @@ -146,6 +146,31 @@ MediaLink::MediaLink(const struct media_v2_link *link, MediaPad *source, > > { > > } > > > > +std::string MediaLink::toString() const > > No warning from doxygen about missing documentation ? Ooops - I didn't look closely to check. I'll update. > > > +{ > > + std::stringstream ss; > > + ss << *this; > > + > > + return ss.str(); > > +} > > + > > +/** > > + * \brief Insert a text representation of a Link into an output stream > > + * \param[in] out The output stream > > + * \param[in] r The MediaLink > > + * \return The output stream \a out > > + */ > > +std::ostream &operator<<(std::ostream &out, const MediaLink &link) > > +{ > > + out << "'" > > + << link.source()->entity()->name() << "'[" > > + << link.source()->index() << "] -> '" > > + << link.sink()->entity()->name() << "'[" > > + << link.sink()->index() << "]"; > > How about also adding the same two helpers for MediaPad ? Then you could > write > > out << link.source() << " -> " << link->sink(); That may as well complete the set! I expect MediaPad is also used as strings elsewhere through the code base quite a lot so is likely to be useful. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > + > > + return out; > > +} > > + > > /** > > * \fn MediaLink::source() > > * \brief Retrieve the link's source pad > > -- > Regards, > > Laurent Pinchart
Hi Kieran, Thank you for the patch. On Thu, Aug 15, 2024 at 10:09:23PM +0100, Kieran Bingham wrote: > Various parts of libcamera print the representation of a MediaLink by > inline joining the parts to make a string representation. > > This repeated use case can be supported with a common helper to print > the MediaLink in a common manner using the existing toString() and > operator<< overload style to make it easier to report on MediaLink > types. > > This implementation will report in the following style: > > 'imx283 1-001a'[0] -> 'video-mux'[0] > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> Regards, Stefan > --- > include/libcamera/internal/media_object.h | 4 ++++ > src/libcamera/media_object.cpp | 25 +++++++++++++++++++++++ > 2 files changed, 29 insertions(+) > > diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h > index c9d77511a991..3ebef938bc49 100644 > --- a/include/libcamera/internal/media_object.h > +++ b/include/libcamera/internal/media_object.h > @@ -48,6 +48,8 @@ public: > unsigned int flags() const { return flags_; } > int setEnabled(bool enable); > > + std::string toString() const; > + > private: > LIBCAMERA_DISABLE_COPY_AND_MOVE(MediaLink) > > @@ -61,6 +63,8 @@ private: > unsigned int flags_; > }; > > +std::ostream &operator<<(std::ostream &out, const MediaLink &link); > + > class MediaPad : public MediaObject > { > public: > diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp > index 1b191a1e4df8..fa2c44b0abb6 100644 > --- a/src/libcamera/media_object.cpp > +++ b/src/libcamera/media_object.cpp > @@ -146,6 +146,31 @@ MediaLink::MediaLink(const struct media_v2_link *link, MediaPad *source, > { > } > > +std::string MediaLink::toString() const > +{ > + std::stringstream ss; > + ss << *this; > + > + return ss.str(); > +} > + > +/** > + * \brief Insert a text representation of a Link into an output stream > + * \param[in] out The output stream > + * \param[in] r The MediaLink > + * \return The output stream \a out > + */ > +std::ostream &operator<<(std::ostream &out, const MediaLink &link) > +{ > + out << "'" > + << link.source()->entity()->name() << "'[" > + << link.source()->index() << "] -> '" > + << link.sink()->entity()->name() << "'[" > + << link.sink()->index() << "]"; > + > + return out; > +} > + > /** > * \fn MediaLink::source() > * \brief Retrieve the link's source pad > -- > 2.34.1 >
diff --git a/include/libcamera/internal/media_object.h b/include/libcamera/internal/media_object.h index c9d77511a991..3ebef938bc49 100644 --- a/include/libcamera/internal/media_object.h +++ b/include/libcamera/internal/media_object.h @@ -48,6 +48,8 @@ public: unsigned int flags() const { return flags_; } int setEnabled(bool enable); + std::string toString() const; + private: LIBCAMERA_DISABLE_COPY_AND_MOVE(MediaLink) @@ -61,6 +63,8 @@ private: unsigned int flags_; }; +std::ostream &operator<<(std::ostream &out, const MediaLink &link); + class MediaPad : public MediaObject { public: diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp index 1b191a1e4df8..fa2c44b0abb6 100644 --- a/src/libcamera/media_object.cpp +++ b/src/libcamera/media_object.cpp @@ -146,6 +146,31 @@ MediaLink::MediaLink(const struct media_v2_link *link, MediaPad *source, { } +std::string MediaLink::toString() const +{ + std::stringstream ss; + ss << *this; + + return ss.str(); +} + +/** + * \brief Insert a text representation of a Link into an output stream + * \param[in] out The output stream + * \param[in] r The MediaLink + * \return The output stream \a out + */ +std::ostream &operator<<(std::ostream &out, const MediaLink &link) +{ + out << "'" + << link.source()->entity()->name() << "'[" + << link.source()->index() << "] -> '" + << link.sink()->entity()->name() << "'[" + << link.sink()->index() << "]"; + + return out; +} + /** * \fn MediaLink::source() * \brief Retrieve the link's source pad
Various parts of libcamera print the representation of a MediaLink by inline joining the parts to make a string representation. This repeated use case can be supported with a common helper to print the MediaLink in a common manner using the existing toString() and operator<< overload style to make it easier to report on MediaLink types. This implementation will report in the following style: 'imx283 1-001a'[0] -> 'video-mux'[0] Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- include/libcamera/internal/media_object.h | 4 ++++ src/libcamera/media_object.cpp | 25 +++++++++++++++++++++++ 2 files changed, 29 insertions(+)