[{"id":2996,"web_url":"https://patchwork.libcamera.org/comment/2996/","msgid":"<20191106081340.yqfcjovqatg655mq@uno.localdomain>","date":"2019-11-06T08:13:40","subject":"Re: [libcamera-devel] [PATCH 09/10] cam: Store camera as shared\n\tpointer everywhere","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n thanks for the patch\n\nOn Mon, Oct 28, 2019 at 03:22:23AM +0100, Niklas Söderlund wrote:\n> Do not store the camera raw pointer in the capture class, this will\n> prevent forwarding the shared pointer in the future.\n>\n> Signed-off-by: Niklas Söderlund \n> ---\n> src/cam/capture.cpp | 2 +-\n> src/cam/capture.h | 4 ++--\n> src/cam/main.cpp | 2 +-\n> 3 files changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> index 27332df1d55a5c2d..e665d819fb777a90 100644\n> --- a/src/cam/capture.cpp\n> +++ b/src/cam/capture.cpp\n> @@ -16,7 +16,7 @@\n>\n> using namespace libcamera;\n>\n> -Capture::Capture(Camera *camera, CameraConfiguration *config)\n> +Capture::Capture(std::shared_ptr camera, CameraConfiguration *config)\n> \t: camera_(camera), config_(config), writer_(nullptr)\n> {\n> }\n> diff --git a/src/cam/capture.h b/src/cam/capture.h\n> index 4d396afb8c771a74..c692d48918f2de1d 100644\n> --- a/src/cam/capture.h\n> +++ b/src/cam/capture.h\n> @@ -21,7 +21,7 @@\n> class Capture\n> {\n> public:\n> -\tCapture(libcamera::Camera *camera,\n> +\tCapture(std::shared_ptr camera,\n> \t\tlibcamera::CameraConfiguration *config);\n>\n> \tint run(EventLoop *loop, const OptionsParser::Options &options);\n> @@ -30,7 +30,7 @@ private:\n>\n> \tvoid requestComplete(libcamera::Request *request);\n>\n> -\tlibcamera::Camera *camera_;\n> +\tstd::shared_ptr camera_;\n> \tlibcamera::CameraConfiguration *config_;\n>\n> \tstd::map streamName_;\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index 9d99f5587cbb77d0..a38cca959aca05ff 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -319,7 +319,7 @@ int CamApp::run()\n> \t}\n>\n> \tif (options_.isSet(OptCapture)) {\n> -\t\tCapture capture(camera_.get(), config_.get());\n> +\t\tCapture capture(camera_, config_.get());\n> \t\treturn capture.run(loop_, options_);\n\nI'm not sure increasing the reference count of the shared pointer to\nstore it in an object whose lifetime is limited to this two lines is\nworth. It doesn't hurt for sure, so I'm fine if you want to keep it.\n\n> \t}\n>\n> --\n> 2.23.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["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 05EB860C41\n\tfor ;\n\tWed, 6 Nov 2019 09:11:44 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 984EB1C000D;\n\tWed, 6 Nov 2019 08:11:43 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Wed, 6 Nov 2019 09:13:40 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191106081340.yqfcjovqatg655mq@uno.localdomain>","References":"<20191028022224.795355-1-niklas.soderlund@ragnatech.se>\n\t<20191028022224.795355-10-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"7mtdjd5t6apuwrfu\"","Content-Disposition":"inline","In-Reply-To":"<20191028022224.795355-10-niklas.soderlund@ragnatech.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 09/10] cam: Store camera as shared\n\tpointer everywhere","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","X-List-Received-Date":"Wed, 06 Nov 2019 08:11:44 -0000"}},{"id":3055,"web_url":"https://patchwork.libcamera.org/comment/3055/","msgid":"<20191118114154.GL4724@pendragon.ideasonboard.com>","date":"2019-11-18T11:41:54","subject":"Re: [libcamera-devel] [PATCH 09/10] cam: Store camera as shared\n\tpointer everywhere","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Wed, Nov 06, 2019 at 09:13:40AM +0100, Jacopo Mondi wrote:\n> On Mon, Oct 28, 2019 at 03:22:23AM +0100, Niklas Söderlund wrote:\n> > Do not store the camera raw pointer in the capture class, this will\n> > prevent forwarding the shared pointer in the future.\n> >\n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > src/cam/capture.cpp | 2 +-\n> > src/cam/capture.h | 4 ++--\n> > src/cam/main.cpp | 2 +-\n> > 3 files changed, 4 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> > index 27332df1d55a5c2d..e665d819fb777a90 100644\n> > --- a/src/cam/capture.cpp\n> > +++ b/src/cam/capture.cpp\n> > @@ -16,7 +16,7 @@\n> >\n> > using namespace libcamera;\n> >\n> > -Capture::Capture(Camera *camera, CameraConfiguration *config)\n> > +Capture::Capture(std::shared_ptr camera, CameraConfiguration *config)\n> > \t: camera_(camera), config_(config), writer_(nullptr)\n> > {\n> > }\n> > diff --git a/src/cam/capture.h b/src/cam/capture.h\n> > index 4d396afb8c771a74..c692d48918f2de1d 100644\n> > --- a/src/cam/capture.h\n> > +++ b/src/cam/capture.h\n> > @@ -21,7 +21,7 @@\n> > class Capture\n> > {\n> > public:\n> > -\tCapture(libcamera::Camera *camera,\n> > +\tCapture(std::shared_ptr camera,\n> > \t\tlibcamera::CameraConfiguration *config);\n> >\n> > \tint run(EventLoop *loop, const OptionsParser::Options &options);\n> > @@ -30,7 +30,7 @@ private:\n> >\n> > \tvoid requestComplete(libcamera::Request *request);\n> >\n> > -\tlibcamera::Camera *camera_;\n> > +\tstd::shared_ptr camera_;\n> > \tlibcamera::CameraConfiguration *config_;\n> >\n> > \tstd::map streamName_;\n> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > index 9d99f5587cbb77d0..a38cca959aca05ff 100644\n> > --- a/src/cam/main.cpp\n> > +++ b/src/cam/main.cpp\n> > @@ -319,7 +319,7 @@ int CamApp::run()\n> > \t}\n> >\n> > \tif (options_.isSet(OptCapture)) {\n> > -\t\tCapture capture(camera_.get(), config_.get());\n> > +\t\tCapture capture(camera_, config_.get());\n> > \t\treturn capture.run(loop_, options_);\n> \n> I'm not sure increasing the reference count of the shared pointer to\n> store it in an object whose lifetime is limited to this two lines is\n> worth. It doesn't hurt for sure, so I'm fine if you want to keep it.\n\nI share Jacopo's opinion. I haven't checked if this change is needed for\nyour buffer rework. If it's not, I'm not opposed to it, but I'm also not\nsure it's really worth it.\n\n> > \t}\n> >","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 D328960F1C\n\tfor ;\n\tMon, 18 Nov 2019 12:41:59 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0084E563;\n\tMon, 18 Nov 2019 12:41:58 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1574077319;\n\tbh=y3J5uETmEvt69/8SOpu3xukRHxEN5bcD10aBwxSzQAs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UrfpHDb8ZoY4YgEPJOfgmm+2/vC0vVIVWWckoyVPBDZwOC8p6VOUE2KFpS+Vtu5C/\n\tg8qJkCGCZDynisPn7CmkBsiASp4cL4EroIvGiyLSt3opgfjJELw7sQmXPKYkRiLe6d\n\tRzuRWSqYo/LPxCU4TDHbU5BIdYLCkp5dtfP0KeyI=","Date":"Mon, 18 Nov 2019 13:41:54 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"Niklas =?utf-8?q?S=C3=B6derlund?= ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20191118114154.GL4724@pendragon.ideasonboard.com>","References":"<20191028022224.795355-1-niklas.soderlund@ragnatech.se>\n\t<20191028022224.795355-10-niklas.soderlund@ragnatech.se>\n\t<20191106081340.yqfcjovqatg655mq@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191106081340.yqfcjovqatg655mq@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 09/10] cam: Store camera as shared\n\tpointer everywhere","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","X-List-Received-Date":"Mon, 18 Nov 2019 11:42:00 -0000"}},{"id":3113,"web_url":"https://patchwork.libcamera.org/comment/3113/","msgid":"<20191119234546.GS8072@bigcity.dyn.berto.se>","date":"2019-11-19T23:45:46","subject":"Re: [libcamera-devel] [PATCH 09/10] cam: Store camera as shared\n\tpointer everywhere","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo, Laurent,\n\nThanks for your feedback.\n\nOn 2019-11-18 13:41:54 +0200, Laurent Pinchart wrote:\n> Hello,\n> \n> On Wed, Nov 06, 2019 at 09:13:40AM +0100, Jacopo Mondi wrote:\n> > On Mon, Oct 28, 2019 at 03:22:23AM +0100, Niklas Söderlund wrote:\n> > > Do not store the camera raw pointer in the capture class, this will\n> > > prevent forwarding the shared pointer in the future.\n> > >\n> > > Signed-off-by: Niklas Söderlund \n> > > ---\n> > > src/cam/capture.cpp | 2 +-\n> > > src/cam/capture.h | 4 ++--\n> > > src/cam/main.cpp | 2 +-\n> > > 3 files changed, 4 insertions(+), 4 deletions(-)\n> > >\n> > > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> > > index 27332df1d55a5c2d..e665d819fb777a90 100644\n> > > --- a/src/cam/capture.cpp\n> > > +++ b/src/cam/capture.cpp\n> > > @@ -16,7 +16,7 @@\n> > >\n> > > using namespace libcamera;\n> > >\n> > > -Capture::Capture(Camera *camera, CameraConfiguration *config)\n> > > +Capture::Capture(std::shared_ptr camera, CameraConfiguration *config)\n> > > \t: camera_(camera), config_(config), writer_(nullptr)\n> > > {\n> > > }\n> > > diff --git a/src/cam/capture.h b/src/cam/capture.h\n> > > index 4d396afb8c771a74..c692d48918f2de1d 100644\n> > > --- a/src/cam/capture.h\n> > > +++ b/src/cam/capture.h\n> > > @@ -21,7 +21,7 @@\n> > > class Capture\n> > > {\n> > > public:\n> > > -\tCapture(libcamera::Camera *camera,\n> > > +\tCapture(std::shared_ptr camera,\n> > > \t\tlibcamera::CameraConfiguration *config);\n> > >\n> > > \tint run(EventLoop *loop, const OptionsParser::Options &options);\n> > > @@ -30,7 +30,7 @@ private:\n> > >\n> > > \tvoid requestComplete(libcamera::Request *request);\n> > >\n> > > -\tlibcamera::Camera *camera_;\n> > > +\tstd::shared_ptr camera_;\n> > > \tlibcamera::CameraConfiguration *config_;\n> > >\n> > > \tstd::map streamName_;\n> > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > > index 9d99f5587cbb77d0..a38cca959aca05ff 100644\n> > > --- a/src/cam/main.cpp\n> > > +++ b/src/cam/main.cpp\n> > > @@ -319,7 +319,7 @@ int CamApp::run()\n> > > \t}\n> > >\n> > > \tif (options_.isSet(OptCapture)) {\n> > > -\t\tCapture capture(camera_.get(), config_.get());\n> > > +\t\tCapture capture(camera_, config_.get());\n> > > \t\treturn capture.run(loop_, options_);\n> > \n> > I'm not sure increasing the reference count of the shared pointer to\n> > store it in an object whose lifetime is limited to this two lines is\n> > worth. It doesn't hurt for sure, so I'm fine if you want to keep it.\n> \n> I share Jacopo's opinion. I haven't checked if this change is needed for\n> your buffer rework. If it's not, I'm not opposed to it, but I'm also not\n> sure it's really worth it.\n\nIt's needed for the buffer rework so it could be moved to that series. \nBut out of taste I also think this is the right thing to do so I kept it \nin the cleanup series.\n\nI will collect Jacopo's and Kieran's tags for this and send it out in v2 \nbut let me know if you think it should be moved to the buffer api \nseries.\n\n> \n> > > \t}\n> > >\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 674F960F1C\n\tfor ;\n\tWed, 20 Nov 2019 00:45:49 +0100 (CET)","by mail-lj1-x241.google.com with SMTP id q2so25407660ljg.7\n\tfor ;\n\tTue, 19 Nov 2019 15:45:49 -0800 (PST)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\tt28sm10683249ljd.47.2019.11.19.15.45.46\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 19 Nov 2019 15:45:47 -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=xG5Sq8hAi/8L71el1dqQTwqjTorgdW0oyLgAlmVKuc8=;\n\tb=LgtHIeu3oeLK7wSzXg4J3582RhAUQzk2Dks6pIbDdnJtv24f6gTCDJfiYZHIyuSzp0\n\tWmVXKQYQhLD+bPQwIcbjX9TLXxfjKS4zkUHg3wohzZdjIYgjUrQSHnxf0C0gpoFSFUQa\n\tbjLfsTwBno8XD2wnKKkDCwfZj7v92CpqKtSmTBCjNaoeexmyYckQJz+3XO3mIMqj7JIO\n\tMuxzXtvziFoXes2EWkgigyO9ggSSUuWl80HoiH0sYwOHp53isOjFoz+L0U/MqDkdqn+Q\n\tI8WSD8RjMWEAjeauHKWxS3k8uLN7WHZuKHHNVehWU5BAgEDAMZB2yhNYQAJySgRedYMU\n\totzQ==","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=xG5Sq8hAi/8L71el1dqQTwqjTorgdW0oyLgAlmVKuc8=;\n\tb=YF5fdXdtYG7WeLUY/GqYiC6WT3+zLE2kRWzMYGndXNaLrRL2fBRAE2G28lu43FzzD9\n\t6tag03czvy+Infm1r2ZWQu2oOHbXl9TOrPTs73uXpcD2dXmqpxEzLqrILf+C1D1RYANh\n\tUKsgMmozrZt0UnWDalaw13qLqjAKh5vv4TSWiXyLBQpRPUGDCdC6Z2teW2gv8/IIgY8p\n\tLX0TgXj+bcD1qOBEJ4sTUId2lmbgVrYKEtCeZV4gpLxVEmurVfsN7I5RHvntP6b36JhI\n\tcr+Ysnw2fN0L+WbkSstpXKlmJOAP0PoXUBtOnGrPrnyvT+WKqwfoKsPsayiAVRMyv/d5\n\tJFlQ==","X-Gm-Message-State":"APjAAAU6lHxi894nySmjrkK2o4lNa8vcVv5EsGrRGKwIO81RFTfabNbg\n\tO3Y6RwaMLEUhdINPSar4OHm/jw==","X-Google-Smtp-Source":"APXvYqzr9qDIK3+W564w3vkGm2e7mt+O/9aDPXGYzcuYMrh3I9pQfKHimHzFwpDTkZDseJUdtB95pw==","X-Received":"by 2002:a05:651c:20a:: with SMTP id\n\ty10mr108070ljn.76.1574207148534; \n\tTue, 19 Nov 2019 15:45:48 -0800 (PST)","Date":"Wed, 20 Nov 2019 00:45:46 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","Message-ID":"<20191119234546.GS8072@bigcity.dyn.berto.se>","References":"<20191028022224.795355-1-niklas.soderlund@ragnatech.se>\n\t<20191028022224.795355-10-niklas.soderlund@ragnatech.se>\n\t<20191106081340.yqfcjovqatg655mq@uno.localdomain>\n\t<20191118114154.GL4724@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":"<20191118114154.GL4724@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH 09/10] cam: Store camera as shared\n\tpointer everywhere","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","X-List-Received-Date":"Tue, 19 Nov 2019 23:45:49 -0000"}}]