| Message ID | 20220810002906.5406-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent 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 []. Thanks j > > Signed-off-by: Laurent Pinchart <laurent.pinchart@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 >
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@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}>" > > > >
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}>"
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. Signed-off-by: Laurent Pinchart <laurent.pinchart@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(-)