[{"id":24272,"web_url":"https://patchwork.libcamera.org/comment/24272/","msgid":"<20220801155413.qrwi2x5bielfrlvs@uno.localdomain>","date":"2022-08-01T15:54:13","subject":"Re: [libcamera-devel] [PATCH 10/13] libcamera: pipeline: simple:\n\tSetup links in the context of sink entities","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:40AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> To setup links the pipeline handler iterates over all entities in the\n> pipeline and disables all links but the ones we want to enable. Some\n> entities don't allow multiple sink links to be enabled, so the iteration\n> needs to be based on sink entities, disabling all their links first and\n> then enabling the sink link that is part of the pipeline.\n>\n> The loop implementation iterates over all Entity instances, and uses\n> their source link to locate the MediaEntity for the connected sink. The\n> sink entity is then processed in the context of the source's loop\n> iteration. This prevents the code from being able to access extra\n> information about the sink entity, as we only have access to the\n> MediaEntity, not the Entity.\n>\n> To prepare for subdev routing support that will require accessing\n> additional entity information, refactor the loop to process the sink\n> entity in the context of its Entity instance.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nI guess that's easier than storing the sink link along with the source\none in SimpleCameraData(), just checking if you think there will be\nmore use cases where the sink link will be usefull.\n\n> ---\n>  src/libcamera/pipeline/simple/simple.cpp | 24 +++++++++++++++++-------\n>  1 file changed, 17 insertions(+), 7 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 731d355efda6..4bde9caa7254 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -578,15 +578,23 @@ int SimpleCameraData::setupLinks()\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> +\t * The entities_ list stores entities along with their source link. We\n> +\t * need to process the link in the context of the sink entity, so\n> +\t * record the source link of the current entity as the sink link of the\n> +\t * next entity, and skip the first entity in the loop.\n>  \t */\n> +\tMediaLink *sinkLink = nullptr;\n> +\n>  \tfor (SimpleCameraData::Entity &e : entities_) {\n> -\t\tif (!e.sourceLink)\n> -\t\t\tbreak;\n> +\t\tif (!sinkLink) {\n> +\t\t\tsinkLink = e.sourceLink;\n> +\t\t\tcontinue;\n> +\t\t}\n\nDo we assume to start from the sensor (which has no sink pads to\ndisable links on) ?\n\nIf that's the case, for my understanding, how is entities_ ordered ?\nIt is populated at SimpleCameraData construction time, by iterating\nover \"parents\" which is an unordered map.\n\nIs there a risk we actually start from a different entity and not the\nsensor here ?\n\nTHanks\n  j\n>\n> -\t\tMediaEntity *remote = e.sourceLink->sink()->entity();\n> -\t\tfor (MediaPad *pad : remote->pads()) {\n> +\t\tfor (MediaPad *pad : e.entity->pads()) {\n>  \t\t\tfor (MediaLink *link : pad->links()) {\n> -\t\t\t\tif (link == e.sourceLink)\n> +\t\t\t\tif (link == sinkLink)\n>  \t\t\t\t\tcontinue;\n>\n>  \t\t\t\tif ((link->flags() & MEDIA_LNK_FL_ENABLED) &&\n> @@ -598,11 +606,13 @@ int SimpleCameraData::setupLinks()\n>  \t\t\t}\n>  \t\t}\n>\n> -\t\tif (!(e.sourceLink->flags() & MEDIA_LNK_FL_ENABLED)) {\n> -\t\t\tret = e.sourceLink->setEnabled(true);\n> +\t\tif (!(sinkLink->flags() & MEDIA_LNK_FL_ENABLED)) {\n> +\t\t\tret = sinkLink->setEnabled(true);\n>  \t\t\tif (ret < 0)\n>  \t\t\t\treturn ret;\n>  \t\t}\n> +\n> +\t\tsinkLink = e.sourceLink;\n>  \t}\n>\n>  \treturn 0;\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 C99BCC3275\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  1 Aug 2022 15:54:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4AC4F6330F;\n\tMon,  1 Aug 2022 17:54:17 +0200 (CEST)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E2D5C603E8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Aug 2022 17:54:15 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id DFBB520005;\n\tMon,  1 Aug 2022 15:54:14 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659369257;\n\tbh=LCC6lJ5xXG7ViENoJYDjXxeA8oJGkZXs6x8EVXvEg2Y=;\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=NKRNYxVoZChG0SDDsjNMyKs3RxopEpVPeQbsw9c88brkbfo3OOA3UOCkbMyk4Sh41\n\tJ7HDQaYHUr3rDB+nuh/IG2PewHO4c2qhoUllagUPP80wtOXl7WFNuY9Q07ZsWYArIu\n\t2mOUPtauoqGh7vpjW/JI8wsZ31Qm6DzHzNkNiY05PJDZk8USM6aauUcRckLT2WJ8gR\n\tJHU0bISBqnVGf4FmBObwFm23HN7+c7Ed4hk2D0jLTjjtgjjlSu5o0/AJkVia/mWXxW\n\t9EYZ1nExnmtAZmPcsuJejKu+9x0WbMQv7A/QsZ0NYIzdK/TaitH7aY49ne7VWgb9DW\n\to9wEcRR8TCVhA==","Date":"Mon, 1 Aug 2022 17:54:13 +0200","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20220801155413.qrwi2x5bielfrlvs@uno.localdomain>","References":"<20220801000543.3501-1-laurent.pinchart@ideasonboard.com>\n\t<20220801000543.3501-11-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220801000543.3501-11-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 10/13] libcamera: pipeline: simple:\n\tSetup links in the context of sink entities","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24277,"web_url":"https://patchwork.libcamera.org/comment/24277/","msgid":"<YugvzMYARuDsycZB@pendragon.ideasonboard.com>","date":"2022-08-01T19:55:56","subject":"Re: [libcamera-devel] [PATCH 10/13] libcamera: pipeline: simple:\n\tSetup links in the context of sink entities","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 05:54:13PM +0200, Jacopo Mondi wrote:\n> On Mon, Aug 01, 2022 at 03:05:40AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > To setup links the pipeline handler iterates over all entities in the\n> > pipeline and disables all links but the ones we want to enable. Some\n> > entities don't allow multiple sink links to be enabled, so the iteration\n> > needs to be based on sink entities, disabling all their links first and\n> > then enabling the sink link that is part of the pipeline.\n> >\n> > The loop implementation iterates over all Entity instances, and uses\n> > their source link to locate the MediaEntity for the connected sink. The\n> > sink entity is then processed in the context of the source's loop\n> > iteration. This prevents the code from being able to access extra\n> > information about the sink entity, as we only have access to the\n> > MediaEntity, not the Entity.\n> >\n> > To prepare for subdev routing support that will require accessing\n> > additional entity information, refactor the loop to process the sink\n> > entity in the context of its Entity instance.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> I guess that's easier than storing the sink link along with the source\n> one in SimpleCameraData(), just checking if you think there will be\n> more use cases where the sink link will be usefull.\n\nI can't rule that out, but I don't have any such use case in mind.\n\n> > ---\n> >  src/libcamera/pipeline/simple/simple.cpp | 24 +++++++++++++++++-------\n> >  1 file changed, 17 insertions(+), 7 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 731d355efda6..4bde9caa7254 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -578,15 +578,23 @@ int SimpleCameraData::setupLinks()\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> > +\t * The entities_ list stores entities along with their source link. We\n> > +\t * need to process the link in the context of the sink entity, so\n> > +\t * record the source link of the current entity as the sink link of the\n> > +\t * next entity, and skip the first entity in the loop.\n> >  \t */\n> > +\tMediaLink *sinkLink = nullptr;\n> > +\n> >  \tfor (SimpleCameraData::Entity &e : entities_) {\n> > -\t\tif (!e.sourceLink)\n> > -\t\t\tbreak;\n> > +\t\tif (!sinkLink) {\n> > +\t\t\tsinkLink = e.sourceLink;\n> > +\t\t\tcontinue;\n> > +\t\t}\n> \n> Do we assume to start from the sensor (which has no sink pads to\n> disable links on) ?\n> \n> If that's the case, for my understanding, how is entities_ ordered ?\n> It is populated at SimpleCameraData construction time, by iterating\n> over \"parents\" which is an unordered map.\n> \n> Is there a risk we actually start from a different entity and not the\n> sensor here ?\n\nWe indeed start from the sensor. The entities_ list is built in\nSimpleCameraData::SimpleCameraData(). The first large while () loop\nperforms a breath-first search starting from the camera sensor, as\npassed to the constructor. At each step, how an entity was reached is\nrecorded in the parents unordered map, whose key is the sink and value\nis the source (a.k.a. parent). The loop stops when it reaches a video\ncapture device.\n\nThen, we create the entities_ list by first adding the video device, and\nthen walking from sink to source using the parents map and pushing each\nentity in turn at the *front* of the list. Note that we don't iterate\nover the parents map, we use it to retrieve the parent (a.k.a. source)\nfor a given sink. The list is thus guaranteed to start with the sensor\nand be ordered from source to sink, ending with the video device.\n\n> >\n> > -\t\tMediaEntity *remote = e.sourceLink->sink()->entity();\n> > -\t\tfor (MediaPad *pad : remote->pads()) {\n> > +\t\tfor (MediaPad *pad : e.entity->pads()) {\n> >  \t\t\tfor (MediaLink *link : pad->links()) {\n> > -\t\t\t\tif (link == e.sourceLink)\n> > +\t\t\t\tif (link == sinkLink)\n> >  \t\t\t\t\tcontinue;\n> >\n> >  \t\t\t\tif ((link->flags() & MEDIA_LNK_FL_ENABLED) &&\n> > @@ -598,11 +606,13 @@ int SimpleCameraData::setupLinks()\n> >  \t\t\t}\n> >  \t\t}\n> >\n> > -\t\tif (!(e.sourceLink->flags() & MEDIA_LNK_FL_ENABLED)) {\n> > -\t\t\tret = e.sourceLink->setEnabled(true);\n> > +\t\tif (!(sinkLink->flags() & MEDIA_LNK_FL_ENABLED)) {\n> > +\t\t\tret = sinkLink->setEnabled(true);\n> >  \t\t\tif (ret < 0)\n> >  \t\t\t\treturn ret;\n> >  \t\t}\n> > +\n> > +\t\tsinkLink = e.sourceLink;\n> >  \t}\n> >\n> >  \treturn 0;","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 AE94EC3275\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  1 Aug 2022 19:56:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 042B863312;\n\tMon,  1 Aug 2022 21:56:02 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2E0E2603E8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Aug 2022 21:56:01 +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 8FF202F3;\n\tMon,  1 Aug 2022 21:56:00 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659383762;\n\tbh=+lm4+qVEZrRKoJKbi6S6p4o34Hk9MlcR3kqIUwurmFk=;\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=XHev8DTagtegCrw0zu0x3VcsKGrOZkN56AQynyX+MBNF+yQwvO43WcbceyAlOdG+R\n\t+Jbu9xHpjf6qLNAombEi/nCq5pA3CldpKiKBwArhNRko02mFjXkVoz5lbe1vLO0LnC\n\tlnauU6HPziuL4sofQWEh34KsXjVi9R1XdpYadfUq+n5J9xtU9QS3H9KpjihhFP2wZI\n\tUlhtc1MFqE157qLWF4nPPTNQyKN7mCOhilh2Qf1bx8UpjNKLM/Q0CKnxs2ax8ezc0P\n\t22twkXM5uLG4WXHB/ek1UwLSQCgTf+2ZRGNf1DcbemopILM1NXI8sQFEkVpH0fpOiK\n\tc0dlLTenbcj6g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659383760;\n\tbh=+lm4+qVEZrRKoJKbi6S6p4o34Hk9MlcR3kqIUwurmFk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pHgnUOxENEv8vurYEEcHaV7PUqJD42+0/Pmh+HZHM+aC8kTVDjioGmSeNwvdQqD9N\n\tQGRO7GtVp3s0qxi23K3rIi+VrGIF5BsgYBm6rv8ua1UzALg2SYx5W5wcNfZtslxGXM\n\to/4auVJxwRpacpqj/lzckzrWqUYSQ4FAiq+QjoS0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"pHgnUOxE\"; dkim-atps=neutral","Date":"Mon, 1 Aug 2022 22:55:56 +0300","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<YugvzMYARuDsycZB@pendragon.ideasonboard.com>","References":"<20220801000543.3501-1-laurent.pinchart@ideasonboard.com>\n\t<20220801000543.3501-11-laurent.pinchart@ideasonboard.com>\n\t<20220801155413.qrwi2x5bielfrlvs@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220801155413.qrwi2x5bielfrlvs@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 10/13] libcamera: pipeline: simple:\n\tSetup links in the context of sink entities","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24286,"web_url":"https://patchwork.libcamera.org/comment/24286/","msgid":"<20220802074520.iisffxzzklfd4nqa@uno.localdomain>","date":"2022-08-02T07:45:20","subject":"Re: [libcamera-devel] [PATCH 10/13] libcamera: pipeline: simple:\n\tSetup links in the context of sink entities","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"On Mon, Aug 01, 2022 at 10:55:56PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Mon, Aug 01, 2022 at 05:54:13PM +0200, Jacopo Mondi wrote:\n> > On Mon, Aug 01, 2022 at 03:05:40AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > To setup links the pipeline handler iterates over all entities in the\n> > > pipeline and disables all links but the ones we want to enable. Some\n> > > entities don't allow multiple sink links to be enabled, so the iteration\n> > > needs to be based on sink entities, disabling all their links first and\n> > > then enabling the sink link that is part of the pipeline.\n> > >\n> > > The loop implementation iterates over all Entity instances, and uses\n> > > their source link to locate the MediaEntity for the connected sink. The\n> > > sink entity is then processed in the context of the source's loop\n> > > iteration. This prevents the code from being able to access extra\n> > > information about the sink entity, as we only have access to the\n> > > MediaEntity, not the Entity.\n> > >\n> > > To prepare for subdev routing support that will require accessing\n> > > additional entity information, refactor the loop to process the sink\n> > > entity in the context of its Entity instance.\n> > >\n> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> >\n> > I guess that's easier than storing the sink link along with the source\n> > one in SimpleCameraData(), just checking if you think there will be\n> > more use cases where the sink link will be usefull.\n>\n> I can't rule that out, but I don't have any such use case in mind.\n>\n> > > ---\n> > >  src/libcamera/pipeline/simple/simple.cpp | 24 +++++++++++++++++-------\n> > >  1 file changed, 17 insertions(+), 7 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > > index 731d355efda6..4bde9caa7254 100644\n> > > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > > @@ -578,15 +578,23 @@ int SimpleCameraData::setupLinks()\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> > > +\t * The entities_ list stores entities along with their source link. We\n> > > +\t * need to process the link in the context of the sink entity, so\n> > > +\t * record the source link of the current entity as the sink link of the\n> > > +\t * next entity, and skip the first entity in the loop.\n> > >  \t */\n> > > +\tMediaLink *sinkLink = nullptr;\n> > > +\n> > >  \tfor (SimpleCameraData::Entity &e : entities_) {\n> > > -\t\tif (!e.sourceLink)\n> > > -\t\t\tbreak;\n> > > +\t\tif (!sinkLink) {\n> > > +\t\t\tsinkLink = e.sourceLink;\n> > > +\t\t\tcontinue;\n> > > +\t\t}\n> >\n> > Do we assume to start from the sensor (which has no sink pads to\n> > disable links on) ?\n> >\n> > If that's the case, for my understanding, how is entities_ ordered ?\n> > It is populated at SimpleCameraData construction time, by iterating\n> > over \"parents\" which is an unordered map.\n> >\n> > Is there a risk we actually start from a different entity and not the\n> > sensor here ?\n>\n> We indeed start from the sensor. The entities_ list is built in\n> SimpleCameraData::SimpleCameraData(). The first large while () loop\n> performs a breath-first search starting from the camera sensor, as\n> passed to the constructor. At each step, how an entity was reached is\n> recorded in the parents unordered map, whose key is the sink and value\n> is the source (a.k.a. parent). The loop stops when it reaches a video\n> capture device.\n>\n\nSo far so good\n\n> Then, we create the entities_ list by first adding the video device, and\n> then walking from sink to source using the parents map and pushing each\n> entity in turn at the *front* of the list. Note that we don't iterate\n> over the parents map, we use it to retrieve the parent (a.k.a. source)\n> for a given sink. The list is thus guaranteed to start with the sensor\n> and be ordered from source to sink, ending with the video device.\n\nI missed the fact we're not iterating, but -searching- on the map.\n\nThat solves it, thanks\n\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\n>\n> > >\n> > > -\t\tMediaEntity *remote = e.sourceLink->sink()->entity();\n> > > -\t\tfor (MediaPad *pad : remote->pads()) {\n> > > +\t\tfor (MediaPad *pad : e.entity->pads()) {\n> > >  \t\t\tfor (MediaLink *link : pad->links()) {\n> > > -\t\t\t\tif (link == e.sourceLink)\n> > > +\t\t\t\tif (link == sinkLink)\n> > >  \t\t\t\t\tcontinue;\n> > >\n> > >  \t\t\t\tif ((link->flags() & MEDIA_LNK_FL_ENABLED) &&\n> > > @@ -598,11 +606,13 @@ int SimpleCameraData::setupLinks()\n> > >  \t\t\t}\n> > >  \t\t}\n> > >\n> > > -\t\tif (!(e.sourceLink->flags() & MEDIA_LNK_FL_ENABLED)) {\n> > > -\t\t\tret = e.sourceLink->setEnabled(true);\n> > > +\t\tif (!(sinkLink->flags() & MEDIA_LNK_FL_ENABLED)) {\n> > > +\t\t\tret = sinkLink->setEnabled(true);\n> > >  \t\t\tif (ret < 0)\n> > >  \t\t\t\treturn ret;\n> > >  \t\t}\n> > > +\n> > > +\t\tsinkLink = e.sourceLink;\n> > >  \t}\n> > >\n> > >  \treturn 0;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 33CEFBE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  2 Aug 2022 07:45:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 94E8F6330F;\n\tTue,  2 Aug 2022 09:45:24 +0200 (CEST)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9D5AC6330D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Aug 2022 09:45:23 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id B507E1C0003;\n\tTue,  2 Aug 2022 07:45:22 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659426324;\n\tbh=k4GOn5ZevoWfdZTIazcNbh8aAAJwcdSWaTRgMpa/4sY=;\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=WLMcFNAL1aoMaCqeQiA0dRay3/k24P7kk8tQd+TKDxCAmem1rMRDv1MmG65TKbESP\n\tZ+4qG33A7R3MIcvo7nUtBOyxThRhFRa/sgiNPAoQGgGJOeA2givt6eHEwMlR8WpHf1\n\tjWVMJBlJJSAG4DwPlBle+8oYNsdjMTGI7CN7o0hkFuuHElr0MqGgFG+VL75jqK90NN\n\tGDzFrTafV2WBwWBHr6mNLLY8/VGlFat8yJNnIaKlmo3X8BjdoilqGXmzu6x+RdE2m2\n\t6YZgxQL6UMp84SQgk0ipERoSOB1DzHrQH8Wu7QW26MLxsQGotaPeIIOpxhcSa7A1if\n\tPXOgh49PCoRAg==","Date":"Tue, 2 Aug 2022 09:45:20 +0200","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20220802074520.iisffxzzklfd4nqa@uno.localdomain>","References":"<20220801000543.3501-1-laurent.pinchart@ideasonboard.com>\n\t<20220801000543.3501-11-laurent.pinchart@ideasonboard.com>\n\t<20220801155413.qrwi2x5bielfrlvs@uno.localdomain>\n\t<YugvzMYARuDsycZB@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<YugvzMYARuDsycZB@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 10/13] libcamera: pipeline: simple:\n\tSetup links in the context of sink entities","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]