[libcamera-devel] Usage of C++17 in the public API (was "[PATCH] cam: convert to libfmt")

[Christian Rauch]

Am 22.05.22 um 11:52 schrieb Laurent Pinchart via libcamera-devel:
> On Sun, May 22, 2022 at 11:08:27AM +0100, Kieran Bingham wrote:
>> Quoting Laurent Pinchart (2022-05-10 14:04:57)
>>> On Tue, May 10, 2022 at 01:59:37PM +0100, Eric Curtin via libcamera-devel wrote:
>>>> On Tue, 10 May 2022 at 11:10, Kieran Bingham via libcamera-devel wrote:
>>>>> Quoting Naushir Patuck (2022-05-10 10:49:14)
>>>>>> On Tue, 10 May 2022 at 08:16, Tomi Valkeinen via libcamera-devel wrote:
>
> [snip]
>
>>>>>> For what it's worth, I absolutely loathe formatting in std iostream, and
>>>>>> libfmt is a wonderful alternative.
>>>>>> It also is close enough to the C++20 std::format implementation that
>>>>>> eventual porting would be low effort.  So I am all for this change :)
>>>
>>> It's going to take a while before we can switch to C++20, but I'm
>>> looking forward to it (and before that to some of the C++17 features
>>> too, it would be nice to use std::optional in the public API).
>>
>>
>> So, I've just seen as Christian said - we already do!
>>
>> It's used by color space ..
>
> Oops...
>
>> So we're faced with either reverting, or rolling back on the color-space
>> change, or pushing forwards with C++17 on the public-api.
>
> Switching to C++17 worries me as it's fairly recent and may not be
> easily available for all users. std::optional could easily rbe replaced
> with a custom implementation in utils::optional if needed. I'm tempted
> to leave std::optional in for now, and accept new users of std::optional
> but no other C++17 APIs. Switching to utils::optional for ColorSpace
> would cover new users of std::optional so we're not creating any new
> liability.

I think that C++17 is quite old by now (nearly 5 years) and all major
compilers support it for multiple versions and years now. Do you know of
an example, where requiring C++17 would hinder the adaptation of
libcamera? Do any of the target system not support this standard?

I think the use of std::optional to express invalid values is quite
significant compared to the previous solution of using default
constructed values. Reimplementing this in libcamera would be a
solution, but it just adds development and maintenance costs for no
obvious reason (to me).

>
>> Maybe this deserves it's own thread...
>
> Done :-)
>
>>>> I am all in for this change also, personally I would have changed to
>>>> printf for now
>>>> to have one less dependency (also an easy port to C++20 std::format). The
>>>> dependency list is getting large, I noticed a build of mine failing
>>>> recently because
>>>> I didn't have libyaml.
>>>
>>> I've posted a patch that falls back to a subproject in that case. Would
>>> you be able to give it a try ?
>>>
>>>> But std::format and libfmt are quite fast and anything is better than
>>>> streams so +1.
>>>>
>>>>> I've never used (yet) {fmt}, but I've only heard good things about
>>>>> performance, and of course it's headed into the standard, so I also
>>>>> think there is some good merit to be found in this development.
>