[{"id":28125,"web_url":"https://patchwork.libcamera.org/comment/28125/","msgid":"","date":"2023-11-22T15:24:40","subject":"Re: [libcamera-devel] [PATCH v2 3/6] controls: build: Allow\n\tseparate vendor control YAML files","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Tue, Nov 21, 2023 at 02:53:47PM +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> 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 \n> ---\n> include/libcamera/meson.build | 42 ++++++++++++++++++++++++++++-\n> meson.build | 2 ++\n> src/libcamera/control_ids_rpi.yaml | 16 +++++++++++\n> src/libcamera/meson.build | 12 +++++++++\n> src/py/libcamera/gen-py-controls.py | 10 +++----\n> src/py/libcamera/meson.build | 8 ++++++\n> utils/gen-controls.py | 12 +++++----\n> 7 files changed, 91 insertions(+), 11 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 5fb772e6dd14..54123c5b858d 100644\n> --- a/include/libcamera/meson.build\n> +++ b/include/libcamera/meson.build\n> @@ -38,10 +38,50 @@ control_source_files = {\n> 'property_ids': 'properties',\n> }\n\nI really like the series, but this patch is the piece I like less.\n\n>\n> +vendor_controls_mapping = {\n> + # Mapping of vendor (pipeline handler) specific controls files\n> + 'controls':\n> + {\n\nIsn't this usually written as\n\n 'controls' : {\n\n }\n\nin meson ?\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')\n> +\n> + # Start by populating the vendor specific mappings into vendor_ctrl_files\n> + # and vendor_prop_files. These will be cached for later use.\n> + vendor_files = []\n> + foreach pipeline, file : vendor_controls_mapping[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> + foreach file : vendor_files\n> + input_files += files('../../src/libcamera/' + file)\n> + endforeach\n> +\n\nWouldn't it be simpler if we try to handle libcamera, draft and vendor\ncontrols as first class citizens in the same manner, instead of\nmaintaing the \"libcamera\" ones on a different level ?\n\nI guess if we could do something like:\n\n-------------------------------------------------------------------------------\n\ncontrols_map {\n 'controls' : {\n 'libcamera' : controls_ids.yaml,\n 'drafts' : controls_ids_draft.yaml\n 'rpi/pisp': 'control_ids_rpi.yaml',\n 'rpi/vc4': 'control_ids_rpi.yaml'\n }\n\n 'properties' = {\n 'libcamera' : controls_ids.yaml,\n 'drafts' : controls_ids_draft.yaml\n }\n}\n\ncontrol_headers = []\nranges_file = files('../../src/libcamera/control_ranges.yaml')\nforeach mode, entry : controls_map\n files_list = []\n input_files=[]\n foreach vendor, header : entry\n if vendor != 'libcamera' or vendor != 'draft'\n if vendor not in pipelines\n continue\n endif\n endif\n\n if header in files_list\n continue\n endif\n files_list += header\n\n input_files += files('../../src/libcamera/' + header)\n endforeach\n\n outfile=''\n if mode == 'controls'\n outfile = 'control_ids.h'\n else\n outfile = 'property_ids.h'\n endif\n\n template = outfile + '.in'\n control_headers += custom_target(header + '_h',\n input : input_files,\n output : outfile,\n command : [gen_controls, '-o', '@OUTPUT@',\n '--mode', mode, '-t', template,\n '-r', ranges_file, '@INPUT@'],\n install : true,\n install_dir : libcamera_headers_install_dir)\nendforeach\n\nlibcamera_public_headers += control_headers\n\n-------------------------------------------------------------------------------\n\nisn't it nicer ?\n\nThe above code is valid in meson, but the meson.build in\nsrc/libcamera/ should be updated accordingly.\n\nIn general, I think the relevant part here is\n\ncontrols_map {\n 'controls' : {\n 'libcamera' : controls_ids.yaml,\n 'drafts' : controls_ids_draft.yaml\n 'rpi/pisp': 'control_ids_rpi.yaml',\n 'rpi/vc4': 'control_ids_rpi.yaml'\n }\n\n 'properties' = {\n 'libcamera' : controls_ids.yaml,\n 'drafts' : controls_ids_draft.yaml\n }\n}\n\nAs seeing all the supported controls and properties listed together\nmight be easier to maintain and parse ?\n\nWhat do you think ?\n\n\n> template_file = files(header + '.h.in')\n> control_headers += custom_target(header + '_h',\n> input : input_files,\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..a6f8b43a5f62\n> --- /dev/null\n> +++ b/src/libcamera/control_ids_rpi.yaml\n> @@ -0,0 +1,16 @@\n> +# SPDX-License-Identifier: LGPL-2.1-or-later\n> +#\n> +# Copyright (C) 2023, Raspberry Pi Ltd\n> +#\n> +%YAML 1.1\n> +---\n> +vendor: rpi\n> +controls:\n> + - PispConfigDumpFile:\n> + type: string\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 05ee38daf22b..7eb26ccbb7f5 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -129,6 +129,18 @@ control_sources = []\n>\n> foreach source, mode : control_source_files\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> diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> index ea28f0139f23..e2ddad8581fd 100755\n> --- a/src/py/libcamera/gen-py-controls.py\n> +++ b/src/py/libcamera/gen-py-controls.py\n> @@ -91,7 +91,7 @@ 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> + parser.add_argument('input', type=str, nargs='+',\n> help='Input file name.')\n> parser.add_argument('-t', dest='template', type=str, required=True,\n> help='Template file name.')\n> @@ -103,11 +103,11 @@ 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> -\n> controls = {}\n> - vendor = yaml.safe_load(data)['vendor']\n> - controls[vendor] = yaml.safe_load(data)['controls']\n> + for input in args.input:\n> + data = open(input, 'rb').read()\n> + vendor = yaml.safe_load(data)['vendor']\n> + controls[vendor] = 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 1c3ea1843ac0..ea38ad6ca829 100644\n> --- a/src/py/libcamera/meson.build\n> +++ b/src/py/libcamera/meson.build\n> @@ -33,6 +33,10 @@ gen_py_controls_template = files('py_controls_generated.cpp.in')\n>\n> gen_py_controls = files('gen-py-controls.py')\n>\n> +foreach file : vendor_ctrl_files\n> + gen_py_controls_input_files += files('../../libcamera/' + file)\n> +endforeach\n> +\n> pycamera_sources += custom_target('py_gen_controls',\n> input : gen_py_controls_input_files,\n> output : ['py_controls_generated.cpp'],\n> @@ -44,6 +48,10 @@ pycamera_sources += custom_target('py_gen_controls',\n> gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> gen_py_properties_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> diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> index 8af33d29cd07..d7862142b8c1 100755\n> --- a/utils/gen-controls.py\n> +++ b/utils/gen-controls.py\n> @@ -359,7 +359,7 @@ 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> + parser.add_argument('input', type=str, nargs='+',\n> help='Input file name.')\n> parser.add_argument('-t', dest='template', type=str, required=True,\n> help='Template file name.')\n> @@ -367,10 +367,12 @@ def main(argv):\n> help='Mode of operation')\n> args = parser.parse_args(argv[1:])\n>\n> - data = open(args.input, 'rb').read()\n> - vendor = yaml.safe_load(data)['vendor']\n> - controls = yaml.safe_load(data)['controls']\n> - controls = [Control(*ctrl.popitem(), vendor) for ctrl in controls]\n> + controls = []\n> + for input in args.input:\n> + data = open(input, 'rb').read()\n> + vendor = yaml.safe_load(data)['vendor']\n> + ctrls = yaml.safe_load(data)['controls']\n> + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls]\n>\n> if args.template.endswith('.cpp.in'):\n> data = generate_cpp(controls)\n> --\n> 2.34.1\n>","headers":{"Return-Path":"","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 33FA3C3220\n\tfor ;\n\tWed, 22 Nov 2023 15:24:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5FCBF629AF;\n\tWed, 22 Nov 2023 16:24:45 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E72A661DAD\n\tfor ;\n\tWed, 22 Nov 2023 16:24:43 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AE1B2276;\n\tWed, 22 Nov 2023 16:24:12 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700666685;\n\tbh=K7f87ECX41+E1nIDWGZPGLHeMKqg3bJg1j/ypW1Fwl4=;\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=w/LOsAA8Yk3LwDPgc7iI2/hv9C2bUGtBaQMDjtTg5Qn9MA73ZtahPWzRsLmx9x8r+\n\tKMVOo+9tLXFytgM1eXhbyQiVJycKaJAuga7x80p1s/iCmIsMPW2fQiW5JSxTTO3h1w\n\t0GwzUnREHzvaJp20eKb06Scqv7Rga0N4msKhcJWjTM/Cp9I2rjHkEkYulkVLGcFyRn\n\tD3dQbUtp572+Qnz4HAeHyHrbITI27iI8aUb1eg5ADWh8Fm9q0ERxhTayCuidAKxews\n\tKNSBTrz2xsTGWKdMx6LKzKdiTayuoF0pAbqyCvHJCSj/UzL6mVFCfAWiecJeoyHS5R\n\tp5Kx8KUcFHurQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1700666652;\n\tbh=K7f87ECX41+E1nIDWGZPGLHeMKqg3bJg1j/ypW1Fwl4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=AopJZkicEWWxi54vvoiUayVNTC9GdSS4qqhWewDUyooLKX6AamQjB8/+0hBps3W8G\n\tvFfRu6l2bFNdWaYy6zprdbwOfTAbWGDf+5mbUQJVfE4TkdHe3T5t70el4YxLtIwK0a\n\t3fXukWaO1Saopszqr03OTC1iwBpt9eKdrD3rzUIQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"AopJZkic\"; dkim-atps=neutral","Date":"Wed, 22 Nov 2023 16:24:40 +0100","To":"Naushir Patuck ","Message-ID":"","References":"<20231121145350.5956-1-naush@raspberrypi.com>\n\t<20231121145350.5956-4-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20231121145350.5956-4-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28127,"web_url":"https://patchwork.libcamera.org/comment/28127/","msgid":"","date":"2023-11-22T15:53:03","subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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 Jacopo,\n\nOn Wed, 22 Nov 2023 at 15:24, Jacopo Mondi\n wrote:\n>\n> Hi Naush\n>\n> On Tue, Nov 21, 2023 at 02:53:47PM +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> > 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 \n> > ---\n> > include/libcamera/meson.build | 42 ++++++++++++++++++++++++++++-\n> > meson.build | 2 ++\n> > src/libcamera/control_ids_rpi.yaml | 16 +++++++++++\n> > src/libcamera/meson.build | 12 +++++++++\n> > src/py/libcamera/gen-py-controls.py | 10 +++----\n> > src/py/libcamera/meson.build | 8 ++++++\n> > utils/gen-controls.py | 12 +++++----\n> > 7 files changed, 91 insertions(+), 11 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 5fb772e6dd14..54123c5b858d 100644\n> > --- a/include/libcamera/meson.build\n> > +++ b/include/libcamera/meson.build\n> > @@ -38,10 +38,50 @@ control_source_files = {\n> > 'property_ids': 'properties',\n> > }\n>\n> I really like the series, but this patch is the piece I like less.\n>\n> >\n> > +vendor_controls_mapping = {\n> > + # Mapping of vendor (pipeline handler) specific controls files\n> > + 'controls':\n> > + {\n>\n> Isn't this usually written as\n>\n> 'controls' : {\n>\n> }\n>\n> in meson ?\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')\n> > +\n> > + # Start by populating the vendor specific mappings into vendor_ctrl_files\n> > + # and vendor_prop_files. These will be cached for later use.\n> > + vendor_files = []\n> > + foreach pipeline, file : vendor_controls_mapping[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> > + foreach file : vendor_files\n> > + input_files += files('../../src/libcamera/' + file)\n> > + endforeach\n> > +\n>\n> Wouldn't it be simpler if we try to handle libcamera, draft and vendor\n> controls as first class citizens in the same manner, instead of\n> maintaing the \"libcamera\" ones on a different level ?\n>\n> I guess if we could do something like:\n>\n> -------------------------------------------------------------------------------\n>\n> controls_map {\n> 'controls' : {\n> 'libcamera' : controls_ids.yaml,\n> 'drafts' : controls_ids_draft.yaml\n> 'rpi/pisp': 'control_ids_rpi.yaml',\n> 'rpi/vc4': 'control_ids_rpi.yaml'\n> }\n>\n> 'properties' = {\n> 'libcamera' : controls_ids.yaml,\n> 'drafts' : controls_ids_draft.yaml\n> }\n> }\n>\n> control_headers = []\n> ranges_file = files('../../src/libcamera/control_ranges.yaml')\n> foreach mode, entry : controls_map\n> files_list = []\n> input_files=[]\n> foreach vendor, header : entry\n> if vendor != 'libcamera' or vendor != 'draft'\n> if vendor not in pipelines\n> continue\n> endif\n> endif\n>\n> if header in files_list\n> continue\n> endif\n> files_list += header\n>\n> input_files += files('../../src/libcamera/' + header)\n> endforeach\n>\n> outfile=''\n> if mode == 'controls'\n> outfile = 'control_ids.h'\n> else\n> outfile = 'property_ids.h'\n> endif\n>\n> template = outfile + '.in'\n> control_headers += custom_target(header + '_h',\n> input : input_files,\n> output : outfile,\n> command : [gen_controls, '-o', '@OUTPUT@',\n> '--mode', mode, '-t', template,\n> '-r', ranges_file, '@INPUT@'],\n> install : true,\n> install_dir : libcamera_headers_install_dir)\n> endforeach\n>\n> libcamera_public_headers += control_headers\n>\n> -------------------------------------------------------------------------------\n>\n> isn't it nicer ?\n>\n> The above code is valid in meson, but the meson.build in\n> src/libcamera/ should be updated accordingly.\n>\n> In general, I think the relevant part here is\n>\n> controls_map {\n> 'controls' : {\n> 'libcamera' : controls_ids.yaml,\n> 'drafts' : controls_ids_draft.yaml\n> 'rpi/pisp': 'control_ids_rpi.yaml',\n> 'rpi/vc4': 'control_ids_rpi.yaml'\n> }\n>\n> 'properties' = {\n> 'libcamera' : controls_ids.yaml,\n> 'drafts' : controls_ids_draft.yaml\n> }\n> }\n>\n> As seeing all the supported controls and properties listed together\n> might be easier to maintain and parse ?\n>\n> What do you think ?\n\nThat looks reasonable. I guess I wanted to keep things seperate for\nvendor controls, but no reason not to combine the libcamera/draft and\nvendor cases into one list. I can make this change for v3.\n\nRegards,\nNaush\n\n\n>\n>\n> > template_file = files(header + '.h.in')\n> > control_headers += custom_target(header + '_h',\n> > input : input_files,\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..a6f8b43a5f62\n> > --- /dev/null\n> > +++ b/src/libcamera/control_ids_rpi.yaml\n> > @@ -0,0 +1,16 @@\n> > +# SPDX-License-Identifier: LGPL-2.1-or-later\n> > +#\n> > +# Copyright (C) 2023, Raspberry Pi Ltd\n> > +#\n> > +%YAML 1.1\n> > +---\n> > +vendor: rpi\n> > +controls:\n> > + - PispConfigDumpFile:\n> > + type: string\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 05ee38daf22b..7eb26ccbb7f5 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -129,6 +129,18 @@ control_sources = []\n> >\n> > foreach source, mode : control_source_files\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> > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> > index ea28f0139f23..e2ddad8581fd 100755\n> > --- a/src/py/libcamera/gen-py-controls.py\n> > +++ b/src/py/libcamera/gen-py-controls.py\n> > @@ -91,7 +91,7 @@ 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> > + parser.add_argument('input', type=str, nargs='+',\n> > help='Input file name.')\n> > parser.add_argument('-t', dest='template', type=str, required=True,\n> > help='Template file name.')\n> > @@ -103,11 +103,11 @@ 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> > -\n> > controls = {}\n> > - vendor = yaml.safe_load(data)['vendor']\n> > - controls[vendor] = yaml.safe_load(data)['controls']\n> > + for input in args.input:\n> > + data = open(input, 'rb').read()\n> > + vendor = yaml.safe_load(data)['vendor']\n> > + controls[vendor] = 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 1c3ea1843ac0..ea38ad6ca829 100644\n> > --- a/src/py/libcamera/meson.build\n> > +++ b/src/py/libcamera/meson.build\n> > @@ -33,6 +33,10 @@ gen_py_controls_template = files('py_controls_generated.cpp.in')\n> >\n> > gen_py_controls = files('gen-py-controls.py')\n> >\n> > +foreach file : vendor_ctrl_files\n> > + gen_py_controls_input_files += files('../../libcamera/' + file)\n> > +endforeach\n> > +\n> > pycamera_sources += custom_target('py_gen_controls',\n> > input : gen_py_controls_input_files,\n> > output : ['py_controls_generated.cpp'],\n> > @@ -44,6 +48,10 @@ pycamera_sources += custom_target('py_gen_controls',\n> > gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> > gen_py_properties_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> > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > index 8af33d29cd07..d7862142b8c1 100755\n> > --- a/utils/gen-controls.py\n> > +++ b/utils/gen-controls.py\n> > @@ -359,7 +359,7 @@ 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> > + parser.add_argument('input', type=str, nargs='+',\n> > help='Input file name.')\n> > parser.add_argument('-t', dest='template', type=str, required=True,\n> > help='Template file name.')\n> > @@ -367,10 +367,12 @@ def main(argv):\n> > help='Mode of operation')\n> > args = parser.parse_args(argv[1:])\n> >\n> > - data = open(args.input, 'rb').read()\n> > - vendor = yaml.safe_load(data)['vendor']\n> > - controls = yaml.safe_load(data)['controls']\n> > - controls = [Control(*ctrl.popitem(), vendor) for ctrl in controls]\n> > + controls = []\n> > + for input in args.input:\n> > + data = open(input, 'rb').read()\n> > + vendor = yaml.safe_load(data)['vendor']\n> > + ctrls = yaml.safe_load(data)['controls']\n> > + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls]\n> >\n> > if args.template.endswith('.cpp.in'):\n> > data = generate_cpp(controls)\n> > --\n> > 2.34.1\n> >","headers":{"Return-Path":"","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 337EBC3220\n\tfor ;\n\tWed, 22 Nov 2023 15:53:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 93CEB629AF;\n\tWed, 22 Nov 2023 16:53:41 +0100 (CET)","from mail-yw1-x1133.google.com (mail-yw1-x1133.google.com\n\t[IPv6:2607:f8b0:4864:20::1133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C0D0A61DAD\n\tfor ;\n\tWed, 22 Nov 2023 16:53:39 +0100 (CET)","by mail-yw1-x1133.google.com with SMTP id\n\t00721157ae682-5cc66213a34so8729947b3.1\n\tfor ;\n\tWed, 22 Nov 2023 07:53:39 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700668421;\n\tbh=oPZ8qoWZgDD0phQP3oQnlf62ux/1N32kXv0Tzw4bPDE=;\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=VUzVNeXZDunvK+VQpDPZAUtszIVxU3DMla816fq12V9ZfVizutBthYK2Rm6APRMG1\n\trKUQsMpoU4Owa4NToS163vpixy89GEsCYspthRKw/QLpfDrWnP48QncZCNU4pn9n5x\n\tEv6npI6pYYuNkmNDWsez9jq68/gSPE+7H7t+WCPKFT+yD9AUUY0LsjnJcJzaL1mwI1\n\toqLK907Cm9efD3rYebFrBaEXytGl1ngXlqEWoicnO399WL+usIgiGzgI9bwXfUoL+w\n\t3CLPx3p3wD/F+k8xlkNDUp+UwdC0q59MLuyLbfx+eMRdVHkq9f9sRwULG/ugFwSEse\n\tGbbGtKd8XtyUA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1700668418; x=1701273218;\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=o+xKwx1o28/gOgaUSgs8X0hKCLtiuQlfZ9fxr/F3CA4=;\n\tb=jIdEy0gHAVaHdfS1g5rvcrN3maBz+F6EZDGfkObuEXV8SnlVJfQQBzS/xnluY5O0MV\n\t0X8M16cVc468E3MQQzMdNW8Hs6BrZWMvJv/U98BBOlV15LIZFEZdsM7mw27B718RkGev\n\t5VX5NoeDdM7SpAXN5RJxvJg7L1fu+UTTpIMKAC35GBopq0M1LCNYp+x7BQQPAnc7lwk4\n\t22PfbOnqbyibV3611jr0n7gCt/aEJcFgIdLr/4OPMfYzPq/yYn7kU0I0HzRZv6nTdiyn\n\tVNBmtF6cDOrwUZvSXXGNvX6VWdC3zdAPXxXa8XGKj47OCo/gzC/2DnWstOvTkZXvY3n1\n\tGnxw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"jIdEy0gH\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1700668418; x=1701273218;\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=o+xKwx1o28/gOgaUSgs8X0hKCLtiuQlfZ9fxr/F3CA4=;\n\tb=BqXHOkLgiG0fBnTajWM+WIq0N0VeLp5sNlagxW8vowsoGGPzqvTnsfT07qy5PyKbMN\n\tIc44vAsiozjZJxaGG6+D9lrVA0YkLOAA+S8Vo+zj1Up0kUWvNr5awqqRoR/nrlkSYRpV\n\tz/jC+s4vBCTwypyjKV1qKz6z9LFjM/YMmvFBmN8qRyPGuNo/HUz3+3TUG7vKmfpkMwYt\n\tZe1XpMYeFGPosiv7jr/Xlr8lmUbT5nYbA+1vwhk7VJco9ew6slOFBxt4GHTkKa47Bp2c\n\trZLCDkErzOTR8voP7jfc/RPs+LtloOZiWiLIJguyOUkEj60ljjVsGUEUOrhbaDtFf/ih\n\t/twQ==","X-Gm-Message-State":"AOJu0YyfczXNdDROFEITgSXqzf32NdQte+iPi9vK7nrZ4sUbwdQqxSob\n\t+eRGqOdcgydcOS61XuuCGSlz7TZPIfd43duY4qDAVw==","X-Google-Smtp-Source":"AGHT+IGgEkf997QZVqKGVTebTPU1Py2JRHtb4fTzep8FQVol9Ns3lgZO+63UJKQcYV4RV9R5tG29PMOqZvv4lnGsQgM=","X-Received":"by 2002:a0d:f102:0:b0:5cb:eac7:38aa with SMTP id\n\ta2-20020a0df102000000b005cbeac738aamr2792544ywf.35.1700668417913;\n\tWed, 22 Nov 2023 07:53:37 -0800 (PST)","MIME-Version":"1.0","References":"<20231121145350.5956-1-naush@raspberrypi.com>\n\t<20231121145350.5956-4-naush@raspberrypi.com>\n\t","In-Reply-To":"","Date":"Wed, 22 Nov 2023 15:53:03 +0000","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28128,"web_url":"https://patchwork.libcamera.org/comment/28128/","msgid":"","date":"2023-11-22T16:05:16","subject":"Re: [libcamera-devel] [PATCH v2 3/6] controls: build: Allow\n\tseparate vendor control YAML files","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Wed, Nov 22, 2023 at 03:53:03PM +0000, Naushir Patuck wrote:\n> Hi Jacopo,\n>\n> On Wed, 22 Nov 2023 at 15:24, Jacopo Mondi\n> wrote:\n> >\n> > Hi Naush\n> >\n> > On Tue, Nov 21, 2023 at 02:53:47PM +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> > > 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 \n> > > ---\n> > > include/libcamera/meson.build | 42 ++++++++++++++++++++++++++++-\n> > > meson.build | 2 ++\n> > > src/libcamera/control_ids_rpi.yaml | 16 +++++++++++\n> > > src/libcamera/meson.build | 12 +++++++++\n> > > src/py/libcamera/gen-py-controls.py | 10 +++----\n> > > src/py/libcamera/meson.build | 8 ++++++\n> > > utils/gen-controls.py | 12 +++++----\n> > > 7 files changed, 91 insertions(+), 11 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 5fb772e6dd14..54123c5b858d 100644\n> > > --- a/include/libcamera/meson.build\n> > > +++ b/include/libcamera/meson.build\n> > > @@ -38,10 +38,50 @@ control_source_files = {\n> > > 'property_ids': 'properties',\n> > > }\n> >\n> > I really like the series, but this patch is the piece I like less.\n> >\n> > >\n> > > +vendor_controls_mapping = {\n> > > + # Mapping of vendor (pipeline handler) specific controls files\n> > > + 'controls':\n> > > + {\n> >\n> > Isn't this usually written as\n> >\n> > 'controls' : {\n> >\n> > }\n> >\n> > in meson ?\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')\n> > > +\n> > > + # Start by populating the vendor specific mappings into vendor_ctrl_files\n> > > + # and vendor_prop_files. These will be cached for later use.\n> > > + vendor_files = []\n> > > + foreach pipeline, file : vendor_controls_mapping[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> > > + foreach file : vendor_files\n> > > + input_files += files('../../src/libcamera/' + file)\n> > > + endforeach\n> > > +\n> >\n> > Wouldn't it be simpler if we try to handle libcamera, draft and vendor\n> > controls as first class citizens in the same manner, instead of\n> > maintaing the \"libcamera\" ones on a different level ?\n> >\n> > I guess if we could do something like:\n> >\n> > -------------------------------------------------------------------------------\n> >\n> > controls_map {\n> > 'controls' : {\n> > 'libcamera' : controls_ids.yaml,\n> > 'drafts' : controls_ids_draft.yaml\n> > 'rpi/pisp': 'control_ids_rpi.yaml',\n> > 'rpi/vc4': 'control_ids_rpi.yaml'\n> > }\n> >\n> > 'properties' = {\n> > 'libcamera' : controls_ids.yaml,\n> > 'drafts' : controls_ids_draft.yaml\n> > }\n> > }\n> >\n> > control_headers = []\n> > ranges_file = files('../../src/libcamera/control_ranges.yaml')\n> > foreach mode, entry : controls_map\n> > files_list = []\n> > input_files=[]\n> > foreach vendor, header : entry\n> > if vendor != 'libcamera' or vendor != 'draft'\n> > if vendor not in pipelines\n> > continue\n> > endif\n> > endif\n> >\n> > if header in files_list\n> > continue\n> > endif\n> > files_list += header\n> >\n> > input_files += files('../../src/libcamera/' + header)\n> > endforeach\n> >\n> > outfile=''\n> > if mode == 'controls'\n> > outfile = 'control_ids.h'\n> > else\n> > outfile = 'property_ids.h'\n> > endif\n> >\n> > template = outfile + '.in'\n> > control_headers += custom_target(header + '_h',\n> > input : input_files,\n> > output : outfile,\n> > command : [gen_controls, '-o', '@OUTPUT@',\n> > '--mode', mode, '-t', template,\n> > '-r', ranges_file, '@INPUT@'],\n> > install : true,\n> > install_dir : libcamera_headers_install_dir)\n> > endforeach\n> >\n> > libcamera_public_headers += control_headers\n> >\n> > -------------------------------------------------------------------------------\n> >\n> > isn't it nicer ?\n> >\n> > The above code is valid in meson, but the meson.build in\n> > src/libcamera/ should be updated accordingly.\n> >\n> > In general, I think the relevant part here is\n> >\n> > controls_map {\n> > 'controls' : {\n> > 'libcamera' : controls_ids.yaml,\n> > 'drafts' : controls_ids_draft.yaml\n> > 'rpi/pisp': 'control_ids_rpi.yaml',\n> > 'rpi/vc4': 'control_ids_rpi.yaml'\n> > }\n> >\n> > 'properties' = {\n> > 'libcamera' : controls_ids.yaml,\n> > 'drafts' : controls_ids_draft.yaml\n> > }\n> > }\n> >\n> > As seeing all the supported controls and properties listed together\n> > might be easier to maintain and parse ?\n> >\n> > What do you think ?\n>\n> That looks reasonable. I guess I wanted to keep things seperate for\n> vendor controls, but no reason not to combine the libcamera/draft and\n> vendor cases into one list. I can make this change for v3.\n\nLet me know if and how I can help. The change is quite invasive in the\nwhole patch series and I don't want to make it more painful than\nnecessary for you\n\n\n>\n> Regards,\n> Naush\n>\n>\n> >\n> >\n> > > template_file = files(header + '.h.in')\n> > > control_headers += custom_target(header + '_h',\n> > > input : input_files,\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..a6f8b43a5f62\n> > > --- /dev/null\n> > > +++ b/src/libcamera/control_ids_rpi.yaml\n> > > @@ -0,0 +1,16 @@\n> > > +# SPDX-License-Identifier: LGPL-2.1-or-later\n> > > +#\n> > > +# Copyright (C) 2023, Raspberry Pi Ltd\n> > > +#\n> > > +%YAML 1.1\n> > > +---\n> > > +vendor: rpi\n> > > +controls:\n> > > + - PispConfigDumpFile:\n> > > + type: string\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 05ee38daf22b..7eb26ccbb7f5 100644\n> > > --- a/src/libcamera/meson.build\n> > > +++ b/src/libcamera/meson.build\n> > > @@ -129,6 +129,18 @@ control_sources = []\n> > >\n> > > foreach source, mode : control_source_files\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> > > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> > > index ea28f0139f23..e2ddad8581fd 100755\n> > > --- a/src/py/libcamera/gen-py-controls.py\n> > > +++ b/src/py/libcamera/gen-py-controls.py\n> > > @@ -91,7 +91,7 @@ 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> > > + parser.add_argument('input', type=str, nargs='+',\n> > > help='Input file name.')\n> > > parser.add_argument('-t', dest='template', type=str, required=True,\n> > > help='Template file name.')\n> > > @@ -103,11 +103,11 @@ 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> > > -\n> > > controls = {}\n> > > - vendor = yaml.safe_load(data)['vendor']\n> > > - controls[vendor] = yaml.safe_load(data)['controls']\n> > > + for input in args.input:\n> > > + data = open(input, 'rb').read()\n> > > + vendor = yaml.safe_load(data)['vendor']\n> > > + controls[vendor] = 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 1c3ea1843ac0..ea38ad6ca829 100644\n> > > --- a/src/py/libcamera/meson.build\n> > > +++ b/src/py/libcamera/meson.build\n> > > @@ -33,6 +33,10 @@ gen_py_controls_template = files('py_controls_generated.cpp.in')\n> > >\n> > > gen_py_controls = files('gen-py-controls.py')\n> > >\n> > > +foreach file : vendor_ctrl_files\n> > > + gen_py_controls_input_files += files('../../libcamera/' + file)\n> > > +endforeach\n> > > +\n> > > pycamera_sources += custom_target('py_gen_controls',\n> > > input : gen_py_controls_input_files,\n> > > output : ['py_controls_generated.cpp'],\n> > > @@ -44,6 +48,10 @@ pycamera_sources += custom_target('py_gen_controls',\n> > > gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> > > gen_py_properties_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> > > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > > index 8af33d29cd07..d7862142b8c1 100755\n> > > --- a/utils/gen-controls.py\n> > > +++ b/utils/gen-controls.py\n> > > @@ -359,7 +359,7 @@ 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> > > + parser.add_argument('input', type=str, nargs='+',\n> > > help='Input file name.')\n> > > parser.add_argument('-t', dest='template', type=str, required=True,\n> > > help='Template file name.')\n> > > @@ -367,10 +367,12 @@ def main(argv):\n> > > help='Mode of operation')\n> > > args = parser.parse_args(argv[1:])\n> > >\n> > > - data = open(args.input, 'rb').read()\n> > > - vendor = yaml.safe_load(data)['vendor']\n> > > - controls = yaml.safe_load(data)['controls']\n> > > - controls = [Control(*ctrl.popitem(), vendor) for ctrl in controls]\n> > > + controls = []\n> > > + for input in args.input:\n> > > + data = open(input, 'rb').read()\n> > > + vendor = yaml.safe_load(data)['vendor']\n> > > + ctrls = yaml.safe_load(data)['controls']\n> > > + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls]\n> > >\n> > > if args.template.endswith('.cpp.in'):\n> > > data = generate_cpp(controls)\n> > > --\n> > > 2.34.1\n> > >","headers":{"Return-Path":"","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 7E0C0BDE6B\n\tfor ;\n\tWed, 22 Nov 2023 16:05:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EF0C3629BC;\n\tWed, 22 Nov 2023 17:05:20 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B8D3761DAD\n\tfor ;\n\tWed, 22 Nov 2023 17:05:19 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6D69E29A;\n\tWed, 22 Nov 2023 17:04:48 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700669121;\n\tbh=0my4pDnfiO16j3tI8eingrRfzFfhnY5ufNIUCRGa3lg=;\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=Jr6X3cYLRMP6XRWbAYb44BfRoJl+iBMQivtdHf4IwK/fCKAgIeKPq+nwf3AvRZo0n\n\tRcTOxbZLUGyxeHWxUdgDxrp/e6eLKtodgad7W5zCHEsy8dN6m79zS0rlQcnybk+0QV\n\tR2exkqeX0fmtLdvN7s5ThkGue2ycWaoY8ZR5DF0wAWjPL8CwmWSD+EizHNtYobv2Ma\n\thfz3cqkO0DAADwtgQoy50JkzD2iNgA9XbnW+DMV973fmS3CkspjK9Xf5NpwUoJ3GsO\n\tVXmbc3iNuUwHQVj7eTKQGSliEtvyrUF/9TVk/9eo+6PetP9KHMYkjTpbk5reoOvOZ+\n\tb2Pp87MRkc2Lg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1700669088;\n\tbh=0my4pDnfiO16j3tI8eingrRfzFfhnY5ufNIUCRGa3lg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UYTA+bvo3grA4r+CyqD9cwIwdVPIkyb0XhWr+oYIlLCGF2PKYd4pC7a8Y8Oliif2x\n\tTA71kq/n33iVllIZc27KS4/PPvcz7OQqUg+KLmsZv9T+g80kQtchiM/Ss0buEeSEAN\n\ts9aysJ+1+CNmjdn5kugOMoo0cXWldbY/OG7cwPqM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"UYTA+bvo\"; dkim-atps=neutral","Date":"Wed, 22 Nov 2023 17:05:16 +0100","To":"Naushir Patuck ","Message-ID":"","References":"<20231121145350.5956-1-naush@raspberrypi.com>\n\t<20231121145350.5956-4-naush@raspberrypi.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28130,"web_url":"https://patchwork.libcamera.org/comment/28130/","msgid":"<43s4cbgj35c7chykypsu7ybly6ub673zfqncvg54qc2dnoxzzu@e5uj55vu6ra7>","date":"2023-11-22T16:54:48","subject":"Re: [libcamera-devel] [PATCH v2 3/6] controls: build: Allow\n\tseparate vendor control YAML files","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"One quick note\n\nOn Wed, Nov 22, 2023 at 04:24:40PM +0100, Jacopo Mondi via libcamera-devel wrote:\n> Hi Naush\n>\n> On Tue, Nov 21, 2023 at 02:53:47PM +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> > 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 \n> > ---\n> > include/libcamera/meson.build | 42 ++++++++++++++++++++++++++++-\n> > meson.build | 2 ++\n> > src/libcamera/control_ids_rpi.yaml | 16 +++++++++++\n> > src/libcamera/meson.build | 12 +++++++++\n> > src/py/libcamera/gen-py-controls.py | 10 +++----\n> > src/py/libcamera/meson.build | 8 ++++++\n> > utils/gen-controls.py | 12 +++++----\n> > 7 files changed, 91 insertions(+), 11 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 5fb772e6dd14..54123c5b858d 100644\n> > --- a/include/libcamera/meson.build\n> > +++ b/include/libcamera/meson.build\n> > @@ -38,10 +38,50 @@ control_source_files = {\n> > 'property_ids': 'properties',\n> > }\n>\n> I really like the series, but this patch is the piece I like less.\n>\n> >\n> > +vendor_controls_mapping = {\n> > + # Mapping of vendor (pipeline handler) specific controls files\n> > + 'controls':\n> > + {\n>\n> Isn't this usually written as\n>\n> 'controls' : {\n>\n> }\n>\n> in meson ?\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')\n> > +\n> > + # Start by populating the vendor specific mappings into vendor_ctrl_files\n> > + # and vendor_prop_files. These will be cached for later use.\n> > + vendor_files = []\n> > + foreach pipeline, file : vendor_controls_mapping[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> > + foreach file : vendor_files\n> > + input_files += files('../../src/libcamera/' + file)\n> > + endforeach\n> > +\n>\n> Wouldn't it be simpler if we try to handle libcamera, draft and vendor\n> controls as first class citizens in the same manner, instead of\n> maintaing the \"libcamera\" ones on a different level ?\n>\n> I guess if we could do something like:\n>\n> -------------------------------------------------------------------------------\n>\n> controls_map {\n> 'controls' : {\n> 'libcamera' : controls_ids.yaml,\n> 'drafts' : controls_ids_draft.yaml\n> 'rpi/pisp': 'control_ids_rpi.yaml',\n> 'rpi/vc4': 'control_ids_rpi.yaml'\n> }\n>\n> 'properties' = {\n> 'libcamera' : controls_ids.yaml,\n> 'drafts' : controls_ids_draft.yaml\n> }\n> }\n>\n> control_headers = []\n> ranges_file = files('../../src/libcamera/control_ranges.yaml')\n> foreach mode, entry : controls_map\n> files_list = []\n> input_files=[]\n> foreach vendor, header : entry\n> if vendor != 'libcamera' or vendor != 'draft'\n\nThis should probably be an 'and' and not an 'or\n\n> if vendor not in pipelines\n> continue\n> endif\n> endif\n>\n> if header in files_list\n> continue\n> endif\n> files_list += header\n>\n> input_files += files('../../src/libcamera/' + header)\n> endforeach\n>\n> outfile=''\n> if mode == 'controls'\n> outfile = 'control_ids.h'\n> else\n> outfile = 'property_ids.h'\n> endif\n>\n> template = outfile + '.in'\n> control_headers += custom_target(header + '_h',\n> input : input_files,\n> output : outfile,\n> command : [gen_controls, '-o', '@OUTPUT@',\n> '--mode', mode, '-t', template,\n> '-r', ranges_file, '@INPUT@'],\n> install : true,\n> install_dir : libcamera_headers_install_dir)\n> endforeach\n>\n> libcamera_public_headers += control_headers\n>\n> -------------------------------------------------------------------------------\n>\n> isn't it nicer ?\n>\n> The above code is valid in meson, but the meson.build in\n> src/libcamera/ should be updated accordingly.\n>\n> In general, I think the relevant part here is\n>\n> controls_map {\n> 'controls' : {\n> 'libcamera' : controls_ids.yaml,\n> 'drafts' : controls_ids_draft.yaml\n> 'rpi/pisp': 'control_ids_rpi.yaml',\n> 'rpi/vc4': 'control_ids_rpi.yaml'\n> }\n>\n> 'properties' = {\n> 'libcamera' : controls_ids.yaml,\n> 'drafts' : controls_ids_draft.yaml\n> }\n> }\n>\n> As seeing all the supported controls and properties listed together\n> might be easier to maintain and parse ?\n>\n> What do you think ?\n>\n>\n> > template_file = files(header + '.h.in')\n> > control_headers += custom_target(header + '_h',\n> > input : input_files,\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..a6f8b43a5f62\n> > --- /dev/null\n> > +++ b/src/libcamera/control_ids_rpi.yaml\n> > @@ -0,0 +1,16 @@\n> > +# SPDX-License-Identifier: LGPL-2.1-or-later\n> > +#\n> > +# Copyright (C) 2023, Raspberry Pi Ltd\n> > +#\n> > +%YAML 1.1\n> > +---\n> > +vendor: rpi\n> > +controls:\n> > + - PispConfigDumpFile:\n> > + type: string\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 05ee38daf22b..7eb26ccbb7f5 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -129,6 +129,18 @@ control_sources = []\n> >\n> > foreach source, mode : control_source_files\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> > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> > index ea28f0139f23..e2ddad8581fd 100755\n> > --- a/src/py/libcamera/gen-py-controls.py\n> > +++ b/src/py/libcamera/gen-py-controls.py\n> > @@ -91,7 +91,7 @@ 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> > + parser.add_argument('input', type=str, nargs='+',\n> > help='Input file name.')\n> > parser.add_argument('-t', dest='template', type=str, required=True,\n> > help='Template file name.')\n> > @@ -103,11 +103,11 @@ 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> > -\n> > controls = {}\n> > - vendor = yaml.safe_load(data)['vendor']\n> > - controls[vendor] = yaml.safe_load(data)['controls']\n> > + for input in args.input:\n> > + data = open(input, 'rb').read()\n> > + vendor = yaml.safe_load(data)['vendor']\n> > + controls[vendor] = 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 1c3ea1843ac0..ea38ad6ca829 100644\n> > --- a/src/py/libcamera/meson.build\n> > +++ b/src/py/libcamera/meson.build\n> > @@ -33,6 +33,10 @@ gen_py_controls_template = files('py_controls_generated.cpp.in')\n> >\n> > gen_py_controls = files('gen-py-controls.py')\n> >\n> > +foreach file : vendor_ctrl_files\n> > + gen_py_controls_input_files += files('../../libcamera/' + file)\n> > +endforeach\n> > +\n> > pycamera_sources += custom_target('py_gen_controls',\n> > input : gen_py_controls_input_files,\n> > output : ['py_controls_generated.cpp'],\n> > @@ -44,6 +48,10 @@ pycamera_sources += custom_target('py_gen_controls',\n> > gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> > gen_py_properties_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> > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > index 8af33d29cd07..d7862142b8c1 100755\n> > --- a/utils/gen-controls.py\n> > +++ b/utils/gen-controls.py\n> > @@ -359,7 +359,7 @@ 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> > + parser.add_argument('input', type=str, nargs='+',\n> > help='Input file name.')\n> > parser.add_argument('-t', dest='template', type=str, required=True,\n> > help='Template file name.')\n> > @@ -367,10 +367,12 @@ def main(argv):\n> > help='Mode of operation')\n> > args = parser.parse_args(argv[1:])\n> >\n> > - data = open(args.input, 'rb').read()\n> > - vendor = yaml.safe_load(data)['vendor']\n> > - controls = yaml.safe_load(data)['controls']\n> > - controls = [Control(*ctrl.popitem(), vendor) for ctrl in controls]\n> > + controls = []\n> > + for input in args.input:\n> > + data = open(input, 'rb').read()\n> > + vendor = yaml.safe_load(data)['vendor']\n> > + ctrls = yaml.safe_load(data)['controls']\n> > + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls]\n> >\n> > if args.template.endswith('.cpp.in'):\n> > data = generate_cpp(controls)\n> > --\n> > 2.34.1\n> >","headers":{"Return-Path":"","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 4F165BDE6B\n\tfor ;\n\tWed, 22 Nov 2023 16:54:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 99CF1629BC;\n\tWed, 22 Nov 2023 17:54:52 +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 AEB9F61DAD\n\tfor ;\n\tWed, 22 Nov 2023 17:54:51 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4691329A;\n\tWed, 22 Nov 2023 17:54:20 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700672092;\n\tbh=Gv/cJVKCIoNQQN1lM5mC5dATZqiCPg0E/L1uMv3TZYI=;\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=p9aI5cDD41mRNRxrHANzcRgyvgd2LrIDwiK7kbCzF7k2GLYC/Q8zy9mdqcAoP2A8F\n\tvrrdXwZNBZJA+qiMuM5qsR1NMlSx3kqcLTah0C2v8ymx/IkgwzwKnAHL7LZ/hAtf0m\n\tdd21TMlL2dYcUeVo9MoZiXBNzRgCwIWkFK3ei5t2uhvi/4esQ52QAJvxnb7f7KbwOT\n\tFcdihr5AlZ/8ZjA36wDzKvJ3zpPz4bK9O4hNIilr0Fk2daY6EiJuX+DLadjgZHBL7P\n\tVMpLLlVU9RkBGeiJtjqKJLflw5xhNxZykKHUGrq8hy7r1+QaK4mj/dmbYT0gmPZuCG\n\tAjE+NTPRuGn6A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1700672060;\n\tbh=Gv/cJVKCIoNQQN1lM5mC5dATZqiCPg0E/L1uMv3TZYI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=X4wqVcpEpW4KzBFIiMCAgREsu8eNbYXshqyZD0/vwit7nRls7/hKQt+bfv4CDgGBe\n\tVh3CvbY+VdYp0GB8pjqtjVZI6H8/+TL85JbW/85s9Ao/UuNtvsKQXRyf62drFZP+sd\n\tdpHGeluRwp+6JVI8SsrlzDchqMQHNmQGxq1YvaAQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"X4wqVcpE\"; dkim-atps=neutral","Date":"Wed, 22 Nov 2023 17:54:48 +0100","To":"Jacopo Mondi ","Message-ID":"<43s4cbgj35c7chykypsu7ybly6ub673zfqncvg54qc2dnoxzzu@e5uj55vu6ra7>","References":"<20231121145350.5956-1-naush@raspberrypi.com>\n\t<20231121145350.5956-4-naush@raspberrypi.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28144,"web_url":"https://patchwork.libcamera.org/comment/28144/","msgid":"","date":"2023-11-23T08:02:28","subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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 Jacopo,\n\nOn Wed, 22 Nov 2023 at 16:05, Jacopo Mondi\n wrote:\n>\n> Hi Naush\n>\n> On Wed, Nov 22, 2023 at 03:53:03PM +0000, Naushir Patuck wrote:\n> > Hi Jacopo,\n> >\n> > On Wed, 22 Nov 2023 at 15:24, Jacopo Mondi\n> > wrote:\n> > >\n> > > Hi Naush\n> > >\n> > > On Tue, Nov 21, 2023 at 02:53:47PM +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> > > > 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 \n> > > > ---\n> > > > include/libcamera/meson.build | 42 ++++++++++++++++++++++++++++-\n> > > > meson.build | 2 ++\n> > > > src/libcamera/control_ids_rpi.yaml | 16 +++++++++++\n> > > > src/libcamera/meson.build | 12 +++++++++\n> > > > src/py/libcamera/gen-py-controls.py | 10 +++----\n> > > > src/py/libcamera/meson.build | 8 ++++++\n> > > > utils/gen-controls.py | 12 +++++----\n> > > > 7 files changed, 91 insertions(+), 11 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 5fb772e6dd14..54123c5b858d 100644\n> > > > --- a/include/libcamera/meson.build\n> > > > +++ b/include/libcamera/meson.build\n> > > > @@ -38,10 +38,50 @@ control_source_files = {\n> > > > 'property_ids': 'properties',\n> > > > }\n> > >\n> > > I really like the series, but this patch is the piece I like less.\n> > >\n> > > >\n> > > > +vendor_controls_mapping = {\n> > > > + # Mapping of vendor (pipeline handler) specific controls files\n> > > > + 'controls':\n> > > > + {\n> > >\n> > > Isn't this usually written as\n> > >\n> > > 'controls' : {\n> > >\n> > > }\n> > >\n> > > in meson ?\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')\n> > > > +\n> > > > + # Start by populating the vendor specific mappings into vendor_ctrl_files\n> > > > + # and vendor_prop_files. These will be cached for later use.\n> > > > + vendor_files = []\n> > > > + foreach pipeline, file : vendor_controls_mapping[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> > > > + foreach file : vendor_files\n> > > > + input_files += files('../../src/libcamera/' + file)\n> > > > + endforeach\n> > > > +\n> > >\n> > > Wouldn't it be simpler if we try to handle libcamera, draft and vendor\n> > > controls as first class citizens in the same manner, instead of\n> > > maintaing the \"libcamera\" ones on a different level ?\n> > >\n> > > I guess if we could do something like:\n> > >\n> > > -------------------------------------------------------------------------------\n> > >\n> > > controls_map {\n> > > 'controls' : {\n> > > 'libcamera' : controls_ids.yaml,\n> > > 'drafts' : controls_ids_draft.yaml\n> > > 'rpi/pisp': 'control_ids_rpi.yaml',\n> > > 'rpi/vc4': 'control_ids_rpi.yaml'\n> > > }\n> > >\n> > > 'properties' = {\n> > > 'libcamera' : controls_ids.yaml,\n> > > 'drafts' : controls_ids_draft.yaml\n> > > }\n> > > }\n> > >\n> > > control_headers = []\n> > > ranges_file = files('../../src/libcamera/control_ranges.yaml')\n> > > foreach mode, entry : controls_map\n> > > files_list = []\n> > > input_files=[]\n> > > foreach vendor, header : entry\n> > > if vendor != 'libcamera' or vendor != 'draft'\n> > > if vendor not in pipelines\n> > > continue\n> > > endif\n> > > endif\n> > >\n> > > if header in files_list\n> > > continue\n> > > endif\n> > > files_list += header\n> > >\n> > > input_files += files('../../src/libcamera/' + header)\n> > > endforeach\n> > >\n> > > outfile=''\n> > > if mode == 'controls'\n> > > outfile = 'control_ids.h'\n> > > else\n> > > outfile = 'property_ids.h'\n> > > endif\n> > >\n> > > template = outfile + '.in'\n> > > control_headers += custom_target(header + '_h',\n> > > input : input_files,\n> > > output : outfile,\n> > > command : [gen_controls, '-o', '@OUTPUT@',\n> > > '--mode', mode, '-t', template,\n> > > '-r', ranges_file, '@INPUT@'],\n> > > install : true,\n> > > install_dir : libcamera_headers_install_dir)\n> > > endforeach\n> > >\n> > > libcamera_public_headers += control_headers\n> > >\n> > > -------------------------------------------------------------------------------\n> > >\n> > > isn't it nicer ?\n> > >\n> > > The above code is valid in meson, but the meson.build in\n> > > src/libcamera/ should be updated accordingly.\n> > >\n> > > In general, I think the relevant part here is\n> > >\n> > > controls_map {\n> > > 'controls' : {\n> > > 'libcamera' : controls_ids.yaml,\n> > > 'drafts' : controls_ids_draft.yaml\n> > > 'rpi/pisp': 'control_ids_rpi.yaml',\n> > > 'rpi/vc4': 'control_ids_rpi.yaml'\n> > > }\n> > >\n> > > 'properties' = {\n> > > 'libcamera' : controls_ids.yaml,\n> > > 'drafts' : controls_ids_draft.yaml\n> > > }\n> > > }\n> > >\n> > > As seeing all the supported controls and properties listed together\n> > > might be easier to maintain and parse ?\n> > >\n> > > What do you think ?\n> >\n> > That looks reasonable. I guess I wanted to keep things seperate for\n> > vendor controls, but no reason not to combine the libcamera/draft and\n> > vendor cases into one list. I can make this change for v3.\n>\n> Let me know if and how I can help. The change is quite invasive in the\n> whole patch series and I don't want to make it more painful than\n> necessary for you\n\nNo worries, I've already made the changes. Should I wait for more\nfeedback or go ahead and push a v3?\n\nRegards,\nNaush\n\n\n>\n>\n> >\n> > Regards,\n> > Naush\n> >\n> >\n> > >\n> > >\n> > > > template_file = files(header + '.h.in')\n> > > > control_headers += custom_target(header + '_h',\n> > > > input : input_files,\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..a6f8b43a5f62\n> > > > --- /dev/null\n> > > > +++ b/src/libcamera/control_ids_rpi.yaml\n> > > > @@ -0,0 +1,16 @@\n> > > > +# SPDX-License-Identifier: LGPL-2.1-or-later\n> > > > +#\n> > > > +# Copyright (C) 2023, Raspberry Pi Ltd\n> > > > +#\n> > > > +%YAML 1.1\n> > > > +---\n> > > > +vendor: rpi\n> > > > +controls:\n> > > > + - PispConfigDumpFile:\n> > > > + type: string\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 05ee38daf22b..7eb26ccbb7f5 100644\n> > > > --- a/src/libcamera/meson.build\n> > > > +++ b/src/libcamera/meson.build\n> > > > @@ -129,6 +129,18 @@ control_sources = []\n> > > >\n> > > > foreach source, mode : control_source_files\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> > > > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> > > > index ea28f0139f23..e2ddad8581fd 100755\n> > > > --- a/src/py/libcamera/gen-py-controls.py\n> > > > +++ b/src/py/libcamera/gen-py-controls.py\n> > > > @@ -91,7 +91,7 @@ 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> > > > + parser.add_argument('input', type=str, nargs='+',\n> > > > help='Input file name.')\n> > > > parser.add_argument('-t', dest='template', type=str, required=True,\n> > > > help='Template file name.')\n> > > > @@ -103,11 +103,11 @@ 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> > > > -\n> > > > controls = {}\n> > > > - vendor = yaml.safe_load(data)['vendor']\n> > > > - controls[vendor] = yaml.safe_load(data)['controls']\n> > > > + for input in args.input:\n> > > > + data = open(input, 'rb').read()\n> > > > + vendor = yaml.safe_load(data)['vendor']\n> > > > + controls[vendor] = 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 1c3ea1843ac0..ea38ad6ca829 100644\n> > > > --- a/src/py/libcamera/meson.build\n> > > > +++ b/src/py/libcamera/meson.build\n> > > > @@ -33,6 +33,10 @@ gen_py_controls_template = files('py_controls_generated.cpp.in')\n> > > >\n> > > > gen_py_controls = files('gen-py-controls.py')\n> > > >\n> > > > +foreach file : vendor_ctrl_files\n> > > > + gen_py_controls_input_files += files('../../libcamera/' + file)\n> > > > +endforeach\n> > > > +\n> > > > pycamera_sources += custom_target('py_gen_controls',\n> > > > input : gen_py_controls_input_files,\n> > > > output : ['py_controls_generated.cpp'],\n> > > > @@ -44,6 +48,10 @@ pycamera_sources += custom_target('py_gen_controls',\n> > > > gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> > > > gen_py_properties_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> > > > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > > > index 8af33d29cd07..d7862142b8c1 100755\n> > > > --- a/utils/gen-controls.py\n> > > > +++ b/utils/gen-controls.py\n> > > > @@ -359,7 +359,7 @@ 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> > > > + parser.add_argument('input', type=str, nargs='+',\n> > > > help='Input file name.')\n> > > > parser.add_argument('-t', dest='template', type=str, required=True,\n> > > > help='Template file name.')\n> > > > @@ -367,10 +367,12 @@ def main(argv):\n> > > > help='Mode of operation')\n> > > > args = parser.parse_args(argv[1:])\n> > > >\n> > > > - data = open(args.input, 'rb').read()\n> > > > - vendor = yaml.safe_load(data)['vendor']\n> > > > - controls = yaml.safe_load(data)['controls']\n> > > > - controls = [Control(*ctrl.popitem(), vendor) for ctrl in controls]\n> > > > + controls = []\n> > > > + for input in args.input:\n> > > > + data = open(input, 'rb').read()\n> > > > + vendor = yaml.safe_load(data)['vendor']\n> > > > + ctrls = yaml.safe_load(data)['controls']\n> > > > + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls]\n> > > >\n> > > > if args.template.endswith('.cpp.in'):\n> > > > data = generate_cpp(controls)\n> > > > --\n> > > > 2.34.1\n> > > >","headers":{"Return-Path":"","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 E1BAABDE6B\n\tfor ;\n\tThu, 23 Nov 2023 08:03:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4C2E1629BB;\n\tThu, 23 Nov 2023 09:03:00 +0100 (CET)","from mail-yw1-x1130.google.com (mail-yw1-x1130.google.com\n\t[IPv6:2607:f8b0:4864:20::1130])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1D93B61DAA\n\tfor ;\n\tThu, 23 Nov 2023 09:02:58 +0100 (CET)","by mail-yw1-x1130.google.com with SMTP id\n\t00721157ae682-5cd2f1a198cso4268767b3.0\n\tfor ;\n\tThu, 23 Nov 2023 00:02:58 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700726580;\n\tbh=YXkaTb1xtiCCr0wIv7SBVtEtM3wp+cIWndaa3Dehefc=;\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=JU5U0mnm/pS9ZZzk7ohZ7Q/N16SSYB9A8hH5yyqfQOgclxIuDyqnkhPFswY8igwlH\n\tdQpZO/pNEdEG1dMWqOCakpkzXABDfd+u8CAbe0tjPjZpmpd/rLf9Qv1JZurLW8Tyws\n\tqXC4qNKGhCb/torFwQa3YraiTRRg3Pb4EVhM6VFPeWw06DAU09t85qe/SCKzuF5XZ+\n\tt9nauEWpm27CWYgcmsUzQoFaFrh2jKxWb2bZAFm3ps1gbFeiue4Pm6od7ep+VmyKz2\n\tCjcuas/csbM3wna8eXJgCnYR+7U/lvQL+f5TU89bXuW+p6SCoebVswUSOl4hqIzMES\n\tooJSu1qq3/icw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1700726577; x=1701331377;\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=+1pnROw2G1GRJ9WIwC8AtrWcHUqASx0XfaMyQwC7f+o=;\n\tb=acjlXZiIg8chIFGkWO89Ix0I9/IrjRaeaH25GvPbGgoIIQTgQDYLEXhx3dGZC7CgpB\n\tDp/xMUebC9tbGo7akCYbJ3Zezqltu9Szff61Pn4iU+Sar0Q6ctWXf8yPsqmFtPl7HdZL\n\tCElh1NQ3Q0y0GF7F5AOAQE23MtyI3TIwA8TiL25vbF+PLTjHlKo5qZRTz82VDWFXnmVK\n\teP5cP4OPWwaoIjXfpCVAfkfndwQKhkomchmZWEwNecd2AfokglVtsC1YqK3J6a0WXBs2\n\tmn0aFlGrupApb6wwZlmU+0sDo0HCCQ+35YH8FUBAB8AdYC4C/5H4L+mIl3iUdhdf1K9m\n\t5mjA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"acjlXZiI\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1700726577; x=1701331377;\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=+1pnROw2G1GRJ9WIwC8AtrWcHUqASx0XfaMyQwC7f+o=;\n\tb=hRnofbcl+LKHmgkXjCu4ALheoyuMrK/+ue2Rd6H8xVLTCtEKjUihIvGAybomDJQaZ0\n\tEFme159mqAjMnSPDIlIGRJSL4+7F0I/tnb3FbIixz8ebXx7YGaFLqPpNI5tPR8v5KH0o\n\tlpAZnpOKz54eJU3zREPXQ9DD72ynXmGxuwqmR15WeRtlb/ocGlBw4Saa7XQIQNQY3Z0W\n\tYCkBRl0JRTnMo9/fK+lLK2pFaAdgsqS3fcQlVmZnCiRK980aQQ58G8MT3En732ablZjE\n\tSgt3sziVJWUXaP5SM/QujA9pcmzpwJYETU0Pvt8RiA8HhUO+bs3ZsoQfbMldG5AfVv99\n\tG3HA==","X-Gm-Message-State":"AOJu0Yyx+xP0fim7frm5kXUxADPw1UH3XlsG+iEeD2LN3aqI/cSMI9/Z\n\tsXsJlm5A0GA9QKyQ/EqOaYocKI0E+tZgOy8xg2CGfg==","X-Google-Smtp-Source":"AGHT+IHV6cP4iuVgF+6AdavfZMzvlcFWtvibmxRt3XN04Eo5lKp3DTF0ELD5Dz/Bw0/CTzfS53DuVzX3FPm267kEoaA=","X-Received":"by 2002:a81:8485:0:b0:5c8:8098:7e60 with SMTP id\n\tu127-20020a818485000000b005c880987e60mr5286201ywf.24.1700726576825;\n\tThu, 23 Nov 2023 00:02:56 -0800 (PST)","MIME-Version":"1.0","References":"<20231121145350.5956-1-naush@raspberrypi.com>\n\t<20231121145350.5956-4-naush@raspberrypi.com>\n\t\n\t\n\t","In-Reply-To":"","Date":"Thu, 23 Nov 2023 08:02:28 +0000","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28153,"web_url":"https://patchwork.libcamera.org/comment/28153/","msgid":"<20231123121348.GK15697@pendragon.ideasonboard.com>","date":"2023-11-23T12:13:48","subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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":"Hello,\n\nOn Thu, Nov 23, 2023 at 08:02:28AM +0000, Naushir Patuck via libcamera-devel wrote:\n> On Wed, 22 Nov 2023 at 16:05, Jacopo Mondi wrote:\n> > On Wed, Nov 22, 2023 at 03:53:03PM +0000, Naushir Patuck wrote:\n> > > On Wed, 22 Nov 2023 at 15:24, Jacopo Mondi wrote:\n> > > > On Tue, Nov 21, 2023 at 02:53:47PM +0000, Naushir Patuck via libcamera-devel wrote:\n> > > > > Add support for using separate YAML files for vendor specific controls.\n\ns/vendor specific/vendor-specific/\n\nSame below where applicable.\n\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> > > > > 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 \n> > > > > ---\n> > > > > include/libcamera/meson.build | 42 ++++++++++++++++++++++++++++-\n> > > > > meson.build | 2 ++\n> > > > > src/libcamera/control_ids_rpi.yaml | 16 +++++++++++\n> > > > > src/libcamera/meson.build | 12 +++++++++\n> > > > > src/py/libcamera/gen-py-controls.py | 10 +++----\n> > > > > src/py/libcamera/meson.build | 8 ++++++\n> > > > > utils/gen-controls.py | 12 +++++----\n> > > > > 7 files changed, 91 insertions(+), 11 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 5fb772e6dd14..54123c5b858d 100644\n> > > > > --- a/include/libcamera/meson.build\n> > > > > +++ b/include/libcamera/meson.build\n> > > > > @@ -38,10 +38,50 @@ control_source_files = {\n> > > > > 'property_ids': 'properties',\n> > > > > }\n> > > >\n> > > > I really like the series, but this patch is the piece I like less.\n> > > >\n> > > > >\n> > > > > +vendor_controls_mapping = {\n> > > > > + # Mapping of vendor (pipeline handler) specific controls files\n> > > > > + 'controls':\n> > > > > + {\n> > > >\n> > > > Isn't this usually written as\n> > > >\n> > > > 'controls' : {\n> > > >\n> > > > }\n> > > >\n> > > > in meson ?\n\nWe seem to do that, yes.\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')\n> > > > > +\n> > > > > + # Start by populating the vendor specific mappings into vendor_ctrl_files\n> > > > > + # and vendor_prop_files. These will be cached for later use.\n> > > > > + vendor_files = []\n> > > > > + foreach pipeline, file : vendor_controls_mapping[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> > > > > + foreach file : vendor_files\n> > > > > + input_files += files('../../src/libcamera/' + file)\n> > > > > + endforeach\n> > > > > +\n> > > >\n> > > > Wouldn't it be simpler if we try to handle libcamera, draft and vendor\n> > > > controls as first class citizens in the same manner, instead of\n> > > > maintaing the \"libcamera\" ones on a different level ?\n> > > >\n> > > > I guess if we could do something like:\n> > > >\n> > > > -------------------------------------------------------------------------------\n> > > >\n> > > > controls_map {\n> > > > 'controls' : {\n> > > > 'libcamera' : controls_ids.yaml,\n> > > > 'drafts' : controls_ids_draft.yaml\n> > > > 'rpi/pisp': 'control_ids_rpi.yaml',\n> > > > 'rpi/vc4': 'control_ids_rpi.yaml'\n> > > > }\n> > > >\n> > > > 'properties' = {\n> > > > 'libcamera' : controls_ids.yaml,\n\nThis should be property_ids.yaml.\n\n> > > > 'drafts' : controls_ids_draft.yaml\n> > > > }\n> > > > }\n> > > >\n> > > > control_headers = []\n> > > > ranges_file = files('../../src/libcamera/control_ranges.yaml')\n> > > > foreach mode, entry : controls_map\n> > > > files_list = []\n> > > > input_files=[]\n> > > > foreach vendor, header : entry\n> > > > if vendor != 'libcamera' or vendor != 'draft'\n> > > > if vendor not in pipelines\n> > > > continue\n> > > > endif\n> > > > endif\n> > > >\n> > > > if header in files_list\n> > > > continue\n> > > > endif\n> > > > files_list += header\n> > > >\n> > > > input_files += files('../../src/libcamera/' + header)\n> > > > endforeach\n> > > >\n> > > > outfile=''\n> > > > if mode == 'controls'\n> > > > outfile = 'control_ids.h'\n> > > > else\n> > > > outfile = 'property_ids.h'\n> > > > endif\n> > > >\n> > > > template = outfile + '.in'\n> > > > control_headers += custom_target(header + '_h',\n> > > > input : input_files,\n> > > > output : outfile,\n> > > > command : [gen_controls, '-o', '@OUTPUT@',\n> > > > '--mode', mode, '-t', template,\n> > > > '-r', ranges_file, '@INPUT@'],\n> > > > install : true,\n> > > > install_dir : libcamera_headers_install_dir)\n> > > > endforeach\n> > > >\n> > > > libcamera_public_headers += control_headers\n> > > >\n> > > > -------------------------------------------------------------------------------\n> > > >\n> > > > isn't it nicer ?\n> > > >\n> > > > The above code is valid in meson, but the meson.build in\n> > > > src/libcamera/ should be updated accordingly.\n> > > >\n> > > > In general, I think the relevant part here is\n> > > >\n> > > > controls_map {\n> > > > 'controls' : {\n> > > > 'libcamera' : controls_ids.yaml,\n> > > > 'drafts' : controls_ids_draft.yaml\n> > > > 'rpi/pisp': 'control_ids_rpi.yaml',\n> > > > 'rpi/vc4': 'control_ids_rpi.yaml'\n> > > > }\n> > > >\n> > > > 'properties' = {\n> > > > 'libcamera' : controls_ids.yaml,\n> > > > 'drafts' : controls_ids_draft.yaml\n> > > > }\n> > > > }\n> > > >\n> > > > As seeing all the supported controls and properties listed together\n> > > > might be easier to maintain and parse ?\n> > > >\n> > > > What do you think ?\n> > >\n> > > That looks reasonable. I guess I wanted to keep things seperate for\n> > > vendor controls, but no reason not to combine the libcamera/draft and\n> > > vendor cases into one list. I can make this change for v3.\n\nThat seems fine to me too. If we go in that direction, should we replace\n'libcamera' with 'core', and name the file controls_ids_core.yaml ? All\ncontrols are libcamera controls.\n\n> > Let me know if and how I can help. The change is quite invasive in the\n> > whole patch series and I don't want to make it more painful than\n> > necessary for you\n> \n> No worries, I've already made the changes. Should I wait for more\n> feedback or go ahead and push a v3?\n> \n> > > > > template_file = files(header + '.h.in')\n> > > > > control_headers += custom_target(header + '_h',\n> > > > > input : input_files,\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..a6f8b43a5f62\n> > > > > --- /dev/null\n> > > > > +++ b/src/libcamera/control_ids_rpi.yaml\n> > > > > @@ -0,0 +1,16 @@\n> > > > > +# SPDX-License-Identifier: LGPL-2.1-or-later\n> > > > > +#\n> > > > > +# Copyright (C) 2023, Raspberry Pi Ltd\n> > > > > +#\n> > > > > +%YAML 1.1\n> > > > > +---\n> > > > > +vendor: rpi\n> > > > > +controls:\n> > > > > + - PispConfigDumpFile:\n> > > > > + type: string\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\nAs the series doesn't contain an implementation of this control I can't\nreally review the description properly, but that's fine, I understand it\nwill come later.\n\n> > > > > +\n> > > > > +...\n> > > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > > > > index 05ee38daf22b..7eb26ccbb7f5 100644\n> > > > > --- a/src/libcamera/meson.build\n> > > > > +++ b/src/libcamera/meson.build\n> > > > > @@ -129,6 +129,18 @@ control_sources = []\n> > > > >\n> > > > > foreach source, mode : control_source_files\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> > > > > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> > > > > index ea28f0139f23..e2ddad8581fd 100755\n> > > > > --- a/src/py/libcamera/gen-py-controls.py\n> > > > > +++ b/src/py/libcamera/gen-py-controls.py\n> > > > > @@ -91,7 +91,7 @@ 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> > > > > + parser.add_argument('input', type=str, nargs='+',\n> > > > > help='Input file name.')\n> > > > > parser.add_argument('-t', dest='template', type=str, required=True,\n> > > > > help='Template file name.')\n> > > > > @@ -103,11 +103,11 @@ 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> > > > > -\n> > > > > controls = {}\n> > > > > - vendor = yaml.safe_load(data)['vendor']\n> > > > > - controls[vendor] = yaml.safe_load(data)['controls']\n> > > > > + for input in args.input:\n> > > > > + data = open(input, 'rb').read()\n> > > > > + vendor = yaml.safe_load(data)['vendor']\n> > > > > + controls[vendor] = 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 1c3ea1843ac0..ea38ad6ca829 100644\n> > > > > --- a/src/py/libcamera/meson.build\n> > > > > +++ b/src/py/libcamera/meson.build\n> > > > > @@ -33,6 +33,10 @@ gen_py_controls_template = files('py_controls_generated.cpp.in')\n> > > > >\n> > > > > gen_py_controls = files('gen-py-controls.py')\n> > > > >\n> > > > > +foreach file : vendor_ctrl_files\n> > > > > + gen_py_controls_input_files += files('../../libcamera/' + file)\n> > > > > +endforeach\n> > > > > +\n> > > > > pycamera_sources += custom_target('py_gen_controls',\n> > > > > input : gen_py_controls_input_files,\n> > > > > output : ['py_controls_generated.cpp'],\n> > > > > @@ -44,6 +48,10 @@ pycamera_sources += custom_target('py_gen_controls',\n> > > > > gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> > > > > gen_py_properties_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> > > > > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > > > > index 8af33d29cd07..d7862142b8c1 100755\n> > > > > --- a/utils/gen-controls.py\n> > > > > +++ b/utils/gen-controls.py\n> > > > > @@ -359,7 +359,7 @@ 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> > > > > + parser.add_argument('input', type=str, nargs='+',\n> > > > > help='Input file name.')\n> > > > > parser.add_argument('-t', dest='template', type=str, required=True,\n> > > > > help='Template file name.')\n> > > > > @@ -367,10 +367,12 @@ def main(argv):\n> > > > > help='Mode of operation')\n> > > > > args = parser.parse_args(argv[1:])\n> > > > >\n> > > > > - data = open(args.input, 'rb').read()\n> > > > > - vendor = yaml.safe_load(data)['vendor']\n> > > > > - controls = yaml.safe_load(data)['controls']\n> > > > > - controls = [Control(*ctrl.popitem(), vendor) for ctrl in controls]\n> > > > > + controls = []\n> > > > > + for input in args.input:\n> > > > > + data = open(input, 'rb').read()\n> > > > > + vendor = yaml.safe_load(data)['vendor']\n> > > > > + ctrls = yaml.safe_load(data)['controls']\n> > > > > + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls]\n> > > > >\n> > > > > if args.template.endswith('.cpp.in'):\n> > > > > data = generate_cpp(controls)","headers":{"Return-Path":"","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 23A5EC3220\n\tfor ;\n\tThu, 23 Nov 2023 12:13:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9919E629BC;\n\tThu, 23 Nov 2023 13:13:42 +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 DC04E61DAC\n\tfor ;\n\tThu, 23 Nov 2023 13:13:41 +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 D9F66A06;\n\tThu, 23 Nov 2023 13:13:09 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700741622;\n\tbh=xy56coQZFZU555M+pYbpjPfIjFtKBLdAd0Adj/G+Q9g=;\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=qSiN+KgzXJb9OLwSG+whNzhd0jyLX8uCVrgq4mZc8j3Lw3EX8NaZzAm33Abzy2vwk\n\tNF67k5rquh653h3w9M3sxGadevKsWwxu8vGYHzX7W4XSgF/guoC3QsBH9YsO2OSuov\n\tfw3e1nKgNqmqgJ91s1PMDTNM2TW40DQOKCTBsFnuujG48WD96w1NVlmfhg4NiAc2So\n\tDTfgr53nUu/QYjqRsh9frBtKAYtb496AP9X44Tz3IvRnHKqALFB6fxaVyJLqwjX80j\n\tOL0L98ncjw6Vv37mjdnhVjS6ihWcwfatB+LfFGV0wUWRNTzahwo4axVIyQMaaWpEme\n\tHrjxXPlAo9G7Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1700741590;\n\tbh=xy56coQZFZU555M+pYbpjPfIjFtKBLdAd0Adj/G+Q9g=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=P76B1u4zwgE4ApqYS+JuPSYilglLibu+0nTAh4EKVgjmEjfqITw+F6YACndJDlLaz\n\tiuA3JadYHoOmFpyksmNXoJf2tyBBA/uTwJSO/fvjxKXz+66fgpEwnw1fPpX/UgqTwa\n\tuxKl5ZzqaE0KlO4lh+mtC26GYO/JnlO+YhyCUvcY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"P76B1u4z\"; dkim-atps=neutral","Date":"Thu, 23 Nov 2023 14:13:48 +0200","To":"Naushir Patuck ","Message-ID":"<20231123121348.GK15697@pendragon.ideasonboard.com>","References":"<20231121145350.5956-1-naush@raspberrypi.com>\n\t<20231121145350.5956-4-naush@raspberrypi.com>\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28158,"web_url":"https://patchwork.libcamera.org/comment/28158/","msgid":"","date":"2023-11-23T12:57:12","subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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 Thu, 23 Nov 2023 at 12:13, Laurent Pinchart\n wrote:\n>\n> Hello,\n>\n> On Thu, Nov 23, 2023 at 08:02:28AM +0000, Naushir Patuck via libcamera-devel wrote:\n> > On Wed, 22 Nov 2023 at 16:05, Jacopo Mondi wrote:\n> > > On Wed, Nov 22, 2023 at 03:53:03PM +0000, Naushir Patuck wrote:\n> > > > On Wed, 22 Nov 2023 at 15:24, Jacopo Mondi wrote:\n> > > > > On Tue, Nov 21, 2023 at 02:53:47PM +0000, Naushir Patuck via libcamera-devel wrote:\n> > > > > > Add support for using separate YAML files for vendor specific controls.\n>\n> s/vendor specific/vendor-specific/\n>\n> Same below where applicable.\n>\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> > > > > > 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 \n> > > > > > ---\n> > > > > > include/libcamera/meson.build | 42 ++++++++++++++++++++++++++++-\n> > > > > > meson.build | 2 ++\n> > > > > > src/libcamera/control_ids_rpi.yaml | 16 +++++++++++\n> > > > > > src/libcamera/meson.build | 12 +++++++++\n> > > > > > src/py/libcamera/gen-py-controls.py | 10 +++----\n> > > > > > src/py/libcamera/meson.build | 8 ++++++\n> > > > > > utils/gen-controls.py | 12 +++++----\n> > > > > > 7 files changed, 91 insertions(+), 11 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 5fb772e6dd14..54123c5b858d 100644\n> > > > > > --- a/include/libcamera/meson.build\n> > > > > > +++ b/include/libcamera/meson.build\n> > > > > > @@ -38,10 +38,50 @@ control_source_files = {\n> > > > > > 'property_ids': 'properties',\n> > > > > > }\n> > > > >\n> > > > > I really like the series, but this patch is the piece I like less.\n> > > > >\n> > > > > >\n> > > > > > +vendor_controls_mapping = {\n> > > > > > + # Mapping of vendor (pipeline handler) specific controls files\n> > > > > > + 'controls':\n> > > > > > + {\n> > > > >\n> > > > > Isn't this usually written as\n> > > > >\n> > > > > 'controls' : {\n> > > > >\n> > > > > }\n> > > > >\n> > > > > in meson ?\n>\n> We seem to do that, yes.\n\nI'll change that to the above convention.\n\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')\n> > > > > > +\n> > > > > > + # Start by populating the vendor specific mappings into vendor_ctrl_files\n> > > > > > + # and vendor_prop_files. These will be cached for later use.\n> > > > > > + vendor_files = []\n> > > > > > + foreach pipeline, file : vendor_controls_mapping[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> > > > > > + foreach file : vendor_files\n> > > > > > + input_files += files('../../src/libcamera/' + file)\n> > > > > > + endforeach\n> > > > > > +\n> > > > >\n> > > > > Wouldn't it be simpler if we try to handle libcamera, draft and vendor\n> > > > > controls as first class citizens in the same manner, instead of\n> > > > > maintaing the \"libcamera\" ones on a different level ?\n> > > > >\n> > > > > I guess if we could do something like:\n> > > > >\n> > > > > -------------------------------------------------------------------------------\n> > > > >\n> > > > > controls_map {\n> > > > > 'controls' : {\n> > > > > 'libcamera' : controls_ids.yaml,\n> > > > > 'drafts' : controls_ids_draft.yaml\n> > > > > 'rpi/pisp': 'control_ids_rpi.yaml',\n> > > > > 'rpi/vc4': 'control_ids_rpi.yaml'\n> > > > > }\n> > > > >\n> > > > > 'properties' = {\n> > > > > 'libcamera' : controls_ids.yaml,\n>\n> This should be property_ids.yaml.\n>\n> > > > > 'drafts' : controls_ids_draft.yaml\n> > > > > }\n> > > > > }\n> > > > >\n> > > > > control_headers = []\n> > > > > ranges_file = files('../../src/libcamera/control_ranges.yaml')\n> > > > > foreach mode, entry : controls_map\n> > > > > files_list = []\n> > > > > input_files=[]\n> > > > > foreach vendor, header : entry\n> > > > > if vendor != 'libcamera' or vendor != 'draft'\n> > > > > if vendor not in pipelines\n> > > > > continue\n> > > > > endif\n> > > > > endif\n> > > > >\n> > > > > if header in files_list\n> > > > > continue\n> > > > > endif\n> > > > > files_list += header\n> > > > >\n> > > > > input_files += files('../../src/libcamera/' + header)\n> > > > > endforeach\n> > > > >\n> > > > > outfile=''\n> > > > > if mode == 'controls'\n> > > > > outfile = 'control_ids.h'\n> > > > > else\n> > > > > outfile = 'property_ids.h'\n> > > > > endif\n> > > > >\n> > > > > template = outfile + '.in'\n> > > > > control_headers += custom_target(header + '_h',\n> > > > > input : input_files,\n> > > > > output : outfile,\n> > > > > command : [gen_controls, '-o', '@OUTPUT@',\n> > > > > '--mode', mode, '-t', template,\n> > > > > '-r', ranges_file, '@INPUT@'],\n> > > > > install : true,\n> > > > > install_dir : libcamera_headers_install_dir)\n> > > > > endforeach\n> > > > >\n> > > > > libcamera_public_headers += control_headers\n> > > > >\n> > > > > -------------------------------------------------------------------------------\n> > > > >\n> > > > > isn't it nicer ?\n> > > > >\n> > > > > The above code is valid in meson, but the meson.build in\n> > > > > src/libcamera/ should be updated accordingly.\n> > > > >\n> > > > > In general, I think the relevant part here is\n> > > > >\n> > > > > controls_map {\n> > > > > 'controls' : {\n> > > > > 'libcamera' : controls_ids.yaml,\n> > > > > 'drafts' : controls_ids_draft.yaml\n> > > > > 'rpi/pisp': 'control_ids_rpi.yaml',\n> > > > > 'rpi/vc4': 'control_ids_rpi.yaml'\n> > > > > }\n> > > > >\n> > > > > 'properties' = {\n> > > > > 'libcamera' : controls_ids.yaml,\n> > > > > 'drafts' : controls_ids_draft.yaml\n> > > > > }\n> > > > > }\n> > > > >\n> > > > > As seeing all the supported controls and properties listed together\n> > > > > might be easier to maintain and parse ?\n> > > > >\n> > > > > What do you think ?\n> > > >\n> > > > That looks reasonable. I guess I wanted to keep things seperate for\n> > > > vendor controls, but no reason not to combine the libcamera/draft and\n> > > > vendor cases into one list. I can make this change for v3.\n>\n> That seems fine to me too. If we go in that direction, should we replace\n> 'libcamera' with 'core', and name the file controls_ids_core.yaml ? All\n> controls are libcamera controls.\n\nYes, that sounds like a good change to introduce.\n\n>\n> > > Let me know if and how I can help. The change is quite invasive in the\n> > > whole patch series and I don't want to make it more painful than\n> > > necessary for you\n> >\n> > No worries, I've already made the changes. Should I wait for more\n> > feedback or go ahead and push a v3?\n> >\n> > > > > > template_file = files(header + '.h.in')\n> > > > > > control_headers += custom_target(header + '_h',\n> > > > > > input : input_files,\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..a6f8b43a5f62\n> > > > > > --- /dev/null\n> > > > > > +++ b/src/libcamera/control_ids_rpi.yaml\n> > > > > > @@ -0,0 +1,16 @@\n> > > > > > +# SPDX-License-Identifier: LGPL-2.1-or-later\n> > > > > > +#\n> > > > > > +# Copyright (C) 2023, Raspberry Pi Ltd\n> > > > > > +#\n> > > > > > +%YAML 1.1\n> > > > > > +---\n> > > > > > +vendor: rpi\n> > > > > > +controls:\n> > > > > > + - PispConfigDumpFile:\n> > > > > > + type: string\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> As the series doesn't contain an implementation of this control I can't\n> really review the description properly, but that's fine, I understand it\n> will come later.\n\nIn v3, I'll remove the addition of the rpi specific vendor control and\nmove into a separate commit. We can make a case for delaying merging\nof that commit until the Pi 5 pipeline handler gets merged.\n\nRegards,\nNaush\n\n\n>\n> > > > > > +\n> > > > > > +...\n> > > > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > > > > > index 05ee38daf22b..7eb26ccbb7f5 100644\n> > > > > > --- a/src/libcamera/meson.build\n> > > > > > +++ b/src/libcamera/meson.build\n> > > > > > @@ -129,6 +129,18 @@ control_sources = []\n> > > > > >\n> > > > > > foreach source, mode : control_source_files\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> > > > > > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> > > > > > index ea28f0139f23..e2ddad8581fd 100755\n> > > > > > --- a/src/py/libcamera/gen-py-controls.py\n> > > > > > +++ b/src/py/libcamera/gen-py-controls.py\n> > > > > > @@ -91,7 +91,7 @@ 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> > > > > > + parser.add_argument('input', type=str, nargs='+',\n> > > > > > help='Input file name.')\n> > > > > > parser.add_argument('-t', dest='template', type=str, required=True,\n> > > > > > help='Template file name.')\n> > > > > > @@ -103,11 +103,11 @@ 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> > > > > > -\n> > > > > > controls = {}\n> > > > > > - vendor = yaml.safe_load(data)['vendor']\n> > > > > > - controls[vendor] = yaml.safe_load(data)['controls']\n> > > > > > + for input in args.input:\n> > > > > > + data = open(input, 'rb').read()\n> > > > > > + vendor = yaml.safe_load(data)['vendor']\n> > > > > > + controls[vendor] = 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 1c3ea1843ac0..ea38ad6ca829 100644\n> > > > > > --- a/src/py/libcamera/meson.build\n> > > > > > +++ b/src/py/libcamera/meson.build\n> > > > > > @@ -33,6 +33,10 @@ gen_py_controls_template = files('py_controls_generated.cpp.in')\n> > > > > >\n> > > > > > gen_py_controls = files('gen-py-controls.py')\n> > > > > >\n> > > > > > +foreach file : vendor_ctrl_files\n> > > > > > + gen_py_controls_input_files += files('../../libcamera/' + file)\n> > > > > > +endforeach\n> > > > > > +\n> > > > > > pycamera_sources += custom_target('py_gen_controls',\n> > > > > > input : gen_py_controls_input_files,\n> > > > > > output : ['py_controls_generated.cpp'],\n> > > > > > @@ -44,6 +48,10 @@ pycamera_sources += custom_target('py_gen_controls',\n> > > > > > gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> > > > > > gen_py_properties_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> > > > > > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > > > > > index 8af33d29cd07..d7862142b8c1 100755\n> > > > > > --- a/utils/gen-controls.py\n> > > > > > +++ b/utils/gen-controls.py\n> > > > > > @@ -359,7 +359,7 @@ 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> > > > > > + parser.add_argument('input', type=str, nargs='+',\n> > > > > > help='Input file name.')\n> > > > > > parser.add_argument('-t', dest='template', type=str, required=True,\n> > > > > > help='Template file name.')\n> > > > > > @@ -367,10 +367,12 @@ def main(argv):\n> > > > > > help='Mode of operation')\n> > > > > > args = parser.parse_args(argv[1:])\n> > > > > >\n> > > > > > - data = open(args.input, 'rb').read()\n> > > > > > - vendor = yaml.safe_load(data)['vendor']\n> > > > > > - controls = yaml.safe_load(data)['controls']\n> > > > > > - controls = [Control(*ctrl.popitem(), vendor) for ctrl in controls]\n> > > > > > + controls = []\n> > > > > > + for input in args.input:\n> > > > > > + data = open(input, 'rb').read()\n> > > > > > + vendor = yaml.safe_load(data)['vendor']\n> > > > > > + ctrls = yaml.safe_load(data)['controls']\n> > > > > > + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls]\n> > > > > >\n> > > > > > if args.template.endswith('.cpp.in'):\n> > > > > > data = generate_cpp(controls)\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","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 4274AC3220\n\tfor ;\n\tThu, 23 Nov 2023 12:57:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 86274629BD;\n\tThu, 23 Nov 2023 13:57:41 +0100 (CET)","from mail-yw1-x1133.google.com (mail-yw1-x1133.google.com\n\t[IPv6:2607:f8b0:4864:20::1133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 52109629AF\n\tfor ;\n\tThu, 23 Nov 2023 13:57:39 +0100 (CET)","by mail-yw1-x1133.google.com with SMTP id\n\t00721157ae682-5ca9114e0e2so8614257b3.3\n\tfor ;\n\tThu, 23 Nov 2023 04:57:39 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700744261;\n\tbh=lHM+lxOImAfImdHjykqhqQAa0kxXWEoaM9R2Qhh2I6U=;\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=CYOH/pyNVOpIcNPdxL9s4drMgz0dqEdkbrfZD1Xt0Hk0oG7XwHfaxr5GjspD9cqiM\n\t0BQvBDARttdcG4KwfntfJpLOyR5KWIcEfL0jUSf7BylOMhm/IkhfhZ2Sh9miW7ys7A\n\t5RjuYZpgumo8WfoJwiAQMZiaAwtX1l3Bixf+lko56HeXETLmas5PW6N88/mQUFonB7\n\tMLp+A4qC2+a2lQtq1qY62gjZQ6EBuYsLwquqtFhy1gLqkbQuysFBw1GKZbwby67pMK\n\te/TXKTs87d2O6fRmMuZpOKQV15n3/LjT8C+FA/csnwLUZIhLO2QE0zweXD/O56fwjF\n\tqSoEkfde5glkw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1700744258; x=1701349058;\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=0cIY/SKvDQIcOTJZV1ezobXS1/N4WGVyR30Nv0SxPv0=;\n\tb=d+JkHoHC8cW9BrI75J/7vUIGTePu46ZivAifevWT0kspyP5GC0NDotMLRinmYssCE6\n\tRGT0ditTkdRvog/VuKE7rBBbxWHenBWOMuCL3FZ5QGgBWa3nyi+XYwHUOe4wYdK+fzux\n\tSkKhI6zl+5pHQeFn5B+gdz5JGyIckCQKoohgqzyu+0Mu4gjVp/Gbfh65CSs/kd47BCyZ\n\tnGoNmfGO+doOWH5xx4cGEMmWLM+8LebBdAJBQjzeSVKC4OFcuLiNcQVGbVI5FD90jtKS\n\tOJ0czx/BsFejgQ+8eTR+tKNcTRILy3HljTelLvcqvniF7sa9DMmQZRBhJX1MpBi6pBjT\n\taBrA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"d+JkHoHC\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1700744258; x=1701349058;\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=0cIY/SKvDQIcOTJZV1ezobXS1/N4WGVyR30Nv0SxPv0=;\n\tb=LyWoLSJpEreKKUcp9kuEup2lysGh+FmM9iNq7afQP9NLixf1rNPGG1L16r+qpkRjFU\n\tb3L61XEtSAWzlrAlj/IVhFLc3+sHEYYhwxyFSiuXFR+kCzGMIey2521NpVQePoe9A3km\n\tWecRVGQlB4Hk/Yu13/GNdK2+HkQwYS7n4xv3zU1g97kfjKtbbv+xSHd+UsSm5PYvr1Se\n\t4soLpX0olH8gLw+kavr6hV5PfIoi4arA5mz4nPy0JAbSksYtNTney10BVi+yM3qowoTB\n\txbdr+/4fRfnx06guxyV9xtceuQYTK9utjmltsTIQGoIDaA1oIfSJ0jAY2GkSrDVtXw4t\n\tCaEw==","X-Gm-Message-State":"AOJu0YxxQRMQgnPvYMvTojmS94VRxe20plWUBQM9RfncH7cgkdxrcYqG\n\tNOGgyc7u2mzJ/M3qoW/ED53R/rkNzQBjO7LodoiXuPcMsHozZwpI4C0=","X-Google-Smtp-Source":"AGHT+IFfHPt42WxOk40vlq0hoGFm3EUp41BXkyCw3Ya4lyUMjjU2jqaEb6x/xJfTicYoBr4uup4bvUAX4U3YWtnwBWQ=","X-Received":"by 2002:a81:8904:0:b0:5ab:c519:6c1 with SMTP id\n\tz4-20020a818904000000b005abc51906c1mr5506524ywf.26.1700744257958;\n\tThu, 23 Nov 2023 04:57:37 -0800 (PST)","MIME-Version":"1.0","References":"<20231121145350.5956-1-naush@raspberrypi.com>\n\t<20231121145350.5956-4-naush@raspberrypi.com>\n\t\n\t\n\t\n\t\n\t<20231123121348.GK15697@pendragon.ideasonboard.com>","In-Reply-To":"<20231123121348.GK15697@pendragon.ideasonboard.com>","Date":"Thu, 23 Nov 2023 12:57:12 +0000","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28161,"web_url":"https://patchwork.libcamera.org/comment/28161/","msgid":"","date":"2023-11-23T13:59:39","subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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":"On Thu, 23 Nov 2023 at 12:57, Naushir Patuck wrote:\n>\n> Hi Laurent,\n>\n> Thank you for your feedback!\n>\n> On Thu, 23 Nov 2023 at 12:13, Laurent Pinchart\n> wrote:\n> >\n> > Hello,\n> >\n> > On Thu, Nov 23, 2023 at 08:02:28AM +0000, Naushir Patuck via libcamera-devel wrote:\n> > > On Wed, 22 Nov 2023 at 16:05, Jacopo Mondi wrote:\n> > > > On Wed, Nov 22, 2023 at 03:53:03PM +0000, Naushir Patuck wrote:\n> > > > > On Wed, 22 Nov 2023 at 15:24, Jacopo Mondi wrote:\n> > > > > > On Tue, Nov 21, 2023 at 02:53:47PM +0000, Naushir Patuck via libcamera-devel wrote:\n> > > > > > > Add support for using separate YAML files for vendor specific controls.\n> >\n> > s/vendor specific/vendor-specific/\n> >\n> > Same below where applicable.\n> >\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> > > > > > > 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 \n> > > > > > > ---\n> > > > > > > include/libcamera/meson.build | 42 ++++++++++++++++++++++++++++-\n> > > > > > > meson.build | 2 ++\n> > > > > > > src/libcamera/control_ids_rpi.yaml | 16 +++++++++++\n> > > > > > > src/libcamera/meson.build | 12 +++++++++\n> > > > > > > src/py/libcamera/gen-py-controls.py | 10 +++----\n> > > > > > > src/py/libcamera/meson.build | 8 ++++++\n> > > > > > > utils/gen-controls.py | 12 +++++----\n> > > > > > > 7 files changed, 91 insertions(+), 11 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 5fb772e6dd14..54123c5b858d 100644\n> > > > > > > --- a/include/libcamera/meson.build\n> > > > > > > +++ b/include/libcamera/meson.build\n> > > > > > > @@ -38,10 +38,50 @@ control_source_files = {\n> > > > > > > 'property_ids': 'properties',\n> > > > > > > }\n> > > > > >\n> > > > > > I really like the series, but this patch is the piece I like less.\n> > > > > >\n> > > > > > >\n> > > > > > > +vendor_controls_mapping = {\n> > > > > > > + # Mapping of vendor (pipeline handler) specific controls files\n> > > > > > > + 'controls':\n> > > > > > > + {\n> > > > > >\n> > > > > > Isn't this usually written as\n> > > > > >\n> > > > > > 'controls' : {\n> > > > > >\n> > > > > > }\n> > > > > >\n> > > > > > in meson ?\n> >\n> > We seem to do that, yes.\n>\n> I'll change that to the above convention.\n>\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')\n> > > > > > > +\n> > > > > > > + # Start by populating the vendor specific mappings into vendor_ctrl_files\n> > > > > > > + # and vendor_prop_files. These will be cached for later use.\n> > > > > > > + vendor_files = []\n> > > > > > > + foreach pipeline, file : vendor_controls_mapping[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> > > > > > > + foreach file : vendor_files\n> > > > > > > + input_files += files('../../src/libcamera/' + file)\n> > > > > > > + endforeach\n> > > > > > > +\n> > > > > >\n> > > > > > Wouldn't it be simpler if we try to handle libcamera, draft and vendor\n> > > > > > controls as first class citizens in the same manner, instead of\n> > > > > > maintaing the \"libcamera\" ones on a different level ?\n> > > > > >\n> > > > > > I guess if we could do something like:\n> > > > > >\n> > > > > > -------------------------------------------------------------------------------\n> > > > > >\n> > > > > > controls_map {\n> > > > > > 'controls' : {\n> > > > > > 'libcamera' : controls_ids.yaml,\n> > > > > > 'drafts' : controls_ids_draft.yaml\n> > > > > > 'rpi/pisp': 'control_ids_rpi.yaml',\n> > > > > > 'rpi/vc4': 'control_ids_rpi.yaml'\n> > > > > > }\n> > > > > >\n> > > > > > 'properties' = {\n> > > > > > 'libcamera' : controls_ids.yaml,\n> >\n> > This should be property_ids.yaml.\n> >\n> > > > > > 'drafts' : controls_ids_draft.yaml\n> > > > > > }\n> > > > > > }\n> > > > > >\n> > > > > > control_headers = []\n> > > > > > ranges_file = files('../../src/libcamera/control_ranges.yaml')\n> > > > > > foreach mode, entry : controls_map\n> > > > > > files_list = []\n> > > > > > input_files=[]\n> > > > > > foreach vendor, header : entry\n> > > > > > if vendor != 'libcamera' or vendor != 'draft'\n> > > > > > if vendor not in pipelines\n> > > > > > continue\n> > > > > > endif\n> > > > > > endif\n> > > > > >\n> > > > > > if header in files_list\n> > > > > > continue\n> > > > > > endif\n> > > > > > files_list += header\n> > > > > >\n> > > > > > input_files += files('../../src/libcamera/' + header)\n> > > > > > endforeach\n> > > > > >\n> > > > > > outfile=''\n> > > > > > if mode == 'controls'\n> > > > > > outfile = 'control_ids.h'\n> > > > > > else\n> > > > > > outfile = 'property_ids.h'\n> > > > > > endif\n> > > > > >\n> > > > > > template = outfile + '.in'\n> > > > > > control_headers += custom_target(header + '_h',\n> > > > > > input : input_files,\n> > > > > > output : outfile,\n> > > > > > command : [gen_controls, '-o', '@OUTPUT@',\n> > > > > > '--mode', mode, '-t', template,\n> > > > > > '-r', ranges_file, '@INPUT@'],\n> > > > > > install : true,\n> > > > > > install_dir : libcamera_headers_install_dir)\n> > > > > > endforeach\n> > > > > >\n> > > > > > libcamera_public_headers += control_headers\n> > > > > >\n> > > > > > -------------------------------------------------------------------------------\n> > > > > >\n> > > > > > isn't it nicer ?\n> > > > > >\n> > > > > > The above code is valid in meson, but the meson.build in\n> > > > > > src/libcamera/ should be updated accordingly.\n> > > > > >\n> > > > > > In general, I think the relevant part here is\n> > > > > >\n> > > > > > controls_map {\n> > > > > > 'controls' : {\n> > > > > > 'libcamera' : controls_ids.yaml,\n> > > > > > 'drafts' : controls_ids_draft.yaml\n> > > > > > 'rpi/pisp': 'control_ids_rpi.yaml',\n> > > > > > 'rpi/vc4': 'control_ids_rpi.yaml'\n> > > > > > }\n> > > > > >\n> > > > > > 'properties' = {\n> > > > > > 'libcamera' : controls_ids.yaml,\n> > > > > > 'drafts' : controls_ids_draft.yaml\n> > > > > > }\n> > > > > > }\n> > > > > >\n> > > > > > As seeing all the supported controls and properties listed together\n> > > > > > might be easier to maintain and parse ?\n> > > > > >\n> > > > > > What do you think ?\n> > > > >\n> > > > > That looks reasonable. I guess I wanted to keep things seperate for\n> > > > > vendor controls, but no reason not to combine the libcamera/draft and\n> > > > > vendor cases into one list. I can make this change for v3.\n> >\n> > That seems fine to me too. If we go in that direction, should we replace\n> > 'libcamera' with 'core', and name the file controls_ids_core.yaml ? All\n> > controls are libcamera controls.\n>\n> Yes, that sounds like a good change to introduce.\n\nOn this change, if I rename 'libcamera' to 'core' above, I should\nreally do the same in the gen-controls.py scripts as well for\nconsistency (even though the 2 labels are unrelated). I'lll do that\nchange unless folks disagree with this.\n\n>\n> >\n> > > > Let me know if and how I can help. The change is quite invasive in the\n> > > > whole patch series and I don't want to make it more painful than\n> > > > necessary for you\n> > >\n> > > No worries, I've already made the changes. Should I wait for more\n> > > feedback or go ahead and push a v3?\n> > >\n> > > > > > > template_file = files(header + '.h.in')\n> > > > > > > control_headers += custom_target(header + '_h',\n> > > > > > > input : input_files,\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..a6f8b43a5f62\n> > > > > > > --- /dev/null\n> > > > > > > +++ b/src/libcamera/control_ids_rpi.yaml\n> > > > > > > @@ -0,0 +1,16 @@\n> > > > > > > +# SPDX-License-Identifier: LGPL-2.1-or-later\n> > > > > > > +#\n> > > > > > > +# Copyright (C) 2023, Raspberry Pi Ltd\n> > > > > > > +#\n> > > > > > > +%YAML 1.1\n> > > > > > > +---\n> > > > > > > +vendor: rpi\n> > > > > > > +controls:\n> > > > > > > + - PispConfigDumpFile:\n> > > > > > > + type: string\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> > As the series doesn't contain an implementation of this control I can't\n> > really review the description properly, but that's fine, I understand it\n> > will come later.\n>\n> In v3, I'll remove the addition of the rpi specific vendor control and\n> move into a separate commit. We can make a case for delaying merging\n> of that commit until the Pi 5 pipeline handler gets merged.\n>\n> Regards,\n> Naush\n>\n>\n> >\n> > > > > > > +\n> > > > > > > +...\n> > > > > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > > > > > > index 05ee38daf22b..7eb26ccbb7f5 100644\n> > > > > > > --- a/src/libcamera/meson.build\n> > > > > > > +++ b/src/libcamera/meson.build\n> > > > > > > @@ -129,6 +129,18 @@ control_sources = []\n> > > > > > >\n> > > > > > > foreach source, mode : control_source_files\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> > > > > > > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> > > > > > > index ea28f0139f23..e2ddad8581fd 100755\n> > > > > > > --- a/src/py/libcamera/gen-py-controls.py\n> > > > > > > +++ b/src/py/libcamera/gen-py-controls.py\n> > > > > > > @@ -91,7 +91,7 @@ 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> > > > > > > + parser.add_argument('input', type=str, nargs='+',\n> > > > > > > help='Input file name.')\n> > > > > > > parser.add_argument('-t', dest='template', type=str, required=True,\n> > > > > > > help='Template file name.')\n> > > > > > > @@ -103,11 +103,11 @@ 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> > > > > > > -\n> > > > > > > controls = {}\n> > > > > > > - vendor = yaml.safe_load(data)['vendor']\n> > > > > > > - controls[vendor] = yaml.safe_load(data)['controls']\n> > > > > > > + for input in args.input:\n> > > > > > > + data = open(input, 'rb').read()\n> > > > > > > + vendor = yaml.safe_load(data)['vendor']\n> > > > > > > + controls[vendor] = 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 1c3ea1843ac0..ea38ad6ca829 100644\n> > > > > > > --- a/src/py/libcamera/meson.build\n> > > > > > > +++ b/src/py/libcamera/meson.build\n> > > > > > > @@ -33,6 +33,10 @@ gen_py_controls_template = files('py_controls_generated.cpp.in')\n> > > > > > >\n> > > > > > > gen_py_controls = files('gen-py-controls.py')\n> > > > > > >\n> > > > > > > +foreach file : vendor_ctrl_files\n> > > > > > > + gen_py_controls_input_files += files('../../libcamera/' + file)\n> > > > > > > +endforeach\n> > > > > > > +\n> > > > > > > pycamera_sources += custom_target('py_gen_controls',\n> > > > > > > input : gen_py_controls_input_files,\n> > > > > > > output : ['py_controls_generated.cpp'],\n> > > > > > > @@ -44,6 +48,10 @@ pycamera_sources += custom_target('py_gen_controls',\n> > > > > > > gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> > > > > > > gen_py_properties_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> > > > > > > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > > > > > > index 8af33d29cd07..d7862142b8c1 100755\n> > > > > > > --- a/utils/gen-controls.py\n> > > > > > > +++ b/utils/gen-controls.py\n> > > > > > > @@ -359,7 +359,7 @@ 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> > > > > > > + parser.add_argument('input', type=str, nargs='+',\n> > > > > > > help='Input file name.')\n> > > > > > > parser.add_argument('-t', dest='template', type=str, required=True,\n> > > > > > > help='Template file name.')\n> > > > > > > @@ -367,10 +367,12 @@ def main(argv):\n> > > > > > > help='Mode of operation')\n> > > > > > > args = parser.parse_args(argv[1:])\n> > > > > > >\n> > > > > > > - data = open(args.input, 'rb').read()\n> > > > > > > - vendor = yaml.safe_load(data)['vendor']\n> > > > > > > - controls = yaml.safe_load(data)['controls']\n> > > > > > > - controls = [Control(*ctrl.popitem(), vendor) for ctrl in controls]\n> > > > > > > + controls = []\n> > > > > > > + for input in args.input:\n> > > > > > > + data = open(input, 'rb').read()\n> > > > > > > + vendor = yaml.safe_load(data)['vendor']\n> > > > > > > + ctrls = yaml.safe_load(data)['controls']\n> > > > > > > + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls]\n> > > > > > >\n> > > > > > > if args.template.endswith('.cpp.in'):\n> > > > > > > data = generate_cpp(controls)\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart","headers":{"Return-Path":"","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 BF7CDBDE6B\n\tfor ;\n\tThu, 23 Nov 2023 14:00:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E515C61DAC;\n\tThu, 23 Nov 2023 15:00:09 +0100 (CET)","from mail-yw1-x1131.google.com (mail-yw1-x1131.google.com\n\t[IPv6:2607:f8b0:4864:20::1131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5D0B061DAC\n\tfor ;\n\tThu, 23 Nov 2023 15:00:07 +0100 (CET)","by mail-yw1-x1131.google.com with SMTP id\n\t00721157ae682-5cd3c4457a0so6511817b3.3\n\tfor ;\n\tThu, 23 Nov 2023 06:00:07 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700748009;\n\tbh=7wYsjiWpRs+gisDMCG3jCuvox97enjjYAk1PWxH9Egg=;\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=ieJI/WMwvznVgkDI+dYlDcicOsm4jN5OvRUvrNtewlOpqkbddGUShOnurFPjCAjE4\n\tdU/GA+8e8AxsNn+2GvhmcxqSnHkDnz/H1iyuuylvXQDdHNHPgL/RiwpSS2jNgZJjqQ\n\tZWh2obe6EEvceidUHFfvnM5lkLYlJJe+rOBJYJCamGCXX52FYqGbvOM5QXyXzes+Gb\n\twW1yrxG7nCJnJFcn8a/S+g4uVjuKIibr61+Tqq2n+J34MdqMs/00JatZloLQM3ayTY\n\ttcGUsH9fISQZx6ocxFLMIr0reM8wJo9TP9EYcTjOppksCFNRl8PpRyCLhKJhUDqFzV\n\tNGTAGBWVmWqQw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1700748006; x=1701352806;\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=xU6TNBE9cGGtc1uGbWKsEQM8zMzE6VBGIMWn28JxEUI=;\n\tb=B9rLbuADIfy94WVpRBzYC9ah3n+STZva8qxizDzHPI1qmI+jl/mWeI8edNT/2LgH1R\n\tjF2XnRUiMFu4iR9FVZVkgQRph3ZujMWK/I16RfjY2gU/PtE5lmLfHW4VqIhQMq5UcP6S\n\tIOAafEn965OuGuYp/KS7rsIH7+EWeh41BxwZagR5yzRty1luQ6cZWYXkw8wv4cMlDduU\n\t5rCqZOtIwSEkD2EniRAlEN3x6qZRtjA8VE14RmhPHcGNmAmYlvuiqfWpNyIcjjTET7VO\n\turGIkKGJHHPy8R4RDcR1EFVKLtFQyP++9nNWaqjRUoqv6VVbH0S6+Ph9EJm0KUfDqmko\n\tw32A=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"B9rLbuAD\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1700748006; x=1701352806;\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=xU6TNBE9cGGtc1uGbWKsEQM8zMzE6VBGIMWn28JxEUI=;\n\tb=Z11+OAsX6yKO7Na5VFNbeVyq5HF0pzhQbQIFd18KUK9cZ+CBOnQZa23RpA3B0Np5Q7\n\tXS31h2Sp22m7/SBdcv0vzITrnezm0w3Tehkv9FBGr7gB0pc7XiIUWCT4lxgAcmqRa+E+\n\tUukjieMxWEXAlW3w3z3g0wk1K5T1asB/kvNz9Oe9O7BACYr1rWKR9AOFdKeRhGKaPDes\n\tEuXwO+k1og1o6U50s1ncPp1RJ43igEIhwT3i+8nm9MX2fBkSgmhOT09ble59jQP+SMuR\n\tNe8/UgCZAFSnSm5v9Azk8Gm/2CV9FDv1tH8a5+SndMPprLl1XH4K91kg4bb4tUSo5XMU\n\tKbBg==","X-Gm-Message-State":"AOJu0YzXk8y2eb/4JlM4c4E1JvY7vo5aMXOtAVa/5jq2P+2YhT7oiOae\n\t5/aK9gg5vy7ni2UhTxrkpszUSifeaqQcXao8A1eNJ+eCLCYJjr6rUJU=","X-Google-Smtp-Source":"AGHT+IFvJbGCpNBayGnOtLBhOqimbJdFHrRXzNv9/Yoz67QBRu5O1LqQOM9+/JxRgIQiZxSjOoNZpRCMvtg5dPjLn4A=","X-Received":"by 2002:a0d:c7c1:0:b0:5ca:3251:570c with SMTP id\n\tj184-20020a0dc7c1000000b005ca3251570cmr5666864ywd.35.1700748005797;\n\tThu, 23 Nov 2023 06:00:05 -0800 (PST)","MIME-Version":"1.0","References":"<20231121145350.5956-1-naush@raspberrypi.com>\n\t<20231121145350.5956-4-naush@raspberrypi.com>\n\t\n\t\n\t\n\t\n\t<20231123121348.GK15697@pendragon.ideasonboard.com>\n\t","In-Reply-To":"","Date":"Thu, 23 Nov 2023 13:59:39 +0000","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28162,"web_url":"https://patchwork.libcamera.org/comment/28162/","msgid":"","date":"2023-11-23T14:02:43","subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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":"On Thu, 23 Nov 2023 at 13:59, Naushir Patuck wrote:\n>\n> On Thu, 23 Nov 2023 at 12:57, Naushir Patuck wrote:\n> >\n> > Hi Laurent,\n> >\n> > Thank you for your feedback!\n> >\n> > On Thu, 23 Nov 2023 at 12:13, Laurent Pinchart\n> > wrote:\n> > >\n> > > Hello,\n> > >\n> > > On Thu, Nov 23, 2023 at 08:02:28AM +0000, Naushir Patuck via libcamera-devel wrote:\n> > > > On Wed, 22 Nov 2023 at 16:05, Jacopo Mondi wrote:\n> > > > > On Wed, Nov 22, 2023 at 03:53:03PM +0000, Naushir Patuck wrote:\n> > > > > > On Wed, 22 Nov 2023 at 15:24, Jacopo Mondi wrote:\n> > > > > > > On Tue, Nov 21, 2023 at 02:53:47PM +0000, Naushir Patuck via libcamera-devel wrote:\n> > > > > > > > Add support for using separate YAML files for vendor specific controls.\n> > >\n> > > s/vendor specific/vendor-specific/\n> > >\n> > > Same below where applicable.\n> > >\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> > > > > > > > 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 \n> > > > > > > > ---\n> > > > > > > > include/libcamera/meson.build | 42 ++++++++++++++++++++++++++++-\n> > > > > > > > meson.build | 2 ++\n> > > > > > > > src/libcamera/control_ids_rpi.yaml | 16 +++++++++++\n> > > > > > > > src/libcamera/meson.build | 12 +++++++++\n> > > > > > > > src/py/libcamera/gen-py-controls.py | 10 +++----\n> > > > > > > > src/py/libcamera/meson.build | 8 ++++++\n> > > > > > > > utils/gen-controls.py | 12 +++++----\n> > > > > > > > 7 files changed, 91 insertions(+), 11 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 5fb772e6dd14..54123c5b858d 100644\n> > > > > > > > --- a/include/libcamera/meson.build\n> > > > > > > > +++ b/include/libcamera/meson.build\n> > > > > > > > @@ -38,10 +38,50 @@ control_source_files = {\n> > > > > > > > 'property_ids': 'properties',\n> > > > > > > > }\n> > > > > > >\n> > > > > > > I really like the series, but this patch is the piece I like less.\n> > > > > > >\n> > > > > > > >\n> > > > > > > > +vendor_controls_mapping = {\n> > > > > > > > + # Mapping of vendor (pipeline handler) specific controls files\n> > > > > > > > + 'controls':\n> > > > > > > > + {\n> > > > > > >\n> > > > > > > Isn't this usually written as\n> > > > > > >\n> > > > > > > 'controls' : {\n> > > > > > >\n> > > > > > > }\n> > > > > > >\n> > > > > > > in meson ?\n> > >\n> > > We seem to do that, yes.\n> >\n> > I'll change that to the above convention.\n> >\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')\n> > > > > > > > +\n> > > > > > > > + # Start by populating the vendor specific mappings into vendor_ctrl_files\n> > > > > > > > + # and vendor_prop_files. These will be cached for later use.\n> > > > > > > > + vendor_files = []\n> > > > > > > > + foreach pipeline, file : vendor_controls_mapping[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> > > > > > > > + foreach file : vendor_files\n> > > > > > > > + input_files += files('../../src/libcamera/' + file)\n> > > > > > > > + endforeach\n> > > > > > > > +\n> > > > > > >\n> > > > > > > Wouldn't it be simpler if we try to handle libcamera, draft and vendor\n> > > > > > > controls as first class citizens in the same manner, instead of\n> > > > > > > maintaing the \"libcamera\" ones on a different level ?\n> > > > > > >\n> > > > > > > I guess if we could do something like:\n> > > > > > >\n> > > > > > > -------------------------------------------------------------------------------\n> > > > > > >\n> > > > > > > controls_map {\n> > > > > > > 'controls' : {\n> > > > > > > 'libcamera' : controls_ids.yaml,\n> > > > > > > 'drafts' : controls_ids_draft.yaml\n> > > > > > > 'rpi/pisp': 'control_ids_rpi.yaml',\n> > > > > > > 'rpi/vc4': 'control_ids_rpi.yaml'\n> > > > > > > }\n> > > > > > >\n> > > > > > > 'properties' = {\n> > > > > > > 'libcamera' : controls_ids.yaml,\n> > >\n> > > This should be property_ids.yaml.\n> > >\n> > > > > > > 'drafts' : controls_ids_draft.yaml\n> > > > > > > }\n> > > > > > > }\n> > > > > > >\n> > > > > > > control_headers = []\n> > > > > > > ranges_file = files('../../src/libcamera/control_ranges.yaml')\n> > > > > > > foreach mode, entry : controls_map\n> > > > > > > files_list = []\n> > > > > > > input_files=[]\n> > > > > > > foreach vendor, header : entry\n> > > > > > > if vendor != 'libcamera' or vendor != 'draft'\n> > > > > > > if vendor not in pipelines\n> > > > > > > continue\n> > > > > > > endif\n> > > > > > > endif\n> > > > > > >\n> > > > > > > if header in files_list\n> > > > > > > continue\n> > > > > > > endif\n> > > > > > > files_list += header\n> > > > > > >\n> > > > > > > input_files += files('../../src/libcamera/' + header)\n> > > > > > > endforeach\n> > > > > > >\n> > > > > > > outfile=''\n> > > > > > > if mode == 'controls'\n> > > > > > > outfile = 'control_ids.h'\n> > > > > > > else\n> > > > > > > outfile = 'property_ids.h'\n> > > > > > > endif\n> > > > > > >\n> > > > > > > template = outfile + '.in'\n> > > > > > > control_headers += custom_target(header + '_h',\n> > > > > > > input : input_files,\n> > > > > > > output : outfile,\n> > > > > > > command : [gen_controls, '-o', '@OUTPUT@',\n> > > > > > > '--mode', mode, '-t', template,\n> > > > > > > '-r', ranges_file, '@INPUT@'],\n> > > > > > > install : true,\n> > > > > > > install_dir : libcamera_headers_install_dir)\n> > > > > > > endforeach\n> > > > > > >\n> > > > > > > libcamera_public_headers += control_headers\n> > > > > > >\n> > > > > > > -------------------------------------------------------------------------------\n> > > > > > >\n> > > > > > > isn't it nicer ?\n> > > > > > >\n> > > > > > > The above code is valid in meson, but the meson.build in\n> > > > > > > src/libcamera/ should be updated accordingly.\n> > > > > > >\n> > > > > > > In general, I think the relevant part here is\n> > > > > > >\n> > > > > > > controls_map {\n> > > > > > > 'controls' : {\n> > > > > > > 'libcamera' : controls_ids.yaml,\n> > > > > > > 'drafts' : controls_ids_draft.yaml\n> > > > > > > 'rpi/pisp': 'control_ids_rpi.yaml',\n> > > > > > > 'rpi/vc4': 'control_ids_rpi.yaml'\n> > > > > > > }\n> > > > > > >\n> > > > > > > 'properties' = {\n> > > > > > > 'libcamera' : controls_ids.yaml,\n> > > > > > > 'drafts' : controls_ids_draft.yaml\n> > > > > > > }\n> > > > > > > }\n> > > > > > >\n> > > > > > > As seeing all the supported controls and properties listed together\n> > > > > > > might be easier to maintain and parse ?\n> > > > > > >\n> > > > > > > What do you think ?\n> > > > > >\n> > > > > > That looks reasonable. I guess I wanted to keep things seperate for\n> > > > > > vendor controls, but no reason not to combine the libcamera/draft and\n> > > > > > vendor cases into one list. I can make this change for v3.\n> > >\n> > > That seems fine to me too. If we go in that direction, should we replace\n> > > 'libcamera' with 'core', and name the file controls_ids_core.yaml ? All\n> > > controls are libcamera controls.\n> >\n> > Yes, that sounds like a good change to introduce.\n>\n> On this change, if I rename 'libcamera' to 'core' above, I should\n> really do the same in the gen-controls.py scripts as well for\n> consistency (even though the 2 labels are unrelated). I'lll do that\n> change unless folks disagree with this.\n\nPlease disregard that. I think the 'libcamera' designation in the gen\nscripts is actually correct (identifying 'libcamera' as a vendor for\n'core' controls). Apologies for the noise!\n\n>\n> >\n> > >\n> > > > > Let me know if and how I can help. The change is quite invasive in the\n> > > > > whole patch series and I don't want to make it more painful than\n> > > > > necessary for you\n> > > >\n> > > > No worries, I've already made the changes. Should I wait for more\n> > > > feedback or go ahead and push a v3?\n> > > >\n> > > > > > > > template_file = files(header + '.h.in')\n> > > > > > > > control_headers += custom_target(header + '_h',\n> > > > > > > > input : input_files,\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..a6f8b43a5f62\n> > > > > > > > --- /dev/null\n> > > > > > > > +++ b/src/libcamera/control_ids_rpi.yaml\n> > > > > > > > @@ -0,0 +1,16 @@\n> > > > > > > > +# SPDX-License-Identifier: LGPL-2.1-or-later\n> > > > > > > > +#\n> > > > > > > > +# Copyright (C) 2023, Raspberry Pi Ltd\n> > > > > > > > +#\n> > > > > > > > +%YAML 1.1\n> > > > > > > > +---\n> > > > > > > > +vendor: rpi\n> > > > > > > > +controls:\n> > > > > > > > + - PispConfigDumpFile:\n> > > > > > > > + type: string\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> > > As the series doesn't contain an implementation of this control I can't\n> > > really review the description properly, but that's fine, I understand it\n> > > will come later.\n> >\n> > In v3, I'll remove the addition of the rpi specific vendor control and\n> > move into a separate commit. We can make a case for delaying merging\n> > of that commit until the Pi 5 pipeline handler gets merged.\n> >\n> > Regards,\n> > Naush\n> >\n> >\n> > >\n> > > > > > > > +\n> > > > > > > > +...\n> > > > > > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > > > > > > > index 05ee38daf22b..7eb26ccbb7f5 100644\n> > > > > > > > --- a/src/libcamera/meson.build\n> > > > > > > > +++ b/src/libcamera/meson.build\n> > > > > > > > @@ -129,6 +129,18 @@ control_sources = []\n> > > > > > > >\n> > > > > > > > foreach source, mode : control_source_files\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> > > > > > > > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> > > > > > > > index ea28f0139f23..e2ddad8581fd 100755\n> > > > > > > > --- a/src/py/libcamera/gen-py-controls.py\n> > > > > > > > +++ b/src/py/libcamera/gen-py-controls.py\n> > > > > > > > @@ -91,7 +91,7 @@ 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> > > > > > > > + parser.add_argument('input', type=str, nargs='+',\n> > > > > > > > help='Input file name.')\n> > > > > > > > parser.add_argument('-t', dest='template', type=str, required=True,\n> > > > > > > > help='Template file name.')\n> > > > > > > > @@ -103,11 +103,11 @@ 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> > > > > > > > -\n> > > > > > > > controls = {}\n> > > > > > > > - vendor = yaml.safe_load(data)['vendor']\n> > > > > > > > - controls[vendor] = yaml.safe_load(data)['controls']\n> > > > > > > > + for input in args.input:\n> > > > > > > > + data = open(input, 'rb').read()\n> > > > > > > > + vendor = yaml.safe_load(data)['vendor']\n> > > > > > > > + controls[vendor] = 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 1c3ea1843ac0..ea38ad6ca829 100644\n> > > > > > > > --- a/src/py/libcamera/meson.build\n> > > > > > > > +++ b/src/py/libcamera/meson.build\n> > > > > > > > @@ -33,6 +33,10 @@ gen_py_controls_template = files('py_controls_generated.cpp.in')\n> > > > > > > >\n> > > > > > > > gen_py_controls = files('gen-py-controls.py')\n> > > > > > > >\n> > > > > > > > +foreach file : vendor_ctrl_files\n> > > > > > > > + gen_py_controls_input_files += files('../../libcamera/' + file)\n> > > > > > > > +endforeach\n> > > > > > > > +\n> > > > > > > > pycamera_sources += custom_target('py_gen_controls',\n> > > > > > > > input : gen_py_controls_input_files,\n> > > > > > > > output : ['py_controls_generated.cpp'],\n> > > > > > > > @@ -44,6 +48,10 @@ pycamera_sources += custom_target('py_gen_controls',\n> > > > > > > > gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> > > > > > > > gen_py_properties_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> > > > > > > > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > > > > > > > index 8af33d29cd07..d7862142b8c1 100755\n> > > > > > > > --- a/utils/gen-controls.py\n> > > > > > > > +++ b/utils/gen-controls.py\n> > > > > > > > @@ -359,7 +359,7 @@ 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> > > > > > > > + parser.add_argument('input', type=str, nargs='+',\n> > > > > > > > help='Input file name.')\n> > > > > > > > parser.add_argument('-t', dest='template', type=str, required=True,\n> > > > > > > > help='Template file name.')\n> > > > > > > > @@ -367,10 +367,12 @@ def main(argv):\n> > > > > > > > help='Mode of operation')\n> > > > > > > > args = parser.parse_args(argv[1:])\n> > > > > > > >\n> > > > > > > > - data = open(args.input, 'rb').read()\n> > > > > > > > - vendor = yaml.safe_load(data)['vendor']\n> > > > > > > > - controls = yaml.safe_load(data)['controls']\n> > > > > > > > - controls = [Control(*ctrl.popitem(), vendor) for ctrl in controls]\n> > > > > > > > + controls = []\n> > > > > > > > + for input in args.input:\n> > > > > > > > + data = open(input, 'rb').read()\n> > > > > > > > + vendor = yaml.safe_load(data)['vendor']\n> > > > > > > > + ctrls = yaml.safe_load(data)['controls']\n> > > > > > > > + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls]\n> > > > > > > >\n> > > > > > > > if args.template.endswith('.cpp.in'):\n> > > > > > > > data = generate_cpp(controls)\n> > >\n> > > --\n> > > Regards,\n> > >\n> > > Laurent Pinchart","headers":{"Return-Path":"","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 8EF54C3220\n\tfor ;\n\tThu, 23 Nov 2023 14:03:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CDF42629BD;\n\tThu, 23 Nov 2023 15:03:11 +0100 (CET)","from mail-ot1-x32e.google.com (mail-ot1-x32e.google.com\n\t[IPv6:2607:f8b0:4864:20::32e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6C52C629AF\n\tfor ;\n\tThu, 23 Nov 2023 15:03:10 +0100 (CET)","by mail-ot1-x32e.google.com with SMTP id\n\t46e09a7af769-6ce2b6b3cb6so544301a34.3\n\tfor ;\n\tThu, 23 Nov 2023 06:03:10 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700748191;\n\tbh=TUdDo16b1skFLagF/Q6swVSFR92AYpxtoeExpoPXZ8s=;\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=fqeket+22MEmLejzd6O1HNb+JrirrZpSg4tqFYVGAP+G5+m0aLoXIKoq5HU3zJuNN\n\tMhiCzNvrcDgEu0o9vr35lWlV3CD3bZaamvUW4INs3yLwQbPdXMAgXLIK88oyYXfCyV\n\tg6ENXfVE8aEg4lpPmCdjGipwjEcAUYbK3nHlE27M2/YfDYFQjrZds4XfVOUdxTqMAI\n\tzE+mm9AlapGLyRiKgPbRtm6ZTxmce/NwYkYCiC+W6h7ZkzNkf7TmjwZm1lEpuYRHEn\n\tO2jOOkyR1Am+QRj8H4yGiRskksd73/7AaTir2Yh/htEil+MSoJCGpBRvINp+N1vdh3\n\tUS08Brt2BIxWw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1700748189; x=1701352989;\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=3hO2LNwTq8BvAWYjbEEMnJfTJAxUOPNy7NHXJjRAXYQ=;\n\tb=i8Zah1jL10c/AJQun2Nk85BzLzxhFzUwqhmfooTsiEUQUCj4nr0AY10d4OXW9ihdj2\n\t1mGn3qM5OhbbIboCA9zZ8Z0jLLldZuzNmHcf3gLFiCzCT+Pu77lyVInU0iKPC5a40buj\n\tVBPMBJVM/Zprs3tLMjs/pEA4BRmIZWlZe9Kdry+PU/jKmD05UfmpE5PT4SX+lWUCodbz\n\tyP6HpxLv6LH7y/oxevQf/idnoVvkja9rZx0rIEIlHh72PNOSFku+lRNEGB/AksvFlVmH\n\tSPxeK5IVmgVrvciOf2jP4GRNQTU3D65o0KJIrwAlFd7hTH2NbBn+k3Tg2JnE4aczlTOf\n\t40Eg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"i8Zah1jL\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1700748189; x=1701352989;\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=3hO2LNwTq8BvAWYjbEEMnJfTJAxUOPNy7NHXJjRAXYQ=;\n\tb=Anicdk9eFuXpLQekIDygerNysSov9oIsJk+D8XplgzaQsj6fY49kpYhFDVuDMBT6jz\n\tBoEIiQ3t3+AN8Wk2mvwgsh6KE84/LL1gl31gx0fk3wQ5T9GydFLYojkg51lK9iadRwn7\n\txatX5VTXIQYdmA9Jzv6g/beLUuSpG+kNCIFnCU+RqoeLhUeGRgkVE+gfM28ZwyE28No0\n\t5oj9mezT8ZRfw1/I5IuuNpmsMb6AF5TM13rhdIGrbB/NOTShEmO0e0/4LFry2pjPtRQl\n\tke36BJDSEvOCNNawNAOV+NSISbhybTj3Harqa9JgUfycQhlZY3xm1rqYldNlkyjEOLt9\n\tnvrw==","X-Gm-Message-State":"AOJu0Yw0b4eXTKmQIi5VwpJg4G2eb4Lj9y6NHyBY15AM2/D+aPpGRYQ7\n\tn7MR4oLp2aeLKZTVy0Mjxx1R0mGqQXzpibzyRLKFEB/KEb1r//kA0+Y=","X-Google-Smtp-Source":"AGHT+IHABWSlgi2j+Kj8J3xWDfDL1LsV5QW3dWREWQz1tEZA0uGTW9G3Gc/gMlbKcsbdT4AquFayCmVQX2UXcKfDjck=","X-Received":"by 2002:a9d:7c91:0:b0:6d3:19bf:2d16 with SMTP id\n\tq17-20020a9d7c91000000b006d319bf2d16mr6519555otn.12.1700748188929;\n\tThu, 23 Nov 2023 06:03:08 -0800 (PST)","MIME-Version":"1.0","References":"<20231121145350.5956-1-naush@raspberrypi.com>\n\t<20231121145350.5956-4-naush@raspberrypi.com>\n\t\n\t\n\t\n\t\n\t<20231123121348.GK15697@pendragon.ideasonboard.com>\n\t\n\t","In-Reply-To":"","Date":"Thu, 23 Nov 2023 14:02:43 +0000","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]