| Message ID | 20221006230747.11688-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thank you for the patch. On 10/7/22 4:37 AM, 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. > > 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 d6b4e30001b2..45b7081e6b1d 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_values = 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') Not sure if there's a case where size is not None, but has len(size) == 0; Anyways, it's not harmful to have a guard here > + > + # Compute the total number of elemens in the array. If any of the s/elemens/elements/ > + # 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 > + Looks good! Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > @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}>" > >
Hi Umang, On Fri, Oct 07, 2022 at 08:06:55PM +0530, Umang Jain wrote: > On 10/7/22 4:37 AM, 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. > > > > 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 d6b4e30001b2..45b7081e6b1d 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_values = 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') > > Not sure if there's a case where size is not None, but has len(size) == > 0; Anyways, it's not harmful to have a guard here It happens with '[]'. > > + > > + # Compute the total number of elemens in the array. If any of the > > s/elemens/elements/ > > > + # 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 > > + > > Looks good! > > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > > > @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}>" > > > >
Hi, On 10/7/22 8:14 PM, Laurent Pinchart wrote: > Hi Umang, > > On Fri, Oct 07, 2022 at 08:06:55PM +0530, Umang Jain wrote: >> On 10/7/22 4:37 AM, 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. >>> >>> 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 d6b4e30001b2..45b7081e6b1d 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_values = 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') >> Not sure if there's a case where size is not None, but has len(size) == >> 0; Anyways, it's not harmful to have a guard here > It happens with '[]'. Ah that makes sense now... my python chops a bit rusty.. > >>> + >>> + # Compute the total number of elemens in the array. If any of the >> s/elemens/elements/ >> >>> + # 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 >>> + >> Looks good! >> >> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> >> >>> @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 d6b4e30001b2..45b7081e6b1d 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_values = 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(-)