[libcamera-devel] [PATCH v9 6/7] qcam: CamSelectDialog: Add capture script button

Kieran Bingham kieran.bingham at ideasonboard.com
Wed Aug 31 11:35:31 CEST 2022


Quoting Utkarsh Tiwari via libcamera-devel (2022-08-31 06:49:37)
> Display a QLabel which is readonly, and displays the currently
> selected capture script. The tooltip of the QLabel displays the file
> path of the script.
> 
> Implement a capture script button which is a QToolButton which when
> clicked opens a QFileDialog this allows to select a capture script
> (*.yaml) file.
> 
> Next to the capture scipt button, show a QToolButton which stops the
> capture script.
> 
> If an invalid script has been selected show a QMesssageBox::critical and
> continue with the capture's previous state.
> 
> Introduce a queueCount_ to keep track of the requests queued.
> 
> When stopping the execution of the capture script the queueCount_ is not
> reset and the capture continues as it is (i.e it is not stopped or
> restarted).
> 
> Requests are queued with any controls the script matching the current
> queueCount_.

"from the script" ?

> ---
> Differnce from v8:
>     1. Now display a QLabel with the fileName and filePath with button
>     on the side.
>     2. infromScriptReset() informScriptRunning() are removed
>     3. Local script makes handling of invalid scripts easy.
>  src/qcam/assets/feathericons/feathericons.qrc |  2 +
>  src/qcam/cam_select_dialog.cpp                | 88 ++++++++++++++++++-
>  src/qcam/cam_select_dialog.h                  | 20 ++++-
>  src/qcam/main_window.cpp                      | 67 +++++++++++++-
>  src/qcam/main_window.h                        |  7 ++
>  src/qcam/meson.build                          |  2 +
>  6 files changed, 181 insertions(+), 5 deletions(-)
> 
> diff --git a/src/qcam/assets/feathericons/feathericons.qrc b/src/qcam/assets/feathericons/feathericons.qrc
> index c5302040..0ea0c2d5 100644
> --- a/src/qcam/assets/feathericons/feathericons.qrc
> +++ b/src/qcam/assets/feathericons/feathericons.qrc
> @@ -3,7 +3,9 @@
>  <qresource>
>         <file>aperture.svg</file>
>         <file>camera-off.svg</file>
> +    <file>delete.svg</file>
>         <file>play-circle.svg</file>
> +    <file>upload.svg</file>

Indentations are wrong here.


