[libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type parsing in capture script
Daniel Semkowicz
dse at thaumatec.com
Tue Jun 28 08:35:03 CEST 2022
Hi Jacopo,
On Mon, Jun 27, 2022 at 6:06 PM Jacopo Mondi <jacopo at 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 at 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 at 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
> >
More information about the libcamera-devel
mailing list