[{"id":24941,"web_url":"https://patchwork.libcamera.org/comment/24941/","msgid":"<01ac7cd0-e1ff-5ad4-4765-33d88a78239d@ideasonboard.com>","date":"2022-09-05T19:42:39","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_subdevice: Silence\n\twarning for unknown metadata formats","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nThank you for the patch.\n\nOn 9/5/22 10:16 PM, Laurent Pinchart via libcamera-devel wrote:\n> Commit e297673e7686 (\"libcamera: v4l2_device: Adjust colorspace based on\n> pixel format\") has introduced a warning when trying to convert a color\n> space from V4L2 to libcamera if the media bus code is unknown. This was\n> meant to catch unknown image formats, but turned out to be also\n> triggered for metadata formats.\n\nerr...\n>\n> Color spaces are not applicable to metadata formats, there should thus\n> be no warning. Fix it by skipping the color space translation and\n> returning std::nullopt directly if the kernel reports\n> V4L2_COLORSPACE_DEFAULT. This doesn't introduce any change in behaviour\n> other than getting rid of the warning, as the V4L2Device::toColorSpace()\n> function returns std::nullopt alread in that case.\n\ns/alread/already\n>\n> Fixes: e297673e7686 (\"libcamera: v4l2_device: Adjust colorspace based on pixel format\")\n> Reported-by: Naushir Patuck \n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/internal/v4l2_subdevice.h | 3 ++\n> src/libcamera/v4l2_subdevice.cpp | 51 ++++++++++-----------\n> 2 files changed, 26 insertions(+), 28 deletions(-)\n>\n> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> index 00be17bb1465..69862de0585a 100644\n> --- a/include/libcamera/internal/v4l2_subdevice.h\n> +++ b/include/libcamera/internal/v4l2_subdevice.h\n> @@ -101,6 +101,9 @@ protected:\n> private:\n> \tLIBCAMERA_DISABLE_COPY(V4L2Subdevice)\n> \n> +\tstd::optional\n> +\ttoColorSpace(const v4l2_mbus_framefmt &format) const;\n> +\n> \tstd::vector enumPadCodes(unsigned int pad);\n> \tstd::vector enumPadSizes(unsigned int pad,\n> \t\t\t\t\t unsigned int code);\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index 95bfde340d8c..f3a9a0096c6c 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -477,6 +477,27 @@ V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)\n> \treturn formats;\n> }\n> \n> +std::optional V4L2Subdevice::toColorSpace(const v4l2_mbus_framefmt &format) const\n> +{\n\nComment about metadata formats <>  V4L2_COLORSPACE_DEFAULT please :-)\n\nOther than that, looks good to me.\n\nReviewed-by: Umang Jain \n> +\tif (format.colorspace == V4L2_COLORSPACE_DEFAULT)\n> +\t\treturn std::nullopt;\n> +\n> +\tPixelFormatInfo::ColourEncoding colourEncoding;\n> +\tauto iter = formatInfoMap.find(format.code);\n> +\tif (iter != formatInfoMap.end()) {\n> +\t\tcolourEncoding = iter->second.colourEncoding;\n> +\t} else {\n> +\t\tLOG(V4L2, Warning)\n> +\t\t\t<< \"Unknown subdev format \"\n> +\t\t\t<< utils::hex(format.code, 4)\n> +\t\t\t<< \", defaulting to RGB encoding\";\n> +\n> +\t\tcolourEncoding = PixelFormatInfo::ColourEncodingRGB;\n> +\t}\n> +\n> +\treturn V4L2Device::toColorSpace(format, colourEncoding);\n> +}\n> +\n> /**\n> * \\brief Retrieve the image format set on one of the V4L2 subdevice pads\n> * \\param[in] pad The 0-indexed pad number the format is to be retrieved from\n> @@ -503,20 +524,7 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> \tformat->size.width = subdevFmt.format.width;\n> \tformat->size.height = subdevFmt.format.height;\n> \tformat->mbus_code = subdevFmt.format.code;\n> -\n> -\tPixelFormatInfo::ColourEncoding colourEncoding;\n> -\tauto iter = formatInfoMap.find(subdevFmt.format.code);\n> -\tif (iter != formatInfoMap.end()) {\n> -\t\tcolourEncoding = iter->second.colourEncoding;\n> -\t} else {\n> -\t\tLOG(V4L2, Warning)\n> -\t\t\t<< \"Unknown subdev format \"\n> -\t\t\t<< utils::hex(subdevFmt.format.code, 4)\n> -\t\t\t<< \", defaulting to RGB encoding\";\n> -\n> -\t\tcolourEncoding = PixelFormatInfo::ColourEncodingRGB;\n> -\t}\n> -\tformat->colorSpace = toColorSpace(subdevFmt.format, colourEncoding);\n> +\tformat->colorSpace = toColorSpace(subdevFmt.format);\n> \n> \treturn 0;\n> }\n> @@ -562,20 +570,7 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> \tformat->size.width = subdevFmt.format.width;\n> \tformat->size.height = subdevFmt.format.height;\n> \tformat->mbus_code = subdevFmt.format.code;\n> -\n> -\tPixelFormatInfo::ColourEncoding colourEncoding;\n> -\tauto iter = formatInfoMap.find(subdevFmt.format.code);\n> -\tif (iter != formatInfoMap.end()) {\n> -\t\tcolourEncoding = iter->second.colourEncoding;\n> -\t} else {\n> -\t\tLOG(V4L2, Warning)\n> -\t\t\t<< \"Unknown subdev format \"\n> -\t\t\t<< utils::hex(subdevFmt.format.code, 4)\n> -\t\t\t<< \", defaulting to RGB encoding\";\n> -\n> -\t\tcolourEncoding = PixelFormatInfo::ColourEncodingRGB;\n> -\t}\n> -\tformat->colorSpace = toColorSpace(subdevFmt.format, colourEncoding);\n> +\tformat->colorSpace = toColorSpace(subdevFmt.format);\n> \n> \treturn 0;\n> }\n>\n> base-commit: da9bb8dea6821edcfd3464e8cba37ad4a91087d6","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 98100C3272\n\tfor ;\n\tMon, 5 Sep 2022 19:42:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B31C962076;\n\tMon, 5 Sep 2022 21:42:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4E23061F9B\n\tfor ;\n\tMon, 5 Sep 2022 21:42:45 +0200 (CEST)","from [IPV6:2401:4900:1f3e:51f6:a5a9:b86d:98ea:c462] (unknown\n\t[IPv6:2401:4900:1f3e:51f6:a5a9:b86d:98ea:c462])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 49B136BC;\n\tMon, 5 Sep 2022 21:42:44 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1662406966;\n\tbh=IQqXbFVTP4NiUSHx/RNzTCwhIPiXVa70NIIzN3ELyaY=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=G7F5TswdWa+2w9xAoZCjJ1yZSKoEOz5Q4jiCkMz589aX/TqVHLX9uhscII3FyffvQ\n\tUz3cqxkPPYKddun5QYHclO0RdPT4m9fN3bMG+byeCIFZDfT+KOzrL/uzPr6Vwv2kXk\n\tsBn63/jp3ml5Vh15l5hOwIaNa/oQoUp1lnZDdyFl6fKlcEPOowGClkn2ACOA9mZ1ly\n\tkT8B2qG8by8oXTFGlvo1DNqgatuND9x2Clzl7gzqTeRPGs8cBnnfN9eonXWo4VHSk3\n\t+vZqG49TV9sMD3UqSq7zjzk1T6MEI/jDvRLSxP4KcZ3VMnz6zO5z/39eejyMj9zo2A\n\trYhr4/uv3gBcA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1662406964;\n\tbh=IQqXbFVTP4NiUSHx/RNzTCwhIPiXVa70NIIzN3ELyaY=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=ugw6Y2R9ORmHNNReJ7ucku7r2LGgLvuTh03f/vuMXkJ4UpV7KyBf0asepTrvUbeii\n\t0uEUpXUeGUv/KtGKCUvGY8MyfvbI6QBTkfeChKvPWcHPFSjCOE1PshHM2utu2MCsLt\n\tGnkHhTNvzELuWXf+YE+AyNbzK5FJxaKxObWFcxGw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ugw6Y2R9\"; dkim-atps=neutral","Message-ID":"<01ac7cd0-e1ff-5ad4-4765-33d88a78239d@ideasonboard.com>","Date":"Tue, 6 Sep 2022 01:12:39 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.12.0","Content-Language":"en-US","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220905164600.31629-1-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20220905164600.31629-1-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_subdevice: Silence\n\twarning for unknown metadata formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24942,"web_url":"https://patchwork.libcamera.org/comment/24942/","msgid":"","date":"2022-09-05T21:18:53","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_subdevice: Silence\n\twarning for unknown metadata formats","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nOn Tue, Sep 06, 2022 at 01:12:39AM +0530, Umang Jain wrote:\n> On 9/5/22 10:16 PM, Laurent Pinchart via libcamera-devel wrote:\n> > Commit e297673e7686 (\"libcamera: v4l2_device: Adjust colorspace based on\n> > pixel format\") has introduced a warning when trying to convert a color\n> > space from V4L2 to libcamera if the media bus code is unknown. This was\n> > meant to catch unknown image formats, but turned out to be also\n> > triggered for metadata formats.\n> \n> err...\n> \n> > Color spaces are not applicable to metadata formats, there should thus\n> > be no warning. Fix it by skipping the color space translation and\n> > returning std::nullopt directly if the kernel reports\n> > V4L2_COLORSPACE_DEFAULT. This doesn't introduce any change in behaviour\n> > other than getting rid of the warning, as the V4L2Device::toColorSpace()\n> > function returns std::nullopt alread in that case.\n> \n> s/alread/already\n> \n> > Fixes: e297673e7686 (\"libcamera: v4l2_device: Adjust colorspace based on pixel format\")\n> > Reported-by: Naushir Patuck \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > include/libcamera/internal/v4l2_subdevice.h | 3 ++\n> > src/libcamera/v4l2_subdevice.cpp | 51 ++++++++++-----------\n> > 2 files changed, 26 insertions(+), 28 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> > index 00be17bb1465..69862de0585a 100644\n> > --- a/include/libcamera/internal/v4l2_subdevice.h\n> > +++ b/include/libcamera/internal/v4l2_subdevice.h\n> > @@ -101,6 +101,9 @@ protected:\n> > private:\n> > \tLIBCAMERA_DISABLE_COPY(V4L2Subdevice)\n> > \n> > +\tstd::optional\n> > +\ttoColorSpace(const v4l2_mbus_framefmt &format) const;\n> > +\n> > \tstd::vector enumPadCodes(unsigned int pad);\n> > \tstd::vector enumPadSizes(unsigned int pad,\n> > \t\t\t\t\t unsigned int code);\n> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > index 95bfde340d8c..f3a9a0096c6c 100644\n> > --- a/src/libcamera/v4l2_subdevice.cpp\n> > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > @@ -477,6 +477,27 @@ V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)\n> > \treturn formats;\n> > }\n> > \n> > +std::optional V4L2Subdevice::toColorSpace(const v4l2_mbus_framefmt &format) const\n> > +{\n> \n> Comment about metadata formats <>  V4L2_COLORSPACE_DEFAULT please :-)\n\nI'll add\n\n\t/*\n\t * Only image formats have a color space, for other formats (such as\n\t * metadata formats) the color space concept isn't applicable. V4L2\n\t * subdev drivers return a colorspace set to V4L2_COLORSPACE_DEFAULT in\n\t * that case (as well as for image formats when the driver hasn't\n\t * bothered implementing color space support). Check the colorspace\n\t * field here and return std::nullopt directly to avoid logging a\n\t * warning.\n\t */\n\n> Other than that, looks good to me.\n> \n> Reviewed-by: Umang Jain \n> \n> > +\tif (format.colorspace == V4L2_COLORSPACE_DEFAULT)\n> > +\t\treturn std::nullopt;\n> > +\n> > +\tPixelFormatInfo::ColourEncoding colourEncoding;\n> > +\tauto iter = formatInfoMap.find(format.code);\n> > +\tif (iter != formatInfoMap.end()) {\n> > +\t\tcolourEncoding = iter->second.colourEncoding;\n> > +\t} else {\n> > +\t\tLOG(V4L2, Warning)\n> > +\t\t\t<< \"Unknown subdev format \"\n> > +\t\t\t<< utils::hex(format.code, 4)\n> > +\t\t\t<< \", defaulting to RGB encoding\";\n> > +\n> > +\t\tcolourEncoding = PixelFormatInfo::ColourEncodingRGB;\n> > +\t}\n> > +\n> > +\treturn V4L2Device::toColorSpace(format, colourEncoding);\n> > +}\n> > +\n> > /**\n> > * \\brief Retrieve the image format set on one of the V4L2 subdevice pads\n> > * \\param[in] pad The 0-indexed pad number the format is to be retrieved from\n> > @@ -503,20 +524,7 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> > \tformat->size.width = subdevFmt.format.width;\n> > \tformat->size.height = subdevFmt.format.height;\n> > \tformat->mbus_code = subdevFmt.format.code;\n> > -\n> > -\tPixelFormatInfo::ColourEncoding colourEncoding;\n> > -\tauto iter = formatInfoMap.find(subdevFmt.format.code);\n> > -\tif (iter != formatInfoMap.end()) {\n> > -\t\tcolourEncoding = iter->second.colourEncoding;\n> > -\t} else {\n> > -\t\tLOG(V4L2, Warning)\n> > -\t\t\t<< \"Unknown subdev format \"\n> > -\t\t\t<< utils::hex(subdevFmt.format.code, 4)\n> > -\t\t\t<< \", defaulting to RGB encoding\";\n> > -\n> > -\t\tcolourEncoding = PixelFormatInfo::ColourEncodingRGB;\n> > -\t}\n> > -\tformat->colorSpace = toColorSpace(subdevFmt.format, colourEncoding);\n> > +\tformat->colorSpace = toColorSpace(subdevFmt.format);\n> > \n> > \treturn 0;\n> > }\n> > @@ -562,20 +570,7 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> > \tformat->size.width = subdevFmt.format.width;\n> > \tformat->size.height = subdevFmt.format.height;\n> > \tformat->mbus_code = subdevFmt.format.code;\n> > -\n> > -\tPixelFormatInfo::ColourEncoding colourEncoding;\n> > -\tauto iter = formatInfoMap.find(subdevFmt.format.code);\n> > -\tif (iter != formatInfoMap.end()) {\n> > -\t\tcolourEncoding = iter->second.colourEncoding;\n> > -\t} else {\n> > -\t\tLOG(V4L2, Warning)\n> > -\t\t\t<< \"Unknown subdev format \"\n> > -\t\t\t<< utils::hex(subdevFmt.format.code, 4)\n> > -\t\t\t<< \", defaulting to RGB encoding\";\n> > -\n> > -\t\tcolourEncoding = PixelFormatInfo::ColourEncodingRGB;\n> > -\t}\n> > -\tformat->colorSpace = toColorSpace(subdevFmt.format, colourEncoding);\n> > +\tformat->colorSpace = toColorSpace(subdevFmt.format);\n> > \n> > \treturn 0;\n> > }\n> >\n> > base-commit: da9bb8dea6821edcfd3464e8cba37ad4a91087d6","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id EA24CC0DA4\n\tfor ;\n\tMon, 5 Sep 2022 21:19:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 41F9862076;\n\tMon, 5 Sep 2022 23:19:12 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CB8AB61F9B\n\tfor ;\n\tMon, 5 Sep 2022 23:19:09 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 273386BC;\n\tMon, 5 Sep 2022 23:19:09 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1662412752;\n\tbh=AxZ98yxTNjFR/18tKfNWzlTy5b/+3l7eVIK8oe07LJs=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=2FaIAKp9HOJL9Xyt/iACMSrS4jVQoIhOk4ZzHCdkOdAGG+VN1ZWPOFmbYqJ6daXuL\n\tp0Llsz2kkHG9nyS+43KxRN27XFfg+CK7Yx3dyzVnUwU+1uDFqwLwZf6ZWwHLrGFhmY\n\tBNR2rXDR5NxILs8+RvfyHJd2+oY1q8xGtSpNQSp9GH2vfcAjQfbnppkwsQdU3+VDvX\n\tIR0nDAZeIuu7mjVBIS/ZnWeuQocR6vaLB+iRSuE5ZvfGoAGNaAIKbAGFMzR9Gt80QY\n\t19RuaKw5AGPqZUODK9Zvq72ItsOgAD1T7jJMATcy+8jN3itjpTNnyKs19M/6uYsKLB\n\twqesoNUIY0z/w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1662412749;\n\tbh=AxZ98yxTNjFR/18tKfNWzlTy5b/+3l7eVIK8oe07LJs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mjk38VIFzqOiWX/FcwVhcNXzlkLGWjK83wA2g6QvIrBrefKVWM54nhs3qLzgekVqy\n\tGc6xgk0MnoHDBkxkLEoPY8UX768jANxHFNpRBNwfomStl+o1Xu4HplO+BHct6h/J76\n\t65L34oFV5J1egdalfwEfxva/vUI5cF/W2ykEXE4Q="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"mjk38VIF\"; dkim-atps=neutral","Date":"Tue, 6 Sep 2022 00:18:53 +0300","To":"Umang Jain ","Message-ID":"","References":"<20220905164600.31629-1-laurent.pinchart@ideasonboard.com>\n\t<01ac7cd0-e1ff-5ad4-4765-33d88a78239d@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<01ac7cd0-e1ff-5ad4-4765-33d88a78239d@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_subdevice: Silence\n\twarning for unknown metadata formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24961,"web_url":"https://patchwork.libcamera.org/comment/24961/","msgid":"","date":"2022-09-08T13:26:26","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_subdevice: Silence\n\twarning for unknown metadata formats","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for this fix.\n\nOn Mon, 5 Sept 2022 at 17:46, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Commit e297673e7686 (\"libcamera: v4l2_device: Adjust colorspace based on\n> pixel format\") has introduced a warning when trying to convert a color\n> space from V4L2 to libcamera if the media bus code is unknown. This was\n> meant to catch unknown image formats, but turned out to be also\n> triggered for metadata formats.\n>\n> Color spaces are not applicable to metadata formats, there should thus\n> be no warning. Fix it by skipping the color space translation and\n> returning std::nullopt directly if the kernel reports\n> V4L2_COLORSPACE_DEFAULT. This doesn't introduce any change in behaviour\n> other than getting rid of the warning, as the V4L2Device::toColorSpace()\n> function returns std::nullopt alread in that case.\n>\n> Fixes: e297673e7686 (\"libcamera: v4l2_device: Adjust colorspace based on\n> pixel format\")\n> Reported-by: Naushir Patuck \n> Signed-off-by: Laurent Pinchart \n>\n\nThis does indeed remove the WARN message that I highlighted.\n\nTested-by: Naushir Patuck \nReviewed-by: Naushir Patuck \n\n---\n> include/libcamera/internal/v4l2_subdevice.h | 3 ++\n> src/libcamera/v4l2_subdevice.cpp | 51 ++++++++++-----------\n> 2 files changed, 26 insertions(+), 28 deletions(-)\n>\n> diff --git a/include/libcamera/internal/v4l2_subdevice.h\n> b/include/libcamera/internal/v4l2_subdevice.h\n> index 00be17bb1465..69862de0585a 100644\n> --- a/include/libcamera/internal/v4l2_subdevice.h\n> +++ b/include/libcamera/internal/v4l2_subdevice.h\n> @@ -101,6 +101,9 @@ protected:\n> private:\n> LIBCAMERA_DISABLE_COPY(V4L2Subdevice)\n>\n> + std::optional\n> + toColorSpace(const v4l2_mbus_framefmt &format) const;\n> +\n> std::vector enumPadCodes(unsigned int pad);\n> std::vector enumPadSizes(unsigned int pad,\n> unsigned int code);\n> diff --git a/src/libcamera/v4l2_subdevice.cpp\n> b/src/libcamera/v4l2_subdevice.cpp\n> index 95bfde340d8c..f3a9a0096c6c 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -477,6 +477,27 @@ V4L2Subdevice::Formats\n> V4L2Subdevice::formats(unsigned int pad)\n> return formats;\n> }\n>\n> +std::optional V4L2Subdevice::toColorSpace(const\n> v4l2_mbus_framefmt &format) const\n> +{\n> + if (format.colorspace == V4L2_COLORSPACE_DEFAULT)\n> + return std::nullopt;\n> +\n> + PixelFormatInfo::ColourEncoding colourEncoding;\n> + auto iter = formatInfoMap.find(format.code);\n> + if (iter != formatInfoMap.end()) {\n> + colourEncoding = iter->second.colourEncoding;\n> + } else {\n> + LOG(V4L2, Warning)\n> + << \"Unknown subdev format \"\n> + << utils::hex(format.code, 4)\n> + << \", defaulting to RGB encoding\";\n> +\n> + colourEncoding = PixelFormatInfo::ColourEncodingRGB;\n> + }\n> +\n> + return V4L2Device::toColorSpace(format, colourEncoding);\n> +}\n> +\n> /**\n> * \\brief Retrieve the image format set on one of the V4L2 subdevice pads\n> * \\param[in] pad The 0-indexed pad number the format is to be retrieved\n> from\n> @@ -503,20 +524,7 @@ int V4L2Subdevice::getFormat(unsigned int pad,\n> V4L2SubdeviceFormat *format,\n> format->size.width = subdevFmt.format.width;\n> format->size.height = subdevFmt.format.height;\n> format->mbus_code = subdevFmt.format.code;\n> -\n> - PixelFormatInfo::ColourEncoding colourEncoding;\n> - auto iter = formatInfoMap.find(subdevFmt.format.code);\n> - if (iter != formatInfoMap.end()) {\n> - colourEncoding = iter->second.colourEncoding;\n> - } else {\n> - LOG(V4L2, Warning)\n> - << \"Unknown subdev format \"\n> - << utils::hex(subdevFmt.format.code, 4)\n> - << \", defaulting to RGB encoding\";\n> -\n> - colourEncoding = PixelFormatInfo::ColourEncodingRGB;\n> - }\n> - format->colorSpace = toColorSpace(subdevFmt.format,\n> colourEncoding);\n> + format->colorSpace = toColorSpace(subdevFmt.format);\n>\n> return 0;\n> }\n> @@ -562,20 +570,7 @@ int V4L2Subdevice::setFormat(unsigned int pad,\n> V4L2SubdeviceFormat *format,\n> format->size.width = subdevFmt.format.width;\n> format->size.height = subdevFmt.format.height;\n> format->mbus_code = subdevFmt.format.code;\n> -\n> - PixelFormatInfo::ColourEncoding colourEncoding;\n> - auto iter = formatInfoMap.find(subdevFmt.format.code);\n> - if (iter != formatInfoMap.end()) {\n> - colourEncoding = iter->second.colourEncoding;\n> - } else {\n> - LOG(V4L2, Warning)\n> - << \"Unknown subdev format \"\n> - << utils::hex(subdevFmt.format.code, 4)\n> - << \", defaulting to RGB encoding\";\n> -\n> - colourEncoding = PixelFormatInfo::ColourEncodingRGB;\n> - }\n> - format->colorSpace = toColorSpace(subdevFmt.format,\n> colourEncoding);\n> + format->colorSpace = toColorSpace(subdevFmt.format);\n>\n> return 0;\n> }\n>\n> base-commit: da9bb8dea6821edcfd3464e8cba37ad4a91087d6\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id CA0A5C0DA4\n\tfor ;\n\tThu, 8 Sep 2022 13:26:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CD779620A6;\n\tThu, 8 Sep 2022 15:26:43 +0200 (CEST)","from mail-lj1-x234.google.com (mail-lj1-x234.google.com\n\t[IPv6:2a00:1450:4864:20::234])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 717D26203B\n\tfor ;\n\tThu, 8 Sep 2022 15:26:42 +0200 (CEST)","by mail-lj1-x234.google.com with SMTP id y29so7286271ljq.7\n\tfor ;\n\tThu, 08 Sep 2022 06:26:42 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1662643603;\n\tbh=kD9sYubvcuQxKN5FER0AHvWrgQWwBPXowjQ1RrvXtMM=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=G5JpJ4hn0KxSECM2mA76z0YbnlGvMOsbel8luJn2VXN52XxrxuL383oUNnW3haLiy\n\thBpGW05DBkcS/JpWhOINNBdbG7R+Z1Cg8JU+yYuSzwH/TsTC0FdFcsPvKqBBQnNt0Q\n\tv97cwc6dtEcDZH1Z+XJ+zi9Bto71HZu4iumzJH+AXBIkDV8+XJ1+MZrmVHmcoYLq3w\n\tq15FB3ezHbyLHTsB/PbhJw9wEH80hAs6gJ2sgKSGcJpGw7c+IErW3+6hXvTeug/tNZ\n\tUdlhHSNAWAg57+VRECXGSesrbmdprgUiNU+j9saCvfxuMXr2Eaan4HHOGyz4TZ5RW4\n\tPCPX7WTKAvDwg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date;\n\tbh=gdSiDrS54+NUW8f2H9FhjG129ldEW3eh4OHyzJ4tvzE=;\n\tb=srFPzXHNSA8PJ8f5Pa0pvCxW3qA3V+gdhI8J05v4pjEeVbs775brn0MlXadlP6O11m\n\tPGY6Wg2++NM51SBIys2sujXpKVmaFUhInc5rnpGqkQPKfsX4Ddwk/MW9fZVqZA8nzRdX\n\tC0jdsmX61AZcs+YHZXPMxN52842k9n6yx+oAfihRm0NVhyQ6ZYxgcX/PGK2/mK1WbGHs\n\tQvXqf6DdO8uBvHPOjVfWIZbRT2O6zQkaMGUUZc3BrpaPe20FieQhjBVH6cfHBOiUmkCA\n\tj2DPMRK+XG7x7R3gMk7Os6UbLp46ePpYz0+qxzWDm4d/PzDLdBjctvlxtGEjYduAWHtC\n\tZyPg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"srFPzXHN\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date;\n\tbh=gdSiDrS54+NUW8f2H9FhjG129ldEW3eh4OHyzJ4tvzE=;\n\tb=zECshCpKF5MbDW53Ah44tc/oWtj0BiwsgjMRODUBeOvPR42ji1KMyx8zD0F8Pwl7bQ\n\t6I/l+OjQ10VFiO+UtOEq8CLelnrq8qlykmcSf3zN5c8qlyRBo8FpQw9Z3F1LvHMT896o\n\tXgbXBxxnMk76YmgkS8ERXhJn9aWvAiwhAQOkx5iF7JrvEwDyz9RQn74W4QzgOM33JNM8\n\tNM6mAefSZjI2tFi3aMNFgcmo1tZnV/wGp378zpgVwI3+CbKDrpr3itOqD5XkJurTn6iS\n\tix3XUOQIOrctpg6kqGxwOYiNxjo6KadXUTrxNB1pegMTsEHBuMDpRWgeDKeTEepst4Vk\n\tunqg==","X-Gm-Message-State":"ACgBeo2oVW0hbs7FCZhhyMkgofdrKgsPSTRsYUF6PkdA2k5gtgldcqYO\n\to6vLHWZ5kI9mE+FFSfB2rP2DLEqg+3ZnYsTfVkNJrQ==","X-Google-Smtp-Source":"AA6agR4BPH4R2g0tJMq6D5Xl0mBmk3t6tNYUJbO9fKy1LXH3fEiHESWiaC7bCuquR8LTJqTFTqIoMGgDzwiRCXx2yw0=","X-Received":"by 2002:a2e:808f:0:b0:26a:b498:2c87 with SMTP id\n\ti15-20020a2e808f000000b0026ab4982c87mr2549981ljg.279.1662643601591;\n\tThu, 08 Sep 2022 06:26:41 -0700 (PDT)","MIME-Version":"1.0","References":"<20220905164600.31629-1-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20220905164600.31629-1-laurent.pinchart@ideasonboard.com>","Date":"Thu, 8 Sep 2022 14:26:26 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"00000000000042eae505e82a61ba\"","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_subdevice: Silence\n\twarning for unknown metadata formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]