| Message ID | 20220524114610.41848-16-tomi.valkeinen@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
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@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@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@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 */
On 27/05/2022 11:05, Laurent Pinchart wrote: > 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@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@ideasonboard.com> Thanks, makes sense. Tomi
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' - 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@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 */
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@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%)