[v1,2/3] treewide: Remove `libcamera::LOG(...)` occurences
diff mbox series

Message ID 20250303154844.745574-3-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • libcamera: base: log: Do not instantiate disabled `LogMessage`s
Related show

Commit Message

Barnabás Pőcze March 3, 2025, 3:48 p.m. UTC 
When a class inherits from `Loggable`, it will have a protected
`_log()` method and that will be used instead of the global
`_log()` method in the expansion of the `LOG()` macro. However,
if such a class has static member functions, then simply writing
`LOG()` will not work because name lookup will find the non-static
member function and no the global function, resulting in a compiler
error because the non-static member cannot be invoked without
an object, and there is no object in a static member function.

This can be avoided by using `using libcamera::_log;`, thereby
bringing the global declaration into the current scope.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/libcamera/sensor/camera_sensor_raw.cpp |  8 +++++---
 src/libcamera/v4l2_device.cpp              | 10 ++++++----
 2 files changed, 11 insertions(+), 7 deletions(-)

Comments

Laurent Pinchart March 3, 2025, 9:06 p.m. UTC | #1 
Hi Barnabás,

Thank you for the patch.

On Mon, Mar 03, 2025 at 04:48:43PM +0100, Barnabás Pőcze wrote:
> When a class inherits from `Loggable`, it will have a protected
> `_log()` method and that will be used instead of the global
> `_log()` method in the expansion of the `LOG()` macro. However,

s/method/function/

