[libcamera-devel] [PATCH v2] cam: Add --monitor option

[Laurent Pinchart]

Hi Umang,

Thank you for the patch.

On Fri, Jul 31, 2020 at 07:46:54PM +0000, Umang Jain wrote:
> Add --monitor to monitor new hotplug and unplug camera events from
> the CameraManager.
> 
> Signed-off-by: Umang Jain <email at uajain.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/cam/main.cpp | 26 ++++++++++++++++++++++++++
>  src/cam/main.h   |  1 +
>  2 files changed, 27 insertions(+)
> 
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index f5aba04..b03437e 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -36,6 +36,8 @@ public:
>  	void quit();
>  
>  private:
> +	void cameraAdded(std::shared_ptr<Camera> cam);
> +	void cameraRemoved(std::shared_ptr<Camera> cam);
>  	int parseOptions(int argc, char *argv[]);
>  	int prepareConfig();
>  	int listControls();
> @@ -121,6 +123,10 @@ int CamApp::init(int argc, char **argv)
>  		ret = prepareConfig();
>  		if (ret)
>  			return ret;
> +	} else if (options_.isSet(OptMonitor)) {
> +		cm_->cameraAdded.connect(this, &CamApp::cameraAdded);
> +		cm_->cameraRemoved.connect(this, &CamApp::cameraRemoved);
> +		std::cout << "Monitoring new hotplug or unplug camera events…" << std::endl;
>  	}
>  
>  	loop_ = new EventLoop(cm_->eventDispatcher());
> @@ -189,6 +195,8 @@ int CamApp::parseOptions(int argc, char *argv[])
>  	parser.addOption(OptStrictFormats, OptionNone,
>  			 "Do not allow requested stream format(s) to be adjusted",
>  			 "strict-formats");
> +	parser.addOption(OptMonitor, OptionNone, "Monitor for hotplug and unplug camera events",

I'd move the message to the next line to avoid too long lines.

> +			 "monitor");
>  
>  	options_ = parser.parse(argc, argv);
>  	if (!options_.valid())
> @@ -309,6 +317,16 @@ int CamApp::infoConfiguration()
>  	return 0;
>  }
>  
> +void CamApp::cameraAdded(std::shared_ptr<Camera> cam)
> +{
> +	std::cout << "Camera Added: " << cam->name() << std::endl;
> +}
> +
> +void CamApp::cameraRemoved(std::shared_ptr<Camera> cam)
> +{
> +	std::cout << "Camera Removed: " << cam->name() << std::endl;
> +}
> +
>  int CamApp::run()
>  {
>  	int ret;
> @@ -342,10 +360,18 @@ int CamApp::run()
>  	}
>  
>  	if (options_.isSet(OptCapture)) {
> +		cm_->cameraAdded.connect(this, &CamApp::cameraAdded);
> +		cm_->cameraRemoved.connect(this, &CamApp::cameraRemoved);

This is done in CamApp::init() already, do we need it here ? One can
specific both --monitor and --capture if they want to monitor for
hotplug during capture, I wouldn't make it automatic.

>  		Capture capture(camera_, config_.get(), loop_);
>  		return capture.run(options_);
>  	}
>  
> +	if (options_.isSet(OptMonitor)) {
> +		ret = loop_->exec();
> +		if (ret)
> +			std::cout << "Failed to run monitor loop" << std::endl;
> +	}
> +
>  	return 0;
>  }
>  
> diff --git a/src/cam/main.h b/src/cam/main.h
> index 6f95add..ea8dfd3 100644
> --- a/src/cam/main.h
> +++ b/src/cam/main.h
> @@ -15,6 +15,7 @@ enum {
>  	OptInfo = 'I',
>  	OptList = 'l',
>  	OptListProperties = 'p',
> +	OptMonitor = 'm',
>  	OptStream = 's',
>  	OptListControls = 256,
>  	OptStrictFormats = 257,

-- 
Regards,

Laurent Pinchart