[{"id":31827,"web_url":"https://patchwork.libcamera.org/comment/31827/","msgid":"<172942221090.31090.2986924029207511237@ping.linuxembedded.co.uk>","date":"2024-10-20T11:03:30","subject":"Re: [PATCH v2] qcam: Automatically select the camera if only one is\n\tavailable","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stanislaw Gruszka (2024-10-19 11:09:25)\n> When only a single camera is available, showing the camera selection\n> dialog is unnecessary. It's better to automatically select the available\n> camera without prompting the user for input.\n> \n> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>\n> ---\n> v2:\n> - Avoid cameras().size() vs cameras()[0] race condition\n> - Update in code comment\n> \n>  src/apps/qcam/main_window.cpp | 17 +++++++++++++----\n>  1 file changed, 13 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp\n> index 5144c6b3..86ffa205 100644\n> --- a/src/apps/qcam/main_window.cpp\n> +++ b/src/apps/qcam/main_window.cpp\n> @@ -296,14 +296,23 @@ std::string MainWindow::chooseCamera()\n>  int MainWindow::openCamera()\n>  {\n>         std::string cameraName;\n> +       int num = 0;\n>  \n>         /*\n> -        * Use the camera specified on the command line, if any, or display the\n> -        * camera selection dialog box otherwise.\n> +        * Use the camera specified on the command line, if any, or select the\n> +        * only one available, otherwise display the camera selection dialog box.\n>          */\n> -       if (options_.isSet(OptCamera))\n> +       if (options_.isSet(OptCamera)) {\n>                 cameraName = static_cast<std::string>(options_[OptCamera]);\n> -       else\n> +       } else {\n> +               for (const auto &cam : cm_->cameras()) {\n> +                       num++;\n> +                       if (num > 1)\n\nReally trivial nit but I would have probably put 'if (++num > 1)' here\nto remove a line.\n\nBut that's no reason to worry.\n\nThe fact that we make it so easy for Laurent to worry about TOCTOU here\nmeans that's probably just unfriendly in the libcamera API. I don't know\nhow to make it easier at the moment though ... so\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> +                               break;\n> +                       cameraName = cam->id();\n> +               }\n> +       }\n> +       if (num > 1)\n>                 cameraName = chooseCamera();\n>  \n>         if (cameraName == \"\")\n> -- \n> 2.43.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DEB7ABD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 20 Oct 2024 11:03:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A8AC36538C;\n\tSun, 20 Oct 2024 13:03:36 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9704B65379\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 20 Oct 2024 13:03:34 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 70365526;\n\tSun, 20 Oct 2024 13:01:49 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ZAAI3cw9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1729422109;\n\tbh=1++QFcYFJVYsQHSBKwYEnY30hnLiI/nrug3PfYJcyO4=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=ZAAI3cw9PytwVGWHJe8Cqn5sTQ+gY9tAyJIXU8aia6w7FCiKiz/I3sp2N+Utc8Cwh\n\ti6pGAV8nS/bWY2WdQEqs+OuSseBgcV1Lka8dQVAaQVkJ06m1brk9FpeIdyTofo/3gz\n\tq/QQh9jg59G5brZM9GcFKMl7AkXl/j1a60TQmA5Q=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241019100925.42808-1-stanislaw.gruszka@linux.intel.com>","References":"<20241019100925.42808-1-stanislaw.gruszka@linux.intel.com>","Subject":"Re: [PATCH v2] qcam: Automatically select the camera if only one is\n\tavailable","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Sun, 20 Oct 2024 12:03:30 +0100","Message-ID":"<172942221090.31090.2986924029207511237@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31831,"web_url":"https://patchwork.libcamera.org/comment/31831/","msgid":"<20241020153539.GA7770@pendragon.ideasonboard.com>","date":"2024-10-20T15:35:39","subject":"Re: [PATCH v2] qcam: Automatically select the camera if only one is\n\tavailable","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Sun, Oct 20, 2024 at 12:03:30PM +0100, Kieran Bingham wrote:\n> Quoting Stanislaw Gruszka (2024-10-19 11:09:25)\n> > When only a single camera is available, showing the camera selection\n> > dialog is unnecessary. It's better to automatically select the available\n> > camera without prompting the user for input.\n> > \n> > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>\n> > ---\n> > v2:\n> > - Avoid cameras().size() vs cameras()[0] race condition\n> > - Update in code comment\n> > \n> >  src/apps/qcam/main_window.cpp | 17 +++++++++++++----\n> >  1 file changed, 13 insertions(+), 4 deletions(-)\n> > \n> > diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp\n> > index 5144c6b3..86ffa205 100644\n> > --- a/src/apps/qcam/main_window.cpp\n> > +++ b/src/apps/qcam/main_window.cpp\n> > @@ -296,14 +296,23 @@ std::string MainWindow::chooseCamera()\n> >  int MainWindow::openCamera()\n> >  {\n> >         std::string cameraName;\n> > +       int num = 0;\n> >  \n> >         /*\n> > -        * Use the camera specified on the command line, if any, or display the\n> > -        * camera selection dialog box otherwise.\n> > +        * Use the camera specified on the command line, if any, or select the\n> > +        * only one available, otherwise display the camera selection dialog box.\n> >          */\n> > -       if (options_.isSet(OptCamera))\n> > +       if (options_.isSet(OptCamera)) {\n> >                 cameraName = static_cast<std::string>(options_[OptCamera]);\n> > -       else\n> > +       } else {\n> > +               for (const auto &cam : cm_->cameras()) {\n> > +                       num++;\n> > +                       if (num > 1)\n> \n> Really trivial nit but I would have probably put 'if (++num > 1)' here\n> to remove a line.\n\nA bit of a larger patch, but would the following be cleaner ?\n\ndiff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp\nindex 86ffa2050251..305bba79b1b2 100644\n--- a/src/apps/qcam/main_window.cpp\n+++ b/src/apps/qcam/main_window.cpp\n@@ -251,16 +251,14 @@ void MainWindow::updateTitle()\n void MainWindow::switchCamera()\n {\n \t/* Get and acquire the new camera. */\n-\tstd::string newCameraId = chooseCamera();\n+\tstd::shared_ptr<Camera> cam = chooseCamera();\n \n-\tif (newCameraId.empty())\n+\tif (!cam)\n \t\treturn;\n \n-\tif (camera_ && newCameraId == camera_->id())\n+\tif (camera_ && cam == camera_)\n \t\treturn;\n \n-\tconst std::shared_ptr<Camera> &cam = cm_->get(newCameraId);\n-\n \tif (cam->acquire()) {\n \t\tqInfo() << \"Failed to acquire camera\" << cam->id().c_str();\n \t\treturn;\n@@ -282,49 +280,42 @@ void MainWindow::switchCamera()\n \tstartStopAction_->setChecked(true);\n \n \t/* Display the current cameraId in the toolbar .*/\n-\tcameraSelectButton_->setText(QString::fromStdString(newCameraId));\n+\tcameraSelectButton_->setText(QString::fromStdString(cam->id()));\n }\n \n-std::string MainWindow::chooseCamera()\n+std::shared_ptr<Camera> MainWindow::chooseCamera()\n {\n \tif (cameraSelectorDialog_->exec() != QDialog::Accepted)\n-\t\treturn std::string();\n+\t\treturn {};\n \n-\treturn cameraSelectorDialog_->getCameraId();\n+\tstd::string id = cameraSelectorDialog_->getCameraId();\n+\treturn cm_->get(id);\n }\n \n int MainWindow::openCamera()\n {\n-\tstd::string cameraName;\n-\tint num = 0;\n-\n \t/*\n-\t * Use the camera specified on the command line, if any, or select the\n-\t * only one available, otherwise display the camera selection dialog box.\n+\t * If a camera is specified on the command line, get it. Otherwise, if\n+\t * only one camera is available, pick it automatically, else, display\n+\t * the selector dialog box.\n \t */\n \tif (options_.isSet(OptCamera)) {\n-\t\tcameraName = static_cast<std::string>(options_[OptCamera]);\n-\t} else {\n-\t\tfor (const auto &cam : cm_->cameras()) {\n-\t\t\tnum++;\n-\t\t\tif (num > 1)\n-\t\t\t\tbreak;\n-\t\t\tcameraName = cam->id();\n+\t\tstd::string cameraName = static_cast<std::string>(options_[OptCamera]);\n+\t\tcamera_ = cm_->get(cameraName);\n+\t\tif (!camera_) {\n+\t\t\tqInfo() << \"Camera\" << cameraName.c_str() << \"not found\";\n+\t\t\treturn -ENODEV;\n \t\t}\n-\t}\n-\tif (num > 1)\n-\t\tcameraName = chooseCamera();\n+\t} else {\n+\t\tstd::vector<std::shared_ptr<Camera>> cameras = cm_->cameras();\n \n-\tif (cameraName == \"\")\n-\t\treturn -EINVAL;\n-\n-\t/* Get and acquire the camera. */\n-\tcamera_ = cm_->get(cameraName);\n-\tif (!camera_) {\n-\t\tqInfo() << \"Camera\" << cameraName.c_str() << \"not found\";\n-\t\treturn -ENODEV;\n+\t\tif (cameras.size() == 1)\n+\t\t\tcamera_ = cameras[0];\n+\t\telse\n+\t\t\tcamera_ = chooseCamera();\n \t}\n \n+\t/* Acquire the camera. */\n \tif (camera_->acquire()) {\n \t\tqInfo() << \"Failed to acquire camera\";\n \t\tcamera_.reset();\n@@ -332,7 +323,7 @@ int MainWindow::openCamera()\n \t}\n \n \t/* Set the camera switch button with the currently selected Camera id. */\n-\tcameraSelectButton_->setText(QString::fromStdString(cameraName));\n+\tcameraSelectButton_->setText(QString::fromStdString(camera_->id()));\n \n \treturn 0;\n }\ndiff --git a/src/apps/qcam/main_window.h b/src/apps/qcam/main_window.h\nindex 4cead7344d27..81fcf915ade5 100644\n--- a/src/apps/qcam/main_window.h\n+++ b/src/apps/qcam/main_window.h\n@@ -73,7 +73,7 @@ private Q_SLOTS:\n private:\n \tint createToolbars();\n \n-\tstd::string chooseCamera();\n+\tstd::shared_ptr<libcamera::Camera> chooseCamera();\n \tint openCamera();\n \n \tint startCapture();\n\nThe change to MainWindow::chooseCamera() is not strictly mandatory, but\nresults (I think) in cleaner code in MainWindow::openCamera(). Feel free\nto propose alternatives, the important part to avoid TOCTOU races and\nkeep MainWindow::openCamera() clean is\n\n\t\tstd::vector<std::shared_ptr<Camera>> cameras = cm_->cameras();\n\n> But that's no reason to worry.\n> \n> The fact that we make it so easy for Laurent to worry about TOCTOU here\n> means that's probably just unfriendly in the libcamera API. I don't know\n> how to make it easier at the moment though ... so\n> \n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> > +                               break;\n> > +                       cameraName = cam->id();\n> > +               }\n> > +       }\n> > +       if (num > 1)\n> >                 cameraName = chooseCamera();\n> >  \n> >         if (cameraName == \"\")","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9A7A1C3306\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 20 Oct 2024 15:35:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B629D6538F;\n\tSun, 20 Oct 2024 17:35:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7ACD465379\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 20 Oct 2024 17:35:45 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F0D0878E;\n\tSun, 20 Oct 2024 17:33:59 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"WCAlUWbH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1729438440;\n\tbh=6W3R+efGU2bEBRLxvPjzExp1aM89jstFbkwDPl0ackI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WCAlUWbH9Q+u7LWDXU+KKGFO3jAK8i4T8rdswoE4bM3Q3ffqa2jOxS3Y4/p0zki3G\n\tKOgRuxt0sGGGZPaOtMea0NaBs664KlFSA126z/KqJd72HclowIUgMQADz+74wBbafx\n\tEOCIj8ozU/k42OUZDoaMap+5q7/byvt/mPpy0MJ8=","Date":"Sun, 20 Oct 2024 18:35:39 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] qcam: Automatically select the camera if only one is\n\tavailable","Message-ID":"<20241020153539.GA7770@pendragon.ideasonboard.com>","References":"<20241019100925.42808-1-stanislaw.gruszka@linux.intel.com>\n\t<172942221090.31090.2986924029207511237@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<172942221090.31090.2986924029207511237@ping.linuxembedded.co.uk>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31859,"web_url":"https://patchwork.libcamera.org/comment/31859/","msgid":"<ZxZyVdcltu+HhEiG@linux.intel.com>","date":"2024-10-21T15:25:09","subject":"Re: [PATCH v2] qcam: Automatically select the camera if only one is\n\tavailable","submitter":{"id":211,"url":"https://patchwork.libcamera.org/api/people/211/","name":"Stanislaw Gruszka","email":"stanislaw.gruszka@linux.intel.com"},"content":"On Sun, Oct 20, 2024 at 06:35:39PM +0300, Laurent Pinchart wrote:\n> On Sun, Oct 20, 2024 at 12:03:30PM +0100, Kieran Bingham wrote:\n> > Quoting Stanislaw Gruszka (2024-10-19 11:09:25)\n> > > When only a single camera is available, showing the camera selection\n> > > dialog is unnecessary. It's better to automatically select the available\n> > > camera without prompting the user for input.\n> > > \n> > > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>\n> > > ---\n> > > v2:\n> > > - Avoid cameras().size() vs cameras()[0] race condition\n> > > - Update in code comment\n> > > \n> > >  src/apps/qcam/main_window.cpp | 17 +++++++++++++----\n> > >  1 file changed, 13 insertions(+), 4 deletions(-)\n> > > \n> > > diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp\n> > > index 5144c6b3..86ffa205 100644\n> > > --- a/src/apps/qcam/main_window.cpp\n> > > +++ b/src/apps/qcam/main_window.cpp\n> > > @@ -296,14 +296,23 @@ std::string MainWindow::chooseCamera()\n> > >  int MainWindow::openCamera()\n> > >  {\n> > >         std::string cameraName;\n> > > +       int num = 0;\n> > >  \n> > >         /*\n> > > -        * Use the camera specified on the command line, if any, or display the\n> > > -        * camera selection dialog box otherwise.\n> > > +        * Use the camera specified on the command line, if any, or select the\n> > > +        * only one available, otherwise display the camera selection dialog box.\n> > >          */\n> > > -       if (options_.isSet(OptCamera))\n> > > +       if (options_.isSet(OptCamera)) {\n> > >                 cameraName = static_cast<std::string>(options_[OptCamera]);\n> > > -       else\n> > > +       } else {\n> > > +               for (const auto &cam : cm_->cameras()) {\n> > > +                       num++;\n> > > +                       if (num > 1)\n> > \n> > Really trivial nit but I would have probably put 'if (++num > 1)' here\n> > to remove a line.\n> \n> A bit of a larger patch, but would the following be cleaner ?\n\nI think it's subjective it's it's cleaner or not. Looks fine for me.\n\n> -\tif (cameraName == \"\")\n> -\t\treturn -EINVAL;\n> -\n> -\t/* Get and acquire the camera. */\n> -\tcamera_ = cm_->get(cameraName);\n> -\tif (!camera_) {\n> -\t\tqInfo() << \"Camera\" << cameraName.c_str() << \"not found\";\n> -\t\treturn -ENODEV;\n> +\t\tif (cameras.size() == 1)\n> +\t\t\tcamera_ = cameras[0];\n> +\t\telse\n> +\t\t\tcamera_ = chooseCamera();\n>  \t}\n>  \n> +\t/* Acquire the camera. */\n>  \tif (camera_->acquire()) {\n\nBut it crashes here if there are no cameras and user press OK\nbutton in dialog window, due to lack of !camera_ check.\nI can fix that and post as v3...\n\nRegards\nStanislaw\n\n\n>  \t\tqInfo() << \"Failed to acquire camera\";\n>  \t\tcamera_.reset();\n> @@ -332,7 +323,7 @@ int MainWindow::openCamera()\n>  \t}\n>  \n>  \t/* Set the camera switch button with the currently selected Camera id. */\n> -\tcameraSelectButton_->setText(QString::fromStdString(cameraName));\n> +\tcameraSelectButton_->setText(QString::fromStdString(camera_->id()));\n>  \n>  \treturn 0;\n>  }\n> diff --git a/src/apps/qcam/main_window.h b/src/apps/qcam/main_window.h\n> index 4cead7344d27..81fcf915ade5 100644\n> --- a/src/apps/qcam/main_window.h\n> +++ b/src/apps/qcam/main_window.h\n> @@ -73,7 +73,7 @@ private Q_SLOTS:\n>  private:\n>  \tint createToolbars();\n>  \n> -\tstd::string chooseCamera();\n> +\tstd::shared_ptr<libcamera::Camera> chooseCamera();\n>  \tint openCamera();\n>  \n>  \tint startCapture();\n> \n> The change to MainWindow::chooseCamera() is not strictly mandatory, but\n> results (I think) in cleaner code in MainWindow::openCamera(). Feel free\n> to propose alternatives, the important part to avoid TOCTOU races and\n> keep MainWindow::openCamera() clean is\n> \n> \t\tstd::vector<std::shared_ptr<Camera>> cameras = cm_->cameras();\n> \n> > But that's no reason to worry.\n> > \n> > The fact that we make it so easy for Laurent to worry about TOCTOU here\n> > means that's probably just unfriendly in the libcamera API. I don't know\n> > how to make it easier at the moment though ... so\n> > \n> > \n> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > \n> > > +                               break;\n> > > +                       cameraName = cam->id();\n> > > +               }\n> > > +       }\n> > > +       if (num > 1)\n> > >                 cameraName = chooseCamera();\n> > >  \n> > >         if (cameraName == \"\")\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 463B1BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 21 Oct 2024 15:25:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B88FB65393;\n\tMon, 21 Oct 2024 17:25:17 +0200 (CEST)","from mgamail.intel.com (mgamail.intel.com [198.175.65.9])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 71B7E6538B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 21 Oct 2024 17:25:15 +0200 (CEST)","from fmviesa006.fm.intel.com ([10.60.135.146])\n\tby orvoesa101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;\n\t21 Oct 2024 08:25:13 -0700","from sgruszka-mobl.ger.corp.intel.com (HELO localhost)\n\t([10.246.22.158]) by fmviesa006-auth.fm.intel.com with\n\tESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Oct 2024 08:25:11 -0700"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=intel.com header.i=@intel.com\n\theader.b=\"VjhD9ewc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=intel.com; i=@intel.com; q=dns/txt; s=Intel;\n\tt=1729524315; x=1761060315;\n\th=date:from:to:cc:subject:message-id:references:\n\tmime-version:in-reply-to;\n\tbh=XtHIfGxzOIChgyfcciet5pauo5zgEooyFms4RPSez1U=;\n\tb=VjhD9ewcgQDdJEkga5VcgCJIsxSUV/xJmMJIGWbliOwbBRyiCwom58Hn\n\tzBzFGqMWNYlVwaxZPfxBOg+zkV4ytxIzJloh6+iSeDFdkxFK0xq1yPXQ/\n\tfhVv5JD0WAir0xn32BXk73NfYN4rJGdwtc2f9zenhepjTOxjMYCaTE/NN\n\t+/+z11r6+5ztToKn/MWyROzT+dS76qWFW1WAMOKorBF8VNX801mrpR7vO\n\tOJt/b/8i+kKSbmaPk2ZynTdTvcPHES+qw2b3z+UevYRGUTfuP19J6E0hQ\n\tjSfOMNmSziOFu+5uPAYU/lVgdIsfH0itdbChiT3t5fvVsm8EJobOifh5q g==;","X-CSE-ConnectionGUID":["F/xU3mPoQ5q2qt4e8vHTNA==","yqfrqzSxTOeCYSd/Lff4kQ=="],"X-CSE-MsgGUID":["Zcz2DPglSjaqZC4EVE1s+w==","CTWAo2LMRhmxqXvq/OovkQ=="],"X-IronPort-AV":["E=McAfee;i=\"6700,10204,11222\"; a=\"51561152\"","E=Sophos;i=\"6.11,199,1725346800\"; d=\"scan'208\";a=\"51561152\"","E=Sophos;i=\"6.11,221,1725346800\"; d=\"scan'208\";a=\"79147406\""],"X-ExtLoop1":"1","Date":"Mon, 21 Oct 2024 17:25:09 +0200","From":"Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] qcam: Automatically select the camera if only one is\n\tavailable","Message-ID":"<ZxZyVdcltu+HhEiG@linux.intel.com>","References":"<20241019100925.42808-1-stanislaw.gruszka@linux.intel.com>\n\t<172942221090.31090.2986924029207511237@ping.linuxembedded.co.uk>\n\t<20241020153539.GA7770@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20241020153539.GA7770@pendragon.ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31877,"web_url":"https://patchwork.libcamera.org/comment/31877/","msgid":"<Zxd4mXtgOSVKEi0C@linux.intel.com>","date":"2024-10-22T10:04:09","subject":"Re: [PATCH v2] qcam: Automatically select the camera if only one is\n\tavailable","submitter":{"id":211,"url":"https://patchwork.libcamera.org/api/people/211/","name":"Stanislaw Gruszka","email":"stanislaw.gruszka@linux.intel.com"},"content":"On Mon, Oct 21, 2024 at 05:25:09PM +0200, Stanislaw Gruszka wrote:\n> But it crashes here if there are no cameras and user press OK\n> button in dialog window, due to lack of !camera_ check.\n> I can fix that and post as v3...\n<snip>\n\n> > The change to MainWindow::chooseCamera() is not strictly mandatory, but\n> > results (I think) in cleaner code in MainWindow::openCamera(). Feel free\n> > to propose alternatives, the important part to avoid TOCTOU races and\n> > keep MainWindow::openCamera() clean is\n> > \n> > \t\tstd::vector<std::shared_ptr<Camera>> cameras = cm_->cameras();\n\nAfter second thought I rather opted to fixup my small patch using above vector\ncopy and add your's comment change since is nicer :-)\n\nThe change of MainWindow::chooseCamera() make sense to me, but can be done\nseparately as cleanup.\n\nRegards\nStanislaw","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 057F3C3274\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 22 Oct 2024 10:04:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 38D5265391;\n\tTue, 22 Oct 2024 12:04:16 +0200 (CEST)","from mgamail.intel.com (mgamail.intel.com [198.175.65.10])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9DE026053E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 22 Oct 2024 12:04:14 +0200 (CEST)","from fmviesa006.fm.intel.com ([10.60.135.146])\n\tby orvoesa102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;\n\t22 Oct 2024 03:04:13 -0700","from sgruszka-mobl.ger.corp.intel.com (HELO localhost)\n\t([10.245.97.183]) by fmviesa006-auth.fm.intel.com with\n\tESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Oct 2024 03:04:11 -0700"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=intel.com header.i=@intel.com\n\theader.b=\"dXx/YRVy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=intel.com; i=@intel.com; q=dns/txt; s=Intel;\n\tt=1729591455; x=1761127455;\n\th=date:from:to:cc:subject:message-id:references:\n\tmime-version:in-reply-to;\n\tbh=cfy+rBQrY2CpUCor1M1B0Sj56z3V/9oFU2Mm/iQFUIM=;\n\tb=dXx/YRVyaqszJluoADMe5Mu6Sd1FjN6XDyKoaT4Iz2KdvbN0A/OPqF2e\n\toneCKFCZP1Tdt/Dt5kBlSQD3FRY5aMrJmR2jwDKSy5FFT4uh+Om3NDuOo\n\tnmTd4Aix5LzX5Hv+TkrfXPHQbC4F0bi51fVFk2ez/PlVP+px0vREbAcNa\n\tPU5BFfEFejD25ipDSbCvAa1d4WpPy/GseCLjrCX82w/iAmx6u9b95FPSD\n\tO+/yX+vCFnvsJTVDdq0IJJlbN6QgCMBmiksQQF/nvF0yx1loiE5bFwcd3\n\trKpOX5SuI1RrN0Bu6+gNfT+Jsu/yaXUN5yS1XO1UwhJAiId9bbyPN5VVW g==;","X-CSE-ConnectionGUID":["IheIlNf2R3Kza1ye5ykm8Q==","jpY4xJOHQhGeGtFoN/ryZw=="],"X-CSE-MsgGUID":["vb+2Hco9RkWgIrxfXMt2kQ==","myZP1cuCTHer3EZsgr1ssg=="],"X-IronPort-AV":["E=McAfee;i=\"6700,10204,11222\"; a=\"46581739\"","E=Sophos;i=\"6.11,199,1725346800\"; d=\"scan'208\";a=\"46581739\"","E=Sophos;i=\"6.11,222,1725346800\"; d=\"scan'208\";a=\"79380939\""],"X-ExtLoop1":"1","Date":"Tue, 22 Oct 2024 12:04:09 +0200","From":"Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] qcam: Automatically select the camera if only one is\n\tavailable","Message-ID":"<Zxd4mXtgOSVKEi0C@linux.intel.com>","References":"<20241019100925.42808-1-stanislaw.gruszka@linux.intel.com>\n\t<172942221090.31090.2986924029207511237@ping.linuxembedded.co.uk>\n\t<20241020153539.GA7770@pendragon.ideasonboard.com>\n\t<ZxZyVdcltu+HhEiG@linux.intel.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<ZxZyVdcltu+HhEiG@linux.intel.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31881,"web_url":"https://patchwork.libcamera.org/comment/31881/","msgid":"<20241022221519.GG9497@pendragon.ideasonboard.com>","date":"2024-10-22T22:15:19","subject":"Re: [PATCH v2] qcam: Automatically select the camera if only one is\n\tavailable","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Oct 22, 2024 at 12:04:09PM +0200, Stanislaw Gruszka wrote:\n> On Mon, Oct 21, 2024 at 05:25:09PM +0200, Stanislaw Gruszka wrote:\n> > But it crashes here if there are no cameras and user press OK\n> > button in dialog window, due to lack of !camera_ check.\n> > I can fix that and post as v3...\n> <snip>\n> \n> > > The change to MainWindow::chooseCamera() is not strictly mandatory, but\n> > > results (I think) in cleaner code in MainWindow::openCamera(). Feel free\n> > > to propose alternatives, the important part to avoid TOCTOU races and\n> > > keep MainWindow::openCamera() clean is\n> > > \n> > > \t\tstd::vector<std::shared_ptr<Camera>> cameras = cm_->cameras();\n> \n> After second thought I rather opted to fixup my small patch using above vector\n> copy and add your's comment change since is nicer :-)\n\nMy proposal was very much meant to generate first *and* second thoughts,\nso I'm happy it was useful for something :-)\n\n> The change of MainWindow::chooseCamera() make sense to me, but can be done\n> separately as cleanup.\n\nFine with me.","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 10E5DBD16B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 22 Oct 2024 22:15:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DBA4D65391;\n\tWed, 23 Oct 2024 00:15:28 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5116C633C8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Oct 2024 00:15:26 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 157003EA;\n\tWed, 23 Oct 2024 00:13:39 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"sIsc0RCh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1729635219;\n\tbh=v6yS92tSRgkf7BThLg9ZlKu84LH3gBsmsqZBRCC0+MI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=sIsc0RChRbvqKaG1iwsEfzlaB6+Lsi//9skN6j3xcnq0/JanzbOiauQIEQsB2rQmP\n\tQ9gcMgjy1AtNfnmhbwXN7XCGH+Y+mCnToVnHHYpwuEzxOHiYzHe8WQWR7PoPA+WedH\n\t6j+hJHlxt8onWw307BBAAfVfxZ2Uets3cXX66yFo=","Date":"Wed, 23 Oct 2024 01:15:19 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] qcam: Automatically select the camera if only one is\n\tavailable","Message-ID":"<20241022221519.GG9497@pendragon.ideasonboard.com>","References":"<20241019100925.42808-1-stanislaw.gruszka@linux.intel.com>\n\t<172942221090.31090.2986924029207511237@ping.linuxembedded.co.uk>\n\t<20241020153539.GA7770@pendragon.ideasonboard.com>\n\t<ZxZyVdcltu+HhEiG@linux.intel.com>\n\t<Zxd4mXtgOSVKEi0C@linux.intel.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<Zxd4mXtgOSVKEi0C@linux.intel.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]