[libcamera-devel] qcam: viewfinder: Print message to report format converter usage

3295 diff mbox series

Print an info message when initializing the viewfinder to report if the
format converter is used or if zero-copy is enabled. This is useful to
notify of a possible impact on performances.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/qcam/viewfinder.cpp | 5 +++++
 1 file changed, 5 insertions(+)

Hi Laurent,

On 24/03/2020 09:41, Laurent Pinchart wrote:
> Print an info message when initializing the viewfinder to report if the
> format converter is used or if zero-copy is enabled. This is useful to
> notify of a possible impact on performances.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/qcam/viewfinder.cpp | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/qcam/viewfinder.cpp b/src/qcam/viewfinder.cpp
> index c7c6d099dfba..df5ee5ee7b83 100644
> --- a/src/qcam/viewfinder.cpp
> +++ b/src/qcam/viewfinder.cpp
> @@ -56,6 +56,11 @@ int ViewFinder::setFormat(const libcamera::PixelFormat &format,
>  			return ret;
>  
>  		image_ = QImage(size, QImage::Format_RGB32);
> +
> +		qInfo() << "Using software format conversion from"
> +			<< format.toString().c_str();


Should this message state the conversion from and to?

Perhaps it doesn't matter too much at the moment as we always convert to
the same destination format - but perhaps that might differ if we have
multiple compatible 'native' formats?

> +	} else {
> +		qInfo() << "Zero-copy enabled";

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

>  	}
>  
>  	format_ = format;
>

On Tue, Mar 24, 2020 at 11:57:52AM +0000, Kieran Bingham wrote:
> Hi Laurent,
> 
> On 24/03/2020 09:41, Laurent Pinchart wrote:
> > Print an info message when initializing the viewfinder to report if the
> > format converter is used or if zero-copy is enabled. This is useful to
> > notify of a possible impact on performances.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/qcam/viewfinder.cpp | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/src/qcam/viewfinder.cpp b/src/qcam/viewfinder.cpp
> > index c7c6d099dfba..df5ee5ee7b83 100644
> > --- a/src/qcam/viewfinder.cpp
> > +++ b/src/qcam/viewfinder.cpp
> > @@ -56,6 +56,11 @@ int ViewFinder::setFormat(const libcamera::PixelFormat &format,
> >  			return ret;
> >  
> >  		image_ = QImage(size, QImage::Format_RGB32);
> > +
> > +		qInfo() << "Using software format conversion from"
> > +			<< format.toString().c_str();
> 
> Should this message state the conversion from and to?
> 
> Perhaps it doesn't matter too much at the moment as we always convert to
> the same destination format - but perhaps that might differ if we have
> multiple compatible 'native' formats?

I initially had a message that stated "... to QImage::Format_RGB32", but
thought it was overkill as that's fixed. If we end up picking the
destination format dynamically, we can update the message then.

> 
> > +	} else {
> > +		qInfo() << "Zero-copy enabled";
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> >  	}
> >  
> >  	format_ = format;