[libcamera-devel] [PATCH 2/3] qcam: Add a GUI way to use capture script

[Laurent Pinchart]

On Sat, Jun 18, 2022 at 04:16:50PM +0100, Kieran Bingham via libcamera-devel wrote:
> Quoting Utkarsh Tiwari via libcamera-devel (2022-06-13 07:03:44)
> > Implement an Open Capture Script button which would allow the user to open a Capture Script (*.yaml). When clicked present them with a QFileDialog to allow them to select a single file.
> > 
> > Introduce a queueCount_ to keep track of the requests queued.
> > 
> > Initialize a Script Parser instance when the user selects a valid capture script. At queueRequest() time if script parser has been initialized, then populate the Request::controls() with it at queueRequest time providing the queueCount_.
> > The queueCount_ is incremented after the getting controls from the parser, so the first request is for frame 0.
> 
> Could you wrap your commit messages please?
> 
> I think 72 chars is the usual git commit message length to wrap at.
> 
> https://cbea.ms/git-commit/ is always a pretty good read for this, but I
> think you're already hitting most of the points listed there pretty
> well.
> 
> 
> > 
> > Signed-off-by: Utkarsh Tiwari <utkarsh02t at gmail.com>
> > ---
> >  src/qcam/assets/feathericons/feathericons.qrc |  1 +
> >  src/qcam/main_window.cpp                      | 38 +++++++++++++++++++
> >  src/qcam/main_window.h                        |  5 +++
> >  src/qcam/meson.build                          |  2 +
> >  4 files changed, 46 insertions(+)
> > 
> > diff --git a/src/qcam/assets/feathericons/feathericons.qrc b/src/qcam/assets/feathericons/feathericons.qrc
> > index c5302040..f0d1e3d0 100644
> > --- a/src/qcam/assets/feathericons/feathericons.qrc
> > +++ b/src/qcam/assets/feathericons/feathericons.qrc
> > @@ -3,6 +3,7 @@
> >  <qresource>
> >         <file>aperture.svg</file>
> >         <file>camera-off.svg</file>
> > +       <file>file.svg</file>
> >         <file>play-circle.svg</file>
> >         <file>save.svg</file>
> >         <file>stop-circle.svg</file>
> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> > index 5ac31d9a..7cdb1238 100644
> > --- a/src/qcam/main_window.cpp
> > +++ b/src/qcam/main_window.cpp
> > @@ -20,6 +20,7 @@
> >  #include <QImage>
> >  #include <QImageWriter>
> >  #include <QInputDialog>
> > +#include <QMessageBox>
> >  #include <QMutexLocker>
> >  #include <QStandardPaths>
> >  #include <QStringList>
> > @@ -232,6 +233,12 @@ int MainWindow::createToolbars()
> >         saveRaw_ = action;
> >  #endif
> >  
> > +       /* Open Script... action. */
> > +       action = toolbar_->addAction(QIcon::fromTheme("document-open",
> > +                                                     QIcon(":file.svg")),
> > +                                    "Open Capture Script");
> > +       connect(action, &QAction::triggered, this, &MainWindow::chooseScript);
> > +
> >         return 0;
> >  }
> >  
> > @@ -255,6 +262,31 @@ void MainWindow::updateTitle()
> >         setWindowTitle(title_ + " : " + QString::number(fps, 'f', 2) + " fps");
> >  }
> >  
> > +/**
> > + *     \brief Load a capture script for handling the capture session.
> > + *
> > + *     If already capturing, it would restart the capture.
> 
> I think those could be slightly less indented, i.e.
> 
> /**
>  * \brief ...
>  *
>  * ...
>  */
> 
> 
> > + */
> > +void MainWindow::chooseScript()
> > +{
> > +       QString scriptFile = QFileDialog::getOpenFileName(this, tr("Open Capture Script"), QDir::currentPath(),
> > +                                                         tr("Capture Script (*.yaml)"));
> > +       if (scriptFile.isEmpty())
> > +               return;
> > +       script_ = std::make_unique<CaptureScript>(camera_, scriptFile.toStdString());

What happens if the user tries to load another capture script while
capture is running ? Can this race with MainWindow::queueRequest() ?

>From a GUI point of view, if we have a way to load a capture script, do
we need a way to also disable it ?

> > +       if (!script_->valid()) {
> > +               script_.reset();
> > +               QMessageBox::critical(this, tr("Invalid Script"), tr("Couldn't execute the capture script"));
> > +               return;
> > +       }
> > +
> > +       //Restart the capture so we can reset every counter
> 
> Please use a consistent comment style:
> 
> 	/* Restart the capture so we can reset counters. */
> 
> With those fixed, then
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> I've tested this and can see the brightness values change on my UVC
> camera as expected.
> 
> Soon I can anticipate we'll want to extend the script language to
> support saving a file out but that can be later.
> 
> 
> Tested-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> 
> > +       if (isCapturing_) {
> > +               toggleCapture(false);
> > +               toggleCapture(true);
> > +       }
> > +}
> > +
> >  /* -----------------------------------------------------------------------------
> >   * Camera Selection
> >   */
> > @@ -510,6 +542,7 @@ int MainWindow::startCapture()
> >         previousFrames_ = 0;
> >         framesCaptured_ = 0;
> >         lastBufferTime_ = 0;
> > +       queueCount_ = 0;
> >  
> >         ret = camera_->start();
> >         if (ret) {
> > @@ -789,5 +822,10 @@ void MainWindow::refillRequest(FrameBuffer *buffer)
> >  
> >  int MainWindow::queueRequest(Request *request)
> >  {
> > +       if (script_)
> > +               request->controls() = script_->frameControls(queueCount_);
> > +
> > +       queueCount_++;
> > +
> >         return camera_->queueRequest(request);
> >  }
> > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> > index 251b8335..cde5842f 100644
> > --- a/src/qcam/main_window.h
> > +++ b/src/qcam/main_window.h
> > @@ -26,6 +26,7 @@
> >  #include <libcamera/request.h>
> >  #include <libcamera/stream.h>
> >  
> > +#include "../cam/capture_script.h"
> >  #include "../cam/stream_options.h"
> >  #include "viewfinder.h"
> >  
> > @@ -86,6 +87,8 @@ private:
> >         void processHotplug(HotplugEvent *e);
> >         void processViewfinder(libcamera::FrameBuffer *buffer);
> >  
> > +       void chooseScript();
> > +
> >         /* UI elements */
> >         QToolBar *toolbar_;
> >         QAction *startStopAction_;
> > @@ -124,6 +127,8 @@ private:
> >         QElapsedTimer frameRateInterval_;
> >         uint32_t previousFrames_;
> >         uint32_t framesCaptured_;
> > +       uint32_t queueCount_;
> >  
> >         std::vector<std::unique_ptr<libcamera::Request>> requests_;
> > +       std::unique_ptr<CaptureScript> script_;
> >  };
> > diff --git a/src/qcam/meson.build b/src/qcam/meson.build
> > index c46f4631..67074252 100644
> > --- a/src/qcam/meson.build
> > +++ b/src/qcam/meson.build
> > @@ -15,6 +15,7 @@ endif
> >  qcam_enabled = true
> >  
> >  qcam_sources = files([
> > +    '../cam/capture_script.cpp',
> >      '../cam/image.cpp',
> >      '../cam/options.cpp',
> >      '../cam/stream_options.cpp',
> > @@ -37,6 +38,7 @@ qcam_resources = files([
> >  qcam_deps = [
> >      libatomic,
> >      libcamera_public,
> > +    libyaml,
> >      qt5_dep,
> >  ]
> >  

-- 
Regards,

Laurent Pinchart