[libcamera-devel] [RFC 4/4] cam: Allow specifying configuration for more then one stream

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Mar 22, 2019 at 02:53:49AM +0100, Niklas Söderlund wrote:
> As a step to allow multiple streams extend the cam tool to be able to
> configure more then one stream. At this point libcamera do not allow an

s/do not/does not/

> application to associate a stream with a intended use-case so instead
> add a 'id' field to the --format parser.
> 
> The numerical id represent the order in the array of streams which are
> handed to the application from the library. This should change once the
> library learns stream use-case. This implementation is a small and
> incomplete step in the direction of extending the cam utility to support
> cameras with more then one stream.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/cam/main.cpp | 39 ++++++++++++++++++++++++++++-----------
>  1 file changed, 28 insertions(+), 11 deletions(-)
> 
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index 1ca7862bf237d85f..c9239ac62a83ce31 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -42,6 +42,8 @@ void signalHandler(int signal)
>  static int parseOptions(int argc, char *argv[])
>  {
>  	KeyValueParser formatKeyValue;
> +	formatKeyValue.addOption("id", OptionInteger, "ID of stream",

"Stream ID" ?

> +				 ArgumentRequired);
>  	formatKeyValue.addOption("width", OptionInteger, "Width in pixels",
>  				 ArgumentRequired);
>  	formatKeyValue.addOption("height", OptionInteger, "Height in pixels",
> @@ -61,7 +63,7 @@ static int parseOptions(int argc, char *argv[])
>  			 "The default file name is 'frame-#.bin'.",
>  			 "file", ArgumentOptional, "filename");
>  	parser.addOption(OptFormat, &formatKeyValue,
> -			 "Set format of the camera's first stream", "format");
> +			 "Set format of the camera's first stream", "format", true);

You can break the line to avoid going over 80 columns.

>  	parser.addOption(OptHelp, OptionNone, "Display this help message",
>  			 "help");
>  	parser.addOption(OptList, OptionNone, "List all cameras", "list");
> @@ -77,22 +79,37 @@ static int parseOptions(int argc, char *argv[])
>  
>  static int configureStreams(Camera *camera, std::set<Stream *> &streams)
>  {
> -	KeyValueParser::Options format = options[OptFormat];
> -	Stream *id = *streams.begin();
> -
>  	std::map<Stream *, StreamConfiguration> config =
>  		camera->streamConfiguration(streams);
>  
>  	if (options.isSet(OptFormat)) {
> -		if (format.isSet("width"))
> -			config[id].width = format["width"];
> +		int num = 0;
> +		for (Stream *id : streams) {
> +			for (auto const &value : options[OptFormat].toArray()) {
> +				KeyValueParser::Options format = value.toKeyValues();
>  
> -		if (format.isSet("height"))
> -			config[id].height = format["height"];
> +				if (!format.isSet("id") || format["id"] != num)
> +					continue;

Shouldn't you accept streams with no ID ? You can pick the next
available stream in that case.

>  
> -		/* TODO: Translate 4CC string to ID. */
> -		if (format.isSet("pixelformat"))
> -			config[id].pixelFormat = format["pixelformat"];
> +				if (format.isSet("width"))
> +					config[id].width = format["width"];
> +
> +				if (format.isSet("height"))
> +					config[id].height = format["height"];
> +
> +				/* TODO: Translate 4CC string to ID. */
> +				if (format.isSet("pixelformat"))
> +					config[id].pixelFormat = format["pixelformat"];
> +			}
> +			num++;
> +		}
> +	}
> +
> +	for (auto const &it : config) {
> +		const StreamConfiguration &conf = it.second;
> +
> +		std::cout << "size: " << conf.width << "x" << conf.height
> +			  << "pixelformat: " << conf.pixelFormat << std::endl;
>  	}
>  
>  	return camera->configureStreams(config);

-- 
Regards,

Laurent Pinchart