| Message ID | 20221228225710.2357657-1-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thank you for the patch. On Wed, Dec 28, 2022 at 04:57:10PM -0600, Paul Elder via libcamera-devel wrote: > Add an 'all' choice to the pipelines option, for building all pipelines. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > This patch depends on "meson: options: Fix typo in pipelines > documentation" [1] > > [1] https://patchwork.libcamera.org/patch/18076/ > > This way we don't have to update buildbot scripts every time we add a > new pipeline handler (unless it doens't happen often enough that we > don't need this option). > > The pipelines need to be explicitly listed for meson to build them. > --- > meson.build | 12 ++++++++++++ > meson_options.txt | 3 ++- > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/meson.build b/meson.build > index df9099d0..4e5c417d 100644 > --- a/meson.build > +++ b/meson.build > @@ -181,6 +181,18 @@ if pipelines.contains('auto') > pipelines += ['uvcvideo'] > endif > > +if pipelines.contains('all') > + pipelines = [ > + 'imx8-isi', > + 'ipu3', > + 'raspberrypi', > + 'rkisp1', > + 'simple', > + 'uvcvideo', > + 'vimc' > + ] > +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 793cda93..2443acf3 100644 > --- a/meson_options.txt > +++ b/meson_options.txt > @@ -39,6 +39,7 @@ option('pipelines', > type : 'array', > value : ['auto'], > choices : [ > + 'all', > 'auto', > 'imx8-isi', > 'ipu3', > @@ -48,7 +49,7 @@ option('pipelines', > 'uvcvideo', > 'vimc' > ], > - description : 'Select which pipeline handlers to build. If this is set to auto, all the pipelines applicable to the target architecture will be built.') > + description : 'Select which pipeline handlers to build. If this is set to auto, all the pipelines applicable to the target architecture will be built. If this is set to all, all the pipelines will be built. ') I'd write "auto" and "all" with quotes (I'll let you decide if single, double or back quotes are best). What if the user sets both "all" and "auto" ? I don't think that's a valid use case, but I'd document whatever behaviour you think is best at least in the commit message and possible here. Conditionally-Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > option('qcam', > type : 'feature',
Hi, On 12/29/22 5:32 AM, Laurent Pinchart via libcamera-devel wrote: > Hi Paul, > > Thank you for the patch. > > On Wed, Dec 28, 2022 at 04:57:10PM -0600, Paul Elder via libcamera-devel wrote: >> Add an 'all' choice to the pipelines option, for building all pipelines. >> >> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> >> >> --- >> This patch depends on "meson: options: Fix typo in pipelines >> documentation" [1] >> >> [1] https://patchwork.libcamera.org/patch/18076/ >> >> This way we don't have to update buildbot scripts every time we add a >> new pipeline handler (unless it doens't happen often enough that we >> don't need this option). >> >> The pipelines need to be explicitly listed for meson to build them. >> --- >> meson.build | 12 ++++++++++++ >> meson_options.txt | 3 ++- >> 2 files changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/meson.build b/meson.build >> index df9099d0..4e5c417d 100644 >> --- a/meson.build >> +++ b/meson.build >> @@ -181,6 +181,18 @@ if pipelines.contains('auto') >> pipelines += ['uvcvideo'] >> endif >> >> +if pipelines.contains('all') >> + pipelines = [ >> + 'imx8-isi', >> + 'ipu3', >> + 'raspberrypi', >> + 'rkisp1', >> + 'simple', >> + 'uvcvideo', >> + 'vimc' >> + ] >> +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 793cda93..2443acf3 100644 >> --- a/meson_options.txt >> +++ b/meson_options.txt >> @@ -39,6 +39,7 @@ option('pipelines', >> type : 'array', >> value : ['auto'], >> choices : [ >> + 'all', >> 'auto', >> 'imx8-isi', >> 'ipu3', >> @@ -48,7 +49,7 @@ option('pipelines', >> 'uvcvideo', >> 'vimc' >> ], >> - description : 'Select which pipeline handlers to build. If this is set to auto, all the pipelines applicable to the target architecture will be built.') >> + description : 'Select which pipeline handlers to build. If this is set to auto, all the pipelines applicable to the target architecture will be built. If this is set to all, all the pipelines will be built. ') > I'd write "auto" and "all" with quotes (I'll let you decide if single, > double or back quotes are best). > > What if the user sets both "all" and "auto" ? I don't think that's a > valid use case, but I'd document whatever behaviour you think is best at > least in the commit message and possible here. Likewise, > > Conditionally-Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Conditionally-Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > >> >> option('qcam', >> type : 'feature',
diff --git a/meson.build b/meson.build index df9099d0..4e5c417d 100644 --- a/meson.build +++ b/meson.build @@ -181,6 +181,18 @@ if pipelines.contains('auto') pipelines += ['uvcvideo'] endif +if pipelines.contains('all') + pipelines = [ + 'imx8-isi', + 'ipu3', + 'raspberrypi', + 'rkisp1', + 'simple', + 'uvcvideo', + 'vimc' + ] +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 793cda93..2443acf3 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -39,6 +39,7 @@ option('pipelines', type : 'array', value : ['auto'], choices : [ + 'all', 'auto', 'imx8-isi', 'ipu3', @@ -48,7 +49,7 @@ option('pipelines', 'uvcvideo', 'vimc' ], - description : 'Select which pipeline handlers to build. If this is set to auto, all the pipelines applicable to the target architecture will be built.') + description : 'Select which pipeline handlers to build. If this is set to auto, all the pipelines applicable to the target architecture will be built. If this is set to all, all the pipelines will be built. ') option('qcam', type : 'feature',
Add an 'all' choice to the pipelines option, for building all pipelines. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- This patch depends on "meson: options: Fix typo in pipelines documentation" [1] [1] https://patchwork.libcamera.org/patch/18076/ This way we don't have to update buildbot scripts every time we add a new pipeline handler (unless it doens't happen often enough that we don't need this option). The pipelines need to be explicitly listed for meson to build them. --- meson.build | 12 ++++++++++++ meson_options.txt | 3 ++- 2 files changed, 14 insertions(+), 1 deletion(-)