[libcamera-devel] correctly report enabled ipa modules
diff mbox series

Message ID 20220726132051.228019-1-foss+libcamera@0leil.net
State Accepted
Commit c698473440abee7330c76d2370c3c01ecc48b8e3
Headers show
Series
  • [libcamera-devel] correctly report enabled ipa modules
Related show

Commit Message

Quentin Schulz July 26, 2022, 1:20 p.m. UTC 
From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

"ipa_modules" stores the value of the ipas meson build option. IPAs are
enabled if and only if there is an enabled pipeline for an IPA listed in
"ipa_modules" array. It is basically the intersection of pipelines and
ipa_modules array.

In order to correctly report which IPAs get enabled, let's create a new
array storing this intersection.

Cc: Quentin Schulz <foss+libcamera@0leil.net>
Reported-by: Daniel Semkowicz <dse@thaumatec.com>
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 meson.build         | 2 +-
 src/ipa/meson.build | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Kieran Bingham July 26, 2022, 2:04 p.m. UTC | #1 
Quoting Quentin Schulz via libcamera-devel (2022-07-26 14:20:51)
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> 
> "ipa_modules" stores the value of the ipas meson build option. IPAs are
> enabled if and only if there is an enabled pipeline for an IPA listed in
> "ipa_modules" array. It is basically the intersection of pipelines and
> ipa_modules array.
> 
> In order to correctly report which IPAs get enabled, let's create a new
> array storing this intersection.
> 

Oh excellent, I think I've seen this issue in the past too and ignored
it ;-)


