[{"id":33338,"web_url":"https://patchwork.libcamera.org/comment/33338/","msgid":"<173936101978.4102114.865368512044452816@ping.linuxembedded.co.uk>","date":"2025-02-12T11:50:19","subject":"Re: [PATCH] libcamera: formats: Silence warning when creating a\n\tPixelFormatInfo from a null format","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stefan Klug (2025-02-12 11:29:42)\n> Calling PixelFormat().toString() correctly returns \"0x0-\" but it\n> also produces the following, possibly confusing, warning:\n> \n> WARN Formats formats.cpp:1006 Unsupported pixel format 0x00000000\n> \n> Silence the warning in PixelFormatInfo::info() in case the format is\n> invalid.\n> \n> While at it, remove code duplication by using\n> PixelFormatInfo::info(const PixelFormat &format) to implement\n> PixelFormatInfo::info(const V4L2PixelFormat &format).\n\nJust to check the obvious - if there is a V4L2 format on a device that\nwe don't have mapped/supported in libcamera, will it still print \n\nWARN Formats formats.cpp:1006 Unsupported pixel format 0x23456789 ?\n\nAs long as that's the case, \n\n\nReviewed-by: Kieran Bingham \n\n> \n> Signed-off-by: Stefan Klug \n> ---\n> \n> Hi all,\n> \n> The attached patch has one noteworthy side effect: If\n> info(V4L2PixelFormat &format) is called with a valid but unsupported (by\n> libcamera) format, we now get the same warning as in the\n> info(PixelFormat &format) case. I believe that is sensible but maybe the\n> warning was left off in the V4L2 case on purpose.\n> \n> Best regards,\n> Stefan\n> \n> src/libcamera/formats.cpp | 12 ++++--------\n> 1 file changed, 4 insertions(+), 8 deletions(-)\n> \n> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> index bfcdfc08960d..df7413f58ba8 100644\n> --- a/src/libcamera/formats.cpp\n> +++ b/src/libcamera/formats.cpp\n> @@ -1001,6 +1001,9 @@ const std::map pixelFormatInfo{\n> */\n> const PixelFormatInfo &PixelFormatInfo::info(const PixelFormat &format)\n> {\n> + if (!format.isValid())\n> + return pixelFormatInfoInvalid;\n> +\n> const auto iter = pixelFormatInfo.find(format);\n> if (iter == pixelFormatInfo.end()) {\n> LOG(Formats, Warning)\n\nDoes the !isValid() check mean it's impossible to get here now? Or is\nthis check still necessary?\n\n\n> @@ -1021,14 +1024,7 @@ const PixelFormatInfo &PixelFormatInfo::info(const PixelFormat &format)\n> const PixelFormatInfo &PixelFormatInfo::info(const V4L2PixelFormat &format)\n> {\n> PixelFormat pixelFormat = format.toPixelFormat(false);\n> - if (!pixelFormat.isValid())\n> - return pixelFormatInfoInvalid;\n> -\n> - const auto iter = pixelFormatInfo.find(pixelFormat);\n> - if (iter == pixelFormatInfo.end())\n> - return pixelFormatInfoInvalid;\n> -\n> - return iter->second;\n> + return info(pixelFormat);\n> \n> }\n> \n> /**\n> -- \n> 2.43.0\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 9FB9BC3301\n\tfor ;\n\tWed, 12 Feb 2025 11:50:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C4C9D6862F;\n\tWed, 12 Feb 2025 12:50:23 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CB3F7685FF\n\tfor ;\n\tWed, 12 Feb 2025 12:50:22 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F3698778;\n\tWed, 12 Feb 2025 12:49:04 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"nybkWsr8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1739360945;\n\tbh=0bsRpzS1xmfrdvsoz7LJLGQl2YxdJEaERj5C88OAoWA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=nybkWsr876vZknZ7vGouNlsRsnGiFXS+ZMQgqzJ3q2shKa+hMwngVdSBKeM2lZBma\n\t8jiro1ffiJlQGGe2r4hFlkHLeOxjQfMjq0tAOIqIvvwNGk9apVe7ZIFB96rP68rxE3\n\t8zsW37ezOHeI0KrsFFJJVsm8X/i+eNSaaQQ96gBs=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250212113328.304528-1-stefan.klug@ideasonboard.com>","References":"<20250212113328.304528-1-stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH] libcamera: formats: Silence warning when creating a\n\tPixelFormatInfo from a null format","From":"Kieran Bingham ","Cc":"Stefan Klug ","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 12 Feb 2025 11:50:19 +0000","Message-ID":"<173936101978.4102114.865368512044452816@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33339,"web_url":"https://patchwork.libcamera.org/comment/33339/","msgid":"","date":"2025-02-12T12:00:21","subject":"Re: [PATCH] libcamera: formats: Silence warning when creating a\n\tPixelFormatInfo from a null format","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the review. \n\nOn Wed, Feb 12, 2025 at 11:50:19AM +0000, Kieran Bingham wrote:\n> Quoting Stefan Klug (2025-02-12 11:29:42)\n> > Calling PixelFormat().toString() correctly returns \"0x0-\" but it\n> > also produces the following, possibly confusing, warning:\n> > \n> > WARN Formats formats.cpp:1006 Unsupported pixel format 0x00000000\n> > \n> > Silence the warning in PixelFormatInfo::info() in case the format is\n> > invalid.\n> > \n> > While at it, remove code duplication by using\n> > PixelFormatInfo::info(const PixelFormat &format) to implement\n> > PixelFormatInfo::info(const V4L2PixelFormat &format).\n> \n> Just to check the obvious - if there is a V4L2 format on a device that\n> we don't have mapped/supported in libcamera, will it still print \n> \n> WARN Formats formats.cpp:1006 Unsupported pixel format 0x23456789 ?\n> \n> As long as that's the case, \n\nWell, we got that warning only for formats passed as PixelFormat. If the\nformat was passed as V4L2PixelFormat there was no warning at all. Now we\nget the warning in both cases.\n\n> \n> \n> Reviewed-by: Kieran Bingham \n> \n> > \n> > Signed-off-by: Stefan Klug \n> > ---\n> > \n> > Hi all,\n> > \n> > The attached patch has one noteworthy side effect: If\n> > info(V4L2PixelFormat &format) is called with a valid but unsupported (by\n> > libcamera) format, we now get the same warning as in the\n> > info(PixelFormat &format) case. I believe that is sensible but maybe the\n> > warning was left off in the V4L2 case on purpose.\n> > \n> > Best regards,\n> > Stefan\n> > \n> > src/libcamera/formats.cpp | 12 ++++--------\n> > 1 file changed, 4 insertions(+), 8 deletions(-)\n> > \n> > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> > index bfcdfc08960d..df7413f58ba8 100644\n> > --- a/src/libcamera/formats.cpp\n> > +++ b/src/libcamera/formats.cpp\n> > @@ -1001,6 +1001,9 @@ const std::map pixelFormatInfo{\n> > */\n> > const PixelFormatInfo &PixelFormatInfo::info(const PixelFormat &format)\n> > {\n> > + if (!format.isValid())\n> > + return pixelFormatInfoInvalid;\n> > +\n> > const auto iter = pixelFormatInfo.find(format);\n> > if (iter == pixelFormatInfo.end()) {\n> > LOG(Formats, Warning)\n> \n> Does the !isValid() check mean it's impossible to get here now? Or is\n> this check still necessary?\n\nisValid() only checks if the fourcc code is 0. So the warning shows up\nif the format is valid but not supported by libcamera. Which was imho\nthe intended case.\n\nBest regards,\nStefan\n\n> \n> \n> > @@ -1021,14 +1024,7 @@ const PixelFormatInfo &PixelFormatInfo::info(const PixelFormat &format)\n> > const PixelFormatInfo &PixelFormatInfo::info(const V4L2PixelFormat &format)\n> > {\n> > PixelFormat pixelFormat = format.toPixelFormat(false);\n> > - if (!pixelFormat.isValid())\n> > - return pixelFormatInfoInvalid;\n> > -\n> > - const auto iter = pixelFormatInfo.find(pixelFormat);\n> > - if (iter == pixelFormatInfo.end())\n> > - return pixelFormatInfoInvalid;\n> > -\n> > - return iter->second;\n> > + return info(pixelFormat);\n> > \n> > }\n> > \n> > /**\n> > -- \n> > 2.43.0\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 63C3AC3304\n\tfor ;\n\tWed, 12 Feb 2025 12:00:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 74E5B6862F;\n\tWed, 12 Feb 2025 13:00:26 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0B284685FF\n\tfor ;\n\tWed, 12 Feb 2025 13:00:25 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:533:2ab9:67b7:5f65])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 24836778;\n\tWed, 12 Feb 2025 12:59:07 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"q9ezOk66\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1739361547;\n\tbh=5FJDQczKXEzdjDu1pr3zPWyniIpVD/2u53Hh0k2Qoz4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=q9ezOk66htKUnXxXkzOSqcsKv3z1FM+Qor3JQriP0qvePCVNHOca69NlDwUbpZ3HB\n\tRACi64J/6H5vvccHM3xQ7jQgWrA3naaHy579dexyeTU4rL9rC3hZMXjssjVDNJh7kQ\n\tGw4sAPH7tUI4AIEGnyO+UW1vhoIxaSeMan/26c1M=","Date":"Wed, 12 Feb 2025 13:00:21 +0100","From":"Stefan Klug ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: formats: Silence warning when creating a\n\tPixelFormatInfo from a null format","Message-ID":"","References":"<20250212113328.304528-1-stefan.klug@ideasonboard.com>\n\t<173936101978.4102114.865368512044452816@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<173936101978.4102114.865368512044452816@ping.linuxembedded.co.uk>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33353,"web_url":"https://patchwork.libcamera.org/comment/33353/","msgid":"<20250213234150.GC3457@pendragon.ideasonboard.com>","date":"2025-02-13T23:41:50","subject":"Re: [PATCH] libcamera: formats: Silence warning when creating a\n\tPixelFormatInfo from a null format","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Feb 12, 2025 at 01:00:21PM +0100, Stefan Klug wrote:\n> On Wed, Feb 12, 2025 at 11:50:19AM +0000, Kieran Bingham wrote:\n> > Quoting Stefan Klug (2025-02-12 11:29:42)\n> > > Calling PixelFormat().toString() correctly returns \"0x0-\" but it\n> > > also produces the following, possibly confusing, warning:\n> > > \n> > > WARN Formats formats.cpp:1006 Unsupported pixel format 0x00000000\n> > > \n> > > Silence the warning in PixelFormatInfo::info() in case the format is\n> > > invalid.\n> > > \n> > > While at it, remove code duplication by using\n> > > PixelFormatInfo::info(const PixelFormat &format) to implement\n> > > PixelFormatInfo::info(const V4L2PixelFormat &format).\n> > \n> > Just to check the obvious - if there is a V4L2 format on a device that\n> > we don't have mapped/supported in libcamera, will it still print \n> > \n> > WARN Formats formats.cpp:1006 Unsupported pixel format 0x23456789 ?\n> > \n> > As long as that's the case, \n> \n> Well, we got that warning only for formats passed as PixelFormat. If the\n> format was passed as V4L2PixelFormat there was no warning at all. Now we\n> get the warning in both cases.\n\nThis would be worth mentioning in the commit message. Or possible split\nin two patches, in case the additional warning for V4L2PixelFormat ends\nup being too noisy and we need to revert that part ?\n\nReviewed-by: Laurent Pinchart \n\n> > Reviewed-by: Kieran Bingham \n> > \n> > > Signed-off-by: Stefan Klug \n> > > ---\n> > > \n> > > Hi all,\n> > > \n> > > The attached patch has one noteworthy side effect: If\n> > > info(V4L2PixelFormat &format) is called with a valid but unsupported (by\n> > > libcamera) format, we now get the same warning as in the\n> > > info(PixelFormat &format) case. I believe that is sensible but maybe the\n> > > warning was left off in the V4L2 case on purpose.\n> > > \n> > > Best regards,\n> > > Stefan\n> > > \n> > > src/libcamera/formats.cpp | 12 ++++--------\n> > > 1 file changed, 4 insertions(+), 8 deletions(-)\n> > > \n> > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> > > index bfcdfc08960d..df7413f58ba8 100644\n> > > --- a/src/libcamera/formats.cpp\n> > > +++ b/src/libcamera/formats.cpp\n> > > @@ -1001,6 +1001,9 @@ const std::map pixelFormatInfo{\n> > > */\n> > > const PixelFormatInfo &PixelFormatInfo::info(const PixelFormat &format)\n> > > {\n> > > + if (!format.isValid())\n> > > + return pixelFormatInfoInvalid;\n> > > +\n> > > const auto iter = pixelFormatInfo.find(format);\n> > > if (iter == pixelFormatInfo.end()) {\n> > > LOG(Formats, Warning)\n> > \n> > Does the !isValid() check mean it's impossible to get here now? Or is\n> > this check still necessary?\n> \n> isValid() only checks if the fourcc code is 0. So the warning shows up\n> if the format is valid but not supported by libcamera. Which was imho\n> the intended case.\n> \n> > > @@ -1021,14 +1024,7 @@ const PixelFormatInfo &PixelFormatInfo::info(const PixelFormat &format)\n> > > const PixelFormatInfo &PixelFormatInfo::info(const V4L2PixelFormat &format)\n> > > {\n> > > PixelFormat pixelFormat = format.toPixelFormat(false);\n> > > - if (!pixelFormat.isValid())\n> > > - return pixelFormatInfoInvalid;\n> > > -\n> > > - const auto iter = pixelFormatInfo.find(pixelFormat);\n> > > - if (iter == pixelFormatInfo.end())\n> > > - return pixelFormatInfoInvalid;\n> > > -\n> > > - return iter->second;\n> > > + return info(pixelFormat);\n> > > \n> > > }\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 0228CBD7D8\n\tfor ;\n\tThu, 13 Feb 2025 23:42:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9452668644;\n\tFri, 14 Feb 2025 00:42:05 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0ADE2685FF\n\tfor ;\n\tFri, 14 Feb 2025 00:42:02 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E70E4581;\n\tFri, 14 Feb 2025 00:40:43 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"hOjJbGog\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1739490044;\n\tbh=9BD0+Z6nhRGFcEepDwCvG3L7MPVHxFsTnaoXnJPfRJE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hOjJbGog0NiOiKE73tsjoZ0SzoIkuk39QxbpgZSIlCST+rfkkkZ2z9IhQQNgCDMWf\n\tW0PAeYiAF9AymEAS4kgjSda88hc6J4uYy2dpw/jictzF0FzfhvQstqmuyDocYqizJ3\n\t87iG9jzPh8jDIuKuDDxQQXS5oQNX9m/y0+Mi4FHI=","Date":"Fri, 14 Feb 2025 01:41:50 +0200","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: formats: Silence warning when creating a\n\tPixelFormatInfo from a null format","Message-ID":"<20250213234150.GC3457@pendragon.ideasonboard.com>","References":"<20250212113328.304528-1-stefan.klug@ideasonboard.com>\n\t<173936101978.4102114.865368512044452816@ping.linuxembedded.co.uk>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]