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

Message ID	20220627122806.29586-2-dse@thaumatec.com
State	Accepted
Headers	show Return-Path: <libcamera-devel-bounces@lists.libcamera.org> To: libcamera-devel@lists.libcamera.org Date: Mon, 27 Jun 2022 14:28:05 +0200 Message-Id: <20220627122806.29586-2-dse@thaumatec.com> In-Reply-To: <20220627122806.29586-1-dse@thaumatec.com> References: <20220627122806.29586-1-dse@thaumatec.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type parsing in capture script Precedence: list From: Daniel Semkowicz via libcamera-devel <libcamera-devel@lists.libcamera.org> Reply-To: Daniel Semkowicz <dse@thaumatec.com> Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" <libcamera-devel-bounces@lists.libcamera.org>
Series	cam: Add Rectangle type parsing in capture script
Related	show [libcamera-devel,v3,0/2] cam: Add Rectangle type parsing in capture script [libcamera-devel,v3,1/2] cam: Add Rectangle type parsing in capture script [libcamera-devel,v3,2/2] cam: capture_script: Check parseFrames() return value for errors

Daniel Semkowicz June 27, 2022, 12:28 p.m. UTC

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 | 145 ++++++++++++++++++++++++++++++++++---
 src/cam/capture_script.h   |   7 +-
 2 files changed, 139 insertions(+), 13 deletions(-)

Jacopo Mondi June 27, 2022, 4:06 p.m. UTC | #1

Hi Daniel

On Mon, Jun 27, 2022 at 02:28:05PM +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 | 145 ++++++++++++++++++++++++++++++++++---
>  src/cam/capture_script.h   |   7 +-
>  2 files changed, 139 insertions(+), 13 deletions(-)
>
> diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp
> index 9f22d5f7..6278a152 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,104 @@ std::string CaptureScript::parseScalar()
>  	return eventScalarValue(event);
>  }
>
> +ControlValue CaptureScript::parseRectangles()
> +{
> +	std::vector<libcamera::Rectangle> rectangles;
> +
> +	std::vector<std::vector<std::string>> arrays = parseArrays();
> +	if (arrays.empty())
> +		return {};
> +
> +	for (const std::vector<std::string> &values : arrays) {
> +		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);
> +	}
> +
> +	ControlValue controlValue;
> +	controlValue.set(Span<const Rectangle>(rectangles));
> +
> +	return controlValue;
> +}
> +
> +std::vector<std::vector<std::string>> CaptureScript::parseArrays()
> +{
> +	EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);
> +	if (!event)
> +		return {};
> +
> +	event = nextEvent();
> +	if (!event)
> +		return {};
> +
> +	std::vector<std::vector<std::string>> valueArrays;
> +
> +	/* Parse single array. */
> +	if (event->type == YAML_SCALAR_EVENT) {
> +		std::string firstValue = eventScalarValue(event);
> +		if (firstValue.empty())
> +			return {};
> +
> +		std::vector<std::string> remaining = parseSingleArray();

nit: remaining can potentilly be {}

> +
> +		std::vector<std::string> values = { firstValue };
> +		values.insert(std::end(values),
> +			      std::begin(remaining), std::end(remaining));
> +		valueArrays.push_back(values);
> +
> +		return valueArrays;
> +	}

This should address Laurent's comment and I assume it now works with
both
        AfWindows:
          - [x, y, w, h]

and
        AfWindows:
          - [[x, y, w, h], [x1, y1, w1, h1] .. ]

> +
> +	/* Parse array of arrays. */
> +	while (1) {
> +		switch (event->type) {
> +		case YAML_SEQUENCE_START_EVENT: {
> +			std::vector<std::string> values = parseSingleArray();
> +			valueArrays.push_back(values);
> +			break;
> +		}
> +		case YAML_SEQUENCE_END_EVENT:
> +			return valueArrays;
> +		default:
> +			return {};
> +		}
> +
> +		event = nextEvent();
> +		if (!event)
> +			return {};
> +	}
> +}
> +
> +std::vector<std::string> CaptureScript::parseSingleArray()
> +{
> +	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 +378,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. */
> +	switch (id->type()) {
> +	case ControlTypeRectangle:
> +		return parseRectangles();
> +	case ControlTypeSize:
> +		/* \todo Parse Sizes. */
> +		return {};
> +	default:
> +		break;
> +	}
> +
> +	/* Parse basic types represented by a single scalar. */
> +	const std::string repr = parseScalar();
> +	if (repr.empty())
> +		return {};
> +

Thanks, this address my comment on v1.

Nits apart it looks good to me
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

>  	ControlValue value{};
>
>  	switch (id->type()) {
> @@ -324,13 +440,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..fffe67e5 100644
> --- a/src/cam/capture_script.h
> +++ b/src/cam/capture_script.h
> @@ -54,9 +54,12 @@ private:
>  	int parseControl(EventPtr event, libcamera::ControlList &controls);
>
>  	std::string parseScalar();
> +	libcamera::ControlValue parseRectangles();
> +	std::vector<std::vector<std::string>> parseArrays();
> +	std::vector<std::string> parseSingleArray();
>
>  	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);
>  };
> --
> 2.34.1
>

Daniel Semkowicz June 28, 2022, 6:35 a.m. UTC | #2

Hi Jacopo,

On Mon, Jun 27, 2022 at 6:06 PM Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> Hi Daniel
>
> On Mon, Jun 27, 2022 at 02:28:05PM +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 | 145 ++++++++++++++++++++++++++++++++++---
> >  src/cam/capture_script.h   |   7 +-
> >  2 files changed, 139 insertions(+), 13 deletions(-)
> >
> > diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp
> > index 9f22d5f7..6278a152 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,104 @@ std::string CaptureScript::parseScalar()
> >       return eventScalarValue(event);
> >  }
> >
> > +ControlValue CaptureScript::parseRectangles()
> > +{
> > +     std::vector<libcamera::Rectangle> rectangles;
> > +
> > +     std::vector<std::vector<std::string>> arrays = parseArrays();
> > +     if (arrays.empty())
> > +             return {};
> > +
> > +     for (const std::vector<std::string> &values : arrays) {
> > +             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);
> > +     }
> > +
> > +     ControlValue controlValue;
> > +     controlValue.set(Span<const Rectangle>(rectangles));
> > +
> > +     return controlValue;
> > +}
> > +
> > +std::vector<std::vector<std::string>> CaptureScript::parseArrays()
> > +{
> > +     EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);
> > +     if (!event)
> > +             return {};
> > +
> > +     event = nextEvent();
> > +     if (!event)
> > +             return {};
> > +
> > +     std::vector<std::vector<std::string>> valueArrays;
> > +
> > +     /* Parse single array. */
> > +     if (event->type == YAML_SCALAR_EVENT) {
> > +             std::string firstValue = eventScalarValue(event);
> > +             if (firstValue.empty())
> > +                     return {};
> > +
> > +             std::vector<std::string> remaining = parseSingleArray();
>
> nit: remaining can potentilly be {}

Yes, but the vector can be empty not only because of error, but also
because there was a valid array but with only one element (already read
in firstValue).
We would need to add additional parameter returned from method to
distinguish between these two situations.
The higher level that calls parseArrays() should already know the
expected number of elements in the single array, so the error will be
caught anyway. I wouldn't spend too much time tweaking this solution as
it is not critical.
What do you think about leaving it as it is?

>
> > +
> > +             std::vector<std::string> values = { firstValue };
> > +             values.insert(std::end(values),
> > +                           std::begin(remaining), std::end(remaining));
> > +             valueArrays.push_back(values);
> > +
> > +             return valueArrays;
> > +     }
>
> This should address Laurent's comment and I assume it now works with
> both
>         AfWindows:
>           - [x, y, w, h]
>
> and
>         AfWindows:
>           - [[x, y, w, h], [x1, y1, w1, h1] .. ]
>

