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

[Laurent Pinchart]

Hi Kieran (and Umang),

Thank you for the patch.

On Mon, Aug 03, 2020 at 01:48:23PM +0100, Kieran Bingham wrote:
> From: Umang Jain <email at uajain.com>
> 
> 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 | 27 +++++++++++++++++++++++++++
>  src/cam/main.h   |  1 +
>  2 files changed, 28 insertions(+)
> 
> v3:
>   - V1 rebased, and slightly modified to update the conditional
>     connection of the signals during CamApp::init()
>   - Adjust formatting of parser.addOption
> 
> 
> Hi all,
> 
> I am aiming to merge Umang's patch, but because I have ended up making a
> more than superficial change to the code path in CamApp::init() I'm
> reposting as v3. This is essentially v1 of Umangs patch (thanks, and
> sorry for incorrectly leading you to make a v2) with the small line
> adjustment to the parser addition, but a more impacting adjustment to
> the conditional which enables the signals.
> 
> 
> If there are no objections, I can merge this patch.
> 
> Testing with the two options combined shows:
> 
> ./build/src/cam/cam -m -c 1 -C
> 
> fps: 25.64 stream0 seq: 000052 bytesused: 36000
> fps: 25.00 stream0 seq: 000053 bytesused: 36024
> fps: 25.00 stream0 seq: 000054 bytesused: 36784
> fps: 25.64 stream0 seq: 000055 bytesused: 36968
> fps: 25.64 stream0 seq: 000056 bytesused: 36792
> fps: 25.00 stream0 seq: 000057 bytesused: 37040
> fps: 25.64 stream0 seq: 000058 bytesused: 36808
> fps: 25.00 stream0 seq: 000059 bytesused: 36848
> fps: 25.00 stream0 seq: 000060 bytesused: 37480
> Camera Removed: VF0520 Live! Cam Sync: VF0520 L
> Camera Added: VF0520 Live! Cam Sync: VF0520 L
> Camera Removed: VF0520 Live! Cam Sync: VF0520 L
> 
> So the stream doesn't continue, but no crashing - so we're fine ;-)
> 
> 
> --
> Kieran
> 
> 
> 
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index f5aba041d7c4..3ceb6576960e 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();
> @@ -123,6 +125,12 @@ int CamApp::init(int argc, char **argv)
>  			return ret;
>  	}
>  
> +	if (options_.isSet(OptMonitor)) {
> +		cm_->cameraAdded.connect(this, &CamApp::cameraAdded);
> +		cm_->cameraRemoved.connect(this, &CamApp::cameraRemoved);
> +		std::cout << "Monitoring new hotplug and unplug events..." << std::endl;
> +	}
> +
>  	loop_ = new EventLoop(cm_->eventDispatcher());
>  
>  	return 0;
> @@ -186,6 +194,9 @@ int CamApp::parseOptions(int argc, char *argv[])
>  			 "list-controls");
>  	parser.addOption(OptListProperties, OptionNone, "List cameras properties",
>  			 "list-properties");
> +	parser.addOption(OptMonitor, OptionNone,
> +			 "Monitor for hotplug and unplug camera events",
> +			 "monitor");
>  	parser.addOption(OptStrictFormats, OptionNone,
>  			 "Do not allow requested stream format(s) to be adjusted",
>  			 "strict-formats");
> @@ -309,6 +320,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;
> @@ -346,6 +367,12 @@ int CamApp::run()
>  		return capture.run(options_);
>  	}
>  
> +	if (options_.isSet(OptMonitor)) {

I wonder if we should add here

		std::cout << "Press Ctrl-C to interrupt" << std::endl;

With or without that (or with a modified version),

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +		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 6f95add31a63..ea8dfd330830 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