[libcamera-devel] [PATCH 30/30] cam: Support using multiple cameras concurrently

[Kieran Bingham]

On 07/07/2021 03:19, Laurent Pinchart wrote:
> To allow testing capture from multiple cameras concurrently, turn the
> --camera option into an array option, and create one CameraSession per
> specified camera. The code is adapted to iterate over the sessions
> vector instead of handling a single session. Thanks to all the
> refactoring previously performed, changes are minimal.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/cam/camera_session.cpp |  8 +++--
>  src/cam/main.cpp           | 67 ++++++++++++++++++++++----------------
>  2 files changed, 45 insertions(+), 30 deletions(-)
> 
> diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
> index 90261a8cde2e..0d49fc1ade83 100644
> --- a/src/cam/camera_session.cpp
> +++ b/src/cam/camera_session.cpp
> @@ -250,9 +250,13 @@ int CameraSession::startCapture()
>  	}
>  
>  	if (captureLimit_)
> -		std::cout << "Capture " << captureLimit_ << " frames" << std::endl;
> +		std::cout << "cam" << cameraIndex_
> +			  << ": Capture " << captureLimit_ << " frames"
> +			  << std::endl;
>  	else
> -		std::cout << "Capture until user interrupts by SIGINT" << std::endl;
> +		std::cout << "cam" << cameraIndex_
> +			  << ": Capture until user interrupts by SIGINT"

Hrm, we've called it SIGINT here and Ctrl-C on the hotplug monitor.

Not a cause of this patch though.


> +			  << std::endl;
>  
>  	return 0;
>  }
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index 96f55831ca69..f9a90672a474 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -112,7 +112,7 @@ int CamApp::parseOptions(int argc, char *argv[])
>  	OptionsParser parser;
>  	parser.addOption(OptCamera, OptionString,
>  			 "Specify which camera to operate on, by id or by index", "camera",
> -			 ArgumentRequired, "camera");
>  	parser.addOption(OptHelp, OptionNone, "Display this help message",
>  			 "help");
>  	parser.addOption(OptInfo, OptionNone,
> @@ -195,45 +195,52 @@ int CamApp::run()
>  		}
>  	}
>  
> -	/* 2. Create the camera session. */
> -	std::unique_ptr<CameraSession> session;
> +	/* 2. Create the camera sessions. */
> +	std::vector<std::unique_ptr<CameraSession>> sessions;
>  
>  	if (options_.isSet(OptCamera)) {
> -		const OptionValue &camera = options_[OptCamera];
> -		session = std::make_unique<CameraSession>(cm_.get(),
> -							  camera.toString(), 0,
> -							  camera.children());
> -		if (!session->isValid()) {
> -			std::cout << "Failed to create camera session" << std::endl;
> -			return -EINVAL;
> +		unsigned int index = 0;
> +
> +		for (const OptionValue &camera : options_[OptCamera].toArray()) {
> +			std::unique_ptr<CameraSession> session =
> +				std::make_unique<CameraSession>(cm_.get(),
> +								camera.toString(),
> +								index,
> +								camera.children());
> +			if (!session->isValid()) {
> +				std::cout << "Failed to create camera session" << std::endl;
> +				return -EINVAL;
> +			}
> +
> +			std::cout << "Using camera " << session->camera()->id()
> +				  << " as cam" << index << std::endl;
> +
> +			session->captureDone.connect(this, &CamApp::captureDone);
> +
> +			sessions.push_back(std::move(session));
> +			index++;
>  		}
> -
> -		std::cout << "Using camera " << session->camera()->id()
> -			  << std::endl;
> -
> -		session->captureDone.connect(this, &CamApp::captureDone);
>  	}
>  
>  	/* 3. Print camera information. */
>  	if (options_.isSet(OptListControls) ||
>  	    options_.isSet(OptListProperties) ||
>  	    options_.isSet(OptInfo)) {
> -		if (!session) {
> -			std::cout << "Cannot print camera information without a camera"
> -				  << std::endl;
> -			return -EINVAL;
> +		for (const auto &session : sessions) {
> +			if (options_.isSet(OptListControls))
> +				session->listControls();
> +			if (options_.isSet(OptListProperties))
> +				session->listProperties();
> +			if (options_.isSet(OptInfo))
> +				session->infoConfiguration();


Still suspecting that these could now be child options of the camera...

Unless they have to be done after the CameraSession is fully set up...


Anyway, ... I haven't tested any of this yet, I think a branch would be
nicer for that....


But so far so good:

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



>  		}
> -
> -		if (options_.isSet(OptListControls))
> -			session->listControls();
> -		if (options_.isSet(OptListProperties))
> -			session->listProperties();
> -		if (options_.isSet(OptInfo))
> -			session->infoConfiguration();
>  	}
>  
>  	/* 4. Start capture. */
> -	if (session && session->options().isSet(OptCapture)) {
> +	for (const auto &session : sessions) {
> +		if (!session->options().isSet(OptCapture))
> +			continue;
> +
>  		ret = session->start();
>  		if (ret) {
>  			std::cout << "Failed to start camera session" << std::endl;
> @@ -258,8 +265,12 @@ int CamApp::run()
>  		loop_.exec();
>  
>  	/* 6. Stop capture. */
> -	if (session && session->options().isSet(OptCapture))
> +	for (const auto &session : sessions) {
> +		if (!session->options().isSet(OptCapture))
> +			continue;
> +
>  		session->stop();
> +	}
>  
>  	return 0;
>  }
>