Exactly, I tested it with both versions :)

Best regards
Daniel

> > +
> > +     /* Parse array of arrays. */
> > +     while (1) {
> > +             switch (event->type) {
> > +             case YAML_SEQUENCE_START_EVENT: {
> > +                     std::vector<std::string> values = parseSingleArray();
> > +                     valueArrays.push_back(values);
> > +                     break;
> > +             }
> > +             case YAML_SEQUENCE_END_EVENT:
> > +                     return valueArrays;
> > +             default:
> > +                     return {};
> > +             }
> > +
> > +             event = nextEvent();
> > +             if (!event)
> > +                     return {};
> > +     }
> > +}
> > +
> > +std::vector<std::string> CaptureScript::parseSingleArray()
> > +{
> > +     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 +378,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. */
> > +     switch (id->type()) {
> > +     case ControlTypeRectangle:
> > +             return parseRectangles();
> > +     case ControlTypeSize:
> > +             /* \todo Parse Sizes. */
> > +             return {};
> > +     default:
> > +             break;
> > +     }
> > +
> > +     /* Parse basic types represented by a single scalar. */
> > +     const std::string repr = parseScalar();
> > +     if (repr.empty())
> > +             return {};
> > +
>
> Thanks, this address my comment on v1.
>
> Nits apart it looks good to me
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Thanks
>    j
>
> >       ControlValue value{};
> >
> >       switch (id->type()) {
> > @@ -324,13 +440,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..fffe67e5 100644
> > --- a/src/cam/capture_script.h
> > +++ b/src/cam/capture_script.h
> > @@ -54,9 +54,12 @@ private:
> >       int parseControl(EventPtr event, libcamera::ControlList &controls);
> >
> >       std::string parseScalar();
> > +     libcamera::ControlValue parseRectangles();
> > +     std::vector<std::vector<std::string>> parseArrays();
> > +     std::vector<std::string> parseSingleArray();
> >
> >       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);
> >  };
> > --
> > 2.34.1
> >

Nicolas Dufresne via libcamera-devel Aug. 4, 2022, 7:47 p.m. UTC | #3

Hi Daniel,

Thanks for the patches.

On Tue, Jun 28, 2022 at 08:35:03AM +0200, Daniel Semkowicz via libcamera-devel wrote:
> Hi Jacopo,
> 
> On Mon, Jun 27, 2022 at 6:06 PM Jacopo Mondi <jacopo@jmondi.org> wrote:
> >
> > Hi Daniel
> >
> > On Mon, Jun 27, 2022 at 02:28:05PM +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 | 145 ++++++++++++++++++++++++++++++++++---
> > >  src/cam/capture_script.h   |   7 +-
> > >  2 files changed, 139 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp
> > > index 9f22d5f7..6278a152 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,104 @@ std::string CaptureScript::parseScalar()
> > >       return eventScalarValue(event);
> > >  }
> > >
> > > +ControlValue CaptureScript::parseRectangles()
> > > +{
> > > +     std::vector<libcamera::Rectangle> rectangles;
> > > +
> > > +     std::vector<std::vector<std::string>> arrays = parseArrays();
> > > +     if (arrays.empty())
> > > +             return {};
> > > +
> > > +     for (const std::vector<std::string> &values : arrays) {
> > > +             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);
> > > +     }
> > > +
> > > +     ControlValue controlValue;
> > > +     controlValue.set(Span<const Rectangle>(rectangles));
> > > +
> > > +     return controlValue;
> > > +}
> > > +
> > > +std::vector<std::vector<std::string>> CaptureScript::parseArrays()
> > > +{
> > > +     EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);
> > > +     if (!event)
> > > +             return {};
> > > +
> > > +     event = nextEvent();
> > > +     if (!event)
> > > +             return {};
> > > +
> > > +     std::vector<std::vector<std::string>> valueArrays;
> > > +
> > > +     /* Parse single array. */
> > > +     if (event->type == YAML_SCALAR_EVENT) {
> > > +             std::string firstValue = eventScalarValue(event);
> > > +             if (firstValue.empty())
> > > +                     return {};
> > > +
> > > +             std::vector<std::string> remaining = parseSingleArray();
> >
> > nit: remaining can potentilly be {}
> 
> Yes, but the vector can be empty not only because of error, but also
> because there was a valid array but with only one element (already read
> in firstValue).
> We would need to add additional parameter returned from method to
> distinguish between these two situations.
> The higher level that calls parseArrays() should already know the
> expected number of elements in the single array, so the error will be
> caught anyway. I wouldn't spend too much time tweaking this solution as
> it is not critical.
> What do you think about leaving it as it is?
> 
> >
> > > +
> > > +             std::vector<std::string> values = { firstValue };
> > > +             values.insert(std::end(values),
> > > +                           std::begin(remaining), std::end(remaining));
> > > +             valueArrays.push_back(values);
> > > +
> > > +             return valueArrays;
> > > +     }
> >
> > This should address Laurent's comment and I assume it now works with
> > both
> >         AfWindows:
> >           - [x, y, w, h]

Gotta be honest, I'm not a big fan of representing Rectangles this way;
it's a bit... flat (if that's the right word).
I'd prefer it to be like (x,y)/wxh


Thanks,

Paul

