[libcamera-devel,28/30] cam: Make camera-related options sub-options of OptCamera
diff mbox series

Message ID 20210707021941.20804-29-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • Multi-camera support in the cam application
Related show

Commit Message

Laurent Pinchart July 7, 2021, 2:19 a.m. UTC 
Use the new hierarchical options feature of the option parser to turn
camera-related option (--capture, --file, --stream, --strict-formats and
--metadata) into children of the --camera option. As an added bonus, we
don't need to check anymore if a camera has been specified when capture
is requested, as that's now enforced by the option parser.

This change prepares for support of multiple cameras.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/cam/camera_session.cpp | 22 ++++++++--------
 src/cam/camera_session.h   |  6 ++++-
 src/cam/main.cpp           | 53 +++++++++++++++++++++-----------------
 3 files changed, 45 insertions(+), 36 deletions(-)

Comments

Kieran Bingham July 12, 2021, 4:11 p.m. UTC | #1 
On 07/07/2021 03:19, Laurent Pinchart wrote:
> Use the new hierarchical options feature of the option parser to turn
> camera-related option (--capture, --file, --stream, --strict-formats and
> --metadata) into children of the --camera option. As an added bonus, we
> don't need to check anymore if a camera has been specified when capture
> is requested, as that's now enforced by the option parser.


Aha that's what I was looking for earlier... so here it is ;-)

Although - wouldn't this also mean we could handle printing camera
information here too ? (--list-controls or such)


Anyway, this is still progress:

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


