[libcamera-devel] [PATCH 2/4] utils: gen-controls: Improve YAML notation for variable-size array controls

[Laurent Pinchart]

Hi Jacopo,

On Wed, Aug 10, 2022 at 10:28:29AM +0200, Jacopo Mondi wrote:
> On Wed, Aug 10, 2022 at 03:29:04AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > Array controls specify the array size through the YAML 'size' element,
> > which stores a list of values, one per dimension. Variable-size arrays
> > currently use an empty 'size' list, which prevents describing the number
> > of dimensions of the array.
> >
> > Improve this by using the same notation for fixed-size and variable-size
> > array controls. Dimensions that are not fixed are described as a string
> > instead of an integer, such as [n], [n,3] or [w,h]. The strings have
> > currently no special meaning, this may change in the future.
> 
> I can't tell what's best between [] and [n]
> 
> For what I can see [n,3] doesn't give much in terms of parse-time
> correctness but it's just a notion for the reader of the
> documentation, so yeah, nice but I would like to understand first why
> Christian proposed initially to go from [n] to [].

At the moment it's for the reader only, I can imagine using it in the
Control class too in the future, but I'm not sure if there would be any
use case for that beside showing that C++ templates can do crazy stuff.

> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/libcamera/control_ids.yaml  |  2 +-
> >  src/libcamera/property_ids.yaml |  4 ++--
> >  utils/gen-controls.py           | 31 +++++++++++++++++++++++++------
> >  3 files changed, 28 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > index c93f362463dd..a456e6c0358f 100644
> > --- a/src/libcamera/control_ids.yaml
> > +++ b/src/libcamera/control_ids.yaml
> > @@ -525,7 +525,7 @@ controls:
> >          the window where the focal distance for the objects shown in that part
> >          of the image are closest to the camera.
> >
> > -      size: []
> > +      size: [n]
> >
> >    - AfTrigger:
> >        type: int32_t
> > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> > index 960e254495b7..cb55e0ed2283 100644
> > --- a/src/libcamera/property_ids.yaml
> > +++ b/src/libcamera/property_ids.yaml
> > @@ -497,7 +497,7 @@ controls:
> >
> >    - PixelArrayOpticalBlackRectangles:
> >        type: Rectangle
> > -      size: []
> > +      size: [n]
> >        description: |
> >          The pixel array region(s) which contain optical black pixels
> >          considered valid for calibration purposes.
> > @@ -592,7 +592,7 @@ controls:
> >
> >    - PixelArrayActiveAreas:
> >        type: Rectangle
> > -      size: []
> > +      size: [n]
> >        description: |
> >          The PixelArrayActiveAreas property defines the (possibly multiple and
> >          overlapping) portions of the camera sensor readable pixel matrix
> > diff --git a/utils/gen-controls.py b/utils/gen-controls.py
> > index f6a3ae39cc84..bcfbeb7f9f17 100755
> > --- a/utils/gen-controls.py
> > +++ b/utils/gen-controls.py
> > @@ -39,11 +39,33 @@ class Control(object):
> >          self.__name = name
> >          self.__data = data
> >          self.__enum = None
> > +        self.__size = None
> >
> >          enum_values = data.get('enum')
> >          if enum_values is not None:
> >              self.__enum_values = [ControlEnum(enum) for enum in enum_values]
> >
> > +        size = self.__data.get('size')
> > +        if size is not None:
> > +            if len(size) == 0:
> > +                raise RuntimeError(f'Control `{self.__name}` size must have at least one dimension')
> > +
> > +            # Compute the total number of elemens in the array. If any of the
> > +            # array dimension is a string, the array is variable-sized.
> > +            num_elems = 1
> > +            for dim in size:
> > +                if type(dim) is str:
> > +                    num_elems = 0
> > +                    break
> > +
> > +                dim = int(dim)
> > +                if dim <= 0:
> > +                    raise RuntimeError(f'Control `{self.__name}` size must have positive values only')
> > +
> > +                num_elems *= dim
> > +
> > +            self.__size = num_elems
> > +
> >      @property
> >      def description(self):
> >          """The control description"""
> > @@ -86,15 +108,12 @@ class Control(object):
> >          if typ == 'string':
> >              return 'std::string'
> >
> > -        if size is None:
> > +        if self.__size is None:
> >              return typ
> >
> > -        if len(size) > 0:
> > -            # fixed-sized Span
> > -            span_size = reduce(operator.mul, size)
> > -            return f"Span<const {typ}, {span_size}>"
> > +        if self.__size:
> > +            return f"Span<const {typ}, {self.__size}>"
> >          else:
> > -            # variable-sized Span
> >              return f"Span<const {typ}>"
> >
> >

-- 
Regards,

Laurent Pinchart