| Message ID | 20240227140953.26093-9-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent On Tue, Feb 27, 2024 at 04:09:52PM +0200, Laurent Pinchart wrote: > The main (and only at the moment) use case for the Routing::toString() > function is to print a representation of the routing table in a log > message. The function is implemented using an std::stringstream, and the > returned std::string is then inserted into an std::ostream. This is > inefficient. Replace the function with a specialization of the > operator<<() and use it in the caller. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/internal/v4l2_subdevice.h | 7 ++--- > src/libcamera/pipeline/simple/simple.cpp | 2 +- > src/libcamera/v4l2_subdevice.cpp | 29 +++++++++++---------- > 3 files changed, 18 insertions(+), 20 deletions(-) > > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h > index 65003394a984..2f64b3deadfe 100644 > --- a/include/libcamera/internal/v4l2_subdevice.h > +++ b/include/libcamera/internal/v4l2_subdevice.h > @@ -85,11 +85,7 @@ public: > unsigned int stream; > }; > > - class Routing : public std::vector<struct v4l2_subdev_route> > - { > - public: > - std::string toString() const; > - }; > + using Routing = std::vector<struct v4l2_subdev_route>; > > explicit V4L2Subdevice(const MediaEntity *entity); > ~V4L2Subdevice(); > @@ -161,5 +157,6 @@ private: > }; > > std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream); > +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing); > > } /* namespace libcamera */ > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 1145e80847ba..1dbbd7fe91d6 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -1387,7 +1387,7 @@ int SimplePipelineHandler::resetRoutingTable(V4L2Subdevice *subdev) > > LOG(SimplePipeline, Debug) > << "Routing table of " << subdev->deviceNode() > - << " reset to " << routing.toString(); > + << " reset to " << routing; > > return 0; > } > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp > index efe8b0f793e9..1670c31a9656 100644 > --- a/src/libcamera/v4l2_subdevice.cpp > +++ b/src/libcamera/v4l2_subdevice.cpp > @@ -844,30 +844,31 @@ std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream) > } > > /** > - * \class V4L2Subdevice::Routing > + * \typedef V4L2Subdevice::Routing > * \brief V4L2 subdevice routing table > * > * This class stores a subdevice routing table as a vector of routes. > */ > > /** > - * \brief Assemble and return a string describing the routing table > - * \return A string describing the routing table > + * \brief Insert a text representation of a V4L2Subdevice::Routing into an > + * output stream Intentional ? This nit apart Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > + * \param[in] out The output stream > + * \param[in] routing The V4L2Subdevice::Routing > + * \return The output stream \a out > */ > -std::string V4L2Subdevice::Routing::toString() const > +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing) > { > - std::stringstream routing; > - > - for (const auto &[i, route] : utils::enumerate(*this)) { > - routing << "[" << i << "] " > - << route.sink_pad << "/" << route.sink_stream << " -> " > - << route.source_pad << "/" << route.source_stream > - << " (" << utils::hex(route.flags) << ")"; > - if (i != size() - 1) > - routing << ", "; > + for (const auto &[i, route] : utils::enumerate(routing)) { > + out << "[" << i << "] " > + << route.sink_pad << "/" << route.sink_stream << " -> " > + << route.source_pad << "/" << route.source_stream > + << " (" << utils::hex(route.flags) << ")"; > + if (i != routing.size() - 1) > + out << ", "; > } > > - return routing.str(); > + return out; > } > > /** > -- > Regards, > > Laurent Pinchart >
On Wed, Feb 28, 2024 at 09:39:49AM +0100, Jacopo Mondi wrote: > On Tue, Feb 27, 2024 at 04:09:52PM +0200, Laurent Pinchart wrote: > > The main (and only at the moment) use case for the Routing::toString() > > function is to print a representation of the routing table in a log > > message. The function is implemented using an std::stringstream, and the > > returned std::string is then inserted into an std::ostream. This is > > inefficient. Replace the function with a specialization of the > > operator<<() and use it in the caller. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > include/libcamera/internal/v4l2_subdevice.h | 7 ++--- > > src/libcamera/pipeline/simple/simple.cpp | 2 +- > > src/libcamera/v4l2_subdevice.cpp | 29 +++++++++++---------- > > 3 files changed, 18 insertions(+), 20 deletions(-) > > > > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h > > index 65003394a984..2f64b3deadfe 100644 > > --- a/include/libcamera/internal/v4l2_subdevice.h > > +++ b/include/libcamera/internal/v4l2_subdevice.h > > @@ -85,11 +85,7 @@ public: > > unsigned int stream; > > }; > > > > - class Routing : public std::vector<struct v4l2_subdev_route> > > - { > > - public: > > - std::string toString() const; > > - }; > > + using Routing = std::vector<struct v4l2_subdev_route>; > > > > explicit V4L2Subdevice(const MediaEntity *entity); > > ~V4L2Subdevice(); > > @@ -161,5 +157,6 @@ private: > > }; > > > > std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream); > > +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing); > > > > } /* namespace libcamera */ > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > > index 1145e80847ba..1dbbd7fe91d6 100644 > > --- a/src/libcamera/pipeline/simple/simple.cpp > > +++ b/src/libcamera/pipeline/simple/simple.cpp > > @@ -1387,7 +1387,7 @@ int SimplePipelineHandler::resetRoutingTable(V4L2Subdevice *subdev) > > > > LOG(SimplePipeline, Debug) > > << "Routing table of " << subdev->deviceNode() > > - << " reset to " << routing.toString(); > > + << " reset to " << routing; > > > > return 0; > > } > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp > > index efe8b0f793e9..1670c31a9656 100644 > > --- a/src/libcamera/v4l2_subdevice.cpp > > +++ b/src/libcamera/v4l2_subdevice.cpp > > @@ -844,30 +844,31 @@ std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream) > > } > > > > /** > > - * \class V4L2Subdevice::Routing > > + * \typedef V4L2Subdevice::Routing > > * \brief V4L2 subdevice routing table > > * > > * This class stores a subdevice routing table as a vector of routes. > > */ > > > > /** > > - * \brief Assemble and return a string describing the routing table > > - * \return A string describing the routing table > > + * \brief Insert a text representation of a V4L2Subdevice::Routing into an > > + * output stream > > Intentional ? Let's continue the discussion on this topic in patch 7/9, I'll then adapt this one. > This nit apart > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > + * \param[in] out The output stream > > + * \param[in] routing The V4L2Subdevice::Routing > > + * \return The output stream \a out > > */ > > -std::string V4L2Subdevice::Routing::toString() const > > +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing) > > { > > - std::stringstream routing; > > - > > - for (const auto &[i, route] : utils::enumerate(*this)) { > > - routing << "[" << i << "] " > > - << route.sink_pad << "/" << route.sink_stream << " -> " > > - << route.source_pad << "/" << route.source_stream > > - << " (" << utils::hex(route.flags) << ")"; > > - if (i != size() - 1) > > - routing << ", "; > > + for (const auto &[i, route] : utils::enumerate(routing)) { > > + out << "[" << i << "] " > > + << route.sink_pad << "/" << route.sink_stream << " -> " > > + << route.source_pad << "/" << route.source_stream > > + << " (" << utils::hex(route.flags) << ")"; > > + if (i != routing.size() - 1) > > + out << ", "; > > } > > > > - return routing.str(); > > + return out; > > } > > > > /**
diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h index 65003394a984..2f64b3deadfe 100644 --- a/include/libcamera/internal/v4l2_subdevice.h +++ b/include/libcamera/internal/v4l2_subdevice.h @@ -85,11 +85,7 @@ public: unsigned int stream; }; - class Routing : public std::vector<struct v4l2_subdev_route> - { - public: - std::string toString() const; - }; + using Routing = std::vector<struct v4l2_subdev_route>; explicit V4L2Subdevice(const MediaEntity *entity); ~V4L2Subdevice(); @@ -161,5 +157,6 @@ private: }; std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream); +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing); } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 1145e80847ba..1dbbd7fe91d6 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -1387,7 +1387,7 @@ int SimplePipelineHandler::resetRoutingTable(V4L2Subdevice *subdev) LOG(SimplePipeline, Debug) << "Routing table of " << subdev->deviceNode() - << " reset to " << routing.toString(); + << " reset to " << routing; return 0; } diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp index efe8b0f793e9..1670c31a9656 100644 --- a/src/libcamera/v4l2_subdevice.cpp +++ b/src/libcamera/v4l2_subdevice.cpp @@ -844,30 +844,31 @@ std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream) } /** - * \class V4L2Subdevice::Routing + * \typedef V4L2Subdevice::Routing * \brief V4L2 subdevice routing table * * This class stores a subdevice routing table as a vector of routes. */ /** - * \brief Assemble and return a string describing the routing table - * \return A string describing the routing table + * \brief Insert a text representation of a V4L2Subdevice::Routing into an + * output stream + * \param[in] out The output stream + * \param[in] routing The V4L2Subdevice::Routing + * \return The output stream \a out */ -std::string V4L2Subdevice::Routing::toString() const +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing) { - std::stringstream routing; - - for (const auto &[i, route] : utils::enumerate(*this)) { - routing << "[" << i << "] " - << route.sink_pad << "/" << route.sink_stream << " -> " - << route.source_pad << "/" << route.source_stream - << " (" << utils::hex(route.flags) << ")"; - if (i != size() - 1) - routing << ", "; + for (const auto &[i, route] : utils::enumerate(routing)) { + out << "[" << i << "] " + << route.sink_pad << "/" << route.sink_stream << " -> " + << route.source_pad << "/" << route.source_stream + << " (" << utils::hex(route.flags) << ")"; + if (i != routing.size() - 1) + out << ", "; } - return routing.str(); + return out; } /**
The main (and only at the moment) use case for the Routing::toString() function is to print a representation of the routing table in a log message. The function is implemented using an std::stringstream, and the returned std::string is then inserted into an std::ostream. This is inefficient. Replace the function with a specialization of the operator<<() and use it in the caller. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/libcamera/internal/v4l2_subdevice.h | 7 ++--- src/libcamera/pipeline/simple/simple.cpp | 2 +- src/libcamera/v4l2_subdevice.cpp | 29 +++++++++++---------- 3 files changed, 18 insertions(+), 20 deletions(-)