[libcamera-devel,v2,1/2] cam: Add Rectangle type parsing in capture script

16368 diff mbox series

This change is required for AfWindows control from capture script.
Parser expects array of arrays of parameters, so it is possible to
specify multiple rectangles.

Signed-off-by: Daniel Semkowicz <dse@thaumatec.com>
---
 src/cam/capture_script.cpp | 109 +++++++++++++++++++++++++++++++++----
 src/cam/capture_script.h   |   6 +-
 2 files changed, 102 insertions(+), 13 deletions(-)

Hi Daniel,

Thank you for the patch.

On Fri, Jun 24, 2022 at 03:05:22PM +0200, Daniel Semkowicz via libcamera-devel wrote:
> This change is required for AfWindows control from capture script.
> Parser expects array of arrays of parameters, so it is possible to
> specify multiple rectangles.
> 
> Signed-off-by: Daniel Semkowicz <dse@thaumatec.com>
> ---
>  src/cam/capture_script.cpp | 109 +++++++++++++++++++++++++++++++++----
>  src/cam/capture_script.h   |   6 +-
>  2 files changed, 102 insertions(+), 13 deletions(-)
> 
> diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp
> index 9f22d5f7..5812b122 100644
> --- a/src/cam/capture_script.cpp
> +++ b/src/cam/capture_script.cpp
> @@ -232,12 +232,15 @@ int CaptureScript::parseControl(EventPtr event, ControlList &controls)
>  		return -EINVAL;
>  	}
>  
> -	std::string value = parseScalar();
> -	if (value.empty())
> +	const ControlId *controlId = it->second;
> +
> +	ControlValue val = unpackControl(controlId);
> +	if (val.isNone()) {
> +		std::cerr << "Error unpacking control '" << name << "'"
> +			  << std::endl;
>  		return -EINVAL;
> +	}
>  
> -	const ControlId *controlId = it->second;
> -	ControlValue val = unpackControl(controlId, value);
>  	controls.set(controlId->id(), val);
>  
>  	return 0;
> @@ -252,6 +255,68 @@ std::string CaptureScript::parseScalar()
>  	return eventScalarValue(event);
>  }
>  
> +ControlValue CaptureScript::parseRectangles()
> +{
> +	EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);
> +	if (!event)
> +		return {};
> +
> +	std::vector<libcamera::Rectangle> rectangles;

Hmmm... we have an issue here. The code will work fine for AfWindows, as
it's an array of rectangles, but it will fail for controls are expressed
as a single rectangle, unless the YAML file stores them as an array
containing one rectangle.

Do you have an idea on how we could fix that ?

> +
> +	while (1) {
> +		event = nextEvent();
> +		if (!event)
> +			return {};
> +
> +		switch (event->type) {
> +		case YAML_SEQUENCE_START_EVENT: {
> +			std::vector<std::string> values = parseArray();
> +			if (values.size() != 4) {
> +				std::cerr << "Error parsing Rectangle: expected "
> +					     "array with 4 parameters" << std::endl;
> +				return {};
> +			}
> +
> +			Rectangle rect = unpackRectangle(values);
> +			rectangles.push_back(rect);
> +			break;
> +		}
> +		case YAML_SEQUENCE_END_EVENT: {
> +			ControlValue controlValue;
> +			controlValue.set(Span<const Rectangle>(rectangles));
> +			return controlValue;
> +		}
> +		default:
> +			return {};
> +		}
> +	}
> +}
> +
> +std::vector<std::string> CaptureScript::parseArray()
> +{
> +	std::vector<std::string> values;
> +
> +	while (1) {
> +		EventPtr event = nextEvent();
> +		if (!event)
> +			return {};
> +
> +		switch (event->type) {
> +		case YAML_SCALAR_EVENT: {
> +			std::string value = eventScalarValue(event);
> +			if (value.empty())
> +				return {};
> +			values.push_back(value);
> +			break;
> +		}
> +		case YAML_SEQUENCE_END_EVENT:
> +			return values;
> +		default:
> +			return {};
> +		}
> +	}
> +}
> +
>  void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)
>  {
>  	static const std::map<unsigned int, const char *> typeNames = {
> @@ -277,9 +342,24 @@ void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)
>  		  << typeName << " control " << id->name() << std::endl;
>  }
>  
> -ControlValue CaptureScript::unpackControl(const ControlId *id,
> -					  const std::string &repr)
> +ControlValue CaptureScript::unpackControl(const ControlId *id)
>  {
> +	/* Parse complex types */

Nitpicking, s/types/types./

> +	switch (id->type()) {
> +	case ControlTypeRectangle:
> +		return parseRectangles();
> +	case ControlTypeSize:
> +		/* \todo Parse Sizes. */
> +		return {};

Feel free to send another patch to address this too ;-)

> +	default:
> +		break;
> +	}
> +
> +	/* Parse basic types represented by a single scalar */

Same here, s/scalar/scalar./

> +	const std::string repr = parseScalar();
> +	if (repr.empty())
> +		return {};
> +
>  	ControlValue value{};
>  
>  	switch (id->type()) {
> @@ -324,13 +404,20 @@ ControlValue CaptureScript::unpackControl(const ControlId *id,
>  		value.set<std::string>(repr);
>  		break;
>  	}
> -	case ControlTypeRectangle:
> -		/* \todo Parse rectangles. */
> -		break;
> -	case ControlTypeSize:
> -		/* \todo Parse Sizes. */
> +	default:
> +		std::cerr << "Unsupported control type" << std::endl;
>  		break;
>  	}
>  
>  	return value;
>  }
> +
> +libcamera::Rectangle CaptureScript::unpackRectangle(const std::vector<std::string> &strVec)
> +{
> +	int x = strtol(strVec[0].c_str(), NULL, 10);
> +	int y = strtol(strVec[1].c_str(), NULL, 10);
> +	unsigned int width = strtoul(strVec[2].c_str(), NULL, 10);
> +	unsigned int height = strtoul(strVec[3].c_str(), NULL, 10);
> +
> +	return Rectangle(x, y, width, height);
> +}
> diff --git a/src/cam/capture_script.h b/src/cam/capture_script.h
> index 8b4f8f62..130872b4 100644
> --- a/src/cam/capture_script.h
> +++ b/src/cam/capture_script.h
> @@ -54,9 +54,11 @@ private:
>  	int parseControl(EventPtr event, libcamera::ControlList &controls);
>  
>  	std::string parseScalar();
> +	libcamera::ControlValue parseRectangles();
> +	std::vector<std::string> parseArray();
>  
>  	void unpackFailure(const libcamera::ControlId *id,
>  			   const std::string &repr);
> -	libcamera::ControlValue unpackControl(const libcamera::ControlId *id,
> -					      const std::string &repr);
> +	libcamera::ControlValue unpackControl(const libcamera::ControlId *id);
> +	libcamera::Rectangle unpackRectangle(const std::vector<std::string> &strVec);
>  };