> >
> > and
> >         AfWindows:
> >           - [[x, y, w, h], [x1, y1, w1, h1] .. ]
> >
> 
> Exactly, I tested it with both versions :)
> 
> Best regards
> Daniel
> 
> > > +
> > > +     /* Parse array of arrays. */
> > > +     while (1) {
> > > +             switch (event->type) {
> > > +             case YAML_SEQUENCE_START_EVENT: {
> > > +                     std::vector<std::string> values = parseSingleArray();
> > > +                     valueArrays.push_back(values);
> > > +                     break;
> > > +             }
> > > +             case YAML_SEQUENCE_END_EVENT:
> > > +                     return valueArrays;
> > > +             default:
> > > +                     return {};
> > > +             }
> > > +
> > > +             event = nextEvent();
> > > +             if (!event)
> > > +                     return {};
> > > +     }
> > > +}
> > > +
> > > +std::vector<std::string> CaptureScript::parseSingleArray()
> > > +{
> > > +     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 +378,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. */
> > > +     switch (id->type()) {
> > > +     case ControlTypeRectangle:
> > > +             return parseRectangles();
> > > +     case ControlTypeSize:
> > > +             /* \todo Parse Sizes. */
> > > +             return {};
> > > +     default:
> > > +             break;
> > > +     }
> > > +
> > > +     /* Parse basic types represented by a single scalar. */
> > > +     const std::string repr = parseScalar();
> > > +     if (repr.empty())
> > > +             return {};
> > > +
> >
> > Thanks, this address my comment on v1.
> >
> > Nits apart it looks good to me
> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> >
> > Thanks
> >    j
> >
> > >       ControlValue value{};
> > >
> > >       switch (id->type()) {
> > > @@ -324,13 +440,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..fffe67e5 100644
> > > --- a/src/cam/capture_script.h
> > > +++ b/src/cam/capture_script.h
> > > @@ -54,9 +54,12 @@ private:
> > >       int parseControl(EventPtr event, libcamera::ControlList &controls);
> > >
> > >       std::string parseScalar();
> > > +     libcamera::ControlValue parseRectangles();
> > > +     std::vector<std::vector<std::string>> parseArrays();
> > > +     std::vector<std::string> parseSingleArray();
> > >
> > >       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);
> > >  };
> > > --
> > > 2.34.1
> > >

Jacopo Mondi Aug. 5, 2022, 7:20 a.m. UTC | #4

Hi Paul

On Fri, Aug 05, 2022 at 04:47:46AM +0900, paul.elder@ideasonboard.com wrote:
> Hi Daniel,
>
> Thanks for the patches.
>
> On Tue, Jun 28, 2022 at 08:35:03AM +0200, Daniel Semkowicz via libcamera-devel wrote:
> > Hi Jacopo,
> >
> > On Mon, Jun 27, 2022 at 6:06 PM Jacopo Mondi <jacopo@jmondi.org> wrote:
> > >
> > > Hi Daniel
> > >
> > > On Mon, Jun 27, 2022 at 02:28:05PM +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 | 145 ++++++++++++++++++++++++++++++++++---
> > > >  src/cam/capture_script.h   |   7 +-
> > > >  2 files changed, 139 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp
> > > > index 9f22d5f7..6278a152 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,104 @@ std::string CaptureScript::parseScalar()
> > > >       return eventScalarValue(event);
> > > >  }
> > > >
> > > > +ControlValue CaptureScript::parseRectangles()
> > > > +{
> > > > +     std::vector<libcamera::Rectangle> rectangles;
> > > > +
> > > > +     std::vector<std::vector<std::string>> arrays = parseArrays();
> > > > +     if (arrays.empty())
> > > > +             return {};
> > > > +
> > > > +     for (const std::vector<std::string> &values : arrays) {
> > > > +             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);
> > > > +     }
> > > > +
> > > > +     ControlValue controlValue;
> > > > +     controlValue.set(Span<const Rectangle>(rectangles));
> > > > +
> > > > +     return controlValue;
> > > > +}
> > > > +
> > > > +std::vector<std::vector<std::string>> CaptureScript::parseArrays()
> > > > +{
> > > > +     EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);
> > > > +     if (!event)
> > > > +             return {};
> > > > +
> > > > +     event = nextEvent();
> > > > +     if (!event)
> > > > +             return {};
> > > > +
> > > > +     std::vector<std::vector<std::string>> valueArrays;
> > > > +
> > > > +     /* Parse single array. */
> > > > +     if (event->type == YAML_SCALAR_EVENT) {
> > > > +             std::string firstValue = eventScalarValue(event);
> > > > +             if (firstValue.empty())
> > > > +                     return {};
> > > > +
> > > > +             std::vector<std::string> remaining = parseSingleArray();
> > >
> > > nit: remaining can potentilly be {}
> >
> > Yes, but the vector can be empty not only because of error, but also
> > because there was a valid array but with only one element (already read
> > in firstValue).
> > We would need to add additional parameter returned from method to
> > distinguish between these two situations.
> > The higher level that calls parseArrays() should already know the
> > expected number of elements in the single array, so the error will be
> > caught anyway. I wouldn't spend too much time tweaking this solution as
> > it is not critical.
> > What do you think about leaving it as it is?
> >
> > >
> > > > +
> > > > +             std::vector<std::string> values = { firstValue };
> > > > +             values.insert(std::end(values),
> > > > +                           std::begin(remaining), std::end(remaining));
> > > > +             valueArrays.push_back(values);
> > > > +
> > > > +             return valueArrays;
> > > > +     }
> > >
> > > This should address Laurent's comment and I assume it now works with
> > > both
> > >         AfWindows:
> > >           - [x, y, w, h]
>
> Gotta be honest, I'm not a big fan of representing Rectangles this way;
> it's a bit... flat (if that's the right word).
> I'd prefer it to be like (x,y)/wxh

This would require specifying the rectangle as a string. How will it
look like in the .yaml file ? Do you have examples you have tested
your version to share ?

>
>
> Thanks,
>
> Paul
>
> > >
> > > and
> > >         AfWindows:
> > >           - [[x, y, w, h], [x1, y1, w1, h1] .. ]
> > >
> >
> > Exactly, I tested it with both versions :)
> >
> > Best regards
> > Daniel
> >
> > > > +
> > > > +     /* Parse array of arrays. */
> > > > +     while (1) {
> > > > +             switch (event->type) {
> > > > +             case YAML_SEQUENCE_START_EVENT: {
> > > > +                     std::vector<std::string> values = parseSingleArray();
> > > > +                     valueArrays.push_back(values);
> > > > +                     break;
> > > > +             }
> > > > +             case YAML_SEQUENCE_END_EVENT:
> > > > +                     return valueArrays;
> > > > +             default:
> > > > +                     return {};
> > > > +             }
> > > > +
> > > > +             event = nextEvent();
> > > > +             if (!event)
> > > > +                     return {};
> > > > +     }
> > > > +}
> > > > +
> > > > +std::vector<std::string> CaptureScript::parseSingleArray()
> > > > +{
> > > > +     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 +378,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. */
> > > > +     switch (id->type()) {
> > > > +     case ControlTypeRectangle:
> > > > +             return parseRectangles();
> > > > +     case ControlTypeSize:
> > > > +             /* \todo Parse Sizes. */
> > > > +             return {};
> > > > +     default:
> > > > +             break;
> > > > +     }
> > > > +
> > > > +     /* Parse basic types represented by a single scalar. */
> > > > +     const std::string repr = parseScalar();
> > > > +     if (repr.empty())
> > > > +             return {};
> > > > +
> > >
> > > Thanks, this address my comment on v1.
> > >
> > > Nits apart it looks good to me
> > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> > >
> > > Thanks
> > >    j
> > >
> > > >       ControlValue value{};
> > > >
> > > >       switch (id->type()) {
> > > > @@ -324,13 +440,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..fffe67e5 100644
> > > > --- a/src/cam/capture_script.h
> > > > +++ b/src/cam/capture_script.h
> > > > @@ -54,9 +54,12 @@ private:
> > > >       int parseControl(EventPtr event, libcamera::ControlList &controls);
> > > >
> > > >       std::string parseScalar();
> > > > +     libcamera::ControlValue parseRectangles();
> > > > +     std::vector<std::vector<std::string>> parseArrays();
> > > > +     std::vector<std::string> parseSingleArray();
> > > >
> > > >       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);
> > > >  };
> > > > --
> > > > 2.34.1
> > > >

