[libcamera-devel] [PATCH v5 2/3] qcam: Add a GUI way to use capture script
Utkarsh Tiwari
utkarsh02t at gmail.com
Thu Jul 28 19:16:19 CEST 2022
Hi Laurent, thanks for the review.
On Wed, Jul 27, 2022 at 01:13:08AM +0300, Laurent Pinchart wrote:
> 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.
As discussed In reply to the 3rd patch I think the capture script ui would
be changed.
>
> > </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 ?
As far my understanding goes, the pipeline handlers at the very start of
the capture do certain special things. So to maintain consistency, we restart
the capture.
>
> > + */
> > +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.
>
Noted.
> > + */
> > + 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.
Oops, noted.
>
> > + 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