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

[Jacopo Mondi]

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 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 ?

This nit apart
Reviewed-by: Jacopo Mondi <jacopo.mondi at 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
>