[libcamera-devel] [PATCH 01/10] libcamera: Support draft controls and properties
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Oct 12 03:03:44 CEST 2020
Hello,
One more thing.
On Mon, Oct 12, 2020 at 03:01:03AM +0300, Laurent Pinchart wrote:
> Hi Jacopo and Kieran,
>
> Thank you for the patch.
>
> On Fri, Oct 09, 2020 at 02:20:52PM +0200, Jacopo Mondi wrote:
> > From: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >
> > Extend the control and property framework to support exposing draft
> > controls and properties in a scoped namespace.
> >
> > The controls/properties themselves will retain the same ordering in the
> > relevant enum/id maps - but the access to any draft control will require
> > explicitly referencing through its' draft:: namespace prefix.
> >
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > # Added missing hunk in control_ids.cpp.in and changed subject
>
> Isn't the usual syntax
>
> [Added missing hunk in control_ids.cpp.in and changed subject]
>
> ? Or is that kernel-only ?
>
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> > include/libcamera/control_ids.h.in | 6 +++++
> > include/libcamera/property_ids.h.in | 6 +++++
> > src/libcamera/control_ids.cpp.in | 13 +++++++++++
> > src/libcamera/property_ids.cpp.in | 13 +++++++++++
> > utils/gen-controls.py | 36 ++++++++++++++++++++++-------
> > 5 files changed, 66 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in
> > index 95a7a7f1e260..baadca83b103 100644
> > --- a/include/libcamera/control_ids.h.in
> > +++ b/include/libcamera/control_ids.h.in
> > @@ -26,6 +26,12 @@ ${controls}
> >
> > extern const ControlIdMap controls;
> >
> > +namespace draft {
> > +
> > +${draft_controls}
> > +
> > +} /* namespace draft */
> > +
> > } /* namespace controls */
> >
> > } /* namespace libcamera */
> > diff --git a/include/libcamera/property_ids.h.in b/include/libcamera/property_ids.h.in
> > index e4dea335cd3b..52646c1f78ae 100644
> > --- a/include/libcamera/property_ids.h.in
> > +++ b/include/libcamera/property_ids.h.in
> > @@ -24,6 +24,12 @@ ${ids}
> >
> > ${controls}
> >
> > +namespace draft {
> > +
> > +${draft_controls}
> > +
> > +} /* namespace draft */
> > +
> > extern const ControlIdMap properties;
> >
> > } /* namespace properties */
> > diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in
> > index cba6258d68dd..d31c1f5011b9 100644
> > --- a/src/libcamera/control_ids.cpp.in
> > +++ b/src/libcamera/control_ids.cpp.in
> > @@ -23,14 +23,27 @@ namespace controls {
> >
> > ${controls_doc}
> >
This requires
/**
* \brief Namespace for libcamera draft controls
*/
> > +namespace draft {
> > +
> > +${draft_controls_doc}
> > +
> > +} /* namespace draft */
> > +
> > #ifndef __DOXYGEN__
> > /*
> > * Keep the controls definitions hidden from doxygen as it incorrectly parses
> > * them as functions.
> > */
> > ${controls_def}
> > +
> > +namespace draft {
> > +
> > +${draft_controls_def}
> > +
> > +} /* namespace draft */
> > #endif
> >
> > +
>
> Extra blank line.
>
> > /**
> > * \brief List of all supported libcamera controls
> > *
> > diff --git a/src/libcamera/property_ids.cpp.in b/src/libcamera/property_ids.cpp.in
> > index bfdd823f63b0..275c1caff3ec 100644
> > --- a/src/libcamera/property_ids.cpp.in
> > +++ b/src/libcamera/property_ids.cpp.in
> > @@ -23,12 +23,25 @@ namespace properties {
> >
> > ${controls_doc}
> >
And here
/**
* \brief Namespace for libcamera draft properties
*/
Otherwise documentation isn't properly generated. Could you double-check
the documentation output ?
> > +namespace draft {
> > +
> > +${draft_controls_doc}
> > +
> > +} /* namespace draft */
> > +
> > #ifndef __DOXYGEN__
> > /*
> > * Keep the properties definitions hidden from doxygen as it incorrectly parses
> > * them as functions.
> > */
> > ${controls_def}
> > +
> > +namespace draft {
> > +
> > +${draft_controls_def}
> > +
> > +} /* namespace draft */
> > +
>
> This one isn't needed either.
>
> > #endif
> >
> > /**
> > diff --git a/utils/gen-controls.py b/utils/gen-controls.py
> > index 87c3d52ada6d..f28db27ba19f 100755
> > --- a/utils/gen-controls.py
> > +++ b/utils/gen-controls.py
> > @@ -36,6 +36,8 @@ ${description}
> >
> > ctrls_doc = []
> > ctrls_def = []
> > + draft_ctrls_doc = []
> > + draft_ctrls_def = []
> > ctrls_map = []
> >
> > for ctrl in controls:
> > @@ -55,6 +57,12 @@ ${description}
> > 'id_name': id_name,
> > }
> >
> > + target_doc = ctrls_doc
> > + target_def = ctrls_def
> > + if ctrl.get('draft'):
> > + target_doc = draft_ctrls_doc
> > + target_def = draft_ctrls_def
> > +
> > enum = ctrl.get('enum')
> > if enum:
> > enum_doc = []
> > @@ -70,15 +78,21 @@ ${description}
> >
> > enum_doc = '\n *\n'.join(enum_doc)
> > enum_doc += '\n */'
> > - ctrls_doc.append(enum_doc)
> > + target_doc.append(enum_doc)
> > +
> > + target_doc.append(doc_template.substitute(info))
> > + target_def.append(def_template.substitute(info))
> > +
> > + if ctrl.get('draft'):
> > + name = 'draft::' + name
> >
> > - ctrls_doc.append(doc_template.substitute(info))
> > - ctrls_def.append(def_template.substitute(info))
> > ctrls_map.append('\t{ ' + id_name + ', &' + name + ' },')
> >
> > return {
> > 'controls_doc': '\n\n'.join(ctrls_doc),
> > 'controls_def': '\n'.join(ctrls_def),
> > + 'draft_controls_doc': '\n\n'.join(draft_ctrls_doc),
> > + 'draft_controls_def': '\n\n'.join(draft_ctrls_def),
> > 'controls_map': '\n'.join(ctrls_map),
> > }
> >
> > @@ -89,6 +103,7 @@ def generate_h(controls):
> > template = string.Template('''extern const Control<${type}> ${name};''')
> >
> > ctrls = []
> > + draft_ctrls = []
> > ids = []
> > id_value = 1
> >
> > @@ -109,22 +124,27 @@ def generate_h(controls):
> > 'type': ctrl_type,
> > }
> >
> > + target_ctrls = ctrls
> > + if ctrl.get('draft'):
> > + target_ctrls = draft_ctrls
> > +
> > enum = ctrl.get('enum')
> > if enum:
> > - ctrls.append(enum_template_start.substitute(info))
> > + target_ctrls.append(enum_template_start.substitute(info))
> >
> > for entry in enum:
> > value_info = {
> > 'name': entry['name'],
> > 'value': entry['value'],
> > }
> > - ctrls.append(enum_value_template.substitute(value_info))
> > - ctrls.append("};")
> > + target_ctrls.append(enum_value_template.substitute(value_info))
> > + target_ctrls.append("};")
> >
> > - ctrls.append(template.substitute(info))
> > + target_ctrls.append(template.substitute(info))
> > id_value += 1
> >
> > - return {'ids': '\n'.join(ids), 'controls': '\n'.join(ctrls)}
> > + return {'ids': '\n'.join(ids), 'controls': '\n'.join(ctrls),
> > + 'draft_controls': '\n'.join(draft_ctrls)}
>
> You could format this as
>
> return {
> 'ids': '\n'.join(ids),
> 'controls': '\n'.join(ctrls),
> 'draft_controls': '\n'.join(draft_ctrls),
> }
>
> to make it a bit more readable.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> >
> >
> > def fill_template(template, data):
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list