[libcamera-devel,v12] libcamera: controls: Generate and use fixed-sized Span types
diff mbox series

Message ID 20220728220024.204239-1-Rauch.Christian@gmx.de
State Accepted
Headers show
Series
  • [libcamera-devel,v12] libcamera: controls: Generate and use fixed-sized Span types
Related show

Commit Message

Christian Rauch July 28, 2022, 10 p.m. UTC 
Define Span types explicitly as either variable- or fixed-sized. This
introduces a new convention for defining Span dimensions in the property
and control value definitions and generates Span types as variable-sized
Span<T> or as fixed-sized Span<T,N>.

Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/libcamera/controls.h        |  2 +-
 src/ipa/raspberrypi/raspberrypi.cpp | 19 +++++++++--------
 src/libcamera/control_ids.yaml      |  4 ++--
 src/libcamera/property_ids.yaml     |  4 ++--
 src/qcam/dng_writer.cpp             |  2 +-
 utils/gen-controls.py               | 32 ++++++++++++++++++++---------
 6 files changed, 38 insertions(+), 25 deletions(-)

--
2.34.1

Comments

Jacopo Mondi Aug. 2, 2022, 9:05 a.m. UTC | #1 
Hi Christian

On Fri, Jul 29, 2022 at 12:00:24AM +0200, Christian Rauch wrote:
> Define Span types explicitly as either variable- or fixed-sized. This
> introduces a new convention for defining Span dimensions in the property
> and control value definitions and generates Span types as variable-sized
> Span<T> or as fixed-sized Span<T,N>.
>
> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

I got this failure when running the series through our buildbot,
specifically with gcc8:
https://buildbot.libcamera.org/#/builders/6/builds/430/steps/3/logs/stdio

I'll have a look as well, but I can't reproduce locally to test
easily.


