[libcamera-devel] [PATCH] qcam: Allow user to select pixel format and resolution

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Tue, May 28, 2019 at 08:16:54PM +0200, Niklas Söderlund wrote:
> Allow users to select pixel format and resolution from two selection
> dialogues. Ideally the dialog should be turned into a toolbar with an
> additional start/stop stream button, but this allows us to demonstrate
> the selection interface for now.

How difficult do you think it would be to implement such a toolbar ?

> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/qcam/main_window.cpp | 64 ++++++++++++++++++++++++++++++++++++++--
>  src/qcam/main_window.h   |  1 +
>  2 files changed, 63 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 16b123132dd96cbe..20a87bbf9a8a2514 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -93,11 +93,71 @@ int MainWindow::openCamera()
>  	return 0;
>  }
>  
> -int MainWindow::startCapture()
> +int MainWindow::selectFormat()
>  {
> -	int ret;
> +	QStringList list;
> +	QString name;
> +	bool result;
>  
>  	config_ = camera_->generateConfiguration({ StreamRole::VideoRecording });
> +
> +	StreamConfiguration &cfg = config_->at(0);
> +
> +	const std::vector<unsigned int> &pixelformats = cfg.formats().pixelformats();

s/pixelformats/pixelFormats/ ?

> +	if (pixelformats.size()) {

	if (!pixelformats.empty()) {

> +		for (unsigned int pixelformat : pixelformats)
> +			list.append(QString::fromStdString(std::to_string(pixelformat)));

How about using the 4CC code instead of the numerical value ? Otherwise
you could use QString::number() instead of going through an std::string.

Shouldn't we also limit the formats to the ones that qcam supports ?

> +
> +		name = QInputDialog::getItem(this, "Select pixel format",
> +					     "Pixel format:", list, 0,
> +					     false, &result);
> +		if (!result)
> +			return -EINVAL;
> +
> +		cfg.pixelFormat = pixelformats.at(list.indexOf(name));
> +	}
> +
> +	const std::vector<Size> &sizes = cfg.formats().sizes(cfg.pixelFormat);
> +	if (sizes.size()) {

You can use .empty() here too.

> +		list.clear();
> +		for (const Size &size : sizes)
> +			list.append(QString::fromStdString(size.toString()));
> +
> +		name = QInputDialog::getItem(this, "Select resolution",
> +					     "Resolution:", list, 0,
> +					     false, &result);
> +		if (!result)
> +			return -EINVAL;
> +
> +		cfg.size = sizes.at(list.indexOf(name));
> +	}

That's a total of 3 dialog boxes when starting the application, it's not
very nice :-( Could we combine them all into a single dialog box, so
that pressing enter once would be enough ?

> +
> +	std::cout << "Trying to use stream configuration" << cfg.toString() << std::endl;
> +	return 0;
> +
> +	switch (config_->validate()) {
> +	case CameraConfiguration::Valid:
> +		break;
> +	case CameraConfiguration::Adjusted:
> +		std::cout << "Camera configuration adjusted" << std::endl;
> +		break;
> +	case CameraConfiguration::Invalid:
> +		std::cout << "Camera configuration invalid" << std::endl;
> +		config_.reset();
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +int MainWindow::startCapture()
> +{
> +	int ret;
> +
> +	ret = selectFormat();
> +	if (ret)
> +		return ret;
> +
>  	ret = camera_->configure(config_.get());
>  	if (ret < 0) {
>  		std::cout << "Failed to configure camera" << std::endl;
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index fe565cbcb4603d9d..43d2d3e3894fb444 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -34,6 +34,7 @@ public:
>  
>  private:
>  	int openCamera();
> +	int selectFormat();
>  
>  	int startCapture();
>  	void stopCapture();

-- 
Regards,

Laurent Pinchart