[libcamera-devel] [PATCH v5 2/3] qcam: Add a GUI way to use capture script
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jul 27 00:13:08 CEST 2022
Hi Utkarsh,
Thank you for the patch.
On Wed, Jul 27, 2022 at 01:11:22AM +0530, Utkarsh Tiwari via libcamera-devel wrote:
> Implement an Open Capture Script button which would allow the user
> to open a Capture Script (*.yaml).
> This button has two states :
> - Open Capture Script
> - Stop the execution of current capture script
>
> When being clicked in open state, present them with a QFileDialog to
> allow user to select a single file.
>
> Introduce a queueCount_ to keep track of the requests queued.
>
> When stopping the execution of the capture script the queueCount_ is not
> reseted and the capture is continues as it is (i.e it is not stopped or
> restarted).
>
> Requests are queued with any controls the script matching the current
> queueCount_.
>
> Signed-off-by: Utkarsh Tiwari <utkarsh02t at gmail.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> Changes from v4
> - Reworded the commit message to clarify button states and
> queueCount_ reseting.
>
> src/qcam/assets/feathericons/feathericons.qrc | 2 +
> src/qcam/main_window.cpp | 68 +++++++++++++++++++
> src/qcam/main_window.h | 6 ++
> src/qcam/meson.build | 2 +
> 4 files changed, 78 insertions(+)
>
> diff --git a/src/qcam/assets/feathericons/feathericons.qrc b/src/qcam/assets/feathericons/feathericons.qrc
> index c5302040..6b08395a 100644
> --- a/src/qcam/assets/feathericons/feathericons.qrc
> +++ b/src/qcam/assets/feathericons/feathericons.qrc
> @@ -3,9 +3,11 @@
> <qresource>
> <file>aperture.svg</file>
> <file>camera-off.svg</file>
> + <file>file.svg</file>
I'm a bit annoyed by the fact that this icon is very generic. I looked
through the feathericons directory and nothing really strikes me as
being a perfect match (it would be too easy). https://feathericons.com/
has a few new icons, but also nothing that really stands out. Here are a
few options you may want to consider:
- activity: Hints at something that evolves over time.
- command: A capture script is somehow a list of commands. Maybe too
Apple-related ?
- list: The other term in a "list of commands".
- settings: The script contains settings, but it may be best to keep
this icon for a future settings dialog.
- sliders: Same here.
- zap: Not sure what the link would be, but there's a zap-off icon that
is handy to replace x-square.svg
> <file>play-circle.svg</file>
> <file>save.svg</file>
> <file>stop-circle.svg</file>
> <file>x-circle.svg</file>
> + <file>x-square.svg</file>
As hinted by the zap proposal above, I think the "on" and "off" states
of the button should have related icons. Another option is to keep the
same icon, and just update the button state, the same way we handle the
"Start Capture" button.
The feather icons are only fallbacks, we also have to select standard
icons from the theme. The latest (but old) version of the icon naming
specification is available at
https://specifications.freedesktop.org/icon-naming-spec/icon-naming-spec-latest.html.
There we have even less options, I've only seen the following ones as
possibly relevant:
system-run
media-playlist-repeat
media-playlist-shuffle
sync-synchronizing
Although there's also text-x-script that may be interesting.
Another option would be to rethink the UI and add capture script support
somewhere else than in the toolbar.
> </qresource>
> </RCC>
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 4e773c31..9dc96fbb 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>
> @@ -233,6 +234,13 @@ int MainWindow::createToolbars()
> saveRaw_ = action;
> #endif
>
> + /* Open Script... action. */
> + action = toolbar_->addAction(QIcon::fromTheme("document-open",
> + QIcon(":file.svg")),
> + "Open Capture Script");
I'd name this "Run Capture Script". The action both opens and runs the
script, but the latter seems to be the most important part.
> + connect(action, &QAction::triggered, this, &MainWindow::chooseScript);
> + scriptExecAction_ = action;
> +
> return 0;
> }
>
> @@ -256,6 +264,60 @@ 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.
Why is that ? Could we apply the capture script from the current point
without restarting the camera ?
> + */
> +void MainWindow::chooseScript()
> +{
> + if (script_) {
> + /*
> + * This is the second valid press of load script button,
> + * It indicates stopping, Stop and set button for new script.
Either ", it" or ". It", and ", stop" or ". Stop".
> + */
> + script_.reset();
> + scriptExecAction_->setIcon(QIcon::fromTheme("document-open",
> + QIcon(":file.svg")));
> + scriptExecAction_->setText("Open Capture Script");
> + return;
> + }
> +
> + QString scriptFile = QFileDialog::getOpenFileName(this, "Open Capture Script", QDir::currentPath(),
> + "Capture Script (*.yaml)");
> + if (scriptFile.isEmpty())
> + return;
> +
> + /*
> + * If we are already capturing,
> + * stop so we don't have stuck image in viewfinder.
* If we are already capturing, stop so we don't have stuck image in
* viewfinder.
> + */
> + bool wasCapturing = isCapturing_;
> + if (isCapturing_)
> + toggleCapture(false);
> +
> + script_ = std::make_unique<CaptureScript>(camera_, scriptFile.toStdString());
> + if (!script_->valid()) {
> + script_.reset();
> + QMessageBox::critical(this, "Invalid Script",
> + "Couldn't load the capture script");
> + if (wasCapturing)
> + toggleCapture(true);
> + return;
> + }
> +
> + /*
> + * Valid script verified
> + * Set the button to indicate stopping availibility.
> + */
Generally speaking, you shouldn't have line wraps at the end of a
sentence in the middle of a paragraph (other than the ones required to
limit the line length to 80 columns):
/*
* Valid script verified. Set the button to indicate stopping
* availibility.
*/
If you want to separate the two sentences in two paragraphs, you need a
blank line:
/*
* Valid script verified.
*
* Set the button to indicate stopping availibility.
*/
I don't think that's what you want here though.
> + scriptExecAction_->setIcon(QIcon(":x-square.svg"));
> + scriptExecAction_->setText("Stop Script execution");
> +
> + /* Start capture again if we were capturing before. */
> + if (wasCapturing)
> + toggleCapture(true);
> +}
> +
> /* -----------------------------------------------------------------------------
> * Camera Selection
> */
> @@ -511,6 +573,7 @@ int MainWindow::startCapture()
> previousFrames_ = 0;
> framesCaptured_ = 0;
> lastBufferTime_ = 0;
> + queueCount_ = 0;
>
> ret = camera_->start();
> if (ret) {
> @@ -790,5 +853,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 bc844711..eb398c1d 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -26,6 +26,7 @@
> #include <QQueue>
> #include <QTimer>
>
> +#include "../cam/capture_script.h"
> #include "../cam/stream_options.h"
>
> #include "viewfinder.h"
> @@ -87,11 +88,14 @@ private:
> void processHotplug(HotplugEvent *e);
> void processViewfinder(libcamera::FrameBuffer *buffer);
>
> + void chooseScript();
> +
> /* UI elements */
> QToolBar *toolbar_;
> QAction *startStopAction_;
> QComboBox *cameraCombo_;
> QAction *saveRaw_;
> + QAction *scriptExecAction_;
> ViewFinder *viewfinder_;
>
> QIcon iconPlay_;
> @@ -125,6 +129,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',
At some point we should probably create an application helpers library
shared between cam and qcam. Out of scope for this series of course.
> @@ -37,6 +38,7 @@ qcam_resources = files([
> qcam_deps = [
> libatomic,
> libcamera_public,
> + libyaml,
> qt5_dep,
> ]
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list