[libcamera-devel] libcamera: Fix incorrect toString() to operator<<() conversions
diff mbox series

Message ID 20220530230045.5865-1-laurent.pinchart@ideasonboard.com
State Accepted
Commit 2094ac4f8cf7c437d4ce8e35eef749f24f6f551e
Headers show
Series
  • [libcamera-devel] libcamera: Fix incorrect toString() to operator<<() conversions
Related show

Commit Message

Laurent Pinchart May 30, 2022, 11 p.m. UTC 
Commit 8a845ab078c3 ("libcamera: Replace toString with operator<<() for
format classes") incorrectly converted some of the toString() usages,
resulting in pointer values being printed instead of formats. Fix it.

Fixes: 8a845ab078c3 ("libcamera: Replace toString with operator<<() for format classes")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/pipeline/ipu3/cio2.cpp     | 2 +-
 src/libcamera/pipeline/ipu3/imgu.cpp     | 4 ++--
 src/libcamera/pipeline/simple/simple.cpp | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)


base-commit: e115a691353151b3136581ac7f49cab6a3b0eb9a

Comments

Nicolas Dufresne via libcamera-devel May 31, 2022, 2:46 a.m. UTC | #1 
Hi Laurent,

On Tue, May 31, 2022 at 02:00:45AM +0300, Laurent Pinchart via libcamera-devel wrote:
> Commit 8a845ab078c3 ("libcamera: Replace toString with operator<<() for
> format classes") incorrectly converted some of the toString() usages,
> resulting in pointer values being printed instead of formats. Fix it.
> 
> Fixes: 8a845ab078c3 ("libcamera: Replace toString with operator<<() for format classes")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/libcamera/pipeline/ipu3/cio2.cpp     | 2 +-
>  src/libcamera/pipeline/ipu3/imgu.cpp     | 4 ++--
>  src/libcamera/pipeline/simple/simple.cpp | 4 ++--
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> index a4e4d302f841..08e254f75eee 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> @@ -211,7 +211,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)
>  	if (ret)
>  		return ret;
>  
> -	LOG(IPU3, Debug) << "CIO2 output format " << outputFormat;
> +	LOG(IPU3, Debug) << "CIO2 output format " << *outputFormat;
>  
>  	return 0;
>  }
> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
> index 34613feb8130..59305f85073c 100644
> --- a/src/libcamera/pipeline/ipu3/imgu.cpp
> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp
> @@ -479,7 +479,7 @@ int ImgUDevice::configure(const PipeConfig &pipeConfig, V4L2DeviceFormat *inputF
>  	if (ret)
>  		return ret;
>  
> -	LOG(IPU3, Debug) << "ImgU input format = " << inputFormat;
> +	LOG(IPU3, Debug) << "ImgU input format = " << *inputFormat;
>  
>  	/*
>  	 * \todo The IPU3 driver implementation shall be changed to use the
> @@ -568,7 +568,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad,
>  
>  	const char *name = dev == output_.get() ? "output" : "viewfinder";
>  	LOG(IPU3, Debug) << "ImgU " << name << " format = "
> -			 << outputFormat;
> +			 << *outputFormat;
>  
>  	return 0;
>  }
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 76bd228b5aba..de75465eb3b5 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -610,7 +610,7 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
>  					<< " produces " << sourceFormat
>  					<< ", sink '" << sink->entity()->name()
>  					<< "':" << sink->index()
> -					<< " requires " << format;
> +					<< " requires " << *format;
>  				return -EINVAL;
>  			}
>  		}
> @@ -620,7 +620,7 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
>  			<< "':" << source->index()
>  			<< " -> '" << sink->entity()->name()
>  			<< "':" << sink->index()
> -			<< " configured with format " << format;
> +			<< " configured with format " << *format;
>  	}
>  
>  	return 0;
> 
> base-commit: e115a691353151b3136581ac7f49cab6a3b0eb9a
Kieran Bingham May 31, 2022, 7:46 a.m. UTC | #2 
Quoting Laurent Pinchart via libcamera-devel (2022-05-31 00:00:45)
> Commit 8a845ab078c3 ("libcamera: Replace toString with operator<<() for
> format classes") incorrectly converted some of the toString() usages,
> resulting in pointer values being printed instead of formats. Fix it.
> 
> Fixes: 8a845ab078c3 ("libcamera: Replace toString with operator<<() for format classes")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Ooops

These look reasonable, but I haven't checked if there are more.

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


> ---
>  src/libcamera/pipeline/ipu3/cio2.cpp     | 2 +-
>  src/libcamera/pipeline/ipu3/imgu.cpp     | 4 ++--
>  src/libcamera/pipeline/simple/simple.cpp | 4 ++--
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> index a4e4d302f841..08e254f75eee 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> @@ -211,7 +211,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)
>         if (ret)
>                 return ret;
>  
> -       LOG(IPU3, Debug) << "CIO2 output format " << outputFormat;
> +       LOG(IPU3, Debug) << "CIO2 output format " << *outputFormat;
>  
>         return 0;
>  }
> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
> index 34613feb8130..59305f85073c 100644
> --- a/src/libcamera/pipeline/ipu3/imgu.cpp
> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp
> @@ -479,7 +479,7 @@ int ImgUDevice::configure(const PipeConfig &pipeConfig, V4L2DeviceFormat *inputF
>         if (ret)
>                 return ret;
>  
> -       LOG(IPU3, Debug) << "ImgU input format = " << inputFormat;
> +       LOG(IPU3, Debug) << "ImgU input format = " << *inputFormat;
>  
>         /*
>          * \todo The IPU3 driver implementation shall be changed to use the
> @@ -568,7 +568,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad,
>  
>         const char *name = dev == output_.get() ? "output" : "viewfinder";
>         LOG(IPU3, Debug) << "ImgU " << name << " format = "
> -                        << outputFormat;
> +                        << *outputFormat;
>  
>         return 0;
>  }
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 76bd228b5aba..de75465eb3b5 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -610,7 +610,7 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
>                                         << " produces " << sourceFormat
>                                         << ", sink '" << sink->entity()->name()
>                                         << "':" << sink->index()
> -                                       << " requires " << format;
> +                                       << " requires " << *format;
>                                 return -EINVAL;
>                         }
>                 }
> @@ -620,7 +620,7 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
>                         << "':" << source->index()
>                         << " -> '" << sink->entity()->name()
>                         << "':" << sink->index()
> -                       << " configured with format " << format;
> +                       << " configured with format " << *format;
>         }
>  
>         return 0;
> 
> base-commit: e115a691353151b3136581ac7f49cab6a3b0eb9a
> -- 
> Regards,
> 
> Laurent Pinchart
>
Laurent Pinchart June 1, 2022, 8:06 a.m. UTC | #3 
On Tue, May 31, 2022 at 08:46:50AM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart via libcamera-devel (2022-05-31 00:00:45)
> > Commit 8a845ab078c3 ("libcamera: Replace toString with operator<<() for
> > format classes") incorrectly converted some of the toString() usages,
> > resulting in pointer values being printed instead of formats. Fix it.
> > 
> > Fixes: 8a845ab078c3 ("libcamera: Replace toString with operator<<() for format classes")
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Ooops
> 
> These look reasonable, but I haven't checked if there are more.

Not to my knowledge :-)

This is one of the drawbacks of the conversion to operator<<() I
suppose, these issues are not caught by the compiler.

> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > ---
> >  src/libcamera/pipeline/ipu3/cio2.cpp     | 2 +-
> >  src/libcamera/pipeline/ipu3/imgu.cpp     | 4 ++--
> >  src/libcamera/pipeline/simple/simple.cpp | 4 ++--
> >  3 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> > index a4e4d302f841..08e254f75eee 100644
> > --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> > @@ -211,7 +211,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)
> >         if (ret)
> >                 return ret;
> >  
> > -       LOG(IPU3, Debug) << "CIO2 output format " << outputFormat;
> > +       LOG(IPU3, Debug) << "CIO2 output format " << *outputFormat;
> >  
> >         return 0;
> >  }
> > diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
> > index 34613feb8130..59305f85073c 100644
> > --- a/src/libcamera/pipeline/ipu3/imgu.cpp
> > +++ b/src/libcamera/pipeline/ipu3/imgu.cpp
> > @@ -479,7 +479,7 @@ int ImgUDevice::configure(const PipeConfig &pipeConfig, V4L2DeviceFormat *inputF
> >         if (ret)
> >                 return ret;
> >  
> > -       LOG(IPU3, Debug) << "ImgU input format = " << inputFormat;
> > +       LOG(IPU3, Debug) << "ImgU input format = " << *inputFormat;
> >  
> >         /*
> >          * \todo The IPU3 driver implementation shall be changed to use the
> > @@ -568,7 +568,7 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad,
> >  
> >         const char *name = dev == output_.get() ? "output" : "viewfinder";
> >         LOG(IPU3, Debug) << "ImgU " << name << " format = "
> > -                        << outputFormat;
> > +                        << *outputFormat;
> >  
> >         return 0;
> >  }
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index 76bd228b5aba..de75465eb3b5 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -610,7 +610,7 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
> >                                         << " produces " << sourceFormat
> >                                         << ", sink '" << sink->entity()->name()
> >                                         << "':" << sink->index()
> > -                                       << " requires " << format;
> > +                                       << " requires " << *format;
> >                                 return -EINVAL;
> >                         }
> >                 }
> > @@ -620,7 +620,7 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
> >                         << "':" << source->index()
> >                         << " -> '" << sink->entity()->name()
> >                         << "':" << sink->index()
> > -                       << " configured with format " << format;
> > +                       << " configured with format " << *format;
> >         }
> >  
> >         return 0;
> > 
> > base-commit: e115a691353151b3136581ac7f49cab6a3b0eb9a

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
index a4e4d302f841..08e254f75eee 100644
--- a/src/libcamera/pipeline/ipu3/cio2.cpp
+++ b/src/libcamera/pipeline/ipu3/cio2.cpp
@@ -211,7 +211,7 @@  int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)
 	if (ret)
 		return ret;
 
-	LOG(IPU3, Debug) << "CIO2 output format " << outputFormat;
+	LOG(IPU3, Debug) << "CIO2 output format " << *outputFormat;
 
 	return 0;
 }
diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
index 34613feb8130..59305f85073c 100644
--- a/src/libcamera/pipeline/ipu3/imgu.cpp
+++ b/src/libcamera/pipeline/ipu3/imgu.cpp
@@ -479,7 +479,7 @@  int ImgUDevice::configure(const PipeConfig &pipeConfig, V4L2DeviceFormat *inputF
 	if (ret)
 		return ret;
 
-	LOG(IPU3, Debug) << "ImgU input format = " << inputFormat;
+	LOG(IPU3, Debug) << "ImgU input format = " << *inputFormat;
 
 	/*
 	 * \todo The IPU3 driver implementation shall be changed to use the
@@ -568,7 +568,7 @@  int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad,
 
 	const char *name = dev == output_.get() ? "output" : "viewfinder";
 	LOG(IPU3, Debug) << "ImgU " << name << " format = "
-			 << outputFormat;
+			 << *outputFormat;
 
 	return 0;
 }
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 76bd228b5aba..de75465eb3b5 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -610,7 +610,7 @@  int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
 					<< " produces " << sourceFormat
 					<< ", sink '" << sink->entity()->name()
 					<< "':" << sink->index()
-					<< " requires " << format;
+					<< " requires " << *format;
 				return -EINVAL;
 			}
 		}
@@ -620,7 +620,7 @@  int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
 			<< "':" << source->index()
 			<< " -> '" << sink->entity()->name()
 			<< "':" << sink->index()
-			<< " configured with format " << format;
+			<< " configured with format " << *format;
 	}
 
 	return 0;