[libcamera-devel] qcam: Show string representation of pixel format

18079 diff mbox series

The raw pixel format in form of the fourcc integer is not easily readable.
Use the string representation instead for easier debugging.

Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
---
 src/apps/qcam/viewfinder_qt.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
2.34.1

Quoting Christian Rauch via libcamera-devel (2022-12-29 22:54:35)
> The raw pixel format in form of the fourcc integer is not easily readable.
> Use the string representation instead for easier debugging.
> 
> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
> ---
>  src/apps/qcam/viewfinder_qt.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/apps/qcam/viewfinder_qt.cpp b/src/apps/qcam/viewfinder_qt.cpp
> index a7482bea..dcb7acdf 100644
> --- a/src/apps/qcam/viewfinder_qt.cpp
> +++ b/src/apps/qcam/viewfinder_qt.cpp
> @@ -71,7 +71,7 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, const QSize &s
> 
>                 image_ = QImage(size, QImage::Format_RGB32);
> 
> -               qInfo() << "Using software format conversion from" << format;
> +               qInfo() << "Using software format conversion from" << format.toString().c_str();

Does the 'conversion from" need a space after it? or does the
format.toString() provide that?

If that's all fine or resolved:


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


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

Hi Kieran,

Am 30.12.22 um 01:25 schrieb Kieran Bingham:
> Quoting Christian Rauch via libcamera-devel (2022-12-29 22:54:35)
>> The raw pixel format in form of the fourcc integer is not easily readable.
>> Use the string representation instead for easier debugging.
>>
>> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
>> ---
>>   src/apps/qcam/viewfinder_qt.cpp | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/apps/qcam/viewfinder_qt.cpp b/src/apps/qcam/viewfinder_qt.cpp
>> index a7482bea..dcb7acdf 100644
>> --- a/src/apps/qcam/viewfinder_qt.cpp
>> +++ b/src/apps/qcam/viewfinder_qt.cpp
>> @@ -71,7 +71,7 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, const QSize &s
>>
>>                  image_ = QImage(size, QImage::Format_RGB32);
>>
>> -               qInfo() << "Using software format conversion from" << format;
>> +               qInfo() << "Using software format conversion from" << format.toString().c_str();
>
> Does the 'conversion from" need a space after it? or does the
> format.toString() provide that?

toString() does not provide the extra space. This seems to be some Qt
magic. Before this patch, the raw pixel format number was also printed
with a space between the static text and the integer format.

> If that's all fine or resolved:
>
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
>
>>          } else {
>>                  qInfo() << "Zero-copy enabled";
>>          }
>> --
>> 2.34.1
>>

Hello,

On Fri, Dec 30, 2022 at 01:44:57AM +0100, Christian Rauch wrote:
> Am 30.12.22 um 01:25 schrieb Kieran Bingham:
> > Quoting Christian Rauch via libcamera-devel (2022-12-29 22:54:35)
> >> The raw pixel format in form of the fourcc integer is not easily readable.
> >> Use the string representation instead for easier debugging.
> >>
> >> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
> >> ---
> >>   src/apps/qcam/viewfinder_qt.cpp | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/src/apps/qcam/viewfinder_qt.cpp b/src/apps/qcam/viewfinder_qt.cpp
> >> index a7482bea..dcb7acdf 100644
> >> --- a/src/apps/qcam/viewfinder_qt.cpp
> >> +++ b/src/apps/qcam/viewfinder_qt.cpp
> >> @@ -71,7 +71,7 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, const QSize &s
> >>
> >>                  image_ = QImage(size, QImage::Format_RGB32);
> >>
> >> -               qInfo() << "Using software format conversion from" << format;
> >> +               qInfo() << "Using software format conversion from" << format.toString().c_str();

This is getting long, let's write

		qInfo() << "Using software format conversion from"
			<< format.toString().c_str();

Sometimes I'm tempted to use the standard C++ library streams in qcam,
as we would then benefit from libcamera's operator<<() implementation
for the PixelFormat class.

> > Does the 'conversion from" need a space after it? or does the
> > format.toString() provide that?
> 
> toString() does not provide the extra space. This seems to be some Qt
> magic. Before this patch, the raw pixel format number was also printed
> with a space between the static text and the integer format.

Qt inserts spaces automatically.

> > If that's all fine or resolved:
> >
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

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

I'll address the line wrap when applying.

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