[PATCH v3] qcam: Automatically select the camera if only one is available

[Laurent Pinchart]

On Tue, Oct 22, 2024 at 12:01:28PM +0100, Kieran Bingham wrote:
> Quoting Stanislaw Gruszka (2024-10-22 11:24:48)
> > When only a single camera is available, showing the camera selection
> > dialog is unnecessary. It's better to automatically select the available
> > camera without prompting the user for input.
> > 
> > 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>
> > ---
> > v2:
> > - Avoid cm_->cameras().size() vs cm_->cameras()[0] race condition
> >   using custom loop.
> > - Update in code comment
> > v3: 
> > - Avoid TOCTOU race using shared_ptr vector copy
> > - Use Laurent comment wording
> > 
> >  src/apps/qcam/main_window.cpp | 16 +++++++++++-----
> >  1 file changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp
> > index 5144c6b3..de487672 100644
> > --- a/src/apps/qcam/main_window.cpp
> > +++ b/src/apps/qcam/main_window.cpp
> > @@ -298,13 +298,19 @@ int MainWindow::openCamera()
> >         std::string cameraName;
> >  
> >         /*
> > -        * Use the camera specified on the command line, if any, or display the
> > -        * camera selection dialog box otherwise.
> > +        * 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))
> > +       if (options_.isSet(OptCamera)) {
> >                 cameraName = static_cast<std::string>(options_[OptCamera]);
> > -       else
> > -               cameraName = chooseCamera();
> > +       } else {
> > +               std::vector<std::shared_ptr<Camera>> cameras = cm_->cameras();
> > +               if (cameras.size() == 1)
> > +                       cameraName = cameras[0]->id();

It's not very efficient to go from camera to name and then back to
camera, but I think that's fine for this patch. We can improve things on
top.

> > +               else
> > +                       cameraName = chooseCamera();
> > +       }
> 
> Ok, now I see how the toctou is easier to fix like this ;-)
> 
> I like this - clean and short and does what it needs.
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> >  
> >         if (cameraName == "")
> >                 return -EINVAL;

-- 
Regards,

Laurent Pinchart