<div dir="ltr"><div dir="ltr">Hi Laurent,<div><br></div><div>Thank you for this fix.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 5 Sept 2022 at 17:46, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</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 e297673e7686 ("libcamera: v4l2_device: Adjust colorspace based on<br>
pixel format") has introduced a warning when trying to convert a color<br>
space from V4L2 to libcamera if the media bus code is unknown. This was<br>
meant to catch unknown image formats, but turned out to be also<br>
triggered for metadata formats.<br>
<br>
Color spaces are not applicable to metadata formats, there should thus<br>
be no warning. Fix it by skipping the color space translation and<br>
returning std::nullopt directly if the kernel reports<br>
V4L2_COLORSPACE_DEFAULT. This doesn't introduce any change in behaviour<br>
other than getting rid of the warning, as the V4L2Device::toColorSpace()<br>
function returns std::nullopt alread in that case.<br>
<br>
Fixes: e297673e7686 ("libcamera: v4l2_device: Adjust colorspace based on pixel format")<br>
Reported-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
Signed-off-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br></blockquote><div><br></div><div>This does indeed remove the WARN message that I highlighted.</div><div><br></div><div>Tested-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com">naush@raspberrypi.com</a>></div><div>Reviewed-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com">naush@raspberrypi.com</a>> </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
---<br>
include/libcamera/internal/v4l2_subdevice.h | 3 ++<br>
src/libcamera/v4l2_subdevice.cpp | 51 ++++++++++-----------<br>
2 files changed, 26 insertions(+), 28 deletions(-)<br>
<br>
diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h<br>
index 00be17bb1465..69862de0585a 100644<br>
--- a/include/libcamera/internal/v4l2_subdevice.h<br>
+++ b/include/libcamera/internal/v4l2_subdevice.h<br>
@@ -101,6 +101,9 @@ protected:<br>
private:<br>
LIBCAMERA_DISABLE_COPY(V4L2Subdevice)<br>
<br>
+ std::optional<ColorSpace><br>
+ toColorSpace(const v4l2_mbus_framefmt &format) const;<br>
+<br>
std::vector<unsigned int> enumPadCodes(unsigned int pad);<br>
std::vector<SizeRange> enumPadSizes(unsigned int pad,<br>
unsigned int code);<br>
diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp<br>
index 95bfde340d8c..f3a9a0096c6c 100644<br>
--- a/src/libcamera/v4l2_subdevice.cpp<br>
+++ b/src/libcamera/v4l2_subdevice.cpp<br>
@@ -477,6 +477,27 @@ V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)<br>
return formats;<br>
}<br>
<br>
+std::optional<ColorSpace> V4L2Subdevice::toColorSpace(const v4l2_mbus_framefmt &format) const<br>
+{<br>
+ if (format.colorspace == V4L2_COLORSPACE_DEFAULT)<br>
+ return std::nullopt;<br>
+<br>
+ PixelFormatInfo::ColourEncoding colourEncoding;<br>
+ auto iter = formatInfoMap.find(format.code);<br>
+ if (iter != formatInfoMap.end()) {<br>
+ colourEncoding = iter->second.colourEncoding;<br>
+ } else {<br>
+ LOG(V4L2, Warning)<br>
+ << "Unknown subdev format "<br>
+ << utils::hex(format.code, 4)<br>
+ << ", defaulting to RGB encoding";<br>
+<br>
+ colourEncoding = PixelFormatInfo::ColourEncodingRGB;<br>
+ }<br>
+<br>
+ return V4L2Device::toColorSpace(format, colourEncoding);<br>
+}<br>
+<br>
/**<br>
* \brief Retrieve the image format set on one of the V4L2 subdevice pads<br>
* \param[in] pad The 0-indexed pad number the format is to be retrieved from<br>
@@ -503,20 +524,7 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,<br>
format->size.width = subdevFmt.format.width;<br>
format->size.height = subdevFmt.format.height;<br>
format->mbus_code = subdevFmt.format.code;<br>
-<br>
- PixelFormatInfo::ColourEncoding colourEncoding;<br>
- auto iter = formatInfoMap.find(subdevFmt.format.code);<br>
- if (iter != formatInfoMap.end()) {<br>
- colourEncoding = iter->second.colourEncoding;<br>
- } else {<br>
- LOG(V4L2, Warning)<br>
- << "Unknown subdev format "<br>
- << utils::hex(subdevFmt.format.code, 4)<br>
- << ", defaulting to RGB encoding";<br>
-<br>
- colourEncoding = PixelFormatInfo::ColourEncodingRGB;<br>
- }<br>
- format->colorSpace = toColorSpace(subdevFmt.format, colourEncoding);<br>
+ format->colorSpace = toColorSpace(subdevFmt.format);<br>
<br>
return 0;<br>
}<br>
@@ -562,20 +570,7 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,<br>
format->size.width = subdevFmt.format.width;<br>
format->size.height = subdevFmt.format.height;<br>
format->mbus_code = subdevFmt.format.code;<br>
-<br>
- PixelFormatInfo::ColourEncoding colourEncoding;<br>
- auto iter = formatInfoMap.find(subdevFmt.format.code);<br>
- if (iter != formatInfoMap.end()) {<br>
- colourEncoding = iter->second.colourEncoding;<br>
- } else {<br>
- LOG(V4L2, Warning)<br>
- << "Unknown subdev format "<br>
- << utils::hex(subdevFmt.format.code, 4)<br>
- << ", defaulting to RGB encoding";<br>
-<br>
- colourEncoding = PixelFormatInfo::ColourEncodingRGB;<br>
- }<br>
- format->colorSpace = toColorSpace(subdevFmt.format, colourEncoding);<br>
+ format->colorSpace = toColorSpace(subdevFmt.format);<br>
<br>
return 0;<br>
}<br>
<br>
base-commit: da9bb8dea6821edcfd3464e8cba37ad4a91087d6<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
<br>
</blockquote></div></div>