[libcamera-devel] [RFC/PATCH 3/5] qcam: Add an option to allow specifying output path

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Thu, Apr 30, 2020 at 02:36:02AM +0200, Niklas Söderlund wrote:
> Add a command line option to specify an output path for files captured.
> This will become more useful when JPEG capture is turned into an quick
> operation instead of using a QFileDialog and when RAW capture support is
> added.

I'll comment on that when reviewing the corresponding patch.

> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/qcam/main.cpp        | 3 +++
>  src/qcam/main_window.cpp | 8 ++++++--
>  src/qcam/main_window.h   | 2 ++
>  3 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qcam/main.cpp b/src/qcam/main.cpp
> index cd73fa764614e7e7..2999e4959d8ecff8 100644
> --- a/src/qcam/main.cpp
> +++ b/src/qcam/main.cpp
> @@ -34,6 +34,9 @@ OptionsParser::Options parseOptions(int argc, char *argv[])
>  			 "help");
>  	parser.addOption(OptStream, &streamKeyValue,
>  			 "Set configuration of a camera stream", "stream", true);
> +	parser.addOption(OptSavePath, OptionString,
> +			 "Path to qcam picture storage",
> +			 "output-dir", ArgumentRequired, "path");

Would it make sense to mention the default here ? Maybe doing

	defaultPath_ = QStandardPaths::writableLocation(QStandardPaths::PicturesLocation);

before parsing options, and having

	parser.addOption(OptSavePath, OptionString,
			 "Path to qcam picture storage (default: " + defaultPath_ + ")",
			 "output-dir", ArgumentRequired, "path");

(likely with QString/std::string conversion I'm afraid :-S)

>  
>  	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 e77bc01df8f3edfe..470f78162cabffe5 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -75,6 +75,11 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
>  		return;
>  	}
>  
> +	if (options_.isSet(OptSavePath))
> +		defaultPath_ = QString::fromUtf8(options_[OptSavePath].toString().c_str());
> +	else
> +		defaultPath_ = QStandardPaths::writableLocation(QStandardPaths::PicturesLocation);

And this would then become

	if (options_.isSet(OptSavePath))
		defaultPath_ = QString::fromUtf8(options_[OptSavePath].toString().c_str());

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +
>  	startStopAction_->setChecked(true);
>  }
>  
> @@ -507,9 +512,8 @@ void MainWindow::stopCapture()
>  void MainWindow::saveImageAs()
>  {
>  	QImage image = viewfinder_->getCurrentImage();
> -	QString defaultPath = QStandardPaths::writableLocation(QStandardPaths::PicturesLocation);
>  
> -	QString filename = QFileDialog::getSaveFileName(this, "Save Image", defaultPath,
> +	QString filename = QFileDialog::getSaveFileName(this, "Save Image", defaultPath_,
>  							"Image Files (*.png *.jpg *.jpeg)");
>  	if (filename.isEmpty())
>  		return;
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index c2040c0ebcd61bfa..7b8a185511e17a8f 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -33,6 +33,7 @@ class QAction;
>  enum {
>  	OptCamera = 'c',
>  	OptHelp = 'h',
> +	OptSavePath = 'o',
>  	OptStream = 's',
>  };
>  
> @@ -83,6 +84,7 @@ private:
>  
>  	/* Options */
>  	const OptionsParser::Options &options_;
> +	QString defaultPath_;
>  
>  	/* Camera manager, camera, configuration and buffers */
>  	CameraManager *cm_;

-- 
Regards,

Laurent Pinchart