[libcamera-devel,28/31] cam: Add option to list camera properties

2945 diff mbox series

From: Jacopo Mondi <jacopo@jmondi.org>

Add the '-p'|'--list-properties' option to the cam application to list
the properties of a camera.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/cam/main.cpp | 28 ++++++++++++++++++++++++++++
 src/cam/main.h   |  1 +
 2 files changed, 29 insertions(+)

On 29/02/2020 16:42, Laurent Pinchart wrote:
> From: Jacopo Mondi <jacopo@jmondi.org>
> 
> Add the '-p'|'--list-properties' option to the cam application to list
> the properties of a camera.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

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

> ---
>  src/cam/main.cpp | 28 ++++++++++++++++++++++++++++
>  src/cam/main.h   |  1 +
>  2 files changed, 29 insertions(+)
> 
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index a38cca959aca..ea6f7914839c 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -11,6 +11,7 @@
>  #include <string.h>
>  
>  #include <libcamera/libcamera.h>
> +#include <libcamera/property_ids.h>
>  
>  #include "capture.h"
>  #include "event_loop.h"
> @@ -36,6 +37,7 @@ public:
>  private:
>  	int parseOptions(int argc, char *argv[]);
>  	int prepareConfig();
> +	int listProperties();
>  	int infoConfiguration();
>  	int run();
>  
> @@ -180,6 +182,8 @@ int CamApp::parseOptions(int argc, char *argv[])
>  	parser.addOption(OptInfo, OptionNone,
>  			 "Display information about stream(s)", "info");
>  	parser.addOption(OptList, OptionNone, "List all cameras", "list");
> +	parser.addOption(OptProps, OptionNone, "List cameras properties",
> +			 "list-properties");
>  
>  	options_ = parser.parse(argc, argv);
>  	if (!options_.valid())
> @@ -268,6 +272,24 @@ int CamApp::prepareConfig()
>  	return 0;
>  }
>  
> +int CamApp::listProperties()
> +{
> +	if (!camera_) {
> +		std::cout << "Cannot list properties without a camera"
> +			  << std::endl;
> +		return -EINVAL;
> +	}

Hrm ...

I feel like this should be handled higher up in cam, and if a camera is
not selected, then the first one gets used.

And if one is *not available* then cam shouldn't be able to do anything?



> +
> +	for (const auto &prop : camera_->properties()) {
> +		const ControlId *id = properties::properties.at(prop.first);
> +		const ControlValue &value = prop.second;
> +
> +		std::cout << "Property: " << id->name() << " = " << value.toString();
> +	}
> +
> +	return 0;
> +}
> +
>  int CamApp::infoConfiguration()
>  {
>  	if (!config_) {
> @@ -312,6 +334,12 @@ int CamApp::run()
>  		}
>  	}
>  
> +	if (options_.isSet(OptProps)) {
> +		ret = listProperties();
> +		if (ret)
> +			return ret;
> +	}
> +
>  	if (options_.isSet(OptInfo)) {
>  		ret = infoConfiguration();
>  		if (ret)
> diff --git a/src/cam/main.h b/src/cam/main.h
> index 0997476bb335..afcad4353b7d 100644
> --- a/src/cam/main.h
> +++ b/src/cam/main.h
> @@ -14,6 +14,7 @@ enum {
>  	OptHelp = 'h',
>  	OptInfo = 'I',
>  	OptList = 'l',
> +	OptProps = 'p',
>  	OptStream = 's',
>  };
>  
>

Hi Kieran,

On Thu, Mar 05, 2020 at 05:32:40PM +0000, Kieran Bingham wrote:
> On 29/02/2020 16:42, Laurent Pinchart wrote:
> > From: Jacopo Mondi <jacopo@jmondi.org>
> > 
> > Add the '-p'|'--list-properties' option to the cam application to list
> > the properties of a camera.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > ---
> >  src/cam/main.cpp | 28 ++++++++++++++++++++++++++++
> >  src/cam/main.h   |  1 +
> >  2 files changed, 29 insertions(+)
> > 
> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> > index a38cca959aca..ea6f7914839c 100644
> > --- a/src/cam/main.cpp
> > +++ b/src/cam/main.cpp
> > @@ -11,6 +11,7 @@
> >  #include <string.h>
> >  
> >  #include <libcamera/libcamera.h>
> > +#include <libcamera/property_ids.h>
> >  
> >  #include "capture.h"
> >  #include "event_loop.h"
> > @@ -36,6 +37,7 @@ public:
> >  private:
> >  	int parseOptions(int argc, char *argv[]);
> >  	int prepareConfig();
> > +	int listProperties();
> >  	int infoConfiguration();
> >  	int run();
> >  
> > @@ -180,6 +182,8 @@ int CamApp::parseOptions(int argc, char *argv[])
> >  	parser.addOption(OptInfo, OptionNone,
> >  			 "Display information about stream(s)", "info");
> >  	parser.addOption(OptList, OptionNone, "List all cameras", "list");
> > +	parser.addOption(OptProps, OptionNone, "List cameras properties",
> > +			 "list-properties");
> >  
> >  	options_ = parser.parse(argc, argv);
> >  	if (!options_.valid())
> > @@ -268,6 +272,24 @@ int CamApp::prepareConfig()
> >  	return 0;
> >  }
> >  
> > +int CamApp::listProperties()
> > +{
> > +	if (!camera_) {
> > +		std::cout << "Cannot list properties without a camera"
> > +			  << std::endl;
> > +		return -EINVAL;
> > +	}
> 
> Hrm ...
> 
> I feel like this should be handled higher up in cam, and if a camera is
> not selected, then the first one gets used.
> 
> And if one is *not available* then cam shouldn't be able to do anything?

There's at least one option that doesn't need to specify a camera,
that's -l. I agree we should rework the code, the infoConfiguration()
function is implemented with similar logic. I thus propose doing so on
top of this patch.

> > +
> > +	for (const auto &prop : camera_->properties()) {
> > +		const ControlId *id = properties::properties.at(prop.first);
> > +		const ControlValue &value = prop.second;
> > +
> > +		std::cout << "Property: " << id->name() << " = " << value.toString();
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  int CamApp::infoConfiguration()
> >  {
> >  	if (!config_) {
> > @@ -312,6 +334,12 @@ int CamApp::run()
> >  		}
> >  	}
> >  
> > +	if (options_.isSet(OptProps)) {
> > +		ret = listProperties();
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> >  	if (options_.isSet(OptInfo)) {
> >  		ret = infoConfiguration();
> >  		if (ret)
> > diff --git a/src/cam/main.h b/src/cam/main.h
> > index 0997476bb335..afcad4353b7d 100644
> > --- a/src/cam/main.h
> > +++ b/src/cam/main.h
> > @@ -14,6 +14,7 @@ enum {
> >  	OptHelp = 'h',
> >  	OptInfo = 'I',
> >  	OptList = 'l',
> > +	OptProps = 'p',
> >  	OptStream = 's',
> >  };
> >