[{"id":4144,"web_url":"https://patchwork.libcamera.org/comment/4144/","msgid":"<20200320144652.GD3717547@oden.dyn.berto.se>","date":"2020-03-20T14:46:52","subject":"Re: [libcamera-devel] [RFC] [PATCH 3/3] qcam: added an option to\n\tenable rendering via OpenGL shader","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Show,\n\nThanks for your work.\n\nOn 2020-03-20 16:50:29 +0800, Show Liu wrote:\n> qcam: added an option to enable rendering via OpenGL shader\n> \n> Signed-off-by: Show Liu <show.liu@linaro.org>\n> ---\n>  src/qcam/main.cpp        |  2 ++\n>  src/qcam/main_window.cpp | 31 ++++++++++++++++++++++++++++---\n>  src/qcam/main_window.h   |  3 +++\n>  3 files changed, 33 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/qcam/main.cpp b/src/qcam/main.cpp\n> index a7ff5c5..7727a09 100644\n> --- a/src/qcam/main.cpp\n> +++ b/src/qcam/main.cpp\n> @@ -39,6 +39,8 @@ OptionsParser::Options parseOptions(int argc, char *argv[])\n>  \t\t\t \"help\");\n>  \tparser.addOption(OptSize, &sizeParser, \"Set the stream size\",\n>  \t\t\t \"size\", true);\n> +\tparser.addOption(OptOpengl, OptionNone, \"Enable YUV format via OpenGL shader\",\n> +\t\t\t \"opengl\");\n>  \n>  \tOptionsParser::Options options = parser.parse(argc, argv);\n>  \tif (options.isSet(OptHelp))\n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index 98e55ba..5a5bc88 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -28,6 +28,7 @@\n>  \n>  #include \"main_window.h\"\n>  #include \"viewfinder.h\"\n> +#include \"glwidget.h\"\n>  \n>  using namespace libcamera;\n>  \n> @@ -43,7 +44,14 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)\n>  \tconnect(&titleTimer_, SIGNAL(timeout()), this, SLOT(updateTitle()));\n>  \n>  \tviewfinder_ = new ViewFinder(this);\n> -\tsetCentralWidget(viewfinder_);\n> +\tglwidget_ = new GLWidget(this);\n> +\n> +\tif (options_.isSet(OptOpengl)) {\n> +\t\tsetCentralWidget(glwidget_);\n> +\t} else {\n> +\t\tsetCentralWidget(viewfinder_);\n> +\t}\n> +\n\nYes this makes it a bit clearer that a single base class would be nicer \n:-)\n\n>  \tadjustSize();\n>  \n>  \tret = openCamera();\n> @@ -232,6 +240,7 @@ int MainWindow::startCapture()\n>  \t}\n>  \n>  \tStream *stream = cfg.stream();\n> +\n>  \tret = viewfinder_->setFormat(cfg.pixelFormat, cfg.size.width,\n>  \t\t\t\t     cfg.size.height);\n>  \tif (ret < 0) {\n> @@ -239,6 +248,11 @@ int MainWindow::startCapture()\n>  \t\treturn ret;\n>  \t}\n>  \n> +\tif (options_.isSet(OptOpengl)) {\n> +\t\tglwidget_->setFrameSize(cfg.size.width, cfg.size.height);\n> +\t\tglwidget_->setFixedSize(cfg.size.width, cfg.size.height);\n> +\t}\n> +\n>  \tstatusBar()->showMessage(QString(cfg.toString().c_str()));\n>  \n>  \tadjustSize();\n> @@ -353,7 +367,13 @@ void MainWindow::stopCapture()\n>  \n>  void MainWindow::saveImageAs()\n>  {\n> -\tQImage image = viewfinder_->getCurrentImage();\n> +\tQImage image;\n> +\tif (options_.isSet(OptOpengl)) {\n> +\t\timage = glwidget_->grabFramebuffer();\n> +\t} else {\n> +\t\timage = viewfinder_->getCurrentImage();\n> +\t}\n> +\n>  \tQString defaultPath = QStandardPaths::writableLocation(QStandardPaths::PicturesLocation);\n>  \n>  \tQString filename = QFileDialog::getSaveFileName(this, \"Save Image\", defaultPath,\n> @@ -416,7 +436,12 @@ int MainWindow::display(FrameBuffer *buffer)\n>  \tconst FrameBuffer::Plane &plane = buffer->planes().front();\n>  \tvoid *memory = mappedBuffers_[plane.fd.fd()].first;\n>  \tunsigned char *raw = static_cast<unsigned char *>(memory);\n> -\tviewfinder_->display(raw, buffer->metadata().planes[0].bytesused);\n> +\n> +\tif (options_.isSet(OptOpengl)) {\n> +\t\tglwidget_->updateFrame(raw);\n> +\t} else {\n> +\t\tviewfinder_->display(raw, buffer->metadata().planes[0].bytesused);\n> +\t}\n>  \n>  \treturn 0;\n>  }\n> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\n> index 40aa10a..5501dd1 100644\n> --- a/src/qcam/main_window.h\n> +++ b/src/qcam/main_window.h\n> @@ -21,6 +21,7 @@\n>  #include <libcamera/stream.h>\n>  \n>  #include \"../cam/options.h\"\n> +#include \"glwidget.h\"\n>  \n>  using namespace libcamera;\n>  \n> @@ -30,6 +31,7 @@ enum {\n>  \tOptCamera = 'c',\n>  \tOptHelp = 'h',\n>  \tOptSize = 's',\n> +\tOptOpengl = 'o',\n>  };\n>  \n>  class MainWindow : public QMainWindow\n> @@ -79,6 +81,7 @@ private:\n>  \n>  \tQToolBar *toolbar_;\n>  \tViewFinder *viewfinder_;\n> +\tGLWidget *glwidget_;\n\nThen this would just be ViewFinder *viewfinder_ and one of the two \nimplementations would be created and a pointer to it stored in \nviewfinder_.\n\n>  \tstd::map<int, std::pair<void *, unsigned int>> mappedBuffers_;\n>  };\n>  \n> -- \n> 2.20.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lf1-x142.google.com (mail-lf1-x142.google.com\n\t[IPv6:2a00:1450:4864:20::142])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 57B3360415\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 20 Mar 2020 15:46:54 +0100 (CET)","by mail-lf1-x142.google.com with SMTP id j11so4756265lfg.4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 20 Mar 2020 07:46:54 -0700 (PDT)","from localhost (h-200-138.A463.priv.bahnhof.se. [176.10.200.138])\n\tby smtp.gmail.com with ESMTPSA id\n\ty6sm3574234lfy.38.2020.03.20.07.46.52\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 20 Mar 2020 07:46:53 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=creAFr8WtB4fplAivkLUoYNpK6jWDtokcHFm9elG87c=;\n\tb=Rp1lg4y2Jt1lYuh0Css6yLCma12piAgaS2UqhHgmGd5CgGNW+fSm5DJZUYNBQLxg8a\n\texrC07rO93LhDYlsjf1ygM3DdpjPAmA3QDwjmuWEcB1ct38oFi/hWKd38WlLj9aVx/7F\n\tMCdVYb4wZi9bxOdgWz6Xt5QlP/p12DSspEiBQMd35AA0yh6weYk9qGyjPmU6aqVPJKLB\n\tGMZHGVZp/5X97R2VZGi/mX6Jn66ZE3GYnIBfGfJStvLiM8Db8bH9LboaszO9OqqEsDWY\n\tT/Z7GkQ0cBKlEwc5sCeAjmQr+lxe5hBL2bKOlvKY34M4fPhPtuBlIPfT7S9IEJDbXGEC\n\tQ51Q==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=creAFr8WtB4fplAivkLUoYNpK6jWDtokcHFm9elG87c=;\n\tb=IPTFFrrbbGnfysFc2m7BqFF5iwQqCVm/fYyfIRf1QQFcCrVwybLfogGKC1lBziOf3t\n\t/sKkNEWu5OI7qneBRTX9opo/X7KpN1cVFfC3LyeXwJFX1c3tS51FYC1nuxjJZ76jZJ9L\n\t6lsiSwQqyoQmBaj2vHI4icuQ4hKVdjQ+cK6O2sO2yJ6+lWx57B2fyusPV59bcy6jaiYh\n\t713giS5YBX2lTqKOj4EEsUHylO3T26H4Se8BI8yyeYz8G3oDIHA4oeEw2ZxJNof+Y/W8\n\t9CCZFU5OFKIdUkoK1tG6TudO9XK510ceXE/u8VsX6AAbILGse4ZZzg8PK32NwDtzqUUW\n\to2Vg==","X-Gm-Message-State":"ANhLgQ2yLzODsirg/TlUya4UWvehfvbYlM+pFdwilLrWEv8C4I5kt2mf\n\tpcgEOCTzfOjhMDzUWMwVUDIZ7WZVF90=","X-Google-Smtp-Source":"ADFU+vv919kkJjsPz504WdOMkAiaKWL8PqpvlqsssySFienZi3TElCORGjK9nB5HJ1h9uJFYdYP62w==","X-Received":"by 2002:a19:6f07:: with SMTP id k7mr2069162lfc.79.1584715613669; \n\tFri, 20 Mar 2020 07:46:53 -0700 (PDT)","Date":"Fri, 20 Mar 2020 15:46:52 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Show Liu <show.liu@linaro.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200320144652.GD3717547@oden.dyn.berto.se>","References":"<20200320085029.17875-1-show.liu@linaro.org>\n\t<20200320085029.17875-4-show.liu@linaro.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200320085029.17875-4-show.liu@linaro.org>","Subject":"Re: [libcamera-devel] [RFC] [PATCH 3/3] qcam: added an option to\n\tenable rendering via OpenGL shader","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Fri, 20 Mar 2020 14:46:54 -0000"}},{"id":4146,"web_url":"https://patchwork.libcamera.org/comment/4146/","msgid":"<20200320145421.GN5193@pendragon.ideasonboard.com>","date":"2020-03-20T14:54:21","subject":"Re: [libcamera-devel] [RFC] [PATCH 3/3] qcam: added an option to\n\tenable rendering via OpenGL shader","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, Mar 20, 2020 at 04:50:29PM +0800, Show Liu wrote:\n> qcam: added an option to enable rendering via OpenGL shader\n\ns/added/Add/\n\nYou don't need to repeat the subject line in the commit message, but it\ncould be useful to add a bit more detail in the commit message (although\nin this case the change is fairly trivial).\n\n> Signed-off-by: Show Liu <show.liu@linaro.org>\n> ---\n>  src/qcam/main.cpp        |  2 ++\n>  src/qcam/main_window.cpp | 31 ++++++++++++++++++++++++++++---\n>  src/qcam/main_window.h   |  3 +++\n>  3 files changed, 33 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/qcam/main.cpp b/src/qcam/main.cpp\n> index a7ff5c5..7727a09 100644\n> --- a/src/qcam/main.cpp\n> +++ b/src/qcam/main.cpp\n> @@ -39,6 +39,8 @@ OptionsParser::Options parseOptions(int argc, char *argv[])\n>  \t\t\t \"help\");\n>  \tparser.addOption(OptSize, &sizeParser, \"Set the stream size\",\n>  \t\t\t \"size\", true);\n> +\tparser.addOption(OptOpengl, OptionNone, \"Enable YUV format via OpenGL shader\",\n> +\t\t\t \"opengl\");\n\nHow about a renderer option instead, that would take a string value ? It\nwould be useful to support more renderer in the future if we need to. Or\nmaybe that will never be needed ?\n\n>  \n>  \tOptionsParser::Options options = parser.parse(argc, argv);\n>  \tif (options.isSet(OptHelp))\n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index 98e55ba..5a5bc88 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -28,6 +28,7 @@\n>  \n>  #include \"main_window.h\"\n>  #include \"viewfinder.h\"\n> +#include \"glwidget.h\"\n\nAlphabetical order please.\n\n>  \n>  using namespace libcamera;\n>  \n> @@ -43,7 +44,14 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)\n>  \tconnect(&titleTimer_, SIGNAL(timeout()), this, SLOT(updateTitle()));\n>  \n>  \tviewfinder_ = new ViewFinder(this);\n> -\tsetCentralWidget(viewfinder_);\n> +\tglwidget_ = new GLWidget(this);\n\nI think we should create a base ViewFinder class (renaming the existing\nViewFinder to ViewFinderQImage or ViewFinderQPainter), and make the two\nimplementations inherit from the base. This code would then become\n\n\tif (options_.isSet(OptOpengl))\n\t\tviewfinder_ = new ViewFinderGL(this);\n\telse\n\t\tviewfinder_ = new ViewFinderQPainter(this);\n\n\tsetCentralWidget(viewfinder_);\n\nand you won't have to test for options_.isSet(OptOpengl) anywere below.\n\n> +\n> +\tif (options_.isSet(OptOpengl)) {\n> +\t\tsetCentralWidget(glwidget_);\n> +\t} else {\n> +\t\tsetCentralWidget(viewfinder_);\n> +\t}\n> +\n>  \tadjustSize();\n>  \n>  \tret = openCamera();\n> @@ -232,6 +240,7 @@ int MainWindow::startCapture()\n>  \t}\n>  \n>  \tStream *stream = cfg.stream();\n> +\n\nI don't think this is needed.\n\n>  \tret = viewfinder_->setFormat(cfg.pixelFormat, cfg.size.width,\n>  \t\t\t\t     cfg.size.height);\n>  \tif (ret < 0) {\n> @@ -239,6 +248,11 @@ int MainWindow::startCapture()\n>  \t\treturn ret;\n>  \t}\n>  \n> +\tif (options_.isSet(OptOpengl)) {\n> +\t\tglwidget_->setFrameSize(cfg.size.width, cfg.size.height);\n> +\t\tglwidget_->setFixedSize(cfg.size.width, cfg.size.height);\n> +\t}\n> +\n>  \tstatusBar()->showMessage(QString(cfg.toString().c_str()));\n>  \n>  \tadjustSize();\n> @@ -353,7 +367,13 @@ void MainWindow::stopCapture()\n>  \n>  void MainWindow::saveImageAs()\n>  {\n> -\tQImage image = viewfinder_->getCurrentImage();\n> +\tQImage image;\n> +\tif (options_.isSet(OptOpengl)) {\n> +\t\timage = glwidget_->grabFramebuffer();\n> +\t} else {\n> +\t\timage = viewfinder_->getCurrentImage();\n> +\t}\n> +\n>  \tQString defaultPath = QStandardPaths::writableLocation(QStandardPaths::PicturesLocation);\n>  \n>  \tQString filename = QFileDialog::getSaveFileName(this, \"Save Image\", defaultPath,\n> @@ -416,7 +436,12 @@ int MainWindow::display(FrameBuffer *buffer)\n>  \tconst FrameBuffer::Plane &plane = buffer->planes().front();\n>  \tvoid *memory = mappedBuffers_[plane.fd.fd()].first;\n>  \tunsigned char *raw = static_cast<unsigned char *>(memory);\n> -\tviewfinder_->display(raw, buffer->metadata().planes[0].bytesused);\n> +\n> +\tif (options_.isSet(OptOpengl)) {\n> +\t\tglwidget_->updateFrame(raw);\n> +\t} else {\n> +\t\tviewfinder_->display(raw, buffer->metadata().planes[0].bytesused);\n> +\t}\n>  \n>  \treturn 0;\n>  }\n> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\n> index 40aa10a..5501dd1 100644\n> --- a/src/qcam/main_window.h\n> +++ b/src/qcam/main_window.h\n> @@ -21,6 +21,7 @@\n>  #include <libcamera/stream.h>\n>  \n>  #include \"../cam/options.h\"\n> +#include \"glwidget.h\"\n>  \n>  using namespace libcamera;\n>  \n> @@ -30,6 +31,7 @@ enum {\n>  \tOptCamera = 'c',\n>  \tOptHelp = 'h',\n>  \tOptSize = 's',\n> +\tOptOpengl = 'o',\n\nMaybe OptOpenGL ?\n\n>  };\n>  \n>  class MainWindow : public QMainWindow\n> @@ -79,6 +81,7 @@ private:\n>  \n>  \tQToolBar *toolbar_;\n>  \tViewFinder *viewfinder_;\n> +\tGLWidget *glwidget_;\n>  \tstd::map<int, std::pair<void *, unsigned int>> mappedBuffers_;\n>  };\n>","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 63F9860416\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 20 Mar 2020 15:54:28 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4BD88504;\n\tFri, 20 Mar 2020 15:54:27 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1584716067;\n\tbh=/1MllMgJENkcsgUOS9KrxFCILWQQDZda7NHsSuAJpJI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LlYJlOL98h0SmlMIgk3WVuK7b9nw6oxdcWlmLeSa94Vh/t2j1nDuOPkybOhC9/4z6\n\tQfE8hMBG+IHU83TdjgLvdR6FKM9V/VCCCK2RB65olUC2Rup0E89/6o/Uou9HEqEu4v\n\t6X4CPjFLKHfLUN3BfbQ8/8AAJGT5INw7itU4xAjg=","Date":"Fri, 20 Mar 2020 16:54:21 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Show Liu <show.liu@linaro.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200320145421.GN5193@pendragon.ideasonboard.com>","References":"<20200320085029.17875-1-show.liu@linaro.org>\n\t<20200320085029.17875-4-show.liu@linaro.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200320085029.17875-4-show.liu@linaro.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [RFC] [PATCH 3/3] qcam: added an option to\n\tenable rendering via OpenGL shader","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Fri, 20 Mar 2020 14:54:28 -0000"}},{"id":4177,"web_url":"https://patchwork.libcamera.org/comment/4177/","msgid":"<CA+yuoHrnaoULC8UWL3hfGCY8SL-nLHeXrnVE0=HO=nwKnkC-Og@mail.gmail.com>","date":"2020-03-23T07:29:33","subject":"Re: [libcamera-devel] [RFC] [PATCH 3/3] qcam: added an option to\n\tenable rendering via OpenGL shader","submitter":{"id":24,"url":"https://patchwork.libcamera.org/api/people/24/","name":"Show Liu","email":"show.liu@linaro.org"},"content":"Hi Laurent,\n\nThanks for your review comments.\nI will have the next version soon accordingly.\n\n\nOn Fri, Mar 20, 2020 at 10:54 PM Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Show,\n>\n> Thank you for the patch.\n>\n> On Fri, Mar 20, 2020 at 04:50:29PM +0800, Show Liu wrote:\n> > qcam: added an option to enable rendering via OpenGL shader\n>\n> s/added/Add/\n>\n> You don't need to repeat the subject line in the commit message, but it\n> could be useful to add a bit more detail in the commit message (although\n> in this case the change is fairly trivial).\n>\n> > Signed-off-by: Show Liu <show.liu@linaro.org>\n> > ---\n> >  src/qcam/main.cpp        |  2 ++\n> >  src/qcam/main_window.cpp | 31 ++++++++++++++++++++++++++++---\n> >  src/qcam/main_window.h   |  3 +++\n> >  3 files changed, 33 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/qcam/main.cpp b/src/qcam/main.cpp\n> > index a7ff5c5..7727a09 100644\n> > --- a/src/qcam/main.cpp\n> > +++ b/src/qcam/main.cpp\n> > @@ -39,6 +39,8 @@ OptionsParser::Options parseOptions(int argc, char\n> *argv[])\n> >                        \"help\");\n> >       parser.addOption(OptSize, &sizeParser, \"Set the stream size\",\n> >                        \"size\", true);\n> > +     parser.addOption(OptOpengl, OptionNone, \"Enable YUV format via\n> OpenGL shader\",\n> > +                      \"opengl\");\n>\n> How about a renderer option instead, that would take a string value ? It\n> would be useful to support more renderer in the future if we need to. Or\n> maybe that will never be needed ?\n>\n> >\n> >       OptionsParser::Options options = parser.parse(argc, argv);\n> >       if (options.isSet(OptHelp))\n> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> > index 98e55ba..5a5bc88 100644\n> > --- a/src/qcam/main_window.cpp\n> > +++ b/src/qcam/main_window.cpp\n> > @@ -28,6 +28,7 @@\n> >\n> >  #include \"main_window.h\"\n> >  #include \"viewfinder.h\"\n> > +#include \"glwidget.h\"\n>\n> Alphabetical order please.\n>\n> >\n> >  using namespace libcamera;\n> >\n> > @@ -43,7 +44,14 @@ MainWindow::MainWindow(CameraManager *cm, const\n> OptionsParser::Options &options)\n> >       connect(&titleTimer_, SIGNAL(timeout()), this,\n> SLOT(updateTitle()));\n> >\n> >       viewfinder_ = new ViewFinder(this);\n> > -     setCentralWidget(viewfinder_);\n> > +     glwidget_ = new GLWidget(this);\n>\n> I think we should create a base ViewFinder class (renaming the existing\n> ViewFinder to ViewFinderQImage or ViewFinderQPainter), and make the two\n> implementations inherit from the base. This code would then become\n>\n>         if (options_.isSet(OptOpengl))\n>                 viewfinder_ = new ViewFinderGL(this);\n>         else\n>                 viewfinder_ = new ViewFinderQPainter(this);\n>\n>         setCentralWidget(viewfinder_);\n>\n> and you won't have to test for options_.isSet(OptOpengl) anywere below.\n>\n> > +\n> > +     if (options_.isSet(OptOpengl)) {\n> > +             setCentralWidget(glwidget_);\n> > +     } else {\n> > +             setCentralWidget(viewfinder_);\n> > +     }\n> > +\n> >       adjustSize();\n> >\n> >       ret = openCamera();\n> > @@ -232,6 +240,7 @@ int MainWindow::startCapture()\n> >       }\n> >\n> >       Stream *stream = cfg.stream();\n> > +\n>\n> I don't think this is needed.\n>\n> >       ret = viewfinder_->setFormat(cfg.pixelFormat, cfg.size.width,\n> >                                    cfg.size.height);\n> >       if (ret < 0) {\n> > @@ -239,6 +248,11 @@ int MainWindow::startCapture()\n> >               return ret;\n> >       }\n> >\n> > +     if (options_.isSet(OptOpengl)) {\n> > +             glwidget_->setFrameSize(cfg.size.width, cfg.size.height);\n> > +             glwidget_->setFixedSize(cfg.size.width, cfg.size.height);\n> > +     }\n> > +\n> >       statusBar()->showMessage(QString(cfg.toString().c_str()));\n> >\n> >       adjustSize();\n> > @@ -353,7 +367,13 @@ void MainWindow::stopCapture()\n> >\n> >  void MainWindow::saveImageAs()\n> >  {\n> > -     QImage image = viewfinder_->getCurrentImage();\n> > +     QImage image;\n> > +     if (options_.isSet(OptOpengl)) {\n> > +             image = glwidget_->grabFramebuffer();\n> > +     } else {\n> > +             image = viewfinder_->getCurrentImage();\n> > +     }\n> > +\n> >       QString defaultPath =\n> QStandardPaths::writableLocation(QStandardPaths::PicturesLocation);\n> >\n> >       QString filename = QFileDialog::getSaveFileName(this, \"Save\n> Image\", defaultPath,\n> > @@ -416,7 +436,12 @@ int MainWindow::display(FrameBuffer *buffer)\n> >       const FrameBuffer::Plane &plane = buffer->planes().front();\n> >       void *memory = mappedBuffers_[plane.fd.fd()].first;\n> >       unsigned char *raw = static_cast<unsigned char *>(memory);\n> > -     viewfinder_->display(raw, buffer->metadata().planes[0].bytesused);\n> > +\n> > +     if (options_.isSet(OptOpengl)) {\n> > +             glwidget_->updateFrame(raw);\n> > +     } else {\n> > +             viewfinder_->display(raw,\n> buffer->metadata().planes[0].bytesused);\n> > +     }\n> >\n> >       return 0;\n> >  }\n> > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\n> > index 40aa10a..5501dd1 100644\n> > --- a/src/qcam/main_window.h\n> > +++ b/src/qcam/main_window.h\n> > @@ -21,6 +21,7 @@\n> >  #include <libcamera/stream.h>\n> >\n> >  #include \"../cam/options.h\"\n> > +#include \"glwidget.h\"\n> >\n> >  using namespace libcamera;\n> >\n> > @@ -30,6 +31,7 @@ enum {\n> >       OptCamera = 'c',\n> >       OptHelp = 'h',\n> >       OptSize = 's',\n> > +     OptOpengl = 'o',\n>\n> Maybe OptOpenGL ?\n>\n> >  };\n> >\n> >  class MainWindow : public QMainWindow\n> > @@ -79,6 +81,7 @@ private:\n> >\n> >       QToolBar *toolbar_;\n> >       ViewFinder *viewfinder_;\n> > +     GLWidget *glwidget_;\n> >       std::map<int, std::pair<void *, unsigned int>> mappedBuffers_;\n> >  };\n> >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n\n\nBest Regards,\nShow Liu","headers":{"Return-Path":"<show.liu@linaro.org>","Received":["from mail-io1-xd41.google.com (mail-io1-xd41.google.com\n\t[IPv6:2607:f8b0:4864:20::d41])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0ACDA60410\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 23 Mar 2020 08:29:46 +0100 (CET)","by mail-io1-xd41.google.com with SMTP id o127so13105065iof.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 23 Mar 2020 00:29:45 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=LToe3BwcYjMvoTLaaES3sniJcmsWKt2oUewxOsPQr8Q=;\n\tb=dS7e/5FWXlgi9jHZRkpzywdsNrsedZ7RydXTen8E5EV5cqs6EJZ+GtQNg3I/v5WiFk\n\tpfYL5ZNWKBK/qp3CGZE5Kbb8N8oRtdPm8zc1PU9bgkSks/AVjMI0DjD/qSFs/eUWJg1F\n\tS+Y7n3ECMGg8Y65Mn6VREaIOTm1MW37ZZGvR92//0bn88+7ZyOBfGMSBv6cddzZT30Kx\n\tRiDEmQjKiT6TN1+gFBZmYKfve+C7MWKQlYIhgmJqxGBuSFeSZzOw+nrkMTqbLqMZYsvc\n\tmSlO3VpBWsdPJsIqsBHbvH5JQ3ushmqSVrq460yqvJEB6COktLTlYHYS7/5ml6I9oJCp\n\tEo5A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=LToe3BwcYjMvoTLaaES3sniJcmsWKt2oUewxOsPQr8Q=;\n\tb=ge9ABtidwu9/vPSmgOj2OjhNcJQEFnerzY9L7Y7raA5zVbMTmpu/0GFNwavXDOsqKZ\n\tlZy48tOgNKoA2lPaDrTazzV6+EZJEkLyx5WRDIJR/i+xyYaVggXPeUPv0HoW9R3tEthu\n\tVqcpyJzdHXq3G2qQPa9cEXhmMYqF3Ustlu3xXEztS8zod6BgKIburRFg90u+dWb1zebJ\n\t8v+NSzGpMJa4TQdXKwWdbJnOQu7Dg0B8DQB0P/CZLQio0L1Y2z20DCAYBh9u6YZm5JOt\n\tS3ktX6gDM5GVLdt0A/O/JeoXAiPwte1n+Yo0g1kQPtAS+WFynTqDGa/6NfCFoyATHNWr\n\t6rKQ==","X-Gm-Message-State":"ANhLgQ0PG+0eYxMIPWsk7hKnFxcNWXF0wUnXTmbKmjoFi2JdqPIdkKpB\n\t6vJxEMeIis4q75LKcRfwYiK+jxmv1MUDMZUlj16u1x9pjNf6Cg==","X-Google-Smtp-Source":"ADFU+vv0PNyy1WJ3HA4Z0YFU3JyPp3x5d58+hNIHw0c73RSsS0wWMucv/ZY8NMjKgIFEfyF9fjzGcW3CiU5bDCTla5I=","X-Received":"by 2002:a5d:8555:: with SMTP id\n\tb21mr17806760ios.200.1584948584760; \n\tMon, 23 Mar 2020 00:29:44 -0700 (PDT)","MIME-Version":"1.0","References":"<20200320085029.17875-1-show.liu@linaro.org>\n\t<20200320085029.17875-4-show.liu@linaro.org>\n\t<20200320145421.GN5193@pendragon.ideasonboard.com>","In-Reply-To":"<20200320145421.GN5193@pendragon.ideasonboard.com>","From":"Show Liu <show.liu@linaro.org>","Date":"Mon, 23 Mar 2020 15:29:33 +0800","Message-ID":"<CA+yuoHrnaoULC8UWL3hfGCY8SL-nLHeXrnVE0=HO=nwKnkC-Og@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/alternative; boundary=\"00000000000061e1a905a180996a\"","Subject":"Re: [libcamera-devel] [RFC] [PATCH 3/3] qcam: added an option to\n\tenable rendering via OpenGL shader","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 23 Mar 2020 07:29:46 -0000"}}]