[libcamera-devel] [PATCH v2 15/19] py: Re-implement controls geneneration

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, May 24, 2022 at 02:46:06PM +0300, Tomi Valkeinen wrote:
> The Python bindings controls generation was not very good. It only
> covered the enums and they were in the main namespace.
> 
> This adds the controls somewhat similarly to the C++ side. We will have
> e.g.:
> 
> libcamera.controls.Brightness
> libcamera.controls.AeMeteringModeEnum.CentreWeighted
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
> ---
>  ...py-control-enums.py => gen-py-controls.py} | 20 +++++++++++--------
>  src/py/libcamera/meson.build                  | 16 +++++++++------
>  ...ed.cpp.in => py_controls_generated.cpp.in} | 17 +++++++++++++---
>  src/py/libcamera/py_main.cpp                  |  4 ++--
>  4 files changed, 38 insertions(+), 19 deletions(-)
>  rename src/py/libcamera/{gen-py-control-enums.py => gen-py-controls.py} (85%)
>  rename src/py/libcamera/{py_control_enums_generated.cpp.in => py_controls_generated.cpp.in} (50%)
> 
> diff --git a/src/py/libcamera/gen-py-control-enums.py b/src/py/libcamera/gen-py-controls.py
> similarity index 85%
> rename from src/py/libcamera/gen-py-control-enums.py
> rename to src/py/libcamera/gen-py-controls.py
> index 6b2b5362..e3e1e178 100755
> --- a/src/py/libcamera/gen-py-control-enums.py
> +++ b/src/py/libcamera/gen-py-controls.py
> @@ -1,7 +1,7 @@
>  #!/usr/bin/env python3
>  # SPDX-License-Identifier: GPL-2.0-or-later
>  #
> -# Generate Python bindings enums for controls from YAML
> +# Generate Python bindings controls from YAML
>  
>  import argparse
>  import string
> @@ -27,18 +27,22 @@ def generate_py(controls):
>      for ctrl in controls:
>          name, ctrl = ctrl.popitem()
>  
> -        enum = ctrl.get('enum')
> -        if not enum:
> -            continue
> -
>          if ctrl.get('draft'):
>              ns = 'libcamera::controls::draft::'
> +            container = "draft"
>          else:
>              ns = 'libcamera::controls::'
> +            container = "controls"
>  
>          cpp_enum = name + 'Enum'

I'd move this line after the enum check below.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  
> -        out += '\tpy::enum_<{}{}>(m, \"{}\")\n'.format(ns, cpp_enum, name)
> +        out += f'\t{container}.def_readonly_static("{name}", static_cast<const libcamera::ControlId *>(&{ns}{name}));\n\n'
> +
> +        enum = ctrl.get('enum')
> +        if not enum:
> +            continue
> +
> +        out += '\tpy::enum_<{}{}>({}, \"{}\")\n'.format(ns, cpp_enum, container, cpp_enum)
>  
>          if name == 'LensShadingMapMode':
>              prefix = 'LensShadingMapMode'
> @@ -54,9 +58,9 @@ def generate_py(controls):
>  
>              out += '\t\t.value(\"{}\", {}{})\n'.format(py_enum, ns, cpp_enum)
>  
> -        out += '\t;\n'
> +        out += '\t;\n\n'
>  
> -    return {'enums': out}
> +    return {'controls': out}
>  
>  
>  def fill_template(template, data):
> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
> index b705ac1f..e8010846 100644
> --- a/src/py/libcamera/meson.build
> +++ b/src/py/libcamera/meson.build
> @@ -18,17 +18,21 @@ pycamera_sources = files([
>      'py_main.cpp',
>  ])
>  
> -gen_py_control_enums_input_files = files([
> +# Generate controls
> +
> +gen_py_controls_input_files = files([
>      '../../libcamera/control_ids.yaml',
> -    'py_control_enums_generated.cpp.in',
> +    'py_controls_generated.cpp.in',
>  ])
>  
> -gen_py_control_enums = files('gen-py-control-enums.py')
> +gen_py_controls = files('gen-py-controls.py')
>  
>  pycamera_sources += custom_target('py_gen_controls',
> -                                  input : gen_py_control_enums_input_files,
> -                                  output : ['py_control_enums_generated.cpp'],
> -                                  command : [gen_py_control_enums, '-o', '@OUTPUT@', '@INPUT@'])
> +                                  input : gen_py_controls_input_files,
> +                                  output : ['py_controls_generated.cpp'],
> +                                  command : [gen_py_controls, '-o', '@OUTPUT@', '@INPUT@'])
> +
> +# Generate formats
>  
>  gen_py_formats_input_files = files([
>      '../../libcamera/formats.yaml',
> diff --git a/src/py/libcamera/py_control_enums_generated.cpp.in b/src/py/libcamera/py_controls_generated.cpp.in
> similarity index 50%
> rename from src/py/libcamera/py_control_enums_generated.cpp.in
> rename to src/py/libcamera/py_controls_generated.cpp.in
> index ed81fbe7..cb8442ba 100644
> --- a/src/py/libcamera/py_control_enums_generated.cpp.in
> +++ b/src/py/libcamera/py_controls_generated.cpp.in
> @@ -2,7 +2,7 @@
>  /*
>   * Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>   *
> - * Python bindings - Auto-generated control enums
> + * Python bindings - Auto-generated controls
>   *
>   * This file is auto-generated. Do not edit.
>   */
> @@ -13,7 +13,18 @@
>  
>  namespace py = pybind11;
>  
> -void init_py_control_enums_generated(py::module& m)
> +class PyControls
>  {
> -${enums}
> +};
> +
> +class PyDraftControls
> +{
> +};
> +
> +void init_py_controls_generated(py::module& m)
> +{
> +	auto controls = py::class_<PyControls>(m, "controls");
> +	auto draft = py::class_<PyDraftControls>(controls, "draft");
> +
> +${controls}
>  }
> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
> index 5d389942..33ecc1cd 100644
> --- a/src/py/libcamera/py_main.cpp
> +++ b/src/py/libcamera/py_main.cpp
> @@ -131,14 +131,14 @@ static void handleRequestCompleted(Request *req)
>  }
>  
>  void init_py_enums(py::module &m);
> -void init_py_control_enums_generated(py::module &m);
> +void init_py_controls_generated(py::module &m);
>  void init_py_formats_generated(py::module &m);
>  void init_py_geometry(py::module &m);
>  
>  PYBIND11_MODULE(_libcamera, m)
>  {
>  	init_py_enums(m);
> -	init_py_control_enums_generated(m);
> +	init_py_controls_generated(m);
>  	init_py_geometry(m);
>  
>  	/* Forward declarations */

-- 
Regards,

Laurent Pinchart