[PATCH] qcam: Use pointer when choosing camera

[Laurent Pinchart]

Hi Stanislaw,

Thank you for the patch.

On Wed, Oct 30, 2024 at 08:17:39AM +0100, Stanislaw Gruszka wrote:
> In order to remove redundant camera ID lookups and comparisons switch
> to pointer-based checks when choosing and switching cameras.
> 
> Co-developed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka at linux.intel.com>
> ---
>  src/apps/qcam/main_window.cpp | 30 +++++++++++++-----------------
>  src/apps/qcam/main_window.h   |  2 +-
>  2 files changed, 14 insertions(+), 18 deletions(-)
> 
> diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp
> index de487672..e0bbcc75 100644
> --- a/src/apps/qcam/main_window.cpp
> +++ b/src/apps/qcam/main_window.cpp
> @@ -251,16 +251,14 @@ void MainWindow::updateTitle()
>  void MainWindow::switchCamera()
>  {
>  	/* Get and acquire the new camera. */
> -	std::string newCameraId = chooseCamera();
> +	std::shared_ptr<Camera> cam = chooseCamera();
>  
> -	if (newCameraId.empty())
> +	if (!cam)
>  		return;
>  
> -	if (camera_ && newCameraId == camera_->id())
> +	if (camera_ && cam == camera_)
>  		return;
>  
> -	const std::shared_ptr<Camera> &cam = cm_->get(newCameraId);
> -
>  	if (cam->acquire()) {
>  		qInfo() << "Failed to acquire camera" << cam->id().c_str();
>  		return;
> @@ -282,20 +280,21 @@ void MainWindow::switchCamera()
>  	startStopAction_->setChecked(true);
>  
>  	/* Display the current cameraId in the toolbar .*/
> -	cameraSelectButton_->setText(QString::fromStdString(newCameraId));
> +	cameraSelectButton_->setText(QString::fromStdString(cam->id()));
>  }
>  
> -std::string MainWindow::chooseCamera()
> +std::shared_ptr<Camera> MainWindow::chooseCamera()
>  {
>  	if (cameraSelectorDialog_->exec() != QDialog::Accepted)
> -		return std::string();
> +		return {};
>  
> -	return cameraSelectorDialog_->getCameraId();
> +	std::string id = cameraSelectorDialog_->getCameraId();
> +	return cm_->get(id);
>  }
>  
>  int MainWindow::openCamera()
>  {
> -	std::string cameraName;
> +	std::string cameraName = "";
>  
>  	/*
>  	 * If a camera is specified on the command line, get it. Otherwise, if
> @@ -304,24 +303,21 @@ int MainWindow::openCamera()
>  	 */
>  	if (options_.isSet(OptCamera)) {
>  		cameraName = static_cast<std::string>(options_[OptCamera]);
> +		camera_ = cm_->get(cameraName);
>  	} else {
>  		std::vector<std::shared_ptr<Camera>> cameras = cm_->cameras();
>  		if (cameras.size() == 1)
> -			cameraName = cameras[0]->id();
> +			camera_ = cameras[0];
>  		else
> -			cameraName = chooseCamera();
> +			camera_ = chooseCamera();
>  	}
>  
> -	if (cameraName == "")
> -		return -EINVAL;
> -
> -	/* Get and acquire the camera. */
> -	camera_ = cm_->get(cameraName);
>  	if (!camera_) {
>  		qInfo() << "Camera" << cameraName.c_str() << "not found";

This will print "Camera  not found" if the user presses the cancel
button in the selector dialog. Should we restrict printing the message
to the case where the camera name is specified on the command line ?
Something like

	if (options_.isSet(OptCamera)) {
		std::string name = static_cast<std::string>(options_[OptCamera]);
		camera_ = cm_->get(name);
		if (!camera_)
			qInfo() << "Camera" << name.c_str() << "not found";
	} else {
		std::vector<std::shared_ptr<Camera>> cameras = cm_->cameras();
		if (cameras.size() == 1)
			camera_ = cameras[0];
		else
			camera_ = chooseCamera();
	}

	if (!camera_)
		return -ENODEV;

>  		return -ENODEV;
>  	}
>  
> +	/* Acquire the camera. */
>  	if (camera_->acquire()) {
>  		qInfo() << "Failed to acquire camera";
>  		camera_.reset();
> diff --git a/src/apps/qcam/main_window.h b/src/apps/qcam/main_window.h
> index 4cead734..81fcf915 100644
> --- a/src/apps/qcam/main_window.h
> +++ b/src/apps/qcam/main_window.h
> @@ -73,7 +73,7 @@ private Q_SLOTS:
>  private:
>  	int createToolbars();
>  
> -	std::string chooseCamera();
> +	std::shared_ptr<libcamera::Camera> chooseCamera();
>  	int openCamera();
>  
>  	int startCapture();

-- 
Regards,

Laurent Pinchart