[v2] qcam: Use pointer when choosing camera

21789 diff mbox series

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@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
v2:
 - better error message when no cameras are present
 - use camera_->id() is switch button, missed in previous patch

 src/apps/qcam/main_window.cpp | 43 +++++++++++++++--------------------
 src/apps/qcam/main_window.h   |  2 +-
 2 files changed, 19 insertions(+), 26 deletions(-)

Quoting Stanislaw Gruszka (2024-10-31 11:02:56)
> 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@ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

Just ran this on my x13s,

Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Looks ok too so:


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

> ---
> v2:
>  - better error message when no cameras are present
>  - use camera_->id() is switch button, missed in previous patch
> 
>  src/apps/qcam/main_window.cpp | 43 +++++++++++++++--------------------
>  src/apps/qcam/main_window.h   |  2 +-
>  2 files changed, 19 insertions(+), 26 deletions(-)
> 
> diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp
> index de487672..3880a846 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,46 +280,41 @@ 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;
> -
>         /*
>          * If a camera is specified on the command line, get it. Otherwise, if
>          * only one camera is available, pick it automatically, else, display
>          * the selector dialog box.
>          */
>         if (options_.isSet(OptCamera)) {
> -               cameraName = static_cast<std::string>(options_[OptCamera]);
> +               std::string cameraName = static_cast<std::string>(options_[OptCamera]);
> +               camera_ = cm_->get(cameraName);
> +               if (!camera_)
> +                       qInfo() << "Camera" << cameraName.c_str() << "not found";
>         } else {
>                 std::vector<std::shared_ptr<Camera>> cameras = cm_->cameras();
> -               if (cameras.size() == 1)
> -                       cameraName = cameras[0]->id();
> -               else
> -                       cameraName = chooseCamera();
> +               camera_ = (cameras.size() == 1) ? cameras[0] : chooseCamera();
> +               if (!camera_)
> +                       qInfo() << "No camera detected";
>         }
>  
> -       if (cameraName == "")
> -               return -EINVAL;
> -
> -       /* Get and acquire the camera. */
> -       camera_ = cm_->get(cameraName);
> -       if (!camera_) {
> -               qInfo() << "Camera" << cameraName.c_str() << "not found";
> +       if (!camera_)
>                 return -ENODEV;
> -       }
>  
> +       /* Acquire the camera. */
>         if (camera_->acquire()) {
>                 qInfo() << "Failed to acquire camera";
>                 camera_.reset();
> @@ -329,7 +322,7 @@ int MainWindow::openCamera()
>         }
>  
>         /* Set the camera switch button with the currently selected Camera id. */
> -       cameraSelectButton_->setText(QString::fromStdString(cameraName));
> +       cameraSelectButton_->setText(QString::fromStdString(camera_->id()));
>  
>         return 0;
>  }
> 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();
> -- 
> 2.43.0
>