Seems reasonable to me so:


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> Cc: Quentin Schulz <foss+libcamera@0leil.net>
> Reported-by: Daniel Semkowicz <dse@thaumatec.com>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>  meson.build         | 2 +-
>  src/ipa/meson.build | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/meson.build b/meson.build
> index 3f7a3f56..e8b81ad8 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -173,7 +173,7 @@ py_mod.find_installation('python3', modules: py_modules)
>  ## Summarise Configurations
>  summary({
>              'Enabled pipelines': pipelines,
> -            'Enabled IPA modules': ipa_modules,
> +            'Enabled IPA modules': enabled_ipa_modules,
>              'Tracing support': tracing_enabled,
>              'Android support': android_enabled,
>              'GStreamer support': gst_enabled,
> diff --git a/src/ipa/meson.build b/src/ipa/meson.build
> index e15a8a06..849bb372 100644
> --- a/src/ipa/meson.build
> +++ b/src/ipa/meson.build
> @@ -27,6 +27,7 @@ ipa_sign = files('ipa-sign.sh')
>  ipa_names = []
>  
>  ipa_modules = get_option('ipas')
> +enabled_ipa_modules = []
>  
>  # The ipa-sign-install.sh script which uses the ipa_names variable will itself
>  # prepend MESON_INSTALL_DESTDIR_PREFIX to each ipa module name, therefore we
> @@ -35,6 +36,7 @@ foreach pipeline : pipelines
>      if ipa_modules.contains(pipeline)
>          subdir(pipeline)
>          ipa_names += ipa_install_dir / ipa_name + '.so'
> +        enabled_ipa_modules += pipeline
>      endif
>  endforeach
>  
> -- 
> 2.37.1
>
Jacopo Mondi July 26, 2022, 2:40 p.m. UTC | #2 
Hi Quentin,

On Tue, Jul 26, 2022 at 03:20:51PM +0200, Quentin Schulz via libcamera-devel wrote:
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> "ipa_modules" stores the value of the ipas meson build option. IPAs are
> enabled if and only if there is an enabled pipeline for an IPA listed in
> "ipa_modules" array. It is basically the intersection of pipelines and
> ipa_modules array.
>
> In order to correctly report which IPAs get enabled, let's create a new
> array storing this intersection.

Indeed I've ignored it for quite a while too!

>
> Cc: Quentin Schulz <foss+libcamera@0leil.net>
> Reported-by: Daniel Semkowicz <dse@thaumatec.com>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>


> ---
>  meson.build         | 2 +-
>  src/ipa/meson.build | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/meson.build b/meson.build
> index 3f7a3f56..e8b81ad8 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -173,7 +173,7 @@ py_mod.find_installation('python3', modules: py_modules)
>  ## Summarise Configurations
>  summary({
>              'Enabled pipelines': pipelines,
> -            'Enabled IPA modules': ipa_modules,
> +            'Enabled IPA modules': enabled_ipa_modules,
>              'Tracing support': tracing_enabled,
>              'Android support': android_enabled,
>              'GStreamer support': gst_enabled,
> diff --git a/src/ipa/meson.build b/src/ipa/meson.build
> index e15a8a06..849bb372 100644
> --- a/src/ipa/meson.build
> +++ b/src/ipa/meson.build
> @@ -27,6 +27,7 @@ ipa_sign = files('ipa-sign.sh')
>  ipa_names = []
>
>  ipa_modules = get_option('ipas')
> +enabled_ipa_modules = []
>
>  # The ipa-sign-install.sh script which uses the ipa_names variable will itself
>  # prepend MESON_INSTALL_DESTDIR_PREFIX to each ipa module name, therefore we
> @@ -35,6 +36,7 @@ foreach pipeline : pipelines
>      if ipa_modules.contains(pipeline)
>          subdir(pipeline)
>          ipa_names += ipa_install_dir / ipa_name + '.so'
> +        enabled_ipa_modules += pipeline
>      endif
>  endforeach
>
> --
> 2.37.1
>
Jacopo Mondi July 26, 2022, 3:10 p.m. UTC | #3 
Hello

Pushed with the subject line changed to

libcamera: Correctly report enabled ipa modules

On Tue, Jul 26, 2022 at 03:20:51PM +0200, Quentin Schulz via libcamera-devel wrote:
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> "ipa_modules" stores the value of the ipas meson build option. IPAs are
> enabled if and only if there is an enabled pipeline for an IPA listed in
> "ipa_modules" array. It is basically the intersection of pipelines and
> ipa_modules array.
>
> In order to correctly report which IPAs get enabled, let's create a new
> array storing this intersection.
>
> Cc: Quentin Schulz <foss+libcamera@0leil.net>
> Reported-by: Daniel Semkowicz <dse@thaumatec.com>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>  meson.build         | 2 +-
>  src/ipa/meson.build | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/meson.build b/meson.build
> index 3f7a3f56..e8b81ad8 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -173,7 +173,7 @@ py_mod.find_installation('python3', modules: py_modules)
>  ## Summarise Configurations
>  summary({
>              'Enabled pipelines': pipelines,
> -            'Enabled IPA modules': ipa_modules,
> +            'Enabled IPA modules': enabled_ipa_modules,
>              'Tracing support': tracing_enabled,
>              'Android support': android_enabled,
>              'GStreamer support': gst_enabled,
> diff --git a/src/ipa/meson.build b/src/ipa/meson.build
> index e15a8a06..849bb372 100644
> --- a/src/ipa/meson.build
> +++ b/src/ipa/meson.build
> @@ -27,6 +27,7 @@ ipa_sign = files('ipa-sign.sh')
>  ipa_names = []
>
>  ipa_modules = get_option('ipas')
> +enabled_ipa_modules = []
>
>  # The ipa-sign-install.sh script which uses the ipa_names variable will itself
>  # prepend MESON_INSTALL_DESTDIR_PREFIX to each ipa module name, therefore we
> @@ -35,6 +36,7 @@ foreach pipeline : pipelines
>      if ipa_modules.contains(pipeline)
>          subdir(pipeline)
>          ipa_names += ipa_install_dir / ipa_name + '.so'
> +        enabled_ipa_modules += pipeline
>      endif
>  endforeach
>
> --
> 2.37.1
>

Patch
diff mbox series 

diff --git a/meson.build b/meson.build
index 3f7a3f56..e8b81ad8 100644
--- a/meson.build
+++ b/meson.build
@@ -173,7 +173,7 @@  py_mod.find_installation('python3', modules: py_modules)
 ## Summarise Configurations
 summary({
             'Enabled pipelines': pipelines,
-            'Enabled IPA modules': ipa_modules,
+            'Enabled IPA modules': enabled_ipa_modules,
             'Tracing support': tracing_enabled,
             'Android support': android_enabled,
             'GStreamer support': gst_enabled,
diff --git a/src/ipa/meson.build b/src/ipa/meson.build
index e15a8a06..849bb372 100644
--- a/src/ipa/meson.build
+++ b/src/ipa/meson.build
@@ -27,6 +27,7 @@  ipa_sign = files('ipa-sign.sh')
 ipa_names = []
 
 ipa_modules = get_option('ipas')
+enabled_ipa_modules = []
 
 # The ipa-sign-install.sh script which uses the ipa_names variable will itself
 # prepend MESON_INSTALL_DESTDIR_PREFIX to each ipa module name, therefore we
@@ -35,6 +36,7 @@  foreach pipeline : pipelines
     if ipa_modules.contains(pipeline)
         subdir(pipeline)
         ipa_names += ipa_install_dir / ipa_name + '.so'
+        enabled_ipa_modules += pipeline
     endif
 endforeach