[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