[libcamera-devel] [PATCH 7/7] ipa: raspberrypi: Remove atomic variable from Algorithm class

[David Plowman]

Hi Laurent

Thanks for the comments.

On Sun, 7 Feb 2021 at 14:22, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi David,
>
> Thank you for the patch.
>
> On Thu, Feb 04, 2021 at 09:34:57AM +0000, David Plowman wrote:
> > Pause() and Resume() are only called synchronously so paused_ does not
> > need to be atomic.
> >
> > With this commit, libatomic can finally be removed as a build
> > dependency.
> >
> > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> >  src/ipa/raspberrypi/controller/algorithm.hpp | 3 +--
> >  src/ipa/raspberrypi/meson.build              | 1 -
> >  2 files changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/controller/algorithm.hpp b/src/ipa/raspberrypi/controller/algorithm.hpp
> > index e9b040c7..5123c87b 100644
> > --- a/src/ipa/raspberrypi/controller/algorithm.hpp
> > +++ b/src/ipa/raspberrypi/controller/algorithm.hpp
> > @@ -12,7 +12,6 @@
> >  #include <string>
> >  #include <memory>
> >  #include <map>
> > -#include <atomic>
> >
> >  #include "controller.hpp"
> >
> > @@ -46,7 +45,7 @@ public:
> >
> >  private:
> >       Controller *controller_;
> > -     std::atomic<bool> paused_;
> > +     bool paused_;
> >  };
> >
> >  // This code is for automatic registration of Front End algorithms with the
>
> This part is fine.
>
> > diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> > index 9445cd09..4cdd0434 100644
> > --- a/src/ipa/raspberrypi/meson.build
> > +++ b/src/ipa/raspberrypi/meson.build
> > @@ -5,7 +5,6 @@ ipa_name = 'ipa_rpi'
> >  rpi_ipa_deps = [
> >      libcamera_dep,
> >      dependency('boost'),
> > -    libatomic,
> >  ]
> >
> >  rpi_ipa_includes = [
>
> But here, I wonder if we may need to keep libatomic. It provides C
> functions that are used under the hood by the compiler to implement
> atomic operations. Those are used by the C++ std::atomic implementation,
> but they could also be used internally by std::thread or related locking
> classes.
>
> I'm not an expert on this topic, so I don't know what the right solution
> is. Should we split this patch in two to merge the removal of
> std::atomic from algorithm.hpp while we try to figure out whether we
> should still keep linking with libatomic ?

Yes, let me turn that one patch into a pair so that we can consider what's best.

Thanks
David

>
> --
> Regards,
>
> Laurent Pinchart