[libcamera-devel,v3,2/4] generate fixed- and variable-sized Span Controls

15657 diff mbox series

This defines Controls with a 'size' as either variable-sized Span<T> or as
fixed-sized Span<T,N>.

Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
---
 utils/gen-controls.py | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

--
2.25.1

Hi Christian,

Thank you for the patch.

On Fri, Apr 08, 2022 at 02:42:29AM +0100, Christian Rauch via libcamera-devel wrote:
> This defines Controls with a 'size' as either variable-sized Span<T> or as
> fixed-sized Span<T,N>.

As with patch 1/4, 2/4 causes new compilation failures. You will need to
squash 3/4 with this. It won't be enough though, there will be two more
compilation errors that will need to be fixed.

> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
> ---
>  utils/gen-controls.py | 32 ++++++++++++++++++++++----------
>  1 file changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/utils/gen-controls.py b/utils/gen-controls.py
> index 3f99b5e2..c9e79a22 100755
> --- a/utils/gen-controls.py
> +++ b/utils/gen-controls.py
> @@ -7,6 +7,7 @@
>  # gen-controls.py - Generate control definitions from YAML
> 
>  import argparse
> +import math
>  import string
>  import sys
>  import yaml
> @@ -22,6 +23,25 @@ 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')
> +    ctrl_is_span = ctrl_size_arr is not None
> +
> +    if ctrl_type == 'string':
> +        return 'std::string'
> +    elif ctrl_is_span:
> +        ctrl_span_size = math.prod(ctrl_size_arr) if len(ctrl_size_arr) > 0 else None
> +        if ctrl_span_size:
> +            # fixed-sized Span
> +            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,

Hi Laurent,

Thanks for the review.

Apart from the std::optional patch, none of the other patches that deal
with Spans will compile individually. You have to merge the entire patch
set (including the std::optional patch) to compile the fixed-sized Span
feature.

I separated my commits logically by which part of the code base they
change and by which feature they add. The first patch changes the yaml
type definitions, the second patch updates the python script to generate
new code and the third patch updates the C++ code. Semantically, this
makes more sense to me than merging one huge commit that touches
multiple areas. I also think this separation makes the review easier.

Does libcamera require that every individual patch/commit is compilable?

Best,
Christian


Am 19.04.22 um 21:46 schrieb Laurent Pinchart:
> Hi Christian,
>
> Thank you for the patch.
>
> On Fri, Apr 08, 2022 at 02:42:29AM +0100, Christian Rauch via libcamera-devel wrote:
>> This defines Controls with a 'size' as either variable-sized Span<T> or as
>> fixed-sized Span<T,N>.
>
> As with patch 1/4, 2/4 causes new compilation failures. You will need to
> squash 3/4 with this. It won't be enough though, there will be two more
> compilation errors that will need to be fixed.
>
>> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
>> ---
>>  utils/gen-controls.py | 32 ++++++++++++++++++++++----------
>>  1 file changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/utils/gen-controls.py b/utils/gen-controls.py
>> index 3f99b5e2..c9e79a22 100755
>> --- a/utils/gen-controls.py
>> +++ b/utils/gen-controls.py
>> @@ -7,6 +7,7 @@
>>  # gen-controls.py - Generate control definitions from YAML
>>
>>  import argparse
>> +import math
>>  import string
>>  import sys
>>  import yaml
>> @@ -22,6 +23,25 @@ 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')
>> +    ctrl_is_span = ctrl_size_arr is not None
>> +
>> +    if ctrl_type == 'string':
>> +        return 'std::string'
>> +    elif ctrl_is_span:
>> +        ctrl_span_size = math.prod(ctrl_size_arr) if len(ctrl_size_arr) > 0 else None
>> +        if ctrl_span_size:
>> +            # fixed-sized Span
>> +            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,
>