[libcamera-devel,PATCH/RFC,01/12] libcamera: camera: Fix std::ostringstream initialisation

Message ID 20190517230621.24668-2-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • Rework camera configuration to introduce negotiation of parameters
Related show

Commit Message

Laurent Pinchart May 17, 2019, 11:06 p.m. UTC
We use the std::ostringstream class to generate log messages in the
Camera class. The stream is initialised with initial content, but is not
opened without seeking to the end, which results in the content being
overwritten immediately. Fix it by opening the stream with
std::ios_base::ate.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/camera.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Niklas Söderlund May 18, 2019, 3:33 p.m. UTC | #1
Hi Laurent,

Thanks for your work.

On 2019-05-18 02:06:10 +0300, Laurent Pinchart wrote:
> We use the std::ostringstream class to generate log messages in the
> Camera class. The stream is initialised with initial content, but is not
> opened without seeking to the end, which results in the content being
> overwritten immediately. Fix it by opening the stream with
> std::ios_base::ate.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/libcamera/camera.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index fbc66dedba51..1a21acac9899 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -560,7 +560,7 @@ Camera::streamConfiguration(const std::vector<StreamUsage> &usages)
>  
>  	CameraConfiguration config = pipe_->streamConfiguration(this, usages);
>  
> -	std::ostringstream msg("streams configuration:");
> +	std::ostringstream msg("streams configuration:", std::ios_base::ate);
>  	unsigned int index = 0;
>  
>  	for (Stream *stream : config) {
> @@ -614,7 +614,7 @@ int Camera::configureStreams(const CameraConfiguration &config)
>  		return -EINVAL;
>  	}
>  
> -	std::ostringstream msg("configuring streams:");
> +	std::ostringstream msg("configuring streams:", std::ios_base::ate);
>  	unsigned int index = 0;
>  
>  	for (Stream *stream : config) {
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Kieran Bingham May 19, 2019, 10:39 a.m. UTC | #2
Hi Laurent,

On 18/05/2019 00:06, Laurent Pinchart wrote:
> We use the std::ostringstream class to generate log messages in the
> Camera class. The stream is initialised with initial content, but is not
> opened without seeking to the end, which results in the content being
> overwritten immediately. Fix it by opening the stream with
> std::ios_base::ate.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/camera.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index fbc66dedba51..1a21acac9899 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -560,7 +560,7 @@ Camera::streamConfiguration(const std::vector<StreamUsage> &usages)
>  
>  	CameraConfiguration config = pipe_->streamConfiguration(this, usages);
>  
> -	std::ostringstream msg("streams configuration:");
> +	std::ostringstream msg("streams configuration:", std::ios_base::ate);

>  	unsigned int index = 0;
>  
>  	for (Stream *stream : config) {
> @@ -614,7 +614,7 @@ int Camera::configureStreams(const CameraConfiguration &config)
>  		return -EINVAL;
>  	}
>  
> -	std::ostringstream msg("configuring streams:");
> +	std::ostringstream msg("configuring streams:", std::ios_base::ate);
>  	unsigned int index = 0;
>  
>  	for (Stream *stream : config) {

Should all of this be moved to the CameraConfiguration class as a
toString() or such? or will this be the only place we print a
CameraConfiguration...

>
Laurent Pinchart May 19, 2019, 2:50 p.m. UTC | #3
Hi Kieran,

On Sun, May 19, 2019 at 11:39:42AM +0100, Kieran Bingham wrote:
> On 18/05/2019 00:06, Laurent Pinchart wrote:
> > We use the std::ostringstream class to generate log messages in the
> > Camera class. The stream is initialised with initial content, but is not
> > opened without seeking to the end, which results in the content being
> > overwritten immediately. Fix it by opening the stream with
> > std::ios_base::ate.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/libcamera/camera.cpp | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index fbc66dedba51..1a21acac9899 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -560,7 +560,7 @@ Camera::streamConfiguration(const std::vector<StreamUsage> &usages)
> >  
> >  	CameraConfiguration config = pipe_->streamConfiguration(this, usages);
> >  
> > -	std::ostringstream msg("streams configuration:");
> > +	std::ostringstream msg("streams configuration:", std::ios_base::ate);
> 
> >  	unsigned int index = 0;
> >  
> >  	for (Stream *stream : config) {
> > @@ -614,7 +614,7 @@ int Camera::configureStreams(const CameraConfiguration &config)
> >  		return -EINVAL;
> >  	}
> >  
> > -	std::ostringstream msg("configuring streams:");
> > +	std::ostringstream msg("configuring streams:", std::ios_base::ate);
> >  	unsigned int index = 0;
> >  
> >  	for (Stream *stream : config) {
> 
> Should all of this be moved to the CameraConfiguration class as a
> toString() or such? or will this be the only place we print a
> CameraConfiguration...

If we end up printing it somewhere else then a toString() method would
be useful, I agree. When objects get more complex, however, I'm not sure
if we can come up with a single string representation that would fit all
needs, but we can try.

Patch

diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index fbc66dedba51..1a21acac9899 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -560,7 +560,7 @@  Camera::streamConfiguration(const std::vector<StreamUsage> &usages)
 
 	CameraConfiguration config = pipe_->streamConfiguration(this, usages);
 
-	std::ostringstream msg("streams configuration:");
+	std::ostringstream msg("streams configuration:", std::ios_base::ate);
 	unsigned int index = 0;
 
 	for (Stream *stream : config) {
@@ -614,7 +614,7 @@  int Camera::configureStreams(const CameraConfiguration &config)
 		return -EINVAL;
 	}
 
-	std::ostringstream msg("configuring streams:");
+	std::ostringstream msg("configuring streams:", std::ios_base::ate);
 	unsigned int index = 0;
 
 	for (Stream *stream : config) {