[libcamera-devel] [PATCH v2.2 21/21] qcam: viewfinder: Add support for more native formats

[Kieran Bingham]

Hi Laurent,

On 23/03/2020 23:28, Laurent Pinchart wrote:
> Qt supports more 24-bit and 32-bit RGB formats for native painting. If
> the frame buffer pixel format matches any of them, disable the converter
> and create a QImage in the appropriate format.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> Changes since v2:
> 
> - Add more formats
> - Remove unnecessary parentheses
> 
> Changes since v1:
> 
> - Rename formats to nativeFormats
> ---
>  src/qcam/viewfinder.cpp | 25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qcam/viewfinder.cpp b/src/qcam/viewfinder.cpp
> index 45e226b58135..7e7407f56e4c 100644
> --- a/src/qcam/viewfinder.cpp
> +++ b/src/qcam/viewfinder.cpp
> @@ -7,16 +7,34 @@
>  
>  #include "viewfinder.h"
>  
> +#include <stdint.h>
>  #include <utility>
>  
>  #include <QImage>
>  #include <QImageWriter>
> +#include <QMap>
>  #include <QMutexLocker>
>  #include <QPainter>
>  #include <QtDebug>
>  
>  #include "format_converter.h"
>  
> +static const QMap<uint32_t, QImage::Format> nativeFormats
> +{
> +#if QT_VERSION >= QT_VERSION_CHECK(5, 2, 0)
> +#if QBYTE_ORDER == Q_LITTLE_ENDIAN
> +	{ DRM_FORMAT_ABGR8888, QImage::Format_RGBA8888 },
> +#else
> +	{ DRM_FORMAT_RGBA8888, QImage::Format_RGBA8888 },
> +#endif

There's no else attributed to the QT_VERSION_CHECK, so we will not
support QImage::Format_RGBA8888 at all on version < 5.2.0...

Presumably that's not an issue.

> +#endif


Can we explain (in a comment perhaps) why we need to handle byte order
for QImage::Format_RGBA8888, but not QImage::Format_RGB32 ?


> +	{ DRM_FORMAT_ARGB8888, QImage::Format_RGB32 },


> +#if QT_VERSION >= QT_VERSION_CHECK(5, 14, 0)
> +	{ DRM_FORMAT_BGR888, QImage::Format_BGR888 },
> +#endif
> +	{ DRM_FORMAT_RGB888, QImage::Format_RGB888 },
> +};
> +
>  ViewFinder::ViewFinder(QWidget *parent)
>  	: QWidget(parent), buffer_(nullptr)
>  {
> @@ -36,7 +54,7 @@ int ViewFinder::setFormat(const libcamera::PixelFormat &format,
>  	 * If format conversion is needed, configure the converter and allocate
>  	 * the destination image.
>  	 */
> -	if (format != DRM_FORMAT_ARGB8888) {
> +	if (!nativeFormats.contains(format)) {
>  		int ret = converter_.configure(format, size);
>  		if (ret < 0)
>  			return ret;
> @@ -64,7 +82,7 @@ void ViewFinder::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)
>  	{
>  		QMutexLocker locker(&mutex_);
>  
> -		if (format_ == DRM_FORMAT_ARGB8888) {
> +		if (nativeFormats.contains(format_)) {
>  			/*
>  			 * If the frame format is identical to the display
>  			 * format, create a QImage that references the frame
> @@ -76,7 +94,8 @@ void ViewFinder::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)
>  			 * computing it naively
>  			 */
>  			image_ = QImage(memory, size_.width(), size_.height(),
> -					size / size_.height(), QImage::Format_RGB32);
> +					size / size_.height(),
> +					nativeFormats[format_]);
>  			std::swap(buffer, buffer_);
>  		} else {
>  			/*
> 

-- 
Regards
--
Kieran