[PATCH v1] meson: Don't override pipeline list when `auto` is selected

Laurent Pinchart laurent.pinchart at ideasonboard.com
Tue Dec 17 17:22:12 CET 2024


Hi Barnabás,

Thank you for the patch.

On Tue, Dec 17, 2024 at 03:15:16PM +0000, Barnabás Pőcze wrote:
> Consider `pipelines=auto,virtual`. Previously that would select
> everything that `auto` would, but not actually enable the `virtual`
> pipeline handler because the `pipelines` list was reset.

Use the imperative mood in the commit message, and the present tense to
describe the current behaviour.:

Consider the `pipelines=auto,virtual` meson option. One could expect it
to auto-select pipeline handlers and to enable the virtual pipeline
handler in addition. libcamera however ignores the `virtual` pipeline
handler because the `pipelines` list is reset by `auto`. As enabling
additional pipeline handlers beside auto-selection, fix this by
considering all pipeline handlers in the list.

> Bug: https://bugs.libcamera.org/show_bug.cgi?id=247
> Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>
> ---
>  meson.build | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 33afbb741..5e5e4756d 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -204,7 +204,7 @@ liblttng = dependency('lttng-ust', required : get_option('tracing'))
>  
>  # Pipeline handlers
>  #
> -pipelines = get_option('pipelines')
> +wanted_pipelines = get_option('pipelines')
>  
>  arch_arm = ['arm', 'aarch64']
>  arch_x86 = ['x86', 'x86_64']
> @@ -220,16 +220,18 @@ pipelines_support = {
>      'virtual':      ['test'],
>  }
>  
> -if pipelines.contains('all')
> +if wanted_pipelines.contains('all')
>      pipelines = pipelines_support.keys()
> -elif pipelines.contains('auto')
> +elif wanted_pipelines.contains('auto')
>      host_cpu = host_machine.cpu_family()
>      pipelines = []
>      foreach pipeline, archs : pipelines_support
> -        if host_cpu in archs or 'any' in archs
> +        if pipeline in wanted_pipelines or host_cpu in archs or 'any' in archs
>              pipelines += pipeline
>          endif
>      endforeach
> +else
> +    pipelines = wanted_pipelines
>  endif

Would the following be clearer ?

pipelines = {}

foreach pipeline : get_option('pipelines')
    if pipeline == 'all'
        pipelines += pipelines_support
    elif pipeline == 'auto'
        host_cpu = host_machine.cpu_family()
        foreach pipeline, archs : pipelines_support
            if host_cpu in archs or 'any' in archs
                pipelines += {pipeline: true}
            endif
        endforeach
    else
        pipelines += {pipeline: true}
    endif
endforeach

pipelines = pipelines.keys()

I don't have a strong preference. Either way,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  # Tests require the vimc pipeline handler, include it automatically when tests

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list