> This change prepares for support of multiple cameras.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/cam/camera_session.cpp | 22 ++++++++--------
>  src/cam/camera_session.h   |  6 ++++-
>  src/cam/main.cpp           | 53 +++++++++++++++++++++-----------------
>  3 files changed, 45 insertions(+), 36 deletions(-)
> 
> diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
> index ceb2c3ab3a92..f2383567af3b 100644
> --- a/src/cam/camera_session.cpp
> +++ b/src/cam/camera_session.cpp
> @@ -21,11 +21,11 @@
>  using namespace libcamera;
>  
>  CameraSession::CameraSession(CameraManager *cm,
> +			     const std::string &cameraId,
>  			     const OptionsParser::Options &options)
> -	: last_(0), queueCount_(0), captureCount_(0),
> +	: options_(options), last_(0), queueCount_(0), captureCount_(0),
>  	  captureLimit_(0), printMetadata_(false)
>  {
> -	const std::string &cameraId = options[OptCamera];
>  	char *endptr;
>  	unsigned long index = strtoul(cameraId.c_str(), &endptr, 10);
>  	if (*endptr == '\0' && index > 0 && index <= cm->cameras().size())
> @@ -44,7 +44,7 @@ CameraSession::CameraSession(CameraManager *cm,
>  		return;
>  	}
>  
> -	StreamRoles roles = StreamKeyValueParser::roles(options[OptStream]);
> +	StreamRoles roles = StreamKeyValueParser::roles(options_[OptStream]);
>  
>  	std::unique_ptr<CameraConfiguration> config =
>  		camera_->generateConfiguration(roles);
> @@ -56,12 +56,12 @@ CameraSession::CameraSession(CameraManager *cm,
>  
>  	/* Apply configuration if explicitly requested. */
>  	if (StreamKeyValueParser::updateConfiguration(config.get(),
> -						      options[OptStream])) {
> +						      options_[OptStream])) {
>  		std::cerr << "Failed to update configuration" << std::endl;
>  		return;
>  	}
>  
> -	bool strictFormats = options.isSet(OptStrictFormats);
> +	bool strictFormats = options_.isSet(OptStrictFormats);
>  
>  	switch (config->validate()) {
>  	case CameraConfiguration::Valid:
> @@ -134,14 +134,14 @@ void CameraSession::infoConfiguration() const
>  	}
>  }
>  
> -int CameraSession::start(const OptionsParser::Options &options)
> +int CameraSession::start()
>  {
>  	int ret;
>  
>  	queueCount_ = 0;
>  	captureCount_ = 0;
> -	captureLimit_ = options[OptCapture].toInteger();
> -	printMetadata_ = options.isSet(OptMetadata);
> +	captureLimit_ = options_[OptCapture].toInteger();
> +	printMetadata_ = options_.isSet(OptMetadata);
>  
>  	ret = camera_->configure(config_.get());
>  	if (ret < 0) {
> @@ -157,9 +157,9 @@ int CameraSession::start(const OptionsParser::Options &options)
>  
>  	camera_->requestCompleted.connect(this, &CameraSession::requestComplete);
>  
> -	if (options.isSet(OptFile)) {
> -		if (!options[OptFile].toString().empty())
> -			writer_ = std::make_unique<BufferWriter>(options[OptFile]);
> +	if (options_.isSet(OptFile)) {
> +		if (!options_[OptFile].toString().empty())
> +			writer_ = std::make_unique<BufferWriter>(options_[OptFile]);
>  		else
>  			writer_ = std::make_unique<BufferWriter>();
>  	}
> diff --git a/src/cam/camera_session.h b/src/cam/camera_session.h
> index 6221aadadf90..f137279ab421 100644
> --- a/src/cam/camera_session.h
> +++ b/src/cam/camera_session.h
> @@ -9,6 +9,7 @@
>  
>  #include <memory>
>  #include <stdint.h>
> +#include <string>
>  #include <vector>
>  
>  #include <libcamera/base/signal.h>
> @@ -27,10 +28,12 @@ class CameraSession
>  {
>  public:
>  	CameraSession(libcamera::CameraManager *cm,
> +		      const std::string &cameraId,
>  		      const OptionsParser::Options &options);
>  	~CameraSession();
>  
>  	bool isValid() const { return config_ != nullptr; }
> +	const OptionsParser::Options &options() { return options_; };
>  
>  	libcamera::Camera *camera() { return camera_.get(); }
>  	libcamera::CameraConfiguration *config() { return config_.get(); }
> @@ -39,7 +42,7 @@ public:
>  	void listProperties() const;
>  	void infoConfiguration() const;
>  
> -	int start(const OptionsParser::Options &options);
> +	int start();
>  	void stop();
>  
>  	libcamera::Signal<> captureDone;
> @@ -51,6 +54,7 @@ private:
>  	void requestComplete(libcamera::Request *request);
>  	void processRequest(libcamera::Request *request);
>  
> +	const OptionsParser::Options &options_;
>  	std::shared_ptr<libcamera::Camera> camera_;
>  	std::unique_ptr<libcamera::CameraConfiguration> config_;
>  
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index 6d7d45e11499..7688fa5540ea 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -113,19 +113,6 @@ int CamApp::parseOptions(int argc, char *argv[])
>  	parser.addOption(OptCamera, OptionString,
>  			 "Specify which camera to operate on, by id or by index", "camera",
>  			 ArgumentRequired, "camera");
> -	parser.addOption(OptCapture, OptionInteger,
> -			 "Capture until interrupted by user or until <count> frames captured",
> -			 "capture", ArgumentOptional, "count");
> -	parser.addOption(OptFile, OptionString,
> -			 "Write captured frames to disk\n"
> -			 "If the file name ends with a '/', it sets the directory in which\n"
> -			 "to write files, using the default file name. Otherwise it sets the\n"
> -			 "full file path and name. The first '#' character in the file name\n"
> -			 "is expanded to the stream name and frame sequence number.\n"
> -			 "The default file name is 'frame-#.bin'.",
> -			 "file", ArgumentOptional, "filename");
> -	parser.addOption(OptStream, &streamKeyValue,
> -			 "Set configuration of a camera stream", "stream", true);
>  	parser.addOption(OptHelp, OptionNone, "Display this help message",
>  			 "help");
>  	parser.addOption(OptInfo, OptionNone,
> @@ -138,12 +125,32 @@ int CamApp::parseOptions(int argc, char *argv[])
>  	parser.addOption(OptMonitor, OptionNone,
>  			 "Monitor for hotplug and unplug camera events",
>  			 "monitor");
> +
> +	/* Sub-options of OptCamera: */
> +	parser.addOption(OptCapture, OptionInteger,
> +			 "Capture until interrupted by user or until <count> frames captured",
> +			 "capture", ArgumentOptional, "count", false,
> +			 OptCamera);
> +	parser.addOption(OptFile, OptionString,
> +			 "Write captured frames to disk\n"
> +			 "If the file name ends with a '/', it sets the directory in which\n"
> +			 "to write files, using the default file name. Otherwise it sets the\n"
> +			 "full file path and name. The first '#' character in the file name\n"
> +			 "is expanded to the stream name and frame sequence number.\n"
> +			 "The default file name is 'frame-#.bin'.",
> +			 "file", ArgumentOptional, "filename", false,
> +			 OptCamera);
> +	parser.addOption(OptStream, &streamKeyValue,
> +			 "Set configuration of a camera stream", "stream", true,
> +			 OptCamera);
>  	parser.addOption(OptStrictFormats, OptionNone,
>  			 "Do not allow requested stream format(s) to be adjusted",
> -			 "strict-formats");
> +			 "strict-formats", ArgumentNone, nullptr, false,
> +			 OptCamera);
>  	parser.addOption(OptMetadata, OptionNone,
>  			 "Print the metadata for completed requests",
> -			 "metadata");
> +			 "metadata", ArgumentNone, nullptr, false,
> +			 OptCamera);
>  
>  	options_ = parser.parse(argc, argv);
>  	if (!options_.valid())
> @@ -192,7 +199,10 @@ int CamApp::run()
>  	std::unique_ptr<CameraSession> session;
>  
>  	if (options_.isSet(OptCamera)) {
> -		session = std::make_unique<CameraSession>(cm_.get(), options_);
> +		const OptionValue &camera = options_[OptCamera];
> +		session = std::make_unique<CameraSession>(cm_.get(),
> +							  camera.toString(),
> +							  camera.children());
>  		if (!session->isValid()) {
>  			std::cout << "Failed to create camera session" << std::endl;
>  			return -EINVAL;
> @@ -223,13 +233,8 @@ int CamApp::run()
>  	}
>  
>  	/* 4. Start capture. */
> -	if (options_.isSet(OptCapture)) {
> -		if (!session) {
> -			std::cout << "Can't capture without a camera" << std::endl;
> -			return -ENODEV;
> -		}
> -
> -		ret = session->start(options_);
> +	if (session && session->options().isSet(OptCapture)) {
> +		ret = session->start();
>  		if (ret) {
>  			std::cout << "Failed to start camera session" << std::endl;
>  			return ret;
> @@ -253,7 +258,7 @@ int CamApp::run()
>  		loop_.exec();
>  
>  	/* 6. Stop capture. */
> -	if (options_.isSet(OptCapture))
> +	if (session && session->options().isSet(OptCapture))
>  		session->stop();
>  
>  	return 0;
>
Laurent Pinchart July 12, 2021, 6:50 p.m. UTC | #2 
Hi Kieran,

On Mon, Jul 12, 2021 at 05:11:38PM +0100, Kieran Bingham wrote:
> On 07/07/2021 03:19, Laurent Pinchart wrote:
> > Use the new hierarchical options feature of the option parser to turn
> > camera-related option (--capture, --file, --stream, --strict-formats and
> > --metadata) into children of the --camera option. As an added bonus, we
> > don't need to check anymore if a camera has been specified when capture
> > is requested, as that's now enforced by the option parser.
> 
> Aha that's what I was looking for earlier... so here it is ;-)
> 
> Although - wouldn't this also mean we could handle printing camera
> information here too ? (--list-controls or such)

Yes, we could do so too, and I'm not sure what would be best. That's why
I haven't moved those arguments yet. Do we expect use cases where we
want to print different information for different cameras ?

> Anyway, this is still progress:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > This change prepares for support of multiple cameras.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/cam/camera_session.cpp | 22 ++++++++--------
> >  src/cam/camera_session.h   |  6 ++++-
> >  src/cam/main.cpp           | 53 +++++++++++++++++++++-----------------
> >  3 files changed, 45 insertions(+), 36 deletions(-)
> > 
> > diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
> > index ceb2c3ab3a92..f2383567af3b 100644
> > --- a/src/cam/camera_session.cpp
> > +++ b/src/cam/camera_session.cpp
> > @@ -21,11 +21,11 @@
> >  using namespace libcamera;
> >  
> >  CameraSession::CameraSession(CameraManager *cm,
> > +			     const std::string &cameraId,
> >  			     const OptionsParser::Options &options)
> > -	: last_(0), queueCount_(0), captureCount_(0),
> > +	: options_(options), last_(0), queueCount_(0), captureCount_(0),
> >  	  captureLimit_(0), printMetadata_(false)
> >  {
> > -	const std::string &cameraId = options[OptCamera];
> >  	char *endptr;
> >  	unsigned long index = strtoul(cameraId.c_str(), &endptr, 10);
> >  	if (*endptr == '\0' && index > 0 && index <= cm->cameras().size())
> > @@ -44,7 +44,7 @@ CameraSession::CameraSession(CameraManager *cm,
> >  		return;
> >  	}
> >  
> > -	StreamRoles roles = StreamKeyValueParser::roles(options[OptStream]);
> > +	StreamRoles roles = StreamKeyValueParser::roles(options_[OptStream]);
> >  
> >  	std::unique_ptr<CameraConfiguration> config =
> >  		camera_->generateConfiguration(roles);
> > @@ -56,12 +56,12 @@ CameraSession::CameraSession(CameraManager *cm,
> >  
> >  	/* Apply configuration if explicitly requested. */
> >  	if (StreamKeyValueParser::updateConfiguration(config.get(),
> > -						      options[OptStream])) {
> > +						      options_[OptStream])) {
> >  		std::cerr << "Failed to update configuration" << std::endl;
> >  		return;
> >  	}
> >  
> > -	bool strictFormats = options.isSet(OptStrictFormats);
> > +	bool strictFormats = options_.isSet(OptStrictFormats);
> >  
> >  	switch (config->validate()) {
> >  	case CameraConfiguration::Valid:
> > @@ -134,14 +134,14 @@ void CameraSession::infoConfiguration() const
> >  	}
> >  }
> >  
> > -int CameraSession::start(const OptionsParser::Options &options)
> > +int CameraSession::start()
> >  {
> >  	int ret;
> >  
> >  	queueCount_ = 0;
> >  	captureCount_ = 0;
> > -	captureLimit_ = options[OptCapture].toInteger();
> > -	printMetadata_ = options.isSet(OptMetadata);
> > +	captureLimit_ = options_[OptCapture].toInteger();
> > +	printMetadata_ = options_.isSet(OptMetadata);
> >  
> >  	ret = camera_->configure(config_.get());
> >  	if (ret < 0) {
> > @@ -157,9 +157,9 @@ int CameraSession::start(const OptionsParser::Options &options)
> >  
> >  	camera_->requestCompleted.connect(this, &CameraSession::requestComplete);
> >  
> > -	if (options.isSet(OptFile)) {
> > -		if (!options[OptFile].toString().empty())
> > -			writer_ = std::make_unique<BufferWriter>(options[OptFile]);
> > +	if (options_.isSet(OptFile)) {
> > +		if (!options_[OptFile].toString().empty())
> > +			writer_ = std::make_unique<BufferWriter>(options_[OptFile]);
> >  		else
> >  			writer_ = std::make_unique<BufferWriter>();
> >  	}
> > diff --git a/src/cam/camera_session.h b/src/cam/camera_session.h
> > index 6221aadadf90..f137279ab421 100644
> > --- a/src/cam/camera_session.h
> > +++ b/src/cam/camera_session.h
> > @@ -9,6 +9,7 @@
> >  
> >  #include <memory>
> >  #include <stdint.h>
> > +#include <string>
> >  #include <vector>
> >  
> >  #include <libcamera/base/signal.h>
> > @@ -27,10 +28,12 @@ class CameraSession
> >  {
> >  public:
> >  	CameraSession(libcamera::CameraManager *cm,
> > +		      const std::string &cameraId,
> >  		      const OptionsParser::Options &options);
> >  	~CameraSession();
> >  
> >  	bool isValid() const { return config_ != nullptr; }
> > +	const OptionsParser::Options &options() { return options_; };
> >  
> >  	libcamera::Camera *camera() { return camera_.get(); }
> >  	libcamera::CameraConfiguration *config() { return config_.get(); }
> > @@ -39,7 +42,7 @@ public:
> >  	void listProperties() const;
> >  	void infoConfiguration() const;
> >  
> > -	int start(const OptionsParser::Options &options);
> > +	int start();
> >  	void stop();
> >  
> >  	libcamera::Signal<> captureDone;
> > @@ -51,6 +54,7 @@ private:
> >  	void requestComplete(libcamera::Request *request);
> >  	void processRequest(libcamera::Request *request);
> >  
> > +	const OptionsParser::Options &options_;
> >  	std::shared_ptr<libcamera::Camera> camera_;
> >  	std::unique_ptr<libcamera::CameraConfiguration> config_;
> >  
> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> > index 6d7d45e11499..7688fa5540ea 100644
> > --- a/src/cam/main.cpp
> > +++ b/src/cam/main.cpp
> > @@ -113,19 +113,6 @@ int CamApp::parseOptions(int argc, char *argv[])
> >  	parser.addOption(OptCamera, OptionString,
> >  			 "Specify which camera to operate on, by id or by index", "camera",
> >  			 ArgumentRequired, "camera");
> > -	parser.addOption(OptCapture, OptionInteger,
> > -			 "Capture until interrupted by user or until <count> frames captured",
> > -			 "capture", ArgumentOptional, "count");
> > -	parser.addOption(OptFile, OptionString,
> > -			 "Write captured frames to disk\n"
> > -			 "If the file name ends with a '/', it sets the directory in which\n"
> > -			 "to write files, using the default file name. Otherwise it sets the\n"
> > -			 "full file path and name. The first '#' character in the file name\n"
> > -			 "is expanded to the stream name and frame sequence number.\n"
> > -			 "The default file name is 'frame-#.bin'.",
> > -			 "file", ArgumentOptional, "filename");
> > -	parser.addOption(OptStream, &streamKeyValue,
> > -			 "Set configuration of a camera stream", "stream", true);
> >  	parser.addOption(OptHelp, OptionNone, "Display this help message",
> >  			 "help");
> >  	parser.addOption(OptInfo, OptionNone,
> > @@ -138,12 +125,32 @@ int CamApp::parseOptions(int argc, char *argv[])
> >  	parser.addOption(OptMonitor, OptionNone,
> >  			 "Monitor for hotplug and unplug camera events",
> >  			 "monitor");
> > +
> > +	/* Sub-options of OptCamera: */
> > +	parser.addOption(OptCapture, OptionInteger,
> > +			 "Capture until interrupted by user or until <count> frames captured",
> > +			 "capture", ArgumentOptional, "count", false,
> > +			 OptCamera);
> > +	parser.addOption(OptFile, OptionString,
> > +			 "Write captured frames to disk\n"
> > +			 "If the file name ends with a '/', it sets the directory in which\n"
> > +			 "to write files, using the default file name. Otherwise it sets the\n"
> > +			 "full file path and name. The first '#' character in the file name\n"
> > +			 "is expanded to the stream name and frame sequence number.\n"
> > +			 "The default file name is 'frame-#.bin'.",
> > +			 "file", ArgumentOptional, "filename", false,
> > +			 OptCamera);
> > +	parser.addOption(OptStream, &streamKeyValue,
> > +			 "Set configuration of a camera stream", "stream", true,
> > +			 OptCamera);
> >  	parser.addOption(OptStrictFormats, OptionNone,
> >  			 "Do not allow requested stream format(s) to be adjusted",
> > -			 "strict-formats");
> > +			 "strict-formats", ArgumentNone, nullptr, false,
> > +			 OptCamera);
> >  	parser.addOption(OptMetadata, OptionNone,
> >  			 "Print the metadata for completed requests",
> > -			 "metadata");
> > +			 "metadata", ArgumentNone, nullptr, false,
> > +			 OptCamera);
> >  
> >  	options_ = parser.parse(argc, argv);
> >  	if (!options_.valid())
> > @@ -192,7 +199,10 @@ int CamApp::run()
> >  	std::unique_ptr<CameraSession> session;
> >  
> >  	if (options_.isSet(OptCamera)) {
> > -		session = std::make_unique<CameraSession>(cm_.get(), options_);
> > +		const OptionValue &camera = options_[OptCamera];
> > +		session = std::make_unique<CameraSession>(cm_.get(),
> > +							  camera.toString(),
> > +							  camera.children());
> >  		if (!session->isValid()) {
> >  			std::cout << "Failed to create camera session" << std::endl;
> >  			return -EINVAL;
> > @@ -223,13 +233,8 @@ int CamApp::run()
> >  	}
> >  
> >  	/* 4. Start capture. */
> > -	if (options_.isSet(OptCapture)) {
> > -		if (!session) {
> > -			std::cout << "Can't capture without a camera" << std::endl;
> > -			return -ENODEV;
> > -		}
> > -
> > -		ret = session->start(options_);
> > +	if (session && session->options().isSet(OptCapture)) {
> > +		ret = session->start();
> >  		if (ret) {
> >  			std::cout << "Failed to start camera session" << std::endl;
> >  			return ret;
> > @@ -253,7 +258,7 @@ int CamApp::run()
> >  		loop_.exec();
> >  
> >  	/* 6. Stop capture. */
> > -	if (options_.isSet(OptCapture))
> > +	if (session && session->options().isSet(OptCapture))
> >  		session->stop();
> >  
> >  	return 0;

Patch
diff mbox series 

diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
index ceb2c3ab3a92..f2383567af3b 100644
--- a/src/cam/camera_session.cpp
+++ b/src/cam/camera_session.cpp
@@ -21,11 +21,11 @@ 
 using namespace libcamera;
 
 CameraSession::CameraSession(CameraManager *cm,
+			     const std::string &cameraId,
 			     const OptionsParser::Options &options)
-	: last_(0), queueCount_(0), captureCount_(0),
+	: options_(options), last_(0), queueCount_(0), captureCount_(0),
 	  captureLimit_(0), printMetadata_(false)
 {
-	const std::string &cameraId = options[OptCamera];
 	char *endptr;
 	unsigned long index = strtoul(cameraId.c_str(), &endptr, 10);
 	if (*endptr == '\0' && index > 0 && index <= cm->cameras().size())
@@ -44,7 +44,7 @@  CameraSession::CameraSession(CameraManager *cm,
 		return;
 	}
 
-	StreamRoles roles = StreamKeyValueParser::roles(options[OptStream]);
+	StreamRoles roles = StreamKeyValueParser::roles(options_[OptStream]);
 
 	std::unique_ptr<CameraConfiguration> config =
 		camera_->generateConfiguration(roles);
@@ -56,12 +56,12 @@  CameraSession::CameraSession(CameraManager *cm,
 
 	/* Apply configuration if explicitly requested. */
 	if (StreamKeyValueParser::updateConfiguration(config.get(),
-						      options[OptStream])) {
+						      options_[OptStream])) {
 		std::cerr << "Failed to update configuration" << std::endl;
 		return;
 	}
 
-	bool strictFormats = options.isSet(OptStrictFormats);
+	bool strictFormats = options_.isSet(OptStrictFormats);
 
 	switch (config->validate()) {
 	case CameraConfiguration::Valid:
@@ -134,14 +134,14 @@  void CameraSession::infoConfiguration() const
 	}
 }
 
-int CameraSession::start(const OptionsParser::Options &options)
+int CameraSession::start()
 {
 	int ret;
 
 	queueCount_ = 0;
 	captureCount_ = 0;
-	captureLimit_ = options[OptCapture].toInteger();
-	printMetadata_ = options.isSet(OptMetadata);
+	captureLimit_ = options_[OptCapture].toInteger();
+	printMetadata_ = options_.isSet(OptMetadata);
 
 	ret = camera_->configure(config_.get());
 	if (ret < 0) {
@@ -157,9 +157,9 @@  int CameraSession::start(const OptionsParser::Options &options)
 
 	camera_->requestCompleted.connect(this, &CameraSession::requestComplete);
 
-	if (options.isSet(OptFile)) {
-		if (!options[OptFile].toString().empty())
-			writer_ = std::make_unique<BufferWriter>(options[OptFile]);
+	if (options_.isSet(OptFile)) {
+		if (!options_[OptFile].toString().empty())
+			writer_ = std::make_unique<BufferWriter>(options_[OptFile]);
 		else
 			writer_ = std::make_unique<BufferWriter>();
 	}
diff --git a/src/cam/camera_session.h b/src/cam/camera_session.h
index 6221aadadf90..f137279ab421 100644
--- a/src/cam/camera_session.h
+++ b/src/cam/camera_session.h
@@ -9,6 +9,7 @@ 
 
 #include <memory>
 #include <stdint.h>
+#include <string>
 #include <vector>
 
 #include <libcamera/base/signal.h>
@@ -27,10 +28,12 @@  class CameraSession
 {
 public:
 	CameraSession(libcamera::CameraManager *cm,
+		      const std::string &cameraId,
 		      const OptionsParser::Options &options);
 	~CameraSession();
 
 	bool isValid() const { return config_ != nullptr; }
+	const OptionsParser::Options &options() { return options_; };
 
 	libcamera::Camera *camera() { return camera_.get(); }
 	libcamera::CameraConfiguration *config() { return config_.get(); }
@@ -39,7 +42,7 @@  public:
 	void listProperties() const;
 	void infoConfiguration() const;
 
-	int start(const OptionsParser::Options &options);
+	int start();
 	void stop();
 
 	libcamera::Signal<> captureDone;
@@ -51,6 +54,7 @@  private:
 	void requestComplete(libcamera::Request *request);
 	void processRequest(libcamera::Request *request);
 
+	const OptionsParser::Options &options_;
 	std::shared_ptr<libcamera::Camera> camera_;
 	std::unique_ptr<libcamera::CameraConfiguration> config_;
 
diff --git a/src/cam/main.cpp b/src/cam/main.cpp
index 6d7d45e11499..7688fa5540ea 100644
--- a/src/cam/main.cpp
+++ b/src/cam/main.cpp
@@ -113,19 +113,6 @@  int CamApp::parseOptions(int argc, char *argv[])
 	parser.addOption(OptCamera, OptionString,
 			 "Specify which camera to operate on, by id or by index", "camera",
 			 ArgumentRequired, "camera");
-	parser.addOption(OptCapture, OptionInteger,
-			 "Capture until interrupted by user or until <count> frames captured",
-			 "capture", ArgumentOptional, "count");
-	parser.addOption(OptFile, OptionString,
-			 "Write captured frames to disk\n"
-			 "If the file name ends with a '/', it sets the directory in which\n"
-			 "to write files, using the default file name. Otherwise it sets the\n"
-			 "full file path and name. The first '#' character in the file name\n"
-			 "is expanded to the stream name and frame sequence number.\n"
-			 "The default file name is 'frame-#.bin'.",
-			 "file", ArgumentOptional, "filename");
-	parser.addOption(OptStream, &streamKeyValue,
-			 "Set configuration of a camera stream", "stream", true);
 	parser.addOption(OptHelp, OptionNone, "Display this help message",
 			 "help");
 	parser.addOption(OptInfo, OptionNone,
@@ -138,12 +125,32 @@  int CamApp::parseOptions(int argc, char *argv[])
 	parser.addOption(OptMonitor, OptionNone,
 			 "Monitor for hotplug and unplug camera events",
 			 "monitor");
+
+	/* Sub-options of OptCamera: */
+	parser.addOption(OptCapture, OptionInteger,
+			 "Capture until interrupted by user or until <count> frames captured",
+			 "capture", ArgumentOptional, "count", false,
+			 OptCamera);
+	parser.addOption(OptFile, OptionString,
+			 "Write captured frames to disk\n"
+			 "If the file name ends with a '/', it sets the directory in which\n"
+			 "to write files, using the default file name. Otherwise it sets the\n"
+			 "full file path and name. The first '#' character in the file name\n"
+			 "is expanded to the stream name and frame sequence number.\n"
+			 "The default file name is 'frame-#.bin'.",
+			 "file", ArgumentOptional, "filename", false,
+			 OptCamera);
+	parser.addOption(OptStream, &streamKeyValue,
+			 "Set configuration of a camera stream", "stream", true,
+			 OptCamera);
 	parser.addOption(OptStrictFormats, OptionNone,
 			 "Do not allow requested stream format(s) to be adjusted",
-			 "strict-formats");
+			 "strict-formats", ArgumentNone, nullptr, false,
+			 OptCamera);
 	parser.addOption(OptMetadata, OptionNone,
 			 "Print the metadata for completed requests",
-			 "metadata");
+			 "metadata", ArgumentNone, nullptr, false,
+			 OptCamera);
 
 	options_ = parser.parse(argc, argv);
 	if (!options_.valid())
@@ -192,7 +199,10 @@  int CamApp::run()
 	std::unique_ptr<CameraSession> session;
 
 	if (options_.isSet(OptCamera)) {
-		session = std::make_unique<CameraSession>(cm_.get(), options_);
+		const OptionValue &camera = options_[OptCamera];
+		session = std::make_unique<CameraSession>(cm_.get(),
+							  camera.toString(),
+							  camera.children());
 		if (!session->isValid()) {
 			std::cout << "Failed to create camera session" << std::endl;
 			return -EINVAL;
@@ -223,13 +233,8 @@  int CamApp::run()
 	}
 
 	/* 4. Start capture. */
-	if (options_.isSet(OptCapture)) {
-		if (!session) {
-			std::cout << "Can't capture without a camera" << std::endl;
-			return -ENODEV;
-		}
-
-		ret = session->start(options_);
+	if (session && session->options().isSet(OptCapture)) {
+		ret = session->start();
 		if (ret) {
 			std::cout << "Failed to start camera session" << std::endl;
 			return ret;
@@ -253,7 +258,7 @@  int CamApp::run()
 		loop_.exec();
 
 	/* 6. Stop capture. */
-	if (options_.isSet(OptCapture))
+	if (session && session->options().isSet(OptCapture))
 		session->stop();
 
 	return 0;