<div dir="ltr"><div dir="ltr">Hi Jacopo,<div><br></div><div>Thanks for fixing this!</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 9 Aug 2022 at 15:49, Jacopo Mondi via libcamera-devel <<a href="mailto:libcamera-devel@lists.libcamera.org">libcamera-devel@lists.libcamera.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Commit f25ad4a2b16b ("libcamera: formats: Reimplement V4L2<br>
PixelFormatInfo::info()") changed the PixelFormatInfo::info(const<br>
V4L2PixelFormat &format) function overload to:<br>
<br>
        return info(format.toPixelFormat());<br>
<br>
As part of the series that contains such commit, the PixelFormatInfo for the<br>
pixel format applied to a video device is now retrieved at<br>
V4L2VideoDevice::open() time. Some video devices register formats not<br>
available to applications, for example metadata formats or, in the case<br>
of ISP devices, formats to describe the ISP statistics and parameters.<br>
<br>
This causes the<br>
<br>
        format.toPixelFormat()<br>
<br>
call to output a WARN message, which spams the log and unnecessary alert<br>
the users.<br>
<br>
Augment V4L2PixelFormat::toPixelFormat() with an optional argument to<br>
suppress warnings in the case a V4L2 pixel format is not known, to<br>
restore the behaviour preceding commit f25ad4a2b16b and returns an<br>
invalid PixelFormatInfo without outputting any unnecessary warning<br>
message.<br>
<br>
Fixes: f25ad4a2b16b ("libcamera: formats: Reimplement V4L2 PixelFormatInfo::info()")<br>
Signed-off-by: Jacopo Mondi <<a href="mailto:jacopo@jmondi.org" target="_blank">jacopo@jmondi.org</a>><br></blockquote><div><br></div><div>Tested-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com">naush@raspberrypi.com</a>></div><div><br></div><div>I have no further comments to add from what has already been raised, so with thosed fixed:</div><div><br></div><div>Reviewed-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com">naush@raspberrypi.com</a>><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
---<br>
<br>
This is the version suggested by Laurent of<br>
[PATCH] libcamera: formats: Search V4L2 format info on map<br>
<br>
I don't like it too much but there's a slight performance improvement.<br>
<br>
---<br>
 include/libcamera/internal/v4l2_pixelformat.h |  2 +-<br>
 src/libcamera/formats.cpp                     | 10 +++++++++-<br>
 src/libcamera/v4l2_pixelformat.cpp            | 16 ++++++++++++----<br>
 3 files changed, 22 insertions(+), 6 deletions(-)<br>
<br>
diff --git a/include/libcamera/internal/v4l2_pixelformat.h b/include/libcamera/internal/v4l2_pixelformat.h<br>
index 34d283db44f4..44439fff73eb 100644<br>
--- a/include/libcamera/internal/v4l2_pixelformat.h<br>
+++ b/include/libcamera/internal/v4l2_pixelformat.h<br>
@@ -45,7 +45,7 @@ public:<br>
        std::string toString() const;<br>
        const char *description() const;<br>
<br>
-       PixelFormat toPixelFormat() const;<br>
+       PixelFormat toPixelFormat(bool warn = true) const;<br>
        static const std::vector<V4L2PixelFormat> &<br>
        fromPixelFormat(const PixelFormat &pixelFormat);<br>
<br>
diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp<br>
index 0bd0e09ae44c..f5769c489e16 100644<br>
--- a/src/libcamera/formats.cpp<br>
+++ b/src/libcamera/formats.cpp<br>
@@ -852,7 +852,15 @@ const PixelFormatInfo &PixelFormatInfo::info(const PixelFormat &format)<br>
  */<br>
 const PixelFormatInfo &PixelFormatInfo::info(const V4L2PixelFormat &format)<br>
 {<br>
-       return info(format.toPixelFormat());<br>
+       PixelFormat pixelFormat = format.toPixelFormat(false);<br>
+       if (!pixelFormat.isValid())<br>
+               return pixelFormatInfoInvalid;<br>
+<br>
+       const auto iter = pixelFormatInfo.find(pixelFormat);<br>
+       if (iter == pixelFormatInfo.end())<br>
+               return pixelFormatInfoInvalid;<br>
+<br>
+       return iter->second;<br>
 }<br>
<br>
 /**<br>
diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp<br>
index 3590fb735011..26c9f02c759f 100644<br>
--- a/src/libcamera/v4l2_pixelformat.cpp<br>
+++ b/src/libcamera/v4l2_pixelformat.cpp<br>
@@ -294,15 +294,23 @@ const char *V4L2PixelFormat::description() const<br>
<br>
 /**<br>
  * \brief Convert the V4L2 pixel format to the corresponding PixelFormat<br>
+ * \param[in] warn Flag to control the warning message if the format is not<br>
+ * supported. Default to true, set to false to suppress warning message.<br>
+ *<br>
+ * Users of this function might try to convert a V4L2PixelFormat to a PixelFormat<br>
+ * just to check if the format is supported or not. In that case, they can suppress<br>
+ * the warning message setting the \a warn argument to false not not pollute the log<br>
+ * with unnecessary warning messages.<br>
+ *<br>
  * \return The PixelFormat corresponding to the V4L2 pixel format<br>
  */<br>
-PixelFormat V4L2PixelFormat::toPixelFormat() const<br>
+PixelFormat V4L2PixelFormat::toPixelFormat(bool warn) const<br>
 {<br>
        const auto iter = vpf2pf.find(*this);<br>
        if (iter == vpf2pf.end()) {<br>
-               LOG(V4L2, Warning)<br>
-                       << "Unsupported V4L2 pixel format "<br>
-                       << toString();<br>
+               if (warn)<br>
+                       LOG(V4L2, Warning) << "Unsupported V4L2 pixel format "<br>
+                                          << toString();<br>
                return PixelFormat();<br>
        }<br>
<br>
--<br>
2.37.1<br>
<br>
</blockquote></div></div>