[{"id":508,"web_url":"https://patchwork.libcamera.org/comment/508/","msgid":"<20190123000758.GH4127@bigcity.dyn.berto.se>","date":"2019-01-23T00:07:58","subject":"Re: [libcamera-devel] [RFC 0/2] Add support for pipeline specific\n\tdata to Cameras","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your work.\n\nOn 2019-01-22 19:12:23 +0100, Jacopo Mondi wrote:\n> RFC as I'm not sure about the idea of delegating ownership of platform data\n> to Cameras, as that requires the pipeline handler to dynamically allocate the\n> resources (if they go away at pipeline handler destruction time, is pointless\n> to store them in Camera).\n> \n> The other way around is the idea of borrowing pipeline handler data to Cameras,\n> but as Cameras are shared objects, they might stay around longer that pipeline\n> handlers, and thus I felt it is safer to tie the CameraData lifetime to the one\n> of the Camera instance they're associated to.\n\nWhile working with the Camera API and it's interaction with the pipeline \nhandler I felt a need for something similar you have here. My initial \nidea to avoid lifetime management issues which you deal with in this \nseries was for the PipelineHandler to be able to set a unsigned int as a \ncookie on the Camera at creation time, which it could retrieve with it's \nlater interaction with a Camera object.\n\nIn the end I went a different way, as it became clear we would need an \ntwo way association between the Camera and the PipelineHandler anyhow.  \nThat is the PipelineHandler would need to keep a reference to the \nCamera, so I ended up using the pointer to the Camera object as the \ncookie as this like a numerical cookie avoids all lifetime issues. That \nway whenever a PipelineHandler is handed a Camera object it can use the \npointer to\n\n1. Validate that the Camera object in question really is its \n   responsibility to handle.\n\n2. Use the pointer as a cookie to look-up any camera specific data in \n   its private data structures.\n\nI think that is quiet neat as it uses infrastructure we need anyhow to \nproperly manage the interactions between the two objects.\n\n> \n> Thanks\n>    j\n> \n> Jacopo Mondi (2):\n>   libcamera: camera: Add CameraData\n>   libcamera: ipu3: Create CIO2 V4L2 devices\n> \n>  include/libcamera/camera.h           | 13 ++++++++\n>  src/libcamera/camera.cpp             | 50 ++++++++++++++++++++++++++++\n>  src/libcamera/pipeline/ipu3/ipu3.cpp | 42 +++++++++++++++++++++++\n>  3 files changed, 105 insertions(+)\n> \n> --\n> 2.20.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x244.google.com (mail-lj1-x244.google.com\n\t[IPv6:2a00:1450:4864:20::244])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4369C60C78\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Jan 2019 01:08:01 +0100 (CET)","by mail-lj1-x244.google.com with SMTP id q2-v6so295777lji.10\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 22 Jan 2019 16:08:01 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\t11-v6sm227309ljv.1.2019.01.22.16.07.59\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 22 Jan 2019 16:07:59 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=1Ziscxi40sv93pPhpjbPn3AFdGFhiuOrWrcg4C1xB4Y=;\n\tb=0pHcLYTGs/SmMaRG8mK6now3s2hazwTeEb7883unaahsg+NHpc9KX0JRsMl2wiV0rI\n\toZDTuR/4q1iTUJHmqD/oBLIEVg2ZlUmHuaeOwHvbNYS80spZRtZOYj9IEzocJzs4vvgp\n\tLHM7kKUv/pNgEkb44EG9Welo9tpVYJbKtwHrxJpEd7D4LVch/hv9EZ3BlZx694EeD2y3\n\tD0hVmCwQ/6TxKXsrPdMa1hflo0ShBQVJz74zjkYHHEgbb8MORR9qMslEIfn9xL6T9H5I\n\tsd63Ail3Dl8tFo0KT/Ui2OzH6KArCgb7hYhvnciQ7DU6insEowzs0W0ldoLuGPnQ5faZ\n\t/CMw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=1Ziscxi40sv93pPhpjbPn3AFdGFhiuOrWrcg4C1xB4Y=;\n\tb=gAKAqwaHJWe3aqorZbMiMNpucapepU6Ok+oVziAUIKw6WI+fuNi7CXGBAroYtlifEQ\n\tYD3++K/e2QiJwb9evaIlHZhcIEY/gem86M4BuBXVGMekZS3dZWybZrOrf2XMKDV03eXN\n\t758TgF5e452MPcNwoKv+C44WiOJihTBfZcVU+TUBapwWKVCldI4HHpurxwEjJ1mN5kEg\n\tZ28xKNGXu22bFPqmnD07QyxMiNzMBb01KEDsaiuWrHAs7jYWCxSkdVOpDb87zWzS78OT\n\tU949PzmJFH0u33M6BevTT7YXPB1A7xZTVLNYl/ngv1HpJuJWOOPgYduVfLAKHMMz/AkQ\n\tc3Ww==","X-Gm-Message-State":"AJcUukf02YknYk247Mcc5DG2iOqseQhtlRsPRP9rNXQP7sSqb7T1Dnis\n\tjSlQ/gnPsg79YYcH6h63tFSoli00N8k=","X-Google-Smtp-Source":"ALg8bN7yuArZpNS/7RWmpTfeS2yFk+JB5nai4i2GA17p4/2lw7Y9ULeaDOtMpZREof/7RhRcXejDoQ==","X-Received":"by 2002:a2e:9cd5:: with SMTP id\n\tg21-v6mr39031ljj.48.1548202080525; \n\tTue, 22 Jan 2019 16:08:00 -0800 (PST)","Date":"Wed, 23 Jan 2019 01:07:58 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190123000758.GH4127@bigcity.dyn.berto.se>","References":"<20190122181225.12922-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190122181225.12922-1-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [RFC 0/2] Add support for pipeline specific\n\tdata to Cameras","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Wed, 23 Jan 2019 00:08:01 -0000"}},{"id":514,"web_url":"https://patchwork.libcamera.org/comment/514/","msgid":"<20190123100922.GF4485@pendragon.ideasonboard.com>","date":"2019-01-23T10:09:22","subject":"Re: [libcamera-devel] [RFC 0/2] Add support for pipeline specific\n\tdata to Cameras","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Wed, Jan 23, 2019 at 01:07:58AM +0100, Niklas Söderlund wrote:\n> On 2019-01-22 19:12:23 +0100, Jacopo Mondi wrote:\n> > RFC as I'm not sure about the idea of delegating ownership of platform data\n> > to Cameras, as that requires the pipeline handler to dynamically allocate the\n> > resources (if they go away at pipeline handler destruction time, is pointless\n> > to store them in Camera).\n> > \n> > The other way around is the idea of borrowing pipeline handler data to Cameras,\n> > but as Cameras are shared objects, they might stay around longer that pipeline\n> > handlers, and thus I felt it is safer to tie the CameraData lifetime to the one\n> > of the Camera instance they're associated to.\n> \n> While working with the Camera API and it's interaction with the pipeline \n> handler I felt a need for something similar you have here. My initial \n> idea to avoid lifetime management issues which you deal with in this \n> series was for the PipelineHandler to be able to set a unsigned int as a \n> cookie on the Camera at creation time, which it could retrieve with it's \n> later interaction with a Camera object.\n> \n> In the end I went a different way, as it became clear we would need an \n> two way association between the Camera and the PipelineHandler anyhow.  \n> That is the PipelineHandler would need to keep a reference to the \n> Camera, so I ended up using the pointer to the Camera object as the \n> cookie as this like a numerical cookie avoids all lifetime issues. That \n> way whenever a PipelineHandler is handed a Camera object it can use the \n> pointer to\n> \n> 1. Validate that the Camera object in question really is its \n>    responsibility to handle.\n\nGiven that all calls to the pipeline handler will go through the Camera\nobject, I think any such validation should be done there to avoid\nduplicating code in pipeline handlers. Furthermore, as the Camera object\nwill need to store a pointer to the pipeline handler in order to\nfirmward the calls, I don't see how we could get it wrong :-)\n\n> 2. Use the pointer as a cookie to look-up any camera specific data in \n>    its private data structures.\n\nThis would require all pipeline handlers to implement the equivalent of\na std::map<Camera *, PrivateData *>. Isn't Jacopo's approach simpler as\nit moves this to a pointer in the Camera object itself, avoiding lookup\ncode duplication in the pipeline handlers.\n\n> I think that is quiet neat as it uses infrastructure we need anyhow to \n> properly manage the interactions between the two objects.\n> \n> > Jacopo Mondi (2):\n> >   libcamera: camera: Add CameraData\n> >   libcamera: ipu3: Create CIO2 V4L2 devices\n> > \n> >  include/libcamera/camera.h           | 13 ++++++++\n> >  src/libcamera/camera.cpp             | 50 ++++++++++++++++++++++++++++\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 42 +++++++++++++++++++++++\n> >  3 files changed, 105 insertions(+)\n> >","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 670A760B1B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Jan 2019 11:09:24 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AD20C23D;\n\tWed, 23 Jan 2019 11:09:23 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548238163;\n\tbh=MZpWCXePvqrYxtL2d0GOnoCCZsFNvlzfzx8m7tzb6B4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=DR74GAhRbuNUV75WlUN7oyCHWh0xE00zoEH8DP4P7u3s+c3ua/sPo8ZHamTN1Bt9c\n\tr7jPbym7uJmEFh7B3UzQvgeSkCiCmP0cmHZWGxnSLOvKtHRksvcXEK1yGvkp3fmIwr\n\tvYaSy0uqJ90iE4FhyuA+0Bh7GBfjFLT9c+MFQgq4=","Date":"Wed, 23 Jan 2019 12:09:22 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","Message-ID":"<20190123100922.GF4485@pendragon.ideasonboard.com>","References":"<20190122181225.12922-1-jacopo@jmondi.org>\n\t<20190123000758.GH4127@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190123000758.GH4127@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [RFC 0/2] Add support for pipeline specific\n\tdata to Cameras","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Wed, 23 Jan 2019 10:09:24 -0000"}},{"id":518,"web_url":"https://patchwork.libcamera.org/comment/518/","msgid":"<20190123103101.GM4127@bigcity.dyn.berto.se>","date":"2019-01-23T10:31:01","subject":"Re: [libcamera-devel] [RFC 0/2] Add support for pipeline specific\n\tdata to Cameras","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nOn 2019-01-23 12:09:22 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> On Wed, Jan 23, 2019 at 01:07:58AM +0100, Niklas Söderlund wrote:\n> > On 2019-01-22 19:12:23 +0100, Jacopo Mondi wrote:\n> > > RFC as I'm not sure about the idea of delegating ownership of platform data\n> > > to Cameras, as that requires the pipeline handler to dynamically allocate the\n> > > resources (if they go away at pipeline handler destruction time, is pointless\n> > > to store them in Camera).\n> > > \n> > > The other way around is the idea of borrowing pipeline handler data to Cameras,\n> > > but as Cameras are shared objects, they might stay around longer that pipeline\n> > > handlers, and thus I felt it is safer to tie the CameraData lifetime to the one\n> > > of the Camera instance they're associated to.\n> > \n> > While working with the Camera API and it's interaction with the pipeline \n> > handler I felt a need for something similar you have here. My initial \n> > idea to avoid lifetime management issues which you deal with in this \n> > series was for the PipelineHandler to be able to set a unsigned int as a \n> > cookie on the Camera at creation time, which it could retrieve with it's \n> > later interaction with a Camera object.\n> > \n> > In the end I went a different way, as it became clear we would need an \n> > two way association between the Camera and the PipelineHandler anyhow.  \n> > That is the PipelineHandler would need to keep a reference to the \n> > Camera, so I ended up using the pointer to the Camera object as the \n> > cookie as this like a numerical cookie avoids all lifetime issues. That \n> > way whenever a PipelineHandler is handed a Camera object it can use the \n> > pointer to\n> > \n> > 1. Validate that the Camera object in question really is its \n> >    responsibility to handle.\n> \n> Given that all calls to the pipeline handler will go through the Camera\n> object, I think any such validation should be done there to avoid\n> duplicating code in pipeline handlers. Furthermore, as the Camera object\n> will need to store a pointer to the pipeline handler in order to\n> firmward the calls, I don't see how we could get it wrong :-)\n\nThat is a good point indeed, maybe we can drop the validation for now \nthen :-) I feel a bit stupid not thinking about it...\n\n> \n> > 2. Use the pointer as a cookie to look-up any camera specific data in \n> >    its private data structures.\n> \n> This would require all pipeline handlers to implement the equivalent of\n> a std::map<Camera *, PrivateData *>. Isn't Jacopo's approach simpler as\n> it moves this to a pointer in the Camera object itself, avoiding lookup\n> code duplication in the pipeline handlers.\n\nOne option is to create a common implementation in the PipelineHandler \nbase class to remove the code duplication. I'm not against Jacopos \nsuggestion categorically, I only feel the same thing could be achieved \nwith less complexity. Or maybe my fear of object lifetime issues is \nbordering on the insane.\n\n> \n> > I think that is quiet neat as it uses infrastructure we need anyhow to \n> > properly manage the interactions between the two objects.\n> > \n> > > Jacopo Mondi (2):\n> > >   libcamera: camera: Add CameraData\n> > >   libcamera: ipu3: Create CIO2 V4L2 devices\n> > > \n> > >  include/libcamera/camera.h           | 13 ++++++++\n> > >  src/libcamera/camera.cpp             | 50 ++++++++++++++++++++++++++++\n> > >  src/libcamera/pipeline/ipu3/ipu3.cpp | 42 +++++++++++++++++++++++\n> > >  3 files changed, 105 insertions(+)\n> > > \n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B7C0360B1B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Jan 2019 11:31:03 +0100 (CET)","by mail-lj1-x243.google.com with SMTP id k15-v6so1433425ljc.8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Jan 2019 02:31:03 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\to5-v6sm421754ljh.75.2019.01.23.02.31.02\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tWed, 23 Jan 2019 02:31:02 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=yivrlEf54gbPD+U7//xUnbJ51cxAb6ZgJS/iu3OuTj8=;\n\tb=TQWV17TdN8mtq5MkYZuIAHzVRP6W1Vx4ydCu3YYWXE6c+lzIv8tJ4+asUWN6nlzEt6\n\tmBE/BHJPoLd6gcZbaXJzmnYc72QZRN729N4Z4kMD5orWf5+5Ho++NSncAHYs5dEyTMwc\n\tlu1sM2WkFCjFIs4x4Xu0noR4Fpy8wg6+c9qKGs2pwrNVnzhtbAJMu8nvcXzjNSuKw7XY\n\tqLRUZoGc4aslvI5VolW4eSpUvkfd6K7Bu51QeM35H1YHk6l2LLCD9VvxH0zG4P0OuPAX\n\tz16FHzrt9cn+Q/eEtJi8GOrP0M9FpSqOWECWiFQUuZqkrx7PnlluHBnCfg5cxQdl7gKJ\n\tP8YA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=yivrlEf54gbPD+U7//xUnbJ51cxAb6ZgJS/iu3OuTj8=;\n\tb=MO0OlnlYcsqG1CgDRaaqnPjEc8GeGYk6wLraC2ieDAjoAySfBtZuehZoC3cT4Wri5y\n\t3pFagJaUn92maTyfPyIfccV2ie+pxlGJklWPZeiqGTgdbqfn4akYIqAfr9cFTHSPZU20\n\tj73f9Sy+rAlvSP9zOhbmOr8MwpS2C0lIu7abAD/oRkZWhzQGTE6NmjOFp/mC3OhKWf4+\n\tE2Gh2sA2VWEkLtEQhQmqrp9kKa2MFVE7UMTrtkDCwgsSBffkaM7HWaVL+X4Wfe7IeMe3\n\teN9/Rct11erh5KfoMt1eABGk1NlOLINbCP7WxGefMDlYqFalnmfZoyyDnomc72vLLIsn\n\tL96A==","X-Gm-Message-State":"AJcUukerKSHmjGS3/sfLFi7lqzetBmIy/8uVkv3y0ebraazuODMUhjln\n\t9ACi9BoHDLJxtJeW6ZyisjTTMA==","X-Google-Smtp-Source":"ALg8bN6B1YkLNCUpCckG9ek2xnkmsKZjbWfmgstVqI/83gjv+ci+IED9DHpPeXfi6Heb8fEnhl7U1w==","X-Received":"by 2002:a2e:6503:: with SMTP id\n\tz3-v6mr1456399ljb.153.1548239462984; \n\tWed, 23 Jan 2019 02:31:02 -0800 (PST)","Date":"Wed, 23 Jan 2019 11:31:01 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","Message-ID":"<20190123103101.GM4127@bigcity.dyn.berto.se>","References":"<20190122181225.12922-1-jacopo@jmondi.org>\n\t<20190123000758.GH4127@bigcity.dyn.berto.se>\n\t<20190123100922.GF4485@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190123100922.GF4485@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [RFC 0/2] Add support for pipeline specific\n\tdata to Cameras","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Wed, 23 Jan 2019 10:31:04 -0000"}},{"id":520,"web_url":"https://patchwork.libcamera.org/comment/520/","msgid":"<20190123103841.GI4485@pendragon.ideasonboard.com>","date":"2019-01-23T10:38:41","subject":"Re: [libcamera-devel] [RFC 0/2] Add support for pipeline specific\n\tdata to Cameras","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Wed, Jan 23, 2019 at 11:31:01AM +0100, Niklas Söderlund wrote:\n> On 2019-01-23 12:09:22 +0200, Laurent Pinchart wrote:\n> > On Wed, Jan 23, 2019 at 01:07:58AM +0100, Niklas Söderlund wrote:\n> >> On 2019-01-22 19:12:23 +0100, Jacopo Mondi wrote:\n> >>> RFC as I'm not sure about the idea of delegating ownership of platform data\n> >>> to Cameras, as that requires the pipeline handler to dynamically allocate the\n> >>> resources (if they go away at pipeline handler destruction time, is pointless\n> >>> to store them in Camera).\n> >>> \n> >>> The other way around is the idea of borrowing pipeline handler data to Cameras,\n> >>> but as Cameras are shared objects, they might stay around longer that pipeline\n> >>> handlers, and thus I felt it is safer to tie the CameraData lifetime to the one\n> >>> of the Camera instance they're associated to.\n> >> \n> >> While working with the Camera API and it's interaction with the pipeline \n> >> handler I felt a need for something similar you have here. My initial \n> >> idea to avoid lifetime management issues which you deal with in this \n> >> series was for the PipelineHandler to be able to set a unsigned int as a \n> >> cookie on the Camera at creation time, which it could retrieve with it's \n> >> later interaction with a Camera object.\n> >> \n> >> In the end I went a different way, as it became clear we would need an \n> >> two way association between the Camera and the PipelineHandler anyhow.  \n> >> That is the PipelineHandler would need to keep a reference to the \n> >> Camera, so I ended up using the pointer to the Camera object as the \n> >> cookie as this like a numerical cookie avoids all lifetime issues. That \n> >> way whenever a PipelineHandler is handed a Camera object it can use the \n> >> pointer to\n> >> \n> >> 1. Validate that the Camera object in question really is its \n> >>    responsibility to handle.\n> > \n> > Given that all calls to the pipeline handler will go through the Camera\n> > object, I think any such validation should be done there to avoid\n> > duplicating code in pipeline handlers. Furthermore, as the Camera object\n> > will need to store a pointer to the pipeline handler in order to\n> > firmward the calls, I don't see how we could get it wrong :-)\n> \n> That is a good point indeed, maybe we can drop the validation for now \n> then :-) I feel a bit stupid not thinking about it...\n> \n> > \n> >> 2. Use the pointer as a cookie to look-up any camera specific data in \n> >>    its private data structures.\n> > \n> > This would require all pipeline handlers to implement the equivalent of\n> > a std::map<Camera *, PrivateData *>. Isn't Jacopo's approach simpler as\n> > it moves this to a pointer in the Camera object itself, avoiding lookup\n> > code duplication in the pipeline handlers.\n> \n> One option is to create a common implementation in the PipelineHandler \n> base class to remove the code duplication.\n\nI haven't thought about that, it's a good point.\n\n> I'm not against Jacopos suggestion categorically, I only feel the same\n> thing could be achieved with less complexity. Or maybe my fear of\n> object lifetime issues is bordering on the insane.\n\nI'll let you discuss the pros and cons with Jacopo :-) It could possibly\nhelp to sketch how this would look like in the IPU3 pipeline handler\n(with the helper method in the PipelineHandler class) to verify that\nlifetime management is sound, and compare the two options.\n\n> >> I think that is quiet neat as it uses infrastructure we need anyhow to \n> >> properly manage the interactions between the two objects.\n> >> \n> >>> Jacopo Mondi (2):\n> >>>   libcamera: camera: Add CameraData\n> >>>   libcamera: ipu3: Create CIO2 V4L2 devices\n> >>> \n> >>>  include/libcamera/camera.h           | 13 ++++++++\n> >>>  src/libcamera/camera.cpp             | 50 ++++++++++++++++++++++++++++\n> >>>  src/libcamera/pipeline/ipu3/ipu3.cpp | 42 +++++++++++++++++++++++\n> >>>  3 files changed, 105 insertions(+)","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ACEA960B1B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Jan 2019 11:38:42 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 28E8623D;\n\tWed, 23 Jan 2019 11:38:42 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548239922;\n\tbh=nIGctrMlF1i70tQQukqovFE3vXHi+58fR2BWvXMSQ90=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=DX/rsth1c49B1sOdVHGJ25NV77vZF0FwOyfczcXH7TU3XfCS3YYvrt/apxGSoXpow\n\tr62ycdx/UbFGlZ+5i6efG9uTXvrw33uH8M1vLgunJVvP7MS77v+SElC9e050gWPYAA\n\tWCA4+uJevJk/79KIhYD5WWT2SYAkj0fMiA3E8glE=","Date":"Wed, 23 Jan 2019 12:38:41 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","Message-ID":"<20190123103841.GI4485@pendragon.ideasonboard.com>","References":"<20190122181225.12922-1-jacopo@jmondi.org>\n\t<20190123000758.GH4127@bigcity.dyn.berto.se>\n\t<20190123100922.GF4485@pendragon.ideasonboard.com>\n\t<20190123103101.GM4127@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190123103101.GM4127@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [RFC 0/2] Add support for pipeline specific\n\tdata to Cameras","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Wed, 23 Jan 2019 10:38:42 -0000"}}]