[libcamera-devel] [PATCH v4 3/3] qcam: Add --script to load capture script

[Kieran Bingham]

Quoting Umang Jain via libcamera-devel (2022-07-25 19:54:32)
> Hi Utkarsh,
> 
> On 7/3/22 10:07, Utkarsh Tiwari via libcamera-devel wrote:
> > Add --script as an individual option to load capture scripts.
> > Load the capture script before starting the capture.
> >
> > If an invalid capture script has been given, show error
> > but start the capture without the script.
> 
> 
> umm, why would you do that? If --script is specified, it is pretty clear 
> that the user intends to use the capture script. If there are errors in 
> the scripts, we sould fail loudly and not proceed with the start() at all.
> 

That's quite a good point. Failing through the GUI makes sense when
started in the GUI - but indeed, if the script was specified on the
command line, and fails - that's probably expected to fail on the
command line ...

Perhaps it would make sense to keep the qmessagebox::critical to pop up
a gui failure, but also present a console warning, and then exit after
the message box is closed.

> The user 'might' get a false impression that the script is valid hence, 
> the capture is started and "miss" to  take a note of the invalidity 
> error. Was this discussed in previous iterations?

I don't think so.

> 
> >
> > Signed-off-by: Utkarsh Tiwari <utkarsh02t at gmail.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> >   src/qcam/main.cpp        |  3 +++
> >   src/qcam/main_window.cpp | 14 ++++++++++++++
> >   src/qcam/main_window.h   |  1 +
> >   3 files changed, 18 insertions(+)
> >
> > diff --git a/src/qcam/main.cpp b/src/qcam/main.cpp
> > index d3f01a85..91166be5 100644
> > --- a/src/qcam/main.cpp
> > +++ b/src/qcam/main.cpp
> > @@ -43,6 +43,9 @@ OptionsParser::Options parseOptions(int argc, char *argv[])
> >                        "Set configuration of a camera stream", "stream", true);
> >       parser.addOption(OptVerbose, OptionNone,
> >                        "Print verbose log messages", "verbose");
> > +     parser.addOption(OptCaptureScript, OptionString,
> > +                      "Load a capture session configuration script from a file",
> > +                      "script", ArgumentRequired, "script");
> >   
> >       OptionsParser::Options options = parser.parse(argc, argv);
> >       if (options.isSet(OptHelp))
> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> > index 4fbaeccc..7ec53a7c 100644
> > --- a/src/qcam/main_window.cpp
> > +++ b/src/qcam/main_window.cpp
> > @@ -151,6 +151,20 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
> >               return;
> >       }
> >   
> > +     if (options_.isSet(OptCaptureScript)) {
> > +             std::string scriptName = options_[OptCaptureScript].toString();
> > +             script_ = std::make_unique<CaptureScript>(camera_, scriptName);
> > +             if (!script_->valid()) {
> > +                     QMessageBox::critical(this, "Invalid Script",
> > +                                           "Couldn't load the capture script");
> > +                     script_.reset();
> > +             }
> > +
> > +             /* Show stopping availability. */
> > +             scriptExecAction_->setIcon(QIcon(":x-square.svg"));
> > +             scriptExecAction_->setText("Stop Script execution");
> > +     }
> > +
> >       startStopAction_->setChecked(true);
> >   }
> >   
> > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> > index 2cdf7169..4d19ab64 100644
> > --- a/src/qcam/main_window.h
> > +++ b/src/qcam/main_window.h
> > @@ -42,6 +42,7 @@ enum {
> >       OptRenderer = 'r',
> >       OptStream = 's',
> >       OptVerbose = 'v',
> > +     OptCaptureScript = 256,
> >   };
> >   
> >   class MainWindow : public QMainWindow