[libcamera-devel] [PATCH] meson: Only build pipeline handlers needed in the host architecture

[Javier Martinez Canillas]

Hello Laurent,

Thanks a lot for your feedback.

On 12/24/22 02:01, Laurent Pinchart wrote:
> Hi Javier,
> 
> Thank you for the patch.
> 
> On Fri, Dec 23, 2022 at 09:22:29AM +0100, Javier Martinez Canillas wrote:
>> By default all pipeline handlers are built, regardless on whether these
>> are needed in the host architecture or not. It makes more sense to build
>> only the pipeline handlers that will be used for the given architecture.
>>
>> Let's do that by default now, but still allow to build the other pipeline
>> handlers if needed, by using the `pipelines` meson option. For example:
> 
> As long as CI systems can build all pipeline handlers, that's fine with
> me. We may start getting somes patches that introduce compilation
> breakages on pipeline handlers that are not compile-tested by
> developers, but we can catch that in CI.
>

Agreed.
 
>> +if pipelines.contains('auto')
>> +    host_cpu = host_machine.cpu_family()
>> +    if host_cpu == 'x86' or host_cpu == 'x86_64'
>> +        pipelines = ['ipu3']
>> +    elif host_cpu == 'aarch64'
>> +        pipelines = ['imx8-isi', 'ipu3', 'raspberrypi', 'rkisp1']
> 
> ipu3 is only for x86.
>

Yes, that's a left over since I didn't meant to include it for aarch64.
 
>> +    elif host_cpu == 'arm'
>> +        pipelines = ['raspberrypi']
>> +    endif
>> +
>> +    # Always include the simple and uvcvideo pipeline handlers.
>> +    pipelines += ['simple', 'uvcvideo']
> 
> The simple pipeline handler is only useful on ARM platforms (both 32 and
> 64 bits) at the moment.
> 

Ah, good to know. I'll adjust that accordingly.

>> +endif
>> +
>>  if get_option('test') and 'vimc' not in pipelines
>>      message('Enabling vimc pipeline handler to support tests')
>>      pipelines += ['vimc']
>> diff --git a/meson_options.txt b/meson_options.txt
>> index 1ba6778ce257..23505805de41 100644
>> --- a/meson_options.txt
>> +++ b/meson_options.txt
>> @@ -37,8 +37,10 @@ option('lc-compliance',
>>  
>>  option('pipelines',
>>          type : 'array',
>> -        choices : ['imx8-isi', 'ipu3', 'raspberrypi', 'rkisp1', 'simple', 'uvcvideo', 'vimc'],
>> -        description : 'Select which pipeline handlers to include')
>> +        value : ['auto'],
>> +        choices : ['auto', 'imx8-isi', 'ipu3', 'raspberrypi', 'rkisp1', 'simple', 'uvcvideo', 'vimc'],
> 
> This is getting long, let's wrap it:
>

OK.

[...]

>> +        description : '''Select which pipeline handlers to build. If this is set to auto, all
>> +                       the pipelines applicable to the target architecture will be built.''')
> 
> The spaces in the description cause weird-looking output depending on
> the terminal width:
> 
>   pipelines                     [imx8-isi, ipu3, raspberrypi,   [auto, imx8-isi, ipu3,          Select which pipeline handlers to build. If this is set to auto,
>                                 rkisp1, simple, uvcvideo, vimc] raspberrypi, rkisp1, simple,    all                        the pipelines applicable to the
>                                                                 uvcvideo,                       target architecture will be built.
>                                                                  vimc]
> 
> You can write
> 
>         description : '''Select which pipeline handlers to build. If this is set
> to auto, all the pipelines applicable to the target architecture will be built.''')
>

Right. I'll do that. Thanks!
 -- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat