[{"id":28781,"web_url":"https://patchwork.libcamera.org/comment/28781/","msgid":"","date":"2024-02-28T08:39:49","subject":"Re: [PATCH 8/9] libcamera: v4l2_subdevice: Replace\n\tRouting::toString() with operator<<()","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Laurent\n\nOn Tue, Feb 27, 2024 at 04:09:52PM +0200, Laurent Pinchart wrote:\n> The main (and only at the moment) use case for the Routing::toString()\n> function is to print a representation of the routing table in a log\n> message. The function is implemented using an std::stringstream, and the\n> returned std::string is then inserted into an std::ostream. This is\n> inefficient. Replace the function with a specialization of the\n> operator<<() and use it in the caller.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/internal/v4l2_subdevice.h | 7 ++---\n> src/libcamera/pipeline/simple/simple.cpp | 2 +-\n> src/libcamera/v4l2_subdevice.cpp | 29 +++++++++++----------\n> 3 files changed, 18 insertions(+), 20 deletions(-)\n>\n> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> index 65003394a984..2f64b3deadfe 100644\n> --- a/include/libcamera/internal/v4l2_subdevice.h\n> +++ b/include/libcamera/internal/v4l2_subdevice.h\n> @@ -85,11 +85,7 @@ public:\n> \t\tunsigned int stream;\n> \t};\n>\n> -\tclass Routing : public std::vector\n> -\t{\n> -\tpublic:\n> -\t\tstd::string toString() const;\n> -\t};\n> +\tusing Routing = std::vector;\n>\n> \texplicit V4L2Subdevice(const MediaEntity *entity);\n> \t~V4L2Subdevice();\n> @@ -161,5 +157,6 @@ private:\n> };\n>\n> std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream);\n> +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing);\n>\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 1145e80847ba..1dbbd7fe91d6 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -1387,7 +1387,7 @@ int SimplePipelineHandler::resetRoutingTable(V4L2Subdevice *subdev)\n>\n> \tLOG(SimplePipeline, Debug)\n> \t\t<< \"Routing table of \" << subdev->deviceNode()\n> -\t\t<< \" reset to \" << routing.toString();\n> +\t\t<< \" reset to \" << routing;\n>\n> \treturn 0;\n> }\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index efe8b0f793e9..1670c31a9656 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -844,30 +844,31 @@ std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream)\n> }\n>\n> /**\n> - * \\class V4L2Subdevice::Routing\n> + * \\typedef V4L2Subdevice::Routing\n> * \\brief V4L2 subdevice routing table\n> *\n> * This class stores a subdevice routing table as a vector of routes.\n> */\n>\n> /**\n> - * \\brief Assemble and return a string describing the routing table\n> - * \\return A string describing the routing table\n> + * \\brief Insert a text representation of a V4L2Subdevice::Routing into an\n> + *\toutput stream\n\nIntentional ?\n\nThis nit apart\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> + * \\param[in] out The output stream\n> + * \\param[in] routing The V4L2Subdevice::Routing\n> + * \\return The output stream \\a out\n> */\n> -std::string V4L2Subdevice::Routing::toString() const\n> +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing)\n> {\n> -\tstd::stringstream routing;\n> -\n> -\tfor (const auto &[i, route] : utils::enumerate(*this)) {\n> -\t\trouting << \"[\" << i << \"] \"\n> -\t\t\t<< route.sink_pad << \"/\" << route.sink_stream << \" -> \"\n> -\t\t\t<< route.source_pad << \"/\" << route.source_stream\n> -\t\t\t<< \" (\" << utils::hex(route.flags) << \")\";\n> -\t\tif (i != size() - 1)\n> -\t\t\trouting << \", \";\n> +\tfor (const auto &[i, route] : utils::enumerate(routing)) {\n> +\t\tout << \"[\" << i << \"] \"\n> +\t\t << route.sink_pad << \"/\" << route.sink_stream << \" -> \"\n> +\t\t << route.source_pad << \"/\" << route.source_stream\n> +\t\t << \" (\" << utils::hex(route.flags) << \")\";\n> +\t\tif (i != routing.size() - 1)\n> +\t\t\tout << \", \";\n> \t}\n>\n> -\treturn routing.str();\n> +\treturn out;\n> }\n>\n> /**\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A6B67BD160\n\tfor ;\n\tWed, 28 Feb 2024 08:39:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F021A6286F;\n\tWed, 28 Feb 2024 09:39:53 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B695562865\n\tfor ;\n\tWed, 28 Feb 2024 09:39:52 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EC214672;\n\tWed, 28 Feb 2024 09:39:39 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Tvjo5sT+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709109580;\n\tbh=mxLncSI1z741x2nQaGRvlDaZv3WX4h/3OGyxVAfwedk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Tvjo5sT+7DVstvNLqjhcZ4m+xTtpKisckU0TzdhkGNS9keVduR7J+UL9rCawrU4uP\n\tAnerjbNhBMtmvcUFEUd2g+Sq3e3jr6o4ROrPjucg3adu+uIjpzgGFzDGVPQLAfUZif\n\t/16LeqvKSDLwDsnOcEoFOdDOiPBi3A1Z/Do/rrjs=","Date":"Wed, 28 Feb 2024 09:39:49 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Subject":"Re: [PATCH 8/9] libcamera: v4l2_subdevice: Replace\n\tRouting::toString() with operator<<()","Message-ID":"","References":"<20240227140953.26093-1-laurent.pinchart@ideasonboard.com>\n\t<20240227140953.26093-9-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240227140953.26093-9-laurent.pinchart@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28787,"web_url":"https://patchwork.libcamera.org/comment/28787/","msgid":"<20240228105223.GI3419@pendragon.ideasonboard.com>","date":"2024-02-28T10:52:23","subject":"Re: [PATCH 8/9] libcamera: v4l2_subdevice: Replace\n\tRouting::toString() with operator<<()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Feb 28, 2024 at 09:39:49AM +0100, Jacopo Mondi wrote:\n> On Tue, Feb 27, 2024 at 04:09:52PM +0200, Laurent Pinchart wrote:\n> > The main (and only at the moment) use case for the Routing::toString()\n> > function is to print a representation of the routing table in a log\n> > message. The function is implemented using an std::stringstream, and the\n> > returned std::string is then inserted into an std::ostream. This is\n> > inefficient. Replace the function with a specialization of the\n> > operator<<() and use it in the caller.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > include/libcamera/internal/v4l2_subdevice.h | 7 ++---\n> > src/libcamera/pipeline/simple/simple.cpp | 2 +-\n> > src/libcamera/v4l2_subdevice.cpp | 29 +++++++++++----------\n> > 3 files changed, 18 insertions(+), 20 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> > index 65003394a984..2f64b3deadfe 100644\n> > --- a/include/libcamera/internal/v4l2_subdevice.h\n> > +++ b/include/libcamera/internal/v4l2_subdevice.h\n> > @@ -85,11 +85,7 @@ public:\n> > \t\tunsigned int stream;\n> > \t};\n> >\n> > -\tclass Routing : public std::vector\n> > -\t{\n> > -\tpublic:\n> > -\t\tstd::string toString() const;\n> > -\t};\n> > +\tusing Routing = std::vector;\n> >\n> > \texplicit V4L2Subdevice(const MediaEntity *entity);\n> > \t~V4L2Subdevice();\n> > @@ -161,5 +157,6 @@ private:\n> > };\n> >\n> > std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream);\n> > +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing);\n> >\n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 1145e80847ba..1dbbd7fe91d6 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -1387,7 +1387,7 @@ int SimplePipelineHandler::resetRoutingTable(V4L2Subdevice *subdev)\n> >\n> > \tLOG(SimplePipeline, Debug)\n> > \t\t<< \"Routing table of \" << subdev->deviceNode()\n> > -\t\t<< \" reset to \" << routing.toString();\n> > +\t\t<< \" reset to \" << routing;\n> >\n> > \treturn 0;\n> > }\n> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > index efe8b0f793e9..1670c31a9656 100644\n> > --- a/src/libcamera/v4l2_subdevice.cpp\n> > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > @@ -844,30 +844,31 @@ std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream)\n> > }\n> >\n> > /**\n> > - * \\class V4L2Subdevice::Routing\n> > + * \\typedef V4L2Subdevice::Routing\n> > * \\brief V4L2 subdevice routing table\n> > *\n> > * This class stores a subdevice routing table as a vector of routes.\n> > */\n> >\n> > /**\n> > - * \\brief Assemble and return a string describing the routing table\n> > - * \\return A string describing the routing table\n> > + * \\brief Insert a text representation of a V4L2Subdevice::Routing into an\n> > + *\toutput stream\n> \n> Intentional ?\n\nLet's continue the discussion on this topic in patch 7/9, I'll then adapt this one.\n\n> This nit apart\n> Reviewed-by: Jacopo Mondi \n> \n> > + * \\param[in] out The output stream\n> > + * \\param[in] routing The V4L2Subdevice::Routing\n> > + * \\return The output stream \\a out\n> > */\n> > -std::string V4L2Subdevice::Routing::toString() const\n> > +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing)\n> > {\n> > -\tstd::stringstream routing;\n> > -\n> > -\tfor (const auto &[i, route] : utils::enumerate(*this)) {\n> > -\t\trouting << \"[\" << i << \"] \"\n> > -\t\t\t<< route.sink_pad << \"/\" << route.sink_stream << \" -> \"\n> > -\t\t\t<< route.source_pad << \"/\" << route.source_stream\n> > -\t\t\t<< \" (\" << utils::hex(route.flags) << \")\";\n> > -\t\tif (i != size() - 1)\n> > -\t\t\trouting << \", \";\n> > +\tfor (const auto &[i, route] : utils::enumerate(routing)) {\n> > +\t\tout << \"[\" << i << \"] \"\n> > +\t\t << route.sink_pad << \"/\" << route.sink_stream << \" -> \"\n> > +\t\t << route.source_pad << \"/\" << route.source_stream\n> > +\t\t << \" (\" << utils::hex(route.flags) << \")\";\n> > +\t\tif (i != routing.size() - 1)\n> > +\t\t\tout << \", \";\n> > \t}\n> >\n> > -\treturn routing.str();\n> > +\treturn out;\n> > }\n> >\n> > /**","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4BB5CBD80A\n\tfor ;\n\tWed, 28 Feb 2024 10:52:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 06F6162871;\n\tWed, 28 Feb 2024 11:52:23 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 712E062865\n\tfor ;\n\tWed, 28 Feb 2024 11:52:21 +0100 (CET)","from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AA296672;\n\tWed, 28 Feb 2024 11:52:08 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"i1bR+7ZP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709117528;\n\tbh=Uo3pIgStFTwvn0bAqhWGOiszMIglgMkW4fgiUKR6Nzg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=i1bR+7ZPF+JBg8X/MtDXiAFi5psXQdRNxaA5hT9VFkk6g/Qc65YnsRSTciAturItt\n\tbvnCGZqJwQnx4oTZG5fIOXMNGmpILI+Nx76qONo5CXxYr0pkJ+7POSNf/HyJjB0T9c\n\t7BTX69xWJaBNer/IpgMqU9eozvirdp7He0J2D7/8=","Date":"Wed, 28 Feb 2024 12:52:23 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Subject":"Re: [PATCH 8/9] libcamera: v4l2_subdevice: Replace\n\tRouting::toString() with operator<<()","Message-ID":"<20240228105223.GI3419@pendragon.ideasonboard.com>","References":"<20240227140953.26093-1-laurent.pinchart@ideasonboard.com>\n\t<20240227140953.26093-9-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]