Nicolas Dufresne via libcamera-devel Aug. 8, 2022, 6:56 a.m. UTC | #5

Hi Jacopo,

On Fri, Aug 05, 2022 at 09:20:26AM +0200, Jacopo Mondi wrote:
> Hi Paul
> 
> On Fri, Aug 05, 2022 at 04:47:46AM +0900, paul.elder@ideasonboard.com wrote:
> > Hi Daniel,
> >
> > Thanks for the patches.
> >
> > On Tue, Jun 28, 2022 at 08:35:03AM +0200, Daniel Semkowicz via libcamera-devel wrote:
> > > Hi Jacopo,
> > >
> > > On Mon, Jun 27, 2022 at 6:06 PM Jacopo Mondi <jacopo@jmondi.org> wrote:
> > > >
> > > > Hi Daniel
> > > >
> > > > On Mon, Jun 27, 2022 at 02:28:05PM +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 | 145 ++++++++++++++++++++++++++++++++++---
> > > > >  src/cam/capture_script.h   |   7 +-
> > > > >  2 files changed, 139 insertions(+), 13 deletions(-)
> > > > >
> > > > > diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp
> > > > > index 9f22d5f7..6278a152 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,104 @@ std::string CaptureScript::parseScalar()
> > > > >       return eventScalarValue(event);
> > > > >  }
> > > > >
> > > > > +ControlValue CaptureScript::parseRectangles()
> > > > > +{
> > > > > +     std::vector<libcamera::Rectangle> rectangles;
> > > > > +
> > > > > +     std::vector<std::vector<std::string>> arrays = parseArrays();
> > > > > +     if (arrays.empty())
> > > > > +             return {};
> > > > > +
> > > > > +     for (const std::vector<std::string> &values : arrays) {
> > > > > +             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);
> > > > > +     }
> > > > > +
> > > > > +     ControlValue controlValue;
> > > > > +     controlValue.set(Span<const Rectangle>(rectangles));
> > > > > +
> > > > > +     return controlValue;
> > > > > +}
> > > > > +
> > > > > +std::vector<std::vector<std::string>> CaptureScript::parseArrays()
> > > > > +{
> > > > > +     EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);
> > > > > +     if (!event)
> > > > > +             return {};
> > > > > +
> > > > > +     event = nextEvent();
> > > > > +     if (!event)
> > > > > +             return {};
> > > > > +
> > > > > +     std::vector<std::vector<std::string>> valueArrays;
> > > > > +
> > > > > +     /* Parse single array. */
> > > > > +     if (event->type == YAML_SCALAR_EVENT) {
> > > > > +             std::string firstValue = eventScalarValue(event);
> > > > > +             if (firstValue.empty())
> > > > > +                     return {};
> > > > > +
> > > > > +             std::vector<std::string> remaining = parseSingleArray();
> > > >
> > > > nit: remaining can potentilly be {}
> > >
> > > Yes, but the vector can be empty not only because of error, but also
> > > because there was a valid array but with only one element (already read
> > > in firstValue).
> > > We would need to add additional parameter returned from method to
> > > distinguish between these two situations.
> > > The higher level that calls parseArrays() should already know the
> > > expected number of elements in the single array, so the error will be
> > > caught anyway. I wouldn't spend too much time tweaking this solution as
> > > it is not critical.
> > > What do you think about leaving it as it is?
> > >
> > > >
> > > > > +
> > > > > +             std::vector<std::string> values = { firstValue };
> > > > > +             values.insert(std::end(values),
> > > > > +                           std::begin(remaining), std::end(remaining));
> > > > > +             valueArrays.push_back(values);
> > > > > +
> > > > > +             return valueArrays;
> > > > > +     }
> > > >
> > > > This should address Laurent's comment and I assume it now works with
> > > > both
> > > >         AfWindows:
> > > >           - [x, y, w, h]
> >
> > Gotta be honest, I'm not a big fan of representing Rectangles this way;
> > it's a bit... flat (if that's the right word).
> > I'd prefer it to be like (x,y)/wxh
> 
> This would require specifying the rectangle as a string. How will it
> look like in the .yaml file ? Do you have examples you have tested
> your version to share ?

Oh right I forgot we've got a yaml spec to conform to :S

I just have it in there as-is:

frames:
  - 1:
      ScalerCrop: (0,0)/1920x1080

I suppose yaml doesn't have a "rectangle" type.

So if it's turning that into a string vs into an array... yeah I guess
an array of four ints is better.

Alright, nvm my objection then :)


Paul