> ---
>  include/libcamera/controls.h        |  2 +-
>  src/ipa/raspberrypi/raspberrypi.cpp | 19 +++++++++--------
>  src/libcamera/control_ids.yaml      |  4 ++--
>  src/libcamera/property_ids.yaml     |  4 ++--
>  src/qcam/dng_writer.cpp             |  2 +-
>  utils/gen-controls.py               | 32 ++++++++++++++++++++---------
>  6 files changed, 38 insertions(+), 25 deletions(-)
>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index b1b52acb..ebc168fc 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -168,7 +168,7 @@ public:
>
>  		using V = typename T::value_type;
>  		const V *value = reinterpret_cast<const V *>(data().data());
> -		return { value, numElements_ };
> +		return T{ value, numElements_ };
>  	}
>
>  #ifndef __DOXYGEN__
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 6befdd71..7d314ed3 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -567,18 +567,19 @@ void IPARPi::reportMetadata()
>
>  	AwbStatus *awbStatus = rpiMetadata_.getLocked<AwbStatus>("awb.status");
>  	if (awbStatus) {
> -		libcameraMetadata_.set(controls::ColourGains, { static_cast<float>(awbStatus->gainR),
> -								static_cast<float>(awbStatus->gainB) });
> +		libcameraMetadata_.set(controls::ColourGains,
> +				       Span<const float, 2>({ static_cast<float>(awbStatus->gainR),
> +							      static_cast<float>(awbStatus->gainB) }));
>  		libcameraMetadata_.set(controls::ColourTemperature, awbStatus->temperatureK);
>  	}
>
>  	BlackLevelStatus *blackLevelStatus = rpiMetadata_.getLocked<BlackLevelStatus>("black_level.status");
>  	if (blackLevelStatus)
>  		libcameraMetadata_.set(controls::SensorBlackLevels,
> -				       { static_cast<int32_t>(blackLevelStatus->blackLevelR),
> -					 static_cast<int32_t>(blackLevelStatus->blackLevelG),
> -					 static_cast<int32_t>(blackLevelStatus->blackLevelG),
> -					 static_cast<int32_t>(blackLevelStatus->blackLevelB) });
> +				       Span<const int32_t, 4>({ blackLevelStatus->blackLevelR,
> +								blackLevelStatus->blackLevelG,
> +								blackLevelStatus->blackLevelG,
> +								blackLevelStatus->blackLevelB }));
>
>  	FocusStatus *focusStatus = rpiMetadata_.getLocked<FocusStatus>("focus.status");
>  	if (focusStatus && focusStatus->num == 12) {
> @@ -883,7 +884,7 @@ void IPARPi::queueRequest(const ControlList &controls)
>  			if (gains[0] != 0.0f && gains[1] != 0.0f)
>  				/* A gain of 0.0f will switch back to auto mode. */
>  				libcameraMetadata_.set(controls::ColourGains,
> -						       { gains[0], gains[1] });
> +						       Span<const float, 2>({ gains[0], gains[1] }));
>  			break;
>  		}
>
> @@ -1167,8 +1168,8 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur
>
>  	/* Return the validated limits via metadata. */
>  	libcameraMetadata_.set(controls::FrameDurationLimits,
> -			       { static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
> -				 static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) });
> +			       Span<const int64_t, 2>({ static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
> +							static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) }));
>
>  	/*
>  	 * Calculate the maximum exposure time possible for the AGC to use.
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> index ecab3ae9..5fa168c6 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -291,7 +291,7 @@ controls:
>          transformation. The 3x3 matrix is stored in conventional reading
>          order in an array of 9 floating point values.
>
> -      size: [3x3]
> +      size: [3,3]
>
>    - ScalerCrop:
>        type: Rectangle
> @@ -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: [n]
> +      size: []
>
>    - AfTrigger:
>        type: int32_t
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> index 11b7ebdc..a87485d7 100644
> --- a/src/libcamera/property_ids.yaml
> +++ b/src/libcamera/property_ids.yaml
> @@ -497,7 +497,7 @@ controls:
>
>    - PixelArrayOpticalBlackRectangles:
>        type: Rectangle
> -      size: [n]
> +      size: []
>        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: [n]
> +      size: []
>        description: |
>          The PixelArrayActiveAreas property defines the (possibly multiple and
>          overlapping) portions of the camera sensor readable pixel matrix
> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
> index 5b94b3f2..b4362537 100644
> --- a/src/qcam/dng_writer.cpp
> +++ b/src/qcam/dng_writer.cpp
> @@ -517,7 +517,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,
>
>  	const auto &blackLevels = metadata.get(controls::SensorBlackLevels);
>  	if (blackLevels) {
> -		Span<const int32_t> levels = *blackLevels;
> +		Span<const int32_t, 4> levels = *blackLevels;
>
>  		/*
>  		 * The black levels control is specified in R, Gr, Gb, B order.
> diff --git a/utils/gen-controls.py b/utils/gen-controls.py
> index 3f99b5e2..46ba4394 100755
> --- a/utils/gen-controls.py
> +++ b/utils/gen-controls.py
> @@ -7,6 +7,8 @@
>  # gen-controls.py - Generate control definitions from YAML
>
>  import argparse
> +from functools import reduce
> +import operator
>  import string
>  import sys
>  import yaml
> @@ -22,6 +24,24 @@ def format_description(description):
>      return '\n'.join([(line and ' * ' or ' *') + line for line in description])
>
>
> +def get_ctrl_type(ctrl):
> +    ctrl_type = ctrl['type']
> +    ctrl_size_arr = ctrl.get('size')
> +
> +    if ctrl_type == 'string':
> +        return 'std::string'
> +    elif ctrl_size_arr is not None:
> +        if len(ctrl_size_arr) > 0:
> +            # fixed-sized Span
> +            ctrl_span_size = reduce(operator.mul, ctrl_size_arr)
> +            return f"Span<const {ctrl_type}, {ctrl_span_size}>"
> +        else:
> +            # variable-sized Span
> +            return f"Span<const {ctrl_type}>"
> +    else:
> +        return ctrl_type
> +
> +
>  def generate_cpp(controls):
>      enum_doc_start_template = string.Template('''/**
>   * \\enum ${name}Enum
> @@ -50,11 +70,7 @@ ${description}
>          name, ctrl = ctrl.popitem()
>          id_name = snake_case(name).upper()
>
> -        ctrl_type = ctrl['type']
> -        if ctrl_type == 'string':
> -            ctrl_type = 'std::string'
> -        elif ctrl.get('size'):
> -            ctrl_type = 'Span<const %s>' % ctrl_type
> +        ctrl_type = get_ctrl_type(ctrl)
>
>          info = {
>              'name': name,
> @@ -135,11 +151,7 @@ def generate_h(controls):
>
>          ids.append('\t' + id_name + ' = ' + str(id_value) + ',')
>
> -        ctrl_type = ctrl['type']
> -        if ctrl_type == 'string':
> -            ctrl_type = 'std::string'
> -        elif ctrl.get('size'):
> -            ctrl_type = 'Span<const %s>' % ctrl_type
> +        ctrl_type = get_ctrl_type(ctrl)
>
>          info = {
>              'name': name,
> --
> 2.34.1
>
Christian Rauch Aug. 2, 2022, 9:10 p.m. UTC | #2 
Hi Jacopo,

Is there a way for others to push to that buildbot?

I gave this a shot and sent a v13 with explicit type casts. Can you test
this? (I will keep the casts in any case.)

Best,
Christian


Am 02.08.22 um 11:05 schrieb Jacopo Mondi:
> Hi Christian
>
> On Fri, Jul 29, 2022 at 12:00:24AM +0200, Christian Rauch wrote:
>> Define Span types explicitly as either variable- or fixed-sized. This
>> introduces a new convention for defining Span dimensions in the property
>> and control value definitions and generates Span types as variable-sized
>> Span<T> or as fixed-sized Span<T,N>.
>>
>> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
>> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>
> I got this failure when running the series through our buildbot,
> specifically with gcc8:
> https://buildbot.libcamera.org/#/builders/6/builds/430/steps/3/logs/stdio
>
> I'll have a look as well, but I can't reproduce locally to test
> easily.
>
>
>> ---
>>  include/libcamera/controls.h        |  2 +-
>>  src/ipa/raspberrypi/raspberrypi.cpp | 19 +++++++++--------
>>  src/libcamera/control_ids.yaml      |  4 ++--
>>  src/libcamera/property_ids.yaml     |  4 ++--
>>  src/qcam/dng_writer.cpp             |  2 +-
>>  utils/gen-controls.py               | 32 ++++++++++++++++++++---------
>>  6 files changed, 38 insertions(+), 25 deletions(-)
>>
>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
>> index b1b52acb..ebc168fc 100644
>> --- a/include/libcamera/controls.h
>> +++ b/include/libcamera/controls.h
>> @@ -168,7 +168,7 @@ public:
>>
>>  		using V = typename T::value_type;
>>  		const V *value = reinterpret_cast<const V *>(data().data());
>> -		return { value, numElements_ };
>> +		return T{ value, numElements_ };
>>  	}
>>
>>  #ifndef __DOXYGEN__
>> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
>> index 6befdd71..7d314ed3 100644
>> --- a/src/ipa/raspberrypi/raspberrypi.cpp
>> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
>> @@ -567,18 +567,19 @@ void IPARPi::reportMetadata()
>>
>>  	AwbStatus *awbStatus = rpiMetadata_.getLocked<AwbStatus>("awb.status");
>>  	if (awbStatus) {
>> -		libcameraMetadata_.set(controls::ColourGains, { static_cast<float>(awbStatus->gainR),
>> -								static_cast<float>(awbStatus->gainB) });
>> +		libcameraMetadata_.set(controls::ColourGains,
>> +				       Span<const float, 2>({ static_cast<float>(awbStatus->gainR),
>> +							      static_cast<float>(awbStatus->gainB) }));
>>  		libcameraMetadata_.set(controls::ColourTemperature, awbStatus->temperatureK);
>>  	}
>>
>>  	BlackLevelStatus *blackLevelStatus = rpiMetadata_.getLocked<BlackLevelStatus>("black_level.status");
>>  	if (blackLevelStatus)
>>  		libcameraMetadata_.set(controls::SensorBlackLevels,
>> -				       { static_cast<int32_t>(blackLevelStatus->blackLevelR),
>> -					 static_cast<int32_t>(blackLevelStatus->blackLevelG),
>> -					 static_cast<int32_t>(blackLevelStatus->blackLevelG),
>> -					 static_cast<int32_t>(blackLevelStatus->blackLevelB) });
>> +				       Span<const int32_t, 4>({ blackLevelStatus->blackLevelR,
>> +								blackLevelStatus->blackLevelG,
>> +								blackLevelStatus->blackLevelG,
>> +								blackLevelStatus->blackLevelB }));
>>
>>  	FocusStatus *focusStatus = rpiMetadata_.getLocked<FocusStatus>("focus.status");
>>  	if (focusStatus && focusStatus->num == 12) {
>> @@ -883,7 +884,7 @@ void IPARPi::queueRequest(const ControlList &controls)
>>  			if (gains[0] != 0.0f && gains[1] != 0.0f)
>>  				/* A gain of 0.0f will switch back to auto mode. */
>>  				libcameraMetadata_.set(controls::ColourGains,
>> -						       { gains[0], gains[1] });
>> +						       Span<const float, 2>({ gains[0], gains[1] }));
>>  			break;
>>  		}
>>
>> @@ -1167,8 +1168,8 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur
>>
>>  	/* Return the validated limits via metadata. */
>>  	libcameraMetadata_.set(controls::FrameDurationLimits,
>> -			       { static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
>> -				 static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) });
>> +			       Span<const int64_t, 2>({ static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
>> +							static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) }));
>>
>>  	/*
>>  	 * Calculate the maximum exposure time possible for the AGC to use.
>> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
>> index ecab3ae9..5fa168c6 100644
>> --- a/src/libcamera/control_ids.yaml
>> +++ b/src/libcamera/control_ids.yaml
>> @@ -291,7 +291,7 @@ controls:
>>          transformation. The 3x3 matrix is stored in conventional reading
>>          order in an array of 9 floating point values.
>>
>> -      size: [3x3]
>> +      size: [3,3]
>>
>>    - ScalerCrop:
>>        type: Rectangle
>> @@ -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: [n]
>> +      size: []
>>
>>    - AfTrigger:
>>        type: int32_t
>> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
>> index 11b7ebdc..a87485d7 100644
>> --- a/src/libcamera/property_ids.yaml
>> +++ b/src/libcamera/property_ids.yaml
>> @@ -497,7 +497,7 @@ controls:
>>
>>    - PixelArrayOpticalBlackRectangles:
>>        type: Rectangle
>> -      size: [n]
>> +      size: []
>>        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: [n]
>> +      size: []
>>        description: |
>>          The PixelArrayActiveAreas property defines the (possibly multiple and
>>          overlapping) portions of the camera sensor readable pixel matrix
>> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
>> index 5b94b3f2..b4362537 100644
>> --- a/src/qcam/dng_writer.cpp
>> +++ b/src/qcam/dng_writer.cpp
>> @@ -517,7 +517,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,
>>
>>  	const auto &blackLevels = metadata.get(controls::SensorBlackLevels);
>>  	if (blackLevels) {
>> -		Span<const int32_t> levels = *blackLevels;
>> +		Span<const int32_t, 4> levels = *blackLevels;
>>
>>  		/*
>>  		 * The black levels control is specified in R, Gr, Gb, B order.
>> diff --git a/utils/gen-controls.py b/utils/gen-controls.py
>> index 3f99b5e2..46ba4394 100755
>> --- a/utils/gen-controls.py
>> +++ b/utils/gen-controls.py
>> @@ -7,6 +7,8 @@
>>  # gen-controls.py - Generate control definitions from YAML
>>
>>  import argparse
>> +from functools import reduce
>> +import operator
>>  import string
>>  import sys
>>  import yaml
>> @@ -22,6 +24,24 @@ def format_description(description):
>>      return '\n'.join([(line and ' * ' or ' *') + line for line in description])
>>
>>
>> +def get_ctrl_type(ctrl):
>> +    ctrl_type = ctrl['type']
>> +    ctrl_size_arr = ctrl.get('size')
>> +
>> +    if ctrl_type == 'string':
>> +        return 'std::string'
>> +    elif ctrl_size_arr is not None:
>> +        if len(ctrl_size_arr) > 0:
>> +            # fixed-sized Span
>> +            ctrl_span_size = reduce(operator.mul, ctrl_size_arr)
>> +            return f"Span<const {ctrl_type}, {ctrl_span_size}>"
>> +        else:
>> +            # variable-sized Span
>> +            return f"Span<const {ctrl_type}>"
>> +    else:
>> +        return ctrl_type
>> +
>> +
>>  def generate_cpp(controls):
>>      enum_doc_start_template = string.Template('''/**
>>   * \\enum ${name}Enum
>> @@ -50,11 +70,7 @@ ${description}
>>          name, ctrl = ctrl.popitem()
>>          id_name = snake_case(name).upper()
>>
>> -        ctrl_type = ctrl['type']
>> -        if ctrl_type == 'string':
>> -            ctrl_type = 'std::string'
>> -        elif ctrl.get('size'):
>> -            ctrl_type = 'Span<const %s>' % ctrl_type
>> +        ctrl_type = get_ctrl_type(ctrl)
>>
>>          info = {
>>              'name': name,
>> @@ -135,11 +151,7 @@ def generate_h(controls):
>>
>>          ids.append('\t' + id_name + ' = ' + str(id_value) + ',')
>>
>> -        ctrl_type = ctrl['type']
>> -        if ctrl_type == 'string':
>> -            ctrl_type = 'std::string'
>> -        elif ctrl.get('size'):
>> -            ctrl_type = 'Span<const %s>' % ctrl_type
>> +        ctrl_type = get_ctrl_type(ctrl)
>>
>>          info = {
>>              'name': name,
>> --
>> 2.34.1
>>
Jacopo Mondi Aug. 3, 2022, 9:19 a.m. UTC | #3 
Hi Christian

On Tue, Aug 02, 2022 at 11:10:27PM +0200, Christian Rauch via libcamera-devel wrote:
> Hi Jacopo,
>
> Is there a way for others to push to that buildbot?
>

Not currently, I'm afraid

> I gave this a shot and sent a v13 with explicit type casts. Can you test
> this? (I will keep the casts in any case.)
>

Thanks, that solves the issue below.

I stomped upon an unrelated failure with YamlParser::getList() (you
have received a patch for this) which makes it impossible for me to
see a build with gcc8 to complete succesfully, but the control part
seems now sane.

I'll send a tag to v13 to mention I've tested it.

> Best,
> Christian
>
>
> Am 02.08.22 um 11:05 schrieb Jacopo Mondi:
> > Hi Christian
> >
> > On Fri, Jul 29, 2022 at 12:00:24AM +0200, Christian Rauch wrote:
> >> Define Span types explicitly as either variable- or fixed-sized. This
> >> introduces a new convention for defining Span dimensions in the property
> >> and control value definitions and generates Span types as variable-sized
> >> Span<T> or as fixed-sized Span<T,N>.
> >>
> >> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
> >> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> >
> > I got this failure when running the series through our buildbot,
> > specifically with gcc8:
> > https://buildbot.libcamera.org/#/builders/6/builds/430/steps/3/logs/stdio
> >
> > I'll have a look as well, but I can't reproduce locally to test
> > easily.
> >
> >
> >> ---
> >>  include/libcamera/controls.h        |  2 +-
> >>  src/ipa/raspberrypi/raspberrypi.cpp | 19 +++++++++--------
> >>  src/libcamera/control_ids.yaml      |  4 ++--
> >>  src/libcamera/property_ids.yaml     |  4 ++--
> >>  src/qcam/dng_writer.cpp             |  2 +-
> >>  utils/gen-controls.py               | 32 ++++++++++++++++++++---------
> >>  6 files changed, 38 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> >> index b1b52acb..ebc168fc 100644
> >> --- a/include/libcamera/controls.h
> >> +++ b/include/libcamera/controls.h
> >> @@ -168,7 +168,7 @@ public:
> >>
> >>  		using V = typename T::value_type;
> >>  		const V *value = reinterpret_cast<const V *>(data().data());
> >> -		return { value, numElements_ };
> >> +		return T{ value, numElements_ };
> >>  	}
> >>
> >>  #ifndef __DOXYGEN__
> >> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> >> index 6befdd71..7d314ed3 100644
> >> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> >> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> >> @@ -567,18 +567,19 @@ void IPARPi::reportMetadata()
> >>
> >>  	AwbStatus *awbStatus = rpiMetadata_.getLocked<AwbStatus>("awb.status");
> >>  	if (awbStatus) {
> >> -		libcameraMetadata_.set(controls::ColourGains, { static_cast<float>(awbStatus->gainR),
> >> -								static_cast<float>(awbStatus->gainB) });
> >> +		libcameraMetadata_.set(controls::ColourGains,
> >> +				       Span<const float, 2>({ static_cast<float>(awbStatus->gainR),
> >> +							      static_cast<float>(awbStatus->gainB) }));
> >>  		libcameraMetadata_.set(controls::ColourTemperature, awbStatus->temperatureK);
> >>  	}
> >>
> >>  	BlackLevelStatus *blackLevelStatus = rpiMetadata_.getLocked<BlackLevelStatus>("black_level.status");
> >>  	if (blackLevelStatus)
> >>  		libcameraMetadata_.set(controls::SensorBlackLevels,
> >> -				       { static_cast<int32_t>(blackLevelStatus->blackLevelR),
> >> -					 static_cast<int32_t>(blackLevelStatus->blackLevelG),
> >> -					 static_cast<int32_t>(blackLevelStatus->blackLevelG),
> >> -					 static_cast<int32_t>(blackLevelStatus->blackLevelB) });
> >> +				       Span<const int32_t, 4>({ blackLevelStatus->blackLevelR,
> >> +								blackLevelStatus->blackLevelG,
> >> +								blackLevelStatus->blackLevelG,
> >> +								blackLevelStatus->blackLevelB }));
> >>
> >>  	FocusStatus *focusStatus = rpiMetadata_.getLocked<FocusStatus>("focus.status");
> >>  	if (focusStatus && focusStatus->num == 12) {
> >> @@ -883,7 +884,7 @@ void IPARPi::queueRequest(const ControlList &controls)
> >>  			if (gains[0] != 0.0f && gains[1] != 0.0f)
> >>  				/* A gain of 0.0f will switch back to auto mode. */
> >>  				libcameraMetadata_.set(controls::ColourGains,
> >> -						       { gains[0], gains[1] });
> >> +						       Span<const float, 2>({ gains[0], gains[1] }));
> >>  			break;
> >>  		}
> >>
> >> @@ -1167,8 +1168,8 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur
> >>
> >>  	/* Return the validated limits via metadata. */
> >>  	libcameraMetadata_.set(controls::FrameDurationLimits,
> >> -			       { static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
> >> -				 static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) });
> >> +			       Span<const int64_t, 2>({ static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
> >> +							static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) }));
> >>
> >>  	/*
> >>  	 * Calculate the maximum exposure time possible for the AGC to use.
> >> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> >> index ecab3ae9..5fa168c6 100644
> >> --- a/src/libcamera/control_ids.yaml
> >> +++ b/src/libcamera/control_ids.yaml
> >> @@ -291,7 +291,7 @@ controls:
> >>          transformation. The 3x3 matrix is stored in conventional reading
> >>          order in an array of 9 floating point values.
> >>
> >> -      size: [3x3]
> >> +      size: [3,3]
> >>
> >>    - ScalerCrop:
> >>        type: Rectangle
> >> @@ -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: [n]
> >> +      size: []
> >>
> >>    - AfTrigger:
> >>        type: int32_t
> >> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> >> index 11b7ebdc..a87485d7 100644
> >> --- a/src/libcamera/property_ids.yaml
> >> +++ b/src/libcamera/property_ids.yaml
> >> @@ -497,7 +497,7 @@ controls:
> >>
> >>    - PixelArrayOpticalBlackRectangles:
> >>        type: Rectangle
> >> -      size: [n]
> >> +      size: []
> >>        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: [n]
> >> +      size: []
> >>        description: |
> >>          The PixelArrayActiveAreas property defines the (possibly multiple and
> >>          overlapping) portions of the camera sensor readable pixel matrix
> >> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
> >> index 5b94b3f2..b4362537 100644
> >> --- a/src/qcam/dng_writer.cpp
> >> +++ b/src/qcam/dng_writer.cpp
> >> @@ -517,7 +517,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,
> >>
> >>  	const auto &blackLevels = metadata.get(controls::SensorBlackLevels);
> >>  	if (blackLevels) {
> >> -		Span<const int32_t> levels = *blackLevels;
> >> +		Span<const int32_t, 4> levels = *blackLevels;
> >>
> >>  		/*
> >>  		 * The black levels control is specified in R, Gr, Gb, B order.
> >> diff --git a/utils/gen-controls.py b/utils/gen-controls.py
> >> index 3f99b5e2..46ba4394 100755
> >> --- a/utils/gen-controls.py
> >> +++ b/utils/gen-controls.py
> >> @@ -7,6 +7,8 @@
> >>  # gen-controls.py - Generate control definitions from YAML
> >>
> >>  import argparse
> >> +from functools import reduce
> >> +import operator
> >>  import string
> >>  import sys
> >>  import yaml
> >> @@ -22,6 +24,24 @@ def format_description(description):
> >>      return '\n'.join([(line and ' * ' or ' *') + line for line in description])
> >>
> >>
> >> +def get_ctrl_type(ctrl):
> >> +    ctrl_type = ctrl['type']
> >> +    ctrl_size_arr = ctrl.get('size')
> >> +
> >> +    if ctrl_type == 'string':
> >> +        return 'std::string'
> >> +    elif ctrl_size_arr is not None:
> >> +        if len(ctrl_size_arr) > 0:
> >> +            # fixed-sized Span
> >> +            ctrl_span_size = reduce(operator.mul, ctrl_size_arr)
> >> +            return f"Span<const {ctrl_type}, {ctrl_span_size}>"
> >> +        else:
> >> +            # variable-sized Span
> >> +            return f"Span<const {ctrl_type}>"
> >> +    else:
> >> +        return ctrl_type
> >> +
> >> +
> >>  def generate_cpp(controls):
> >>      enum_doc_start_template = string.Template('''/**
> >>   * \\enum ${name}Enum
> >> @@ -50,11 +70,7 @@ ${description}
> >>          name, ctrl = ctrl.popitem()
> >>          id_name = snake_case(name).upper()
> >>
> >> -        ctrl_type = ctrl['type']
> >> -        if ctrl_type == 'string':
> >> -            ctrl_type = 'std::string'
> >> -        elif ctrl.get('size'):
> >> -            ctrl_type = 'Span<const %s>' % ctrl_type
> >> +        ctrl_type = get_ctrl_type(ctrl)
> >>
> >>          info = {
> >>              'name': name,
> >> @@ -135,11 +151,7 @@ def generate_h(controls):
> >>
> >>          ids.append('\t' + id_name + ' = ' + str(id_value) + ',')
> >>
> >> -        ctrl_type = ctrl['type']
> >> -        if ctrl_type == 'string':
> >> -            ctrl_type = 'std::string'
> >> -        elif ctrl.get('size'):
> >> -            ctrl_type = 'Span<const %s>' % ctrl_type
> >> +        ctrl_type = get_ctrl_type(ctrl)
> >>
> >>          info = {
> >>              'name': name,
> >> --
> >> 2.34.1
> >>

Patch
diff mbox series 

diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index b1b52acb..ebc168fc 100644
--- a/include/libcamera/controls.h
+++ b/include/libcamera/controls.h
@@ -168,7 +168,7 @@  public:

 		using V = typename T::value_type;
 		const V *value = reinterpret_cast<const V *>(data().data());
-		return { value, numElements_ };
+		return T{ value, numElements_ };
 	}

 #ifndef __DOXYGEN__
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 6befdd71..7d314ed3 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -567,18 +567,19 @@  void IPARPi::reportMetadata()

 	AwbStatus *awbStatus = rpiMetadata_.getLocked<AwbStatus>("awb.status");
 	if (awbStatus) {
-		libcameraMetadata_.set(controls::ColourGains, { static_cast<float>(awbStatus->gainR),
-								static_cast<float>(awbStatus->gainB) });
+		libcameraMetadata_.set(controls::ColourGains,
+				       Span<const float, 2>({ static_cast<float>(awbStatus->gainR),
+							      static_cast<float>(awbStatus->gainB) }));
 		libcameraMetadata_.set(controls::ColourTemperature, awbStatus->temperatureK);
 	}

 	BlackLevelStatus *blackLevelStatus = rpiMetadata_.getLocked<BlackLevelStatus>("black_level.status");
 	if (blackLevelStatus)
 		libcameraMetadata_.set(controls::SensorBlackLevels,
-				       { static_cast<int32_t>(blackLevelStatus->blackLevelR),
-					 static_cast<int32_t>(blackLevelStatus->blackLevelG),
-					 static_cast<int32_t>(blackLevelStatus->blackLevelG),
-					 static_cast<int32_t>(blackLevelStatus->blackLevelB) });
+				       Span<const int32_t, 4>({ blackLevelStatus->blackLevelR,
+								blackLevelStatus->blackLevelG,
+								blackLevelStatus->blackLevelG,
+								blackLevelStatus->blackLevelB }));

 	FocusStatus *focusStatus = rpiMetadata_.getLocked<FocusStatus>("focus.status");
 	if (focusStatus && focusStatus->num == 12) {
@@ -883,7 +884,7 @@  void IPARPi::queueRequest(const ControlList &controls)
 			if (gains[0] != 0.0f && gains[1] != 0.0f)
 				/* A gain of 0.0f will switch back to auto mode. */
 				libcameraMetadata_.set(controls::ColourGains,
-						       { gains[0], gains[1] });
+						       Span<const float, 2>({ gains[0], gains[1] }));
 			break;
 		}

