[libcamera-devel] [PATCH 6/6] cam: add --format option to configure a stream

[Kieran Bingham]

Hi Niklas,

On 28/01/2019 00:41, Niklas Söderlund wrote:
> Add a option to configure the first stream of a camera from an argument
> with options and parse the width, height and pixel format from that
> list.
> 
> The pixel format is still specified as a integer which should correspond
> to the kernels FOURCC identifiers. Going forward this should be turned
> into a string representation and the cam parser should translate between
> the two.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/cam/main.cpp | 104 +++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 92 insertions(+), 12 deletions(-)
> 
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index bde47a8f17983912..4b4ce9aa29c80bd1 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -20,7 +20,8 @@ using namespace libcamera;
>  OptionsParser::Options options;
>  
>  enum {
> -	OptCamera = 'c',
> +	OptCamera = 'C',
> +	OptFormat = 'f',
>  	OptHelp = 'h',
>  	OptList = 'l',
>  };
> @@ -35,10 +36,20 @@ void signalHandler(int signal)
>  
>  static int parseOptions(int argc, char *argv[])
>  {
> +	KeyValueParser formatKeyValue;
> +	formatKeyValue.addOption("width", "Set width in pixels",
> +				 ArgumentRequired, "w");
> +	formatKeyValue.addOption("height", "Set height in pixels",
> +				 ArgumentRequired, "h");
> +	formatKeyValue.addOption("pixelformat", "Set pixel format",
> +				 ArgumentRequired, "pf");

Hrm - can a short-option have two letters? (if so, should it?)

> +
>  	OptionsParser parser;
>  
>  	parser.addOption(OptCamera, "Specify which camera to operate on",
>  			 "camera", ArgumentRequired, "camera");
> +	parser.addOption(OptFormat, "Set format of the cameras first stream",
> +			 formatKeyValue, "format");
>  	parser.addOption(OptHelp, "Display this help message", "help");
>  	parser.addOption(OptList, "List all cameras", "list");
>  
> @@ -54,6 +65,42 @@ static int parseOptions(int argc, char *argv[])
>  	return 0;
>  }
>  
> +int str2uint(std::string str, unsigned int *dest)
> +{
> +	if (!dest || str == "" || !sscanf(str.c_str(), "%d", dest))
> +		return -EINVAL;
> +	return 0;
> +}
> +
> +bool configureStreams(Camera *camera, std::vector<Stream> &streams)
> +{
> +	if (!options.isKeyValue(OptFormat))
> +		return false;
> +
> +	KeyValueParser::Options format = options.keyValues(OptFormat);
> +
> +	std::map<unsigned int, StreamConfiguration> config = camera->streamConfiguration(streams);
> +	unsigned int id = streams.front().id();
> +
> +	if (format.isSet("width"))
> +		if (str2uint(format["width"], &config[id].width))
> +			return false;
> +
> +	if (format.isSet("height"))
> +		if (str2uint(format["height"], &config[id].height))
> +			return false;
> +
> +	/* TODO: Translate 4CC string to ID. */
> +	if (format.isSet("pixelformat"))
> +		if (str2uint(format["pixelformat"], &config[id].pixelFormat))
> +			return false;
> +
> +	if (camera->configureStreams(config))
> +		return false;
> +
> +	return true;
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	int ret;
> @@ -63,6 +110,8 @@ int main(int argc, char **argv)
>  		return EXIT_FAILURE;
>  
>  	CameraManager *cm = CameraManager::instance();
> +	std::shared_ptr<Camera> camera;
> +	std::vector<Stream> streams;
>  
>  	ret = cm->start();
>  	if (ret) {
> @@ -71,31 +120,62 @@ int main(int argc, char **argv)
>  		return EXIT_FAILURE;
>  	}
>  
> +	loop = new EventLoop(cm->eventDispatcher());
> +
> +	struct sigaction sa = {};
> +	sa.sa_handler = &signalHandler;
> +	sigaction(SIGINT, &sa, nullptr);
> +
>  	if (options.isSet(OptList)) {
>  		std::cout << "Available cameras:" << std::endl;
> -		for (const std::shared_ptr<Camera> &camera : cm->cameras())
> -			std::cout << "- " << camera->name() << std::endl;
> +		for (const std::shared_ptr<Camera> &cam : cm->cameras())
> +			std::cout << "- " << cam->name() << std::endl;
>  	}
>  
>  	if (options.isSet(OptCamera)) {
> -		std::shared_ptr<Camera> cam = cm->get(options[OptCamera]);
> -
> -		if (cam) {
> -			std::cout << "Using camera " << cam->name() << std::endl;
> -		} else {
> +		camera = cm->get(options[OptCamera]);
> +		if (!camera) {
>  			std::cout << "Camera " << options[OptCamera]
>  				  << " not found" << std::endl;
> +			goto out;
> +		}
> +
> +		streams = camera->streams();
> +		if (streams.size() != 1) {
> +			std::cout << "Camera have " << streams.size()

Camera 'has' N streams...

> +				  << " streams, I only know how to work with 1"
> +				  << std::endl;
> +			goto out;
> +		}
> +
> +		if (camera->acquire()) {
> +			std::cout << "Failed to acquire camera" << std::endl;
> +			goto out;
>  		}
> +
> +		std::cout << "Using camera " << camera->name() << std::endl;
>  	}
>  
> -	loop = new EventLoop(cm->eventDispatcher());
> +	if (options.isSet(OptFormat)) {
> +		if (!camera) {
> +			std::cout << "Can't configure stream, no camera selected" << std::endl;
> +			goto out_camera;
> +		}
>  
> -	struct sigaction sa = {};
> -	sa.sa_handler = &signalHandler;
> -	sigaction(SIGINT, &sa, nullptr);
> +		if (!configureStreams(camera.get(), streams)) {
> +			std::cout << "Failed to configure camera" << std::endl;
> +			goto out_camera;
> +		}
> +	}
>  
>  	ret = loop->exec();
>  
> +out_camera:
> +	if (camera) {
> +		camera->release();
> +		camera.reset();

Uhm ... first a pointer call '->' then a direct member call ? '.' ?


> +	}
> +out:
>  	delete loop;
>  
>  	cm->stop();
> 

-- 
Regards
--
Kieran