<div dir="ltr"><div dir="ltr">Hi Kieran thanks for the review.</div><div><br></div><div>I accept all the grammetical mistakes.<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Aug 10, 2022 at 3:29 AM Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Quoting Utkarsh Tiwari via libcamera-devel (2022-08-09 21:50:40)<br>
> Implement an Capture Script in CamSelectDialog button which would allow<br>
> the user to open a Capture Script (*.yaml).<br>
> This button has three states :<br>
> - Open Capture Script<br>
> - Loaded<br>
> - Stop the execution of current capture script<br>
> <br>
> When being clicked in open state, present them with a QFileDialog to<br>
<br>
"When clicked in an open state, present the user with" ...<br>
<br>
> allow user to select a single file. When the script is loaded the button<br>
<br>
"to allow selecting a single file." ...<br>
<br>
> displays "Loaded", the script has not been verified yet. Verifying the<br>
> script and executing it happens after user presses Ok.<br>
> <br>
> Introduce a queueCount_ to keep track of the requests queued.<br>
> <br>
> When stopping the execution of the capture script the queueCount_ is not<br>
> reseted and the capture is continues as it is (i.e it is not stopped or<br>
<br>
s/reseted/reset/<br>
<br>
s/capture is continues/capture continues/<br>
<br>
> restarted).<br>
> <br>
> Requests are queued with any controls the script matching the current<br>
> queueCount_.<br>
> <br>
> Signed-off-by: Utkarsh Tiwari <<a href="mailto:utkarsh02t@gmail.com" target="_blank">utkarsh02t@gmail.com</a>><br>
> ---<br>
> Difference:<br>
> 1. override accept and reject to handle capture script paths.<br>
> src/qcam/cam_select_dialog.cpp | 68 ++++++++++++++++++++++++++++++++--<br>
> src/qcam/cam_select_dialog.h | 21 ++++++++++-<br>
> src/qcam/main_window.cpp | 59 ++++++++++++++++++++++++++++-<br>
> src/qcam/main_window.h | 7 ++++<br>
> src/qcam/meson.build | 2 +<br>
> 5 files changed, 152 insertions(+), 5 deletions(-)<br>
> <br>
> diff --git a/src/qcam/cam_select_dialog.cpp b/src/qcam/cam_select_dialog.cpp<br>
> index f97ad6eb..f3df9970 100644<br>
> --- a/src/qcam/cam_select_dialog.cpp<br>
> +++ b/src/qcam/cam_select_dialog.cpp<br>
> @@ -20,8 +20,8 @@<br>
> #include <QString><br>
> <br>
> CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManager,<br>
> - QWidget *parent)<br>
> - : QDialog(parent), cm_(cameraManager)<br>
> + bool isScriptRunning, QWidget *parent)<br>
> + : QDialog(parent), cm_(cameraManager), isScriptRunning_(isScriptRunning)<br>
> {<br>
> /* Use a QFormLayout for the dialog. */<br>
> QFormLayout *layout = new QFormLayout(this);<br>
> @@ -39,6 +39,16 @@ CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManag<br>
> connect(cameraIdComboBox_, &QComboBox::currentTextChanged,<br>
> this, &CameraSelectorDialog::handleCameraChange);<br>
> <br>
> + captureScriptButton_ = new QPushButton;<br>
> + connect(captureScriptButton_, &QPushButton::clicked,<br>
> + this, &CameraSelectorDialog::handleCaptureScriptButton);<br>
> +<br>
> + /* Display the action that would be performed when button is clicked. */<br>
> + if (isScriptRunning_)<br>
> + captureScriptButton_->setText("Stop");<br>
> + else<br>
> + captureScriptButton_->setText("Open");<br>
> +<br>
> /* Setup the QDialogButton Box */<br>
> QDialogButtonBox *buttonBox =<br>
> new QDialogButtonBox(QDialogButtonBox::Ok |<br>
> @@ -50,10 +60,10 @@ CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManag<br>
> this, &QDialog::reject);<br>
> <br>
> /* Set the layout. */<br>
> -<br>
> layout->addRow("Camera:", cameraIdComboBox_);<br>
> layout->addRow("Location:", cameraLocation_);<br>
> layout->addRow("Model:", cameraModel_);<br>
> + layout->addRow("Capture Script:", captureScriptButton_);<br>
> layout->addWidget(buttonBox);<br>
> }<br>
> <br>
> @@ -62,6 +72,11 @@ std::string CameraSelectorDialog::getCameraId()<br>
> return cameraIdComboBox_->currentText().toStdString();<br>
> }<br>
> <br>
> +std::string CameraSelectorDialog::getCaptureScript()<br>
> +{<br>
> + return scriptPath_;<br>
> +}<br>
> +<br>
> /* Hotplug / Unplug Support. */<br>
> void CameraSelectorDialog::cameraAdded(libcamera::Camera *camera)<br>
> {<br>
> @@ -115,3 +130,50 @@ void CameraSelectorDialog::updateCamInfo(const std::shared_ptr<libcamera::Camera<br>
> <br>
> cameraModel_->setText(QString::fromStdString(model));<br>
> }<br>
> +<br>
> +/* Capture script support. */<br>
> +void CameraSelectorDialog::handleCaptureScriptButton()<br>
> +{<br>
> + if (isScriptRunning_) {<br>
> + Q_EMIT stopCaptureScript();<br>
> + isScriptRunning_ = false;<br>
> + captureScriptButton_->setText("Open");<br>
> + } else {<br>
> + selectedScriptPath_ = QFileDialog::getOpenFileName(this,<br>
> + "Run Capture Script", QDir::currentPath(),<br>
> + "Capture Script (*.yaml)")<br>
> + .toStdString();<br>
> +<br>
> + if (!selectedScriptPath_.empty())<br>
> + captureScriptButton_->setText("Loaded");<br>
> + else<br>
> + captureScriptButton_->setText("Open");<br>
> + }<br>
> +}<br>
> +<br>
> +void CameraSelectorDialog::accept()<br>
> +{<br>
> + scriptPath_ = selectedScriptPath_;<br>
> + QDialog::accept();<br>
> +}<br>
> +<br>
> +void CameraSelectorDialog::reject()<br>
> +{<br>
> + if (isScriptRunning_)<br>
> + selectedScriptPath_ = scriptPath_;<br>
<br>
What is this for ?<br>
<br>
<br>
> + QDialog::reject();<br>
> +}<br>
> +<br>
> +void CameraSelectorDialog::informScriptReset()<br>
> +{<br>
> + isScriptRunning_ = false;<br>
> + scriptPath_.clear();<br>
> + captureScriptButton_->setText("Open");<br>
> +}<br>
> +<br>
> +void CameraSelectorDialog::informScriptRunning(std::string scriptPath)<br>
> +{<br>
> + isScriptRunning_ = true;<br>
> + scriptPath_ = scriptPath;<br>
> + captureScriptButton_->setText("Stop");<br>
> +}<br>
> diff --git a/src/qcam/cam_select_dialog.h b/src/qcam/cam_select_dialog.h<br>
> index 359df811..56d90596 100644<br>
> --- a/src/qcam/cam_select_dialog.h<br>
> +++ b/src/qcam/cam_select_dialog.h<br>
> @@ -17,8 +17,10 @@<br>
> #include <QComboBox><br>
> #include <QDialog><br>
> #include <QDialogButtonBox><br>
> +#include <QFileDialog><br>
> #include <QFormLayout><br>
> #include <QLabel><br>
> +#include <QPushButton><br>
> #include <QString><br>
> <br>
> class CameraSelectorDialog : public QDialog<br>
> @@ -26,12 +28,14 @@ class CameraSelectorDialog : public QDialog<br>
> Q_OBJECT<br>
> public:<br>
> CameraSelectorDialog(libcamera::CameraManager *cameraManager,<br>
> - QWidget *parent);<br>
> + bool isScriptRunning, QWidget *parent);<br>
> <br>
> ~CameraSelectorDialog() = default;<br>
> <br>
> std::string getCameraId();<br>
> <br>
> + std::string getCaptureScript();<br>
> +<br>
> /* Hotplug / Unplug Support. */<br>
> void cameraAdded(libcamera::Camera *camera);<br>
> <br>
> @@ -41,11 +45,26 @@ public:<br>
> void updateCamInfo(const std::shared_ptr<libcamera::Camera> &camera);<br>
> void handleCameraChange();<br>
> <br>
> + /* Capture script support. */<br>
> + void handleCaptureScriptButton();<br>
> + void informScriptReset();<br>
> + void informScriptRunning(std::string scriptPath);<br>
> + void accept() override;<br>
> + void reject() override;<br>
> +<br>
> +Q_SIGNALS:<br>
> + void stopCaptureScript();<br>
> +<br>
> private:<br>
> libcamera::CameraManager *cm_;<br>
> <br>
> + bool isScriptRunning_;<br>
> + std::string scriptPath_;<br>
> + std::string selectedScriptPath_;<br>
> +<br>
> /* UI elements. */<br>
> QComboBox *cameraIdComboBox_;<br>
> QLabel *cameraLocation_;<br>
> QLabel *cameraModel_;<br>
> + QPushButton *captureScriptButton_;<br>
> };<br>
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp<br>
> index 3feabcff..d73fb42a 100644<br>
> --- a/src/qcam/main_window.cpp<br>
> +++ b/src/qcam/main_window.cpp<br>
> @@ -9,6 +9,7 @@<br>
> <br>
> #include <assert.h><br>
> #include <iomanip><br>
> +#include <memory><br>
> #include <string><br>
> <br>
> #include <libcamera/camera_manager.h><br>
> @@ -19,6 +20,7 @@<br>
> #include <QFileDialog><br>
> #include <QImage><br>
> #include <QImageWriter><br>
> +#include <QMessageBox><br>
> #include <QMutexLocker><br>
> #include <QStandardPaths><br>
> #include <QStringList><br>
> @@ -151,6 +153,9 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)<br>
> return;<br>
> }<br>
> <br>
> + /* Start capture script. */<br>
> + loadCaptureScript();<br>
> +<br>
> startStopAction_->setChecked(true);<br>
> }<br>
> <br>
> @@ -289,10 +294,53 @@ void MainWindow::switchCamera()<br>
> startStopAction_->setChecked(true);<br>
> }<br>
> <br>
> +void MainWindow::stopCaptureScript()<br>
> +{<br>
> + if (script_) {<br>
> + script_.reset();<br>
> + cameraSelectorDialog_->informScriptReset();<br>
> + }<br>
> +}<br>
> +<br>
> +void MainWindow::loadCaptureScript()<br>
> +{<br>
> + if (scriptPath_.empty() || camera_ == nullptr)<br>
> + return;<br>
> +<br>
> + script_ = std::make_unique<CaptureScript>(camera_, scriptPath_);<br>
> +<br>
> + /*<br>
> + * If we are already capturing, stop so we don't have stuck image<br>
> + * in viewfinder.<br>
> + */<br>
> + bool wasCapturing = isCapturing_;<br>
> + if (isCapturing_)<br>
> + toggleCapture(false);<br>
> +<br>
> + if (!script_->valid()) {<br>
> + script_.reset();<br>
> + cameraSelectorDialog_->informScriptReset();<br>
> +<br>
> + QMessageBox::critical(this, "Invalid Script",<br>
> + "Couldn't load the capture script");<br>
> +<br>
> + } else<br>
> + cameraSelectorDialog_->informScriptRunning(scriptPath_);<br>
> +<br>
> + /* Start capture again if we were capturing before. */<br>
> + if (wasCapturing)<br>
> + toggleCapture(true);<br>
> +}<br>
> +<br>
> std::string MainWindow::chooseCamera()<br>
> {<br>
> + bool scriptRunning = script_ != nullptr;<br>
> +<br>
> /* Construct the selection dialog, unconditionally. */<br>
> - cameraSelectorDialog_ = new CameraSelectorDialog(cm_, this);<br>
> + cameraSelectorDialog_ = new CameraSelectorDialog(cm_, scriptRunning, this);<br>
> +<br>
> + connect(cameraSelectorDialog_, &CameraSelectorDialog::stopCaptureScript,<br>
> + this, &MainWindow::stopCaptureScript);<br>
<br>
I fear that creating a new dialog box every time we choose a camera<br>
means that every call here leaks at least a CameraSelectorDialog<br>
allocation, *and* because we're connecting more signals - we're going to<br>
end up with duplicted signals?<br>
<br></blockquote><div>Ah shoots this should be done in the MainWindow constructor. <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> <br>
> /*<br>
> * Use the camera specified on the command line, if any, or display the<br>
> @@ -305,6 +353,9 @@ std::string MainWindow::chooseCamera()<br>
> std::string cameraId = cameraSelectorDialog_->getCameraId();<br>
> cameraSelectButton_->setText(QString::fromStdString(cameraId));<br>
> <br>
> + scriptPath_ = cameraSelectorDialog_->getCaptureScript();<br>
> + loadCaptureScript();<br>
> +<br>
> return cameraId;<br>
> } else<br>
> return std::string();<br>
> @@ -502,6 +553,7 @@ int MainWindow::startCapture()<br>
> previousFrames_ = 0;<br>
> framesCaptured_ = 0;<br>
> lastBufferTime_ = 0;<br>
> + queueCount_ = 0;<br>
> <br>
> ret = camera_->start();<br>
> if (ret) {<br>
> @@ -779,5 +831,10 @@ void MainWindow::renderComplete(FrameBuffer *buffer)<br>
> <br>
> int MainWindow::queueRequest(Request *request)<br>
> {<br>
> + if (script_)<br>
> + request->controls() = script_->frameControls(queueCount_);<br>
> +<br>
> + queueCount_++;<br>
> +<br>
> return camera_->queueRequest(request);<br>
> }<br>
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h<br>
> index bd6f0172..887f1db1 100644<br>
> --- a/src/qcam/main_window.h<br>
> +++ b/src/qcam/main_window.h<br>
> @@ -28,6 +28,7 @@<br>
> #include <QQueue><br>
> #include <QTimer><br>
> <br>
> +#include "../cam/capture_script.h"<br>
> #include "../cam/stream_options.h"<br>
> <br>
> #include "cam_select_dialog.h"<br>
> @@ -90,6 +91,9 @@ private:<br>
> void processHotplug(HotplugEvent *e);<br>
> void processViewfinder(libcamera::FrameBuffer *buffer);<br>
> <br>
> + void loadCaptureScript();<br>
> + void stopCaptureScript();<br>
> +<br>
> /* UI elements */<br>
> QToolBar *toolbar_;<br>
> QAction *startStopAction_;<br>
> @@ -130,6 +134,9 @@ private:<br>
> QElapsedTimer frameRateInterval_;<br>
> uint32_t previousFrames_;<br>
> uint32_t framesCaptured_;<br>
> + uint32_t queueCount_;<br>
> <br>
> std::vector<std::unique_ptr<libcamera::Request>> requests_;<br>
> + std::unique_ptr<CaptureScript> script_;<br>
> + std::string scriptPath_;<br>
> };<br>
> diff --git a/src/qcam/meson.build b/src/qcam/meson.build<br>
> index 61861ea6..70a18d7e 100644<br>
> --- a/src/qcam/meson.build<br>
> +++ b/src/qcam/meson.build<br>
> @@ -15,6 +15,7 @@ endif<br>
> qcam_enabled = true<br>
> <br>
> qcam_sources = files([<br>
> + '../cam/capture_script.cpp',<br>
> '../cam/image.cpp',<br>
> '../cam/options.cpp',<br>
> '../cam/stream_options.cpp',<br>
> @@ -39,6 +40,7 @@ qcam_resources = files([<br>
> qcam_deps = [<br>
> libatomic,<br>
> libcamera_public,<br>
> + libyaml,<br>
> qt5_dep,<br>
> ]<br>
> <br>
> -- <br>
> 2.25.1<br>
><br>
</blockquote></div></div>