[{"id":15328,"web_url":"https://patchwork.libcamera.org/comment/15328/","msgid":"","date":"2021-02-28T15:10:38","subject":"Re: [libcamera-devel] [PATCH v5 1/2] pipeline: rkisp1: Share the\n\tISP subdevice","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Sebastian,\n\nThank you for the patch.\n\nOn Sat, Feb 27, 2021 at 07:01:25PM +0100, Sebastian Fricke wrote:\n> Share the ISP subdevice between the RkISP1CameraData and the\n> PipelineHandlerRkISP1 class. This enables other classes like\n> RkISP1CameraConfiguration to get access to the device.\n\nWhile the code should work, I don't think it convey the right meaning.\n\nstd::shared_ptr<> and std::unique_ptr<> are used to denote ownership\nrules. The former is used for objects that have multiple owners, while\nthe later is for single-owner objects. See\nDocumentation/coding-style.rst for more information.\n\nThere's one ISP at the hardware level (well, there are actually two\ninstances, but that's a separate question, the rkisp1 pipeline handler\nonly uses once instance). There should thus be a single instance of the\ncorresponding V4L2Subdevice, and that instance should belong to the\nPipelineHandlerRkISP1 class. Its ownership doesn't need to be shared\nbetween multiple classes.\n\nThis doesn't preclude storing references to the ISP in other classes.\nThose are called borrowed references. They require the code to carefully\nensure that the object will not be destroyed before all borrowed\nreferences are released.\n\n> Signed-off-by: Sebastian Fricke \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 ++++-\n> 1 file changed, 4 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 538c0139..50eaa6a4 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -90,6 +90,7 @@ public:\n> \tStream mainPathStream_;\n> \tStream selfPathStream_;\n> \tstd::unique_ptr sensor_;\n> +\tstd::shared_ptr isp_;\n\n\tV4L2Subdevice *isp_;\n\n> \tstd::unique_ptr delayedCtrls_;\n> \tunsigned int frame_;\n> \tstd::vector ipaBuffers_;\n> @@ -172,7 +173,7 @@ private:\n> \tint freeBuffers(Camera *camera);\n> \n> \tMediaDevice *media_;\n> -\tstd::unique_ptr isp_;\n> +\tstd::shared_ptr isp_;\n> \tstd::unique_ptr param_;\n> \tstd::unique_ptr stat_;\n> \n> @@ -930,6 +931,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\tdata->isp_ = isp_;\n> +\n\n\tdata->isp_ = isp_.get();\n\nAs RkISP1CameraData will be destroyed before PipelineHandlerRkISP1, this\nwill not cause any issue.\n\n> \t/* Initialize the camera properties. */\n> \tdata->properties_ = data->sensor_->properties();\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 6AEC5BD1F1\n\tfor ;\n\tSun, 28 Feb 2021 15:11:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 45B6168A6D;\n\tSun, 28 Feb 2021 16:11:10 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 81D6068A45\n\tfor ;\n\tSun, 28 Feb 2021 16:11:08 +0100 (CET)","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 E95C180F;\n\tSun, 28 Feb 2021 16:11:07 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"SuHvMDPw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1614525068;\n\tbh=GAHj3l2YGwugKGnnruV5OB/AvMZMxKum3gJ7TPjuLNA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SuHvMDPw6rF+TX5be2u6V9Ag05blKnNM4JYrKmI2Ax++D89eJY/MsbCHvpghcWa31\n\tsbBX7MZPHbJPhMoRIb33aCn2tI+tepVqYh9dHX8v/EktupRBoAPpdIX/beytlwSk7F\n\txt+DOqG4+NjQeON/HQQjrB9Y3LrwR7Gh9aF9Stw8=","Date":"Sun, 28 Feb 2021 17:10:38 +0200","From":"Laurent Pinchart ","To":"Sebastian Fricke ","Message-ID":"","References":"<20210227180126.37591-1-sebastian.fricke@posteo.net>\n\t<20210227180126.37591-2-sebastian.fricke@posteo.net>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210227180126.37591-2-sebastian.fricke@posteo.net>","Subject":"Re: [libcamera-devel] [PATCH v5 1/2] pipeline: rkisp1: Share the\n\tISP subdevice","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15342,"web_url":"https://patchwork.libcamera.org/comment/15342/","msgid":"<5f700743-9dea-22ba-9446-b733a64b6a90@collabora.com>","date":"2021-03-01T07:13:13","subject":"Re: [libcamera-devel] [PATCH v5 1/2] pipeline: rkisp1: Share the\n\tISP subdevice","submitter":{"id":46,"url":"https://patchwork.libcamera.org/api/people/46/","name":"Dafna Hirschfeld","email":"dafna.hirschfeld@collabora.com"},"content":"Hi,\n\nOn 28.02.21 16:10, Laurent Pinchart wrote:\n> Hi Sebastian,\n> \n> Thank you for the patch.\n> \n> On Sat, Feb 27, 2021 at 07:01:25PM +0100, Sebastian Fricke wrote:\n>> Share the ISP subdevice between the RkISP1CameraData and the\n>> PipelineHandlerRkISP1 class. This enables other classes like\n>> RkISP1CameraConfiguration to get access to the device.\n> \n> While the code should work, I don't think it convey the right meaning.\n> \n> std::shared_ptr<> and std::unique_ptr<> are used to denote ownership\n> rules. The former is used for objects that have multiple owners, while\n> the later is for single-owner objects. See\n> Documentation/coding-style.rst for more information.\n> \n> There's one ISP at the hardware level (well, there are actually two\n> instances, but that's a separate question, the rkisp1 pipeline handler\n> only uses once instance). There should thus be a single instance of the\n> corresponding V4L2Subdevice, and that instance should belong to the\n> PipelineHandlerRkISP1 class. Its ownership doesn't need to be shared\n> between multiple classes.\n\nI see that in other pipeline-handlers, the topology entities are all members of\nCameraData. In rkisp1, the sensor, mainpath and selfpath are members of RkISP1CameraData\nwhile the isp, params, stats, are members of PipelineHandlerRkISP1.\nI can't see why it is done like that.\nIt seems that for consistency with other pipline-handler we can move\nall entities to be unique_pointers in RkISP1CameraData\n\nThanks,\nDafna\n\n> \n> This doesn't preclude storing references to the ISP in other classes.\n> Those are called borrowed references. They require the code to carefully\n> ensure that the object will not be destroyed before all borrowed\n> references are released.\n> \n>> Signed-off-by: Sebastian Fricke \n>> ---\n>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 ++++-\n>> 1 file changed, 4 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> index 538c0139..50eaa6a4 100644\n>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> @@ -90,6 +90,7 @@ public:\n>> \tStream mainPathStream_;\n>> \tStream selfPathStream_;\n>> \tstd::unique_ptr sensor_;\n>> +\tstd::shared_ptr isp_;\n> \n> \tV4L2Subdevice *isp_;\n> \n>> \tstd::unique_ptr delayedCtrls_;\n>> \tunsigned int frame_;\n>> \tstd::vector ipaBuffers_;\n>> @@ -172,7 +173,7 @@ private:\n>> \tint freeBuffers(Camera *camera);\n>> \n>> \tMediaDevice *media_;\n>> -\tstd::unique_ptr isp_;\n>> +\tstd::shared_ptr isp_;\n>> \tstd::unique_ptr param_;\n>> \tstd::unique_ptr stat_;\n>> \n>> @@ -930,6 +931,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n>> \tif (ret)\n>> \t\treturn ret;\n>> \n>> +\tdata->isp_ = isp_;\n>> +\n> \n> \tdata->isp_ = isp_.get();\n> \n> As RkISP1CameraData will be destroyed before PipelineHandlerRkISP1, this\n> will not cause any issue.\n> \n>> \t/* Initialize the camera properties. */\n>> \tdata->properties_ = data->sensor_->properties();\n>> \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 CE1C0BD1F1\n\tfor ;\n\tMon, 1 Mar 2021 07:13:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 68D9B689DD;\n\tMon, 1 Mar 2021 08:13:18 +0100 (CET)","from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 08FAA602EC\n\tfor ;\n\tMon, 1 Mar 2021 08:13:17 +0100 (CET)","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: dafna) with ESMTPSA id 6A4A21F44F60"],"To":"Laurent Pinchart ,\n\tSebastian Fricke ","References":"<20210227180126.37591-1-sebastian.fricke@posteo.net>\n\t<20210227180126.37591-2-sebastian.fricke@posteo.net>\n\t","From":"Dafna Hirschfeld ","Message-ID":"<5f700743-9dea-22ba-9446-b733a64b6a90@collabora.com>","Date":"Mon, 1 Mar 2021 08:13:13 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v5 1/2] pipeline: rkisp1: Share the\n\tISP subdevice","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","Cc":"libcamera-devel@lists.libcamera.org,\n\tCollabora Kernel ML ","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15388,"web_url":"https://patchwork.libcamera.org/comment/15388/","msgid":"","date":"2021-03-02T02:35:17","subject":"Re: [libcamera-devel] [PATCH v5 1/2] pipeline: rkisp1: Share the\n\tISP subdevice","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Dafna,\n\nOn Mon, Mar 01, 2021 at 08:13:13AM +0100, Dafna Hirschfeld wrote:\n> On 28.02.21 16:10, Laurent Pinchart wrote:\n> > On Sat, Feb 27, 2021 at 07:01:25PM +0100, Sebastian Fricke wrote:\n> >> Share the ISP subdevice between the RkISP1CameraData and the\n> >> PipelineHandlerRkISP1 class. This enables other classes like\n> >> RkISP1CameraConfiguration to get access to the device.\n> > \n> > While the code should work, I don't think it convey the right meaning.\n> > \n> > std::shared_ptr<> and std::unique_ptr<> are used to denote ownership\n> > rules. The former is used for objects that have multiple owners, while\n> > the later is for single-owner objects. See\n> > Documentation/coding-style.rst for more information.\n> > \n> > There's one ISP at the hardware level (well, there are actually two\n> > instances, but that's a separate question, the rkisp1 pipeline handler\n> > only uses once instance). There should thus be a single instance of the\n> > corresponding V4L2Subdevice, and that instance should belong to the\n> > PipelineHandlerRkISP1 class. Its ownership doesn't need to be shared\n> > between multiple classes.\n> \n> I see that in other pipeline-handlers, the topology entities are all members of\n> CameraData. In rkisp1, the sensor, mainpath and selfpath are members of RkISP1CameraData\n> while the isp, params, stats, are members of PipelineHandlerRkISP1.\n> I can't see why it is done like that.\n\nThere's no hard rule here. What we usually do is that resources that are\nshared by all cameras are stored in the pipeline handler, while\nresources specific to one camera are stored in the camera data. Another\noption is to store all resources in the pipeline handler, and store\npointers to the camera-specific resources in each camera data instance.\nBy storing resources I mean for example an instance of a V4L2Subdevice,\nusually stored in a unique_ptr, while pointers to resources are normal\npointers.\n\n> It seems that for consistency with other pipline-handler we can move\n> all entities to be unique_pointers in RkISP1CameraData\n\nThere's a single ISP instance, so there should be a single corresponding\nV4L2Subdevice instance, and it thus has to be stored in the pipeline\nhandler. The camera data instances can point to it for convenience.\n\n> > This doesn't preclude storing references to the ISP in other classes.\n> > Those are called borrowed references. They require the code to carefully\n> > ensure that the object will not be destroyed before all borrowed\n> > references are released.\n> > \n> >> Signed-off-by: Sebastian Fricke \n> >> ---\n> >> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 ++++-\n> >> 1 file changed, 4 insertions(+), 1 deletion(-)\n> >>\n> >> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> >> index 538c0139..50eaa6a4 100644\n> >> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> >> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> >> @@ -90,6 +90,7 @@ public:\n> >> \tStream mainPathStream_;\n> >> \tStream selfPathStream_;\n> >> \tstd::unique_ptr sensor_;\n> >> +\tstd::shared_ptr isp_;\n> > \n> > \tV4L2Subdevice *isp_;\n> > \n> >> \tstd::unique_ptr delayedCtrls_;\n> >> \tunsigned int frame_;\n> >> \tstd::vector ipaBuffers_;\n> >> @@ -172,7 +173,7 @@ private:\n> >> \tint freeBuffers(Camera *camera);\n> >> \n> >> \tMediaDevice *media_;\n> >> -\tstd::unique_ptr isp_;\n> >> +\tstd::shared_ptr isp_;\n> >> \tstd::unique_ptr param_;\n> >> \tstd::unique_ptr stat_;\n> >> \n> >> @@ -930,6 +931,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> >> \tif (ret)\n> >> \t\treturn ret;\n> >> \n> >> +\tdata->isp_ = isp_;\n> >> +\n> > \n> > \tdata->isp_ = isp_.get();\n> > \n> > As RkISP1CameraData will be destroyed before PipelineHandlerRkISP1, this\n> > will not cause any issue.\n> > \n> >> \t/* Initialize the camera properties. */\n> >> \tdata->properties_ = data->sensor_->properties();\n> >> \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 BACB0BD1F1\n\tfor ;\n\tTue, 2 Mar 2021 02:35:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4A84668A92;\n\tTue, 2 Mar 2021 03:35:48 +0100 (CET)","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 7129660522\n\tfor ;\n\tTue, 2 Mar 2021 03:35:46 +0100 (CET)","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 DE98645D;\n\tTue, 2 Mar 2021 03:35:45 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"W2pqGL1b\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1614652546;\n\tbh=GMy07ClRA1V8gDQQYnwZHfnlK/AZASCBw7MEqM0msts=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=W2pqGL1bdX8XXYuei55oKviyW58MeTHYpb3+eGqsUxUDeUApBnzrKkKtbx8+UfLfd\n\tbmCTrrAD+GbV9bz5RBoGE5BkLWx2iVq/tiIA6joG/4ukHw6imHf5TTT1lBSZK+sTkj\n\to672zeCG21liqh5BTZgimBGCv5yMF39aghISMUd4=","Date":"Tue, 2 Mar 2021 04:35:17 +0200","From":"Laurent Pinchart ","To":"Dafna Hirschfeld ","Message-ID":"","References":"<20210227180126.37591-1-sebastian.fricke@posteo.net>\n\t<20210227180126.37591-2-sebastian.fricke@posteo.net>\n\t\n\t<5f700743-9dea-22ba-9446-b733a64b6a90@collabora.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<5f700743-9dea-22ba-9446-b733a64b6a90@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH v5 1/2] pipeline: rkisp1: Share the\n\tISP subdevice","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","Cc":"libcamera-devel@lists.libcamera.org,\n\tCollabora Kernel ML ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15395,"web_url":"https://patchwork.libcamera.org/comment/15395/","msgid":"<22d07a63-ea82-e616-cb4f-bfc482a5a2ed@collabora.com>","date":"2021-03-02T07:12:16","subject":"Re: [libcamera-devel] [PATCH v5 1/2] pipeline: rkisp1: Share the\n\tISP subdevice","submitter":{"id":46,"url":"https://patchwork.libcamera.org/api/people/46/","name":"Dafna Hirschfeld","email":"dafna.hirschfeld@collabora.com"},"content":"On 02.03.21 03:35, Laurent Pinchart wrote:\n> Hi Dafna,\n> \n> On Mon, Mar 01, 2021 at 08:13:13AM +0100, Dafna Hirschfeld wrote:\n>> On 28.02.21 16:10, Laurent Pinchart wrote:\n>>> On Sat, Feb 27, 2021 at 07:01:25PM +0100, Sebastian Fricke wrote:\n>>>> Share the ISP subdevice between the RkISP1CameraData and the\n>>>> PipelineHandlerRkISP1 class. This enables other classes like\n>>>> RkISP1CameraConfiguration to get access to the device.\n>>>\n>>> While the code should work, I don't think it convey the right meaning.\n>>>\n>>> std::shared_ptr<> and std::unique_ptr<> are used to denote ownership\n>>> rules. The former is used for objects that have multiple owners, while\n>>> the later is for single-owner objects. See\n>>> Documentation/coding-style.rst for more information.\n>>>\n>>> There's one ISP at the hardware level (well, there are actually two\n>>> instances, but that's a separate question, the rkisp1 pipeline handler\n>>> only uses once instance). There should thus be a single instance of the\n>>> corresponding V4L2Subdevice, and that instance should belong to the\n>>> PipelineHandlerRkISP1 class. Its ownership doesn't need to be shared\n>>> between multiple classes.\n>>\n>> I see that in other pipeline-handlers, the topology entities are all members of\n>> CameraData. In rkisp1, the sensor, mainpath and selfpath are members of RkISP1CameraData\n>> while the isp, params, stats, are members of PipelineHandlerRkISP1.\n>> I can't see why it is done like that.\n> \n> There's no hard rule here. What we usually do is that resources that are\n> shared by all cameras are stored in the pipeline handler, while\n> resources specific to one camera are stored in the camera data. Another\n> option is to store all resources in the pipeline handler, and store\n> pointers to the camera-specific resources in each camera data instance.\n> By storing resources I mean for example an instance of a V4L2Subdevice,\n> usually stored in a unique_ptr, while pointers to resources are normal\n> pointers.\n> \n>> It seems that for consistency with other pipline-handler we can move\n>> all entities to be unique_pointers in RkISP1CameraData\n> \n> There's a single ISP instance, so there should be a single corresponding\n> V4L2Subdevice instance, and it thus has to be stored in the pipeline\n> handler. The camera data instances can point to it for convenience.\n\nok, I see it now, thanks for clarifying.\n\nDafna\n\n> \n>>> This doesn't preclude storing references to the ISP in other classes.\n>>> Those are called borrowed references. They require the code to carefully\n>>> ensure that the object will not be destroyed before all borrowed\n>>> references are released.\n>>>\n>>>> Signed-off-by: Sebastian Fricke \n>>>> ---\n>>>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 ++++-\n>>>> 1 file changed, 4 insertions(+), 1 deletion(-)\n>>>>\n>>>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>>> index 538c0139..50eaa6a4 100644\n>>>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>>> @@ -90,6 +90,7 @@ public:\n>>>> \tStream mainPathStream_;\n>>>> \tStream selfPathStream_;\n>>>> \tstd::unique_ptr sensor_;\n>>>> +\tstd::shared_ptr isp_;\n>>>\n>>> \tV4L2Subdevice *isp_;\n>>>\n>>>> \tstd::unique_ptr delayedCtrls_;\n>>>> \tunsigned int frame_;\n>>>> \tstd::vector ipaBuffers_;\n>>>> @@ -172,7 +173,7 @@ private:\n>>>> \tint freeBuffers(Camera *camera);\n>>>> \n>>>> \tMediaDevice *media_;\n>>>> -\tstd::unique_ptr isp_;\n>>>> +\tstd::shared_ptr isp_;\n>>>> \tstd::unique_ptr param_;\n>>>> \tstd::unique_ptr stat_;\n>>>> \n>>>> @@ -930,6 +931,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n>>>> \tif (ret)\n>>>> \t\treturn ret;\n>>>> \n>>>> +\tdata->isp_ = isp_;\n>>>> +\n>>>\n>>> \tdata->isp_ = isp_.get();\n>>>\n>>> As RkISP1CameraData will be destroyed before PipelineHandlerRkISP1, this\n>>> will not cause any issue.\n>>>\n>>>> \t/* Initialize the camera properties. */\n>>>> \tdata->properties_ = data->sensor_->properties();\n>>>> \n>>>\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 EC584BD808\n\tfor ;\n\tTue, 2 Mar 2021 07:12:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8C03F68A8D;\n\tTue, 2 Mar 2021 08:12:21 +0100 (CET)","from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1027760522\n\tfor ;\n\tTue, 2 Mar 2021 08:12:20 +0100 (CET)","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: dafna) with ESMTPSA id A2D191F4538B"],"To":"Laurent Pinchart ","References":"<20210227180126.37591-1-sebastian.fricke@posteo.net>\n\t<20210227180126.37591-2-sebastian.fricke@posteo.net>\n\t\n\t<5f700743-9dea-22ba-9446-b733a64b6a90@collabora.com>\n\t","From":"Dafna Hirschfeld ","Message-ID":"<22d07a63-ea82-e616-cb4f-bfc482a5a2ed@collabora.com>","Date":"Tue, 2 Mar 2021 08:12:16 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v5 1/2] pipeline: rkisp1: Share the\n\tISP subdevice","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","Cc":"libcamera-devel@lists.libcamera.org,\n\tCollabora Kernel ML ","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]