[{"id":28768,"web_url":"https://patchwork.libcamera.org/comment/28768/","msgid":"<evkcho4k3fqkuhoegn5uhpndslhpxdj2ao4lqlks7hvsjtlmn4@66hm3kmkpms5>","date":"2024-02-27T16:55:25","subject":"Re: [PATCH 9/9] libcamera: v4l2_subdevice: Add V4L2Subdevice::Route\n\tstructure","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:53PM +0200, Laurent Pinchart wrote:\n> The V4L2Subdevice class deals with streams in two places:\n>\n> - In routing tables, streams as expressed as a pad number and a stream\n>   number in a v4l2_subdev_route instance.\n> - In the format and selection get and set functions, streams as\n>   expressed using the Stream structure, which binds the pad number and\n>   stream number.\n>\n> Expressing streams in different ways requires pipeline handlers and\n> other helpers to convert between the two representations. This isn't\n> much of an issue yet as libcamera has little stream-aware code, but it\n> is expected to increasingly become a burden.\n>\n> To simplify the API, introduce a V4L2Subdevice::Route structure that\n> mimicks the kernel v4l2_subdev_route structure but represents streams as\n> V4L2Subdevice::Stream instances. This will improve seamless integration\n> of routes, formats and selection rectangles.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  include/libcamera/internal/v4l2_subdevice.h |  9 ++-\n>  src/libcamera/pipeline/simple/simple.cpp    |  8 +-\n>  src/libcamera/v4l2_subdevice.cpp            | 86 ++++++++++++++++++---\n>  3 files changed, 89 insertions(+), 14 deletions(-)\n>\n> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> index 2f64b3deadfe..38c340236908 100644\n> --- a/include/libcamera/internal/v4l2_subdevice.h\n> +++ b/include/libcamera/internal/v4l2_subdevice.h\n> @@ -85,7 +85,13 @@ public:\n>  \t\tunsigned int stream;\n>  \t};\n>\n> -\tusing Routing = std::vector<struct v4l2_subdev_route>;\n> +\tstruct Route {\n> +\t\tStream sink;\n> +\t\tStream source;\n> +\t\tuint32_t flags;\n> +\t};\n> +\n> +\tusing Routing = std::vector<Route>;\n>\n>  \texplicit V4L2Subdevice(const MediaEntity *entity);\n>  \t~V4L2Subdevice();\n> @@ -157,6 +163,7 @@ private:\n>  };\n>\n>  std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream);\n> +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Route &route);\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 1dbbd7fe91d6..c1eeae643d71 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -851,17 +851,17 @@ std::vector<const MediaPad *> SimpleCameraData::routedSourcePads(MediaPad *sink)\n>\n>  \tstd::vector<const MediaPad *> pads;\n>\n> -\tfor (const struct v4l2_subdev_route &route : routing) {\n> -\t\tif (sink->index() != route.sink_pad ||\n> +\tfor (const V4L2Subdevice::Route &route : routing) {\n> +\t\tif (sink->index() != route.sink.pad ||\n>  \t\t    !(route.flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))\n>  \t\t\tcontinue;\n>\n> -\t\tconst MediaPad *pad = entity->getPadByIndex(route.source_pad);\n> +\t\tconst MediaPad *pad = entity->getPadByIndex(route.source.pad);\n>  \t\tif (!pad) {\n>  \t\t\tLOG(SimplePipeline, Warning)\n>  \t\t\t\t<< \"Entity \" << entity->name()\n>  \t\t\t\t<< \" has invalid route source pad \"\n> -\t\t\t\t<< route.source_pad;\n> +\t\t\t\t<< route.source.pad;\n>  \t\t}\n>\n>  \t\tpads.push_back(pad);\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index 1670c31a9656..2e9e05c86059 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -843,6 +843,39 @@ std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream)\n>  \treturn out;\n>  }\n>\n> +/**\n> + * \\class V4L2Subdevice::Route\n> + * \\brief V4L2 subdevice routing table entry\n> + *\n> + * This class models a route in the subdevice routing table. It is similar to\n> + * the v4l2_subdev_route structure, but uses the V4L2Subdevice::Stream class\n> + * for easier usage with the V4L2Subdevice stream-aware functions.\n> + *\n> + * \\var V4L2Subdevice::Route::sink\n> + * \\brief The sink stream of the route\n> + *\n> + * \\var V4L2Subdevice::Route::source\n> + * \\brief The source stream of the route\n> + *\n> + * \\var V4L2Subdevice::Route::flags\n> + * \\brief The route flags (V4L2_SUBDEV_ROUTE_FL_*)\n> + */\n> +\n> +/**\n> + * \\brief Insert a text representation of a V4L2Subdevice::Route into an\n> + *\toutput stream\n> + * \\param[in] out The output stream\n> + * \\param[in] route The V4L2Subdevice::Route\n> + * \\return The output stream \\a out\n> + */\n> +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Route &route)\n> +{\n> +\tout << route.sink << \" -> \" << route.source\n> +\t    << \" (\" << utils::hex(route.flags) << \")\";\n> +\n> +\treturn out;\n> +}\n> +\n>  /**\n>   * \\typedef V4L2Subdevice::Routing\n>   * \\brief V4L2 subdevice routing table\n> @@ -860,10 +893,7 @@ std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream)\n>  std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing)\n>  {\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\tout << \"[\" << i << \"] \" << route;\n>  \t\tif (i != routing.size() - 1)\n>  \t\t\tout << \", \";\n>  \t}\n> @@ -1222,6 +1252,30 @@ int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n>   * \\return 0 on success or a negative error code otherwise\n>   */\n>\n> +namespace {\n> +\n> +void routeFromKernel(V4L2Subdevice::Route &route,\n> +\t\t     const struct v4l2_subdev_route &kroute)\n> +{\n> +\troute.sink.pad = kroute.sink_pad;\n> +\troute.sink.stream = kroute.sink_stream;\n> +\troute.source.pad = kroute.source_pad;\n> +\troute.source.stream = kroute.source_stream;\n> +\troute.flags = kroute.flags;\n> +}\n> +\n> +void routeToKernel(const V4L2Subdevice::Route &route,\n> +\t\t   struct v4l2_subdev_route &kroute)\n> +{\n> +\tkroute.sink_pad = route.sink.pad;\n> +\tkroute.sink_stream = route.sink.stream;\n> +\tkroute.source_pad = route.source.pad;\n> +\tkroute.source_stream = route.source.stream;\n> +\tkroute.flags = route.flags;\n> +}\n> +\n> +} /* namespace */\n> +\n\nWhy 2 helpers in an anonymous namespace ? I would question if these\nare worth to be helpers at all, as they're called from a single\nplace..\n\n>  /**\n>   * \\brief Retrieve the subdevice's internal routing table\n>   * \\param[out] routing The routing table\n> @@ -1250,8 +1304,8 @@ int V4L2Subdevice::getRouting(Routing *routing, Whence whence)\n>  \t\treturn ret;\n>  \t}\n>\n> -\trouting->resize(rt.num_routes);\n> -\trt.routes = reinterpret_cast<uintptr_t>(routing->data());\n> +\tstd::vector<struct v4l2_subdev_route> routes{ rt.num_routes };\n> +\trt.routes = reinterpret_cast<uintptr_t>(routes.data());\n>\n>  \tret = ioctl(VIDIOC_SUBDEV_G_ROUTING, &rt);\n>  \tif (ret) {\n> @@ -1260,11 +1314,16 @@ int V4L2Subdevice::getRouting(Routing *routing, Whence whence)\n>  \t\treturn ret;\n>  \t}\n>\n> -\tif (rt.num_routes != routing->size()) {\n> +\tif (rt.num_routes != routes.size()) {\n>  \t\tLOG(V4L2, Error) << \"Invalid number of routes\";\n>  \t\treturn -EINVAL;\n>  \t}\n>\n> +\trouting->resize(rt.num_routes);\n> +\n> +\tfor (const auto &[i, route] : utils::enumerate(routes))\n> +\t\trouteFromKernel((*routing)[i], route);\n> +\n>  \treturn 0;\n>  }\n>\n> @@ -1285,10 +1344,15 @@ int V4L2Subdevice::setRouting(Routing *routing, Whence whence)\n>  \tif (!caps_.hasStreams())\n>  \t\treturn 0;\n>\n> +\tstd::vector<struct v4l2_subdev_route> routes{ routing->size() };\n> +\n> +\tfor (const auto &[i, route] : utils::enumerate(*routing))\n> +\t\trouteToKernel(route, routes[i]);\n> +\n>  \tstruct v4l2_subdev_routing rt = {};\n>  \trt.which = whence;\n> -\trt.num_routes = routing->size();\n> -\trt.routes = reinterpret_cast<uintptr_t>(routing->data());\n> +\trt.num_routes = routes.size();\n> +\trt.routes = reinterpret_cast<uintptr_t>(routes.data());\n>\n>  \tint ret = ioctl(VIDIOC_SUBDEV_S_ROUTING, &rt);\n>  \tif (ret) {\n> @@ -1296,8 +1360,12 @@ int V4L2Subdevice::setRouting(Routing *routing, Whence whence)\n>  \t\treturn ret;\n>  \t}\n>\n> +\troutes.resize(rt.num_routes);\n>  \trouting->resize(rt.num_routes);\n>\n> +\tfor (const auto &[i, route] : utils::enumerate(routes))\n> +\t\trouteFromKernel((*routing)[i], route);\n> +\n>  \treturn 0;\n>  }\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 75CA2BD80A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 27 Feb 2024 16:55:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C21086285F;\n\tTue, 27 Feb 2024 17:55:29 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 39FA562806\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 27 Feb 2024 17:55:28 +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 EB5D98C;\n\tTue, 27 Feb 2024 17:55:15 +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=\"rIuC64Gw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709052916;\n\tbh=aMCIbHT0jIoNMV1XXgPi4Pp8miF4OgKNXA8XGLxjuWI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rIuC64GwZqoUcaF2ADso+olyjWVHaEPiK19JowqpD/VSYRRipqx1NW4UXUTfZEdXI\n\tH1oE7RFcs5TxDtyhgqxVt3VlG4iiav72QeK06h3CZnsMzmVpnU18P2g2oG5nkx+xtQ\n\tijx9kvsKMt2Poay7lvPIGRtFxo7/qradxap1rQ/c=","Date":"Tue, 27 Feb 2024 17:55:25 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Subject":"Re: [PATCH 9/9] libcamera: v4l2_subdevice: Add V4L2Subdevice::Route\n\tstructure","Message-ID":"<evkcho4k3fqkuhoegn5uhpndslhpxdj2ao4lqlks7hvsjtlmn4@66hm3kmkpms5>","References":"<20240227140953.26093-1-laurent.pinchart@ideasonboard.com>\n\t<20240227140953.26093-10-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240227140953.26093-10-laurent.pinchart@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28788,"web_url":"https://patchwork.libcamera.org/comment/28788/","msgid":"<20240228105343.GJ3419@pendragon.ideasonboard.com>","date":"2024-02-28T10:53:43","subject":"Re: [PATCH 9/9] libcamera: v4l2_subdevice: Add V4L2Subdevice::Route\n\tstructure","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tue, Feb 27, 2024 at 05:55:25PM +0100, Jacopo Mondi wrote:\n> On Tue, Feb 27, 2024 at 04:09:53PM +0200, Laurent Pinchart wrote:\n> > The V4L2Subdevice class deals with streams in two places:\n> >\n> > - In routing tables, streams as expressed as a pad number and a stream\n> >   number in a v4l2_subdev_route instance.\n> > - In the format and selection get and set functions, streams as\n> >   expressed using the Stream structure, which binds the pad number and\n> >   stream number.\n> >\n> > Expressing streams in different ways requires pipeline handlers and\n> > other helpers to convert between the two representations. This isn't\n> > much of an issue yet as libcamera has little stream-aware code, but it\n> > is expected to increasingly become a burden.\n> >\n> > To simplify the API, introduce a V4L2Subdevice::Route structure that\n> > mimicks the kernel v4l2_subdev_route structure but represents streams as\n> > V4L2Subdevice::Stream instances. This will improve seamless integration\n> > of routes, formats and selection rectangles.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  include/libcamera/internal/v4l2_subdevice.h |  9 ++-\n> >  src/libcamera/pipeline/simple/simple.cpp    |  8 +-\n> >  src/libcamera/v4l2_subdevice.cpp            | 86 ++++++++++++++++++---\n> >  3 files changed, 89 insertions(+), 14 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> > index 2f64b3deadfe..38c340236908 100644\n> > --- a/include/libcamera/internal/v4l2_subdevice.h\n> > +++ b/include/libcamera/internal/v4l2_subdevice.h\n> > @@ -85,7 +85,13 @@ public:\n> >  \t\tunsigned int stream;\n> >  \t};\n> >\n> > -\tusing Routing = std::vector<struct v4l2_subdev_route>;\n> > +\tstruct Route {\n> > +\t\tStream sink;\n> > +\t\tStream source;\n> > +\t\tuint32_t flags;\n> > +\t};\n> > +\n> > +\tusing Routing = std::vector<Route>;\n> >\n> >  \texplicit V4L2Subdevice(const MediaEntity *entity);\n> >  \t~V4L2Subdevice();\n> > @@ -157,6 +163,7 @@ private:\n> >  };\n> >\n> >  std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream);\n> > +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Route &route);\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 1dbbd7fe91d6..c1eeae643d71 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -851,17 +851,17 @@ std::vector<const MediaPad *> SimpleCameraData::routedSourcePads(MediaPad *sink)\n> >\n> >  \tstd::vector<const MediaPad *> pads;\n> >\n> > -\tfor (const struct v4l2_subdev_route &route : routing) {\n> > -\t\tif (sink->index() != route.sink_pad ||\n> > +\tfor (const V4L2Subdevice::Route &route : routing) {\n> > +\t\tif (sink->index() != route.sink.pad ||\n> >  \t\t    !(route.flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))\n> >  \t\t\tcontinue;\n> >\n> > -\t\tconst MediaPad *pad = entity->getPadByIndex(route.source_pad);\n> > +\t\tconst MediaPad *pad = entity->getPadByIndex(route.source.pad);\n> >  \t\tif (!pad) {\n> >  \t\t\tLOG(SimplePipeline, Warning)\n> >  \t\t\t\t<< \"Entity \" << entity->name()\n> >  \t\t\t\t<< \" has invalid route source pad \"\n> > -\t\t\t\t<< route.source_pad;\n> > +\t\t\t\t<< route.source.pad;\n> >  \t\t}\n> >\n> >  \t\tpads.push_back(pad);\n> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > index 1670c31a9656..2e9e05c86059 100644\n> > --- a/src/libcamera/v4l2_subdevice.cpp\n> > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > @@ -843,6 +843,39 @@ std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream)\n> >  \treturn out;\n> >  }\n> >\n> > +/**\n> > + * \\class V4L2Subdevice::Route\n> > + * \\brief V4L2 subdevice routing table entry\n> > + *\n> > + * This class models a route in the subdevice routing table. It is similar to\n> > + * the v4l2_subdev_route structure, but uses the V4L2Subdevice::Stream class\n> > + * for easier usage with the V4L2Subdevice stream-aware functions.\n> > + *\n> > + * \\var V4L2Subdevice::Route::sink\n> > + * \\brief The sink stream of the route\n> > + *\n> > + * \\var V4L2Subdevice::Route::source\n> > + * \\brief The source stream of the route\n> > + *\n> > + * \\var V4L2Subdevice::Route::flags\n> > + * \\brief The route flags (V4L2_SUBDEV_ROUTE_FL_*)\n> > + */\n> > +\n> > +/**\n> > + * \\brief Insert a text representation of a V4L2Subdevice::Route into an\n> > + *\toutput stream\n> > + * \\param[in] out The output stream\n> > + * \\param[in] route The V4L2Subdevice::Route\n> > + * \\return The output stream \\a out\n> > + */\n> > +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Route &route)\n> > +{\n> > +\tout << route.sink << \" -> \" << route.source\n> > +\t    << \" (\" << utils::hex(route.flags) << \")\";\n> > +\n> > +\treturn out;\n> > +}\n> > +\n> >  /**\n> >   * \\typedef V4L2Subdevice::Routing\n> >   * \\brief V4L2 subdevice routing table\n> > @@ -860,10 +893,7 @@ std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream)\n> >  std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Routing &routing)\n> >  {\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\tout << \"[\" << i << \"] \" << route;\n> >  \t\tif (i != routing.size() - 1)\n> >  \t\t\tout << \", \";\n> >  \t}\n> > @@ -1222,6 +1252,30 @@ int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n> >   * \\return 0 on success or a negative error code otherwise\n> >   */\n> >\n> > +namespace {\n> > +\n> > +void routeFromKernel(V4L2Subdevice::Route &route,\n> > +\t\t     const struct v4l2_subdev_route &kroute)\n> > +{\n> > +\troute.sink.pad = kroute.sink_pad;\n> > +\troute.sink.stream = kroute.sink_stream;\n> > +\troute.source.pad = kroute.source_pad;\n> > +\troute.source.stream = kroute.source_stream;\n> > +\troute.flags = kroute.flags;\n> > +}\n> > +\n> > +void routeToKernel(const V4L2Subdevice::Route &route,\n> > +\t\t   struct v4l2_subdev_route &kroute)\n> > +{\n> > +\tkroute.sink_pad = route.sink.pad;\n> > +\tkroute.sink_stream = route.sink.stream;\n> > +\tkroute.source_pad = route.source.pad;\n> > +\tkroute.source_stream = route.source.stream;\n> > +\tkroute.flags = route.flags;\n> > +}\n> > +\n> > +} /* namespace */\n> > +\n> \n> Why 2 helpers in an anonymous namespace ? I would question if these\n> are worth to be helpers at all, as they're called from a single\n> place..\n\nrouteFromKernel() is called in two places, hence the helper. I then\nadded routeToKernel() as well as I found it more readable to have both\nhelpers.\n\n> >  /**\n> >   * \\brief Retrieve the subdevice's internal routing table\n> >   * \\param[out] routing The routing table\n> > @@ -1250,8 +1304,8 @@ int V4L2Subdevice::getRouting(Routing *routing, Whence whence)\n> >  \t\treturn ret;\n> >  \t}\n> >\n> > -\trouting->resize(rt.num_routes);\n> > -\trt.routes = reinterpret_cast<uintptr_t>(routing->data());\n> > +\tstd::vector<struct v4l2_subdev_route> routes{ rt.num_routes };\n> > +\trt.routes = reinterpret_cast<uintptr_t>(routes.data());\n> >\n> >  \tret = ioctl(VIDIOC_SUBDEV_G_ROUTING, &rt);\n> >  \tif (ret) {\n> > @@ -1260,11 +1314,16 @@ int V4L2Subdevice::getRouting(Routing *routing, Whence whence)\n> >  \t\treturn ret;\n> >  \t}\n> >\n> > -\tif (rt.num_routes != routing->size()) {\n> > +\tif (rt.num_routes != routes.size()) {\n> >  \t\tLOG(V4L2, Error) << \"Invalid number of routes\";\n> >  \t\treturn -EINVAL;\n> >  \t}\n> >\n> > +\trouting->resize(rt.num_routes);\n> > +\n> > +\tfor (const auto &[i, route] : utils::enumerate(routes))\n> > +\t\trouteFromKernel((*routing)[i], route);\n> > +\n> >  \treturn 0;\n> >  }\n> >\n> > @@ -1285,10 +1344,15 @@ int V4L2Subdevice::setRouting(Routing *routing, Whence whence)\n> >  \tif (!caps_.hasStreams())\n> >  \t\treturn 0;\n> >\n> > +\tstd::vector<struct v4l2_subdev_route> routes{ routing->size() };\n> > +\n> > +\tfor (const auto &[i, route] : utils::enumerate(*routing))\n> > +\t\trouteToKernel(route, routes[i]);\n> > +\n> >  \tstruct v4l2_subdev_routing rt = {};\n> >  \trt.which = whence;\n> > -\trt.num_routes = routing->size();\n> > -\trt.routes = reinterpret_cast<uintptr_t>(routing->data());\n> > +\trt.num_routes = routes.size();\n> > +\trt.routes = reinterpret_cast<uintptr_t>(routes.data());\n> >\n> >  \tint ret = ioctl(VIDIOC_SUBDEV_S_ROUTING, &rt);\n> >  \tif (ret) {\n> > @@ -1296,8 +1360,12 @@ int V4L2Subdevice::setRouting(Routing *routing, Whence whence)\n> >  \t\treturn ret;\n> >  \t}\n> >\n> > +\troutes.resize(rt.num_routes);\n> >  \trouting->resize(rt.num_routes);\n> >\n> > +\tfor (const auto &[i, route] : utils::enumerate(routes))\n> > +\t\trouteFromKernel((*routing)[i], route);\n> > +\n> >  \treturn 0;\n> >  }\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 48107BD80A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 28 Feb 2024 10:53:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F10CA62865;\n\tWed, 28 Feb 2024 11:53:43 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9B2F262865\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 28 Feb 2024 11:53:41 +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 D4B90672;\n\tWed, 28 Feb 2024 11:53:28 +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=\"M942ebhH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709117609;\n\tbh=A1FzB7FR21W3k0AIzdl5ywAkcKDjVVJsG0ZmMg7WfSY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=M942ebhHQmpkvTg7U972J60DjVhsXnJ5wX+oFtk1ZDMZugEWZOODQ3g9OaSp+t5Yc\n\tQQ6JUHCcTIAXHufEvjR4bm9UjT0up5yxBok52Ls9zNX4GWiGpyBc8kYuUln5WMtS8W\n\tueMqeI3PdHM9X/Ye1E2tfPuXHqPpAhzaz/Ufdwog=","Date":"Wed, 28 Feb 2024 12:53:43 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Subject":"Re: [PATCH 9/9] libcamera: v4l2_subdevice: Add V4L2Subdevice::Route\n\tstructure","Message-ID":"<20240228105343.GJ3419@pendragon.ideasonboard.com>","References":"<20240227140953.26093-1-laurent.pinchart@ideasonboard.com>\n\t<20240227140953.26093-10-laurent.pinchart@ideasonboard.com>\n\t<evkcho4k3fqkuhoegn5uhpndslhpxdj2ao4lqlks7hvsjtlmn4@66hm3kmkpms5>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<evkcho4k3fqkuhoegn5uhpndslhpxdj2ao4lqlks7hvsjtlmn4@66hm3kmkpms5>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]