[libcamera-devel,v3,26/30] qcam: Print bytesused for all planes

13692 diff mbox series

Fix the debug message that prints frame metadata to print the number of
bytes used for each plane, not just the first one.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
---
 src/qcam/main_window.cpp | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

On 06/09/2021 23:56, Laurent Pinchart wrote:
> Fix the debug message that prints frame metadata to print the number of
> bytes used for each plane, not just the first one.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> ---
>  src/qcam/main_window.cpp | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index ac853e360aea..0a00b1001570 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -20,6 +20,7 @@
>  #include <QInputDialog>
>  #include <QMutexLocker>
>  #include <QStandardPaths>
> +#include <QStringList>
>  #include <QTimer>
>  #include <QToolBar>
>  #include <QToolButton>
> @@ -754,9 +755,13 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)
>  	fps = lastBufferTime_ && fps ? 1000000000.0 / fps : 0.0;
>  	lastBufferTime_ = metadata.timestamp;
>  
> +	QStringList bytesused;
> +	for (const FrameMetadata::Plane &plane : metadata.planes())
> +		bytesused << QString::number(plane.bytesused);
> +
>  	qDebug().noquote()
>  		<< QString("seq: %1").arg(metadata.sequence, 6, 10, QLatin1Char('0'))
> -		<< "bytesused:" << metadata.planes()[0].bytesused
> +		<< "bytesused:" << bytesused.join("/")

A bit weary that this will look out of place for two planes.

	bytesused: 1500/500

Might look like a buffer overflow (1500 used from 500).

But ... it may not be an issue and maybe no one will complain.
Otherwise,
	bytesused: 1500, 500

or
	bytesused: {1500, 500}
?

But either way,

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

>  		<< "timestamp:" << metadata.timestamp
>  		<< "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps;
>  
>

Hi Kieran,

On Tue, Sep 07, 2021 at 12:53:56PM +0100, Kieran Bingham wrote:
> On 06/09/2021 23:56, Laurent Pinchart wrote:
> > Fix the debug message that prints frame metadata to print the number of
> > bytes used for each plane, not just the first one.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> > ---
> >  src/qcam/main_window.cpp | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> > index ac853e360aea..0a00b1001570 100644
> > --- a/src/qcam/main_window.cpp
> > +++ b/src/qcam/main_window.cpp
> > @@ -20,6 +20,7 @@
> >  #include <QInputDialog>
> >  #include <QMutexLocker>
> >  #include <QStandardPaths>
> > +#include <QStringList>
> >  #include <QTimer>
> >  #include <QToolBar>
> >  #include <QToolButton>
> > @@ -754,9 +755,13 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)
> >  	fps = lastBufferTime_ && fps ? 1000000000.0 / fps : 0.0;
> >  	lastBufferTime_ = metadata.timestamp;
> >  
> > +	QStringList bytesused;
> > +	for (const FrameMetadata::Plane &plane : metadata.planes())
> > +		bytesused << QString::number(plane.bytesused);
> > +
> >  	qDebug().noquote()
> >  		<< QString("seq: %1").arg(metadata.sequence, 6, 10, QLatin1Char('0'))
> > -		<< "bytesused:" << metadata.planes()[0].bytesused
> > +		<< "bytesused:" << bytesused.join("/")
> 
> A bit weary that this will look out of place for two planes.
> 
> 	bytesused: 1500/500
> 
> Might look like a buffer overflow (1500 used from 500).
> 
> But ... it may not be an issue and maybe no one will complain.
> Otherwise,
> 	bytesused: 1500, 500
> 
> or
> 	bytesused: {1500, 500}
> ?

Good point. I can easily do

	bytesused: { 1500, 500 }

as qDebug() adds spaces automatically.

> But either way,
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> >  		<< "timestamp:" << metadata.timestamp
> >  		<< "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps;
> >  
> >