[{"id":18140,"web_url":"https://patchwork.libcamera.org/comment/18140/","msgid":"<211d5935-ddfb-e80b-aa4c-8853e4eac93a@ideasonboard.com>","date":"2021-07-12T16:11:38","subject":"Re: [libcamera-devel] [PATCH 28/30] cam: Make camera-related\n\toptions sub-options of OptCamera","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 07/07/2021 03:19, Laurent Pinchart wrote:\n> Use the new hierarchical options feature of the option parser to turn\n> camera-related option (--capture, --file, --stream, --strict-formats and\n> --metadata) into children of the --camera option. As an added bonus, we\n> don't need to check anymore if a camera has been specified when capture\n> is requested, as that's now enforced by the option parser.\n\n\nAha that's what I was looking for earlier... so here it is ;-)\n\nAlthough - wouldn't this also mean we could handle printing camera\ninformation here too ? (--list-controls or such)\n\n\nAnyway, this is still progress:\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\n> This change prepares for support of multiple cameras.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  src/cam/camera_session.cpp | 22 ++++++++--------\n>  src/cam/camera_session.h   |  6 ++++-\n>  src/cam/main.cpp           | 53 +++++++++++++++++++++-----------------\n>  3 files changed, 45 insertions(+), 36 deletions(-)\n> \n> diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp\n> index ceb2c3ab3a92..f2383567af3b 100644\n> --- a/src/cam/camera_session.cpp\n> +++ b/src/cam/camera_session.cpp\n> @@ -21,11 +21,11 @@\n>  using namespace libcamera;\n>  \n>  CameraSession::CameraSession(CameraManager *cm,\n> +\t\t\t     const std::string &cameraId,\n>  \t\t\t     const OptionsParser::Options &options)\n> -\t: last_(0), queueCount_(0), captureCount_(0),\n> +\t: options_(options), last_(0), queueCount_(0), captureCount_(0),\n>  \t  captureLimit_(0), printMetadata_(false)\n>  {\n> -\tconst std::string &cameraId = options[OptCamera];\n>  \tchar *endptr;\n>  \tunsigned long index = strtoul(cameraId.c_str(), &endptr, 10);\n>  \tif (*endptr == '\\0' && index > 0 && index <= cm->cameras().size())\n> @@ -44,7 +44,7 @@ CameraSession::CameraSession(CameraManager *cm,\n>  \t\treturn;\n>  \t}\n>  \n> -\tStreamRoles roles = StreamKeyValueParser::roles(options[OptStream]);\n> +\tStreamRoles roles = StreamKeyValueParser::roles(options_[OptStream]);\n>  \n>  \tstd::unique_ptr<CameraConfiguration> config =\n>  \t\tcamera_->generateConfiguration(roles);\n> @@ -56,12 +56,12 @@ CameraSession::CameraSession(CameraManager *cm,\n>  \n>  \t/* Apply configuration if explicitly requested. */\n>  \tif (StreamKeyValueParser::updateConfiguration(config.get(),\n> -\t\t\t\t\t\t      options[OptStream])) {\n> +\t\t\t\t\t\t      options_[OptStream])) {\n>  \t\tstd::cerr << \"Failed to update configuration\" << std::endl;\n>  \t\treturn;\n>  \t}\n>  \n> -\tbool strictFormats = options.isSet(OptStrictFormats);\n> +\tbool strictFormats = options_.isSet(OptStrictFormats);\n>  \n>  \tswitch (config->validate()) {\n>  \tcase CameraConfiguration::Valid:\n> @@ -134,14 +134,14 @@ void CameraSession::infoConfiguration() const\n>  \t}\n>  }\n>  \n> -int CameraSession::start(const OptionsParser::Options &options)\n> +int CameraSession::start()\n>  {\n>  \tint ret;\n>  \n>  \tqueueCount_ = 0;\n>  \tcaptureCount_ = 0;\n> -\tcaptureLimit_ = options[OptCapture].toInteger();\n> -\tprintMetadata_ = options.isSet(OptMetadata);\n> +\tcaptureLimit_ = options_[OptCapture].toInteger();\n> +\tprintMetadata_ = options_.isSet(OptMetadata);\n>  \n>  \tret = camera_->configure(config_.get());\n>  \tif (ret < 0) {\n> @@ -157,9 +157,9 @@ int CameraSession::start(const OptionsParser::Options &options)\n>  \n>  \tcamera_->requestCompleted.connect(this, &CameraSession::requestComplete);\n>  \n> -\tif (options.isSet(OptFile)) {\n> -\t\tif (!options[OptFile].toString().empty())\n> -\t\t\twriter_ = std::make_unique<BufferWriter>(options[OptFile]);\n> +\tif (options_.isSet(OptFile)) {\n> +\t\tif (!options_[OptFile].toString().empty())\n> +\t\t\twriter_ = std::make_unique<BufferWriter>(options_[OptFile]);\n>  \t\telse\n>  \t\t\twriter_ = std::make_unique<BufferWriter>();\n>  \t}\n> diff --git a/src/cam/camera_session.h b/src/cam/camera_session.h\n> index 6221aadadf90..f137279ab421 100644\n> --- a/src/cam/camera_session.h\n> +++ b/src/cam/camera_session.h\n> @@ -9,6 +9,7 @@\n>  \n>  #include <memory>\n>  #include <stdint.h>\n> +#include <string>\n>  #include <vector>\n>  \n>  #include <libcamera/base/signal.h>\n> @@ -27,10 +28,12 @@ class CameraSession\n>  {\n>  public:\n>  \tCameraSession(libcamera::CameraManager *cm,\n> +\t\t      const std::string &cameraId,\n>  \t\t      const OptionsParser::Options &options);\n>  \t~CameraSession();\n>  \n>  \tbool isValid() const { return config_ != nullptr; }\n> +\tconst OptionsParser::Options &options() { return options_; };\n>  \n>  \tlibcamera::Camera *camera() { return camera_.get(); }\n>  \tlibcamera::CameraConfiguration *config() { return config_.get(); }\n> @@ -39,7 +42,7 @@ public:\n>  \tvoid listProperties() const;\n>  \tvoid infoConfiguration() const;\n>  \n> -\tint start(const OptionsParser::Options &options);\n> +\tint start();\n>  \tvoid stop();\n>  \n>  \tlibcamera::Signal<> captureDone;\n> @@ -51,6 +54,7 @@ private:\n>  \tvoid requestComplete(libcamera::Request *request);\n>  \tvoid processRequest(libcamera::Request *request);\n>  \n> +\tconst OptionsParser::Options &options_;\n>  \tstd::shared_ptr<libcamera::Camera> camera_;\n>  \tstd::unique_ptr<libcamera::CameraConfiguration> config_;\n>  \n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index 6d7d45e11499..7688fa5540ea 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -113,19 +113,6 @@ int CamApp::parseOptions(int argc, char *argv[])\n>  \tparser.addOption(OptCamera, OptionString,\n>  \t\t\t \"Specify which camera to operate on, by id or by index\", \"camera\",\n>  \t\t\t ArgumentRequired, \"camera\");\n> -\tparser.addOption(OptCapture, OptionInteger,\n> -\t\t\t \"Capture until interrupted by user or until <count> frames captured\",\n> -\t\t\t \"capture\", ArgumentOptional, \"count\");\n> -\tparser.addOption(OptFile, OptionString,\n> -\t\t\t \"Write captured frames to disk\\n\"\n> -\t\t\t \"If the file name ends with a '/', it sets the directory in which\\n\"\n> -\t\t\t \"to write files, using the default file name. Otherwise it sets the\\n\"\n> -\t\t\t \"full file path and name. The first '#' character in the file name\\n\"\n> -\t\t\t \"is expanded to the stream name and frame sequence number.\\n\"\n> -\t\t\t \"The default file name is 'frame-#.bin'.\",\n> -\t\t\t \"file\", ArgumentOptional, \"filename\");\n> -\tparser.addOption(OptStream, &streamKeyValue,\n> -\t\t\t \"Set configuration of a camera stream\", \"stream\", true);\n>  \tparser.addOption(OptHelp, OptionNone, \"Display this help message\",\n>  \t\t\t \"help\");\n>  \tparser.addOption(OptInfo, OptionNone,\n> @@ -138,12 +125,32 @@ int CamApp::parseOptions(int argc, char *argv[])\n>  \tparser.addOption(OptMonitor, OptionNone,\n>  \t\t\t \"Monitor for hotplug and unplug camera events\",\n>  \t\t\t \"monitor\");\n> +\n> +\t/* Sub-options of OptCamera: */\n> +\tparser.addOption(OptCapture, OptionInteger,\n> +\t\t\t \"Capture until interrupted by user or until <count> frames captured\",\n> +\t\t\t \"capture\", ArgumentOptional, \"count\", false,\n> +\t\t\t OptCamera);\n> +\tparser.addOption(OptFile, OptionString,\n> +\t\t\t \"Write captured frames to disk\\n\"\n> +\t\t\t \"If the file name ends with a '/', it sets the directory in which\\n\"\n> +\t\t\t \"to write files, using the default file name. Otherwise it sets the\\n\"\n> +\t\t\t \"full file path and name. The first '#' character in the file name\\n\"\n> +\t\t\t \"is expanded to the stream name and frame sequence number.\\n\"\n> +\t\t\t \"The default file name is 'frame-#.bin'.\",\n> +\t\t\t \"file\", ArgumentOptional, \"filename\", false,\n> +\t\t\t OptCamera);\n> +\tparser.addOption(OptStream, &streamKeyValue,\n> +\t\t\t \"Set configuration of a camera stream\", \"stream\", true,\n> +\t\t\t OptCamera);\n>  \tparser.addOption(OptStrictFormats, OptionNone,\n>  \t\t\t \"Do not allow requested stream format(s) to be adjusted\",\n> -\t\t\t \"strict-formats\");\n> +\t\t\t \"strict-formats\", ArgumentNone, nullptr, false,\n> +\t\t\t OptCamera);\n>  \tparser.addOption(OptMetadata, OptionNone,\n>  \t\t\t \"Print the metadata for completed requests\",\n> -\t\t\t \"metadata\");\n> +\t\t\t \"metadata\", ArgumentNone, nullptr, false,\n> +\t\t\t OptCamera);\n>  \n>  \toptions_ = parser.parse(argc, argv);\n>  \tif (!options_.valid())\n> @@ -192,7 +199,10 @@ int CamApp::run()\n>  \tstd::unique_ptr<CameraSession> session;\n>  \n>  \tif (options_.isSet(OptCamera)) {\n> -\t\tsession = std::make_unique<CameraSession>(cm_.get(), options_);\n> +\t\tconst OptionValue &camera = options_[OptCamera];\n> +\t\tsession = std::make_unique<CameraSession>(cm_.get(),\n> +\t\t\t\t\t\t\t  camera.toString(),\n> +\t\t\t\t\t\t\t  camera.children());\n>  \t\tif (!session->isValid()) {\n>  \t\t\tstd::cout << \"Failed to create camera session\" << std::endl;\n>  \t\t\treturn -EINVAL;\n> @@ -223,13 +233,8 @@ int CamApp::run()\n>  \t}\n>  \n>  \t/* 4. Start capture. */\n> -\tif (options_.isSet(OptCapture)) {\n> -\t\tif (!session) {\n> -\t\t\tstd::cout << \"Can't capture without a camera\" << std::endl;\n> -\t\t\treturn -ENODEV;\n> -\t\t}\n> -\n> -\t\tret = session->start(options_);\n> +\tif (session && session->options().isSet(OptCapture)) {\n> +\t\tret = session->start();\n>  \t\tif (ret) {\n>  \t\t\tstd::cout << \"Failed to start camera session\" << std::endl;\n>  \t\t\treturn ret;\n> @@ -253,7 +258,7 @@ int CamApp::run()\n>  \t\tloop_.exec();\n>  \n>  \t/* 6. Stop capture. */\n> -\tif (options_.isSet(OptCapture))\n> +\tif (session && session->options().isSet(OptCapture))\n>  \t\tsession->stop();\n>  \n>  \treturn 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 F07F4C3226\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 12 Jul 2021 16:11:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6BD4868524;\n\tMon, 12 Jul 2021 18:11:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 323FC68513\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 12 Jul 2021 18:11:41 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A2B06CC;\n\tMon, 12 Jul 2021 18:11:40 +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=\"UsiWaE+/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1626106300;\n\tbh=UAS5/sFg98vmXMXNq0rZIcHEDKKIasKJ4R2K+ySkD+Q=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=UsiWaE+/uCP0+HXk4EzIumPuKF9u/6h0LrMR1SUleMx8crO7AFAlpNPn/u3UI4L90\n\tFm1U0rzuxPFKYau4/L9eH1TluoS9/i6i1pTgpQQAkLQr2cY+Xk62xwT6LNIm6aLT+T\n\tGHvyNqO5RwnatvZpZ/A3BmiDu+0x5ky4/RyTcfLg=","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210707021941.20804-1-laurent.pinchart@ideasonboard.com>\n\t<20210707021941.20804-29-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<211d5935-ddfb-e80b-aa4c-8853e4eac93a@ideasonboard.com>","Date":"Mon, 12 Jul 2021 17:11:38 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"<20210707021941.20804-29-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH 28/30] cam: Make camera-related\n\toptions sub-options of OptCamera","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":18152,"web_url":"https://patchwork.libcamera.org/comment/18152/","msgid":"<YOyO/YL9BaqSyMjz@pendragon.ideasonboard.com>","date":"2021-07-12T18:50:37","subject":"Re: [libcamera-devel] [PATCH 28/30] cam: Make camera-related\n\toptions sub-options of OptCamera","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Mon, Jul 12, 2021 at 05:11:38PM +0100, Kieran Bingham wrote:\n> On 07/07/2021 03:19, Laurent Pinchart wrote:\n> > Use the new hierarchical options feature of the option parser to turn\n> > camera-related option (--capture, --file, --stream, --strict-formats and\n> > --metadata) into children of the --camera option. As an added bonus, we\n> > don't need to check anymore if a camera has been specified when capture\n> > is requested, as that's now enforced by the option parser.\n> \n> Aha that's what I was looking for earlier... so here it is ;-)\n> \n> Although - wouldn't this also mean we could handle printing camera\n> information here too ? (--list-controls or such)\n\nYes, we could do so too, and I'm not sure what would be best. That's why\nI haven't moved those arguments yet. Do we expect use cases where we\nwant to print different information for different cameras ?\n\n> Anyway, this is still progress:\n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> > This change prepares for support of multiple cameras.\n> > \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  src/cam/camera_session.cpp | 22 ++++++++--------\n> >  src/cam/camera_session.h   |  6 ++++-\n> >  src/cam/main.cpp           | 53 +++++++++++++++++++++-----------------\n> >  3 files changed, 45 insertions(+), 36 deletions(-)\n> > \n> > diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp\n> > index ceb2c3ab3a92..f2383567af3b 100644\n> > --- a/src/cam/camera_session.cpp\n> > +++ b/src/cam/camera_session.cpp\n> > @@ -21,11 +21,11 @@\n> >  using namespace libcamera;\n> >  \n> >  CameraSession::CameraSession(CameraManager *cm,\n> > +\t\t\t     const std::string &cameraId,\n> >  \t\t\t     const OptionsParser::Options &options)\n> > -\t: last_(0), queueCount_(0), captureCount_(0),\n> > +\t: options_(options), last_(0), queueCount_(0), captureCount_(0),\n> >  \t  captureLimit_(0), printMetadata_(false)\n> >  {\n> > -\tconst std::string &cameraId = options[OptCamera];\n> >  \tchar *endptr;\n> >  \tunsigned long index = strtoul(cameraId.c_str(), &endptr, 10);\n> >  \tif (*endptr == '\\0' && index > 0 && index <= cm->cameras().size())\n> > @@ -44,7 +44,7 @@ CameraSession::CameraSession(CameraManager *cm,\n> >  \t\treturn;\n> >  \t}\n> >  \n> > -\tStreamRoles roles = StreamKeyValueParser::roles(options[OptStream]);\n> > +\tStreamRoles roles = StreamKeyValueParser::roles(options_[OptStream]);\n> >  \n> >  \tstd::unique_ptr<CameraConfiguration> config =\n> >  \t\tcamera_->generateConfiguration(roles);\n> > @@ -56,12 +56,12 @@ CameraSession::CameraSession(CameraManager *cm,\n> >  \n> >  \t/* Apply configuration if explicitly requested. */\n> >  \tif (StreamKeyValueParser::updateConfiguration(config.get(),\n> > -\t\t\t\t\t\t      options[OptStream])) {\n> > +\t\t\t\t\t\t      options_[OptStream])) {\n> >  \t\tstd::cerr << \"Failed to update configuration\" << std::endl;\n> >  \t\treturn;\n> >  \t}\n> >  \n> > -\tbool strictFormats = options.isSet(OptStrictFormats);\n> > +\tbool strictFormats = options_.isSet(OptStrictFormats);\n> >  \n> >  \tswitch (config->validate()) {\n> >  \tcase CameraConfiguration::Valid:\n> > @@ -134,14 +134,14 @@ void CameraSession::infoConfiguration() const\n> >  \t}\n> >  }\n> >  \n> > -int CameraSession::start(const OptionsParser::Options &options)\n> > +int CameraSession::start()\n> >  {\n> >  \tint ret;\n> >  \n> >  \tqueueCount_ = 0;\n> >  \tcaptureCount_ = 0;\n> > -\tcaptureLimit_ = options[OptCapture].toInteger();\n> > -\tprintMetadata_ = options.isSet(OptMetadata);\n> > +\tcaptureLimit_ = options_[OptCapture].toInteger();\n> > +\tprintMetadata_ = options_.isSet(OptMetadata);\n> >  \n> >  \tret = camera_->configure(config_.get());\n> >  \tif (ret < 0) {\n> > @@ -157,9 +157,9 @@ int CameraSession::start(const OptionsParser::Options &options)\n> >  \n> >  \tcamera_->requestCompleted.connect(this, &CameraSession::requestComplete);\n> >  \n> > -\tif (options.isSet(OptFile)) {\n> > -\t\tif (!options[OptFile].toString().empty())\n> > -\t\t\twriter_ = std::make_unique<BufferWriter>(options[OptFile]);\n> > +\tif (options_.isSet(OptFile)) {\n> > +\t\tif (!options_[OptFile].toString().empty())\n> > +\t\t\twriter_ = std::make_unique<BufferWriter>(options_[OptFile]);\n> >  \t\telse\n> >  \t\t\twriter_ = std::make_unique<BufferWriter>();\n> >  \t}\n> > diff --git a/src/cam/camera_session.h b/src/cam/camera_session.h\n> > index 6221aadadf90..f137279ab421 100644\n> > --- a/src/cam/camera_session.h\n> > +++ b/src/cam/camera_session.h\n> > @@ -9,6 +9,7 @@\n> >  \n> >  #include <memory>\n> >  #include <stdint.h>\n> > +#include <string>\n> >  #include <vector>\n> >  \n> >  #include <libcamera/base/signal.h>\n> > @@ -27,10 +28,12 @@ class CameraSession\n> >  {\n> >  public:\n> >  \tCameraSession(libcamera::CameraManager *cm,\n> > +\t\t      const std::string &cameraId,\n> >  \t\t      const OptionsParser::Options &options);\n> >  \t~CameraSession();\n> >  \n> >  \tbool isValid() const { return config_ != nullptr; }\n> > +\tconst OptionsParser::Options &options() { return options_; };\n> >  \n> >  \tlibcamera::Camera *camera() { return camera_.get(); }\n> >  \tlibcamera::CameraConfiguration *config() { return config_.get(); }\n> > @@ -39,7 +42,7 @@ public:\n> >  \tvoid listProperties() const;\n> >  \tvoid infoConfiguration() const;\n> >  \n> > -\tint start(const OptionsParser::Options &options);\n> > +\tint start();\n> >  \tvoid stop();\n> >  \n> >  \tlibcamera::Signal<> captureDone;\n> > @@ -51,6 +54,7 @@ private:\n> >  \tvoid requestComplete(libcamera::Request *request);\n> >  \tvoid processRequest(libcamera::Request *request);\n> >  \n> > +\tconst OptionsParser::Options &options_;\n> >  \tstd::shared_ptr<libcamera::Camera> camera_;\n> >  \tstd::unique_ptr<libcamera::CameraConfiguration> config_;\n> >  \n> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > index 6d7d45e11499..7688fa5540ea 100644\n> > --- a/src/cam/main.cpp\n> > +++ b/src/cam/main.cpp\n> > @@ -113,19 +113,6 @@ int CamApp::parseOptions(int argc, char *argv[])\n> >  \tparser.addOption(OptCamera, OptionString,\n> >  \t\t\t \"Specify which camera to operate on, by id or by index\", \"camera\",\n> >  \t\t\t ArgumentRequired, \"camera\");\n> > -\tparser.addOption(OptCapture, OptionInteger,\n> > -\t\t\t \"Capture until interrupted by user or until <count> frames captured\",\n> > -\t\t\t \"capture\", ArgumentOptional, \"count\");\n> > -\tparser.addOption(OptFile, OptionString,\n> > -\t\t\t \"Write captured frames to disk\\n\"\n> > -\t\t\t \"If the file name ends with a '/', it sets the directory in which\\n\"\n> > -\t\t\t \"to write files, using the default file name. Otherwise it sets the\\n\"\n> > -\t\t\t \"full file path and name. The first '#' character in the file name\\n\"\n> > -\t\t\t \"is expanded to the stream name and frame sequence number.\\n\"\n> > -\t\t\t \"The default file name is 'frame-#.bin'.\",\n> > -\t\t\t \"file\", ArgumentOptional, \"filename\");\n> > -\tparser.addOption(OptStream, &streamKeyValue,\n> > -\t\t\t \"Set configuration of a camera stream\", \"stream\", true);\n> >  \tparser.addOption(OptHelp, OptionNone, \"Display this help message\",\n> >  \t\t\t \"help\");\n> >  \tparser.addOption(OptInfo, OptionNone,\n> > @@ -138,12 +125,32 @@ int CamApp::parseOptions(int argc, char *argv[])\n> >  \tparser.addOption(OptMonitor, OptionNone,\n> >  \t\t\t \"Monitor for hotplug and unplug camera events\",\n> >  \t\t\t \"monitor\");\n> > +\n> > +\t/* Sub-options of OptCamera: */\n> > +\tparser.addOption(OptCapture, OptionInteger,\n> > +\t\t\t \"Capture until interrupted by user or until <count> frames captured\",\n> > +\t\t\t \"capture\", ArgumentOptional, \"count\", false,\n> > +\t\t\t OptCamera);\n> > +\tparser.addOption(OptFile, OptionString,\n> > +\t\t\t \"Write captured frames to disk\\n\"\n> > +\t\t\t \"If the file name ends with a '/', it sets the directory in which\\n\"\n> > +\t\t\t \"to write files, using the default file name. Otherwise it sets the\\n\"\n> > +\t\t\t \"full file path and name. The first '#' character in the file name\\n\"\n> > +\t\t\t \"is expanded to the stream name and frame sequence number.\\n\"\n> > +\t\t\t \"The default file name is 'frame-#.bin'.\",\n> > +\t\t\t \"file\", ArgumentOptional, \"filename\", false,\n> > +\t\t\t OptCamera);\n> > +\tparser.addOption(OptStream, &streamKeyValue,\n> > +\t\t\t \"Set configuration of a camera stream\", \"stream\", true,\n> > +\t\t\t OptCamera);\n> >  \tparser.addOption(OptStrictFormats, OptionNone,\n> >  \t\t\t \"Do not allow requested stream format(s) to be adjusted\",\n> > -\t\t\t \"strict-formats\");\n> > +\t\t\t \"strict-formats\", ArgumentNone, nullptr, false,\n> > +\t\t\t OptCamera);\n> >  \tparser.addOption(OptMetadata, OptionNone,\n> >  \t\t\t \"Print the metadata for completed requests\",\n> > -\t\t\t \"metadata\");\n> > +\t\t\t \"metadata\", ArgumentNone, nullptr, false,\n> > +\t\t\t OptCamera);\n> >  \n> >  \toptions_ = parser.parse(argc, argv);\n> >  \tif (!options_.valid())\n> > @@ -192,7 +199,10 @@ int CamApp::run()\n> >  \tstd::unique_ptr<CameraSession> session;\n> >  \n> >  \tif (options_.isSet(OptCamera)) {\n> > -\t\tsession = std::make_unique<CameraSession>(cm_.get(), options_);\n> > +\t\tconst OptionValue &camera = options_[OptCamera];\n> > +\t\tsession = std::make_unique<CameraSession>(cm_.get(),\n> > +\t\t\t\t\t\t\t  camera.toString(),\n> > +\t\t\t\t\t\t\t  camera.children());\n> >  \t\tif (!session->isValid()) {\n> >  \t\t\tstd::cout << \"Failed to create camera session\" << std::endl;\n> >  \t\t\treturn -EINVAL;\n> > @@ -223,13 +233,8 @@ int CamApp::run()\n> >  \t}\n> >  \n> >  \t/* 4. Start capture. */\n> > -\tif (options_.isSet(OptCapture)) {\n> > -\t\tif (!session) {\n> > -\t\t\tstd::cout << \"Can't capture without a camera\" << std::endl;\n> > -\t\t\treturn -ENODEV;\n> > -\t\t}\n> > -\n> > -\t\tret = session->start(options_);\n> > +\tif (session && session->options().isSet(OptCapture)) {\n> > +\t\tret = session->start();\n> >  \t\tif (ret) {\n> >  \t\t\tstd::cout << \"Failed to start camera session\" << std::endl;\n> >  \t\t\treturn ret;\n> > @@ -253,7 +258,7 @@ int CamApp::run()\n> >  \t\tloop_.exec();\n> >  \n> >  \t/* 6. Stop capture. */\n> > -\tif (options_.isSet(OptCapture))\n> > +\tif (session && session->options().isSet(OptCapture))\n> >  \t\tsession->stop();\n> >  \n> >  \treturn 0;","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 23683C3226\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 12 Jul 2021 18:51:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 809DE68524;\n\tMon, 12 Jul 2021 20:51:25 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1715A68513\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 12 Jul 2021 20:51:24 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8A381CC;\n\tMon, 12 Jul 2021 20:51:23 +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=\"OLIwxirn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1626115883;\n\tbh=7qEPN4SYvX2uliqkVYWeK0Ebm5FoZS2cFIB6NNjU5mQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OLIwxirnjDr2yOE4XH4fKneNjHEkikAMP/oEnqePZEsPwmPb9RFRJhOQ/gK4EhnRr\n\tsytjI9Nhy/uoMvdRedzu7hc0MGDQrikobUWsRjHBtPPflpk6VdYG2bKV84ythHzvMl\n\tiC+SV8zbHxrumb1bmgfl0o/efl19HV8w/WzvtNtA=","Date":"Mon, 12 Jul 2021 21:50:37 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<YOyO/YL9BaqSyMjz@pendragon.ideasonboard.com>","References":"<20210707021941.20804-1-laurent.pinchart@ideasonboard.com>\n\t<20210707021941.20804-29-laurent.pinchart@ideasonboard.com>\n\t<211d5935-ddfb-e80b-aa4c-8853e4eac93a@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<211d5935-ddfb-e80b-aa4c-8853e4eac93a@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 28/30] cam: Make camera-related\n\toptions sub-options of OptCamera","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>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]