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

[Laurent Pinchart]

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 ?

-- 
Regards,

Laurent Pinchart