[{"id":31892,"web_url":"https://patchwork.libcamera.org/comment/31892/","msgid":"<6eFAprqW7U0beSinpt2WZJczlC5tgBiQdX2cqJy0yxqV93xSwZKQNONKv2N4tPq71Z1CaoU-IWCLe63wzD07PJ35xE8BAXFFJfr0_xd3nEg=@protonmail.com>","date":"2024-10-23T17:33:32","subject":"Re: [PATCH] libcamera: Add static Object::Deleter function","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2024. október 23., szerda 18:58 keltezéssel, Harvey Yang írta:\n\n> This patch allows a smart pointer of Object to be destructed in other\n> threads.\n> \n> There will be multiple usages of smart pointers of Object. This patch\n> adds the deleter function to avoid duplicated code.\n> \n> Signed-off-by: Harvey Yang \n> ---\n> include/libcamera/base/object.h | 2 ++\n> src/libcamera/base/object.cpp | 11 +++++++++++\n> 2 files changed, 13 insertions(+)\n> \n> diff --git a/include/libcamera/base/object.h b/include/libcamera/base/object.h\n> index 508773cd0..c4522d480 100644\n> --- a/include/libcamera/base/object.h\n> +++ b/include/libcamera/base/object.h\n> @@ -24,6 +24,8 @@ class Thread;\n> class Object\n> {\n> public:\n> +\tstatic void Deleter(Object *obj);\n\nI think a functor would be preferable, see camera.cpp:Camera::create():\n\n struct Deleter {\n void operator()(Object *obj) const\n {\n if (Thread::current() == obj->thread())\n delete obj;\n else\n obj->deleteLater();\n }\n };\n\nOr is there a reason why this wouldn't work?\n\n\nRegards,\nBarnabás Pőcze\n\n> +\n> \tObject(Object *parent = nullptr);\n> \tvirtual ~Object();\n> \n> diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp\n> index 745d2565a..2c04b99a5 100644\n> --- a/src/libcamera/base/object.cpp\n> +++ b/src/libcamera/base/object.cpp\n> @@ -59,6 +59,17 @@ LOG_DEFINE_CATEGORY(Object)\n> * \\sa Message, Signal, Thread\n> */\n> \n> +/**\n> + * \\brief A deleter function that calls Object::deleteLater\n> + * \\param[in] obj The object itself\n> + *\n> + * The static deleter function that's used in smart pointers.\n> + */\n> +void Object::Deleter(Object *obj)\n> +{\n> +\tobj->deleteLater();\n> +}\n> +\n> /**\n> * \\brief Construct an Object instance\n> * \\param[in] parent The object parent\n> --\n> 2.47.0.105.g07ac214952-goog\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 4E242C3213\n\tfor ;\n\tWed, 23 Oct 2024 17:33:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8F5D365393;\n\tWed, 23 Oct 2024 19:33:38 +0200 (CEST)","from mail-4316.protonmail.ch (mail-4316.protonmail.ch\n\t[185.70.43.16])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 754696537E\n\tfor ;\n\tWed, 23 Oct 2024 19:33:37 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"X40j3aLj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1729704816; x=1729964016;\n\tbh=Wt7S3epnzxPx3+xS9lcIc7G2XVxeXkbrUUhPZGzS0TI=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=X40j3aLjEQ36uJn6+rkllTPjsN/xYgyB1rZADeq1iMas2c09L0KCtGoE5194BKHWu\n\trgzm9TPlcVN5leb3ulS8ApnkozJavJYUvTKFSgTL/vHQ99CktYlPhUdnFv3XxZARq9\n\tGpY02RUYK4nCNhxlWiB4eKno1vA8YNTLDRrXIOgyxiH0Mw5S4ll8UiWAuPLHPf6Zt/\n\tRbjG4NFz/O+8BsIDmFDjpGDfArHKrzctwsvJKbYE/OPWrHfuBxM4JLg98B8jZp0Rdo\n\tMuxDvG/oUyURa17L77q/qMFHz+GqvYJVAZ27EuwhL+YxOdDYwwPlYEB0h3Pc20V6nk\n\tx+7uCQt4YVOrQ==","Date":"Wed, 23 Oct 2024 17:33:32 +0000","To":"Harvey Yang ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: Add static Object::Deleter function","Message-ID":"<6eFAprqW7U0beSinpt2WZJczlC5tgBiQdX2cqJy0yxqV93xSwZKQNONKv2N4tPq71Z1CaoU-IWCLe63wzD07PJ35xE8BAXFFJfr0_xd3nEg=@protonmail.com>","In-Reply-To":"<20241023165812.868221-1-chenghaoyang@chromium.org>","References":"<20241023165812.868221-1-chenghaoyang@chromium.org>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"6ad31181f1d1db3f20700b9fe20a002b81919cd9","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31901,"web_url":"https://patchwork.libcamera.org/comment/31901/","msgid":"","date":"2024-10-24T09:11:51","subject":"Re: [PATCH] libcamera: Add static Object::Deleter function","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Barnabás,\n\nOn Thu, Oct 24, 2024 at 1:33 AM Barnabás Pőcze wrote:\n>\n> Hi\n>\n>\n> 2024. október 23., szerda 18:58 keltezéssel, Harvey Yang írta:\n>\n> > This patch allows a smart pointer of Object to be destructed in other\n> > threads.\n> >\n> > There will be multiple usages of smart pointers of Object. This patch\n> > adds the deleter function to avoid duplicated code.\n> >\n> > Signed-off-by: Harvey Yang \n> > ---\n> > include/libcamera/base/object.h | 2 ++\n> > src/libcamera/base/object.cpp | 11 +++++++++++\n> > 2 files changed, 13 insertions(+)\n> >\n> > diff --git a/include/libcamera/base/object.h b/include/libcamera/base/object.h\n> > index 508773cd0..c4522d480 100644\n> > --- a/include/libcamera/base/object.h\n> > +++ b/include/libcamera/base/object.h\n> > @@ -24,6 +24,8 @@ class Thread;\n> > class Object\n> > {\n> > public:\n> > + static void Deleter(Object *obj);\n>\n> I think a functor would be preferable, see camera.cpp:Camera::create():\n>\n> struct Deleter {\n> void operator()(Object *obj) const\n> {\n> if (Thread::current() == obj->thread())\n> delete obj;\n> else\n> obj->deleteLater();\n> }\n> };\n>\n> Or is there a reason why this wouldn't work?\n\nTrue, thanks for the notice, and it works.\n\nI also just found that it's stated in object.cpp:\nhttps://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/base/object.cpp#n143\n\nDo you think I should add this functor to avoid duplication,\nor the developers should copy and paste the above\nfunctor each time for a derived class?\n\nBR,\nHarvey\n\n>\n>\n> Regards,\n> Barnabás Pőcze\n>\n> > +\n> > Object(Object *parent = nullptr);\n> > virtual ~Object();\n> >\n> > diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp\n> > index 745d2565a..2c04b99a5 100644\n> > --- a/src/libcamera/base/object.cpp\n> > +++ b/src/libcamera/base/object.cpp\n> > @@ -59,6 +59,17 @@ LOG_DEFINE_CATEGORY(Object)\n> > * \\sa Message, Signal, Thread\n> > */\n> >\n> > +/**\n> > + * \\brief A deleter function that calls Object::deleteLater\n> > + * \\param[in] obj The object itself\n> > + *\n> > + * The static deleter function that's used in smart pointers.\n> > + */\n> > +void Object::Deleter(Object *obj)\n> > +{\n> > + obj->deleteLater();\n> > +}\n> > +\n> > /**\n> > * \\brief Construct an Object instance\n> > * \\param[in] parent The object parent\n> > --\n> > 2.47.0.105.g07ac214952-goog\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 7BA76BE08B\n\tfor ;\n\tThu, 24 Oct 2024 09:12:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 452B465393;\n\tThu, 24 Oct 2024 11:12:05 +0200 (CEST)","from mail-lj1-x22f.google.com (mail-lj1-x22f.google.com\n\t[IPv6:2a00:1450:4864:20::22f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 03465603ED\n\tfor ;\n\tThu, 24 Oct 2024 11:12:03 +0200 (CEST)","by mail-lj1-x22f.google.com with SMTP id\n\t38308e7fff4ca-2fb57f97d75so5413561fa.2\n\tfor ;\n\tThu, 24 Oct 2024 02:12:03 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"c/ji49LV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1729761123; x=1730365923;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=RGUg7/pVYFrsBqv7+H9l9REaFzg4V1FshaO9jqK1L0A=;\n\tb=c/ji49LVW6Eh0KTBIpfbiCZGz+4w8omn9Y+iHy84xoxnnBgbujtsmYd5PSYG6PHDV2\n\tIZzNYi+GoZ0A+DjsfuwOFyNctrvCdIOsteZoPtXzjquU2FkhALt7cQ257/Dsv1K8Yl/a\n\t2tTvhPzPZEWobTiysZI8Xq4nUOOoEO5qMVOig=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1729761123; x=1730365923;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=RGUg7/pVYFrsBqv7+H9l9REaFzg4V1FshaO9jqK1L0A=;\n\tb=ORyjJZ1/JmiJ8dlvfr1mNTYaR7TW5PRkpMScYYG7RmMtAEpZxj4/p2leYTh2Ah6BUG\n\tt2Hx8e6zreIHDoSoMYxZjYGvVBGHv2iB0hJJdQZ9J0qektmklkqOhcNHS0adB5VByjZk\n\tmGL+sSuVriGv4xFogrJvWxgY3TJ+Psf8KvbYV7F+Do9tEBK0+49CTH3h5msuRE6+hdLS\n\th1d+6B9S3WiLdy4ok7fmsWyMFHys7kRuzBHdC2gsA+rxN+4B0Jf6qPOnIb260o3VQdon\n\tHvz5LDNILP9E0v9mt68BjUCUS+BzETZVLjBV9JHB5g1o7Li5CBHrzCFwxcnXwNCKiDgr\n\tjkhw==","X-Gm-Message-State":"AOJu0YwkuZ1n+eegVTKc2iOw7Irb376CUD/sZme9bJM/9XXPin2FqbXO\n\tTu1qL0tOdObnGvmLrIxzVTkjTD4IxoF7/cdAyuwwANTP+qdLwna614bgZXEWLqg2zkSy1i00SEL\n\twT6Zv7ENBe7bJQw/luRyfl4DaIIeRGKA35RU+vu056zT7EVrbpQ==","X-Google-Smtp-Source":"AGHT+IHKMHyUaels55CqWY2g0MxHZTE7LOcK1w14PPXEDGmwt14r5RTJgEdP3j9w50EUb4pIou+xbfCuBTolSGwnkyw=","X-Received":"by 2002:a2e:4a19:0:b0:2fa:bb5d:db67 with SMTP id\n\t38308e7fff4ca-2fca8261caemr6368801fa.32.1729761122988;\n\tThu, 24 Oct 2024 02:12:02 -0700 (PDT)","MIME-Version":"1.0","References":"<20241023165812.868221-1-chenghaoyang@chromium.org>\n\t<6eFAprqW7U0beSinpt2WZJczlC5tgBiQdX2cqJy0yxqV93xSwZKQNONKv2N4tPq71Z1CaoU-IWCLe63wzD07PJ35xE8BAXFFJfr0_xd3nEg=@protonmail.com>","In-Reply-To":"<6eFAprqW7U0beSinpt2WZJczlC5tgBiQdX2cqJy0yxqV93xSwZKQNONKv2N4tPq71Z1CaoU-IWCLe63wzD07PJ35xE8BAXFFJfr0_xd3nEg=@protonmail.com>","From":"Cheng-Hao Yang ","Date":"Thu, 24 Oct 2024 17:11:51 +0800","Message-ID":"","Subject":"Re: [PATCH] libcamera: Add static Object::Deleter function","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31906,"web_url":"https://patchwork.libcamera.org/comment/31906/","msgid":"","date":"2024-10-24T12:59:30","subject":"Re: [PATCH] libcamera: Add static Object::Deleter function","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2024. október 24., csütörtök 11:11 keltezéssel, Cheng-Hao Yang írta:\n\n> Hi Barnabás,\n> \n> On Thu, Oct 24, 2024 at 1:33 AM Barnabás Pőcze wrote:\n> >\n> > Hi\n> >\n> >\n> > 2024. október 23., szerda 18:58 keltezéssel, Harvey Yang írta:\n> >\n> > > This patch allows a smart pointer of Object to be destructed in other\n> > > threads.\n> > >\n> > > There will be multiple usages of smart pointers of Object. This patch\n> > > adds the deleter function to avoid duplicated code.\n> > >\n> > > Signed-off-by: Harvey Yang \n> > > ---\n> > > include/libcamera/base/object.h | 2 ++\n> > > src/libcamera/base/object.cpp | 11 +++++++++++\n> > > 2 files changed, 13 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/base/object.h b/include/libcamera/base/object.h\n> > > index 508773cd0..c4522d480 100644\n> > > --- a/include/libcamera/base/object.h\n> > > +++ b/include/libcamera/base/object.h\n> > > @@ -24,6 +24,8 @@ class Thread;\n> > > class Object\n> > > {\n> > > public:\n> > > + static void Deleter(Object *obj);\n> >\n> > I think a functor would be preferable, see camera.cpp:Camera::create():\n> >\n> > struct Deleter {\n> > void operator()(Object *obj) const\n> > {\n> > if (Thread::current() == obj->thread())\n> > delete obj;\n> > else\n> > obj->deleteLater();\n> > }\n> > };\n> >\n> > Or is there a reason why this wouldn't work?\n> \n> True, thanks for the notice, and it works.\n> \n> I also just found that it's stated in object.cpp:\n> https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/base/object.cpp#n143\n\nAhh you're right, I didn't notice that.\n\n\n> \n> Do you think I should add this functor to avoid duplication,\n> or the developers should copy and paste the above\n> functor each time for a derived class?\n\nWell, I think having just the functor here is a bit suboptimal because generally\nit makes sense to keep the deleter and the smart pointer construction close\nto each other.\n\nSo now I am thinking about something like this:\n\n template\n static std::unique_ptr create(Args&&... args)\n {\n return std::unique_ptr(new T(std::forward(args)...));\n }\n\nAnd if you want a shared_ptr, you can simply do\n\n std::shared_ptr(Object::create(...));\n\nOr I guess you could add two functions: `make_unique` and `make_shared` instead of `create`.\n\nI don't know what your use cases are, but I believe this would likely allow\nyou to make simplifications.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> BR,\n> Harvey\n> \n> >\n> >\n> > Regards,\n> > Barnabás Pőcze\n> >\n> > > +\n> > > Object(Object *parent = nullptr);\n> > > virtual ~Object();\n> > >\n> > > diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp\n> > > index 745d2565a..2c04b99a5 100644\n> > > --- a/src/libcamera/base/object.cpp\n> > > +++ b/src/libcamera/base/object.cpp\n> > > @@ -59,6 +59,17 @@ LOG_DEFINE_CATEGORY(Object)\n> > > * \\sa Message, Signal, Thread\n> > > */\n> > >\n> > > +/**\n> > > + * \\brief A deleter function that calls Object::deleteLater\n> > > + * \\param[in] obj The object itself\n> > > + *\n> > > + * The static deleter function that's used in smart pointers.\n> > > + */\n> > > +void Object::Deleter(Object *obj)\n> > > +{\n> > > + obj->deleteLater();\n> > > +}\n> > > +\n> > > /**\n> > > * \\brief Construct an Object instance\n> > > * \\param[in] parent The object parent\n> > > --\n> > > 2.47.0.105.g07ac214952-goog\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 7FB16C3220\n\tfor ;\n\tThu, 24 Oct 2024 12:59:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9E0DD65393;\n\tThu, 24 Oct 2024 14:59:36 +0200 (CEST)","from mail-40131.protonmail.ch (mail-40131.protonmail.ch\n\t[185.70.40.131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 918CC618C1\n\tfor ;\n\tThu, 24 Oct 2024 14:59:34 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"AxweFYtB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1729774773; x=1730033973;\n\tbh=G4a8soXPnic4mJK1kKIynAC4iklb2QXM48KNFnQcuPs=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=AxweFYtBHVYFtT0EGsxcZlR1dkpd+EfeG4lcuO+xE8+w9gtftwdwJ19ZIvZGLQLMp\n\t2/HIzTMEMwdSyY0DJqLd/eyUY5W4E+nCDSX/iznekzTAJ3DfpD7PqagSho7WI8smaD\n\tsA+TN1EIlIDxIjTc/HLtdZsraH3cswX7gdoh3HFhFjID91wmrE8xYcFFSl1LcU1gXH\n\tL7hK6Xhtte5JkLkLTolSKKScwhp2sLvkfMZ6iUD0T6AAvInu96LbLLCcluGKjzudAv\n\tM8w6o/66IN0tN6GaGvvbKhwVa5PtjN6mgK5oCIgTxx8KN5s9eB1NhKcNKeOrMCk1A2\n\tK7sRG2u5+nO+w==","Date":"Thu, 24 Oct 2024 12:59:30 +0000","To":"Cheng-Hao Yang ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: Add static Object::Deleter function","Message-ID":"","In-Reply-To":"","References":"<20241023165812.868221-1-chenghaoyang@chromium.org>\n\t<6eFAprqW7U0beSinpt2WZJczlC5tgBiQdX2cqJy0yxqV93xSwZKQNONKv2N4tPq71Z1CaoU-IWCLe63wzD07PJ35xE8BAXFFJfr0_xd3nEg=@protonmail.com>\n\t","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"a21d3781a856522f6f783fdd18ea635923895b24","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31919,"web_url":"https://patchwork.libcamera.org/comment/31919/","msgid":"","date":"2024-10-25T08:55:44","subject":"Re: [PATCH] libcamera: Add static Object::Deleter function","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Barnabás,\n\nOn Thu, Oct 24, 2024 at 8:59 PM Barnabás Pőcze wrote:\n>\n> Hi\n>\n>\n> 2024. október 24., csütörtök 11:11 keltezéssel, Cheng-Hao Yang írta:\n>\n> > Hi Barnabás,\n> >\n> > On Thu, Oct 24, 2024 at 1:33 AM Barnabás Pőcze wrote:\n> > >\n> > > Hi\n> > >\n> > >\n> > > 2024. október 23., szerda 18:58 keltezéssel, Harvey Yang írta:\n> > >\n> > > > This patch allows a smart pointer of Object to be destructed in other\n> > > > threads.\n> > > >\n> > > > There will be multiple usages of smart pointers of Object. This patch\n> > > > adds the deleter function to avoid duplicated code.\n> > > >\n> > > > Signed-off-by: Harvey Yang \n> > > > ---\n> > > > include/libcamera/base/object.h | 2 ++\n> > > > src/libcamera/base/object.cpp | 11 +++++++++++\n> > > > 2 files changed, 13 insertions(+)\n> > > >\n> > > > diff --git a/include/libcamera/base/object.h b/include/libcamera/base/object.h\n> > > > index 508773cd0..c4522d480 100644\n> > > > --- a/include/libcamera/base/object.h\n> > > > +++ b/include/libcamera/base/object.h\n> > > > @@ -24,6 +24,8 @@ class Thread;\n> > > > class Object\n> > > > {\n> > > > public:\n> > > > + static void Deleter(Object *obj);\n> > >\n> > > I think a functor would be preferable, see camera.cpp:Camera::create():\n> > >\n> > > struct Deleter {\n> > > void operator()(Object *obj) const\n> > > {\n> > > if (Thread::current() == obj->thread())\n> > > delete obj;\n> > > else\n> > > obj->deleteLater();\n> > > }\n> > > };\n> > >\n> > > Or is there a reason why this wouldn't work?\n> >\n> > True, thanks for the notice, and it works.\n> >\n> > I also just found that it's stated in object.cpp:\n> > https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/base/object.cpp#n143\n>\n> Ahh you're right, I didn't notice that.\n>\n>\n> >\n> > Do you think I should add this functor to avoid duplication,\n> > or the developers should copy and paste the above\n> > functor each time for a derived class?\n>\n> Well, I think having just the functor here is a bit suboptimal because generally\n> it makes sense to keep the deleter and the smart pointer construction close\n> to each other.\n>\n> So now I am thinking about something like this:\n>\n> template\n> static std::unique_ptr create(Args&&... args)\n> {\n> return std::unique_ptr(new T(std::forward(args)...));\n> }\n>\n> And if you want a shared_ptr, you can simply do\n>\n> std::shared_ptr(Object::create(...));\n\nI encountered some undefined symbols with this template function,\nwhich I normally\nfixed by putting the definition in the header file instead of the source file.\nHowever, it seems that including `base/thread.h` in `base/object.h` causes other\nissues (cyclic dependencies...?).\n\n>\n> Or I guess you could add two functions: `make_unique` and `make_shared` instead of `create`.\n\nI think defining make_unique of Object would overwrite the default one, and thus\ncause some issues in other Object derived classes' usages. Let's avoid\nthis as well.\n\n>\n> I don't know what your use cases are, but I believe this would likely allow\n> you to make simplifications.\n\nMy use case currently is for std::unique_ptr [1], FYI.\n\nAs there doesn't seem to be a clean way to do this, we can also just\ndrop this patch.\nWDYT?\n\nBR,\nHarvey\n\n[1]:https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/mtkisp7/src/ipa/mtkisp7/mtkisp7.cpp;l=43\n\n>\n>\n> Regards,\n> Barnabás Pőcze\n>\n> >\n> > BR,\n> > Harvey\n> >\n> > >\n> > >\n> > > Regards,\n> > > Barnabás Pőcze\n> > >\n> > > > +\n> > > > Object(Object *parent = nullptr);\n> > > > virtual ~Object();\n> > > >\n> > > > diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp\n> > > > index 745d2565a..2c04b99a5 100644\n> > > > --- a/src/libcamera/base/object.cpp\n> > > > +++ b/src/libcamera/base/object.cpp\n> > > > @@ -59,6 +59,17 @@ LOG_DEFINE_CATEGORY(Object)\n> > > > * \\sa Message, Signal, Thread\n> > > > */\n> > > >\n> > > > +/**\n> > > > + * \\brief A deleter function that calls Object::deleteLater\n> > > > + * \\param[in] obj The object itself\n> > > > + *\n> > > > + * The static deleter function that's used in smart pointers.\n> > > > + */\n> > > > +void Object::Deleter(Object *obj)\n> > > > +{\n> > > > + obj->deleteLater();\n> > > > +}\n> > > > +\n> > > > /**\n> > > > * \\brief Construct an Object instance\n> > > > * \\param[in] parent The object parent\n> > > > --\n> > > > 2.47.0.105.g07ac214952-goog\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 2FE2EC3213\n\tfor ;\n\tFri, 25 Oct 2024 08:55:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4931965394;\n\tFri, 25 Oct 2024 10:55:58 +0200 (CEST)","from mail-lj1-x235.google.com (mail-lj1-x235.google.com\n\t[IPv6:2a00:1450:4864:20::235])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 28FB86538A\n\tfor ;\n\tFri, 25 Oct 2024 10:55:56 +0200 (CEST)","by mail-lj1-x235.google.com with SMTP id\n\t38308e7fff4ca-2fb559b0b00so15751901fa.0\n\tfor ;\n\tFri, 25 Oct 2024 01:55:56 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"Rm7sd2rE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1729846555; x=1730451355;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=bHzgjgE/3rhkqDd9w6swTy9zHn1FCKNXVj7gTjsJuCE=;\n\tb=Rm7sd2rE1CtYJ9oMJlZFlCsrU8I/oZWl6agpeYpE/JCYcAXk4PO7vswowO7/ZPwIaU\n\t/1XeMSzAczG15DjsHY3Wf3WseT7nnVTJeL1uHaZO1pt4P1CwUCQWb227Ej86sJAkoDo2\n\tiN8efqlj3TysE6gkZgtfBjMnqCI7UL+Ze41JU=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1729846555; x=1730451355;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=bHzgjgE/3rhkqDd9w6swTy9zHn1FCKNXVj7gTjsJuCE=;\n\tb=TY6wJUZrcqyPmIG5O8zfyWMQz5wd4025nI374cOgeco20hLa3BSu/pn6sPWMqaCvgg\n\tX9a8nRhr5QGEPvTOjQp6nwgGKWwQqnzQg+OTZ2fefEDHJrep/AxEMGCekYIG8Afsm5ui\n\tVnTXvj/bPK/j+8DJE846MKBK+JnXOJlZbeP8rLeIlan9Z7b9ok7c39eBXKBZ2RVpfPa/\n\tx8REbK7MboHfQvGPHy9cVBlP2PjNXFlJMCMd1kLo91yGuX+RMP5YoXqslPBaVY9eo4vh\n\tRm+MXdO868xyDZLSbCK9BPrgtqL3GPcIFh9UonOURjApjJUQVYO3gC5n4GAiYVjDEfmg\n\tlWeQ==","X-Gm-Message-State":"AOJu0YxSloQX9wEGt3gS/yrwfzCkRML7tUZs6sShhKfY/zAFRP1VHU11\n\timu1jDEUYPg2IWPUcPwHqUoR9tCVSz9AnxpT5se4iT1P3ZC9zLjNYg3rxoToJcAErefAY9xuxaF\n\tbigEwZ5xc8uVLq/zJGvwIPc9MgOxE8iKle7JYDY5NuwyyQo1+xA==","X-Google-Smtp-Source":"AGHT+IE7VxRfa6mfiP4IQM6EmKBZtVh4VpptGVm61HDXd7739+JH+D1pcflC6ij/oR/49myiAkOcE3fjtHESFsU0gYY=","X-Received":"by 2002:a2e:9c46:0:b0:2fb:3d91:f218 with SMTP id\n\t38308e7fff4ca-2fc9d376f10mr38309211fa.26.1729846555189;\n\tFri, 25 Oct 2024 01:55:55 -0700 (PDT)","MIME-Version":"1.0","References":"<20241023165812.868221-1-chenghaoyang@chromium.org>\n\t<6eFAprqW7U0beSinpt2WZJczlC5tgBiQdX2cqJy0yxqV93xSwZKQNONKv2N4tPq71Z1CaoU-IWCLe63wzD07PJ35xE8BAXFFJfr0_xd3nEg=@protonmail.com>\n\t\n\t","In-Reply-To":"","From":"Cheng-Hao Yang ","Date":"Fri, 25 Oct 2024 16:55:44 +0800","Message-ID":"","Subject":"Re: [PATCH] libcamera: Add static Object::Deleter function","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]