[{"id":24158,"web_url":"https://patchwork.libcamera.org/comment/24158/","msgid":"<YuBm9P0TDgKUb5+N@pendragon.ideasonboard.com>","date":"2022-07-26T22:13:08","subject":"Re: [libcamera-devel] [PATCH v5 2/3] qcam: Add a GUI way to use\n\tcapture script","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Utkarsh,\n\nThank you for the patch.\n\nOn Wed, Jul 27, 2022 at 01:11:22AM +0530, Utkarsh Tiwari via libcamera-devel wrote:\n> Implement an Open Capture Script button which would allow the user\n> to open a Capture Script (*.yaml).\n> This button has two states :\n>     - Open Capture Script\n>     - Stop the execution of current capture script\n> \n> When being clicked in open state, present them with a QFileDialog to\n> allow user to select a single file.\n> \n> Introduce a queueCount_ to keep track of the requests queued.\n> \n> When stopping the execution of the capture script the queueCount_ is not\n> reseted and the capture is continues as it is (i.e it is not stopped or\n> restarted).\n> \n> Requests are queued with any controls the script matching the current\n> queueCount_.\n> \n> Signed-off-by: Utkarsh Tiwari <utkarsh02t@gmail.com>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>  Changes from v4\n> \t- Reworded the commit message to clarify button states and\n> \tqueueCount_ reseting.\n> \n>  src/qcam/assets/feathericons/feathericons.qrc |  2 +\n>  src/qcam/main_window.cpp                      | 68 +++++++++++++++++++\n>  src/qcam/main_window.h                        |  6 ++\n>  src/qcam/meson.build                          |  2 +\n>  4 files changed, 78 insertions(+)\n> \n> diff --git a/src/qcam/assets/feathericons/feathericons.qrc b/src/qcam/assets/feathericons/feathericons.qrc\n> index c5302040..6b08395a 100644\n> --- a/src/qcam/assets/feathericons/feathericons.qrc\n> +++ b/src/qcam/assets/feathericons/feathericons.qrc\n> @@ -3,9 +3,11 @@\n>  <qresource>\n>  \t<file>aperture.svg</file>\n>  \t<file>camera-off.svg</file>\n> +\t<file>file.svg</file>\n\nI'm a bit annoyed by the fact that this icon is very generic. I looked\nthrough the feathericons directory and nothing really strikes me as\nbeing a perfect match (it would be too easy). https://feathericons.com/\nhas a few new icons, but also nothing that really stands out. Here are a\nfew options you may want to consider:\n\n- activity: Hints at something that evolves over time.\n- command: A capture script is somehow a list of commands. Maybe too\n  Apple-related ?\n- list: The other term in a \"list of commands\".\n- settings: The script contains settings, but it may be best to keep\n  this icon for a future settings dialog.\n- sliders: Same here.\n- zap: Not sure what the link would be, but there's a zap-off icon that\n  is handy to replace x-square.svg\n\n>  \t<file>play-circle.svg</file>\n>  \t<file>save.svg</file>\n>  \t<file>stop-circle.svg</file>\n>  \t<file>x-circle.svg</file>\n> +\t<file>x-square.svg</file>\n\nAs hinted by the zap proposal above, I think the \"on\" and \"off\" states\nof the button should have related icons. Another option is to keep the\nsame icon, and just update the button state, the same way we handle the\n\"Start Capture\" button.\n\nThe feather icons are only fallbacks, we also have to select standard\nicons from the theme. The latest (but old) version of the icon naming\nspecification is available at\nhttps://specifications.freedesktop.org/icon-naming-spec/icon-naming-spec-latest.html.\nThere we have even less options, I've only seen the following ones as\npossibly relevant:\n\nsystem-run\nmedia-playlist-repeat\nmedia-playlist-shuffle\nsync-synchronizing\n\nAlthough there's also text-x-script that may be interesting.\n\nAnother option would be to rethink the UI and add capture script support\nsomewhere else than in the toolbar.\n\n>  </qresource>\n>  </RCC>\n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index 4e773c31..9dc96fbb 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -20,6 +20,7 @@\n>  #include <QImage>\n>  #include <QImageWriter>\n>  #include <QInputDialog>\n> +#include <QMessageBox>\n>  #include <QMutexLocker>\n>  #include <QStandardPaths>\n>  #include <QStringList>\n> @@ -233,6 +234,13 @@ int MainWindow::createToolbars()\n>  \tsaveRaw_ = action;\n>  #endif\n>  \n> +\t/* Open Script... action. */\n> +\taction = toolbar_->addAction(QIcon::fromTheme(\"document-open\",\n> +\t\t\t\t\t\t      QIcon(\":file.svg\")),\n> +\t\t\t\t     \"Open Capture Script\");\n\nI'd name this \"Run Capture Script\". The action both opens and runs the\nscript, but the latter seems to be the most important part.\n\n> +\tconnect(action, &QAction::triggered, this, &MainWindow::chooseScript);\n> +\tscriptExecAction_ = action;\n> +\n>  \treturn 0;\n>  }\n>  \n> @@ -256,6 +264,60 @@ void MainWindow::updateTitle()\n>  \tsetWindowTitle(title_ + \" : \" + QString::number(fps, 'f', 2) + \" fps\");\n>  }\n>  \n> +/**\n> + * \\brief Load a capture script for handling the capture session.\n> + *\n> + * If already capturing, it would restart the capture.\n\nWhy is that ? Could we apply the capture script from the current point\nwithout restarting the camera ?\n\n> + */\n> +void MainWindow::chooseScript()\n> +{\n> +\tif (script_) {\n> +\t\t/*\n> +\t\t * This is the second valid press of load script button,\n> +\t\t * It indicates stopping, Stop and set button for new script.\n\nEither \", it\" or \". It\", and \", stop\" or \". Stop\".\n\n> +\t\t */\n> +\t\tscript_.reset();\n> +\t\tscriptExecAction_->setIcon(QIcon::fromTheme(\"document-open\",\n> +\t\t\t\t\t\t\t    QIcon(\":file.svg\")));\n> +\t\tscriptExecAction_->setText(\"Open Capture Script\");\n> +\t\treturn;\n> +\t}\n> +\n> +\tQString scriptFile = QFileDialog::getOpenFileName(this, \"Open Capture Script\", QDir::currentPath(),\n> +\t\t\t\t\t\t\t  \"Capture Script (*.yaml)\");\n> +\tif (scriptFile.isEmpty())\n> +\t\treturn;\n> +\n> +\t/*\n> +\t * If we are already capturing,\n> +\t * stop so we don't have stuck image in viewfinder.\n\n\t * If we are already capturing, stop so we don't have stuck image in\n\t * viewfinder.\n\n> +\t */\n> +\tbool wasCapturing = isCapturing_;\n> +\tif (isCapturing_)\n> +\t\ttoggleCapture(false);\n> +\n> +\tscript_ = std::make_unique<CaptureScript>(camera_, scriptFile.toStdString());\n> +\tif (!script_->valid()) {\n> +\t\tscript_.reset();\n> +\t\tQMessageBox::critical(this, \"Invalid Script\",\n> +\t\t\t\t      \"Couldn't load the capture script\");\n> +\t\tif (wasCapturing)\n> +\t\t\ttoggleCapture(true);\n> +\t\treturn;\n> +\t}\n> +\n> +\t/*\n> +\t * Valid script verified\n> +\t * Set the button to indicate stopping availibility.\n> +\t */\n\nGenerally speaking, you shouldn't have line wraps at the end of a\nsentence in the middle of a paragraph (other than the ones required to\nlimit the line length to 80 columns):\n\n\t/*\n\t * Valid script verified. Set the button to indicate stopping\n\t * availibility.\n\t */\n\nIf you want to separate the two sentences in two paragraphs, you need a\nblank line:\n\n\t/*\n\t * Valid script verified.\n\t *\n\t * Set the button to indicate stopping availibility.\n\t */\n\nI don't think that's what you want here though.\n\n> +\tscriptExecAction_->setIcon(QIcon(\":x-square.svg\"));\n> +\tscriptExecAction_->setText(\"Stop Script execution\");\n> +\n> +\t/* Start capture again if we were capturing before. */\n> +\tif (wasCapturing)\n> +\t\ttoggleCapture(true);\n> +}\n> +\n>  /* -----------------------------------------------------------------------------\n>   * Camera Selection\n>   */\n> @@ -511,6 +573,7 @@ int MainWindow::startCapture()\n>  \tpreviousFrames_ = 0;\n>  \tframesCaptured_ = 0;\n>  \tlastBufferTime_ = 0;\n> +\tqueueCount_ = 0;\n>  \n>  \tret = camera_->start();\n>  \tif (ret) {\n> @@ -790,5 +853,10 @@ void MainWindow::renderComplete(FrameBuffer *buffer)\n>  \n>  int MainWindow::queueRequest(Request *request)\n>  {\n> +\tif (script_)\n> +\t\trequest->controls() = script_->frameControls(queueCount_);\n> +\n> +\tqueueCount_++;\n> +\n>  \treturn camera_->queueRequest(request);\n>  }\n> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\n> index bc844711..eb398c1d 100644\n> --- a/src/qcam/main_window.h\n> +++ b/src/qcam/main_window.h\n> @@ -26,6 +26,7 @@\n>  #include <QQueue>\n>  #include <QTimer>\n>  \n> +#include \"../cam/capture_script.h\"\n>  #include \"../cam/stream_options.h\"\n>  \n>  #include \"viewfinder.h\"\n> @@ -87,11 +88,14 @@ private:\n>  \tvoid processHotplug(HotplugEvent *e);\n>  \tvoid processViewfinder(libcamera::FrameBuffer *buffer);\n>  \n> +\tvoid chooseScript();\n> +\n>  \t/* UI elements */\n>  \tQToolBar *toolbar_;\n>  \tQAction *startStopAction_;\n>  \tQComboBox *cameraCombo_;\n>  \tQAction *saveRaw_;\n> +\tQAction *scriptExecAction_;\n>  \tViewFinder *viewfinder_;\n>  \n>  \tQIcon iconPlay_;\n> @@ -125,6 +129,8 @@ private:\n>  \tQElapsedTimer frameRateInterval_;\n>  \tuint32_t previousFrames_;\n>  \tuint32_t framesCaptured_;\n> +\tuint32_t queueCount_;\n>  \n>  \tstd::vector<std::unique_ptr<libcamera::Request>> requests_;\n> +\tstd::unique_ptr<CaptureScript> script_;\n>  };\n> diff --git a/src/qcam/meson.build b/src/qcam/meson.build\n> index c46f4631..67074252 100644\n> --- a/src/qcam/meson.build\n> +++ b/src/qcam/meson.build\n> @@ -15,6 +15,7 @@ endif\n>  qcam_enabled = true\n>  \n>  qcam_sources = files([\n> +    '../cam/capture_script.cpp',\n>      '../cam/image.cpp',\n>      '../cam/options.cpp',\n>      '../cam/stream_options.cpp',\n\nAt some point we should probably create an application helpers library\nshared between cam and qcam. Out of scope for this series of course.\n\n> @@ -37,6 +38,7 @@ qcam_resources = files([\n>  qcam_deps = [\n>      libatomic,\n>      libcamera_public,\n> +    libyaml,\n>      qt5_dep,\n>  ]\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 18651BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 26 Jul 2022 22:13:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6D9FE63312;\n\tWed, 27 Jul 2022 00:13:12 +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 5203D6330A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Jul 2022 00:13:10 +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 A95DD415;\n\tWed, 27 Jul 2022 00:13:09 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658873592;\n\tbh=TJkQ5N9VZSGPlMxe9naMadJRM+YdJogexYRFhiiyR3Y=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=gppKUZcFPPWYeybvDbyrjb+jIDXTGwhOcDHkVUR1G1PHw2hSWcpRAQEj/TFuIlRQ/\n\tJPYKuckWwoOhYhefHf0i0h3Tix4rCwIEI/hnzk/Z8ODm0Z6+FOZ6ihzFQYJZHHQW2h\n\tzM1AScWwxRncJdH0Pj0JA1zyl+o+Q5Kc2eQokTo+VWpHg8fuX65sCTOhYP6nrJWGIm\n\tSLMx6EtesIYJNWCBgbw4G7MGE10AFWFfMF4FPeGgvk7R4+K4r0e9otVMM76pkEn9KM\n\tqTRohHB/M7/tnu18Dc03nsI9NukVUce99rp1UXLDZKAd30D5oXPxlthDql+6cy7HB9\n\tbWz5EviZjE3vg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658873589;\n\tbh=TJkQ5N9VZSGPlMxe9naMadJRM+YdJogexYRFhiiyR3Y=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=MNXPcJCZdBwI1O1KES4TeHFO5Eyq3UHl0Dxj6hwYS4VwFyJVloCBqyOAFXJhQfsM9\n\t8utl0Y3xrfIf03EpYybeY6gT5XL9BzSZaGZfO29MGcWLyMmr3rSzn7E+tLKFyzGQuJ\n\tSHFVo8OHHRRJLYnuK8AefJzSRQybdgSjHAT7vvIQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"MNXPcJCZ\"; dkim-atps=neutral","Date":"Wed, 27 Jul 2022 01:13:08 +0300","To":"Utkarsh Tiwari <utkarsh02t@gmail.com>","Message-ID":"<YuBm9P0TDgKUb5+N@pendragon.ideasonboard.com>","References":"<20220726194123.170208-1-utkarsh02t@gmail.com>\n\t<20220726194123.170208-2-utkarsh02t@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220726194123.170208-2-utkarsh02t@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v5 2/3] qcam: Add a GUI way to use\n\tcapture script","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24231,"web_url":"https://patchwork.libcamera.org/comment/24231/","msgid":"<20220728171619.GA186879@gmail.com>","date":"2022-07-28T17:16:19","subject":"Re: [libcamera-devel] [PATCH v5 2/3] qcam: Add a GUI way to use\n\tcapture script","submitter":{"id":114,"url":"https://patchwork.libcamera.org/api/people/114/","name":"Utkarsh Tiwari","email":"utkarsh02t@gmail.com"},"content":"Hi Laurent, thanks for the review.\n\nOn Wed, Jul 27, 2022 at 01:13:08AM +0300, Laurent Pinchart wrote:\n> Hi Utkarsh,\n> \n> Thank you for the patch.\n> \n> On Wed, Jul 27, 2022 at 01:11:22AM +0530, Utkarsh Tiwari via libcamera-devel wrote:\n> > Implement an Open Capture Script button which would allow the user\n> > to open a Capture Script (*.yaml).\n> > This button has two states :\n> >     - Open Capture Script\n> >     - Stop the execution of current capture script\n> > \n> > When being clicked in open state, present them with a QFileDialog to\n> > allow user to select a single file.\n> > \n> > Introduce a queueCount_ to keep track of the requests queued.\n> > \n> > When stopping the execution of the capture script the queueCount_ is not\n> > reseted and the capture is continues as it is (i.e it is not stopped or\n> > restarted).\n> > \n> > Requests are queued with any controls the script matching the current\n> > queueCount_.\n> > \n> > Signed-off-by: Utkarsh Tiwari <utkarsh02t@gmail.com>\n> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > ---\n> >  Changes from v4\n> > \t- Reworded the commit message to clarify button states and\n> > \tqueueCount_ reseting.\n> > \n> >  src/qcam/assets/feathericons/feathericons.qrc |  2 +\n> >  src/qcam/main_window.cpp                      | 68 +++++++++++++++++++\n> >  src/qcam/main_window.h                        |  6 ++\n> >  src/qcam/meson.build                          |  2 +\n> >  4 files changed, 78 insertions(+)\n> > \n> > diff --git a/src/qcam/assets/feathericons/feathericons.qrc b/src/qcam/assets/feathericons/feathericons.qrc\n> > index c5302040..6b08395a 100644\n> > --- a/src/qcam/assets/feathericons/feathericons.qrc\n> > +++ b/src/qcam/assets/feathericons/feathericons.qrc\n> > @@ -3,9 +3,11 @@\n> >  <qresource>\n> >  \t<file>aperture.svg</file>\n> >  \t<file>camera-off.svg</file>\n> > +\t<file>file.svg</file>\n> \n> I'm a bit annoyed by the fact that this icon is very generic. I looked\n> through the feathericons directory and nothing really strikes me as\n> being a perfect match (it would be too easy). https://feathericons.com/\n> has a few new icons, but also nothing that really stands out. Here are a\n> few options you may want to consider:\n> \n> - activity: Hints at something that evolves over time.\n> - command: A capture script is somehow a list of commands. Maybe too\n>   Apple-related ?\n> - list: The other term in a \"list of commands\".\n> - settings: The script contains settings, but it may be best to keep\n>   this icon for a future settings dialog.\n> - sliders: Same here.\n> - zap: Not sure what the link would be, but there's a zap-off icon that\n>   is handy to replace x-square.svg\n> \n> >  \t<file>play-circle.svg</file>\n> >  \t<file>save.svg</file>\n> >  \t<file>stop-circle.svg</file>\n> >  \t<file>x-circle.svg</file>\n> > +\t<file>x-square.svg</file>\n> \n> As hinted by the zap proposal above, I think the \"on\" and \"off\" states\n> of the button should have related icons. Another option is to keep the\n> same icon, and just update the button state, the same way we handle the\n> \"Start Capture\" button.\n> \n> The feather icons are only fallbacks, we also have to select standard\n> icons from the theme. The latest (but old) version of the icon naming\n> specification is available at\n> https://specifications.freedesktop.org/icon-naming-spec/icon-naming-spec-latest.html.\n> There we have even less options, I've only seen the following ones as\n> possibly relevant:\n> \n> system-run\n> media-playlist-repeat\n> media-playlist-shuffle\n> sync-synchronizing\n> \n> Although there's also text-x-script that may be interesting.\n> \n> Another option would be to rethink the UI and add capture script support\n> somewhere else than in the toolbar.\n\nAs discussed In reply to the 3rd patch I think the capture script ui would\nbe changed.\n\n> \n> >  </qresource>\n> >  </RCC>\n> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> > index 4e773c31..9dc96fbb 100644\n> > --- a/src/qcam/main_window.cpp\n> > +++ b/src/qcam/main_window.cpp\n> > @@ -20,6 +20,7 @@\n> >  #include <QImage>\n> >  #include <QImageWriter>\n> >  #include <QInputDialog>\n> > +#include <QMessageBox>\n> >  #include <QMutexLocker>\n> >  #include <QStandardPaths>\n> >  #include <QStringList>\n> > @@ -233,6 +234,13 @@ int MainWindow::createToolbars()\n> >  \tsaveRaw_ = action;\n> >  #endif\n> >  \n> > +\t/* Open Script... action. */\n> > +\taction = toolbar_->addAction(QIcon::fromTheme(\"document-open\",\n> > +\t\t\t\t\t\t      QIcon(\":file.svg\")),\n> > +\t\t\t\t     \"Open Capture Script\");\n> \n> I'd name this \"Run Capture Script\". The action both opens and runs the\n> script, but the latter seems to be the most important part.\n> \n> > +\tconnect(action, &QAction::triggered, this, &MainWindow::chooseScript);\n> > +\tscriptExecAction_ = action;\n> > +\n> >  \treturn 0;\n> >  }\n> >  \n> > @@ -256,6 +264,60 @@ void MainWindow::updateTitle()\n> >  \tsetWindowTitle(title_ + \" : \" + QString::number(fps, 'f', 2) + \" fps\");\n> >  }\n> >  \n> > +/**\n> > + * \\brief Load a capture script for handling the capture session.\n> > + *\n> > + * If already capturing, it would restart the capture.\n> \n> Why is that ? Could we apply the capture script from the current point\n> without restarting the camera ?\n\nAs far my understanding goes, the pipeline handlers at the very start of\nthe capture do certain special things. So to maintain consistency, we restart\nthe  capture.\n\n> \n> > + */\n> > +void MainWindow::chooseScript()\n> > +{\n> > +\tif (script_) {\n> > +\t\t/*\n> > +\t\t * This is the second valid press of load script button,\n> > +\t\t * It indicates stopping, Stop and set button for new script.\n> \n> Either \", it\" or \". It\", and \", stop\" or \". Stop\".\n> \n> > +\t\t */\n> > +\t\tscript_.reset();\n> > +\t\tscriptExecAction_->setIcon(QIcon::fromTheme(\"document-open\",\n> > +\t\t\t\t\t\t\t    QIcon(\":file.svg\")));\n> > +\t\tscriptExecAction_->setText(\"Open Capture Script\");\n> > +\t\treturn;\n> > +\t}\n> > +\n> > +\tQString scriptFile = QFileDialog::getOpenFileName(this, \"Open Capture Script\", QDir::currentPath(),\n> > +\t\t\t\t\t\t\t  \"Capture Script (*.yaml)\");\n> > +\tif (scriptFile.isEmpty())\n> > +\t\treturn;\n> > +\n> > +\t/*\n> > +\t * If we are already capturing,\n> > +\t * stop so we don't have stuck image in viewfinder.\n> \n> \t * If we are already capturing, stop so we don't have stuck image in\n> \t * viewfinder.\n> \n\nNoted.\n\n> > +\t */\n> > +\tbool wasCapturing = isCapturing_;\n> > +\tif (isCapturing_)\n> > +\t\ttoggleCapture(false);\n> > +\n> > +\tscript_ = std::make_unique<CaptureScript>(camera_, scriptFile.toStdString());\n> > +\tif (!script_->valid()) {\n> > +\t\tscript_.reset();\n> > +\t\tQMessageBox::critical(this, \"Invalid Script\",\n> > +\t\t\t\t      \"Couldn't load the capture script\");\n> > +\t\tif (wasCapturing)\n> > +\t\t\ttoggleCapture(true);\n> > +\t\treturn;\n> > +\t}\n> > +\n> > +\t/*\n> > +\t * Valid script verified\n> > +\t * Set the button to indicate stopping availibility.\n> > +\t */\n> \n> Generally speaking, you shouldn't have line wraps at the end of a\n> sentence in the middle of a paragraph (other than the ones required to\n> limit the line length to 80 columns):\n> \n> \t/*\n> \t * Valid script verified. Set the button to indicate stopping\n> \t * availibility.\n> \t */\n> \n> If you want to separate the two sentences in two paragraphs, you need a\n> blank line:\n> \n> \t/*\n> \t * Valid script verified.\n> \t *\n> \t * Set the button to indicate stopping availibility.\n> \t */\n> \n> I don't think that's what you want here though.\n\nOops, noted.\n\n> \n> > +\tscriptExecAction_->setIcon(QIcon(\":x-square.svg\"));\n> > +\tscriptExecAction_->setText(\"Stop Script execution\");\n> > +\n> > +\t/* Start capture again if we were capturing before. */\n> > +\tif (wasCapturing)\n> > +\t\ttoggleCapture(true);\n> > +}\n> > +\n> >  /* -----------------------------------------------------------------------------\n> >   * Camera Selection\n> >   */\n> > @@ -511,6 +573,7 @@ int MainWindow::startCapture()\n> >  \tpreviousFrames_ = 0;\n> >  \tframesCaptured_ = 0;\n> >  \tlastBufferTime_ = 0;\n> > +\tqueueCount_ = 0;\n> >  \n> >  \tret = camera_->start();\n> >  \tif (ret) {\n> > @@ -790,5 +853,10 @@ void MainWindow::renderComplete(FrameBuffer *buffer)\n> >  \n> >  int MainWindow::queueRequest(Request *request)\n> >  {\n> > +\tif (script_)\n> > +\t\trequest->controls() = script_->frameControls(queueCount_);\n> > +\n> > +\tqueueCount_++;\n> > +\n> >  \treturn camera_->queueRequest(request);\n> >  }\n> > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\n> > index bc844711..eb398c1d 100644\n> > --- a/src/qcam/main_window.h\n> > +++ b/src/qcam/main_window.h\n> > @@ -26,6 +26,7 @@\n> >  #include <QQueue>\n> >  #include <QTimer>\n> >  \n> > +#include \"../cam/capture_script.h\"\n> >  #include \"../cam/stream_options.h\"\n> >  \n> >  #include \"viewfinder.h\"\n> > @@ -87,11 +88,14 @@ private:\n> >  \tvoid processHotplug(HotplugEvent *e);\n> >  \tvoid processViewfinder(libcamera::FrameBuffer *buffer);\n> >  \n> > +\tvoid chooseScript();\n> > +\n> >  \t/* UI elements */\n> >  \tQToolBar *toolbar_;\n> >  \tQAction *startStopAction_;\n> >  \tQComboBox *cameraCombo_;\n> >  \tQAction *saveRaw_;\n> > +\tQAction *scriptExecAction_;\n> >  \tViewFinder *viewfinder_;\n> >  \n> >  \tQIcon iconPlay_;\n> > @@ -125,6 +129,8 @@ private:\n> >  \tQElapsedTimer frameRateInterval_;\n> >  \tuint32_t previousFrames_;\n> >  \tuint32_t framesCaptured_;\n> > +\tuint32_t queueCount_;\n> >  \n> >  \tstd::vector<std::unique_ptr<libcamera::Request>> requests_;\n> > +\tstd::unique_ptr<CaptureScript> script_;\n> >  };\n> > diff --git a/src/qcam/meson.build b/src/qcam/meson.build\n> > index c46f4631..67074252 100644\n> > --- a/src/qcam/meson.build\n> > +++ b/src/qcam/meson.build\n> > @@ -15,6 +15,7 @@ endif\n> >  qcam_enabled = true\n> >  \n> >  qcam_sources = files([\n> > +    '../cam/capture_script.cpp',\n> >      '../cam/image.cpp',\n> >      '../cam/options.cpp',\n> >      '../cam/stream_options.cpp',\n> \n> At some point we should probably create an application helpers library\n> shared between cam and qcam. Out of scope for this series of course.\n> \n> > @@ -37,6 +38,7 @@ qcam_resources = files([\n> >  qcam_deps = [\n> >      libatomic,\n> >      libcamera_public,\n> > +    libyaml,\n> >      qt5_dep,\n> >  ]\n> >  \n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 1396BBE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 28 Jul 2022 17:16:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4EB5463312;\n\tThu, 28 Jul 2022 19:16:27 +0200 (CEST)","from mail-pg1-x52c.google.com (mail-pg1-x52c.google.com\n\t[IPv6:2607:f8b0:4864:20::52c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 08FC56330D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Jul 2022 19:16:26 +0200 (CEST)","by mail-pg1-x52c.google.com with SMTP id f65so2005714pgc.12\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Jul 2022 10:16:25 -0700 (PDT)","from gmail.com ([2404:bd00:3:d6bf:e341:92da:d751:c081])\n\tby smtp.gmail.com with ESMTPSA id\n\tk15-20020aa7998f000000b0052592a8ef62sm1029054pfh.110.2022.07.28.10.16.21\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 28 Jul 2022 10:16:22 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659028587;\n\tbh=YSJvGs71tf3B01yuXWgwZxtpYkcDOHeSdvLJAbqUma8=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=Wd0dhLkLmM4u/wU+6IoSJbDoeg+DWAvFSXaP8hIeHuIvC/2wAO/ntikxEUw7bg9gv\n\t0O0Y9n3S9MuEle4arLW+mGz6pBfOaXdcfk7DW9KoD1w0uqjeHJ8CYnH6Ot+bVlzLJN\n\tCMI9ILDn1pT0U/fXkb6GCYOz9fLsmZ0+jEeI4EvZeAAK6IjyPCavIMRhz55JgEk8wA\n\tR5WfG5hYs8vFd05XFdw4uMAl/2MFnXXW/ZC2PWAvP8zGy9DqDqXRF+PtIaD9gNAV0t\n\tXtnTzyInBKy8/E59j2paWJFz1JW1rhrlLkUMqmcbLd8X6wHr+IHPn5gMftQG29xzTj\n\taxHo4bfh9hK+A==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=date:from:to:cc:subject:message-id:mail-followup-to:references\n\t:mime-version:content-disposition:in-reply-to;\n\tbh=dVhmt7NSO1vwG5+Dqs5uH5siO2k8J9YjFonmepemjRs=;\n\tb=gheOAaqGtIrRV71kz17kN4/w6UWsCIxxVrsOvlfvPcdojiJ/XcTIUWVY8heLSGU56z\n\trTYXaJd6XSNB3j0LmuLVuiGI+NILNZ31uvswCW8cUgVk6y7mV4DhOwrjvo/7OMubTe+A\n\tURKW4NGEhHBKCPky8xJkuRIHiEP3Ey9svtvGAh5oZaC08aFaU+xSLkGrpLq69/LoFkmY\n\tqIczPJhampqTSq+mDN9aG0WnVa0wN/buNvaLxwBaOZ1txcZnOx2vB2Aszojz+fJLObof\n\tFbRAQJFpS3sWEjVXhV7F6WHLMjvMG9ABy0frFcJwT5XOBGYsxuwR9FHk3O0mqWx7kZQi\n\tcTHQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"gheOAaqG\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id\n\t:mail-followup-to:references:mime-version:content-disposition\n\t:in-reply-to;\n\tbh=dVhmt7NSO1vwG5+Dqs5uH5siO2k8J9YjFonmepemjRs=;\n\tb=IcW6VGV396zVGudvsxJWC6Z+dE+VVXKW/Ukikbv6F/QCOXAD5ElFaTvsE2oO/X7Rug\n\tdm5KaKy3bpbuRCeHIF/8QG6haHQD5ah/4mbuJ6gAzQg7YBm3adFZTUoi792lJJXR4DiY\n\tzFUWJaZfrOX2PPXPfL1gq8fn5CtfZFMxE5tUb3KgFsKH0HfwUF6xuOP9CrpXBjW4YycY\n\tvygZYluT8QF13rbUiZqaB3ZttJIcpVamuRc+XQ8i7ITXmLyd/cIa6dOMtLzhj3BSXuar\n\tb1L7hiz98K9Q8g/ZcJPtusOSJwVdPk6yEfkGApiGi0PoDqGNTCQZNVGsD8aAVa5+5uEm\n\tLwlA==","X-Gm-Message-State":"AJIora9U4UdjcXaloi/2nyn0oefDzGmj14CED+JhhzU13S/QHT7nZIsc\n\to8z8QJzET4RKK8Zxs3FDVb4=","X-Google-Smtp-Source":"AGRyM1tIkt9drUn1A21ytXn03EBOtkUwgyMsJIhV8HD+3hpy/Xd2lw7lAGxpEYuz6pZAnZmrJfwFuQ==","X-Received":"by 2002:a05:6a00:1a49:b0:528:6ea0:14e2 with SMTP id\n\th9-20020a056a001a4900b005286ea014e2mr27317686pfv.22.1659028583357; \n\tThu, 28 Jul 2022 10:16:23 -0700 (PDT)","Date":"Thu, 28 Jul 2022 22:46:19 +0530","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20220728171619.GA186879@gmail.com>","Mail-Followup-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220726194123.170208-1-utkarsh02t@gmail.com>\n\t<20220726194123.170208-2-utkarsh02t@gmail.com>\n\t<YuBm9P0TDgKUb5+N@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<YuBm9P0TDgKUb5+N@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 2/3] qcam: Add a GUI way to use\n\tcapture script","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>","From":"Utkarsh Tiwari via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Utkarsh Tiwari <utkarsh02t@gmail.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]