@@ -1167,8 +1168,8 @@  void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur

 	/* Return the validated limits via metadata. */
 	libcameraMetadata_.set(controls::FrameDurationLimits,
-			       { static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
-				 static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) });
+			       Span<const int64_t, 2>({ static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
+							static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) }));

 	/*
 	 * Calculate the maximum exposure time possible for the AGC to use.
diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
index ecab3ae9..5fa168c6 100644
--- a/src/libcamera/control_ids.yaml
+++ b/src/libcamera/control_ids.yaml
@@ -291,7 +291,7 @@  controls:
         transformation. The 3x3 matrix is stored in conventional reading
         order in an array of 9 floating point values.

-      size: [3x3]
+      size: [3,3]

   - ScalerCrop:
       type: Rectangle
@@ -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: [n]
+      size: []

   - AfTrigger:
       type: int32_t
diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
index 11b7ebdc..a87485d7 100644
--- a/src/libcamera/property_ids.yaml
+++ b/src/libcamera/property_ids.yaml
@@ -497,7 +497,7 @@  controls:

   - PixelArrayOpticalBlackRectangles:
       type: Rectangle
-      size: [n]
+      size: []
       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: [n]
+      size: []
       description: |
         The PixelArrayActiveAreas property defines the (possibly multiple and
         overlapping) portions of the camera sensor readable pixel matrix
diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
index 5b94b3f2..b4362537 100644
--- a/src/qcam/dng_writer.cpp
+++ b/src/qcam/dng_writer.cpp
@@ -517,7 +517,7 @@  int DNGWriter::write(const char *filename, const Camera *camera,

 	const auto &blackLevels = metadata.get(controls::SensorBlackLevels);
 	if (blackLevels) {
-		Span<const int32_t> levels = *blackLevels;
+		Span<const int32_t, 4> levels = *blackLevels;

 		/*
 		 * The black levels control is specified in R, Gr, Gb, B order.
diff --git a/utils/gen-controls.py b/utils/gen-controls.py
index 3f99b5e2..46ba4394 100755
--- a/utils/gen-controls.py
+++ b/utils/gen-controls.py
@@ -7,6 +7,8 @@ 
 # gen-controls.py - Generate control definitions from YAML

 import argparse
+from functools import reduce
+import operator
 import string
 import sys
 import yaml
@@ -22,6 +24,24 @@  def format_description(description):
     return '\n'.join([(line and ' * ' or ' *') + line for line in description])


+def get_ctrl_type(ctrl):
+    ctrl_type = ctrl['type']
+    ctrl_size_arr = ctrl.get('size')
+
+    if ctrl_type == 'string':
+        return 'std::string'
+    elif ctrl_size_arr is not None:
+        if len(ctrl_size_arr) > 0:
+            # fixed-sized Span
+            ctrl_span_size = reduce(operator.mul, ctrl_size_arr)
+            return f"Span<const {ctrl_type}, {ctrl_span_size}>"
+        else:
+            # variable-sized Span
+            return f"Span<const {ctrl_type}>"
+    else:
+        return ctrl_type
+
+
 def generate_cpp(controls):
     enum_doc_start_template = string.Template('''/**
  * \\enum ${name}Enum
@@ -50,11 +70,7 @@  ${description}
         name, ctrl = ctrl.popitem()
         id_name = snake_case(name).upper()

-        ctrl_type = ctrl['type']
-        if ctrl_type == 'string':
-            ctrl_type = 'std::string'
-        elif ctrl.get('size'):
-            ctrl_type = 'Span<const %s>' % ctrl_type
+        ctrl_type = get_ctrl_type(ctrl)

         info = {
             'name': name,
@@ -135,11 +151,7 @@  def generate_h(controls):

         ids.append('\t' + id_name + ' = ' + str(id_value) + ',')

-        ctrl_type = ctrl['type']
-        if ctrl_type == 'string':
-            ctrl_type = 'std::string'
-        elif ctrl.get('size'):
-            ctrl_type = 'Span<const %s>' % ctrl_type
+        ctrl_type = get_ctrl_type(ctrl)

         info = {
             'name': name,