>         <file>save.svg</file>
>         <file>stop-circle.svg</file>
>         <file>x-circle.svg</file>
> diff --git a/src/qcam/cam_select_dialog.cpp b/src/qcam/cam_select_dialog.cpp
> index 6543228a..99405cc1 100644
> --- a/src/qcam/cam_select_dialog.cpp
> +++ b/src/qcam/cam_select_dialog.cpp
> @@ -14,13 +14,18 @@
>  
>  #include <QComboBox>
>  #include <QDialogButtonBox>
> +#include <QFileDialog>
>  #include <QFormLayout>
> +#include <QHBoxLayout>
> +#include <QIcon>
>  #include <QLabel>
>  #include <QString>
> +#include <QToolButton>
> +#include <QWidget>
>  
>  CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManager,
> -                                          QWidget *parent)
> -       : QDialog(parent), cm_(cameraManager)
> +                                          std::string scriptPath, QWidget *parent)
> +       : QDialog(parent), cm_(cameraManager), scriptPath_(scriptPath)
>  {
>         /* Use a QFormLayout for the dialog. */
>         QFormLayout *layout = new QFormLayout(this);
> @@ -38,6 +43,41 @@ CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManag
>         connect(cameraIdComboBox_, &QComboBox::currentTextChanged,
>                 this, &CameraSelectorDialog::updateCamInfo);
>  
> +       /* Set capture script selection / removal button. */
> +       QWidget *captureWidget = new QWidget(this);
> +       QHBoxLayout *captureLayout = new QHBoxLayout(captureWidget);
> +
> +       scriptFileLine_ = new QLabel;
> +       scriptFileLine_->setFrameStyle(QFrame::Sunken | QFrame::StyledPanel);
> +
> +       chooseCaptureScriptButton_ = new QToolButton;
> +       chooseCaptureScriptButton_->setIcon(QIcon::fromTheme("document-open",
> +                                                            QIcon(":upload.svg")));
> +       chooseCaptureScriptButton_->setStyleSheet("border:none");
> +       connect(chooseCaptureScriptButton_, &QToolButton::clicked,
> +               this, &CameraSelectorDialog::selectCaptureScript);
> +
> +       QToolButton *stopCaptureScriptButton = new QToolButton;
> +       stopCaptureScriptButton->setIcon(QIcon::fromTheme("edit-clear",
> +                                                         QIcon(":delete.svg")));
> +       stopCaptureScriptButton->setStyleSheet("border:node;");
> +       connect(stopCaptureScriptButton, &QToolButton::clicked,
> +               this, &CameraSelectorDialog::resetCaptureScript);
> +
> +       captureLayout->addWidget(scriptFileLine_);
> +       captureLayout->addWidget(chooseCaptureScriptButton_);
> +       captureLayout->addWidget(stopCaptureScriptButton);
> +       captureLayout->setMargin(0);
> +
> +       /* Set the file name of the capture script. */
> +       if (scriptPath_.empty()) {
> +               scriptFileLine_->setText("No File Selected");
> +       } else {
> +               scriptFileInfo_.setFile(QString::fromStdString(scriptPath_));
> +               scriptFileLine_->setText(scriptFileInfo_.fileName());
> +               scriptFileLine_->setToolTip(scriptFileInfo_.filePath());
> +       }
> +
>         /* Setup the QDialogButton Box */
>         QDialogButtonBox *buttonBox =
>                 new QDialogButtonBox(QDialogButtonBox::Ok |
> @@ -52,6 +92,7 @@ CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManag
>         layout->addRow("Camera:", cameraIdComboBox_);
>         layout->addRow("Location:", cameraLocation_);
>         layout->addRow("Model:", cameraModel_);
> +       layout->addRow("Capture Script:", captureWidget);
>         layout->addWidget(buttonBox);
>  }
>  
> @@ -110,3 +151,46 @@ void CameraSelectorDialog::updateCamInfo(QString cameraId)
>  
>         cameraModel_->setText(QString::fromStdString(model));
>  }
> +
> +/* Capture script support. */
> +void CameraSelectorDialog::selectCaptureScript()
> +{
> +       selectedScriptPath_ = QFileDialog::getOpenFileName(this,
> +                                                          "Run Capture Script", QDir::currentPath(),
> +                                                          "Capture Script (*.yaml)")
> +                                     .toStdString();

Where does checkstyle recommend to place this? It seems to be 'floating'
too.

It's tricky, but I'm not sure there is much better options anyway, so
I'll move on.

> +
> +       if (!selectedScriptPath_.empty()) {
> +               scriptFileInfo_.setFile(QString::fromStdString(selectedScriptPath_));
> +               scriptFileLine_->setText(scriptFileInfo_.fileName());
> +               scriptFileLine_->setToolTip(scriptFileInfo_.filePath());
> +       } else {
> +               selectedScriptPath_ = scriptPath_;
> +       }
> +}
> +
> +void CameraSelectorDialog::resetCaptureScript()
> +{
> +       Q_EMIT stopCaptureScript();
> +       scriptPath_.clear();
> +       selectedScriptPath_.clear();
> +       scriptFileLine_->setText("No File Selected");
> +}
> +
> +void CameraSelectorDialog::accept()
> +{
> +       scriptPath_ = selectedScriptPath_;
> +       QDialog::accept();
> +}
> +
> +void CameraSelectorDialog::reject()
> +{
> +       if (scriptPath_.empty()) {
> +               scriptFileLine_->setText("No File Selected");
> +       } else {
> +               scriptFileInfo_.setFile(QString::fromStdString(scriptPath_));
> +               scriptFileLine_->setText(scriptFileInfo_.fileName());
> +               scriptFileLine_->setToolTip(scriptFileInfo_.filePath());
> +       }
> +       QDialog::reject();
> +}
> diff --git a/src/qcam/cam_select_dialog.h b/src/qcam/cam_select_dialog.h
> index c91b7ebe..377faebc 100644
> --- a/src/qcam/cam_select_dialog.h
> +++ b/src/qcam/cam_select_dialog.h
> @@ -15,21 +15,24 @@
>  #include <libcamera/property_ids.h>
>  
>  #include <QDialog>
> +#include <QFileInfo>
>  #include <QString>
>  
>  class QComboBox;
>  class QLabel;
> +class QToolButton;
>  
>  class CameraSelectorDialog : public QDialog
>  {
>         Q_OBJECT
>  public:
>         CameraSelectorDialog(libcamera::CameraManager *cameraManager,
> -                            QWidget *parent);
> +                            std::string scriptPath, QWidget *parent);
>  
>         ~CameraSelectorDialog();
>  
>         std::string getCameraId();
> +       std::string getCaptureScript() { return scriptPath_; };
>  
>         /* Hotplug / Unplug Support. */
>         void addCamera(QString cameraId);
> @@ -38,11 +41,26 @@ public:
>         /* Camera Information */
>         void updateCamInfo(QString cameraId);
>  
> +       /* Capture script support. */
> +       void selectCaptureScript();
> +       void resetCaptureScript();
> +
> +       void accept() override;
> +       void reject() override;
> +
> +Q_SIGNALS:
> +       void stopCaptureScript();
> +
>  private:
>         libcamera::CameraManager *cm_;
>  
> +       std::string scriptPath_;
> +       std::string selectedScriptPath_;
> +       QFileInfo scriptFileInfo_;
>         /* UI elements. */
>         QComboBox *cameraIdComboBox_;
>         QLabel *cameraLocation_;
>         QLabel *cameraModel_;
> +       QLabel *scriptFileLine_;
> +       QToolButton *chooseCaptureScriptButton_;
>  };
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 2a9ca830..af992b94 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -9,6 +9,7 @@
>  
>  #include <assert.h>
>  #include <iomanip>
> +#include <memory>
>  #include <string>
>  
>  #include <libcamera/camera_manager.h>
> @@ -18,6 +19,7 @@
>  #include <QFileDialog>
>  #include <QImage>
>  #include <QImageWriter>
> +#include <QMessageBox>
>  #include <QMutexLocker>
>  #include <QStandardPaths>
>  #include <QStringList>
> @@ -143,7 +145,9 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
>         cm_->cameraAdded.connect(this, &MainWindow::addCamera);
>         cm_->cameraRemoved.connect(this, &MainWindow::removeCamera);
>  
> -       cameraSelectorDialog_ = new CameraSelectorDialog(cm_, this);
> +       cameraSelectorDialog_ = new CameraSelectorDialog(cm_, scriptPath_, this);
> +       connect(cameraSelectorDialog_, &CameraSelectorDialog::stopCaptureScript,
> +               this, &MainWindow::stopCaptureScript);
>  
>         /* Open the camera and start capture. */
>         ret = openCamera();
> @@ -152,6 +156,10 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
>                 return;
>         }
>  
> +       /* Start capture script. */
> +       if (!scriptPath_.empty())
> +               ret = loadCaptureScript();
> +
>         startStopAction_->setChecked(true);
>  }
>  
> @@ -266,8 +274,11 @@ void MainWindow::switchCamera()
>         if (newCameraId.empty())
>                 return;
>  
> -       if (camera_ && newCameraId == camera_->id())
> +       if (camera_ && newCameraId == camera_->id()) {
> +               // When user opens camera selection dialog for CaptureScript selection

Perhaps:
		/*
		 * If the camera has not changed, We still need to
		 * handle any update to the capture script. This will
		 * cause the script to restart if it was already
		 * selected.
		 */


Or something like that (and correctly formatted).


> +               loadCaptureScript();
>                 return;
> +       }
>  
>         const std::shared_ptr<Camera> &cam = cm_->get(newCameraId);
>  
> @@ -287,17 +298,63 @@ void MainWindow::switchCamera()
>         camera_->release();
>         camera_ = cam;
>  
> +       loadCaptureScript();
> +
>         startStopAction_->setChecked(true);
>  
>         /* Display the current cameraId in the toolbar .*/
>         cameraSelectButton_->setText(QString::fromStdString(newCameraId));
>  }
>  
> +void MainWindow::stopCaptureScript()
> +{
> +       if (script_)
> +               script_.reset();
> +}

