[libcamera-devel] [PATCH v6 4/4] qcam: add additional command line option to select the render type

[Laurent Pinchart]

Hi Show,

Thank you for the patch.

On Fri, Sep 11, 2020 at 04:55:14PM +0800, Show Liu wrote:
> Add new option "--render=qt|gles" to select the render type,
> "--render=gles" to accelerate format conversion and rendering
> "--render=qt" is the original Qt rendering

s/--render/renderer/

> Signed-off-by: Show Liu <show.liu at linaro.org>
> ---
>  src/qcam/main.cpp        |  3 +++
>  src/qcam/main_window.cpp | 31 +++++++++++++++++++++++++++----
>  src/qcam/main_window.h   |  3 +++
>  3 files changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qcam/main.cpp b/src/qcam/main.cpp
> index bae358d..4b7d041 100644
> --- a/src/qcam/main.cpp
> +++ b/src/qcam/main.cpp
> @@ -33,6 +33,9 @@ OptionsParser::Options parseOptions(int argc, char *argv[])
>  			 ArgumentRequired, "camera");
>  	parser.addOption(OptHelp, OptionNone, "Display this help message",
>  			 "help");
> +	parser.addOption(OptRendered, OptionString,

I'm sorry, I made a typo when reviewing the previous version, I meant
OptRenderer, not OptRendered.

> +			 "Choose the renderer type {qt,gles} (default: qt)", "renderer",
> +			 ArgumentRequired, "render");

s/render/renderer/

>  	parser.addOption(OptStream, &streamKeyValue,
>  			 "Set configuration of a camera stream", "stream", true);
>  
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 612d978..315102c 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -95,6 +95,9 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
>  {
>  	int ret;
>  
> +	/* Render Type Qt or GLES, and set Qt by default */
> +	std::string renderType_ = "qt";

No not for a trailing _ for local variables. I would also move the
variable declaration to where it gets used, ...

> +
>  	/*
>  	 * Initialize the UI: Create the toolbar, set the window title and
>  	 * create the viewfinder widget.
> @@ -105,10 +108,30 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
>  	setWindowTitle(title_);
>  	connect(&titleTimer_, SIGNAL(timeout()), this, SLOT(updateTitle()));
>  
> -	viewfinder_ = new ViewFinder(this);
> -	connect(viewfinder_, &ViewFinder::renderComplete,
> -		this, &MainWindow::queueRequest);
> -	setCentralWidget(viewfinder_);

... here.

> +	if (options_.isSet(OptRendered))
> +		renderType_ = options_[OptRendered].toString();
> +
> +	if (renderType_ == "qt") {
> +		ViewFinderQt *viewfinder = new ViewFinderQt(this);
> +		connect(viewfinder, &ViewFinderQt::renderComplete,
> +			this, &MainWindow::queueRequest);
> +		viewfinder_ = viewfinder;
> +		setCentralWidget(viewfinder);
> +#ifndef QT_NO_OPENGL
> +	} else if (renderType_ == "gles") {
> +		ViewFinderGL *viewfinder = new ViewFinderGL(this);
> +		connect(viewfinder, &ViewFinderGL::renderComplete,
> +			this, &MainWindow::queueRequest);
> +		viewfinder_ = viewfinder;
> +		setCentralWidget(viewfinder);
> +#endif
> +	} else {
> +		qWarning() << "Invalid render type"

s/render/renderer/

> +			   << QString::fromStdString(renderType_);
> +		quit();
> +		return;
> +	}
> +
>  	adjustSize();
>  
>  	/* Hotplug/unplug support */
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index 3d21779..251f78b 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -26,6 +26,8 @@
>  
>  #include "../cam/stream_options.h"
>  #include "viewfinder.h"
> +#include "viewfinder_gl.h"
> +#include "viewfinder_qt.h"

You can move those two includes to main_window.cpp.

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

>  
>  using namespace libcamera;
>  
> @@ -37,6 +39,7 @@ class HotplugEvent;
>  enum {
>  	OptCamera = 'c',
>  	OptHelp = 'h',
> +	OptRendered = 'r',
>  	OptStream = 's',
>  };
>  

-- 
Regards,

Laurent Pinchart