[{"id":33555,"web_url":"https://patchwork.libcamera.org/comment/33555/","msgid":"<20250303210615.GB30583@pendragon.ideasonboard.com>","date":"2025-03-03T21:06:15","subject":"Re: [PATCH v1 2/3] treewide: Remove `libcamera::LOG(...)` occurences","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Barnabás,\n\nThank you for the patch.\n\nOn Mon, Mar 03, 2025 at 04:48:43PM +0100, Barnabás Pőcze wrote:\n> When a class inherits from `Loggable`, it will have a protected\n> `_log()` method and that will be used instead of the global\n> `_log()` method in the expansion of the `LOG()` macro. However,\n\ns/method/function/\n\n> if such a class has static member functions, then simply writing\n> `LOG()` will not work because name lookup will find the non-static\n> member function and no the global function, resulting in a compiler\n> error because the non-static member cannot be invoked without\n> an object, and there is no object in a static member function.\n> \n> This can be avoided by using `using libcamera::_log;`, thereby\n> bringing the global declaration into the current scope.\n> \n> Signed-off-by: Barnabás Pőcze \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/libcamera/sensor/camera_sensor_raw.cpp | 8 +++++---\n> src/libcamera/v4l2_device.cpp | 10 ++++++----\n> 2 files changed, 11 insertions(+), 7 deletions(-)\n> \n> diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp\n> index ab75b1f82..b6f16f58a 100644\n> --- a/src/libcamera/sensor/camera_sensor_raw.cpp\n> +++ b/src/libcamera/sensor/camera_sensor_raw.cpp\n> @@ -171,10 +171,12 @@ CameraSensorRaw::~CameraSensorRaw() = default;\n> std::variant, int>\n> CameraSensorRaw::match(MediaEntity *entity)\n> {\n> +\tusing libcamera::_log;\n> +\n> \t/* Check the entity type. */\n> \tif (entity->type() != MediaEntity::Type::V4L2Subdevice ||\n> \t entity->function() != MEDIA_ENT_F_CAM_SENSOR) {\n> -\t\tlibcamera::LOG(CameraSensor, Debug)\n> +\t\tLOG(CameraSensor, Debug)\n> \t\t\t<< entity->name() << \": unsupported entity type (\"\n> \t\t\t<< utils::to_underlying(entity->type())\n> \t\t\t<< \") or function (\" << utils::hex(entity->function()) << \")\";\n> @@ -199,7 +201,7 @@ CameraSensorRaw::match(MediaEntity *entity)\n> \t\t\tbreak;\n> \n> \t\tdefault:\n> -\t\t\tlibcamera::LOG(CameraSensor, Debug)\n> +\t\t\tLOG(CameraSensor, Debug)\n> \t\t\t\t<< entity->name() << \": unsupported pad \" << pad->index()\n> \t\t\t\t<< \" type \" << utils::hex(pad->flags());\n> \t\t\treturn { 0 };\n> @@ -207,7 +209,7 @@ CameraSensorRaw::match(MediaEntity *entity)\n> \t}\n> \n> \tif (numSinks < 1 || numSinks > 2 || numSources != 1) {\n> -\t\tlibcamera::LOG(CameraSensor, Debug)\n> +\t\tLOG(CameraSensor, Debug)\n> \t\t\t<< entity->name() << \": unsupported number of sinks (\"\n> \t\t\t<< numSinks << \") or sources (\" << numSources << \")\";\n> \t\treturn { 0 };\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 2f65a43a0..8fda38949 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -989,6 +989,8 @@ template std::optional V4L2Device::toColorSpace(const struct v4l2_mb\n> template\n> int V4L2Device::fromColorSpace(const std::optional &colorSpace, T &v4l2Format)\n> {\n> +\tusing libcamera::_log;\n> +\n> \tv4l2Format.colorspace = V4L2_COLORSPACE_DEFAULT;\n> \tv4l2Format.xfer_func = V4L2_XFER_FUNC_DEFAULT;\n> \tv4l2Format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;\n> @@ -1017,7 +1019,7 @@ int V4L2Device::fromColorSpace(const std::optional &colorSpace, T &v\n> \tif (itPrimaries != primariesToV4l2.end()) {\n> \t\tv4l2Format.colorspace = itPrimaries->second;\n> \t} else {\n> -\t\tlibcamera::LOG(V4L2, Warning)\n> +\t\tLOG(V4L2, Warning)\n> \t\t\t<< \"Unrecognised primaries in \"\n> \t\t\t<< ColorSpace::toString(colorSpace);\n> \t\tret = -EINVAL;\n> @@ -1027,7 +1029,7 @@ int V4L2Device::fromColorSpace(const std::optional &colorSpace, T &v\n> \tif (itTransfer != transferFunctionToV4l2.end()) {\n> \t\tv4l2Format.xfer_func = itTransfer->second;\n> \t} else {\n> -\t\tlibcamera::LOG(V4L2, Warning)\n> +\t\tLOG(V4L2, Warning)\n> \t\t\t<< \"Unrecognised transfer function in \"\n> \t\t\t<< ColorSpace::toString(colorSpace);\n> \t\tret = -EINVAL;\n> @@ -1037,7 +1039,7 @@ int V4L2Device::fromColorSpace(const std::optional &colorSpace, T &v\n> \tif (itYcbcrEncoding != ycbcrEncodingToV4l2.end()) {\n> \t\tv4l2Format.ycbcr_enc = itYcbcrEncoding->second;\n> \t} else {\n> -\t\tlibcamera::LOG(V4L2, Warning)\n> +\t\tLOG(V4L2, Warning)\n> \t\t\t<< \"Unrecognised YCbCr encoding in \"\n> \t\t\t<< ColorSpace::toString(colorSpace);\n> \t\tret = -EINVAL;\n> @@ -1047,7 +1049,7 @@ int V4L2Device::fromColorSpace(const std::optional &colorSpace, T &v\n> \tif (itRange != rangeToV4l2.end()) {\n> \t\tv4l2Format.quantization = itRange->second;\n> \t} else {\n> -\t\tlibcamera::LOG(V4L2, Warning)\n> +\t\tLOG(V4L2, Warning)\n> \t\t\t<< \"Unrecognised quantization in \"\n> \t\t\t<< ColorSpace::toString(colorSpace);\n> \t\tret = -EINVAL;","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 49EE9C3257\n\tfor ;\n\tMon, 3 Mar 2025 21:06:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 033F768779;\n\tMon, 3 Mar 2025 22:06:39 +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 F3D1568772\n\tfor ;\n\tMon, 3 Mar 2025 22:06:36 +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 4CF56346;\n\tMon, 3 Mar 2025 22:05:05 +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=\"n6gIuG4D\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1741035905;\n\tbh=wB48CjgShKzSQtSGPhO2BhhZ5j0WP/aZecd91VDEzAo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=n6gIuG4D2/2pt+0DsC1OvzI2u+us+fWEobEQEDWjkonCuIH90Vc6+4CTvSobDzi+3\n\tli3m3qeyU9OPcACsXXdtdZoIhvvdO75AlIec9cvX9NkyjA/llFmbtKDYhA0pQ/q5s2\n\tZ4an50UaNOpfUfSZLahPS2F1nqdOZVpoJS2SLkSI=","Date":"Mon, 3 Mar 2025 23:06:15 +0200","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 2/3] treewide: Remove `libcamera::LOG(...)` occurences","Message-ID":"<20250303210615.GB30583@pendragon.ideasonboard.com>","References":"<20250303154844.745574-1-barnabas.pocze@ideasonboard.com>\n\t<20250303154844.745574-3-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250303154844.745574-3-barnabas.pocze@ideasonboard.com>","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":34986,"web_url":"https://patchwork.libcamera.org/comment/34986/","msgid":"<175312324095.50296.9885208236478905255@ping.linuxembedded.co.uk>","date":"2025-07-21T18:40:40","subject":"Re: [PATCH v1 2/3] treewide: Remove `libcamera::LOG(...)` occurences","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2025-03-03 21:06:15)\n> Hi Barnabás,\n> \n> Thank you for the patch.\n> \n> On Mon, Mar 03, 2025 at 04:48:43PM +0100, Barnabás Pőcze wrote:\n> > When a class inherits from `Loggable`, it will have a protected\n> > `_log()` method and that will be used instead of the global\n> > `_log()` method in the expansion of the `LOG()` macro. However,\n> \n> s/method/function/\n> \n> > if such a class has static member functions, then simply writing\n> > `LOG()` will not work because name lookup will find the non-static\n> > member function and no the global function, resulting in a compiler\n> > error because the non-static member cannot be invoked without\n> > an object, and there is no object in a static member function.\n> > \n> > This can be avoided by using `using libcamera::_log;`, thereby\n> > bringing the global declaration into the current scope.\n> > \n> > Signed-off-by: Barnabás Pőcze \n> \n> Reviewed-by: Laurent Pinchart \n> \n> > ---\n> > src/libcamera/sensor/camera_sensor_raw.cpp | 8 +++++---\n> > src/libcamera/v4l2_device.cpp | 10 ++++++----\n> > 2 files changed, 11 insertions(+), 7 deletions(-)\n> > \n> > diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp\n> > index ab75b1f82..b6f16f58a 100644\n> > --- a/src/libcamera/sensor/camera_sensor_raw.cpp\n> > +++ b/src/libcamera/sensor/camera_sensor_raw.cpp\n> > @@ -171,10 +171,12 @@ CameraSensorRaw::~CameraSensorRaw() = default;\n> > std::variant, int>\n> > CameraSensorRaw::match(MediaEntity *entity)\n> > {\n> > + using libcamera::_log;\n> > +\n\nThis feels like a bit of magic that might need to be documented\nsomewhere - but I'm not sure where...\n\nReviewed-by: Kieran Bingham \n\n> > /* Check the entity type. */\n> > if (entity->type() != MediaEntity::Type::V4L2Subdevice ||\n> > entity->function() != MEDIA_ENT_F_CAM_SENSOR) {\n> > - libcamera::LOG(CameraSensor, Debug)\n> > + LOG(CameraSensor, Debug)\n> > << entity->name() << \": unsupported entity type (\"\n> > << utils::to_underlying(entity->type())\n> > << \") or function (\" << utils::hex(entity->function()) << \")\";\n> > @@ -199,7 +201,7 @@ CameraSensorRaw::match(MediaEntity *entity)\n> > break;\n> > \n> > default:\n> > - libcamera::LOG(CameraSensor, Debug)\n> > + LOG(CameraSensor, Debug)\n> > << entity->name() << \": unsupported pad \" << pad->index()\n> > << \" type \" << utils::hex(pad->flags());\n> > return { 0 };\n> > @@ -207,7 +209,7 @@ CameraSensorRaw::match(MediaEntity *entity)\n> > }\n> > \n> > if (numSinks < 1 || numSinks > 2 || numSources != 1) {\n> > - libcamera::LOG(CameraSensor, Debug)\n> > + LOG(CameraSensor, Debug)\n> > << entity->name() << \": unsupported number of sinks (\"\n> > << numSinks << \") or sources (\" << numSources << \")\";\n> > return { 0 };\n> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > index 2f65a43a0..8fda38949 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -989,6 +989,8 @@ template std::optional V4L2Device::toColorSpace(const struct v4l2_mb\n> > template\n> > int V4L2Device::fromColorSpace(const std::optional &colorSpace, T &v4l2Format)\n> > {\n> > + using libcamera::_log;\n> > +\n> > v4l2Format.colorspace = V4L2_COLORSPACE_DEFAULT;\n> > v4l2Format.xfer_func = V4L2_XFER_FUNC_DEFAULT;\n> > v4l2Format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;\n> > @@ -1017,7 +1019,7 @@ int V4L2Device::fromColorSpace(const std::optional &colorSpace, T &v\n> > if (itPrimaries != primariesToV4l2.end()) {\n> > v4l2Format.colorspace = itPrimaries->second;\n> > } else {\n> > - libcamera::LOG(V4L2, Warning)\n> > + LOG(V4L2, Warning)\n> > << \"Unrecognised primaries in \"\n> > << ColorSpace::toString(colorSpace);\n> > ret = -EINVAL;\n> > @@ -1027,7 +1029,7 @@ int V4L2Device::fromColorSpace(const std::optional &colorSpace, T &v\n> > if (itTransfer != transferFunctionToV4l2.end()) {\n> > v4l2Format.xfer_func = itTransfer->second;\n> > } else {\n> > - libcamera::LOG(V4L2, Warning)\n> > + LOG(V4L2, Warning)\n> > << \"Unrecognised transfer function in \"\n> > << ColorSpace::toString(colorSpace);\n> > ret = -EINVAL;\n> > @@ -1037,7 +1039,7 @@ int V4L2Device::fromColorSpace(const std::optional &colorSpace, T &v\n> > if (itYcbcrEncoding != ycbcrEncodingToV4l2.end()) {\n> > v4l2Format.ycbcr_enc = itYcbcrEncoding->second;\n> > } else {\n> > - libcamera::LOG(V4L2, Warning)\n> > + LOG(V4L2, Warning)\n> > << \"Unrecognised YCbCr encoding in \"\n> > << ColorSpace::toString(colorSpace);\n> > ret = -EINVAL;\n> > @@ -1047,7 +1049,7 @@ int V4L2Device::fromColorSpace(const std::optional &colorSpace, T &v\n> > if (itRange != rangeToV4l2.end()) {\n> > v4l2Format.quantization = itRange->second;\n> > } else {\n> > - libcamera::LOG(V4L2, Warning)\n> > + LOG(V4L2, Warning)\n> > << \"Unrecognised quantization in \"\n> > << ColorSpace::toString(colorSpace);\n> > ret = -EINVAL;\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 AD036C3237\n\tfor ;\n\tMon, 21 Jul 2025 18:40:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E268069000;\n\tMon, 21 Jul 2025 20:40:45 +0200 (CEST)","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 E2B3368FB1\n\tfor ;\n\tMon, 21 Jul 2025 20:40:43 +0200 (CEST)","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 98AB67917;\n\tMon, 21 Jul 2025 20:40:06 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"rwEBfdWb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753123206;\n\tbh=Mo5LGcis9CklwS7+vACxBwqerUs2x2EbGrOfgWycdX0=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=rwEBfdWblpyW6/9nCRDEzvonumFTC3dQKnI6Xty4iWKj3aH0ZFI31254sIJhyXilP\n\tXUSDTWOO2VgA7vnYiqzj1reLRrG3O6LL+ApckUdPo88B2ifVLfSYNxNsnhi77wmZjv\n\tGf3Ii7r/9oCuZzocoWkbwXJvjUUT10cgHIzbt2qg=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250303210615.GB30583@pendragon.ideasonboard.com>","References":"<20250303154844.745574-1-barnabas.pocze@ideasonboard.com>\n\t<20250303154844.745574-3-barnabas.pocze@ideasonboard.com>\n\t<20250303210615.GB30583@pendragon.ideasonboard.com>","Subject":"Re: [PATCH v1 2/3] treewide: Remove `libcamera::LOG(...)` occurences","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tLaurent Pinchart ","Date":"Mon, 21 Jul 2025 19:40:40 +0100","Message-ID":"<175312324095.50296.9885208236478905255@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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\" "}}]