[libcamera-devel] [PATCH v2 03/10] libcamera: properties: Generate libcamera properties

Niklas Söderlund niklas.soderlund at ragnatech.se
Fri Dec 6 22:42:03 CET 2019


Hi Jacopo,

Thanks for your work.

On 2019-12-05 21:43:43 +0100, Jacopo Mondi wrote:
> Re-use the Control generation infrastructure to generate libcamera properties.
> 
> Introduce three additional files, one that enumerates the properties ids
> (include/libcamera/property_ids.h) and one the defines Control<> instances,
> one for each property (src/libcamera/property_ids.cpp) and provide
> properties definitions in src/libcamera/property_ids.yaml

This is also hard for me to parse, how about

Introduce three additional files,

- include/libcamera/property_ids.h
  Defines the properties ids.

- src/libcamera/property_ids.cpp
  Defines the properties Control<> instances.

- src/libcamera/property_ids.yaml
  Provide properties definitions.

> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  include/libcamera/meson.build       | 26 +++++++++++------
>  include/libcamera/property_ids.h.in | 33 ++++++++++++++++++++++
>  src/libcamera/meson.build           | 21 ++++++++------
>  src/libcamera/property_ids.cpp.in   | 43 +++++++++++++++++++++++++++++
>  src/libcamera/property_ids.yaml     | 34 +++++++++++++++++++++++
>  5 files changed, 140 insertions(+), 17 deletions(-)
>  create mode 100644 include/libcamera/property_ids.h.in
>  create mode 100644 src/libcamera/property_ids.cpp.in
>  create mode 100644 src/libcamera/property_ids.yaml
> 
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 99abf0609940..25f503660960 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -23,15 +23,23 @@ install_headers(libcamera_api,
>  
>  gen_controls = files('../../src/libcamera/gen-controls.py')
>  
> -control_ids_h = custom_target('control_ids_h',
> -                              input : files('../../src/libcamera/control_ids.yaml', 'control_ids.h.in'),
> -                              output : 'control_ids.h',
> -                              depend_files : gen_controls,
> -                              command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'],
> -                              install : true,
> -                              install_dir : join_paths('include', include_dir))
> -
> -libcamera_api += control_ids_h
> +control_source_files = [
> +    'control_ids',
> +    'property_ids',
> +]
> +
> +control_headers = []
> +
> +foreach header : control_source_files
> +    input_files = files('../../src/libcamera/' + header +'.yaml', header + '.h.in')
> +    control_headers += custom_target(header + '_h',
> +                                     input : input_files,
> +                                     output : header + '.h',
> +                                     depend_files : gen_controls,
> +                                     command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'],
> +                                     install : true,
> +                                     install_dir : join_paths('include', include_dir))
> +endforeach

I would have split this commit in two and added the foreach loop in a 
separate patch.

>  
>  gen_header = files('gen-header.sh')
>  
> diff --git a/include/libcamera/property_ids.h.in b/include/libcamera/property_ids.h.in
> new file mode 100644
> index 000000000000..62799b3e8c54
> --- /dev/null
> +++ b/include/libcamera/property_ids.h.in
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * property_ids.h : Property ID list
> + *
> + * This file is auto-generated. Do not edit.
> + */
> +
> +#ifndef __LIBCAMERA_PROPERTY_IDS_H__
> +#define __LIBCAMERA_PROPERTY_IDS_H__
> +
> +#include <stdint.h>
> +
> +#include <libcamera/controls.h>
> +
> +namespace libcamera {
> +
> +namespace properties {
> +
> +enum {
> +${ids}
> +};
> +
> +${controls}
> +
> +extern const ControlIdMap properties;
> +
> +} /* namespace propertiess */
> +
> +} /* namespace libcamera */
> +
> +#endif // __LIBCAMERA_PROPERTY_IDS_H__
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index c4f965bd7413..14aff6e5fc13 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -67,14 +67,19 @@ endif
>  
>  gen_controls = files('gen-controls.py')
>  
> -control_ids_cpp = custom_target('control_ids_cpp',
> -                                input : files('control_ids.yaml', 'control_ids.cpp.in'),
> -                                output : 'control_ids.cpp',
> -                                depend_files : gen_controls,
> -                                command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'])
> -
> -libcamera_sources += control_ids_cpp
> -libcamera_sources += control_ids_h
> +control_sources = []
> +
> +foreach source : control_source_files
> +    input_files = files(source +'.yaml', source + '.cpp.in')
> +    control_sources += custom_target(source + '_cpp',
> +                                     input : input_files,
> +                                     output : source + '.cpp',
> +                                     depend_files : gen_controls,
> +                                     command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'])
> +endforeach

Same here the foreach conversion could be put together with the one 
above in a separate patch.

> +
> +libcamera_sources += control_headers
> +libcamera_sources += control_sources
>  
>  gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')
>  
> diff --git a/src/libcamera/property_ids.cpp.in b/src/libcamera/property_ids.cpp.in
> new file mode 100644
> index 000000000000..bfdd823f63b0
> --- /dev/null
> +++ b/src/libcamera/property_ids.cpp.in
> @@ -0,0 +1,43 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * property_ids.cpp : Property ID list
> + *
> + * This file is auto-generated. Do not edit.
> + */
> +
> +#include <libcamera/property_ids.h>
> +
> +/**
> + * \file property_ids.h
> + * \brief Camera property identifiers
> + */
> +
> +namespace libcamera {
> +
> +/**
> + * \brief Namespace for libcamera properties
> + */
> +namespace properties {
> +
> +${controls_doc}
> +
> +#ifndef __DOXYGEN__
> +/*
> + * Keep the properties definitions hidden from doxygen as it incorrectly parses
> + * them as functions.
> + */
> +${controls_def}
> +#endif
> +
> +/**
> + * \brief List of all supported libcamera properties
> + */
> +extern const ControlIdMap properties {
> +${controls_map}
> +};
> +
> +} /* namespace properties */
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> new file mode 100644
> index 000000000000..6ab5be83bc49
> --- /dev/null
> +++ b/src/libcamera/property_ids.yaml
> @@ -0,0 +1,34 @@
> +# SPDX-License-Identifier: LGPL-2.1-or-later
> +#
> +# Copyright (C) 2019, Google Inc.
> +#
> +%YAML 1.2
> +---
> +controls:
> +  - Location:
> +      type: int32_t
> +      description: |
> +        Camera mounting location
> +      values:
> +        - CAMERA_LOCATION_FRONT:
> +          value: 0
> +          description: |
> +            The camera is mounted on the front side of the device, facing the
> +            user
> +        - CAMERA_LOCATION_BACK:
> +          value: 1
> +          description: |
> +            The camera is mounted on the back facing side of the device
> +        - CAMERA_LOCATION_EXTERNAL:
> +          value: 2
> +          description: |
> +            The camera is attached to the device in a way that allows it to
> +            move freely
> +
> +  - Rotation:
> +      type: int32_t
> +      description: |
> +        Camera mounting rotation expressed as counterclockwise rotation degrees
> +        towards the axis perpendicular to the sensor surface and directed away
> +        from it

I know writing this description is really hard. This is the best one I 
seen so far and not sure I can do better. Only missing peace of 
information is where 0 degrees rotation really means. On a hand held 
device is that if the camera of a handheld device is not rotated while 
holding it in portrait or landscape mode?

> +...
> -- 
> 2.23.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund


More information about the libcamera-devel mailing list