[{"id":28100,"web_url":"https://patchwork.libcamera.org/comment/28100/","msgid":"<20231120040953.GF524@pendragon.ideasonboard.com>","date":"2023-11-20T04:09:53","subject":"Re: [libcamera-devel] [PATCH v1 2/5] controls: build: Allow\n\tseparate vendor control YAML files","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Fri, Nov 10, 2023 at 10:59:59AM +0000, Naushir Patuck via libcamera-devel wrote:\n> Add support for using separate YAML files for vendor specific controls.\n> This simplifies management of vendor control definitions and avoids\n> possible merge conflicts when changing the control_ids.yaml file for\n> core and draft controls. The mapping of vendor/pipeline handler to\n> control file is done through the vendor_controls_mapping variable in\n> include/libcamera/meson.build.\n> \n> The command line argument handling for the gen-py-controls.py and\n> gen-controls.py have now changed to allow multiple input files to be\n> provided. This means that all positional arguments have been replaced\n> by non-positional equivalents.\n\nThis could have been done in a separate patch to ease review.\n\n> Add a new control_ids_rpi.yaml file to hold the Raspberry Pi specific\n> vendor controls. This contains a single control PispConfigDumpFile that\n> will be used in the Pi 5 pipeline handler as a trigger to dump the\n> Backend configuration as a JSON file.\n> \n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> ---\n>  include/libcamera/meson.build       | 48 ++++++++++++++++++++++++++---\n>  meson.build                         |  2 ++\n>  src/libcamera/control_ids_rpi.yaml  | 17 ++++++++++\n>  src/libcamera/meson.build           | 19 ++++++++++--\n>  src/py/libcamera/gen-py-controls.py | 12 +++++---\n>  src/py/libcamera/meson.build        | 26 ++++++++++------\n>  utils/gen-controls.py               | 15 +++++----\n>  7 files changed, 111 insertions(+), 28 deletions(-)\n>  create mode 100644 src/libcamera/control_ids_rpi.yaml\n> \n> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> index f736cca07228..f8af51174f08 100644\n> --- a/include/libcamera/meson.build\n> +++ b/include/libcamera/meson.build\n> @@ -32,21 +32,61 @@ install_headers(libcamera_public_headers,\n>  \n>  libcamera_headers_install_dir = get_option('includedir') / libcamera_include_dir\n>  \n> -# control_ids.h and property_ids.h\n> +# control_ids.h and property_ids.h and associated modes\n\nI think this belongs to the previous patch.\n\n>  control_source_files = {\n>      'control_ids': 'controls',\n>      'property_ids': 'properties',\n>  }\n>  \n> +vendor_mappings = {\n\nThe commit message names this vendor_controls_mapping, which I think is\na better name as it's related to controls.\n\n> +    # Mapping of vendor (pipeline handler) specific controls files\n> +    'controls':\n> +    {\n> +        'rpi/pisp': 'control_ids_rpi.yaml',\n> +        'rpi/vc4': 'control_ids_rpi.yaml'\n> +    },\n> +    # Mapping of vendor (pipeline handler) specific properties files\n> +    'properties':\n> +    {\n> +\n> +    }\n> +}\n> +\n>  control_headers = []\n> +vendor_ctrl_files = []\n> +vendor_prop_files = []\n>  \n>  foreach header, mode : control_source_files\n> -    input_files = files('../../src/libcamera/' + header +'.yaml', header + '.h.in')\n> +    # Start by populating the vendor specific mappings into vendor_ctrl_files\n> +    # vendor_prop_files. These will be cached for later use.\n\ns/vendor_prop_files/and vendor_prop_files/\n\n> +    vendor_files = []\n> +    foreach pipeline, file : vendor_mappings[mode]\n> +        if pipeline not in pipelines\n> +            continue\n> +        endif\n> +        if file not in vendor_files\n> +            vendor_files += file\n> +        endif\n> +    endforeach\n> +\n> +    if mode == 'controls'\n> +        vendor_ctrl_files = vendor_files\n> +    else\n> +        vendor_prop_files = vendor_files\n> +    endif\n> +\n> +    input_files = files('../../src/libcamera/' + header +'.yaml')\n\nWhile at it, add the missing space before the '.yaml'.\n\n> +\n> +    foreach file : vendor_files\n> +        input_files += files('../../src/libcamera/' + file)\n> +    endforeach\n> +\n> +    template_file = files(header + '.h.in')\n>      control_headers += custom_target(header + '_h',\n>                                       input : input_files,\n>                                       output : header + '.h',\n> -                                     command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',\n> -                                                '--mode', mode],\n> +                                     command : [gen_controls, '-o', '@OUTPUT@', '-i', '@INPUT@',\n> +                                                '--mode', mode, '-t', template_file],\n>                                       install : true,\n>                                       install_dir : libcamera_headers_install_dir)\n>  endforeach\n> diff --git a/meson.build b/meson.build\n> index e9a1c7e360ce..1423abf16c77 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -267,6 +267,8 @@ py_mod.find_installation('python3', modules : py_modules)\n>  summary({\n>              'Enabled pipelines': pipelines,\n>              'Enabled IPA modules': enabled_ipa_names,\n> +            'Vendor controls': vendor_ctrl_files,\n> +            'Vendor properties': vendor_prop_files,\n>              'Hotplug support': libudev.found(),\n>              'Tracing support': tracing_enabled,\n>              'Android support': android_enabled,\n> diff --git a/src/libcamera/control_ids_rpi.yaml b/src/libcamera/control_ids_rpi.yaml\n> new file mode 100644\n> index 000000000000..239904a008a8\n> --- /dev/null\n> +++ b/src/libcamera/control_ids_rpi.yaml\n> @@ -0,0 +1,17 @@\n> +# SPDX-License-Identifier: LGPL-2.1-or-later\n> +#\n> +# Copyright (C) 2023, Raspberry Pi Ltd\n> +#\n> +%YAML 1.1\n> +---\n> +controls:\n> +\n> +  - PispConfigDumpFile:\n> +      type: string\n> +      vendor: rpi\n> +      description: |\n> +        Triggers the Raspberry Pi PiSP pipeline handler to generate a JSON\n> +        formatted dump of the Backend configuration to the filename given by the\n> +        value of the control.\n> +\n> +...\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index e49bf850b355..8891052a3316 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -128,12 +128,25 @@ endif\n>  control_sources = []\n>  \n>  foreach source, mode : control_source_files\n> -    input_files = files(source +'.yaml', source + '.cpp.in')\n> +    input_files = files(source +'.yaml')\n> +\n> +    # Add the vendor specific files to the input.\n> +    if mode == 'controls'\n> +        vendor_files = vendor_ctrl_files\n> +    else\n> +        vendor_files = vendor_prop_files\n> +    endif\n> +\n> +    foreach file : vendor_files\n> +        input_files += files(file)\n> +    endforeach\n> +\n> +    template_file = files(source + '.cpp.in')\n>      control_sources += custom_target(source + '_cpp',\n>                                       input : input_files,\n>                                       output : source + '.cpp',\n> -                                     command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',\n> -                                                '--mode', mode])\n> +                                     command : [gen_controls, '-o', '@OUTPUT@', '-i', '@INPUT@',\n\nmeson will not expand this to multiple -i arguments, I'm actually\nsurprised it works as expected. Could we keep the input files as\npositional arguments ?\n\n> +                                                '--mode', mode, '-t', template_file])\n>  endforeach\n>  \n>  libcamera_sources += control_sources\n> diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> index e89de674966a..a32ef09c2f48 100755\n> --- a/src/py/libcamera/gen-py-controls.py\n> +++ b/src/py/libcamera/gen-py-controls.py\n> @@ -91,9 +91,9 @@ def main(argv):\n>      parser = argparse.ArgumentParser()\n>      parser.add_argument('-o', dest='output', metavar='file', type=str,\n>                          help='Output file name. Defaults to standard output if not specified.')\n> -    parser.add_argument('input', type=str,\n> -                        help='Input file name.')\n> -    parser.add_argument('template', type=str,\n> +    parser.add_argument('-i', dest='inputs', type=str, nargs='+', required=True,\n> +                        help='Input file name(s).')\n> +    parser.add_argument('-t', dest='template', type=str, required=True,\n>                          help='Template file name.')\n>      parser.add_argument('--mode', type=str, required=True,\n>                          help='Mode is either \"controls\" or \"properties\"')\n> @@ -103,8 +103,10 @@ def main(argv):\n>          print(f'Invalid mode option \"{args.mode}\"', file=sys.stderr)\n>          return -1\n>  \n> -    data = open(args.input, 'rb').read()\n> -    controls = yaml.safe_load(data)['controls']\n> +    controls = []\n> +    for input in args.inputs:\n> +        data = open(input, 'rb').read()\n> +        controls += yaml.safe_load(data)['controls']\n>  \n>      data = generate_py(controls, args.mode)\n>  \n> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\n> index f58c7198ee9e..e4bc208b4ac1 100644\n> --- a/src/py/libcamera/meson.build\n> +++ b/src/py/libcamera/meson.build\n> @@ -28,29 +28,35 @@ pycamera_sources = files([\n>  \n>  # Generate controls\n>  \n> -gen_py_controls_input_files = files([\n> -    '../../libcamera/control_ids.yaml',\n> -    'py_controls_generated.cpp.in',\n> -])\n> +gen_py_controls_input_files = files('../../libcamera/control_ids.yaml')\n> +\n> +foreach file : vendor_ctrl_files\n> +    gen_py_controls_input_files += files('../../libcamera/' + file)\n> +endforeach\n>  \n>  gen_py_controls = files('gen-py-controls.py')\n> +gen_py_controls_template = files('py_controls_generated.cpp.in')\n>  \n>  pycamera_sources += custom_target('py_gen_controls',\n>                                    input : gen_py_controls_input_files,\n>                                    output : ['py_controls_generated.cpp'],\n> -                                  command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@', '@INPUT@'])\n> +                                  command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@', '-i', '@INPUT@',\n> +                                             '-t', gen_py_controls_template])\n>  \n>  # Generate properties\n>  \n> -gen_py_property_enums_input_files = files([\n> -    '../../libcamera/property_ids.yaml',\n> -    'py_properties_generated.cpp.in',\n> -])\n> +gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> +gen_py_property_template = files('py_properties_generated.cpp.in')\n> +\n> +foreach file : vendor_prop_files\n> +    gen_py_property_enums_input_files += files('../../libcamera/' + file)\n> +endforeach\n>  \n>  pycamera_sources += custom_target('py_gen_properties',\n>                                    input : gen_py_property_enums_input_files,\n>                                    output : ['py_properties_generated.cpp'],\n> -                                  command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@', '@INPUT@'])\n> +                                  command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@', '-i', '@INPUT@',\n> +                                             '-t', gen_py_property_template])\n>  \n>  # Generate formats\n>  \n> diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> index dd55753e792c..3c490a562676 100755\n> --- a/utils/gen-controls.py\n> +++ b/utils/gen-controls.py\n> @@ -362,18 +362,21 @@ def main(argv):\n>  \n>      # Parse command line arguments\n>      parser = argparse.ArgumentParser()\n> -    parser.add_argument('-o', dest='output', metavar='file', type=str,\n> +    parser.add_argument('-o', dest='output', metavar='file', type=str, required=True,\n>                          help='Output file name. Defaults to standard output if not specified.')\n> -    parser.add_argument('input', type=str,\n> -                        help='Input file name.')\n> -    parser.add_argument('template', type=str,\n> +    parser.add_argument('-i', dest='inputs', type=str, nargs='+', required=True,\n> +                        help='Input file name(s).')\n> +    parser.add_argument('-t', dest='template', type=str, required=True,\n>                          help='Template file name.')\n>      parser.add_argument('--mode', type=str, required=True, choices=['controls', 'properties'],\n>                          help='Mode of operation')\n>      args = parser.parse_args(argv[1:])\n>  \n> -    data = open(args.input, 'rb').read()\n> -    controls = yaml.safe_load(data)['controls']\n> +    controls = []\n> +    for input in args.inputs:\n> +        data = open(input, 'rb').read()\n> +        controls = controls + yaml.safe_load(data)['controls']\n> +\n>      controls = [Control(*ctrl.popitem()) for ctrl in controls]\n>  \n>      if args.template.endswith('.cpp.in'):","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DD215BDE6B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 20 Nov 2023 04:09:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2BB2B629BC;\n\tMon, 20 Nov 2023 05:09:49 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8FD6A61DA9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 Nov 2023 05:09:47 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CED5C74A;\n\tMon, 20 Nov 2023 05:09:17 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700453389;\n\tbh=KtHrOJ6nDy6dI7vlVIJ5ruac4oXbr7GZ/wuQkJ3P6sQ=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=rVJPiVakOVlU8hMXWP3FTkWdkF1QeUF5ckC7qFG3sKtj/DQ4ojT64HKTWKTib/smt\n\tCqWTCBDdSa/D8u6bNMsStUQabsoRe6cv9ZuqEeMep5H/ririWk0HJm5l00mjZsaZPg\n\tZ7TpXxkT95MEFJyjwpgtHUTwNU+8nji2PPf+RRk8pjDcO27pMbH19qYRfniyTj1533\n\tTXy4CwMiiMR6ux0cEwDUSvZz/ehLVoZ5OjZ9pgG6UN5HJbQ9Y1XdwPlud0Kz55Wuty\n\t9mDs2qkaRF9V8lAI08vWXWWiKjPh3D2vgbMY4lqcAr6tHKEulmiSWKKw3U1g517HVO\n\t2ER42qrUOd+0Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1700453358;\n\tbh=KtHrOJ6nDy6dI7vlVIJ5ruac4oXbr7GZ/wuQkJ3P6sQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VwrnROteu42qAtCE0oRUc1RdNG+u4/vYupdqgOUhe9W0dIaPNKA8dJTrzrhJxibGR\n\tLp70wl4m3KnVcoQq+YCDdBqRRFwF8hyvbN/KdZlWfgi097EJY/BeRnY12ODlyEp+9u\n\t1U2Ow4vMAGxxVRV9Pt90PeaHrnMujTp5Y+j9sOu0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"VwrnROte\"; dkim-atps=neutral","Date":"Mon, 20 Nov 2023 06:09:53 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<20231120040953.GF524@pendragon.ideasonboard.com>","References":"<20231110110002.21381-1-naush@raspberrypi.com>\n\t<20231110110002.21381-3-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20231110110002.21381-3-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v1 2/5] controls: build: Allow\n\tseparate vendor control YAML files","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28109,"web_url":"https://patchwork.libcamera.org/comment/28109/","msgid":"<CAEmqJPoY-FNpkEpwVOjJRWa8JJiQSzRfOo1hk3_Yxauw0sFwJg@mail.gmail.com>","date":"2023-11-20T15:00:52","subject":"Re: [libcamera-devel] [PATCH v1 2/5] controls: build: Allow\n\tseparate vendor control YAML files","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for your feedback.\n\nOn Mon, 20 Nov 2023 at 04:09, Laurent Pinchart\n<laurent.pinchart@ideasonboard.com> wrote:\n>\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Fri, Nov 10, 2023 at 10:59:59AM +0000, Naushir Patuck via libcamera-devel wrote:\n> > Add support for using separate YAML files for vendor specific controls.\n> > This simplifies management of vendor control definitions and avoids\n> > possible merge conflicts when changing the control_ids.yaml file for\n> > core and draft controls. The mapping of vendor/pipeline handler to\n> > control file is done through the vendor_controls_mapping variable in\n> > include/libcamera/meson.build.\n> >\n> > The command line argument handling for the gen-py-controls.py and\n> > gen-controls.py have now changed to allow multiple input files to be\n> > provided. This means that all positional arguments have been replaced\n> > by non-positional equivalents.\n>\n> This could have been done in a separate patch to ease review.\n\nI'll split this to a separate patch as best I can in the next version.\n\n>\n> > Add a new control_ids_rpi.yaml file to hold the Raspberry Pi specific\n> > vendor controls. This contains a single control PispConfigDumpFile that\n> > will be used in the Pi 5 pipeline handler as a trigger to dump the\n> > Backend configuration as a JSON file.\n> >\n> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > ---\n> >  include/libcamera/meson.build       | 48 ++++++++++++++++++++++++++---\n> >  meson.build                         |  2 ++\n> >  src/libcamera/control_ids_rpi.yaml  | 17 ++++++++++\n> >  src/libcamera/meson.build           | 19 ++++++++++--\n> >  src/py/libcamera/gen-py-controls.py | 12 +++++---\n> >  src/py/libcamera/meson.build        | 26 ++++++++++------\n> >  utils/gen-controls.py               | 15 +++++----\n> >  7 files changed, 111 insertions(+), 28 deletions(-)\n> >  create mode 100644 src/libcamera/control_ids_rpi.yaml\n> >\n> > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> > index f736cca07228..f8af51174f08 100644\n> > --- a/include/libcamera/meson.build\n> > +++ b/include/libcamera/meson.build\n> > @@ -32,21 +32,61 @@ install_headers(libcamera_public_headers,\n> >\n> >  libcamera_headers_install_dir = get_option('includedir') / libcamera_include_dir\n> >\n> > -# control_ids.h and property_ids.h\n> > +# control_ids.h and property_ids.h and associated modes\n>\n> I think this belongs to the previous patch.\n\nAck.\n\n>\n> >  control_source_files = {\n> >      'control_ids': 'controls',\n> >      'property_ids': 'properties',\n> >  }\n> >\n> > +vendor_mappings = {\n>\n> The commit message names this vendor_controls_mapping, which I think is\n> a better name as it's related to controls.\n\nAck\n\n>\n> > +    # Mapping of vendor (pipeline handler) specific controls files\n> > +    'controls':\n> > +    {\n> > +        'rpi/pisp': 'control_ids_rpi.yaml',\n> > +        'rpi/vc4': 'control_ids_rpi.yaml'\n> > +    },\n> > +    # Mapping of vendor (pipeline handler) specific properties files\n> > +    'properties':\n> > +    {\n> > +\n> > +    }\n> > +}\n> > +\n> >  control_headers = []\n> > +vendor_ctrl_files = []\n> > +vendor_prop_files = []\n> >\n> >  foreach header, mode : control_source_files\n> > -    input_files = files('../../src/libcamera/' + header +'.yaml', header + '.h.in')\n> > +    # Start by populating the vendor specific mappings into vendor_ctrl_files\n> > +    # vendor_prop_files. These will be cached for later use.\n>\n> s/vendor_prop_files/and vendor_prop_files/\n\nAck\n\n>\n> > +    vendor_files = []\n> > +    foreach pipeline, file : vendor_mappings[mode]\n> > +        if pipeline not in pipelines\n> > +            continue\n> > +        endif\n> > +        if file not in vendor_files\n> > +            vendor_files += file\n> > +        endif\n> > +    endforeach\n> > +\n> > +    if mode == 'controls'\n> > +        vendor_ctrl_files = vendor_files\n> > +    else\n> > +        vendor_prop_files = vendor_files\n> > +    endif\n> > +\n> > +    input_files = files('../../src/libcamera/' + header +'.yaml')\n>\n> While at it, add the missing space before the '.yaml'.\n\nAck\n\n>\n> > +\n> > +    foreach file : vendor_files\n> > +        input_files += files('../../src/libcamera/' + file)\n> > +    endforeach\n> > +\n> > +    template_file = files(header + '.h.in')\n> >      control_headers += custom_target(header + '_h',\n> >                                       input : input_files,\n> >                                       output : header + '.h',\n> > -                                     command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',\n> > -                                                '--mode', mode],\n> > +                                     command : [gen_controls, '-o', '@OUTPUT@', '-i', '@INPUT@',\n> > +                                                '--mode', mode, '-t', template_file],\n> >                                       install : true,\n> >                                       install_dir : libcamera_headers_install_dir)\n> >  endforeach\n> > diff --git a/meson.build b/meson.build\n> > index e9a1c7e360ce..1423abf16c77 100644\n> > --- a/meson.build\n> > +++ b/meson.build\n> > @@ -267,6 +267,8 @@ py_mod.find_installation('python3', modules : py_modules)\n> >  summary({\n> >              'Enabled pipelines': pipelines,\n> >              'Enabled IPA modules': enabled_ipa_names,\n> > +            'Vendor controls': vendor_ctrl_files,\n> > +            'Vendor properties': vendor_prop_files,\n> >              'Hotplug support': libudev.found(),\n> >              'Tracing support': tracing_enabled,\n> >              'Android support': android_enabled,\n> > diff --git a/src/libcamera/control_ids_rpi.yaml b/src/libcamera/control_ids_rpi.yaml\n> > new file mode 100644\n> > index 000000000000..239904a008a8\n> > --- /dev/null\n> > +++ b/src/libcamera/control_ids_rpi.yaml\n> > @@ -0,0 +1,17 @@\n> > +# SPDX-License-Identifier: LGPL-2.1-or-later\n> > +#\n> > +# Copyright (C) 2023, Raspberry Pi Ltd\n> > +#\n> > +%YAML 1.1\n> > +---\n> > +controls:\n> > +\n> > +  - PispConfigDumpFile:\n> > +      type: string\n> > +      vendor: rpi\n> > +      description: |\n> > +        Triggers the Raspberry Pi PiSP pipeline handler to generate a JSON\n> > +        formatted dump of the Backend configuration to the filename given by the\n> > +        value of the control.\n> > +\n> > +...\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index e49bf850b355..8891052a3316 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -128,12 +128,25 @@ endif\n> >  control_sources = []\n> >\n> >  foreach source, mode : control_source_files\n> > -    input_files = files(source +'.yaml', source + '.cpp.in')\n> > +    input_files = files(source +'.yaml')\n> > +\n> > +    # Add the vendor specific files to the input.\n> > +    if mode == 'controls'\n> > +        vendor_files = vendor_ctrl_files\n> > +    else\n> > +        vendor_files = vendor_prop_files\n> > +    endif\n> > +\n> > +    foreach file : vendor_files\n> > +        input_files += files(file)\n> > +    endforeach\n> > +\n> > +    template_file = files(source + '.cpp.in')\n> >      control_sources += custom_target(source + '_cpp',\n> >                                       input : input_files,\n> >                                       output : source + '.cpp',\n> > -                                     command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',\n> > -                                                '--mode', mode])\n> > +                                     command : [gen_controls, '-o', '@OUTPUT@', '-i', '@INPUT@',\n>\n> meson will not expand this to multiple -i arguments, I'm actually\n> surprised it works as expected. Could we keep the input files as\n> positional arguments ?\n\nThe above construct will add \"-i <file1> <file2>...\" which is what the\nscript expects (i.e. not multiple -i arguments).  I'll change things\naround so input files are treated as the only positional argument.\n\nRegards,\nNaush\n\n>\n> > +                                                '--mode', mode, '-t', template_file])\n> >  endforeach\n> >\n> >  libcamera_sources += control_sources\n> > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> > index e89de674966a..a32ef09c2f48 100755\n> > --- a/src/py/libcamera/gen-py-controls.py\n> > +++ b/src/py/libcamera/gen-py-controls.py\n> > @@ -91,9 +91,9 @@ def main(argv):\n> >      parser = argparse.ArgumentParser()\n> >      parser.add_argument('-o', dest='output', metavar='file', type=str,\n> >                          help='Output file name. Defaults to standard output if not specified.')\n> > -    parser.add_argument('input', type=str,\n> > -                        help='Input file name.')\n> > -    parser.add_argument('template', type=str,\n> > +    parser.add_argument('-i', dest='inputs', type=str, nargs='+', required=True,\n> > +                        help='Input file name(s).')\n> > +    parser.add_argument('-t', dest='template', type=str, required=True,\n> >                          help='Template file name.')\n> >      parser.add_argument('--mode', type=str, required=True,\n> >                          help='Mode is either \"controls\" or \"properties\"')\n> > @@ -103,8 +103,10 @@ def main(argv):\n> >          print(f'Invalid mode option \"{args.mode}\"', file=sys.stderr)\n> >          return -1\n> >\n> > -    data = open(args.input, 'rb').read()\n> > -    controls = yaml.safe_load(data)['controls']\n> > +    controls = []\n> > +    for input in args.inputs:\n> > +        data = open(input, 'rb').read()\n> > +        controls += yaml.safe_load(data)['controls']\n> >\n> >      data = generate_py(controls, args.mode)\n> >\n> > diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\n> > index f58c7198ee9e..e4bc208b4ac1 100644\n> > --- a/src/py/libcamera/meson.build\n> > +++ b/src/py/libcamera/meson.build\n> > @@ -28,29 +28,35 @@ pycamera_sources = files([\n> >\n> >  # Generate controls\n> >\n> > -gen_py_controls_input_files = files([\n> > -    '../../libcamera/control_ids.yaml',\n> > -    'py_controls_generated.cpp.in',\n> > -])\n> > +gen_py_controls_input_files = files('../../libcamera/control_ids.yaml')\n> > +\n> > +foreach file : vendor_ctrl_files\n> > +    gen_py_controls_input_files += files('../../libcamera/' + file)\n> > +endforeach\n> >\n> >  gen_py_controls = files('gen-py-controls.py')\n> > +gen_py_controls_template = files('py_controls_generated.cpp.in')\n> >\n> >  pycamera_sources += custom_target('py_gen_controls',\n> >                                    input : gen_py_controls_input_files,\n> >                                    output : ['py_controls_generated.cpp'],\n> > -                                  command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@', '@INPUT@'])\n> > +                                  command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@', '-i', '@INPUT@',\n> > +                                             '-t', gen_py_controls_template])\n> >\n> >  # Generate properties\n> >\n> > -gen_py_property_enums_input_files = files([\n> > -    '../../libcamera/property_ids.yaml',\n> > -    'py_properties_generated.cpp.in',\n> > -])\n> > +gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> > +gen_py_property_template = files('py_properties_generated.cpp.in')\n> > +\n> > +foreach file : vendor_prop_files\n> > +    gen_py_property_enums_input_files += files('../../libcamera/' + file)\n> > +endforeach\n> >\n> >  pycamera_sources += custom_target('py_gen_properties',\n> >                                    input : gen_py_property_enums_input_files,\n> >                                    output : ['py_properties_generated.cpp'],\n> > -                                  command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@', '@INPUT@'])\n> > +                                  command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@', '-i', '@INPUT@',\n> > +                                             '-t', gen_py_property_template])\n> >\n> >  # Generate formats\n> >\n> > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > index dd55753e792c..3c490a562676 100755\n> > --- a/utils/gen-controls.py\n> > +++ b/utils/gen-controls.py\n> > @@ -362,18 +362,21 @@ def main(argv):\n> >\n> >      # Parse command line arguments\n> >      parser = argparse.ArgumentParser()\n> > -    parser.add_argument('-o', dest='output', metavar='file', type=str,\n> > +    parser.add_argument('-o', dest='output', metavar='file', type=str, required=True,\n> >                          help='Output file name. Defaults to standard output if not specified.')\n> > -    parser.add_argument('input', type=str,\n> > -                        help='Input file name.')\n> > -    parser.add_argument('template', type=str,\n> > +    parser.add_argument('-i', dest='inputs', type=str, nargs='+', required=True,\n> > +                        help='Input file name(s).')\n> > +    parser.add_argument('-t', dest='template', type=str, required=True,\n> >                          help='Template file name.')\n> >      parser.add_argument('--mode', type=str, required=True, choices=['controls', 'properties'],\n> >                          help='Mode of operation')\n> >      args = parser.parse_args(argv[1:])\n> >\n> > -    data = open(args.input, 'rb').read()\n> > -    controls = yaml.safe_load(data)['controls']\n> > +    controls = []\n> > +    for input in args.inputs:\n> > +        data = open(input, 'rb').read()\n> > +        controls = controls + yaml.safe_load(data)['controls']\n> > +\n> >      controls = [Control(*ctrl.popitem()) for ctrl in controls]\n> >\n> >      if args.template.endswith('.cpp.in'):\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A4E36BDE6B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 20 Nov 2023 15:01:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E44F7629AF;\n\tMon, 20 Nov 2023 16:01:27 +0100 (CET)","from mail-ot1-x32a.google.com (mail-ot1-x32a.google.com\n\t[IPv6:2607:f8b0:4864:20::32a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 70D8561DAF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 Nov 2023 16:01:26 +0100 (CET)","by mail-ot1-x32a.google.com with SMTP id\n\t46e09a7af769-6cd1918afb2so3014540a34.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 Nov 2023 07:01:26 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700492487;\n\tbh=tCspkuHo4KK0glb1Y3l8x8bcIy+ShEg9veewksvSFM4=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=pGRdVVGO+No0Q3QUlyf+U4RHmAnVNBeN1qKdIo/LEdlGiPgPfUP17DL2Z031fwsXW\n\tyDZFTQ9gcUNCnleH9CVfLxJ/GWHF1u+XxgLLq5uvTzt48ruUTEwHs+/Ez9AOBW9hPH\n\tEUA8nS/gtsgJKSr59D/H7UMNBVhdpFNNR2OGcDc1HQ83CRQpxslL/+JWxNWbmm5Pyz\n\thcXD3IOgLELWgnptledFaCyFIdu8Czav+azIziUIBPcDjGN9wg8Dc7bFve8D3AGUsy\n\tr1144o2Tz9QkJRlmoppii6SkVUpIjxBejeO1/tTqMdZ2NZTrrwknM9G6Itg6sRFgQr\n\tZ7At7Fd9mvuUA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1700492485; x=1701097285;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=KBAvyrdK8vePlnbJTB+r4WvSA3d++YmRG+8SNsGjC9I=;\n\tb=QCQ8N8569Wa4q27AgiQXps0Hd7ziENUJKh72dR012wEKMvp3h6IYDL6LAqhelZfb3R\n\tXgommJpkj+is1rl04uTuqCVbJgIK6/sV1jfDemJL1IocKCKxS8DpqI8bpFFOCbX54VXV\n\tSPO1+trXIHYK4Q0RTjGjrsu+AFvWCwU1HjjobD1+Dpt33gQ5geHD1Wk7L3b55oSJi4wy\n\t5yOIia/8bxEHJN+5MpBiccYqTSh+RRw15w+jwmvn0+i6fad0MaoXsEWrSavom1oQkGnZ\n\t1F2J9AJEkZsD0j3fhDQEE8r6IGwkyeqMtQ2OC5QPCCQYE6VJVS81CJ3c3dAXfMiN7c84\n\t6axg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"QCQ8N856\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1700492485; x=1701097285;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=KBAvyrdK8vePlnbJTB+r4WvSA3d++YmRG+8SNsGjC9I=;\n\tb=nbOsis305GjkK/bu60+Hg/BKW1IrdQaV+Ruw9aN1LxM6nYww1FNhgrLlfHPaIGaEQa\n\tfM/bbM8PxF52MEooFvN234aLuMVAiZAZK1vhWAT9jg80xy61qUBqy2bLebnlNLU8a60B\n\tZIiO/JGuYw6NXsqQOZyBw8Bn++y6HhQZyUfrfWmqDO4vNWiSXgqA7Yy8qOxCTr4B6edP\n\tA4Qm3X5Fl5GT22CrxPvfh1MJo3eI4rE2IPLQihatJfvzL/ncwSZSsBL2ZlCR+VdDFdsn\n\tABxVLAaiQua1fH37WLddhJnaRSYYOngtp51Mz/kPgBRrBG/xFPz3oHW1/VQamO5n9u2U\n\tjoAg==","X-Gm-Message-State":"AOJu0Yz7oRafFh30xKVeXd/y3YVi+ouQCA8opUy1BzQy3idgGFKoN07S\n\tVM331fYJ3isUBeYhZPqMNunYRKbI2VptBEI+tyfNdw==","X-Google-Smtp-Source":"AGHT+IEygX3qNyto/qcJtk3jn1XVLSpvWYJ3XmR82km02LYKJPFG9XaFNoIMRn6h/HkZdx03515jMTsxUfptGPoyA10=","X-Received":"by 2002:a05:6830:1157:b0:6c4:897a:31d0 with SMTP id\n\tx23-20020a056830115700b006c4897a31d0mr2093474otq.24.1700492484751;\n\tMon, 20 Nov 2023 07:01:24 -0800 (PST)","MIME-Version":"1.0","References":"<20231110110002.21381-1-naush@raspberrypi.com>\n\t<20231110110002.21381-3-naush@raspberrypi.com>\n\t<20231120040953.GF524@pendragon.ideasonboard.com>","In-Reply-To":"<20231120040953.GF524@pendragon.ideasonboard.com>","Date":"Mon, 20 Nov 2023 15:00:52 +0000","Message-ID":"<CAEmqJPoY-FNpkEpwVOjJRWa8JJiQSzRfOo1hk3_Yxauw0sFwJg@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v1 2/5] controls: build: Allow\n\tseparate vendor control YAML files","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28110,"web_url":"https://patchwork.libcamera.org/comment/28110/","msgid":"<20231120160322.GG6824@pendragon.ideasonboard.com>","date":"2023-11-20T16:03:22","subject":"Re: [libcamera-devel] [PATCH v1 2/5] controls: build: Allow\n\tseparate vendor control YAML files","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Mon, Nov 20, 2023 at 03:00:52PM +0000, Naushir Patuck wrote:\n> On Mon, 20 Nov 2023 at 04:09, Laurent Pinchart wrote:\n> > On Fri, Nov 10, 2023 at 10:59:59AM +0000, Naushir Patuck via libcamera-devel wrote:\n> > > Add support for using separate YAML files for vendor specific controls.\n> > > This simplifies management of vendor control definitions and avoids\n> > > possible merge conflicts when changing the control_ids.yaml file for\n> > > core and draft controls. The mapping of vendor/pipeline handler to\n> > > control file is done through the vendor_controls_mapping variable in\n> > > include/libcamera/meson.build.\n> > >\n> > > The command line argument handling for the gen-py-controls.py and\n> > > gen-controls.py have now changed to allow multiple input files to be\n> > > provided. This means that all positional arguments have been replaced\n> > > by non-positional equivalents.\n> >\n> > This could have been done in a separate patch to ease review.\n> \n> I'll split this to a separate patch as best I can in the next version.\n> \n> > > Add a new control_ids_rpi.yaml file to hold the Raspberry Pi specific\n> > > vendor controls. This contains a single control PispConfigDumpFile that\n> > > will be used in the Pi 5 pipeline handler as a trigger to dump the\n> > > Backend configuration as a JSON file.\n> > >\n> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > > ---\n> > >  include/libcamera/meson.build       | 48 ++++++++++++++++++++++++++---\n> > >  meson.build                         |  2 ++\n> > >  src/libcamera/control_ids_rpi.yaml  | 17 ++++++++++\n> > >  src/libcamera/meson.build           | 19 ++++++++++--\n> > >  src/py/libcamera/gen-py-controls.py | 12 +++++---\n> > >  src/py/libcamera/meson.build        | 26 ++++++++++------\n> > >  utils/gen-controls.py               | 15 +++++----\n> > >  7 files changed, 111 insertions(+), 28 deletions(-)\n> > >  create mode 100644 src/libcamera/control_ids_rpi.yaml\n> > >\n> > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> > > index f736cca07228..f8af51174f08 100644\n> > > --- a/include/libcamera/meson.build\n> > > +++ b/include/libcamera/meson.build\n> > > @@ -32,21 +32,61 @@ install_headers(libcamera_public_headers,\n> > >\n> > >  libcamera_headers_install_dir = get_option('includedir') / libcamera_include_dir\n> > >\n> > > -# control_ids.h and property_ids.h\n> > > +# control_ids.h and property_ids.h and associated modes\n> >\n> > I think this belongs to the previous patch.\n> \n> Ack.\n> \n> > >  control_source_files = {\n> > >      'control_ids': 'controls',\n> > >      'property_ids': 'properties',\n> > >  }\n> > >\n> > > +vendor_mappings = {\n> >\n> > The commit message names this vendor_controls_mapping, which I think is\n> > a better name as it's related to controls.\n> \n> Ack\n> \n> > > +    # Mapping of vendor (pipeline handler) specific controls files\n> > > +    'controls':\n> > > +    {\n> > > +        'rpi/pisp': 'control_ids_rpi.yaml',\n> > > +        'rpi/vc4': 'control_ids_rpi.yaml'\n> > > +    },\n> > > +    # Mapping of vendor (pipeline handler) specific properties files\n> > > +    'properties':\n> > > +    {\n> > > +\n> > > +    }\n> > > +}\n> > > +\n> > >  control_headers = []\n> > > +vendor_ctrl_files = []\n> > > +vendor_prop_files = []\n> > >\n> > >  foreach header, mode : control_source_files\n> > > -    input_files = files('../../src/libcamera/' + header +'.yaml', header + '.h.in')\n> > > +    # Start by populating the vendor specific mappings into vendor_ctrl_files\n> > > +    # vendor_prop_files. These will be cached for later use.\n> >\n> > s/vendor_prop_files/and vendor_prop_files/\n> \n> Ack\n> \n> > > +    vendor_files = []\n> > > +    foreach pipeline, file : vendor_mappings[mode]\n> > > +        if pipeline not in pipelines\n> > > +            continue\n> > > +        endif\n> > > +        if file not in vendor_files\n> > > +            vendor_files += file\n> > > +        endif\n> > > +    endforeach\n> > > +\n> > > +    if mode == 'controls'\n> > > +        vendor_ctrl_files = vendor_files\n> > > +    else\n> > > +        vendor_prop_files = vendor_files\n> > > +    endif\n> > > +\n> > > +    input_files = files('../../src/libcamera/' + header +'.yaml')\n> >\n> > While at it, add the missing space before the '.yaml'.\n> \n> Ack\n> \n> > > +\n> > > +    foreach file : vendor_files\n> > > +        input_files += files('../../src/libcamera/' + file)\n> > > +    endforeach\n> > > +\n> > > +    template_file = files(header + '.h.in')\n> > >      control_headers += custom_target(header + '_h',\n> > >                                       input : input_files,\n> > >                                       output : header + '.h',\n> > > -                                     command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',\n> > > -                                                '--mode', mode],\n> > > +                                     command : [gen_controls, '-o', '@OUTPUT@', '-i', '@INPUT@',\n> > > +                                                '--mode', mode, '-t', template_file],\n> > >                                       install : true,\n> > >                                       install_dir : libcamera_headers_install_dir)\n> > >  endforeach\n> > > diff --git a/meson.build b/meson.build\n> > > index e9a1c7e360ce..1423abf16c77 100644\n> > > --- a/meson.build\n> > > +++ b/meson.build\n> > > @@ -267,6 +267,8 @@ py_mod.find_installation('python3', modules : py_modules)\n> > >  summary({\n> > >              'Enabled pipelines': pipelines,\n> > >              'Enabled IPA modules': enabled_ipa_names,\n> > > +            'Vendor controls': vendor_ctrl_files,\n> > > +            'Vendor properties': vendor_prop_files,\n> > >              'Hotplug support': libudev.found(),\n> > >              'Tracing support': tracing_enabled,\n> > >              'Android support': android_enabled,\n> > > diff --git a/src/libcamera/control_ids_rpi.yaml b/src/libcamera/control_ids_rpi.yaml\n> > > new file mode 100644\n> > > index 000000000000..239904a008a8\n> > > --- /dev/null\n> > > +++ b/src/libcamera/control_ids_rpi.yaml\n> > > @@ -0,0 +1,17 @@\n> > > +# SPDX-License-Identifier: LGPL-2.1-or-later\n> > > +#\n> > > +# Copyright (C) 2023, Raspberry Pi Ltd\n> > > +#\n> > > +%YAML 1.1\n> > > +---\n> > > +controls:\n> > > +\n> > > +  - PispConfigDumpFile:\n> > > +      type: string\n> > > +      vendor: rpi\n> > > +      description: |\n> > > +        Triggers the Raspberry Pi PiSP pipeline handler to generate a JSON\n> > > +        formatted dump of the Backend configuration to the filename given by the\n> > > +        value of the control.\n> > > +\n> > > +...\n> > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > > index e49bf850b355..8891052a3316 100644\n> > > --- a/src/libcamera/meson.build\n> > > +++ b/src/libcamera/meson.build\n> > > @@ -128,12 +128,25 @@ endif\n> > >  control_sources = []\n> > >\n> > >  foreach source, mode : control_source_files\n> > > -    input_files = files(source +'.yaml', source + '.cpp.in')\n> > > +    input_files = files(source +'.yaml')\n> > > +\n> > > +    # Add the vendor specific files to the input.\n> > > +    if mode == 'controls'\n> > > +        vendor_files = vendor_ctrl_files\n> > > +    else\n> > > +        vendor_files = vendor_prop_files\n> > > +    endif\n> > > +\n> > > +    foreach file : vendor_files\n> > > +        input_files += files(file)\n> > > +    endforeach\n> > > +\n> > > +    template_file = files(source + '.cpp.in')\n> > >      control_sources += custom_target(source + '_cpp',\n> > >                                       input : input_files,\n> > >                                       output : source + '.cpp',\n> > > -                                     command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',\n> > > -                                                '--mode', mode])\n> > > +                                     command : [gen_controls, '-o', '@OUTPUT@', '-i', '@INPUT@',\n> >\n> > meson will not expand this to multiple -i arguments, I'm actually\n> > surprised it works as expected. Could we keep the input files as\n> > positional arguments ?\n> \n> The above construct will add \"-i <file1> <file2>...\" which is what the\n> script expects (i.e. not multiple -i arguments).  I'll change things\n> around so input files are treated as the only positional argument.\n\nThat's what I've seen, and I was surprised that\n\n    parser.add_argument('-i', dest='inputs', type=str, nargs='+', required=True,\n                        help='Input file name(s).')\n\nactually supports that fine. I thought it would only work with multiple\ninstances of the -i argument. I don't know if it's a guaranteed\nbehaviour or an undefined behaviour, so that's why I'm thinking\npositional parameters would be safer (beside being more standard).\n\n> > > +                                                '--mode', mode, '-t', template_file])\n> > >  endforeach\n> > >\n> > >  libcamera_sources += control_sources\n> > > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> > > index e89de674966a..a32ef09c2f48 100755\n> > > --- a/src/py/libcamera/gen-py-controls.py\n> > > +++ b/src/py/libcamera/gen-py-controls.py\n> > > @@ -91,9 +91,9 @@ def main(argv):\n> > >      parser = argparse.ArgumentParser()\n> > >      parser.add_argument('-o', dest='output', metavar='file', type=str,\n> > >                          help='Output file name. Defaults to standard output if not specified.')\n> > > -    parser.add_argument('input', type=str,\n> > > -                        help='Input file name.')\n> > > -    parser.add_argument('template', type=str,\n> > > +    parser.add_argument('-i', dest='inputs', type=str, nargs='+', required=True,\n> > > +                        help='Input file name(s).')\n> > > +    parser.add_argument('-t', dest='template', type=str, required=True,\n> > >                          help='Template file name.')\n> > >      parser.add_argument('--mode', type=str, required=True,\n> > >                          help='Mode is either \"controls\" or \"properties\"')\n> > > @@ -103,8 +103,10 @@ def main(argv):\n> > >          print(f'Invalid mode option \"{args.mode}\"', file=sys.stderr)\n> > >          return -1\n> > >\n> > > -    data = open(args.input, 'rb').read()\n> > > -    controls = yaml.safe_load(data)['controls']\n> > > +    controls = []\n> > > +    for input in args.inputs:\n> > > +        data = open(input, 'rb').read()\n> > > +        controls += yaml.safe_load(data)['controls']\n> > >\n> > >      data = generate_py(controls, args.mode)\n> > >\n> > > diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\n> > > index f58c7198ee9e..e4bc208b4ac1 100644\n> > > --- a/src/py/libcamera/meson.build\n> > > +++ b/src/py/libcamera/meson.build\n> > > @@ -28,29 +28,35 @@ pycamera_sources = files([\n> > >\n> > >  # Generate controls\n> > >\n> > > -gen_py_controls_input_files = files([\n> > > -    '../../libcamera/control_ids.yaml',\n> > > -    'py_controls_generated.cpp.in',\n> > > -])\n> > > +gen_py_controls_input_files = files('../../libcamera/control_ids.yaml')\n> > > +\n> > > +foreach file : vendor_ctrl_files\n> > > +    gen_py_controls_input_files += files('../../libcamera/' + file)\n> > > +endforeach\n> > >\n> > >  gen_py_controls = files('gen-py-controls.py')\n> > > +gen_py_controls_template = files('py_controls_generated.cpp.in')\n> > >\n> > >  pycamera_sources += custom_target('py_gen_controls',\n> > >                                    input : gen_py_controls_input_files,\n> > >                                    output : ['py_controls_generated.cpp'],\n> > > -                                  command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@', '@INPUT@'])\n> > > +                                  command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@', '-i', '@INPUT@',\n> > > +                                             '-t', gen_py_controls_template])\n> > >\n> > >  # Generate properties\n> > >\n> > > -gen_py_property_enums_input_files = files([\n> > > -    '../../libcamera/property_ids.yaml',\n> > > -    'py_properties_generated.cpp.in',\n> > > -])\n> > > +gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> > > +gen_py_property_template = files('py_properties_generated.cpp.in')\n> > > +\n> > > +foreach file : vendor_prop_files\n> > > +    gen_py_property_enums_input_files += files('../../libcamera/' + file)\n> > > +endforeach\n> > >\n> > >  pycamera_sources += custom_target('py_gen_properties',\n> > >                                    input : gen_py_property_enums_input_files,\n> > >                                    output : ['py_properties_generated.cpp'],\n> > > -                                  command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@', '@INPUT@'])\n> > > +                                  command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@', '-i', '@INPUT@',\n> > > +                                             '-t', gen_py_property_template])\n> > >\n> > >  # Generate formats\n> > >\n> > > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > > index dd55753e792c..3c490a562676 100755\n> > > --- a/utils/gen-controls.py\n> > > +++ b/utils/gen-controls.py\n> > > @@ -362,18 +362,21 @@ def main(argv):\n> > >\n> > >      # Parse command line arguments\n> > >      parser = argparse.ArgumentParser()\n> > > -    parser.add_argument('-o', dest='output', metavar='file', type=str,\n> > > +    parser.add_argument('-o', dest='output', metavar='file', type=str, required=True,\n> > >                          help='Output file name. Defaults to standard output if not specified.')\n> > > -    parser.add_argument('input', type=str,\n> > > -                        help='Input file name.')\n> > > -    parser.add_argument('template', type=str,\n> > > +    parser.add_argument('-i', dest='inputs', type=str, nargs='+', required=True,\n> > > +                        help='Input file name(s).')\n> > > +    parser.add_argument('-t', dest='template', type=str, required=True,\n> > >                          help='Template file name.')\n> > >      parser.add_argument('--mode', type=str, required=True, choices=['controls', 'properties'],\n> > >                          help='Mode of operation')\n> > >      args = parser.parse_args(argv[1:])\n> > >\n> > > -    data = open(args.input, 'rb').read()\n> > > -    controls = yaml.safe_load(data)['controls']\n> > > +    controls = []\n> > > +    for input in args.inputs:\n> > > +        data = open(input, 'rb').read()\n> > > +        controls = controls + yaml.safe_load(data)['controls']\n> > > +\n> > >      controls = [Control(*ctrl.popitem()) for ctrl in controls]\n> > >\n> > >      if args.template.endswith('.cpp.in'):","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0E013C3200\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 20 Nov 2023 16:03:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4DEBE629B6;\n\tMon, 20 Nov 2023 17:03:17 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7112D629AF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 Nov 2023 17:03:15 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 70D3174A;\n\tMon, 20 Nov 2023 17:02:45 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700496197;\n\tbh=boXsuXUQ0dRpB2vribdCI1qJTBsWDLHHVFn8Ztn2HXg=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=b+QCNdR61Xx6xGM8SZbpyZdh1SCo2JFu+py5KUpZ4AAArmLajg5QSJt2q47rke0Fj\n\th+V2mPNBsOGVbpetnILym/Cgw737tbqw52S1bPz8cgQy3oh/1ZoGFw9NLbT+N9vQ6v\n\tiCsoldHSyMX5IoEDOAy95+/0HWEgrqyQ+vR9yZIy8x5hxVD+TUMurCMScZsJNxrcUo\n\tKrsfuhZ8mSMqCdg11sbSTnUwY/KBzXBS1P1yT3ZGXf+s0oOBQ3r1b1C4JlS6USotKZ\n\toxSntL3wY0ObTeDmvvts5z07sNEwHbOlR/bBQmPQb8oZId+ht/nSepZEzjH3Wpj4mp\n\tRWigDnsLQuI3A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1700496165;\n\tbh=boXsuXUQ0dRpB2vribdCI1qJTBsWDLHHVFn8Ztn2HXg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fXKWhTg9xdlcLSth8IYdzeA9FurzbZdz4jIk1yUr0oP3/wPaXhmNnkNUJsk66MkZt\n\tCiL+pyyT0iyjBCxOS6xnDyYHQGt6soP3kouBk225RT0tdaN91RZTXgk7/MJOb4NPj4\n\tG6DLVxDWH5uRliglTVMqCOgRhfT43YLSHrn5gbLo="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"fXKWhTg9\"; dkim-atps=neutral","Date":"Mon, 20 Nov 2023 18:03:22 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<20231120160322.GG6824@pendragon.ideasonboard.com>","References":"<20231110110002.21381-1-naush@raspberrypi.com>\n\t<20231110110002.21381-3-naush@raspberrypi.com>\n\t<20231120040953.GF524@pendragon.ideasonboard.com>\n\t<CAEmqJPoY-FNpkEpwVOjJRWa8JJiQSzRfOo1hk3_Yxauw0sFwJg@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPoY-FNpkEpwVOjJRWa8JJiQSzRfOo1hk3_Yxauw0sFwJg@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v1 2/5] controls: build: Allow\n\tseparate vendor control YAML files","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]