[{"id":33359,"web_url":"https://patchwork.libcamera.org/comment/33359/","msgid":"<20250214162733.GA17930@pendragon.ideasonboard.com>","date":"2025-02-14T16:27:33","subject":"Re: [PATCH v2 1/2] libcamera: formats: Silence warning when creating\n\ta PixelFormatInfo from a null format","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Stefan,\n\nThank you for the patch.\n\nOn Fri, Feb 14, 2025 at 05:09:47PM +0100, Stefan Klug wrote:\n> Calling PixelFormat().toString() correctly returns \"0x0-<INVALID>\" 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\nSleeping over this made me wonder where/when you hit this warning. Is\nthere a valid use case to call this function with a default-constructed\npixel format ?\n\n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  src/libcamera/formats.cpp | 3 +++\n>  1 file changed, 3 insertions(+)\n> \n> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> index bfcdfc08960d..b4518e61d04a 100644\n> --- a/src/libcamera/formats.cpp\n> +++ b/src/libcamera/formats.cpp\n> @@ -1001,6 +1001,9 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{\n>   */\n>  const PixelFormatInfo &PixelFormatInfo::info(const PixelFormat &format)\n>  {\n> +\tif (!format.isValid())\n> +\t\treturn pixelFormatInfoInvalid;\n> +\n>  \tconst auto iter = pixelFormatInfo.find(format);\n>  \tif (iter == pixelFormatInfo.end()) {\n>  \t\tLOG(Formats, Warning)","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 695BFC3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 14 Feb 2025 16:27:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1734F6864E;\n\tFri, 14 Feb 2025 17:27:48 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 959166862A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 14 Feb 2025 17:27:45 +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 35892710;\n\tFri, 14 Feb 2025 17:26:26 +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=\"BYQiq8Qv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1739550386;\n\tbh=wiVhXcyZ1Lyy0JN9mGhgEo4v+raOgdsj0XXcwxUNzQw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BYQiq8QvgZO4hwfChpe32htaV+Jpm1zkDfLPHK6bx3zIoG2a0Bar9Gj/JCfgWttZJ\n\tgOhHa9YTGz1hlV22LZ8EUXVccNG3zJPV5F1jfLJiniT+rVnkX/boGLy5ApwYi/NZaF\n\tfubAYNteS3N5oYP1G8t6uesSifpnhU/wgjXpbIR4=","Date":"Fri, 14 Feb 2025 18:27:33 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>","Subject":"Re: [PATCH v2 1/2] libcamera: formats: Silence warning when creating\n\ta PixelFormatInfo from a null format","Message-ID":"<20250214162733.GA17930@pendragon.ideasonboard.com>","References":"<20250214161031.240481-1-stefan.klug@ideasonboard.com>\n\t<20250214161031.240481-2-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250214161031.240481-2-stefan.klug@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33361,"web_url":"https://patchwork.libcamera.org/comment/33361/","msgid":"<4c3nuuqz5a3utjg753fzvrfxjw5crvqlhf6jc4kwtzviatdmu6@hdgddgmcg5sn>","date":"2025-02-14T16:42:16","subject":"Re: [PATCH v2 1/2] libcamera: formats: Silence warning when creating\n\ta PixelFormatInfo 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 Laurent,\n\nOn Fri, Feb 14, 2025 at 06:27:33PM +0200, Laurent Pinchart wrote:\n> Hi Stefan,\n> \n> Thank you for the patch.\n> \n> On Fri, Feb 14, 2025 at 05:09:47PM +0100, Stefan Klug wrote:\n> > Calling PixelFormat().toString() correctly returns \"0x0-<INVALID>\" 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> Sleeping over this made me wonder where/when you hit this warning. Is\n> there a valid use case to call this function with a default-constructed\n> pixel format ?\n\nAt least I can explain where it happens. Let's see if that is valid...\nThe V4L2M2MStream has a logPrefix() function which calls\nstream_->configuration().toString(). So when you log something early in\nthe lifetime of the converter (even if it is a debug message that\ndoesn't show up) that warning shows up. I was puzzled by this until I\nrealized it is just normal flow. And I see no better way to solve it.\n\nBest regards,\nStefan\n\n> \n> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  src/libcamera/formats.cpp | 3 +++\n> >  1 file changed, 3 insertions(+)\n> > \n> > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> > index bfcdfc08960d..b4518e61d04a 100644\n> > --- a/src/libcamera/formats.cpp\n> > +++ b/src/libcamera/formats.cpp\n> > @@ -1001,6 +1001,9 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{\n> >   */\n> >  const PixelFormatInfo &PixelFormatInfo::info(const PixelFormat &format)\n> >  {\n> > +\tif (!format.isValid())\n> > +\t\treturn pixelFormatInfoInvalid;\n> > +\n> >  \tconst auto iter = pixelFormatInfo.find(format);\n> >  \tif (iter == pixelFormatInfo.end()) {\n> >  \t\tLOG(Formats, Warning)\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 78614C3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 14 Feb 2025 16:42:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9768F6864F;\n\tFri, 14 Feb 2025 17:42:20 +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 544026862A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 14 Feb 2025 17:42:19 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:2ff9:b837:9e53:886e])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 068D3710;\n\tFri, 14 Feb 2025 17:40:59 +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=\"NFug9CEe\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1739551260;\n\tbh=UpU7D2j/+yetdTg+nLh3YjtDdxAyQ0yuwQqFxEtakBs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NFug9CEeGtXyGLBvoAMkDEgtyJd51ek0tnvjek6BXIRJFzfeEDH6NQxZHjYK2xAp7\n\tZ39oQiTbQO4lmEegKOETN4mePE6xm7j8pWuOCum0MjB2UKr5SSpNZAicu4y1VA/wTR\n\tBHb+WLczs8v2kB6CAwuiTfz19aowEvIAM15Mhr5M=","Date":"Fri, 14 Feb 2025 17:42:16 +0100","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, \n\tKieran Bingham <kieran.bingham@ideasonboard.com>","Subject":"Re: [PATCH v2 1/2] libcamera: formats: Silence warning when creating\n\ta PixelFormatInfo from a null format","Message-ID":"<4c3nuuqz5a3utjg753fzvrfxjw5crvqlhf6jc4kwtzviatdmu6@hdgddgmcg5sn>","References":"<20250214161031.240481-1-stefan.klug@ideasonboard.com>\n\t<20250214161031.240481-2-stefan.klug@ideasonboard.com>\n\t<20250214162733.GA17930@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250214162733.GA17930@pendragon.ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33362,"web_url":"https://patchwork.libcamera.org/comment/33362/","msgid":"<20250214165114.GE12632@pendragon.ideasonboard.com>","date":"2025-02-14T16:51:14","subject":"Re: [PATCH v2 1/2] libcamera: formats: Silence warning when creating\n\ta PixelFormatInfo 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 Fri, Feb 14, 2025 at 05:42:16PM +0100, Stefan Klug wrote:\n> On Fri, Feb 14, 2025 at 06:27:33PM +0200, Laurent Pinchart wrote:\n> > On Fri, Feb 14, 2025 at 05:09:47PM +0100, Stefan Klug wrote:\n> > > Calling PixelFormat().toString() correctly returns \"0x0-<INVALID>\" 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> > Sleeping over this made me wonder where/when you hit this warning. Is\n> > there a valid use case to call this function with a default-constructed\n> > pixel format ?\n> \n> At least I can explain where it happens. Let's see if that is valid...\n> The V4L2M2MStream has a logPrefix() function which calls\n> stream_->configuration().toString(). So when you log something early in\n> the lifetime of the converter (even if it is a debug message that\n> doesn't show up) that warning shows up. I was puzzled by this until I\n> realized it is just normal flow. And I see no better way to solve it.\n\nThis was introduced in\n\ncommit cc3a3c46a5ae4353b7bc9fe740521cef1008c998\nAuthor: Umang Jain <umang.jain@ideasonboard.com>\nDate:   Mon Jun 24 19:18:59 2024 +0530\n\n    libcamera: converter: Replace usage of stream index by Stream pointer\n\nwith\n\n std::string V4L2M2MConverter::V4L2M2MStream::logPrefix() const\n {\n-        return \"stream\" + std::to_string(index_);\n+        return stream_->configuration().toString();\n }\n\nUsage of the stream configuration there is not nice :-/ We could have\ntwo streams with the same configuration, and wouldn't be able to\ndifferentiate them in the logs. Also, the log identification of a stream\ncan change over the lifetime of the object. The log prefix is meant to\ndifferentiate between class instances. Can we do something better here ?\n\n> > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > ---\n> > >  src/libcamera/formats.cpp | 3 +++\n> > >  1 file changed, 3 insertions(+)\n> > > \n> > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> > > index bfcdfc08960d..b4518e61d04a 100644\n> > > --- a/src/libcamera/formats.cpp\n> > > +++ b/src/libcamera/formats.cpp\n> > > @@ -1001,6 +1001,9 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{\n> > >   */\n> > >  const PixelFormatInfo &PixelFormatInfo::info(const PixelFormat &format)\n> > >  {\n> > > +\tif (!format.isValid())\n> > > +\t\treturn pixelFormatInfoInvalid;\n> > > +\n> > >  \tconst auto iter = pixelFormatInfo.find(format);\n> > >  \tif (iter == pixelFormatInfo.end()) {\n> > >  \t\tLOG(Formats, Warning)","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 AF98AC32A3\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 14 Feb 2025 16:51:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C4C3168653;\n\tFri, 14 Feb 2025 17:51:27 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C428F68649\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 14 Feb 2025 17:51:26 +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 3BA3F710;\n\tFri, 14 Feb 2025 17:50: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=\"UcG9/2dV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1739551807;\n\tbh=3ksF3RhNsEqOKR1J6RoPddzuYFgmdMjxeHGf6gXgjjQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UcG9/2dVk8Xcev1hPfj9SVQTAFDjp0vqNplCo45EUiYBpfZIQuPtJwB9s+BI9qHlg\n\t1cwvDp6ZGE0W/ay/etgwe/8uaBnORdmOOxEwx53xW33gGP1eX30lsAqoAj8r6JpD4S\n\thU1SXAskTD4nSIxyeQzOyQxiF/7u5eMS+41NRvTs=","Date":"Fri, 14 Feb 2025 18:51:14 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>","Subject":"Re: [PATCH v2 1/2] libcamera: formats: Silence warning when creating\n\ta PixelFormatInfo from a null format","Message-ID":"<20250214165114.GE12632@pendragon.ideasonboard.com>","References":"<20250214161031.240481-1-stefan.klug@ideasonboard.com>\n\t<20250214161031.240481-2-stefan.klug@ideasonboard.com>\n\t<20250214162733.GA17930@pendragon.ideasonboard.com>\n\t<4c3nuuqz5a3utjg753fzvrfxjw5crvqlhf6jc4kwtzviatdmu6@hdgddgmcg5sn>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<4c3nuuqz5a3utjg753fzvrfxjw5crvqlhf6jc4kwtzviatdmu6@hdgddgmcg5sn>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]