[{"id":12467,"web_url":"https://patchwork.libcamera.org/comment/12467/","msgid":"<20200912021847.GN6808@pendragon.ideasonboard.com>","date":"2020-09-12T02:18:47","subject":"Re: [libcamera-devel] [PATCH v6 4/4] qcam: add additional command\n\tline option to select the render type","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Show,\n\nThank you for the patch.\n\nOn Fri, Sep 11, 2020 at 04:55:14PM +0800, Show Liu wrote:\n> Add new option \"--render=qt|gles\" to select the render type,\n> \"--render=gles\" to accelerate format conversion and rendering\n> \"--render=qt\" is the original Qt rendering\n\ns/--render/renderer/\n\n> Signed-off-by: Show Liu \n> ---\n> src/qcam/main.cpp | 3 +++\n> src/qcam/main_window.cpp | 31 +++++++++++++++++++++++++++----\n> src/qcam/main_window.h | 3 +++\n> 3 files changed, 33 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/qcam/main.cpp b/src/qcam/main.cpp\n> index bae358d..4b7d041 100644\n> --- a/src/qcam/main.cpp\n> +++ b/src/qcam/main.cpp\n> @@ -33,6 +33,9 @@ OptionsParser::Options parseOptions(int argc, char *argv[])\n> \t\t\t ArgumentRequired, \"camera\");\n> \tparser.addOption(OptHelp, OptionNone, \"Display this help message\",\n> \t\t\t \"help\");\n> +\tparser.addOption(OptRendered, OptionString,\n\nI'm sorry, I made a typo when reviewing the previous version, I meant\nOptRenderer, not OptRendered.\n\n> +\t\t\t \"Choose the renderer type {qt,gles} (default: qt)\", \"renderer\",\n> +\t\t\t ArgumentRequired, \"render\");\n\ns/render/renderer/\n\n> \tparser.addOption(OptStream, &streamKeyValue,\n> \t\t\t \"Set configuration of a camera stream\", \"stream\", true);\n> \n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index 612d978..315102c 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -95,6 +95,9 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)\n> {\n> \tint ret;\n> \n> +\t/* Render Type Qt or GLES, and set Qt by default */\n> +\tstd::string renderType_ = \"qt\";\n\nNo not for a trailing _ for local variables. I would also move the\nvariable declaration to where it gets used, ...\n\n> +\n> \t/*\n> \t * Initialize the UI: Create the toolbar, set the window title and\n> \t * create the viewfinder widget.\n> @@ -105,10 +108,30 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)\n> \tsetWindowTitle(title_);\n> \tconnect(&titleTimer_, SIGNAL(timeout()), this, SLOT(updateTitle()));\n> \n> -\tviewfinder_ = new ViewFinder(this);\n> -\tconnect(viewfinder_, &ViewFinder::renderComplete,\n> -\t\tthis, &MainWindow::queueRequest);\n> -\tsetCentralWidget(viewfinder_);\n\n... here.\n\n> +\tif (options_.isSet(OptRendered))\n> +\t\trenderType_ = options_[OptRendered].toString();\n> +\n> +\tif (renderType_ == \"qt\") {\n> +\t\tViewFinderQt *viewfinder = new ViewFinderQt(this);\n> +\t\tconnect(viewfinder, &ViewFinderQt::renderComplete,\n> +\t\t\tthis, &MainWindow::queueRequest);\n> +\t\tviewfinder_ = viewfinder;\n> +\t\tsetCentralWidget(viewfinder);\n> +#ifndef QT_NO_OPENGL\n> +\t} else if (renderType_ == \"gles\") {\n> +\t\tViewFinderGL *viewfinder = new ViewFinderGL(this);\n> +\t\tconnect(viewfinder, &ViewFinderGL::renderComplete,\n> +\t\t\tthis, &MainWindow::queueRequest);\n> +\t\tviewfinder_ = viewfinder;\n> +\t\tsetCentralWidget(viewfinder);\n> +#endif\n> +\t} else {\n> +\t\tqWarning() << \"Invalid render type\"\n\ns/render/renderer/\n\n> +\t\t\t << QString::fromStdString(renderType_);\n> +\t\tquit();\n> +\t\treturn;\n> +\t}\n> +\n> \tadjustSize();\n> \n> \t/* Hotplug/unplug support */\n> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\n> index 3d21779..251f78b 100644\n> --- a/src/qcam/main_window.h\n> +++ b/src/qcam/main_window.h\n> @@ -26,6 +26,8 @@\n> \n> #include \"../cam/stream_options.h\"\n> #include \"viewfinder.h\"\n> +#include \"viewfinder_gl.h\"\n> +#include \"viewfinder_qt.h\"\n\nYou can move those two includes to main_window.cpp.\n\nReviewed-by: Laurent Pinchart \n\n> \n> using namespace libcamera;\n> \n> @@ -37,6 +39,7 @@ class HotplugEvent;\n> enum {\n> \tOptCamera = 'c',\n> \tOptHelp = 'h',\n> +\tOptRendered = 'r',\n> \tOptStream = 's',\n> };\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1D33FBF01C\n\tfor ;\n\tSat, 12 Sep 2020 02:19:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A973462C8C;\n\tSat, 12 Sep 2020 04:19:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 79EBC62C43\n\tfor ;\n\tSat, 12 Sep 2020 04:19:16 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DEDF2276;\n\tSat, 12 Sep 2020 04:19:15 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"qFpX9H9t\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1599877156;\n\tbh=C/U04o4mMVX0/g+WTIcw4PIB6V/ZzDaz3iXTHOibWyE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qFpX9H9tsUs44IwK+IGrz2WV7NZxmfk6S4Ekmdhl9GhvuJND6a79irXKEBr10JHGD\n\tGDg1SnXdrX9KZjkU8Rn/zGPSvmeCL9GxND1xjguSQ9MTzAjk3WsOQIVLThBI8d8nNc\n\tSEbZ4M9SO0Wmaqz1j9fiV8SJRVGmBRFjPVxmdP2o=","Date":"Sat, 12 Sep 2020 05:18:47 +0300","From":"Laurent Pinchart ","To":"Show Liu ","Message-ID":"<20200912021847.GN6808@pendragon.ideasonboard.com>","References":"<20200911085514.30021-1-show.liu@linaro.org>\n\t<20200911085514.30021-5-show.liu@linaro.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200911085514.30021-5-show.liu@linaro.org>","Subject":"Re: [libcamera-devel] [PATCH v6 4/4] qcam: add additional command\n\tline option to select the render type","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]