> if such a class has static member functions, then simply writing
> `LOG()` will not work because name lookup will find the non-static
> member function and no the global function, resulting in a compiler
> error because the non-static member cannot be invoked without
> an object, and there is no object in a static member function.
> 
> This can be avoided by using `using libcamera::_log;`, thereby
> bringing the global declaration into the current scope.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/libcamera/sensor/camera_sensor_raw.cpp |  8 +++++---
>  src/libcamera/v4l2_device.cpp              | 10 ++++++----
>  2 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp
> index ab75b1f82..b6f16f58a 100644
> --- a/src/libcamera/sensor/camera_sensor_raw.cpp
> +++ b/src/libcamera/sensor/camera_sensor_raw.cpp
> @@ -171,10 +171,12 @@ CameraSensorRaw::~CameraSensorRaw() = default;
>  std::variant<std::unique_ptr<CameraSensor>, int>
>  CameraSensorRaw::match(MediaEntity *entity)
>  {
> +	using libcamera::_log;
> +
>  	/* Check the entity type. */
>  	if (entity->type() != MediaEntity::Type::V4L2Subdevice ||
>  	    entity->function() != MEDIA_ENT_F_CAM_SENSOR) {
> -		libcamera::LOG(CameraSensor, Debug)
> +		LOG(CameraSensor, Debug)
>  			<< entity->name() << ": unsupported entity type ("
>  			<< utils::to_underlying(entity->type())
>  			<< ") or function (" << utils::hex(entity->function()) << ")";
> @@ -199,7 +201,7 @@ CameraSensorRaw::match(MediaEntity *entity)
>  			break;
>  
>  		default:
> -			libcamera::LOG(CameraSensor, Debug)
> +			LOG(CameraSensor, Debug)
>  				<< entity->name() << ": unsupported pad " << pad->index()
>  				<< " type " << utils::hex(pad->flags());
>  			return { 0 };
> @@ -207,7 +209,7 @@ CameraSensorRaw::match(MediaEntity *entity)
>  	}
>  
>  	if (numSinks < 1 || numSinks > 2 || numSources != 1) {
> -		libcamera::LOG(CameraSensor, Debug)
> +		LOG(CameraSensor, Debug)
>  			<< entity->name() << ": unsupported number of sinks ("
>  			<< numSinks << ") or sources (" << numSources << ")";
>  		return { 0 };
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 2f65a43a0..8fda38949 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -989,6 +989,8 @@ template std::optional<ColorSpace> V4L2Device::toColorSpace(const struct v4l2_mb
>  template<typename T>
>  int V4L2Device::fromColorSpace(const std::optional<ColorSpace> &colorSpace, T &v4l2Format)
>  {
> +	using libcamera::_log;
> +
>  	v4l2Format.colorspace = V4L2_COLORSPACE_DEFAULT;
>  	v4l2Format.xfer_func = V4L2_XFER_FUNC_DEFAULT;
>  	v4l2Format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> @@ -1017,7 +1019,7 @@ int V4L2Device::fromColorSpace(const std::optional<ColorSpace> &colorSpace, T &v
>  	if (itPrimaries != primariesToV4l2.end()) {
>  		v4l2Format.colorspace = itPrimaries->second;
>  	} else {
> -		libcamera::LOG(V4L2, Warning)
> +		LOG(V4L2, Warning)
>  			<< "Unrecognised primaries in "
>  			<< ColorSpace::toString(colorSpace);
>  		ret = -EINVAL;
> @@ -1027,7 +1029,7 @@ int V4L2Device::fromColorSpace(const std::optional<ColorSpace> &colorSpace, T &v
>  	if (itTransfer != transferFunctionToV4l2.end()) {
>  		v4l2Format.xfer_func = itTransfer->second;
>  	} else {
> -		libcamera::LOG(V4L2, Warning)
> +		LOG(V4L2, Warning)
>  			<< "Unrecognised transfer function in "
>  			<< ColorSpace::toString(colorSpace);
>  		ret = -EINVAL;
> @@ -1037,7 +1039,7 @@ int V4L2Device::fromColorSpace(const std::optional<ColorSpace> &colorSpace, T &v
>  	if (itYcbcrEncoding != ycbcrEncodingToV4l2.end()) {
>  		v4l2Format.ycbcr_enc = itYcbcrEncoding->second;
>  	} else {
> -		libcamera::LOG(V4L2, Warning)
> +		LOG(V4L2, Warning)
>  			<< "Unrecognised YCbCr encoding in "
>  			<< ColorSpace::toString(colorSpace);
>  		ret = -EINVAL;
> @@ -1047,7 +1049,7 @@ int V4L2Device::fromColorSpace(const std::optional<ColorSpace> &colorSpace, T &v
>  	if (itRange != rangeToV4l2.end()) {
>  		v4l2Format.quantization = itRange->second;
>  	} else {
> -		libcamera::LOG(V4L2, Warning)
> +		LOG(V4L2, Warning)
>  			<< "Unrecognised quantization in "
>  			<< ColorSpace::toString(colorSpace);
>  		ret = -EINVAL;
Kieran Bingham July 21, 2025, 6:40 p.m. UTC | #2 
Quoting Laurent Pinchart (2025-03-03 21:06:15)
> Hi Barnabás,
> 
> Thank you for the patch.
> 
> On Mon, Mar 03, 2025 at 04:48:43PM +0100, Barnabás Pőcze wrote:
> > When a class inherits from `Loggable`, it will have a protected
> > `_log()` method and that will be used instead of the global
> > `_log()` method in the expansion of the `LOG()` macro. However,
> 
> s/method/function/
> 
> > if such a class has static member functions, then simply writing
> > `LOG()` will not work because name lookup will find the non-static
> > member function and no the global function, resulting in a compiler
> > error because the non-static member cannot be invoked without
> > an object, and there is no object in a static member function.
> > 
> > This can be avoided by using `using libcamera::_log;`, thereby
> > bringing the global declaration into the current scope.
> > 
> > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > ---
> >  src/libcamera/sensor/camera_sensor_raw.cpp |  8 +++++---
> >  src/libcamera/v4l2_device.cpp              | 10 ++++++----
> >  2 files changed, 11 insertions(+), 7 deletions(-)
> > 
> > diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp
> > index ab75b1f82..b6f16f58a 100644
> > --- a/src/libcamera/sensor/camera_sensor_raw.cpp
> > +++ b/src/libcamera/sensor/camera_sensor_raw.cpp
> > @@ -171,10 +171,12 @@ CameraSensorRaw::~CameraSensorRaw() = default;
> >  std::variant<std::unique_ptr<CameraSensor>, int>
> >  CameraSensorRaw::match(MediaEntity *entity)
> >  {
> > +     using libcamera::_log;
> > +

This feels like a bit of magic that might need to be documented
somewhere - but I'm not sure where...

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> >       /* Check the entity type. */
> >       if (entity->type() != MediaEntity::Type::V4L2Subdevice ||
> >           entity->function() != MEDIA_ENT_F_CAM_SENSOR) {
> > -             libcamera::LOG(CameraSensor, Debug)
> > +             LOG(CameraSensor, Debug)
> >                       << entity->name() << ": unsupported entity type ("
> >                       << utils::to_underlying(entity->type())
> >                       << ") or function (" << utils::hex(entity->function()) << ")";
> > @@ -199,7 +201,7 @@ CameraSensorRaw::match(MediaEntity *entity)
> >                       break;
> >  
> >               default:
> > -                     libcamera::LOG(CameraSensor, Debug)
> > +                     LOG(CameraSensor, Debug)
> >                               << entity->name() << ": unsupported pad " << pad->index()
> >                               << " type " << utils::hex(pad->flags());
> >                       return { 0 };
> > @@ -207,7 +209,7 @@ CameraSensorRaw::match(MediaEntity *entity)
> >       }
> >  
> >       if (numSinks < 1 || numSinks > 2 || numSources != 1) {
> > -             libcamera::LOG(CameraSensor, Debug)
> > +             LOG(CameraSensor, Debug)
> >                       << entity->name() << ": unsupported number of sinks ("
> >                       << numSinks << ") or sources (" << numSources << ")";
> >               return { 0 };
> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > index 2f65a43a0..8fda38949 100644
> > --- a/src/libcamera/v4l2_device.cpp
> > +++ b/src/libcamera/v4l2_device.cpp
> > @@ -989,6 +989,8 @@ template std::optional<ColorSpace> V4L2Device::toColorSpace(const struct v4l2_mb
> >  template<typename T>
> >  int V4L2Device::fromColorSpace(const std::optional<ColorSpace> &colorSpace, T &v4l2Format)
> >  {
> > +     using libcamera::_log;
> > +
> >       v4l2Format.colorspace = V4L2_COLORSPACE_DEFAULT;
> >       v4l2Format.xfer_func = V4L2_XFER_FUNC_DEFAULT;
> >       v4l2Format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > @@ -1017,7 +1019,7 @@ int V4L2Device::fromColorSpace(const std::optional<ColorSpace> &colorSpace, T &v
> >       if (itPrimaries != primariesToV4l2.end()) {
> >               v4l2Format.colorspace = itPrimaries->second;
> >       } else {
> > -             libcamera::LOG(V4L2, Warning)
> > +             LOG(V4L2, Warning)
> >                       << "Unrecognised primaries in "
> >                       << ColorSpace::toString(colorSpace);
> >               ret = -EINVAL;
> > @@ -1027,7 +1029,7 @@ int V4L2Device::fromColorSpace(const std::optional<ColorSpace> &colorSpace, T &v
> >       if (itTransfer != transferFunctionToV4l2.end()) {
> >               v4l2Format.xfer_func = itTransfer->second;
> >       } else {
> > -             libcamera::LOG(V4L2, Warning)
> > +             LOG(V4L2, Warning)
> >                       << "Unrecognised transfer function in "
> >                       << ColorSpace::toString(colorSpace);
> >               ret = -EINVAL;
> > @@ -1037,7 +1039,7 @@ int V4L2Device::fromColorSpace(const std::optional<ColorSpace> &colorSpace, T &v
> >       if (itYcbcrEncoding != ycbcrEncodingToV4l2.end()) {
> >               v4l2Format.ycbcr_enc = itYcbcrEncoding->second;
> >       } else {
> > -             libcamera::LOG(V4L2, Warning)
> > +             LOG(V4L2, Warning)
> >                       << "Unrecognised YCbCr encoding in "
> >                       << ColorSpace::toString(colorSpace);
> >               ret = -EINVAL;
> > @@ -1047,7 +1049,7 @@ int V4L2Device::fromColorSpace(const std::optional<ColorSpace> &colorSpace, T &v
> >       if (itRange != rangeToV4l2.end()) {
> >               v4l2Format.quantization = itRange->second;
> >       } else {
> > -             libcamera::LOG(V4L2, Warning)
> > +             LOG(V4L2, Warning)
> >                       << "Unrecognised quantization in "
> >                       << ColorSpace::toString(colorSpace);
> >               ret = -EINVAL;
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp
index ab75b1f82..b6f16f58a 100644
--- a/src/libcamera/sensor/camera_sensor_raw.cpp
+++ b/src/libcamera/sensor/camera_sensor_raw.cpp
@@ -171,10 +171,12 @@  CameraSensorRaw::~CameraSensorRaw() = default;
 std::variant<std::unique_ptr<CameraSensor>, int>
 CameraSensorRaw::match(MediaEntity *entity)
 {
+	using libcamera::_log;
+
 	/* Check the entity type. */
 	if (entity->type() != MediaEntity::Type::V4L2Subdevice ||
 	    entity->function() != MEDIA_ENT_F_CAM_SENSOR) {
-		libcamera::LOG(CameraSensor, Debug)
+		LOG(CameraSensor, Debug)
 			<< entity->name() << ": unsupported entity type ("
 			<< utils::to_underlying(entity->type())
 			<< ") or function (" << utils::hex(entity->function()) << ")";
@@ -199,7 +201,7 @@  CameraSensorRaw::match(MediaEntity *entity)
 			break;
 
 		default:
-			libcamera::LOG(CameraSensor, Debug)
+			LOG(CameraSensor, Debug)
 				<< entity->name() << ": unsupported pad " << pad->index()
 				<< " type " << utils::hex(pad->flags());
 			return { 0 };
@@ -207,7 +209,7 @@  CameraSensorRaw::match(MediaEntity *entity)
 	}
 
 	if (numSinks < 1 || numSinks > 2 || numSources != 1) {
-		libcamera::LOG(CameraSensor, Debug)
+		LOG(CameraSensor, Debug)
 			<< entity->name() << ": unsupported number of sinks ("
 			<< numSinks << ") or sources (" << numSources << ")";
 		return { 0 };
diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
index 2f65a43a0..8fda38949 100644
--- a/src/libcamera/v4l2_device.cpp
+++ b/src/libcamera/v4l2_device.cpp
@@ -989,6 +989,8 @@  template std::optional<ColorSpace> V4L2Device::toColorSpace(const struct v4l2_mb
 template<typename T>
 int V4L2Device::fromColorSpace(const std::optional<ColorSpace> &colorSpace, T &v4l2Format)
 {
+	using libcamera::_log;
+
 	v4l2Format.colorspace = V4L2_COLORSPACE_DEFAULT;
 	v4l2Format.xfer_func = V4L2_XFER_FUNC_DEFAULT;
 	v4l2Format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
@@ -1017,7 +1019,7 @@  int V4L2Device::fromColorSpace(const std::optional<ColorSpace> &colorSpace, T &v
 	if (itPrimaries != primariesToV4l2.end()) {
 		v4l2Format.colorspace = itPrimaries->second;
 	} else {
-		libcamera::LOG(V4L2, Warning)
+		LOG(V4L2, Warning)
 			<< "Unrecognised primaries in "
 			<< ColorSpace::toString(colorSpace);
 		ret = -EINVAL;
@@ -1027,7 +1029,7 @@  int V4L2Device::fromColorSpace(const std::optional<ColorSpace> &colorSpace, T &v
 	if (itTransfer != transferFunctionToV4l2.end()) {
 		v4l2Format.xfer_func = itTransfer->second;
 	} else {
-		libcamera::LOG(V4L2, Warning)
+		LOG(V4L2, Warning)
 			<< "Unrecognised transfer function in "
 			<< ColorSpace::toString(colorSpace);
 		ret = -EINVAL;
@@ -1037,7 +1039,7 @@  int V4L2Device::fromColorSpace(const std::optional<ColorSpace> &colorSpace, T &v
 	if (itYcbcrEncoding != ycbcrEncodingToV4l2.end()) {
 		v4l2Format.ycbcr_enc = itYcbcrEncoding->second;
 	} else {
-		libcamera::LOG(V4L2, Warning)
+		LOG(V4L2, Warning)
 			<< "Unrecognised YCbCr encoding in "
 			<< ColorSpace::toString(colorSpace);
 		ret = -EINVAL;
@@ -1047,7 +1049,7 @@  int V4L2Device::fromColorSpace(const std::optional<ColorSpace> &colorSpace, T &v
 	if (itRange != rangeToV4l2.end()) {
 		v4l2Format.quantization = itRange->second;
 	} else {
-		libcamera::LOG(V4L2, Warning)
+		LOG(V4L2, Warning)
 			<< "Unrecognised quantization in "
 			<< ColorSpace::toString(colorSpace);
 		ret = -EINVAL;