[libcamera-devel] [PATCH v2] libcamera: formats: Fix warning for unkown V4L2 pixfmt

[Naushir Patuck]

Hi Jacopo,

Thanks for fixing this!

On Tue, 9 Aug 2022 at 15:49, Jacopo Mondi via libcamera-devel <
libcamera-devel at lists.libcamera.org> wrote:

> Commit f25ad4a2b16b ("libcamera: formats: Reimplement V4L2
> PixelFormatInfo::info()") changed the PixelFormatInfo::info(const
> V4L2PixelFormat &format) function overload to:
>
>         return info(format.toPixelFormat());
>
> As part of the series that contains such commit, the PixelFormatInfo for
> the
> pixel format applied to a video device is now retrieved at
> V4L2VideoDevice::open() time. Some video devices register formats not
> available to applications, for example metadata formats or, in the case
> of ISP devices, formats to describe the ISP statistics and parameters.
>
> This causes the
>
>         format.toPixelFormat()
>
> call to output a WARN message, which spams the log and unnecessary alert
> the users.
>
> Augment V4L2PixelFormat::toPixelFormat() with an optional argument to
> suppress warnings in the case a V4L2 pixel format is not known, to
> restore the behaviour preceding commit f25ad4a2b16b and returns an
> invalid PixelFormatInfo without outputting any unnecessary warning
> message.
>
> Fixes: f25ad4a2b16b ("libcamera: formats: Reimplement V4L2
> PixelFormatInfo::info()")
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
>

Tested-by: Naushir Patuck <naush at raspberrypi.com>

I have no further comments to add from what has already been raised, so
with thosed fixed:

Reviewed-by: Naushir Patuck <naush at raspberrypi.com>



> ---
>
> This is the version suggested by Laurent of
> [PATCH] libcamera: formats: Search V4L2 format info on map
>
> I don't like it too much but there's a slight performance improvement.
>
> ---
>  include/libcamera/internal/v4l2_pixelformat.h |  2 +-
>  src/libcamera/formats.cpp                     | 10 +++++++++-
>  src/libcamera/v4l2_pixelformat.cpp            | 16 ++++++++++++----
>  3 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/include/libcamera/internal/v4l2_pixelformat.h
> b/include/libcamera/internal/v4l2_pixelformat.h
> index 34d283db44f4..44439fff73eb 100644
> --- a/include/libcamera/internal/v4l2_pixelformat.h
> +++ b/include/libcamera/internal/v4l2_pixelformat.h
> @@ -45,7 +45,7 @@ public:
>         std::string toString() const;
>         const char *description() const;
>
> -       PixelFormat toPixelFormat() const;
> +       PixelFormat toPixelFormat(bool warn = true) const;
>         static const std::vector<V4L2PixelFormat> &
>         fromPixelFormat(const PixelFormat &pixelFormat);
>
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index 0bd0e09ae44c..f5769c489e16 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -852,7 +852,15 @@ const PixelFormatInfo &PixelFormatInfo::info(const
> PixelFormat &format)
>   */
>  const PixelFormatInfo &PixelFormatInfo::info(const V4L2PixelFormat
> &format)
>  {
> -       return info(format.toPixelFormat());
> +       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;
>  }
>
>  /**
> diff --git a/src/libcamera/v4l2_pixelformat.cpp
> b/src/libcamera/v4l2_pixelformat.cpp
> index 3590fb735011..26c9f02c759f 100644
> --- a/src/libcamera/v4l2_pixelformat.cpp
> +++ b/src/libcamera/v4l2_pixelformat.cpp
> @@ -294,15 +294,23 @@ const char *V4L2PixelFormat::description() const
>
>  /**
>   * \brief Convert the V4L2 pixel format to the corresponding PixelFormat
> + * \param[in] warn Flag to control the warning message if the format is
> not
> + * supported. Default to true, set to false to suppress warning message.
> + *
> + * Users of this function might try to convert a V4L2PixelFormat to a
> PixelFormat
> + * just to check if the format is supported or not. In that case, they
> can suppress
> + * the warning message setting the \a warn argument to false not not
> pollute the log
> + * with unnecessary warning messages.
> + *
>   * \return The PixelFormat corresponding to the V4L2 pixel format
>   */
> -PixelFormat V4L2PixelFormat::toPixelFormat() const
> +PixelFormat V4L2PixelFormat::toPixelFormat(bool warn) const
>  {
>         const auto iter = vpf2pf.find(*this);
>         if (iter == vpf2pf.end()) {
> -               LOG(V4L2, Warning)
> -                       << "Unsupported V4L2 pixel format "
> -                       << toString();
> +               if (warn)
> +                       LOG(V4L2, Warning) << "Unsupported V4L2 pixel
> format "
> +                                          << toString();
>                 return PixelFormat();
>         }
>
> --
> 2.37.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220812/d9eb716e/attachment.htm>