[PATCH] libcamera: formats: Silence warning when creating a PixelFormatInfo from a null format

[Stefan Klug]

Hi Kieran,

Thank you for the review. 

On Wed, Feb 12, 2025 at 11:50:19AM +0000, Kieran Bingham wrote:
> Quoting Stefan Klug (2025-02-12 11:29:42)
> > 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.
> > 
> > While at it, remove code duplication by using
> > PixelFormatInfo::info(const PixelFormat &format) to implement
> > PixelFormatInfo::info(const V4L2PixelFormat &format).
> 
> Just to check the obvious - if there is a V4L2 format on a device that
> we don't have mapped/supported in libcamera, will it still print 
> 
> WARN Formats formats.cpp:1006 Unsupported pixel format 0x23456789 ?
> 
> As long as that's the case, 

Well, we got that warning only for formats passed as PixelFormat. If the
format was passed as V4L2PixelFormat there was no warning at all. Now we
get the warning in both cases.

> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> > 
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > ---
> > 
> > Hi all,
> > 
> > The attached patch has one noteworthy side effect: If
> > info(V4L2PixelFormat &format) is called with a valid but unsupported (by
> > libcamera) format, we now get the same warning as in the
> > info(PixelFormat &format) case. I believe that is sensible but maybe the
> > warning was left off in the V4L2 case on purpose.
> > 
> > Best regards,
> > Stefan
> > 
> >  src/libcamera/formats.cpp | 12 ++++--------
> >  1 file changed, 4 insertions(+), 8 deletions(-)
> > 
> > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> > index bfcdfc08960d..df7413f58ba8 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)
> 
> Does the !isValid() check mean it's impossible to get here now? Or is
> this check still necessary?

isValid() only checks if the fourcc code is 0. So the warning shows up
if the format is valid but not supported by libcamera. Which was imho
the intended case.

Best regards,
Stefan

> 
> 
> > @@ -1021,14 +1024,7 @@ const PixelFormatInfo &PixelFormatInfo::info(const PixelFormat &format)
> >  const PixelFormatInfo &PixelFormatInfo::info(const V4L2PixelFormat &format)
> >  {
> >         PixelFormat pixelFormat = format.toPixelFormat(false);
> > -       if (!pixelFormat.isValid())
> > -               return pixelFormatInfoInvalid;
> > -
> > -       const auto iter = pixelFormatInfo.find(pixelFormat);
> > -       if (iter == pixelFormatInfo.end())
> > -               return pixelFormatInfoInvalid;
> > -
> > -       return iter->second;
> > +       return info(pixelFormat);
> > 
> >  }
> >  
> >  /**
> > -- 
> > 2.43.0
> >