This needs a blank line.

> +/**
> + * \brief Loads and validates the current capture script
> + *

Can we describe here that the current capture session will be restarted
if it was active?

> + * returns -EINVAL on failure and 0 on success
> + */
> +int MainWindow::loadCaptureScript()
> +{
> +       if (scriptPath_.empty() || camera_ == nullptr)
> +               return -EINVAL;

Perhaps -ENODEV? But I expect this can't happen. I think it's fine to be
defensive here though.

> +
> +       auto script = std::make_unique<CaptureScript>(camera_, scriptPath_);
> +
> +       if (!script->valid()) {
> +               script.reset();
> +
> +               QMessageBox::critical(this, "Invalid Script",
> +                                     "Couldn't load the capture script");
> +
> +               return -EINVAL;
> +       }
> +
> +       /*
> +        * If we are already capturing, stop so we don't have stuck image

"have a stuck image"

> +        * in viewfinder.

"in the viewfinder"

> +        */
> +       bool wasCapturing = isCapturing_;
> +       if (isCapturing_)
> +               toggleCapture(false);
> +
> +       script_ = std::move(script);
> +
> +       /* Start capture again if we were capturing before. */
> +       if (wasCapturing)
> +               toggleCapture(true);

New line here.


All of those comments seem quite minor, so with those resolved:


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> +       return 0;
> +}
> +
>  std::string MainWindow::chooseCamera()
>  {
>         if (cameraSelectorDialog_->exec() != QDialog::Accepted)
>                 return std::string();
>  
> +       scriptPath_ = cameraSelectorDialog_->getCaptureScript();
> +
>         return cameraSelectorDialog_->getCameraId();
>  }
>  
> @@ -499,6 +556,7 @@ int MainWindow::startCapture()
>         previousFrames_ = 0;
>         framesCaptured_ = 0;
>         lastBufferTime_ = 0;
> +       queueCount_ = 0;
>  
>         ret = camera_->start();
>         if (ret) {
> @@ -777,5 +835,10 @@ void MainWindow::renderComplete(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 22c85247..7c877ae1 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -27,6 +27,7 @@
>  #include <QQueue>
>  #include <QTimer>
>  
> +#include "../cam/capture_script.h"
>  #include "../cam/stream_options.h"
>  
>  #include "viewfinder.h"
> @@ -89,6 +90,9 @@ private:
>         void processHotplug(HotplugEvent *e);
>         void processViewfinder(libcamera::FrameBuffer *buffer);
>  
> +       int loadCaptureScript();
> +       void stopCaptureScript();
> +
>         /* UI elements */
>         QToolBar *toolbar_;
>         QAction *startStopAction_;
> @@ -129,6 +133,9 @@ 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_;
> +       std::string scriptPath_;
>  };
> diff --git a/src/qcam/meson.build b/src/qcam/meson.build
> index 61861ea6..70a18d7e 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',
> @@ -39,6 +40,7 @@ qcam_resources = files([
>  qcam_deps = [
>      libatomic,
>      libcamera_public,
> +    libyaml,
>      qt5_dep,
>  ]
>  
> -- 
> 2.34.1
>


More information about the libcamera-devel mailing list