[libcamera-devel] [PATCH v9 3/7] qcam: MainWindow: Replace cameraCombo_ with CameraSelectorDialog

[Laurent Pinchart]

Hi Utkarsh,

Thank you for the patch.

On Wed, Aug 31, 2022 at 11:19:34AM +0530, Utkarsh Tiwari wrote:
> Replace the cameraCombo_ on the toolbar with a QPushButton which
> displays the CameraSelectorDialog. This would allow the user to view
> information about the camera when switching.
> 
> The QPushButton text is set to the camera Id currently in use.
> 
> Signed-off-by: Utkarsh Tiwari <utkarsh02t at gmail.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> Differences from v9:
>     1. Checking for OptCamera now happens in 1/7 in openCamera dropping
>     the firstCameraSelect_
>     2. Improve curly brace structure
>     3. Drop QComboBox
>  src/qcam/main_window.cpp | 33 ++++++++++++++++-----------------
>  src/qcam/main_window.h   |  5 +++--
>  2 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index e8e22d49..505bdc56 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -14,7 +14,6 @@
>  #include <libcamera/camera_manager.h>
>  #include <libcamera/version.h>
>  
> -#include <QComboBox>
>  #include <QCoreApplication>
>  #include <QFileDialog>
>  #include <QImage>
> @@ -195,14 +194,11 @@ int MainWindow::createToolbars()
>  	connect(action, &QAction::triggered, this, &MainWindow::quit);
>  
>  	/* Camera selector. */
> -	cameraCombo_ = new QComboBox();
> -	connect(cameraCombo_, QOverload<int>::of(&QComboBox::activated),
> +	cameraSelectButton_ = new QPushButton;
> +	connect(cameraSelectButton_, &QPushButton::clicked,
>  		this, &MainWindow::switchCamera);
>  
> -	for (const std::shared_ptr<Camera> &cam : cm_->cameras())
> -		cameraCombo_->addItem(QString::fromStdString(cam->id()));
> -
> -	toolbar_->addWidget(cameraCombo_);
> +	toolbar_->addWidget(cameraSelectButton_);
>  
>  	toolbar_->addSeparator();
>  
> @@ -262,14 +258,18 @@ void MainWindow::updateTitle()
>   * Camera Selection
>   */
>  
> -void MainWindow::switchCamera(int index)
> +void MainWindow::switchCamera()
>  {
>  	/* Get and acquire the new camera. */
> -	const auto &cameras = cm_->cameras();
> -	if (static_cast<unsigned int>(index) >= cameras.size())
> +	std::string newCameraId = chooseCamera();
> +
> +	if (newCameraId.empty())
>  		return;
>  
> -	const std::shared_ptr<Camera> &cam = cameras[index];
> +	if (camera_ && newCameraId == camera_->id())
> +		return;
> +
> +	const std::shared_ptr<Camera> &cam = cm_->get(newCameraId);
>  
>  	if (cam->acquire()) {
>  		qInfo() << "Failed to acquire camera" << cam->id().c_str();
> @@ -288,6 +288,9 @@ void MainWindow::switchCamera(int index)
>  	camera_ = cam;
>  
>  	startStopAction_->setChecked(true);
> +
> +	/* Display the current cameraId in the toolbar .*/
> +	cameraSelectButton_->setText(QString::fromStdString(newCameraId));
>  }
>  
>  std::string MainWindow::chooseCamera()
> @@ -324,8 +327,8 @@ int MainWindow::openCamera()
>  		return -EBUSY;
>  	}
>  
> -	/* Set the combo-box entry with the currently selected Camera. */
> -	cameraCombo_->setCurrentText(QString::fromStdString(cameraName));
> +	/* Set the camera switch button with the currently selected Camera id. */
> +	cameraSelectButton_->setText(QString::fromStdString(cameraName));
>  
>  	return 0;
>  }
> @@ -591,7 +594,6 @@ void MainWindow::processHotplug(HotplugEvent *e)
>  	HotplugEvent::PlugEvent event = e->hotplugEvent();
>  
>  	if (event == HotplugEvent::HotPlug) {
> -		cameraCombo_->addItem(cameraId);
>  		cameraSelectorDialog_->addCamera(cameraId);
>  	} else if (event == HotplugEvent::HotUnplug) {
>  		/* Check if the currently-streaming camera is removed. */
> @@ -599,11 +601,8 @@ void MainWindow::processHotplug(HotplugEvent *e)
>  			toggleCapture(false);
>  			camera_->release();
>  			camera_.reset();
> -			cameraCombo_->setCurrentIndex(0);
>  		}
>  
> -		int camIndex = cameraCombo_->findText(cameraId);
> -		cameraCombo_->removeItem(camIndex);
>  		cameraSelectorDialog_->removeCamera(cameraId);
>  	}
>  }
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index def44605..79723256 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -23,6 +23,7 @@
>  #include <QMainWindow>
>  #include <QMutex>
>  #include <QObject>
> +#include <QPushButton>
>  #include <QQueue>
>  #include <QTimer>
>  
> @@ -60,7 +61,7 @@ private Q_SLOTS:
>  	void quit();
>  	void updateTitle();
>  
> -	void switchCamera(int index);
> +	void switchCamera();
>  	void toggleCapture(bool start);
>  
>  	void saveImageAs();
> @@ -90,7 +91,7 @@ private:
>  	/* UI elements */
>  	QToolBar *toolbar_;
>  	QAction *startStopAction_;
> -	QComboBox *cameraCombo_;

You can drop the forward declaration of QComboBox above in this file.

> +	QPushButton *cameraSelectButton_;
>  	QAction *saveRaw_;
>  	ViewFinder *viewfinder_;
>  

-- 
Regards,

Laurent Pinchart