[PATCH 8/9] libcamera: v4l2_subdevice: Replace Routing::toString() with operator<<()

[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 at 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 at 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;
> >  }
> >
> >  /**

-- 
Regards,

Laurent Pinchart