> 
> >
> >
> > > >
> > > > and
> > > >         AfWindows:
> > > >           - [[x, y, w, h], [x1, y1, w1, h1] .. ]
> > > >
> > >
> > > Exactly, I tested it with both versions :)
> > >
> > > Best regards
> > > Daniel
> > >
> > > > > +
> > > > > +     /* Parse array of arrays. */
> > > > > +     while (1) {
> > > > > +             switch (event->type) {
> > > > > +             case YAML_SEQUENCE_START_EVENT: {
> > > > > +                     std::vector<std::string> values = parseSingleArray();
> > > > > +                     valueArrays.push_back(values);
> > > > > +                     break;
> > > > > +             }
> > > > > +             case YAML_SEQUENCE_END_EVENT:
> > > > > +                     return valueArrays;
> > > > > +             default:
> > > > > +                     return {};
> > > > > +             }
> > > > > +
> > > > > +             event = nextEvent();
> > > > > +             if (!event)
> > > > > +                     return {};
> > > > > +     }
> > > > > +}
> > > > > +
> > > > > +std::vector<std::string> CaptureScript::parseSingleArray()
> > > > > +{
> > > > > +     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 +378,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. */
> > > > > +     switch (id->type()) {
> > > > > +     case ControlTypeRectangle:
> > > > > +             return parseRectangles();
> > > > > +     case ControlTypeSize:
> > > > > +             /* \todo Parse Sizes. */
> > > > > +             return {};
> > > > > +     default:
> > > > > +             break;
> > > > > +     }
> > > > > +
> > > > > +     /* Parse basic types represented by a single scalar. */
> > > > > +     const std::string repr = parseScalar();
> > > > > +     if (repr.empty())
> > > > > +             return {};
> > > > > +
> > > >
> > > > Thanks, this address my comment on v1.
> > > >
> > > > Nits apart it looks good to me
> > > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> > > >
> > > > Thanks
> > > >    j
> > > >
> > > > >       ControlValue value{};
> > > > >
> > > > >       switch (id->type()) {
> > > > > @@ -324,13 +440,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..fffe67e5 100644
> > > > > --- a/src/cam/capture_script.h
> > > > > +++ b/src/cam/capture_script.h
> > > > > @@ -54,9 +54,12 @@ private:
> > > > >       int parseControl(EventPtr event, libcamera::ControlList &controls);
> > > > >
> > > > >       std::string parseScalar();
> > > > > +     libcamera::ControlValue parseRectangles();
> > > > > +     std::vector<std::vector<std::string>> parseArrays();
> > > > > +     std::vector<std::string> parseSingleArray();
> > > > >
> > > > >       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);
> > > > >  };
> > > > > --
> > > > > 2.34.1
> > > > >

Jacopo Mondi Aug. 8, 2022, 8:49 a.m. UTC | #6

Hi Paul,

On Mon, Aug 08, 2022 at 03:56:05PM +0900, paul.elder@ideasonboard.com wrote:
> Hi Jacopo,
>
> On Fri, Aug 05, 2022 at 09:20:26AM +0200, Jacopo Mondi wrote:
> > Hi Paul
> >
> > On Fri, Aug 05, 2022 at 04:47:46AM +0900, paul.elder@ideasonboard.com wrote:
> > > Hi Daniel,
> > >
> > > Thanks for the patches.
> > >
> > > On Tue, Jun 28, 2022 at 08:35:03AM +0200, Daniel Semkowicz via libcamera-devel wrote:
> > > > Hi Jacopo,
> > > >
> > > > On Mon, Jun 27, 2022 at 6:06 PM Jacopo Mondi <jacopo@jmondi.org> wrote:
> > > > >
> > > > > Hi Daniel
> > > > >
> > > > > On Mon, Jun 27, 2022 at 02:28:05PM +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 | 145 ++++++++++++++++++++++++++++++++++---
> > > > > >  src/cam/capture_script.h   |   7 +-
> > > > > >  2 files changed, 139 insertions(+), 13 deletions(-)
> > > > > >
> > > > > > diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp
> > > > > > index 9f22d5f7..6278a152 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,104 @@ std::string CaptureScript::parseScalar()
> > > > > >       return eventScalarValue(event);
> > > > > >  }
> > > > > >
> > > > > > +ControlValue CaptureScript::parseRectangles()
> > > > > > +{
> > > > > > +     std::vector<libcamera::Rectangle> rectangles;
> > > > > > +
> > > > > > +     std::vector<std::vector<std::string>> arrays = parseArrays();
> > > > > > +     if (arrays.empty())
> > > > > > +             return {};
> > > > > > +
> > > > > > +     for (const std::vector<std::string> &values : arrays) {
> > > > > > +             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);
> > > > > > +     }
> > > > > > +
> > > > > > +     ControlValue controlValue;
> > > > > > +     controlValue.set(Span<const Rectangle>(rectangles));
> > > > > > +
> > > > > > +     return controlValue;
> > > > > > +}
> > > > > > +
> > > > > > +std::vector<std::vector<std::string>> CaptureScript::parseArrays()
> > > > > > +{
> > > > > > +     EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);
> > > > > > +     if (!event)
> > > > > > +             return {};
> > > > > > +
> > > > > > +     event = nextEvent();
> > > > > > +     if (!event)
> > > > > > +             return {};
> > > > > > +
> > > > > > +     std::vector<std::vector<std::string>> valueArrays;
> > > > > > +
> > > > > > +     /* Parse single array. */
> > > > > > +     if (event->type == YAML_SCALAR_EVENT) {
> > > > > > +             std::string firstValue = eventScalarValue(event);
> > > > > > +             if (firstValue.empty())
> > > > > > +                     return {};
> > > > > > +
> > > > > > +             std::vector<std::string> remaining = parseSingleArray();
> > > > >
> > > > > nit: remaining can potentilly be {}
> > > >
> > > > Yes, but the vector can be empty not only because of error, but also
> > > > because there was a valid array but with only one element (already read
> > > > in firstValue).
> > > > We would need to add additional parameter returned from method to
> > > > distinguish between these two situations.
> > > > The higher level that calls parseArrays() should already know the
> > > > expected number of elements in the single array, so the error will be
> > > > caught anyway. I wouldn't spend too much time tweaking this solution as
> > > > it is not critical.
> > > > What do you think about leaving it as it is?
> > > >
> > > > >
> > > > > > +
> > > > > > +             std::vector<std::string> values = { firstValue };
> > > > > > +             values.insert(std::end(values),
> > > > > > +                           std::begin(remaining), std::end(remaining));
> > > > > > +             valueArrays.push_back(values);
> > > > > > +
> > > > > > +             return valueArrays;
> > > > > > +     }
> > > > >
> > > > > This should address Laurent's comment and I assume it now works with
> > > > > both
> > > > >         AfWindows:
> > > > >           - [x, y, w, h]
> > >
> > > Gotta be honest, I'm not a big fan of representing Rectangles this way;
> > > it's a bit... flat (if that's the right word).
> > > I'd prefer it to be like (x,y)/wxh
> >
> > This would require specifying the rectangle as a string. How will it
> > look like in the .yaml file ? Do you have examples you have tested
> > your version to share ?
>
> Oh right I forgot we've got a yaml spec to conform to :S
>
> I just have it in there as-is:
>
> frames:
>   - 1:
>       ScalerCrop: (0,0)/1920x1080
>
> I suppose yaml doesn't have a "rectangle" type.
>
> So if it's turning that into a string vs into an array... yeah I guess
> an array of four ints is better.

I would indeed prefer to avoid strings. From a correctness point of
view we won't lose much as your code parses the string and if we ever
would have a schema we can impose a format, but I feel it would be
harder to express multiple rectangles. We could consider more complex
mappings like:

         Rectangles:
           - x: 0
             y: 0
             w: 100
             h: 100
           - x: 120
             y: 120
             w: 80
             h: 80

which are maybe easier to validate, but as far as I can tell won't
give us anything more than a list of integers

>
> Alright, nvm my objection then :)

Care to send a tag ? :)
I guess we can collect that patch if it's fine with you!

Thanks
  j

