[libcamera-devel] [PATCH] cam: convert to libfmt

[Tomi Valkeinen]

On 10/05/2022 16:13, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Tue, May 10, 2022 at 12:10:21PM +0300, Tomi Valkeinen wrote:
>> On 10/05/2022 10:16, Tomi Valkeinen wrote:
>>> This is just a conversation starter, not for merging. I really like
>>> libfmt, and I really hate the C++ stream-based string formatting. libfmt
>>> is the first library I add to any new C++ project I make.
>>>
>>> You can find more information about libfmt from:
>>>
>>> https://github.com/fmtlib/fmt
>>> https://fmt.dev/latest/index.html
>>>
>>> This patch is just a crude conversion with ugly macros to showcase what
>>> the formatting code might look like.
>>
>> As for the PR and EPR macros, those was just something to get forward
>> with. I think fmt::print("") is fine, but fmt::print(stderr, "") is a
>> bit long.
>>
>> Compared to cout, "fmt::print(" is actually shorter than "std::cout <<
>> ", and without all those << and std::endl the lines are shorter.
>>
>> Not so with cerr and fmt::print(stderr, (although in many cases the
>> total length would still be shorter) but I think it makes sense to have
>> a macro/inline func for error prints, if only to make it more obvious
>> that it's an error print.
>>
>> And, of course, libfmt could also be used for logging:
>>
>> LOG(Camera, Error) << "Camera in " << camera_state_names[currentState]
>> 		   << " state trying " << from << "() requiring state "
>> 		   << camera_state_names[state];
>>
>> to
>>
>> LOG(Camera, Error, "Camera in {} state trying {}() requiring state {}",
>>       camera_state_names[currentState], from, camera_state_names[state]);
> 
> That's an interesting example, it clearly shows one of the main semantic
> differences between the two options. std::format makes it easier to see
> the whole message as it's stored in a single string, but the drawback is
> that you have to go back and forth between the format string and the
> arguments to figure out what is getting printed. std::ostream makes the
> reading flow more natural in my opinion.

I think the bigger difference comes apparent when using more complex 
formats than just plain {} (i.e. setting precision, width, alignment, etc).

> I'm pretty sure my preference will vary based on the logging statement,
> some will become more readable, others will get worse. That won't help
> making a decision :-) It would be nice, however, to see how it would

Maybe there's a logging statement out there which is more readable with 
streams, but I'm not aware of it ;).

> affect logging, with a sample patch for the logger. It should ideally
> also add a custom formatter for at least one of the geometry classes.

We already have that in this patch, as camera_session.cpp prints a Size. 
But it uses the existing std::ostream formatter from geometry.cpp, so... 
it's a bit boring =).

libfmt has its own formatter support, which is useful if you want to 
pass formatting parameters to the formatter. Say, {:#x} and {:.2f} could 
be used to print "(0x4, 0x7)" and "(4.00, 7.00)" for the same Size.

  Tomi