[{"id":24274,"web_url":"https://patchwork.libcamera.org/comment/24274/","msgid":"<20220801162523.gc5nhfhmtinlz6ia@uno.localdomain>","date":"2022-08-01T16:25:23","subject":"Re: [libcamera-devel] [PATCH 12/13] libcamera: pipeline: simple:\n\tDon't disable links carrying other streams","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Mon, Aug 01, 2022 at 03:05:42AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> From: Phi-Bang Nguyen \n>\n> If a subdev supports the internal routing API, pads unrelated to the\n> pipeline for a given camera sensor may carry streams for other cameras.\n> The link setup logic is updated to take this into account, by avoiding\n> disabling links to unrelated pads.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 28 +++++++++++++++++++++---\n> 1 file changed, 25 insertions(+), 3 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 2a8811183907..c80e462bc449 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -221,6 +221,11 @@ public:\n> \tstruct Entity {\n> \t\t/* The media entity, always valid. */\n> \t\tMediaEntity *entity;\n> +\t\t/*\n> +\t\t * Whether or not the entity is a subdev that supports the\n> +\t\t * routing API.\n> +\t\t */\n> +\t\tbool supportsRouting;\n> \t\t/*\n> \t\t * The local sink pad connected to the upstream entity, null for\n> \t\t * the camera sensor at the beginning of the pipeline.\n> @@ -404,9 +409,13 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> \t\t */\n>\n> \t\tstd::vector pads;\n> +\t\tbool supportsRouting = false;\n>\n> -\t\tif (sinkPad)\n> +\t\tif (sinkPad) {\n> \t\t\tpads = routedSourcePads(sinkPad);\n> +\t\t\tif (!pads.empty())\n> +\t\t\t\tsupportsRouting = true;\n> +\t\t}\n>\n> \t\tif (pads.empty()) {\n> \t\t\tfor (const MediaPad *pad : entity->pads()) {\n> @@ -421,7 +430,9 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> \t\t\t\tMediaEntity *next = link->sink()->entity();\n> \t\t\t\tif (visited.find(next) == visited.end()) {\n> \t\t\t\t\tqueue.push({ next, link->sink() });\n> -\t\t\t\t\tparents.insert({ next, { entity, sinkPad, pad, link } });\n> +\n> +\t\t\t\t\tEntity e{ entity, supportsRouting, sinkPad, pad, link };\n> +\t\t\t\t\tparents.insert({ next, e });\n> \t\t\t\t}\n> \t\t\t}\n> \t\t}\n> @@ -435,7 +446,7 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> \t * to the sensor. Store all the entities in the pipeline, from the\n> \t * camera sensor to the video node, in entities_.\n> \t */\n> -\tentities_.push_front({ entity, sinkPad, nullptr, nullptr });\n> +\tentities_.push_front({ entity, false, sinkPad, nullptr, nullptr });\n>\n> \tfor (auto it = parents.find(entity); it != parents.end();\n> \t it = parents.find(entity)) {\n> @@ -617,6 +628,17 @@ int SimpleCameraData::setupLinks()\n\nI would also update the comment at function begin that reports\n\n\t/*\n\t * Configure all links along the pipeline. Some entities may not allow\n\t * multiple sink links to be enabled together, even on different sink\n\t * pads. We must thus start by disabling all sink links (but the one we\n\t * want to enable) before enabling the pipeline link.\n\t */\n\nTo mention that this doesn't apply to routed subdevs\n\n> \t\t}\n>\n> \t\tfor (MediaPad *pad : e.entity->pads()) {\n> +\t\t\t/*\n> +\t\t\t * If the entity supports the V4L2 internal routing API,\n> +\t\t\t * assume that it may carry multiple independent streams\n> +\t\t\t * concurrently, and only disable links on the sink and\n> +\t\t\t * source pads used by the pipeline.\n> +\t\t\t */\n> +\t\t\tif (e.supportsRouting) {\n> +\t\t\t\tif (pad != e.sink && pad != e.source)\n\nOr\n if (e.supportsRouting &&\n (pad != e.sink && pad != e.source))\n\n> +\t\t\t\t\tcontinue;\n> +\t\t\t}\n> +\n> \t\t\tfor (MediaLink *link : pad->links()) {\n> \t\t\t\tif (link == sinkLink)\n> \t\t\t\t\tcontinue;\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 E4D66C3275\n\tfor ;\n\tMon, 1 Aug 2022 16:25:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6E96363312;\n\tMon, 1 Aug 2022 18:25:30 +0200 (CEST)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 130F6603E8\n\tfor ;\n\tMon, 1 Aug 2022 18:25:29 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id A1D791BF205;\n\tMon, 1 Aug 2022 16:25:27 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659371130;\n\tbh=bkgs80L40R/zsVbE8HuWc9EvKAaAJB/3vExc10gtkJA=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=pujmTDOXtZ8rTgnDT32Fg3ZiR+rF9uGtXWP3F1tnWnPYYX7pZEkHCaYCamgMkgvow\n\tGIh6whoSS0c87oNZDk/o7Way87emOKp8sITpM6X1LEn0T1eD00gYFnEPfGMekXJnKz\n\tzgsJyuScOD3G07F2XU8+gisGc7kBnE5seffOO0uhkU2/L78x+XNKEqt+dKEXJb95+o\n\tf1V6D+I9XCHYWjxE0B8E2hmRvCsOj1mpnF9GNVqve2c3RgcA+7WVvgBFDTw7dg53yp\n\tinwXwGztV8MNGWLadbqQye98/RLLCRE0wgFHb2qTEusRAxiXDd5IwLfdvY9/geKm88\n\tYekvxDl3VgUug==","Date":"Mon, 1 Aug 2022 18:25:23 +0200","To":"Laurent Pinchart ","Message-ID":"<20220801162523.gc5nhfhmtinlz6ia@uno.localdomain>","References":"<20220801000543.3501-1-laurent.pinchart@ideasonboard.com>\n\t<20220801000543.3501-13-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220801000543.3501-13-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 12/13] libcamera: pipeline: simple:\n\tDon't disable links carrying other streams","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24281,"web_url":"https://patchwork.libcamera.org/comment/24281/","msgid":"","date":"2022-08-01T20:59:33","subject":"Re: [libcamera-devel] [PATCH 12/13] libcamera: pipeline: simple:\n\tDon't disable links carrying other streams","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Aug 01, 2022 at 06:25:23PM +0200, Jacopo Mondi wrote:\n> On Mon, Aug 01, 2022 at 03:05:42AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > From: Phi-Bang Nguyen \n> >\n> > If a subdev supports the internal routing API, pads unrelated to the\n> > pipeline for a given camera sensor may carry streams for other cameras.\n> > The link setup logic is updated to take this into account, by avoiding\n> > disabling links to unrelated pads.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/pipeline/simple/simple.cpp | 28 +++++++++++++++++++++---\n> > 1 file changed, 25 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 2a8811183907..c80e462bc449 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -221,6 +221,11 @@ public:\n> > \tstruct Entity {\n> > \t\t/* The media entity, always valid. */\n> > \t\tMediaEntity *entity;\n> > +\t\t/*\n> > +\t\t * Whether or not the entity is a subdev that supports the\n> > +\t\t * routing API.\n> > +\t\t */\n> > +\t\tbool supportsRouting;\n> > \t\t/*\n> > \t\t * The local sink pad connected to the upstream entity, null for\n> > \t\t * the camera sensor at the beginning of the pipeline.\n> > @@ -404,9 +409,13 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> > \t\t */\n> >\n> > \t\tstd::vector pads;\n> > +\t\tbool supportsRouting = false;\n> >\n> > -\t\tif (sinkPad)\n> > +\t\tif (sinkPad) {\n> > \t\t\tpads = routedSourcePads(sinkPad);\n> > +\t\t\tif (!pads.empty())\n> > +\t\t\t\tsupportsRouting = true;\n> > +\t\t}\n> >\n> > \t\tif (pads.empty()) {\n> > \t\t\tfor (const MediaPad *pad : entity->pads()) {\n> > @@ -421,7 +430,9 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> > \t\t\t\tMediaEntity *next = link->sink()->entity();\n> > \t\t\t\tif (visited.find(next) == visited.end()) {\n> > \t\t\t\t\tqueue.push({ next, link->sink() });\n> > -\t\t\t\t\tparents.insert({ next, { entity, sinkPad, pad, link } });\n> > +\n> > +\t\t\t\t\tEntity e{ entity, supportsRouting, sinkPad, pad, link };\n> > +\t\t\t\t\tparents.insert({ next, e });\n> > \t\t\t\t}\n> > \t\t\t}\n> > \t\t}\n> > @@ -435,7 +446,7 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> > \t * to the sensor. Store all the entities in the pipeline, from the\n> > \t * camera sensor to the video node, in entities_.\n> > \t */\n> > -\tentities_.push_front({ entity, sinkPad, nullptr, nullptr });\n> > +\tentities_.push_front({ entity, false, sinkPad, nullptr, nullptr });\n> >\n> > \tfor (auto it = parents.find(entity); it != parents.end();\n> > \t it = parents.find(entity)) {\n> > @@ -617,6 +628,17 @@ int SimpleCameraData::setupLinks()\n> \n> I would also update the comment at function begin that reports\n> \n> \t/*\n> \t * Configure all links along the pipeline. Some entities may not allow\n> \t * multiple sink links to be enabled together, even on different sink\n> \t * pads. We must thus start by disabling all sink links (but the one we\n> \t * want to enable) before enabling the pipeline link.\n> \t */\n> \n> To mention that this doesn't apply to routed subdevs\n\nI had a look but it would really duplicate the comment below. As it\ndescribes the internal implementation and not a public API, and the two\ncomments are 9 lines of code apart, would you mind if I left it as-is ?\n\n> > \t\t}\n> >\n> > \t\tfor (MediaPad *pad : e.entity->pads()) {\n> > +\t\t\t/*\n> > +\t\t\t * If the entity supports the V4L2 internal routing API,\n> > +\t\t\t * assume that it may carry multiple independent streams\n> > +\t\t\t * concurrently, and only disable links on the sink and\n> > +\t\t\t * source pads used by the pipeline.\n> > +\t\t\t */\n> > +\t\t\tif (e.supportsRouting) {\n> > +\t\t\t\tif (pad != e.sink && pad != e.source)\n> \n> Or\n> if (e.supportsRouting &&\n> (pad != e.sink && pad != e.source))\n\nI'll drop the inner parentheses too.\n\n> > +\t\t\t\t\tcontinue;\n> > +\t\t\t}\n> > +\n> > \t\t\tfor (MediaLink *link : pad->links()) {\n> > \t\t\t\tif (link == sinkLink)\n> > \t\t\t\t\tcontinue;","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 974B2C3275\n\tfor ;\n\tMon, 1 Aug 2022 20:59:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E7EA163312;\n\tMon, 1 Aug 2022 22:59:38 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E7617603E8\n\tfor ;\n\tMon, 1 Aug 2022 22:59:37 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 58670512;\n\tMon, 1 Aug 2022 22:59:37 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659387578;\n\tbh=X4Zotv3G7HWRSHB5eLxznlfTI1vMSDKGU6PMniJiW8w=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=fcuc2j7+rOqcDM6n6n9yLBRzNNbirspI6kpJyTeexQ7bGs1TcXior3XjAxC5Lt6RO\n\tN+DugG9kmWIObrzO+BsuD9RAZUi3tVB/BeiltO32TrlRtIZIddwLXYGT9aJUCqqPXO\n\t43cN4fikHYEuElq2yWPYlOSxrDKGERgrgUa6J1HEGQn1AM0HVJ/fUx81gmjHPhn/Vg\n\tOR1qCMU7eOHIguCwpGS9Bg9miimlToOboWoAk25j9+hLI7CfK5vj5kn+DS0Khm3lFr\n\tTsSsWibTkMHvFPXxnXp+2Ap5nWGR5g3TP83u2YlHLPaji1UT+uIDZ4/RS2HiAc0M+V\n\tgn3Bh0FmFEvrA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659387577;\n\tbh=X4Zotv3G7HWRSHB5eLxznlfTI1vMSDKGU6PMniJiW8w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HidfgBQnadtV6DLvfrM9Sjdg0C74TesgriT0Cp8DWe9m8KxEUC1Q/V78BtBJDL5cP\n\tuNLHWRBHlPiT58hyKZGMhsRUtDd5qZn2jT3rBGXTzx7cSCBCoFMkCOJRp2dhWYUD8a\n\tkzb8FJlVuL6uWoQeknIg1shQrvvbKK8L9nA6rvek="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"HidfgBQn\"; dkim-atps=neutral","Date":"Mon, 1 Aug 2022 23:59:33 +0300","To":"Jacopo Mondi ","Message-ID":"","References":"<20220801000543.3501-1-laurent.pinchart@ideasonboard.com>\n\t<20220801000543.3501-13-laurent.pinchart@ideasonboard.com>\n\t<20220801162523.gc5nhfhmtinlz6ia@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220801162523.gc5nhfhmtinlz6ia@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 12/13] libcamera: pipeline: simple:\n\tDon't disable links carrying other streams","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24288,"web_url":"https://patchwork.libcamera.org/comment/24288/","msgid":"<20220802074923.irq5l5ikmsqvcngu@uno.localdomain>","date":"2022-08-02T07:51:10","subject":"Re: [libcamera-devel] [PATCH 12/13] libcamera: pipeline: simple:\n\tDon't disable links carrying other streams","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Mon, Aug 01, 2022 at 11:59:33PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Mon, Aug 01, 2022 at 06:25:23PM +0200, Jacopo Mondi wrote:\n> > On Mon, Aug 01, 2022 at 03:05:42AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > From: Phi-Bang Nguyen \n> > >\n> > > If a subdev supports the internal routing API, pads unrelated to the\n> > > pipeline for a given camera sensor may carry streams for other cameras.\n> > > The link setup logic is updated to take this into account, by avoiding\n> > > disabling links to unrelated pads.\n> > >\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/libcamera/pipeline/simple/simple.cpp | 28 +++++++++++++++++++++---\n> > > 1 file changed, 25 insertions(+), 3 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > > index 2a8811183907..c80e462bc449 100644\n> > > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > > @@ -221,6 +221,11 @@ public:\n> > > \tstruct Entity {\n> > > \t\t/* The media entity, always valid. */\n> > > \t\tMediaEntity *entity;\n> > > +\t\t/*\n> > > +\t\t * Whether or not the entity is a subdev that supports the\n> > > +\t\t * routing API.\n> > > +\t\t */\n> > > +\t\tbool supportsRouting;\n> > > \t\t/*\n> > > \t\t * The local sink pad connected to the upstream entity, null for\n> > > \t\t * the camera sensor at the beginning of the pipeline.\n> > > @@ -404,9 +409,13 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> > > \t\t */\n> > >\n> > > \t\tstd::vector pads;\n> > > +\t\tbool supportsRouting = false;\n> > >\n> > > -\t\tif (sinkPad)\n> > > +\t\tif (sinkPad) {\n> > > \t\t\tpads = routedSourcePads(sinkPad);\n> > > +\t\t\tif (!pads.empty())\n> > > +\t\t\t\tsupportsRouting = true;\n> > > +\t\t}\n> > >\n> > > \t\tif (pads.empty()) {\n> > > \t\t\tfor (const MediaPad *pad : entity->pads()) {\n> > > @@ -421,7 +430,9 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> > > \t\t\t\tMediaEntity *next = link->sink()->entity();\n> > > \t\t\t\tif (visited.find(next) == visited.end()) {\n> > > \t\t\t\t\tqueue.push({ next, link->sink() });\n> > > -\t\t\t\t\tparents.insert({ next, { entity, sinkPad, pad, link } });\n> > > +\n> > > +\t\t\t\t\tEntity e{ entity, supportsRouting, sinkPad, pad, link };\n> > > +\t\t\t\t\tparents.insert({ next, e });\n> > > \t\t\t\t}\n> > > \t\t\t}\n> > > \t\t}\n> > > @@ -435,7 +446,7 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> > > \t * to the sensor. Store all the entities in the pipeline, from the\n> > > \t * camera sensor to the video node, in entities_.\n> > > \t */\n> > > -\tentities_.push_front({ entity, sinkPad, nullptr, nullptr });\n> > > +\tentities_.push_front({ entity, false, sinkPad, nullptr, nullptr });\n> > >\n> > > \tfor (auto it = parents.find(entity); it != parents.end();\n> > > \t it = parents.find(entity)) {\n> > > @@ -617,6 +628,17 @@ int SimpleCameraData::setupLinks()\n> >\n> > I would also update the comment at function begin that reports\n> >\n> > \t/*\n> > \t * Configure all links along the pipeline. Some entities may not allow\n> > \t * multiple sink links to be enabled together, even on different sink\n> > \t * pads. We must thus start by disabling all sink links (but the one we\n> > \t * want to enable) before enabling the pipeline link.\n> > \t */\n> >\n> > To mention that this doesn't apply to routed subdevs\n>\n> I had a look but it would really duplicate the comment below. As it\n> describes the internal implementation and not a public API, and the two\n> comments are 9 lines of code apart, would you mind if I left it as-is ?\n>\n\nfine\n\n> > > \t\t}\n> > >\n> > > \t\tfor (MediaPad *pad : e.entity->pads()) {\n> > > +\t\t\t/*\n> > > +\t\t\t * If the entity supports the V4L2 internal routing API,\n> > > +\t\t\t * assume that it may carry multiple independent streams\n> > > +\t\t\t * concurrently, and only disable links on the sink and\n> > > +\t\t\t * source pads used by the pipeline.\n> > > +\t\t\t */\n> > > +\t\t\tif (e.supportsRouting) {\n> > > +\t\t\t\tif (pad != e.sink && pad != e.source)\n> >\n> > Or\n> > if (e.supportsRouting &&\n> > (pad != e.sink && pad != e.source))\n>\n> I'll drop the inner parentheses too.\n\nBetter thanks\n\n>\n> > > +\t\t\t\t\tcontinue;\n> > > +\t\t\t}\n> > > +\n> > > \t\t\tfor (MediaLink *link : pad->links()) {\n> > > \t\t\t\tif (link == sinkLink)\n> > > \t\t\t\t\tcontinue;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 7D1EBBE173\n\tfor ;\n\tTue, 2 Aug 2022 07:51:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E9A4F63312;\n\tTue, 2 Aug 2022 09:51:15 +0200 (CEST)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::221])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7D2BA6330D\n\tfor ;\n\tTue, 2 Aug 2022 09:51:14 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id BA236240003;\n\tTue, 2 Aug 2022 07:51:13 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659426676;\n\tbh=FShosmjYBdtXhp0Udf1wA0sI62JG3EYyYkcxMoBA9YM=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=X4w/61aJG0oDLp2pCrgD6Dd+Fo1nxE7LDW9jBDPEroTPGmwLTREwumWTfahRfSI0r\n\tvpd+/6IdBvrRwFH/t2oOFgW7WJ9bs5HqXEoZnCsffTs50trOMisCFUk6R5jKOJ5eDg\n\tY+lZxdz6RkUN8GvjV+3uN6y3L2/BOieA494mLcRjAgDe11O4Fg3sNXJHwDyVJho3fR\n\tMQouB6pXi7AzCz4JngcNJbTijQPHv9ipezj4ZKM9B+Wr0T63blQxvXPUZXhYR/GJlp\n\tTE+mFHdWBhislP6cT4B5ocmrgwG6gOhmvrAWtUjmEMDkuETiT54NcFRP4lhNLzjwYr\n\t7NKUOwheTyXyQ==","Date":"Tue, 2 Aug 2022 09:51:10 +0200","To":"Laurent Pinchart ","Message-ID":"<20220802074923.irq5l5ikmsqvcngu@uno.localdomain>","References":"<20220801000543.3501-1-laurent.pinchart@ideasonboard.com>\n\t<20220801000543.3501-13-laurent.pinchart@ideasonboard.com>\n\t<20220801162523.gc5nhfhmtinlz6ia@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 12/13] libcamera: pipeline: simple:\n\tDon't disable links carrying other streams","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24341,"web_url":"https://patchwork.libcamera.org/comment/24341/","msgid":"<20220803120434.v64usjibrz4fcy5s@uno.localdomain>","date":"2022-08-03T12:04:34","subject":"Re: [libcamera-devel] [PATCH 12/13] libcamera: pipeline: simple:\n\tDon't disable links carrying other streams","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\n forgot to leave a tag here\n\n Reviewed-by: Jacopo Mondi \n\n\nOn Tue, Aug 02, 2022 at 09:51:10AM +0200, Jacopo Mondi via libcamera-devel wrote:\n> Hi Laurent\n>\n> On Mon, Aug 01, 2022 at 11:59:33PM +0300, Laurent Pinchart wrote:\n> > Hi Jacopo,\n> >\n> > On Mon, Aug 01, 2022 at 06:25:23PM +0200, Jacopo Mondi wrote:\n> > > On Mon, Aug 01, 2022 at 03:05:42AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > > From: Phi-Bang Nguyen \n> > > >\n> > > > If a subdev supports the internal routing API, pads unrelated to the\n> > > > pipeline for a given camera sensor may carry streams for other cameras.\n> > > > The link setup logic is updated to take this into account, by avoiding\n> > > > disabling links to unrelated pads.\n> > > >\n> > > > Signed-off-by: Laurent Pinchart \n> > > > ---\n> > > > src/libcamera/pipeline/simple/simple.cpp | 28 +++++++++++++++++++++---\n> > > > 1 file changed, 25 insertions(+), 3 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > > > index 2a8811183907..c80e462bc449 100644\n> > > > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > > > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > > > @@ -221,6 +221,11 @@ public:\n> > > > \tstruct Entity {\n> > > > \t\t/* The media entity, always valid. */\n> > > > \t\tMediaEntity *entity;\n> > > > +\t\t/*\n> > > > +\t\t * Whether or not the entity is a subdev that supports the\n> > > > +\t\t * routing API.\n> > > > +\t\t */\n> > > > +\t\tbool supportsRouting;\n> > > > \t\t/*\n> > > > \t\t * The local sink pad connected to the upstream entity, null for\n> > > > \t\t * the camera sensor at the beginning of the pipeline.\n> > > > @@ -404,9 +409,13 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> > > > \t\t */\n> > > >\n> > > > \t\tstd::vector pads;\n> > > > +\t\tbool supportsRouting = false;\n> > > >\n> > > > -\t\tif (sinkPad)\n> > > > +\t\tif (sinkPad) {\n> > > > \t\t\tpads = routedSourcePads(sinkPad);\n> > > > +\t\t\tif (!pads.empty())\n> > > > +\t\t\t\tsupportsRouting = true;\n> > > > +\t\t}\n> > > >\n> > > > \t\tif (pads.empty()) {\n> > > > \t\t\tfor (const MediaPad *pad : entity->pads()) {\n> > > > @@ -421,7 +430,9 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> > > > \t\t\t\tMediaEntity *next = link->sink()->entity();\n> > > > \t\t\t\tif (visited.find(next) == visited.end()) {\n> > > > \t\t\t\t\tqueue.push({ next, link->sink() });\n> > > > -\t\t\t\t\tparents.insert({ next, { entity, sinkPad, pad, link } });\n> > > > +\n> > > > +\t\t\t\t\tEntity e{ entity, supportsRouting, sinkPad, pad, link };\n> > > > +\t\t\t\t\tparents.insert({ next, e });\n> > > > \t\t\t\t}\n> > > > \t\t\t}\n> > > > \t\t}\n> > > > @@ -435,7 +446,7 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> > > > \t * to the sensor. Store all the entities in the pipeline, from the\n> > > > \t * camera sensor to the video node, in entities_.\n> > > > \t */\n> > > > -\tentities_.push_front({ entity, sinkPad, nullptr, nullptr });\n> > > > +\tentities_.push_front({ entity, false, sinkPad, nullptr, nullptr });\n> > > >\n> > > > \tfor (auto it = parents.find(entity); it != parents.end();\n> > > > \t it = parents.find(entity)) {\n> > > > @@ -617,6 +628,17 @@ int SimpleCameraData::setupLinks()\n> > >\n> > > I would also update the comment at function begin that reports\n> > >\n> > > \t/*\n> > > \t * Configure all links along the pipeline. Some entities may not allow\n> > > \t * multiple sink links to be enabled together, even on different sink\n> > > \t * pads. We must thus start by disabling all sink links (but the one we\n> > > \t * want to enable) before enabling the pipeline link.\n> > > \t */\n> > >\n> > > To mention that this doesn't apply to routed subdevs\n> >\n> > I had a look but it would really duplicate the comment below. As it\n> > describes the internal implementation and not a public API, and the two\n> > comments are 9 lines of code apart, would you mind if I left it as-is ?\n> >\n>\n> fine\n>\n> > > > \t\t}\n> > > >\n> > > > \t\tfor (MediaPad *pad : e.entity->pads()) {\n> > > > +\t\t\t/*\n> > > > +\t\t\t * If the entity supports the V4L2 internal routing API,\n> > > > +\t\t\t * assume that it may carry multiple independent streams\n> > > > +\t\t\t * concurrently, and only disable links on the sink and\n> > > > +\t\t\t * source pads used by the pipeline.\n> > > > +\t\t\t */\n> > > > +\t\t\tif (e.supportsRouting) {\n> > > > +\t\t\t\tif (pad != e.sink && pad != e.source)\n> > >\n> > > Or\n> > > if (e.supportsRouting &&\n> > > (pad != e.sink && pad != e.source))\n> >\n> > I'll drop the inner parentheses too.\n>\n> Better thanks\n>\n> >\n> > > > +\t\t\t\t\tcontinue;\n> > > > +\t\t\t}\n> > > > +\n> > > > \t\t\tfor (MediaLink *link : pad->links()) {\n> > > > \t\t\t\tif (link == sinkLink)\n> > > > \t\t\t\t\tcontinue;\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart","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 04A21C3272\n\tfor ;\n\tWed, 3 Aug 2022 12:04:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C628863310;\n\tWed, 3 Aug 2022 14:04:37 +0200 (CEST)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::228])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 458A0603E6\n\tfor ;\n\tWed, 3 Aug 2022 14:04:37 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 76A611BF205;\n\tWed, 3 Aug 2022 12:04:36 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659528277;\n\tbh=nVIZ3a5ZbXVXeLlNMTXQX+CLBM0Bp8o8aSuxt3U1CSU=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=3bWNOi2iaau84BHwH6hPeiHlOtSd+EQAUscOnfSLppyLE7TxU/89SlN1h6yqCDrlw\n\tfBSaiON7ZF4RRdngRLJS5+jEw/WAwPo8sEEiQKYgmhuDYR4dIT37PcX0B/oX+zJxuA\n\t/Vy/E4+3/gIE0/BLaYTs1VAsWsPTJmoKajwpcUoFAxfvnNXL7sYbLqVgSTcLx/iSt9\n\tTDYC+d2W2lCxxplJKi21CYUvUg3Ml8nFkzzwReC1lFDtYYgM6YBDLFlPhfxqAn42A8\n\tA6BEI0ieo0+4djneqmFZ6ItN7MnKyM5p91PiG8qg1SK4/tqr29przOJkkTAun/Cgi+\n\td2zgPw4R6xE4g==","Date":"Wed, 3 Aug 2022 14:04:34 +0200","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20220803120434.v64usjibrz4fcy5s@uno.localdomain>","References":"<20220801000543.3501-1-laurent.pinchart@ideasonboard.com>\n\t<20220801000543.3501-13-laurent.pinchart@ideasonboard.com>\n\t<20220801162523.gc5nhfhmtinlz6ia@uno.localdomain>\n\t\n\t<20220802074923.irq5l5ikmsqvcngu@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220802074923.irq5l5ikmsqvcngu@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 12/13] libcamera: pipeline: simple:\n\tDon't disable links carrying other streams","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]