[libcamera-devel,v9,3/7] qcam: MainWindow: Replace cameraCombo_ with CameraSelectorDialog
diff mbox series

Message ID 20220831054938.21617-4-utkarsh02t@gmail.com
State Accepted
Headers show
Series
  • Introduce capture scripts to qcam
Related show

Commit Message

Utkarsh Tiwari Aug. 31, 2022, 5:49 a.m. UTC 
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@gmail.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@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(-)

Comments

Kieran Bingham Aug. 31, 2022, 9:02 a.m. UTC | #1 
Quoting Utkarsh Tiwari (2022-08-31 06:49:34)
> 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@gmail.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@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));

Really minor, and doesn't deserve a new version/iteration (could be done
while applying) - but I'd set the text before calling the
startStopAction(true).

Logically - it makes sense to update the context before starting it.
That's all - functionally here, I don't think it will have a significant
difference.

This already has tags and is good to go though I think.


>  }
>  
>  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_;
> +       QPushButton *cameraSelectButton_;
>         QAction *saveRaw_;
>         ViewFinder *viewfinder_;
>  
> -- 
> 2.34.1
>
Laurent Pinchart Aug. 31, 2022, 9:14 a.m. UTC | #2 
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@gmail.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@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_;
>

Patch
diff mbox series 

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_;
+	QPushButton *cameraSelectButton_;
 	QAction *saveRaw_;
 	ViewFinder *viewfinder_;