[libcamera-devel] [PATCH 06/21] qcam: main_window: Replace start and stop actions with a toggle action

[Kieran Bingham]

Hi Laurent,

On 23/03/2020 14:21, Laurent Pinchart wrote:
> The main window toolbar contains a start button and a stop button. This
> allows starting an already started camera (which is currently not
> handled and results in an error) or stopping an already stopped camera.
> Replace the two actions with a single start/stop toggle action,
> preventing UI misuse and reducing confusion.

Oh - I was actually using this as a test already :-) - I have used it to
start a running stream, and stop a stopped stream.

It even found a bug in a pipeline already - but perhaps that just
highlights that we need a pipeline validation tool already, so it's not
particularly a requirement of the GUI interface.

> 

Otherwise a question below - but:

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/qcam/main_window.cpp | 30 ++++++++++++++++++++----------
>  src/qcam/main_window.h   |  9 ++++++---
>  2 files changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index afd8b9c413f5..86f92360a1a9 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -63,11 +63,10 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
>  	adjustSize();
>  
>  	ret = openCamera();
> -	if (!ret)
> -		ret = startCapture();
> -
>  	if (ret < 0)
>  		quit();
> +
> +	startStopAction_->setChecked(true);
>  }
>  
>  MainWindow::~MainWindow()
> @@ -113,11 +112,10 @@ int MainWindow::createToolbars()
>  
>  	toolbar_->addSeparator();
>  
> -	action = toolbar_->addAction(QIcon(":play-circle.svg"), "start");
> -	connect(action, &QAction::triggered, this, &MainWindow::startCapture);
> -
> -	action = toolbar_->addAction(QIcon(":stop-circle.svg"), "stop");
> -	connect(action, &QAction::triggered, this, &MainWindow::stopCapture);
> +	action = toolbar_->addAction(QIcon(":play-circle.svg"), "Start Capture");
> +	action->setCheckable(true);
> +	connect(action, &QAction::toggled, this, &MainWindow::toggleCapture);
> +	startStopAction_ = action;
>  
>  	action = toolbar_->addAction(QIcon(":save.svg"), "saveAs");
>  	connect(action, &QAction::triggered, this, &MainWindow::saveImageAs);
> @@ -159,12 +157,12 @@ void MainWindow::switchCamera(int index)
>  
>  	std::cout << "Switching to camera " << cam->name() << std::endl;
>  
> -	stopCapture();
> +	startStopAction_->setChecked(false);
>  
>  	camera_->release();
>  	camera_ = cam;
>  
> -	startCapture();
> +	startStopAction_->setChecked(true);
>  }
>  
>  std::string MainWindow::chooseCamera()
> @@ -217,6 +215,17 @@ int MainWindow::openCamera()
>  	return 0;
>  }
>  
> +void MainWindow::toggleCapture(bool start)
> +{
> +	if (start) {
> +		startCapture();
> +		startStopAction_->setText("Stop Capture");
> +	} else {
> +		stopCapture();
> +		startStopAction_->setText("Start Capture");

Shouldn't this update the icon to reflect the change in state somehow?

> +	}
> +}
> +
>  int MainWindow::startCapture()
>  {
>  	int ret;
> @@ -322,6 +331,7 @@ int MainWindow::startCapture()
>  	}
>  
>  	isCapturing_ = true;
> +
>  	return 0;
>  
>  error_disconnect:
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index c623120d5894..3d8d02b44696 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -26,6 +26,7 @@
>  
>  using namespace libcamera;
>  
> +class QAction;
>  class ViewFinder;
>  
>  enum {
> @@ -49,9 +50,7 @@ private Q_SLOTS:
>  	void updateTitle();
>  
>  	void switchCamera(int index);
> -
> -	int startCapture();
> -	void stopCapture();
> +	void toggleCapture(bool start);
>  
>  	void saveImageAs();
>  
> @@ -60,6 +59,9 @@ private:
>  	std::string chooseCamera();
>  	int openCamera();
>  
> +	int startCapture();
> +	void stopCapture();
> +
>  	void requestComplete(Request *request);
>  	void processCapture();
>  	int display(FrameBuffer *buffer);
> @@ -87,6 +89,7 @@ private:
>  	QQueue<FrameBuffer *> doneQueue_;
>  
>  	QToolBar *toolbar_;
> +	QAction *startStopAction_;
>  	ViewFinder *viewfinder_;
>  	std::map<int, std::pair<void *, unsigned int>> mappedBuffers_;
>  };
> 

-- 
Regards
--
Kieran