[{"id":28147,"web_url":"https://patchwork.libcamera.org/comment/28147/","msgid":"<20231123094132.GG15697@pendragon.ideasonboard.com>","date":"2023-11-23T09:41:32","subject":"Re: [libcamera-devel] [PATCH v2 4/6] libcamera: control: Add vendor\n\tcontrol id range reservation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Tue, Nov 21, 2023 at 02:53:48PM +0000, Naushir Patuck via libcamera-devel wrote:\n> Add a new control_ranges.yaml file that is used to reserve control id\n> ranges/offsets for libcamera and vendor specific controls. This file is\n> used by the gen-controls.py script to generate control id values for\n> each control.\n> \n> Draft controls now have a separate range from core libcamera controls,\n> breaking the existing numbering behaviour.\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> include/libcamera/meson.build | 4 +++-\n> src/libcamera/control_ranges.yaml | 18 ++++++++++++++++++\n> src/libcamera/meson.build | 4 +++-\n> utils/gen-controls.py | 15 ++++++++++++---\n> 4 files changed, 36 insertions(+), 5 deletions(-)\n> create mode 100644 src/libcamera/control_ranges.yaml\n> \n> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> index 54123c5b858d..6cac385fdee4 100644\n> --- a/include/libcamera/meson.build\n> +++ b/include/libcamera/meson.build\n> @@ -83,11 +83,13 @@ foreach header, mode : control_source_files\n> endforeach\n> \n> template_file = files(header + '.h.in')\n> + ranges_file = files('../../src/libcamera/control_ranges.yaml')\n> control_headers += custom_target(header + '_h',\n> input : input_files,\n> output : header + '.h',\n> command : [gen_controls, '-o', '@OUTPUT@',\n> - '--mode', mode, '-t', template_file, '@INPUT@'],\n> + '--mode', mode, '-t', template_file,\n> + '-r', ranges_file, '@INPUT@'],\n> install : true,\n> install_dir : libcamera_headers_install_dir)\n> endforeach\n> diff --git a/src/libcamera/control_ranges.yaml b/src/libcamera/control_ranges.yaml\n> new file mode 100644\n> index 000000000000..150ae7fa5efd\n> --- /dev/null\n> +++ b/src/libcamera/control_ranges.yaml\n> @@ -0,0 +1,18 @@\n> +# SPDX-License-Identifier: LGPL-2.1-or-later\n> +#\n> +# Copyright (C) 2023, Raspberry Pi Ltd\n> +#\n> +%YAML 1.1\n> +---\n> +# Specifies the control id ranges/offsets for core libcamera and vendor specific\n> +# controls.\n> +ranges:\n> + # Core libcamera controls\n> + libcamera: 0\n> + # Draft designated libcamera controls\n> + draft: 10000\n> + # Raspberry Pi vendor controls\n> + rpi: 20000\n> + # Next range starts at 30000\n> +\n> +...\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 7eb26ccbb7f5..303eee84a77e 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -142,11 +142,13 @@ foreach source, mode : control_source_files\n> endforeach\n> \n> template_file = files(source + '.cpp.in')\n> + ranges_file = files('control_ranges.yaml')\n> control_sources += custom_target(source + '_cpp',\n> input : input_files,\n> output : source + '.cpp',\n> command : [gen_controls, '-o', '@OUTPUT@',\n> - '--mode', mode, '-t', template_file, '@INPUT@'])\n> + '--mode', mode, '-t', template_file,\n> + '-r', ranges_file, '@INPUT@'])\n> endforeach\n> \n> libcamera_sources += control_sources\n> diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> index d7862142b8c1..9eede8940f24 100755\n> --- a/utils/gen-controls.py\n> +++ b/utils/gen-controls.py\n> @@ -257,7 +257,7 @@ ${vendor_controls_map}\n> }\n> \n> \n> -def generate_h(controls, mode):\n> +def generate_h(controls, mode, ranges):\n> enum_template_start = string.Template('''enum ${name}Enum {''')\n> enum_value_template = string.Template('''\\t${name} = ${value},''')\n> enum_values_template = string.Template('''extern const std::array ${name}Values;''')\n> @@ -272,8 +272,10 @@ def generate_h(controls, mode):\n> \n> vendor = 'draft' if ctrl.is_draft else ctrl.vendor\n> if vendor not in ctrls:\n> + if vendor not in ranges.keys():\n> + raise RuntimeError(f'Control id range is not defined for vendor {vendor}')\n> + id_value[vendor] = ranges[vendor] + 1\n> ids[vendor] = []\n> - id_value[vendor] = 1\n> ctrls[vendor] = []\n> \n> # Core and draft controls use the same ID value\n> @@ -365,8 +367,15 @@ def main(argv):\n> help='Template file name.')\n> parser.add_argument('--mode', type=str, required=True, choices=['controls', 'properties'],\n> help='Mode of operation')\n> + parser.add_argument('-r', dest='ranges', type=str, required=True,\n> + help='Control id range reservation file.')\n> args = parser.parse_args(argv[1:])\n> \n> + ranges = []\n> + if args.ranges:\n\nAs the argument is required, I would drop the check.\n\nReviewed-by: Laurent Pinchart \n\n> + data = open(args.ranges, 'rb').read()\n> + ranges = yaml.safe_load(data)['ranges']\n> +\n> controls = []\n> for input in args.input:\n> data = open(input, 'rb').read()\n> @@ -377,7 +386,7 @@ def main(argv):\n> if args.template.endswith('.cpp.in'):\n> data = generate_cpp(controls)\n> elif args.template.endswith('.h.in'):\n> - data = generate_h(controls, args.mode)\n> + data = generate_h(controls, args.mode, ranges)\n> else:\n> raise RuntimeError('Unknown template type')\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 C3098BDE6B\n\tfor ;\n\tThu, 23 Nov 2023 09:41:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 21015629BB;\n\tThu, 23 Nov 2023 10:41:28 +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 0B2DF629AF\n\tfor ;\n\tThu, 23 Nov 2023 10:41:26 +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 208A1A06;\n\tThu, 23 Nov 2023 10:40:54 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700732488;\n\tbh=2hQ+G8+N9/l4T2Qbo61Z1c1Ad89/QwXCrJ5G8wcvbDo=;\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=IezSOiA8PGk4OGS4IDm2dVtEEXsc2Wrkc7Iktke4GnqDLcxJQ4q6Kfd85crHWN+Yq\n\t2Oscp3GCKIQxqoaGRwHs3VOv+FVebykHrxTYkpCKzgB+38L7LLdXYgVfeJ7ex3bYMk\n\tJVPK79zvO/BZnRWY/r9G4LEnXuMApvT4ea66z5yqZfT951RIDFE5jUCLXfX15VdPuE\n\thNwI8bpChJ/zZIVjDYOkfD+H0kaN4jlws7DWLlzwmqybn/kRLDQG+iCXYOAX8wNnA5\n\tj4KuP6Z6YfS42PwJhAG7u79+LLAL4w1RWgX7fGYLqSAPU/piUqxGzwjfYNKwDuOLHm\n\t9fyFBKTKMhjJg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1700732454;\n\tbh=2hQ+G8+N9/l4T2Qbo61Z1c1Ad89/QwXCrJ5G8wcvbDo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FBTrxNq/ydif/GVWG5dNVM1R27yLs8sSzpRB4TNe4iEwI3bu0YEHYGu3H/ckCnN5f\n\tHefwuwScYOsbXIYjmg810VHJDPdsaKeWGNu3wfCG0ZMo+EwR74YiYHTQqmjSf9nOQ8\n\t2jWmTuDjJZ7UzSAy3mJUZuWV/+PgK8zKB4BQFBbM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"FBTrxNq/\"; dkim-atps=neutral","Date":"Thu, 23 Nov 2023 11:41:32 +0200","To":"Naushir Patuck ","Message-ID":"<20231123094132.GG15697@pendragon.ideasonboard.com>","References":"<20231121145350.5956-1-naush@raspberrypi.com>\n\t<20231121145350.5956-5-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20231121145350.5956-5-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 4/6] libcamera: control: Add vendor\n\tcontrol id range reservation","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":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]