[libcamera-devel] [PATCH v2 2/6] controls: Update argument handling for controls generation scripts
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Nov 23 15:08:28 CET 2023
Hi Naush,
On Thu, Nov 23, 2023 at 01:26:42PM +0000, Naushir Patuck wrote:
> On Thu, 23 Nov 2023 at 09:57, Laurent Pinchart wrote:
> > On Tue, Nov 21, 2023 at 02:53:46PM +0000, Naushir Patuck via libcamera-devel wrote:
> > > The template file to the gen-controls.py and gen-py-controls.py is now
> > > passed in through the '-t' command line argument instead of being a
> > > positional argument. This will allow multiple input files to be
> > > provided to the scripts in a future commit.
> > >
> > > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > > ---
> > > include/libcamera/meson.build | 7 ++++---
> > > src/libcamera/meson.build | 7 ++++---
> > > src/py/libcamera/gen-py-controls.py | 2 +-
> > > src/py/libcamera/meson.build | 18 ++++++++----------
> > > utils/gen-controls.py | 2 +-
> > > 5 files changed, 18 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> > > index 2c8c0258c95e..5fb772e6dd14 100644
> > > --- a/include/libcamera/meson.build
> > > +++ b/include/libcamera/meson.build
> > > @@ -41,12 +41,13 @@ control_source_files = {
> > > control_headers = []
> > >
> > > foreach header, mode : control_source_files
> > > - input_files = files('../../src/libcamera/' + header +'.yaml', header + '.h.in')
> > > + input_files = files('../../src/libcamera/' + header +'.yaml')
> > > + template_file = files(header + '.h.in')
> > > control_headers += custom_target(header + '_h',
> > > input : input_files,
> > > output : header + '.h',
> > > - command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',
> > > - '--mode', mode],
> > > + command : [gen_controls, '-o', '@OUTPUT@',
> > > + '--mode', mode, '-t', template_file, '@INPUT@'],
> > > install : true,
> > > install_dir : libcamera_headers_install_dir)
> > > endforeach
> > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > > index e49bf850b355..05ee38daf22b 100644
> > > --- a/src/libcamera/meson.build
> > > +++ b/src/libcamera/meson.build
> > > @@ -128,12 +128,13 @@ endif
> > > control_sources = []
> > >
> > > foreach source, mode : control_source_files
> > > - input_files = files(source +'.yaml', source + '.cpp.in')
> > > + input_files = files(source +'.yaml')
> > > + template_file = files(source + '.cpp.in')
> >
> > Hmmm... I think we have a problem here. The custom_target() input
> > argument is used to track dependencies, and cause the target to be
> > rebuild if any of the dependencies has changed. Passing the template
> > file through '-t' is fine, but dropping it from the inputs means that
> > dependencies won't be tracked properly. You can test this by modifying
> > the template file (without touching anything else) and seeing if all the
> > source files get rebuilt.
>
> This seems logical, but surprisingly it works! If do a full build,
> make an edit to any .in template file and rebuild, the output does get
> regenerated. Could it be working because I wrap the template in a
> file() construct?
Indeed, meson seems to handle this automatically. It's magic :-) Please
ignore my comment.
> > > control_sources += custom_target(source + '_cpp',
> > > input : input_files,
> > > output : source + '.cpp',
> > > - command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',
> > > - '--mode', mode])
> > > + command : [gen_controls, '-o', '@OUTPUT@',
> > > + '--mode', mode, '-t', template_file, '@INPUT@'])
> > > endforeach
> > >
> > > libcamera_sources += control_sources
> > > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py
> > > index 8f14cdafe313..ea28f0139f23 100755
> > > --- a/src/py/libcamera/gen-py-controls.py
> > > +++ b/src/py/libcamera/gen-py-controls.py
> > > @@ -93,7 +93,7 @@ def main(argv):
> > > help='Output file name. Defaults to standard output if not specified.')
> > > parser.add_argument('input', type=str,
> > > help='Input file name.')
> > > - parser.add_argument('template', type=str,
> > > + parser.add_argument('-t', dest='template', type=str, required=True,
> > > help='Template file name.')
> >
> > While at it, could you make the arguments sorted alphabetically ? As we
> > have '--mode', I would make the template argument
> >
> > parser.add_argument('--template', '-', dest='template', type=str,
> > required=True, help='Template file name')
> >
> > and change '--mode' to '--mode', '-m' for consistency.
> >
> > A "while at it" comment in the commit message is fine with me, unless
> > you insist splitting it in a separate patch :-)
> >
> > > parser.add_argument('--mode', type=str, required=True,
> > > help='Mode is either "controls" or "properties"')
> > > diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
> > > index f58c7198ee9e..1c3ea1843ac0 100644
> > > --- a/src/py/libcamera/meson.build
> > > +++ b/src/py/libcamera/meson.build
> > > @@ -28,29 +28,27 @@ pycamera_sources = files([
> > >
> > > # Generate controls
> > >
> > > -gen_py_controls_input_files = files([
> > > - '../../libcamera/control_ids.yaml',
> > > - 'py_controls_generated.cpp.in',
> > > -])
> > > +gen_py_controls_input_files = files('../../libcamera/control_ids.yaml')
> > > +gen_py_controls_template = files('py_controls_generated.cpp.in')
> > >
> > > gen_py_controls = files('gen-py-controls.py')
> > >
> > > pycamera_sources += custom_target('py_gen_controls',
> > > input : gen_py_controls_input_files,
> > > output : ['py_controls_generated.cpp'],
> > > - command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@', '@INPUT@'])
> > > + command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@',
> > > + '-t', gen_py_controls_template, '@INPUT@'])
> > >
> > > # Generate properties
> > >
> > > -gen_py_property_enums_input_files = files([
> > > - '../../libcamera/property_ids.yaml',
> > > - 'py_properties_generated.cpp.in',
> > > -])
> > > +gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')
> > > +gen_py_properties_template = files('py_properties_generated.cpp.in')
> > >
> > > pycamera_sources += custom_target('py_gen_properties',
> > > input : gen_py_property_enums_input_files,
> > > output : ['py_properties_generated.cpp'],
> > > - command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@', '@INPUT@'])
> > > + command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@',
> > > + '-t', gen_py_properties_template, '@INPUT@'])
> > >
> > > # Generate formats
> > >
> > > diff --git a/utils/gen-controls.py b/utils/gen-controls.py
> > > index 8f2f8fdb02c3..8af33d29cd07 100755
> > > --- a/utils/gen-controls.py
> > > +++ b/utils/gen-controls.py
> > > @@ -361,7 +361,7 @@ def main(argv):
> > > help='Output file name. Defaults to standard output if not specified.')
> > > parser.add_argument('input', type=str,
> > > help='Input file name.')
> > > - parser.add_argument('template', type=str,
> > > + parser.add_argument('-t', dest='template', type=str, required=True,
> > > help='Template file name.')
> >
> > Same comment as above.
> >
> > > parser.add_argument('--mode', type=str, required=True, choices=['controls', 'properties'],
> > > help='Mode of operation')
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list