[PATCH v2 1/2] libcamera: formats: Silence warning when creating a PixelFormatInfo from a null format

[Laurent Pinchart]

On Fri, Feb 14, 2025 at 05:42:16PM +0100, Stefan Klug wrote:
> On Fri, Feb 14, 2025 at 06:27:33PM +0200, Laurent Pinchart wrote:
> > On Fri, Feb 14, 2025 at 05:09:47PM +0100, Stefan Klug wrote:
> > > Calling PixelFormat().toString() correctly returns "0x0-<INVALID>" but it
> > > also produces the following, possibly confusing, warning:
> > > 
> > > WARN Formats formats.cpp:1006 Unsupported pixel format 0x00000000
> > > 
> > > Silence the warning in PixelFormatInfo::info() in case the format is
> > > invalid.
> > 
> > Sleeping over this made me wonder where/when you hit this warning. Is
> > there a valid use case to call this function with a default-constructed
> > pixel format ?
> 
> At least I can explain where it happens. Let's see if that is valid...
> The V4L2M2MStream has a logPrefix() function which calls
> stream_->configuration().toString(). So when you log something early in
> the lifetime of the converter (even if it is a debug message that
> doesn't show up) that warning shows up. I was puzzled by this until I
> realized it is just normal flow. And I see no better way to solve it.

This was introduced in

commit cc3a3c46a5ae4353b7bc9fe740521cef1008c998
Author: Umang Jain <umang.jain at ideasonboard.com>
Date:   Mon Jun 24 19:18:59 2024 +0530

    libcamera: converter: Replace usage of stream index by Stream pointer

with

 std::string V4L2M2MConverter::V4L2M2MStream::logPrefix() const
 {
-        return "stream" + std::to_string(index_);
+        return stream_->configuration().toString();
 }

Usage of the stream configuration there is not nice :-/ We could have
two streams with the same configuration, and wouldn't be able to
differentiate them in the logs. Also, the log identification of a stream
can change over the lifetime of the object. The log prefix is meant to
differentiate between class instances. Can we do something better here ?

> > > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > >  src/libcamera/formats.cpp | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> > > index bfcdfc08960d..b4518e61d04a 100644
> > > --- a/src/libcamera/formats.cpp
> > > +++ b/src/libcamera/formats.cpp
> > > @@ -1001,6 +1001,9 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
> > >   */
> > >  const PixelFormatInfo &PixelFormatInfo::info(const PixelFormat &format)
> > >  {
> > > +	if (!format.isValid())
> > > +		return pixelFormatInfoInvalid;
> > > +
> > >  	const auto iter = pixelFormatInfo.find(format);
> > >  	if (iter == pixelFormatInfo.end()) {
> > >  		LOG(Formats, Warning)

-- 
Regards,

Laurent Pinchart