{"id":19728,"url":"https://patchwork.libcamera.org/api/1.1/patches/19728/?format=json","web_url":"https://patchwork.libcamera.org/patch/19728/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20240315001613.2033-10-laurent.pinchart@ideasonboard.com>","date":"2024-03-15T00:16:08","name":"[v2,09/14] libcamera: v4l2_subdevice: Add V4L2Subdevice::Route structure","commit_ref":"c5a8152af269e798297debcf6ab1bfb5ed82ebc1","pull_url":null,"state":"accepted","archived":false,"hash":"5d50b691eca6505a3aa66d1175b9104e605c11be","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/1.1/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/19728/mbox/","series":[{"id":4226,"url":"https://patchwork.libcamera.org/api/1.1/series/4226/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4226","date":"2024-03-15T00:15:59","name":"libcamera: Prepare for new camera sensor class","version":2,"mbox":"https://patchwork.libcamera.org/series/4226/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/19728/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/19728/checks/","tags":{},"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 8E017BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 15 Mar 2024 00:16:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D634162CAE;\n\tFri, 15 Mar 2024 01:16:38 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 450A362C93\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 15 Mar 2024 01:16:29 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 909B4A9A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 15 Mar 2024 01:16:05 +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=\"JeIAtNl5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1710461765;\n\tbh=GqtDp/jzJkaFBfGx4Qgz8/jXuDT4suXoHYmy2+893xQ=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=JeIAtNl5z2lH7o0ARzHwaKwx8iIXzMjV55uLYaerdow0oUZ/VveSid/ngJMgFEmaU\n\t4imxjdOdrTTmZnbIfnsbdJR4no5Erdbdn45G8ssXF4JdCkOYjuGIFB2dplxjotQhDf\n\tbvw6cOA9TahWX++AalFRaV1QlS7IO5bgTSr3D59Y=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Subject":"[PATCH v2 09/14] libcamera: v4l2_subdevice: Add V4L2Subdevice::Route\n\tstructure","Date":"Fri, 15 Mar 2024 02:16:08 +0200","Message-ID":"<20240315001613.2033-10-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.43.2","In-Reply-To":"<20240315001613.2033-1-laurent.pinchart@ideasonboard.com>","References":"<20240315001613.2033-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"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\nExpressing streams in different ways requires pipeline handlers and\nother helpers to convert between the two representations. This isn't\nmuch of an issue yet as libcamera has little stream-aware code, but it\nis expected to increasingly become a burden.\n\nTo simplify the API, introduce a V4L2Subdevice::Route structure that\nmimicks the kernel v4l2_subdev_route structure but represents streams as\nV4L2Subdevice::Stream instances. This will improve seamless integration\nof routes, formats and selection rectangles.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n---\nChanges since combined RFC:\n\n- Fix compilation in imx8-isi pipeline handler\n- Fix indentation in comments\n\nChanges since v1:\n\n- Clear routing at the beginning of getRouting() and setRouting()\n---\n include/libcamera/internal/v4l2_subdevice.h  |  19 +++-\n src/libcamera/pipeline/imx8-isi/imx8-isi.cpp |  14 +--\n src/libcamera/pipeline/simple/simple.cpp     |   8 +-\n src/libcamera/v4l2_subdevice.cpp             | 108 +++++++++++++++++--\n 4 files changed, 123 insertions(+), 26 deletions(-)","diff":"diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\nindex 2939dc2411c6..01ed4c2fc397 100644\n--- a/include/libcamera/internal/v4l2_subdevice.h\n+++ b/include/libcamera/internal/v4l2_subdevice.h\n@@ -95,7 +95,23 @@ public:\n \t\tunsigned int stream;\n \t};\n \n-\tusing Routing = std::vector<struct v4l2_subdev_route>;\n+\tstruct Route {\n+\t\tRoute()\n+\t\t\t: flags(0)\n+\t\t{\n+\t\t}\n+\n+\t\tRoute(const Stream &snk, const Stream &src, uint32_t f)\n+\t\t\t: sink(snk), source(src), flags(f)\n+\t\t{\n+\t\t}\n+\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@@ -174,6 +190,7 @@ static inline bool operator!=(const V4L2Subdevice::Stream &lhs,\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 */\ndiff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\nindex a3782aea2ba9..63082cea7e56 100644\n--- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n+++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n@@ -827,16 +827,10 @@ int PipelineHandlerISI::configure(Camera *camera, CameraConfiguration *c)\n \tunsigned int xbarFirstSource = crossbar_->entity()->pads().size() / 2 + 1;\n \n \tfor (const auto &[idx, config] : utils::enumerate(*c)) {\n-\t\tstruct v4l2_subdev_route route = {\n-\t\t\t.sink_pad = data->xbarSink_,\n-\t\t\t.sink_stream = 0,\n-\t\t\t.source_pad = static_cast<uint32_t>(xbarFirstSource + idx),\n-\t\t\t.source_stream = 0,\n-\t\t\t.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,\n-\t\t\t.reserved = {}\n-\t\t};\n-\n-\t\trouting.push_back(route);\n+\t\tuint32_t sourcePad = xbarFirstSource + idx;\n+\t\trouting.emplace_back(V4L2Subdevice::Stream{ data->xbarSink_, 0 },\n+\t\t\t\t     V4L2Subdevice::Stream{ sourcePad, 0 },\n+\t\t\t\t     V4L2_SUBDEV_ROUTE_FL_ACTIVE);\n \t}\n \n \tint ret = crossbar_->setRouting(&routing, V4L2Subdevice::ActiveFormat);\ndiff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\nindex feea26fd124f..01f2a97798ba 100644\n--- a/src/libcamera/pipeline/simple/simple.cpp\n+++ b/src/libcamera/pipeline/simple/simple.cpp\n@@ -852,17 +852,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);\ndiff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\nindex deef681e0d3a..1076b7006b0b 100644\n--- a/src/libcamera/v4l2_subdevice.cpp\n+++ b/src/libcamera/v4l2_subdevice.cpp\n@@ -898,6 +898,53 @@ 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+ * \\fn V4L2Subdevice::Route::Route()\n+ * \\brief Construct a Route with default streams\n+ */\n+\n+/**\n+ * \\fn V4L2Subdevice::Route::Route(const Stream &sink, const Stream &source,\n+ * uint32_t flags)\n+ * \\brief Construct a Route from \\a sink to \\a source\n+ * \\param[in] sink The sink stream\n+ * \\param[in] source The source stream\n+ * \\param[in] flags The route flags\n+ */\n+\n+/**\n+ * \\brief Insert a text representation of a V4L2Subdevice::Route into an\n+ * output 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@@ -907,7 +954,7 @@ std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream)\n \n /**\n  * \\brief Insert a text representation of a V4L2Subdevice::Routing into an\n- *\toutput stream\n+ * output stream\n  * \\param[in] out The output stream\n  * \\param[in] routing The V4L2Subdevice::Routing\n  * \\return The output stream \\a out\n@@ -915,10 +962,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@@ -1272,6 +1316,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  * \\brief Retrieve the subdevice's internal routing table\n  * \\param[out] routing The routing table\n@@ -1282,6 +1350,8 @@ int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n  */\n int V4L2Subdevice::getRouting(Routing *routing, Whence whence)\n {\n+\trouting->clear();\n+\n \tif (!caps_.hasStreams())\n \t\treturn 0;\n \n@@ -1300,8 +1370,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@@ -1310,11 +1380,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@@ -1332,13 +1407,20 @@ int V4L2Subdevice::getRouting(Routing *routing, Whence whence)\n  */\n int V4L2Subdevice::setRouting(Routing *routing, Whence whence)\n {\n-\tif (!caps_.hasStreams())\n+\tif (!caps_.hasStreams()) {\n+\t\trouting->clear();\n \t\treturn 0;\n+\t}\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@@ -1346,8 +1428,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","prefixes":["v2","09/14"]}