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

[Laurent Pinchart]

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'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
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.

This being said, we don't have to switch to libfmt (assuming we want to
do so) everywhere, or everywhere at the same time. The cam application
and the unit tests could possibly go first, as they make heavy use of
std::cout and std::cerr (for the unit tests it would actually be nice to
have a better form of status reporting, std::cerr may be best replaced
with assertions that raise exceptions).

-- 
Regards,

Laurent Pinchart