| Message ID | 20250815123138.2213654-3-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás, Thank you for the patch. On Fri, Aug 15, 2025 at 02:31:38PM +0200, Barnabás Pőcze wrote: > Forcing the "non-pod" members to be initialized from `const T&` is not the > ideal solution because it disallows e.g. move constructors. So generate a > templated constructor, which members to be initialized more freely, e.g. > using move constructors. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > changes in v2: > * use `std::is_convertible_v` to avoid accidental use of explicit > constructors/conversion operators > --- > .../libcamera_templates/core_ipa_interface.h.tmpl | 2 ++ > .../libcamera_templates/definition_functions.tmpl | 15 +++++++++++---- > .../module_ipa_interface.h.tmpl | 2 ++ > 3 files changed, 15 insertions(+), 4 deletions(-) > > diff --git a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl > index 3942e5708..93f988cd9 100644 > --- a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl > +++ b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl > @@ -16,6 +16,8 @@ > > {% if has_map %}#include <map>{% endif %} > {% if has_string %}#include <string>{% endif %} > +#include <type_traits> > +#include <utility> > {% if has_array %}#include <vector>{% endif %} > > #include <libcamera/controls.h> > diff --git a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl > index 3a57a3c2c..dec25558f 100644 > --- a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl > +++ b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl > @@ -28,14 +28,21 @@ public: > #ifndef __DOXYGEN__ > {{struct.mojom_name}}() = default; > > + template< > + {%- for field in struct.fields %} > + typename T{{loop.index}} = {{field|name}}, > + {%- endfor %} > + {%- for field in struct.fields %} > + std::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr{{"," if not loop.last}} > + {%- endfor %} > + > Isn't this overkill for the pod members, shouldn't we restrict this to non-pod ? > {{struct.mojom_name}}( > {%- for field in struct.fields -%} > -{{"const " if not field|is_pod}}{{field|name}} {{"&" if not field|is_pod}}_{{field.mojom_name}}{{", " if not loop.last}} > + T{{loop.index}} &&_{{field.mojom_name}}{{ ", " if not loop.last }} > {%- endfor -%} There must be something I'm missing. Looking for instance at the constructor of IPABuffer, we get template< typename T1 = uint32_t, typename T2 = std::vector<FrameBuffer::Plane>, std::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr, std::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr > IPABuffer(T1 &&_id, T2 &&_planes) : id(std::forward<T1>(_id)) , planes(std::forward<T2>(_planes)) { } T1 and T2 are defaulted, and I don't expect to see any code create an IPABuffer with explicit template parameter types. How is that therefore different from IPABuffer(uint32_t &&_id, std::vector<FrameBuffer::Plane> &&_planes) : id(std::forward<uint32_t>(_id)) , planes(std::forward<std::vector<FrameBuffer::Plane>>(_planes)) { } ? There's clearly a difference as the latter doesn't compile. Is the compiler allowed to deduce types even when they have a default value ? The following compiles too: template< typename T1, typename T2, std::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr, std::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr > IPABuffer(T1 &&_id, T2 &&_planes) : id(std::forward<T1>(_id)) , planes(std::forward<T2>(_planes)) { } What's the difference ? > ) > - : > -{%- for field in struct.fields -%} > -{{" " if loop.first}}{{field.mojom_name}}(_{{field.mojom_name}}){{", " if not loop.last}} > +{%- for field in struct.fields %} > + {{": " if loop.first else ", "}}{{field.mojom_name}}(std::forward<T{{loop.index}}>(_{{field.mojom_name}})) > {%- endfor %} > { > } > diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl > index 5d70ea6a2..3913eb1fb 100644 > --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl > +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl > @@ -16,6 +16,8 @@ > > {% if has_map %}#include <map>{% endif %} > {% if has_string %}#include <string>{% endif %} > +#include <type_traits> > +#include <utility> > {% if has_array %}#include <vector>{% endif %} > > #include <libcamera/base/flags.h>
2025. 08. 19. 4:41 keltezéssel, Laurent Pinchart írta: > Hi Barnabás, > > Thank you for the patch. > > On Fri, Aug 15, 2025 at 02:31:38PM +0200, Barnabás Pőcze wrote: >> Forcing the "non-pod" members to be initialized from `const T&` is not the >> ideal solution because it disallows e.g. move constructors. So generate a >> templated constructor, which members to be initialized more freely, e.g. >> using move constructors. >> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> >> --- >> changes in v2: >> * use `std::is_convertible_v` to avoid accidental use of explicit >> constructors/conversion operators >> --- >> .../libcamera_templates/core_ipa_interface.h.tmpl | 2 ++ >> .../libcamera_templates/definition_functions.tmpl | 15 +++++++++++---- >> .../module_ipa_interface.h.tmpl | 2 ++ >> 3 files changed, 15 insertions(+), 4 deletions(-) >> >> diff --git a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl >> index 3942e5708..93f988cd9 100644 >> --- a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl >> +++ b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl >> @@ -16,6 +16,8 @@ >> >> {% if has_map %}#include <map>{% endif %} >> {% if has_string %}#include <string>{% endif %} >> +#include <type_traits> >> +#include <utility> >> {% if has_array %}#include <vector>{% endif %} >> >> #include <libcamera/controls.h> >> diff --git a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl >> index 3a57a3c2c..dec25558f 100644 >> --- a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl >> +++ b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl >> @@ -28,14 +28,21 @@ public: >> #ifndef __DOXYGEN__ >> {{struct.mojom_name}}() = default; >> >> + template< >> + {%- for field in struct.fields %} >> + typename T{{loop.index}} = {{field|name}}, >> + {%- endfor %} >> + {%- for field in struct.fields %} >> + std::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr{{"," if not loop.last}} >> + {%- endfor %} >> + > > > Isn't this overkill for the pod members, shouldn't we restrict this to non-pod ? Possibly, but I thought it simpler to handle everything uniformly. > >> {{struct.mojom_name}}( >> {%- for field in struct.fields -%} >> -{{"const " if not field|is_pod}}{{field|name}} {{"&" if not field|is_pod}}_{{field.mojom_name}}{{", " if not loop.last}} >> + T{{loop.index}} &&_{{field.mojom_name}}{{ ", " if not loop.last }} >> {%- endfor -%} > > There must be something I'm missing. Looking for instance at the > constructor of IPABuffer, we get > > template< > typename T1 = uint32_t, > typename T2 = std::vector<FrameBuffer::Plane>, > std::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr, > std::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr > > > IPABuffer(T1 &&_id, T2 &&_planes) > : id(std::forward<T1>(_id)) > , planes(std::forward<T2>(_planes)) > { > } > > T1 and T2 are defaulted, and I don't expect to see any code create an > IPABuffer with explicit template parameter types. How is that therefore > different from > > IPABuffer(uint32_t &&_id, std::vector<FrameBuffer::Plane> &&_planes) > : id(std::forward<uint32_t>(_id)) > , planes(std::forward<std::vector<FrameBuffer::Plane>>(_planes)) > { > } > > ? There's clearly a difference as the latter doesn't compile. Is the > compiler allowed to deduce types even when they have a default value ? The difference is that in the proposed change, those references are forwarding references[0]. While in the above code, they are just normal rvalue references, so they won't bind, for example, to `const T&`, while the forwarding references will. The default template parameters are there in case the type cannot be deduced from the call, such as when an initializer list or similar is used. [0]: https://en.cppreference.com/w/cpp/language/reference.html#Forwarding_references Regards, Barnabás Pőcze > The following compiles too: > > template< > typename T1, > typename T2, > std::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr, > std::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr > > > IPABuffer(T1 &&_id, T2 &&_planes) > : id(std::forward<T1>(_id)) > , planes(std::forward<T2>(_planes)) > { > } > > What's the difference ? > >> ) >> - : >> -{%- for field in struct.fields -%} >> -{{" " if loop.first}}{{field.mojom_name}}(_{{field.mojom_name}}){{", " if not loop.last}} >> +{%- for field in struct.fields %} >> + {{": " if loop.first else ", "}}{{field.mojom_name}}(std::forward<T{{loop.index}}>(_{{field.mojom_name}})) >> {%- endfor %} >> { >> } >> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl >> index 5d70ea6a2..3913eb1fb 100644 >> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl >> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl >> @@ -16,6 +16,8 @@ >> >> {% if has_map %}#include <map>{% endif %} >> {% if has_string %}#include <string>{% endif %} >> +#include <type_traits> >> +#include <utility> >> {% if has_array %}#include <vector>{% endif %} >> >> #include <libcamera/base/flags.h> >
On Tue, Aug 19, 2025 at 09:25:43AM +0200, Barnabás Pőcze wrote: > 2025. 08. 19. 4:41 keltezéssel, Laurent Pinchart írta: > > On Fri, Aug 15, 2025 at 02:31:38PM +0200, Barnabás Pőcze wrote: > >> Forcing the "non-pod" members to be initialized from `const T&` is not the > >> ideal solution because it disallows e.g. move constructors. So generate a > >> templated constructor, which members to be initialized more freely, e.g. > >> using move constructors. > >> > >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > >> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > >> --- > >> changes in v2: > >> * use `std::is_convertible_v` to avoid accidental use of explicit > >> constructors/conversion operators > >> --- > >> .../libcamera_templates/core_ipa_interface.h.tmpl | 2 ++ > >> .../libcamera_templates/definition_functions.tmpl | 15 +++++++++++---- > >> .../module_ipa_interface.h.tmpl | 2 ++ > >> 3 files changed, 15 insertions(+), 4 deletions(-) > >> > >> diff --git a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl > >> index 3942e5708..93f988cd9 100644 > >> --- a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl > >> +++ b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl > >> @@ -16,6 +16,8 @@ > >> > >> {% if has_map %}#include <map>{% endif %} > >> {% if has_string %}#include <string>{% endif %} > >> +#include <type_traits> > >> +#include <utility> > >> {% if has_array %}#include <vector>{% endif %} > >> > >> #include <libcamera/controls.h> > >> diff --git a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl > >> index 3a57a3c2c..dec25558f 100644 > >> --- a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl > >> +++ b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl > >> @@ -28,14 +28,21 @@ public: > >> #ifndef __DOXYGEN__ > >> {{struct.mojom_name}}() = default; > >> > >> + template< > >> + {%- for field in struct.fields %} > >> + typename T{{loop.index}} = {{field|name}}, > >> + {%- endfor %} > >> + {%- for field in struct.fields %} > >> + std::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr{{"," if not loop.last}} > >> + {%- endfor %} > >> + > > > > > Isn't this overkill for the pod members, shouldn't we restrict this > > to non-pod ? > > Possibly, but I thought it simpler to handle everything uniformly. Any impact on either the complexity of the binary resulting from this, or the compilation time ? If not, I suppose most people won't look at the generated code, so we could ignore this. > >> {{struct.mojom_name}}( > >> {%- for field in struct.fields -%} > >> -{{"const " if not field|is_pod}}{{field|name}} {{"&" if not field|is_pod}}_{{field.mojom_name}}{{", " if not loop.last}} > >> + T{{loop.index}} &&_{{field.mojom_name}}{{ ", " if not loop.last }} > >> {%- endfor -%} > > > > There must be something I'm missing. Looking for instance at the > > constructor of IPABuffer, we get > > > > template< > > typename T1 = uint32_t, > > typename T2 = std::vector<FrameBuffer::Plane>, > > std::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr, > > std::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr > > > > > IPABuffer(T1 &&_id, T2 &&_planes) > > : id(std::forward<T1>(_id)) > > , planes(std::forward<T2>(_planes)) > > { > > } > > > > T1 and T2 are defaulted, and I don't expect to see any code create an > > IPABuffer with explicit template parameter types. How is that therefore > > different from > > > > IPABuffer(uint32_t &&_id, std::vector<FrameBuffer::Plane> &&_planes) > > : id(std::forward<uint32_t>(_id)) > > , planes(std::forward<std::vector<FrameBuffer::Plane>>(_planes)) > > { > > } > > > > ? There's clearly a difference as the latter doesn't compile. Is the > > compiler allowed to deduce types even when they have a default value ? > > The difference is that in the proposed change, those references are forwarding > references[0]. While in the above code, they are just normal rvalue references, > so they won't bind, for example, to `const T&`, while the forwarding references will. Yes, that part I understood. > The default template parameters are there in case the type cannot be deduced from > the call, such as when an initializer list or similar is used. So providing default template parameters is just a hint, compilers are free to deduce a different type ? That's the part I wasn't aware of. > [0]: https://en.cppreference.com/w/cpp/language/reference.html#Forwarding_references > > > The following compiles too: > > > > template< > > typename T1, > > typename T2, > > std::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr, > > std::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr > > > > > IPABuffer(T1 &&_id, T2 &&_planes) > > : id(std::forward<T1>(_id)) > > , planes(std::forward<T2>(_planes)) > > { > > } > > > > What's the difference ? > > > >> ) > >> - : > >> -{%- for field in struct.fields -%} > >> -{{" " if loop.first}}{{field.mojom_name}}(_{{field.mojom_name}}){{", " if not loop.last}} > >> +{%- for field in struct.fields %} > >> + {{": " if loop.first else ", "}}{{field.mojom_name}}(std::forward<T{{loop.index}}>(_{{field.mojom_name}})) > >> {%- endfor %} > >> { > >> } > >> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl > >> index 5d70ea6a2..3913eb1fb 100644 > >> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl > >> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl > >> @@ -16,6 +16,8 @@ > >> > >> {% if has_map %}#include <map>{% endif %} > >> {% if has_string %}#include <string>{% endif %} > >> +#include <type_traits> > >> +#include <utility> > >> {% if has_array %}#include <vector>{% endif %} > >> > >> #include <libcamera/base/flags.h>
2025. 08. 19. 10:35 keltezéssel, Laurent Pinchart írta: > On Tue, Aug 19, 2025 at 09:25:43AM +0200, Barnabás Pőcze wrote: >> 2025. 08. 19. 4:41 keltezéssel, Laurent Pinchart írta: >>> On Fri, Aug 15, 2025 at 02:31:38PM +0200, Barnabás Pőcze wrote: >>>> Forcing the "non-pod" members to be initialized from `const T&` is not the >>>> ideal solution because it disallows e.g. move constructors. So generate a >>>> templated constructor, which members to be initialized more freely, e.g. >>>> using move constructors. >>>> >>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>>> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> >>>> --- >>>> changes in v2: >>>> * use `std::is_convertible_v` to avoid accidental use of explicit >>>> constructors/conversion operators >>>> --- >>>> .../libcamera_templates/core_ipa_interface.h.tmpl | 2 ++ >>>> .../libcamera_templates/definition_functions.tmpl | 15 +++++++++++---- >>>> .../module_ipa_interface.h.tmpl | 2 ++ >>>> 3 files changed, 15 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl >>>> index 3942e5708..93f988cd9 100644 >>>> --- a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl >>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl >>>> @@ -16,6 +16,8 @@ >>>> >>>> {% if has_map %}#include <map>{% endif %} >>>> {% if has_string %}#include <string>{% endif %} >>>> +#include <type_traits> >>>> +#include <utility> >>>> {% if has_array %}#include <vector>{% endif %} >>>> >>>> #include <libcamera/controls.h> >>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl >>>> index 3a57a3c2c..dec25558f 100644 >>>> --- a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl >>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl >>>> @@ -28,14 +28,21 @@ public: >>>> #ifndef __DOXYGEN__ >>>> {{struct.mojom_name}}() = default; >>>> >>>> + template< >>>> + {%- for field in struct.fields %} >>>> + typename T{{loop.index}} = {{field|name}}, >>>> + {%- endfor %} >>>> + {%- for field in struct.fields %} >>>> + std::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr{{"," if not loop.last}} >>>> + {%- endfor %} >>>> + > >>> >>> Isn't this overkill for the pod members, shouldn't we restrict this >>> to non-pod ? >> >> Possibly, but I thought it simpler to handle everything uniformly. > > Any impact on either the complexity of the binary resulting from this, > or the compilation time ? If not, I suppose most people won't look at > the generated code, so we could ignore this. > >>>> {{struct.mojom_name}}( >>>> {%- for field in struct.fields -%} >>>> -{{"const " if not field|is_pod}}{{field|name}} {{"&" if not field|is_pod}}_{{field.mojom_name}}{{", " if not loop.last}} >>>> + T{{loop.index}} &&_{{field.mojom_name}}{{ ", " if not loop.last }} >>>> {%- endfor -%} >>> >>> There must be something I'm missing. Looking for instance at the >>> constructor of IPABuffer, we get >>> >>> template< >>> typename T1 = uint32_t, >>> typename T2 = std::vector<FrameBuffer::Plane>, >>> std::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr, >>> std::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr >>> > >>> IPABuffer(T1 &&_id, T2 &&_planes) >>> : id(std::forward<T1>(_id)) >>> , planes(std::forward<T2>(_planes)) >>> { >>> } >>> >>> T1 and T2 are defaulted, and I don't expect to see any code create an >>> IPABuffer with explicit template parameter types. How is that therefore >>> different from >>> >>> IPABuffer(uint32_t &&_id, std::vector<FrameBuffer::Plane> &&_planes) >>> : id(std::forward<uint32_t>(_id)) >>> , planes(std::forward<std::vector<FrameBuffer::Plane>>(_planes)) >>> { >>> } >>> >>> ? There's clearly a difference as the latter doesn't compile. Is the >>> compiler allowed to deduce types even when they have a default value ? >> >> The difference is that in the proposed change, those references are forwarding >> references[0]. While in the above code, they are just normal rvalue references, >> so they won't bind, for example, to `const T&`, while the forwarding references will. > > Yes, that part I understood. > >> The default template parameters are there in case the type cannot be deduced from >> the call, such as when an initializer list or similar is used. > > So providing default template parameters is just a hint, compilers are > free to deduce a different type ? That's the part I wasn't aware of. Sorry, yes, that's correct. > >> [0]: https://en.cppreference.com/w/cpp/language/reference.html#Forwarding_references >> >>> The following compiles too: >>> >>> template< >>> typename T1, >>> typename T2, >>> std::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr, >>> std::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr >>> > >>> IPABuffer(T1 &&_id, T2 &&_planes) >>> : id(std::forward<T1>(_id)) >>> , planes(std::forward<T2>(_planes)) >>> { >>> } >>> >>> What's the difference ? >>> >>>> ) >>>> - : >>>> -{%- for field in struct.fields -%} >>>> -{{" " if loop.first}}{{field.mojom_name}}(_{{field.mojom_name}}){{", " if not loop.last}} >>>> +{%- for field in struct.fields %} >>>> + {{": " if loop.first else ", "}}{{field.mojom_name}}(std::forward<T{{loop.index}}>(_{{field.mojom_name}})) >>>> {%- endfor %} >>>> { >>>> } >>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl >>>> index 5d70ea6a2..3913eb1fb 100644 >>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl >>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl >>>> @@ -16,6 +16,8 @@ >>>> >>>> {% if has_map %}#include <map>{% endif %} >>>> {% if has_string %}#include <string>{% endif %} >>>> +#include <type_traits> >>>> +#include <utility> >>>> {% if has_array %}#include <vector>{% endif %} >>>> >>>> #include <libcamera/base/flags.h> >
On Tue, Aug 19, 2025 at 10:43:38AM +0200, Barnabás Pőcze wrote: > 2025. 08. 19. 10:35 keltezéssel, Laurent Pinchart írta: > > On Tue, Aug 19, 2025 at 09:25:43AM +0200, Barnabás Pőcze wrote: > >> 2025. 08. 19. 4:41 keltezéssel, Laurent Pinchart írta: > >>> On Fri, Aug 15, 2025 at 02:31:38PM +0200, Barnabás Pőcze wrote: > >>>> Forcing the "non-pod" members to be initialized from `const T&` is not the > >>>> ideal solution because it disallows e.g. move constructors. So generate a > >>>> templated constructor, which members to be initialized more freely, e.g. > >>>> using move constructors. > >>>> > >>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > >>>> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > >>>> --- > >>>> changes in v2: > >>>> * use `std::is_convertible_v` to avoid accidental use of explicit > >>>> constructors/conversion operators > >>>> --- > >>>> .../libcamera_templates/core_ipa_interface.h.tmpl | 2 ++ > >>>> .../libcamera_templates/definition_functions.tmpl | 15 +++++++++++---- > >>>> .../module_ipa_interface.h.tmpl | 2 ++ > >>>> 3 files changed, 15 insertions(+), 4 deletions(-) > >>>> > >>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl > >>>> index 3942e5708..93f988cd9 100644 > >>>> --- a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl > >>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl > >>>> @@ -16,6 +16,8 @@ > >>>> > >>>> {% if has_map %}#include <map>{% endif %} > >>>> {% if has_string %}#include <string>{% endif %} > >>>> +#include <type_traits> > >>>> +#include <utility> > >>>> {% if has_array %}#include <vector>{% endif %} > >>>> > >>>> #include <libcamera/controls.h> > >>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl > >>>> index 3a57a3c2c..dec25558f 100644 > >>>> --- a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl > >>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl > >>>> @@ -28,14 +28,21 @@ public: > >>>> #ifndef __DOXYGEN__ > >>>> {{struct.mojom_name}}() = default; > >>>> > >>>> + template< > >>>> + {%- for field in struct.fields %} > >>>> + typename T{{loop.index}} = {{field|name}}, > >>>> + {%- endfor %} > >>>> + {%- for field in struct.fields %} > >>>> + std::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr{{"," if not loop.last}} > >>>> + {%- endfor %} > >>>> + > > >>> > >>> Isn't this overkill for the pod members, shouldn't we restrict this > >>> to non-pod ? > >> > >> Possibly, but I thought it simpler to handle everything uniformly. > > > > Any impact on either the complexity of the binary resulting from this, > > or the compilation time ? If not, I suppose most people won't look at > > the generated code, so we could ignore this. I gave it a try. When using rvalue references and std::forward for non-POD arguments only compared to your version, I get a reduction of 54 bytes in .text and an increase of 32 bytes in .rodata for libcamera.so. The impact on the binary isn't significant. Either way, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> If anyone is interested, here's how I implemented the change: diff --git a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl index fd9a61df100f..22e0af570485 100644 --- a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl +++ b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl @@ -29,20 +29,34 @@ public: {{struct.mojom_name}}() = default; template< - {%- for field in struct.fields %} +{%- for field in struct.fields -%} + {%- if not field|is_pod %} typename T{{loop.index}} = {{field|name}}, - {%- endfor %} - {%- for field in struct.fields %} - std::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr{{"," if not loop.last}} - {%- endfor %} + {%- endif -%} +{%- endfor %} +{%- for field in struct.fields %} + {%- if not field|is_pod %} + std::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr, + {%- endif -%} +{%- endfor %} + bool B = true > {{struct.mojom_name}}( {%- for field in struct.fields -%} - T{{loop.index}} &&_{{field.mojom_name}}{{ ", " if not loop.last }} + {%- if field|is_pod -%} + {{field|name}} _{{field.mojom_name}} + {%- else -%} + T{{loop.index}} &&_{{field.mojom_name}} + {%- endif -%} + {{", " if not loop.last}} {%- endfor -%} ) -{%- for field in struct.fields %} +{%- for field in struct.fields -%} + {%- if field|is_pod %} + {{": " if loop.first else ", "}}{{field.mojom_name}}(_{{field.mojom_name}}) + {%- else %} {{": " if loop.first else ", "}}{{field.mojom_name}}(std::forward<T{{loop.index}}>(_{{field.mojom_name}})) + {%- endif -%} {%- endfor %} { } > >>>> {{struct.mojom_name}}( > >>>> {%- for field in struct.fields -%} > >>>> -{{"const " if not field|is_pod}}{{field|name}} {{"&" if not field|is_pod}}_{{field.mojom_name}}{{", " if not loop.last}} > >>>> + T{{loop.index}} &&_{{field.mojom_name}}{{ ", " if not loop.last }} > >>>> {%- endfor -%} > >>> > >>> There must be something I'm missing. Looking for instance at the > >>> constructor of IPABuffer, we get > >>> > >>> template< > >>> typename T1 = uint32_t, > >>> typename T2 = std::vector<FrameBuffer::Plane>, > >>> std::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr, > >>> std::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr > >>> > > >>> IPABuffer(T1 &&_id, T2 &&_planes) > >>> : id(std::forward<T1>(_id)) > >>> , planes(std::forward<T2>(_planes)) > >>> { > >>> } > >>> > >>> T1 and T2 are defaulted, and I don't expect to see any code create an > >>> IPABuffer with explicit template parameter types. How is that therefore > >>> different from > >>> > >>> IPABuffer(uint32_t &&_id, std::vector<FrameBuffer::Plane> &&_planes) > >>> : id(std::forward<uint32_t>(_id)) > >>> , planes(std::forward<std::vector<FrameBuffer::Plane>>(_planes)) > >>> { > >>> } > >>> > >>> ? There's clearly a difference as the latter doesn't compile. Is the > >>> compiler allowed to deduce types even when they have a default value ? > >> > >> The difference is that in the proposed change, those references are forwarding > >> references[0]. While in the above code, they are just normal rvalue references, > >> so they won't bind, for example, to `const T&`, while the forwarding references will. > > > > Yes, that part I understood. > > > >> The default template parameters are there in case the type cannot be deduced from > >> the call, such as when an initializer list or similar is used. > > > > So providing default template parameters is just a hint, compilers are > > free to deduce a different type ? That's the part I wasn't aware of. > > Sorry, yes, that's correct. > > >> [0]: https://en.cppreference.com/w/cpp/language/reference.html#Forwarding_references > >> > >>> The following compiles too: > >>> > >>> template< > >>> typename T1, > >>> typename T2, > >>> std::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr, > >>> std::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr > >>> > > >>> IPABuffer(T1 &&_id, T2 &&_planes) > >>> : id(std::forward<T1>(_id)) > >>> , planes(std::forward<T2>(_planes)) > >>> { > >>> } > >>> > >>> What's the difference ? > >>> > >>>> ) > >>>> - : > >>>> -{%- for field in struct.fields -%} > >>>> -{{" " if loop.first}}{{field.mojom_name}}(_{{field.mojom_name}}){{", " if not loop.last}} > >>>> +{%- for field in struct.fields %} > >>>> + {{": " if loop.first else ", "}}{{field.mojom_name}}(std::forward<T{{loop.index}}>(_{{field.mojom_name}})) > >>>> {%- endfor %} > >>>> { > >>>> } > >>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl > >>>> index 5d70ea6a2..3913eb1fb 100644 > >>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl > >>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl > >>>> @@ -16,6 +16,8 @@ > >>>> > >>>> {% if has_map %}#include <map>{% endif %} > >>>> {% if has_string %}#include <string>{% endif %} > >>>> +#include <type_traits> > >>>> +#include <utility> > >>>> {% if has_array %}#include <vector>{% endif %} > >>>> > >>>> #include <libcamera/base/flags.h>
2025. 09. 01. 15:55 keltezéssel, Laurent Pinchart írta: > On Tue, Aug 19, 2025 at 10:43:38AM +0200, Barnabás Pőcze wrote: >> 2025. 08. 19. 10:35 keltezéssel, Laurent Pinchart írta: >>> On Tue, Aug 19, 2025 at 09:25:43AM +0200, Barnabás Pőcze wrote: >>>> 2025. 08. 19. 4:41 keltezéssel, Laurent Pinchart írta: >>>>> On Fri, Aug 15, 2025 at 02:31:38PM +0200, Barnabás Pőcze wrote: >>>>>> Forcing the "non-pod" members to be initialized from `const T&` is not the >>>>>> ideal solution because it disallows e.g. move constructors. So generate a >>>>>> templated constructor, which members to be initialized more freely, e.g. >>>>>> using move constructors. >>>>>> >>>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>>>>> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> >>>>>> --- >>>>>> changes in v2: >>>>>> * use `std::is_convertible_v` to avoid accidental use of explicit >>>>>> constructors/conversion operators >>>>>> --- >>>>>> .../libcamera_templates/core_ipa_interface.h.tmpl | 2 ++ >>>>>> .../libcamera_templates/definition_functions.tmpl | 15 +++++++++++---- >>>>>> .../module_ipa_interface.h.tmpl | 2 ++ >>>>>> 3 files changed, 15 insertions(+), 4 deletions(-) >>>>>> >>>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl >>>>>> index 3942e5708..93f988cd9 100644 >>>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl >>>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl >>>>>> @@ -16,6 +16,8 @@ >>>>>> >>>>>> {% if has_map %}#include <map>{% endif %} >>>>>> {% if has_string %}#include <string>{% endif %} >>>>>> +#include <type_traits> >>>>>> +#include <utility> >>>>>> {% if has_array %}#include <vector>{% endif %} >>>>>> >>>>>> #include <libcamera/controls.h> >>>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl >>>>>> index 3a57a3c2c..dec25558f 100644 >>>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl >>>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl >>>>>> @@ -28,14 +28,21 @@ public: >>>>>> #ifndef __DOXYGEN__ >>>>>> {{struct.mojom_name}}() = default; >>>>>> >>>>>> + template< >>>>>> + {%- for field in struct.fields %} >>>>>> + typename T{{loop.index}} = {{field|name}}, >>>>>> + {%- endfor %} >>>>>> + {%- for field in struct.fields %} >>>>>> + std::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr{{"," if not loop.last}} >>>>>> + {%- endfor %} >>>>>> + > >>>>> >>>>> Isn't this overkill for the pod members, shouldn't we restrict this >>>>> to non-pod ? >>>> >>>> Possibly, but I thought it simpler to handle everything uniformly. >>> >>> Any impact on either the complexity of the binary resulting from this, >>> or the compilation time ? If not, I suppose most people won't look at >>> the generated code, so we could ignore this. > > I gave it a try. When using rvalue references and std::forward for > non-POD arguments only compared to your version, I get a reduction of 54 > bytes in .text and an increase of 32 bytes in .rodata for libcamera.so. > The impact on the binary isn't significant. > > Either way, > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> In that case I think I will go ahead with the version using perfect forwarding unconditionally as that is shorter. Regards, Barnabás Pőcze > > If anyone is interested, here's how I implemented the change: > > diff --git a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl > index fd9a61df100f..22e0af570485 100644 > --- a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl > +++ b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl > @@ -29,20 +29,34 @@ public: > {{struct.mojom_name}}() = default; > > template< > - {%- for field in struct.fields %} > +{%- for field in struct.fields -%} > + {%- if not field|is_pod %} > typename T{{loop.index}} = {{field|name}}, > - {%- endfor %} > - {%- for field in struct.fields %} > - std::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr{{"," if not loop.last}} > - {%- endfor %} > + {%- endif -%} > +{%- endfor %} > +{%- for field in struct.fields %} > + {%- if not field|is_pod %} > + std::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr, > + {%- endif -%} > +{%- endfor %} > + bool B = true > > > {{struct.mojom_name}}( > {%- for field in struct.fields -%} > - T{{loop.index}} &&_{{field.mojom_name}}{{ ", " if not loop.last }} > + {%- if field|is_pod -%} > + {{field|name}} _{{field.mojom_name}} > + {%- else -%} > + T{{loop.index}} &&_{{field.mojom_name}} > + {%- endif -%} > + {{", " if not loop.last}} > {%- endfor -%} > ) > -{%- for field in struct.fields %} > +{%- for field in struct.fields -%} > + {%- if field|is_pod %} > + {{": " if loop.first else ", "}}{{field.mojom_name}}(_{{field.mojom_name}}) > + {%- else %} > {{": " if loop.first else ", "}}{{field.mojom_name}}(std::forward<T{{loop.index}}>(_{{field.mojom_name}})) > + {%- endif -%} > {%- endfor %} > { > } > >>>>>> {{struct.mojom_name}}( >>>>>> {%- for field in struct.fields -%} >>>>>> -{{"const " if not field|is_pod}}{{field|name}} {{"&" if not field|is_pod}}_{{field.mojom_name}}{{", " if not loop.last}} >>>>>> + T{{loop.index}} &&_{{field.mojom_name}}{{ ", " if not loop.last }} >>>>>> {%- endfor -%} >>>>> >>>>> There must be something I'm missing. Looking for instance at the >>>>> constructor of IPABuffer, we get >>>>> >>>>> template< >>>>> typename T1 = uint32_t, >>>>> typename T2 = std::vector<FrameBuffer::Plane>, >>>>> std::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr, >>>>> std::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr >>>>> > >>>>> IPABuffer(T1 &&_id, T2 &&_planes) >>>>> : id(std::forward<T1>(_id)) >>>>> , planes(std::forward<T2>(_planes)) >>>>> { >>>>> } >>>>> >>>>> T1 and T2 are defaulted, and I don't expect to see any code create an >>>>> IPABuffer with explicit template parameter types. How is that therefore >>>>> different from >>>>> >>>>> IPABuffer(uint32_t &&_id, std::vector<FrameBuffer::Plane> &&_planes) >>>>> : id(std::forward<uint32_t>(_id)) >>>>> , planes(std::forward<std::vector<FrameBuffer::Plane>>(_planes)) >>>>> { >>>>> } >>>>> >>>>> ? There's clearly a difference as the latter doesn't compile. Is the >>>>> compiler allowed to deduce types even when they have a default value ? >>>> >>>> The difference is that in the proposed change, those references are forwarding >>>> references[0]. While in the above code, they are just normal rvalue references, >>>> so they won't bind, for example, to `const T&`, while the forwarding references will. >>> >>> Yes, that part I understood. >>> >>>> The default template parameters are there in case the type cannot be deduced from >>>> the call, such as when an initializer list or similar is used. >>> >>> So providing default template parameters is just a hint, compilers are >>> free to deduce a different type ? That's the part I wasn't aware of. >> >> Sorry, yes, that's correct. >> >>>> [0]: https://en.cppreference.com/w/cpp/language/reference.html#Forwarding_references >>>> >>>>> The following compiles too: >>>>> >>>>> template< >>>>> typename T1, >>>>> typename T2, >>>>> std::enable_if_t<std::is_convertible_v<T1&&, uint32_t>> * = nullptr, >>>>> std::enable_if_t<std::is_convertible_v<T2&&, std::vector<FrameBuffer::Plane>>> * = nullptr >>>>> > >>>>> IPABuffer(T1 &&_id, T2 &&_planes) >>>>> : id(std::forward<T1>(_id)) >>>>> , planes(std::forward<T2>(_planes)) >>>>> { >>>>> } >>>>> >>>>> What's the difference ? >>>>> >>>>>> ) >>>>>> - : >>>>>> -{%- for field in struct.fields -%} >>>>>> -{{" " if loop.first}}{{field.mojom_name}}(_{{field.mojom_name}}){{", " if not loop.last}} >>>>>> +{%- for field in struct.fields %} >>>>>> + {{": " if loop.first else ", "}}{{field.mojom_name}}(std::forward<T{{loop.index}}>(_{{field.mojom_name}})) >>>>>> {%- endfor %} >>>>>> { >>>>>> } >>>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl >>>>>> index 5d70ea6a2..3913eb1fb 100644 >>>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl >>>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl >>>>>> @@ -16,6 +16,8 @@ >>>>>> >>>>>> {% if has_map %}#include <map>{% endif %} >>>>>> {% if has_string %}#include <string>{% endif %} >>>>>> +#include <type_traits> >>>>>> +#include <utility> >>>>>> {% if has_array %}#include <vector>{% endif %} >>>>>> >>>>>> #include <libcamera/base/flags.h> >
diff --git a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl index 3942e5708..93f988cd9 100644 --- a/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl +++ b/utils/codegen/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl @@ -16,6 +16,8 @@ {% if has_map %}#include <map>{% endif %} {% if has_string %}#include <string>{% endif %} +#include <type_traits> +#include <utility> {% if has_array %}#include <vector>{% endif %} #include <libcamera/controls.h> diff --git a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl index 3a57a3c2c..dec25558f 100644 --- a/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl +++ b/utils/codegen/ipc/generators/libcamera_templates/definition_functions.tmpl @@ -28,14 +28,21 @@ public: #ifndef __DOXYGEN__ {{struct.mojom_name}}() = default; + template< + {%- for field in struct.fields %} + typename T{{loop.index}} = {{field|name}}, + {%- endfor %} + {%- for field in struct.fields %} + std::enable_if_t<std::is_convertible_v<T{{loop.index}}&&, {{field|name}}>> * = nullptr{{"," if not loop.last}} + {%- endfor %} + > {{struct.mojom_name}}( {%- for field in struct.fields -%} -{{"const " if not field|is_pod}}{{field|name}} {{"&" if not field|is_pod}}_{{field.mojom_name}}{{", " if not loop.last}} + T{{loop.index}} &&_{{field.mojom_name}}{{ ", " if not loop.last }} {%- endfor -%} ) - : -{%- for field in struct.fields -%} -{{" " if loop.first}}{{field.mojom_name}}(_{{field.mojom_name}}){{", " if not loop.last}} +{%- for field in struct.fields %} + {{": " if loop.first else ", "}}{{field.mojom_name}}(std::forward<T{{loop.index}}>(_{{field.mojom_name}})) {%- endfor %} { } diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl index 5d70ea6a2..3913eb1fb 100644 --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl @@ -16,6 +16,8 @@ {% if has_map %}#include <map>{% endif %} {% if has_string %}#include <string>{% endif %} +#include <type_traits> +#include <utility> {% if has_array %}#include <vector>{% endif %} #include <libcamera/base/flags.h>