[{"id":15041,"web_url":"https://patchwork.libcamera.org/comment/15041/","msgid":"","date":"2021-02-07T14:21:57","subject":"Re: [libcamera-devel] [PATCH 7/7] ipa: raspberrypi: Remove atomic\n\tvariable from Algorithm class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nThank you for the patch.\n\nOn Thu, Feb 04, 2021 at 09:34:57AM +0000, David Plowman wrote:\n> Pause() and Resume() are only called synchronously so paused_ does not\n> need to be atomic.\n> \n> With this commit, libatomic can finally be removed as a build\n> dependency.\n> \n> Signed-off-by: David Plowman \n> ---\n> src/ipa/raspberrypi/controller/algorithm.hpp | 3 +--\n> src/ipa/raspberrypi/meson.build | 1 -\n> 2 files changed, 1 insertion(+), 3 deletions(-)\n> \n> diff --git a/src/ipa/raspberrypi/controller/algorithm.hpp b/src/ipa/raspberrypi/controller/algorithm.hpp\n> index e9b040c7..5123c87b 100644\n> --- a/src/ipa/raspberrypi/controller/algorithm.hpp\n> +++ b/src/ipa/raspberrypi/controller/algorithm.hpp\n> @@ -12,7 +12,6 @@\n> #include \n> #include \n> #include \n> -#include \n> \n> #include \"controller.hpp\"\n> \n> @@ -46,7 +45,7 @@ public:\n> \n> private:\n> \tController *controller_;\n> -\tstd::atomic paused_;\n> +\tbool paused_;\n> };\n> \n> // This code is for automatic registration of Front End algorithms with the\n\nThis part is fine.\n\n> diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build\n> index 9445cd09..4cdd0434 100644\n> --- a/src/ipa/raspberrypi/meson.build\n> +++ b/src/ipa/raspberrypi/meson.build\n> @@ -5,7 +5,6 @@ ipa_name = 'ipa_rpi'\n> rpi_ipa_deps = [\n> libcamera_dep,\n> dependency('boost'),\n> - libatomic,\n> ]\n> \n> rpi_ipa_includes = [\n\nBut here, I wonder if we may need to keep libatomic. It provides C\nfunctions that are used under the hood by the compiler to implement\natomic operations. Those are used by the C++ std::atomic implementation,\nbut they could also be used internally by std::thread or related locking\nclasses.\n\nI'm not an expert on this topic, so I don't know what the right solution\nis. Should we split this patch in two to merge the removal of\nstd::atomic from algorithm.hpp while we try to figure out whether we\nshould still keep linking with libatomic ?","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 86044BD162\n\tfor ;\n\tSun, 7 Feb 2021 14:22:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C9940601B5;\n\tSun, 7 Feb 2021 15:22:21 +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 A8AB3601AB\n\tfor ;\n\tSun, 7 Feb 2021 15:22:20 +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 210C0F3;\n\tSun, 7 Feb 2021 15:22:20 +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=\"ZOicnCAV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1612707740;\n\tbh=1wU9Q11j53p77U9eyVR6bbwRiA1qK9bmPOO0oYK0TT8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZOicnCAVWq5XF9r2fAi67L0LxOrpMFEeKMdjyxB9DxNFeLoxl3yj92dAhldpdqNIM\n\tdPIhPL0PzOKtX2z+0eapKTzh2hw9imKXKyerdmUsf07PacKDeSyc0hkxNeArCq9NHt\n\t3SZbgaIq+td7yDXDY4VaonI44qSYK65wa/G1aVVU=","Date":"Sun, 7 Feb 2021 16:21:57 +0200","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"","References":"<20210204093457.6879-1-david.plowman@raspberrypi.com>\n\t<20210204093457.6879-8-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210204093457.6879-8-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 7/7] ipa: raspberrypi: Remove atomic\n\tvariable from Algorithm class","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":15043,"web_url":"https://patchwork.libcamera.org/comment/15043/","msgid":"","date":"2021-02-07T18:08:45","subject":"Re: [libcamera-devel] [PATCH 7/7] ipa: raspberrypi: Remove atomic\n\tvariable from Algorithm class","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Laurent\n\nThanks for the comments.\n\nOn Sun, 7 Feb 2021 at 14:22, Laurent Pinchart\n wrote:\n>\n> Hi David,\n>\n> Thank you for the patch.\n>\n> On Thu, Feb 04, 2021 at 09:34:57AM +0000, David Plowman wrote:\n> > Pause() and Resume() are only called synchronously so paused_ does not\n> > need to be atomic.\n> >\n> > With this commit, libatomic can finally be removed as a build\n> > dependency.\n> >\n> > Signed-off-by: David Plowman \n> > ---\n> > src/ipa/raspberrypi/controller/algorithm.hpp | 3 +--\n> > src/ipa/raspberrypi/meson.build | 1 -\n> > 2 files changed, 1 insertion(+), 3 deletions(-)\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/algorithm.hpp b/src/ipa/raspberrypi/controller/algorithm.hpp\n> > index e9b040c7..5123c87b 100644\n> > --- a/src/ipa/raspberrypi/controller/algorithm.hpp\n> > +++ b/src/ipa/raspberrypi/controller/algorithm.hpp\n> > @@ -12,7 +12,6 @@\n> > #include \n> > #include \n> > #include \n> > -#include \n> >\n> > #include \"controller.hpp\"\n> >\n> > @@ -46,7 +45,7 @@ public:\n> >\n> > private:\n> > Controller *controller_;\n> > - std::atomic paused_;\n> > + bool paused_;\n> > };\n> >\n> > // This code is for automatic registration of Front End algorithms with the\n>\n> This part is fine.\n>\n> > diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build\n> > index 9445cd09..4cdd0434 100644\n> > --- a/src/ipa/raspberrypi/meson.build\n> > +++ b/src/ipa/raspberrypi/meson.build\n> > @@ -5,7 +5,6 @@ ipa_name = 'ipa_rpi'\n> > rpi_ipa_deps = [\n> > libcamera_dep,\n> > dependency('boost'),\n> > - libatomic,\n> > ]\n> >\n> > rpi_ipa_includes = [\n>\n> But here, I wonder if we may need to keep libatomic. It provides C\n> functions that are used under the hood by the compiler to implement\n> atomic operations. Those are used by the C++ std::atomic implementation,\n> but they could also be used internally by std::thread or related locking\n> classes.\n>\n> I'm not an expert on this topic, so I don't know what the right solution\n> is. Should we split this patch in two to merge the removal of\n> std::atomic from algorithm.hpp while we try to figure out whether we\n> should still keep linking with libatomic ?\n\nYes, let me turn that one patch into a pair so that we can consider what's best.\n\nThanks\nDavid\n\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 19FABBD160\n\tfor ;\n\tSun, 7 Feb 2021 18:08:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 869E160D0D;\n\tSun, 7 Feb 2021 19:08:57 +0100 (CET)","from mail-ot1-x329.google.com (mail-ot1-x329.google.com\n\t[IPv6:2607:f8b0:4864:20::329])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 56C01601AB\n\tfor ;\n\tSun, 7 Feb 2021 19:08:56 +0100 (CET)","by mail-ot1-x329.google.com with SMTP id q4so2433613otm.9\n\tfor ;\n\tSun, 07 Feb 2021 10:08:56 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"U+mN6As6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=EpmBnT04tC4RCLMGnY1JSUhEISngcPEWMg8bdjQh3LE=;\n\tb=U+mN6As6s2CyXZ2laqzzqWaJ/J1ZeheNOTalPrab7k4WWWqMMztqkcmXlwQAmzpAUg\n\tGLMZmaeB49XNOzFVxcUFKO7YG7lBhvcFXT1Mq1/Hogx5FJvoqzok8qjSdFm2jBjyU4d4\n\td3TzxxGXtAwdRy024Mordv/Udg+XtThbj5ipzj8mn5Fzo0mevN80EMsEiq+KVLzdfiWN\n\tRNvj/hvmSzVvThdfK5evjt8JPcRKB6Rvl94IS01nXDcrflgwB4MTPO6W4uBALDpOVNM9\n\t3D6avvLD3YZw4pWerWPzZGB3qKaqBNnSbdXIWWfqW6NyxmEV+67DMJEEWEDGVxbUtPL5\n\tkNrQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=EpmBnT04tC4RCLMGnY1JSUhEISngcPEWMg8bdjQh3LE=;\n\tb=m70xwJGO9Vq/g+U9+mMSvyA/eqyWpOEokJpLY/vuYjBYOKdwu/U+UE/A2T4cXHLqRG\n\tuy3wwnN79bYCSVWcnRkJaCqtR+IScRseEe6mlQmn1vPn5lJKM0YJtOSqitPZtSmu+Arn\n\tMXGOEKOPOm3zVIWj0HFhPfje2fzPud7SlpZZv0FM89KoAglPVKUYWBb7BUfmfTM7bCWd\n\txkS8aPhL96r+b3LcQEfZQTXMmdVGGs+sahNUAgkwT5WS7mRLddX9Qp0DmhlvcTzuqMm3\n\tL9mgmzvVSdldIAMpE9WruLTfgwcF2nB8KOzXqoe+pyspNY8wVp73lGn6FcBzF0b0Xuxv\n\t+0BQ==","X-Gm-Message-State":"AOAM531oi4+vnViecE4VCdfDByA/eBE0UfsQFxkpYuM1W1NnDF0elifp\n\tVmYEgUJywZuPZYJif9FzhaA/MX8gndrCYIAaHTjyCBBCmODBPQ==","X-Google-Smtp-Source":"ABdhPJz+7J0vk+hys1Zov5AGmF20ZHFrB0LYLW/GZPU9eld5+zdNOLLLqe+NqYWLYAG5FoGNsF6oGWWnXeanEOKigu4=","X-Received":"by 2002:a9d:6852:: with SMTP id\n\tc18mr10341506oto.166.1612721335224; \n\tSun, 07 Feb 2021 10:08:55 -0800 (PST)","MIME-Version":"1.0","References":"<20210204093457.6879-1-david.plowman@raspberrypi.com>\n\t<20210204093457.6879-8-david.plowman@raspberrypi.com>\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Sun, 7 Feb 2021 18:08:45 +0000","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH 7/7] ipa: raspberrypi: Remove atomic\n\tvariable from Algorithm class","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 ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]