>
>
> Paul
>
> >
> > >
> > >
> > > > >
> > > > > and
> > > > >         AfWindows:
> > > > >           - [[x, y, w, h], [x1, y1, w1, h1] .. ]
> > > > >
> > > >
> > > > Exactly, I tested it with both versions :)
> > > >
> > > > Best regards
> > > > Daniel
> > > >
> > > > > > +
> > > > > > +     /* Parse array of arrays. */
> > > > > > +     while (1) {
> > > > > > +             switch (event->type) {
> > > > > > +             case YAML_SEQUENCE_START_EVENT: {
> > > > > > +                     std::vector<std::string> values = parseSingleArray();
> > > > > > +                     valueArrays.push_back(values);
> > > > > > +                     break;
> > > > > > +             }
> > > > > > +             case YAML_SEQUENCE_END_EVENT:
> > > > > > +                     return valueArrays;
> > > > > > +             default:
> > > > > > +                     return {};
> > > > > > +             }
> > > > > > +
> > > > > > +             event = nextEvent();
> > > > > > +             if (!event)
> > > > > > +                     return {};
> > > > > > +     }
> > > > > > +}
> > > > > > +
> > > > > > +std::vector<std::string> CaptureScript::parseSingleArray()
> > > > > > +{
> > > > > > +     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 +378,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. */
> > > > > > +     switch (id->type()) {
> > > > > > +     case ControlTypeRectangle:
> > > > > > +             return parseRectangles();
> > > > > > +     case ControlTypeSize:
> > > > > > +             /* \todo Parse Sizes. */
> > > > > > +             return {};
> > > > > > +     default:
> > > > > > +             break;
> > > > > > +     }
> > > > > > +
> > > > > > +     /* Parse basic types represented by a single scalar. */
> > > > > > +     const std::string repr = parseScalar();
> > > > > > +     if (repr.empty())
> > > > > > +             return {};
> > > > > > +
> > > > >
> > > > > Thanks, this address my comment on v1.
> > > > >
> > > > > Nits apart it looks good to me
> > > > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> > > > >
> > > > > Thanks
> > > > >    j
> > > > >
> > > > > >       ControlValue value{};
> > > > > >
> > > > > >       switch (id->type()) {
> > > > > > @@ -324,13 +440,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..fffe67e5 100644
> > > > > > --- a/src/cam/capture_script.h
> > > > > > +++ b/src/cam/capture_script.h
> > > > > > @@ -54,9 +54,12 @@ private:
> > > > > >       int parseControl(EventPtr event, libcamera::ControlList &controls);
> > > > > >
> > > > > >       std::string parseScalar();
> > > > > > +     libcamera::ControlValue parseRectangles();
> > > > > > +     std::vector<std::vector<std::string>> parseArrays();
> > > > > > +     std::vector<std::string> parseSingleArray();
> > > > > >
> > > > > >       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);
> > > > > >  };
> > > > > > --
> > > > > > 2.34.1
> > > > > >

Nicolas Dufresne via libcamera-devel Aug. 8, 2022, 10:26 a.m. UTC | #7

Hi Daniel,

On Mon, Jun 27, 2022 at 02:28:05PM +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>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/cam/capture_script.cpp | 145 ++++++++++++++++++++++++++++++++++---
>  src/cam/capture_script.h   |   7 +-
>  2 files changed, 139 insertions(+), 13 deletions(-)
> 
> diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp
> index 9f22d5f7..6278a152 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,104 @@ std::string CaptureScript::parseScalar()
>  	return eventScalarValue(event);
>  }
>  
> +ControlValue CaptureScript::parseRectangles()
> +{
> +	std::vector<libcamera::Rectangle> rectangles;
> +
> +	std::vector<std::vector<std::string>> arrays = parseArrays();
> +	if (arrays.empty())
> +		return {};
> +
> +	for (const std::vector<std::string> &values : arrays) {
> +		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);
> +	}
> +
> +	ControlValue controlValue;
> +	controlValue.set(Span<const Rectangle>(rectangles));
> +
> +	return controlValue;
> +}
> +
> +std::vector<std::vector<std::string>> CaptureScript::parseArrays()
> +{
> +	EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);
> +	if (!event)
> +		return {};
> +
> +	event = nextEvent();
> +	if (!event)
> +		return {};
> +
> +	std::vector<std::vector<std::string>> valueArrays;
> +
> +	/* Parse single array. */
> +	if (event->type == YAML_SCALAR_EVENT) {
> +		std::string firstValue = eventScalarValue(event);
> +		if (firstValue.empty())
> +			return {};
> +
> +		std::vector<std::string> remaining = parseSingleArray();
> +
> +		std::vector<std::string> values = { firstValue };
> +		values.insert(std::end(values),
> +			      std::begin(remaining), std::end(remaining));
> +		valueArrays.push_back(values);
> +
> +		return valueArrays;
> +	}
> +
> +	/* Parse array of arrays. */
> +	while (1) {
> +		switch (event->type) {
> +		case YAML_SEQUENCE_START_EVENT: {
> +			std::vector<std::string> values = parseSingleArray();
> +			valueArrays.push_back(values);
> +			break;
> +		}
> +		case YAML_SEQUENCE_END_EVENT:
> +			return valueArrays;
> +		default:
> +			return {};
> +		}
> +
> +		event = nextEvent();
> +		if (!event)
> +			return {};
> +	}
> +}
> +
> +std::vector<std::string> CaptureScript::parseSingleArray()
> +{
> +	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 +378,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. */
> +	switch (id->type()) {
> +	case ControlTypeRectangle:
> +		return parseRectangles();
> +	case ControlTypeSize:
> +		/* \todo Parse Sizes. */
> +		return {};
> +	default:
> +		break;
> +	}
> +
> +	/* Parse basic types represented by a single scalar. */
> +	const std::string repr = parseScalar();
> +	if (repr.empty())
> +		return {};
> +
>  	ControlValue value{};
>  
>  	switch (id->type()) {
> @@ -324,13 +440,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..fffe67e5 100644
> --- a/src/cam/capture_script.h
> +++ b/src/cam/capture_script.h
> @@ -54,9 +54,12 @@ private:
>  	int parseControl(EventPtr event, libcamera::ControlList &controls);
>  
>  	std::string parseScalar();
> +	libcamera::ControlValue parseRectangles();
> +	std::vector<std::vector<std::string>> parseArrays();
> +	std::vector<std::string> parseSingleArray();
>  
>  	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);
>  };
> -- 
> 2.34.1
>

Daniel Semkowicz Sept. 2, 2022, 8:02 a.m. UTC | #8

Hi,

Are there plans to merge it to master or additional changes are required?

Best regards
Daniel

