[{"id":1759,"web_url":"https://patchwork.libcamera.org/comment/1759/","msgid":"<20190604120744.GA7812@pendragon.ideasonboard.com>","date":"2019-06-04T12:07:44","subject":"Re: [libcamera-devel] [PATCH v2 09/10] libcamera: pipeline: store\n\tIPA in pipeline data","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Mon, Jun 03, 2019 at 07:16:36PM -0400, Paul Elder wrote:\n> After the pipeline handler acquires an IPA, it should save it for future\n> use. Store it in the pipeline data.\n> \n> Signed-off-by: Paul Elder \n> ---\n> New patch\n> \n> src/libcamera/include/pipeline_handler.h | 3 +++\n> src/libcamera/pipeline_handler.cpp | 8 ++++++++\n> 2 files changed, 11 insertions(+)\n> \n> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> index 67971b3..a2d9624 100644\n> --- a/src/libcamera/include/pipeline_handler.h\n> +++ b/src/libcamera/include/pipeline_handler.h\n> @@ -14,6 +14,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> \n> namespace libcamera {\n> @@ -93,6 +94,8 @@ protected:\n> \tconst char *name_;\n> \tuint32_t version_;\n> \n> +\tstd::unique_ptr ipa_;\n> +\n> private:\n> \tvoid mediaDeviceDisconnected(MediaDevice *media);\n> \tvirtual void disconnect();\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index b18f14d..d6e68b0 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -530,6 +530,14 @@ CameraData *PipelineHandler::cameraData(const Camera *camera)\n> * \\brief Pipeline handler version\n> */\n> \n> +/**\n> + * \\var PipelineHandler::ipa_\n> + * \\brief The IPA that the pipeline handler will use\n\n\"The IPA interface instance for this pipeline handler\" ?\n\nAnd thinking about it, will we always have a 1:1 mapping between\npipeline handlers and IPA instances ? In the IPU3 case, if we support\ncapturing from two cameras simultaneously, I think we'll have one IPA\ninstance per camera, while both cameras will be handled by the same\npipeline handler. We could store the IPA instance in the camera data\ninstead. Or, for now, until we figure the details out, store it in the\nvimc pipeline handler in patch 10/10 (thus dropping this patch). What do\nyou think ?\n\n> + *\n> + * The pipeline handler can acquire an IPA from the IPAManager. Once it is\n> + * acquired, it will be stored here.\n> + */\n> +\n> /**\n> * \\class PipelineHandlerFactory\n> * \\brief Registration of PipelineHandler classes and creation of instances","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3D4F3630FA\n\tfor ;\n\tTue, 4 Jun 2019 14:07:58 +0200 (CEST)","from pendragon.ideasonboard.com (unknown\n\t[IPv6:2a02:2788:668:163:5bb7:9f6c:564c:d55e])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CFBEA5D;\n\tTue, 4 Jun 2019 14:07:57 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1559650077;\n\tbh=ASznopPYno6DcDKQENb1Y5drzTKJfQ6UXKNu3h8QsA8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ESHiQr+LbbBqVUmbPPXfYi0wUoYd4kTUKu6XlLX/6gouvvgxFKha0tDJqb6XC3gYD\n\tNn2b6jzBxkPnkKkIReASEn9YMBf5uhiAf8E6dZv6hnptgno+vs19ZPsUYuTxqdBPbA\n\tVstY7cf5ZU4TU2wkEuT6DGlXyTVy+gl1ePc5rPqQ=","Date":"Tue, 4 Jun 2019 15:07:44 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190604120744.GA7812@pendragon.ideasonboard.com>","References":"<20190603231637.28554-1-paul.elder@ideasonboard.com>\n\t<20190603231637.28554-10-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190603231637.28554-10-paul.elder@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 09/10] libcamera: pipeline: store\n\tIPA in pipeline data","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 04 Jun 2019 12:07:58 -0000"}},{"id":1763,"web_url":"https://patchwork.libcamera.org/comment/1763/","msgid":"<20190604162403.GN32191@localhost.localdomain>","date":"2019-06-04T16:24:03","subject":"Re: [libcamera-devel] [PATCH v2 09/10] libcamera: pipeline: store\n\tIPA in pipeline data","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Tue, Jun 04, 2019 at 03:07:44PM +0300, Laurent Pinchart wrote:\n> Hi Paul,\n> \n> Thank you for the patch.\n\nThank you for the review.\n\n> On Mon, Jun 03, 2019 at 07:16:36PM -0400, Paul Elder wrote:\n> > After the pipeline handler acquires an IPA, it should save it for future\n> > use. Store it in the pipeline data.\n> > \n> > Signed-off-by: Paul Elder \n> > ---\n> > New patch\n> > \n> > src/libcamera/include/pipeline_handler.h | 3 +++\n> > src/libcamera/pipeline_handler.cpp | 8 ++++++++\n> > 2 files changed, 11 insertions(+)\n> > \n> > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> > index 67971b3..a2d9624 100644\n> > --- a/src/libcamera/include/pipeline_handler.h\n> > +++ b/src/libcamera/include/pipeline_handler.h\n> > @@ -14,6 +14,7 @@\n> > #include \n> > #include \n> > \n> > +#include \n> > #include \n> > \n> > namespace libcamera {\n> > @@ -93,6 +94,8 @@ protected:\n> > \tconst char *name_;\n> > \tuint32_t version_;\n> > \n> > +\tstd::unique_ptr ipa_;\n> > +\n> > private:\n> > \tvoid mediaDeviceDisconnected(MediaDevice *media);\n> > \tvirtual void disconnect();\n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index b18f14d..d6e68b0 100644\n> > --- a/src/libcamera/pipeline_handler.cpp\n> > +++ b/src/libcamera/pipeline_handler.cpp\n> > @@ -530,6 +530,14 @@ CameraData *PipelineHandler::cameraData(const Camera *camera)\n> > * \\brief Pipeline handler version\n> > */\n> > \n> > +/**\n> > + * \\var PipelineHandler::ipa_\n> > + * \\brief The IPA that the pipeline handler will use\n> \n> \"The IPA interface instance for this pipeline handler\" ?\n> \n> And thinking about it, will we always have a 1:1 mapping between\n> pipeline handlers and IPA instances ? In the IPU3 case, if we support\n\ntbh, I was doubting a 1:1 mapping between pipeline handlers and IPA\ninstances.\n\n> capturing from two cameras simultaneously, I think we'll have one IPA\n> instance per camera, while both cameras will be handled by the same\n> pipeline handler. We could store the IPA instance in the camera data\n> instead. Or, for now, until we figure the details out, store it in the\n> vimc pipeline handler in patch 10/10 (thus dropping this patch). What do\n> you think ?\n\nIf the mapping is 1:1 between cameras and IPA instances then I think we\ncan put the IPA instance in the camera data.\n\n> > + *\n> > + * The pipeline handler can acquire an IPA from the IPAManager. Once it is\n> > + * acquired, it will be stored here.\n> > + */\n> > +\n> > /**\n> > * \\class PipelineHandlerFactory\n> > * \\brief Registration of PipelineHandler classes and creation of instances\n\nThanks,\n\nPaul","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 12C9163916\n\tfor ;\n\tTue, 4 Jun 2019 18:24:11 +0200 (CEST)","from localhost.localdomain (unknown [96.44.9.117])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 55A952D1;\n\tTue, 4 Jun 2019 18:24:10 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1559665450;\n\tbh=WlL4IA91nDr32NPD+kpcHIdPWJZh1cAXH8e9fntRgxU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=DkGZOjII/sHqaBkXnMGo/hQHyndBEhZUAxJzqdk5LWivKICW9gspoxUIXkHee1yOe\n\tfelBNvH+94XPjnAJAp1FYTzqRadg8TBJCC/muBqoyHWdJLtQSlWgLCIlLCIkWssuvA\n\t71K7mJ5VuL8cAgCYIBuHjRMlalLUhcFe+lgmHzp8=","Date":"Tue, 4 Jun 2019 12:24:03 -0400","From":"Paul Elder ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190604162403.GN32191@localhost.localdomain>","References":"<20190603231637.28554-1-paul.elder@ideasonboard.com>\n\t<20190603231637.28554-10-paul.elder@ideasonboard.com>\n\t<20190604120744.GA7812@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20190604120744.GA7812@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 09/10] libcamera: pipeline: store\n\tIPA in pipeline data","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 04 Jun 2019 16:24:11 -0000"}},{"id":1764,"web_url":"https://patchwork.libcamera.org/comment/1764/","msgid":"<20190604162730.GA21733@pendragon.ideasonboard.com>","date":"2019-06-04T16:27:30","subject":"Re: [libcamera-devel] [PATCH v2 09/10] libcamera: pipeline: store\n\tIPA in pipeline data","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Tue, Jun 04, 2019 at 12:24:03PM -0400, Paul Elder wrote:\n> On Tue, Jun 04, 2019 at 03:07:44PM +0300, Laurent Pinchart wrote:\n> > Hi Paul,\n> > \n> > Thank you for the patch.\n> \n> Thank you for the review.\n> \n> > On Mon, Jun 03, 2019 at 07:16:36PM -0400, Paul Elder wrote:\n> >> After the pipeline handler acquires an IPA, it should save it for future\n> >> use. Store it in the pipeline data.\n> >> \n> >> Signed-off-by: Paul Elder \n> >> ---\n> >> New patch\n> >> \n> >> src/libcamera/include/pipeline_handler.h | 3 +++\n> >> src/libcamera/pipeline_handler.cpp | 8 ++++++++\n> >> 2 files changed, 11 insertions(+)\n> >> \n> >> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> >> index 67971b3..a2d9624 100644\n> >> --- a/src/libcamera/include/pipeline_handler.h\n> >> +++ b/src/libcamera/include/pipeline_handler.h\n> >> @@ -14,6 +14,7 @@\n> >> #include \n> >> #include \n> >> \n> >> +#include \n> >> #include \n> >> \n> >> namespace libcamera {\n> >> @@ -93,6 +94,8 @@ protected:\n> >> \tconst char *name_;\n> >> \tuint32_t version_;\n> >> \n> >> +\tstd::unique_ptr ipa_;\n> >> +\n> >> private:\n> >> \tvoid mediaDeviceDisconnected(MediaDevice *media);\n> >> \tvirtual void disconnect();\n> >> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> >> index b18f14d..d6e68b0 100644\n> >> --- a/src/libcamera/pipeline_handler.cpp\n> >> +++ b/src/libcamera/pipeline_handler.cpp\n> >> @@ -530,6 +530,14 @@ CameraData *PipelineHandler::cameraData(const Camera *camera)\n> >> * \\brief Pipeline handler version\n> >> */\n> >> \n> >> +/**\n> >> + * \\var PipelineHandler::ipa_\n> >> + * \\brief The IPA that the pipeline handler will use\n> > \n> > \"The IPA interface instance for this pipeline handler\" ?\n> > \n> > And thinking about it, will we always have a 1:1 mapping between\n> > pipeline handlers and IPA instances ? In the IPU3 case, if we support\n> \n> tbh, I was doubting a 1:1 mapping between pipeline handlers and IPA\n> instances.\n> \n> > capturing from two cameras simultaneously, I think we'll have one IPA\n> > instance per camera, while both cameras will be handled by the same\n> > pipeline handler. We could store the IPA instance in the camera data\n> > instead. Or, for now, until we figure the details out, store it in the\n> > vimc pipeline handler in patch 10/10 (thus dropping this patch). What do\n> > you think ?\n> \n> If the mapping is 1:1 between cameras and IPA instances then I think we\n> can put the IPA instance in the camera data.\n\nI don't think it will be the case either. The rkisp1 can handle two\ncameras, but not at the same time, so they will likely be a single IPA\ninstance (although that's actually debatable). How about not trying to\nmake the code too generic yet and move the ipa_ member to the\nPipelineHandlerVimc class for now ?\n\n> >> + *\n> >> + * The pipeline handler can acquire an IPA from the IPAManager. Once it is\n> >> + * acquired, it will be stored here.\n> >> + */\n> >> +\n> >> /**\n> >> * \\class PipelineHandlerFactory\n> >> * \\brief Registration of PipelineHandler classes and creation of instances","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C2B1863A06\n\tfor ;\n\tTue, 4 Jun 2019 18:27:44 +0200 (CEST)","from pendragon.ideasonboard.com (unknown\n\t[IPv6:2a02:2788:668:163:5bb7:9f6c:564c:d55e])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 534F42D1;\n\tTue, 4 Jun 2019 18:27:44 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1559665664;\n\tbh=d7mMKae98cSOvDzdKetru4QghR7gJze0hokblKq9V04=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TxhMr46aqR3hjY2yMGi06GLAY5922BtNWYW1HTyPYlg3OiBJsMeEG38MPtex6ySyf\n\tz1TkgZQHlDlEwqYJm1KgM3M/taovzKHskB6hOHCWmtZLuDwct/MCL5KB7EGpR83ZYc\n\t6yKsYq4Y6z3jTzmE4qslCgFXiGqP6YOCD7XWxsOk=","Date":"Tue, 4 Jun 2019 19:27:30 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190604162730.GA21733@pendragon.ideasonboard.com>","References":"<20190603231637.28554-1-paul.elder@ideasonboard.com>\n\t<20190603231637.28554-10-paul.elder@ideasonboard.com>\n\t<20190604120744.GA7812@pendragon.ideasonboard.com>\n\t<20190604162403.GN32191@localhost.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190604162403.GN32191@localhost.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 09/10] libcamera: pipeline: store\n\tIPA in pipeline data","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 04 Jun 2019 16:27:45 -0000"}}]