Hi Sakari,
On Tuesday 08 May 2012 22:43:50 Sakari Ailus wrote:
> Laurent Pinchart wrote:
> > On Monday 07 May 2012 16:46:35 Sakari Ailus wrote:
> >> More flexible and extensible syntax for format which allows better usage
> >> of the selection API.
[snip]
> >> diff --git a/src/v4l2subdev.c b/src/v4l2subdev.c
> >> index a2ab0c4..6881553 100644
> >> --- a/src/v4l2subdev.c
> >> +++ b/src/v4l2subdev.c
> >> @@ -233,13 +233,13 @@ static int v4l2_subdev_parse_format(struct
> >> v4l2_mbus_framefmt *format, char *end;
> >>
> >> for (; isspace(*p); ++p);
> >>
> >> - for (end = (char *)p; !isspace(*end) && *end != '\0'; ++end);
> >> + for (end = (char *)p; *end != '/' && *end != '\0'; ++end);
> >
> > I wouldn't change this to keep compatibility with the existing syntax.
>
> Ok. How about allowing both '/' and ' '?
Do you hate the space that much ? :-) The format code and the resolution are
not that closely related, / somehow doesn't look intuitive to me.
> >> code = v4l2_subdev_string_to_pixelcode(p, end - p);
> >> if (code == (enum v4l2_mbus_pixelcode)-1)
> >>
> >> return -EINVAL;
> >>
> >> - for (p = end; isspace(*p); ++p);
> >> + p = end + 1;
> >>
> >> width = strtoul(p, &end, 10);
> >> if (*end != 'x')
> >>
> >> return -EINVAL;
> >>
[snip]
> >> @@ -326,30 +337,37 @@ static struct media_pad
> >> *v4l2_subdev_parse_pad_format( if (*p++ != '[')
> >>
> >> return NULL;
> >>
> >> - for (; isspace(*p); ++p);
> >> + for (;;) {
> >> + for (; isspace(*p); p++);
> >>
> >> - if (isalnum(*p)) {
> >> - ret = v4l2_subdev_parse_format(format, p, &end);
> >> - if (ret < 0)
> >> - return NULL;
> >> + if (!strhazit("fmt:", &p)) {
> >> + ret = v4l2_subdev_parse_format(format, p, &end);
> >> + if (ret < 0)
> >> + return NULL;
> >>
> >> - for (p = end; isspace(*p); p++);
> >> - }
> >> + p = end;
> >> + continue;
> >> + }
> >
> > I'd like to keep compatibility with the existing syntax here. Checking
> > whether this is the first argument and whether it starts with an
> > uppercase letter should be enough, would you be OK with that ?
>
> Right. I may have missed something related to keeping the compatibility.
>
> Capital letter might not be enough in the future; for now it's ok
> though. How about this: if the string doesn't match with anything else,
> interpret it as format?
I've thought about this, but I'm not sure it's a good idea to introduce
extensions to the existing syntax (we currently have no format starting with
something else than an uppercase letter) as we're deprecating it.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