On Mon, Aug 8, 2022 at 12:27 PM <paul.elder@ideasonboard.com> wrote:
>
> Hi Daniel,
>
> On Mon, Jun 27, 2022 at 02:28:05PM +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>
>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
>
> > ---
> >  src/cam/capture_script.cpp | 145 ++++++++++++++++++++++++++++++++++---
> >  src/cam/capture_script.h   |   7 +-
> >  2 files changed, 139 insertions(+), 13 deletions(-)
> >
> > diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp
> > index 9f22d5f7..6278a152 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,104 @@ std::string CaptureScript::parseScalar()
> >       return eventScalarValue(event);
> >  }
> >
> > +ControlValue CaptureScript::parseRectangles()
> > +{
> > +     std::vector<libcamera::Rectangle> rectangles;
> > +
> > +     std::vector<std::vector<std::string>> arrays = parseArrays();
> > +     if (arrays.empty())
> > +             return {};
> > +
> > +     for (const std::vector<std::string> &values : arrays) {
> > +             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);
> > +     }
> > +
> > +     ControlValue controlValue;
> > +     controlValue.set(Span<const Rectangle>(rectangles));
> > +
> > +     return controlValue;
> > +}
> > +
> > +std::vector<std::vector<std::string>> CaptureScript::parseArrays()
> > +{
> > +     EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);
> > +     if (!event)
> > +             return {};
> > +
> > +     event = nextEvent();
> > +     if (!event)
> > +             return {};
> > +
> > +     std::vector<std::vector<std::string>> valueArrays;
> > +
> > +     /* Parse single array. */
> > +     if (event->type == YAML_SCALAR_EVENT) {
> > +             std::string firstValue = eventScalarValue(event);
> > +             if (firstValue.empty())
> > +                     return {};
> > +
> > +             std::vector<std::string> remaining = parseSingleArray();
> > +
> > +             std::vector<std::string> values = { firstValue };
> > +             values.insert(std::end(values),
> > +                           std::begin(remaining), std::end(remaining));
> > +             valueArrays.push_back(values);
> > +
> > +             return valueArrays;
> > +     }
> > +
> > +     /* Parse array of arrays. */
> > +     while (1) {
> > +             switch (event->type) {
> > +             case YAML_SEQUENCE_START_EVENT: {
> > +                     std::vector<std::string> values = parseSingleArray();
> > +                     valueArrays.push_back(values);
> > +                     break;
> > +             }
> > +             case YAML_SEQUENCE_END_EVENT:
> > +                     return valueArrays;
> > +             default:
> > +                     return {};
> > +             }
> > +
> > +             event = nextEvent();
> > +             if (!event)
> > +                     return {};
> > +     }
> > +}
> > +
> > +std::vector<std::string> CaptureScript::parseSingleArray()
> > +{
> > +     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 +378,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. */
> > +     switch (id->type()) {
> > +     case ControlTypeRectangle:
> > +             return parseRectangles();
> > +     case ControlTypeSize:
> > +             /* \todo Parse Sizes. */
> > +             return {};
> > +     default:
> > +             break;
> > +     }
> > +
> > +     /* Parse basic types represented by a single scalar. */
> > +     const std::string repr = parseScalar();
> > +     if (repr.empty())
> > +             return {};
> > +
> >       ControlValue value{};
> >
> >       switch (id->type()) {
> > @@ -324,13 +440,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..fffe67e5 100644
> > --- a/src/cam/capture_script.h
> > +++ b/src/cam/capture_script.h
> > @@ -54,9 +54,12 @@ private:
> >       int parseControl(EventPtr event, libcamera::ControlList &controls);
> >
> >       std::string parseScalar();
> > +     libcamera::ControlValue parseRectangles();
> > +     std::vector<std::vector<std::string>> parseArrays();
> > +     std::vector<std::string> parseSingleArray();
> >
> >       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);
> >  };
> > --
> > 2.34.1
> >

Jacopo Mondi Sept. 2, 2022, 10:32 a.m. UTC | #9

You're right, the series is tagged and can be merged.

I'll take care of that and sorry for the delay :)

On Fri, Sep 02, 2022 at 10:02:40AM +0200, Daniel Semkowicz via libcamera-devel wrote:
> Hi,
>
> Are there plans to merge it to master or additional changes are required?
>
> Best regards
> Daniel
>
> On Mon, Aug 8, 2022 at 12:27 PM <paul.elder@ideasonboard.com> wrote:
> >
> > Hi Daniel,
> >
> > On Mon, Jun 27, 2022 at 02:28:05PM +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>
> >
> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> >
> > > ---
> > >  src/cam/capture_script.cpp | 145 ++++++++++++++++++++++++++++++++++---
> > >  src/cam/capture_script.h   |   7 +-
> > >  2 files changed, 139 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp
> > > index 9f22d5f7..6278a152 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,104 @@ std::string CaptureScript::parseScalar()
> > >       return eventScalarValue(event);
> > >  }
> > >
> > > +ControlValue CaptureScript::parseRectangles()
> > > +{
> > > +     std::vector<libcamera::Rectangle> rectangles;
> > > +
> > > +     std::vector<std::vector<std::string>> arrays = parseArrays();
> > > +     if (arrays.empty())
> > > +             return {};
> > > +
> > > +     for (const std::vector<std::string> &values : arrays) {
> > > +             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);
> > > +     }
> > > +
> > > +     ControlValue controlValue;
> > > +     controlValue.set(Span<const Rectangle>(rectangles));
> > > +
> > > +     return controlValue;
> > > +}
> > > +
> > > +std::vector<std::vector<std::string>> CaptureScript::parseArrays()
> > > +{
> > > +     EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);
> > > +     if (!event)
> > > +             return {};
> > > +
> > > +     event = nextEvent();
> > > +     if (!event)
> > > +             return {};
> > > +
> > > +     std::vector<std::vector<std::string>> valueArrays;
> > > +
> > > +     /* Parse single array. */
> > > +     if (event->type == YAML_SCALAR_EVENT) {
> > > +             std::string firstValue = eventScalarValue(event);
> > > +             if (firstValue.empty())
> > > +                     return {};
> > > +
> > > +             std::vector<std::string> remaining = parseSingleArray();
> > > +
> > > +             std::vector<std::string> values = { firstValue };
> > > +             values.insert(std::end(values),
> > > +                           std::begin(remaining), std::end(remaining));
> > > +             valueArrays.push_back(values);
> > > +
> > > +             return valueArrays;
> > > +     }
> > > +
> > > +     /* Parse array of arrays. */
> > > +     while (1) {
> > > +             switch (event->type) {
> > > +             case YAML_SEQUENCE_START_EVENT: {
> > > +                     std::vector<std::string> values = parseSingleArray();
> > > +                     valueArrays.push_back(values);
> > > +                     break;
> > > +             }
> > > +             case YAML_SEQUENCE_END_EVENT:
> > > +                     return valueArrays;
> > > +             default:
> > > +                     return {};
> > > +             }
> > > +
> > > +             event = nextEvent();
> > > +             if (!event)
> > > +                     return {};
> > > +     }
> > > +}
> > > +
> > > +std::vector<std::string> CaptureScript::parseSingleArray()
> > > +{
> > > +     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 +378,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. */
> > > +     switch (id->type()) {
> > > +     case ControlTypeRectangle:
> > > +             return parseRectangles();
> > > +     case ControlTypeSize:
> > > +             /* \todo Parse Sizes. */
> > > +             return {};
> > > +     default:
> > > +             break;
> > > +     }
> > > +
> > > +     /* Parse basic types represented by a single scalar. */
> > > +     const std::string repr = parseScalar();
> > > +     if (repr.empty())
> > > +             return {};
> > > +
> > >       ControlValue value{};
> > >
> > >       switch (id->type()) {
> > > @@ -324,13 +440,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..fffe67e5 100644
> > > --- a/src/cam/capture_script.h
> > > +++ b/src/cam/capture_script.h
> > > @@ -54,9 +54,12 @@ private:
> > >       int parseControl(EventPtr event, libcamera::ControlList &controls);
> > >
> > >       std::string parseScalar();
> > > +     libcamera::ControlValue parseRectangles();
> > > +     std::vector<std::vector<std::string>> parseArrays();
> > > +     std::vector<std::string> parseSingleArray();
> > >
> > >       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);
> > >  };
> > > --
> > > 2.34.1
> > >

