[v2,08/14] libcamera: v4l2_subdevice: Replace Routing::toString() with operator<<()

19727 diff mbox series

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>
Reviewed-by: Jacopo Mondi <jacopo.mondi@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(-)

Quoting Laurent Pinchart (2024-03-15 00:16:07)
> 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>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>


Reviewed-by: Kieran Bingham <kieran.bingham@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 6cd36730371a..2939dc2411c6 100644
> --- a/include/libcamera/internal/v4l2_subdevice.h
> +++ b/include/libcamera/internal/v4l2_subdevice.h
> @@ -95,11 +95,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();
> @@ -178,5 +174,6 @@ static inline bool operator!=(const V4L2Subdevice::Stream &lhs,
>  }
>  
>  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 3d0424969a89..feea26fd124f 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -1388,7 +1388,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 cc079425bb16..deef681e0d3a 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -899,30 +899,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;
>  }
>  
>  /**
> -- 
> Regards,
> 
> Laurent Pinchart
>