Daniel Semkowicz Sept. 2, 2022, 12:47 p.m. UTC | #10

Thank you :)

On Fri, Sep 2, 2022 at 12:32 PM Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> You're right, the series is tagged and can be merged.
>
> I'll take care of that and sorry for the delay :)
>
> On Fri, Sep 02, 2022 at 10:02:40AM +0200, Daniel Semkowicz via libcamera-devel wrote:
> > Hi,
> >
> > Are there plans to merge it to master or additional changes are required?
> >
> > Best regards
> > Daniel
> >
> > On Mon, Aug 8, 2022 at 12:27 PM <paul.elder@ideasonboard.com> wrote:
> > >
> > > Hi Daniel,
> > >
> > > On Mon, Jun 27, 2022 at 02:28:05PM +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>
> > >
> > > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> > >
> > > > ---
> > > >  src/cam/capture_script.cpp | 145 ++++++++++++++++++++++++++++++++++---
> > > >  src/cam/capture_script.h   |   7 +-
> > > >  2 files changed, 139 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp
> > > > index 9f22d5f7..6278a152 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,104 @@ std::string CaptureScript::parseScalar()
> > > >       return eventScalarValue(event);
> > > >  }
> > > >
> > > > +ControlValue CaptureScript::parseRectangles()
> > > > +{
> > > > +     std::vector<libcamera::Rectangle> rectangles;
> > > > +
> > > > +     std::vector<std::vector<std::string>> arrays = parseArrays();
> > > > +     if (arrays.empty())
> > > > +             return {};
> > > > +
> > > > +     for (const std::vector<std::string> &values : arrays) {
> > > > +             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);
> > > > +     }
> > > > +
> > > > +     ControlValue controlValue;
> > > > +     controlValue.set(Span<const Rectangle>(rectangles));
> > > > +
> > > > +     return controlValue;
> > > > +}
> > > > +
> > > > +std::vector<std::vector<std::string>> CaptureScript::parseArrays()
> > > > +{
> > > > +     EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);
> > > > +     if (!event)
> > > > +             return {};
> > > > +
> > > > +     event = nextEvent();
> > > > +     if (!event)
> > > > +             return {};
> > > > +
> > > > +     std::vector<std::vector<std::string>> valueArrays;
> > > > +
> > > > +     /* Parse single array. */
> > > > +     if (event->type == YAML_SCALAR_EVENT) {
> > > > +             std::string firstValue = eventScalarValue(event);
> > > > +             if (firstValue.empty())
> > > > +                     return {};
> > > > +
> > > > +             std::vector<std::string> remaining = parseSingleArray();
> > > > +
> > > > +             std::vector<std::string> values = { firstValue };
> > > > +             values.insert(std::end(values),
> > > > +                           std::begin(remaining), std::end(remaining));
> > > > +             valueArrays.push_back(values);
> > > > +
> > > > +             return valueArrays;
> > > > +     }
> > > > +
> > > > +     /* Parse array of arrays. */
> > > > +     while (1) {
> > > > +             switch (event->type) {
> > > > +             case YAML_SEQUENCE_START_EVENT: {
> > > > +                     std::vector<std::string> values = parseSingleArray();
> > > > +                     valueArrays.push_back(values);
> > > > +                     break;
> > > > +             }
> > > > +             case YAML_SEQUENCE_END_EVENT:
> > > > +                     return valueArrays;
> > > > +             default:
> > > > +                     return {};
> > > > +             }
> > > > +
> > > > +             event = nextEvent();
> > > > +             if (!event)
> > > > +                     return {};
> > > > +     }
> > > > +}
> > > > +
> > > > +std::vector<std::string> CaptureScript::parseSingleArray()
> > > > +{
> > > > +     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 +378,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. */
> > > > +     switch (id->type()) {
> > > > +     case ControlTypeRectangle:
> > > > +             return parseRectangles();
> > > > +     case ControlTypeSize:
> > > > +             /* \todo Parse Sizes. */
> > > > +             return {};
> > > > +     default:
> > > > +             break;
> > > > +     }
> > > > +
> > > > +     /* Parse basic types represented by a single scalar. */
> > > > +     const std::string repr = parseScalar();
> > > > +     if (repr.empty())
> > > > +             return {};
> > > > +
> > > >       ControlValue value{};
> > > >
> > > >       switch (id->type()) {
> > > > @@ -324,13 +440,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..fffe67e5 100644
> > > > --- a/src/cam/capture_script.h
> > > > +++ b/src/cam/capture_script.h
> > > > @@ -54,9 +54,12 @@ private:
> > > >       int parseControl(EventPtr event, libcamera::ControlList &controls);
> > > >
> > > >       std::string parseScalar();
> > > > +     libcamera::ControlValue parseRectangles();
> > > > +     std::vector<std::vector<std::string>> parseArrays();
> > > > +     std::vector<std::string> parseSingleArray();
> > > >
> > > >       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);
> > > >  };
> > > > --
> > > > 2.34.1
> > > >

[libcamera-devel,v3,1/2] cam: Add Rectangle type parsing in capture script
diff mbox series

Commit Message

Comments

Patch
diff mbox series

[libcamera-devel,v3,1/2] cam: Add Rectangle type parsing in capture script 16377 diff mbox series

Commit Message

Comments

Patch 16377 diff mbox series

[libcamera-devel,v3,1/2] cam: Add Rectangle type parsing in capture script
diff mbox series

Patch
diff mbox series