m_option: redo code for parsing -vf to accept quotes

Parsing sub-configs (like --rawvideo=subopts or the suboptions for
--vo=opengl:subopts) was completely different from the -vf parsing code
for a variety of reasons. This change at least makes -vf use the same
splitter code as sub-config options.

The main improvement is that -vf now accepts quotes, so you can write
things like:

     -vf 'lavfi=graph="gradfun=10:20"'

(The '' quotes are for shell escaping.)

This is a rather big and intrusive change. Trying some -vf lines from
etc/encoding-example-profiles.conf seems to confirm it still works.

This also attempts to unify one subtle difference in handling of
positional arguments. One consequence is that a minor detail changes.
Sub-configs don't know positional arguments, and something like "--
opt=sub1=val1:sub2" means that sub2 has to be a flag option. In -vf
parsing, sub2 would be a positional option value. To remove this
conflict and to facilitate actual unification of the parsers in the
future, the sub2 will be considered a flag option if and only if such a
flag option exists. Otherwise, it's considered a value for a positional
option.

E.g. if there's a filter "foo" with a string option "sopt" and a flag
option "fopt", the behavior of the following changes:

   -vf foo=fopt

Before this commit, this would set "sopt=fopt" in the filter. Now, it
enables the fopt flag, and the sopt option remains unset. This is not an
actual problem to my knowledge.

core/m_option.c

@@ -1718,160 +1718,151 @@ static int find_obj_desc(struct bstr name, const m_obj_list_t *l,
     return 0;
 }
 
-static int get_obj_param(struct bstr opt_name, struct bstr obj_name,
+// Consider -vf a=b=c:d=e. This verifies "b"="c" and "d"="e" and that the
-                         const m_struct_t *desc, struct bstr str, int *nold,
+// option names/values are correct. Try to determine whether an option
-                         int oldmax, char **dst)
+// without '=' sets a flag, or whether it's a positional argument.
+static int get_obj_param(bstr opt_name, bstr obj_name, const m_struct_t *desc,
+                         bstr name, bstr val, int *nold, int oldmax,
+                         bstr *out_name, bstr *out_val)
 {
-    const m_option_t *opt;
+    const m_option_t *opt = m_option_list_findb(desc->fields, name);
     int r;
 
-    int eq = bstrchr(str, '=');
+    // va.start != NULL => of the form name=val (not positional)
-
+    // If it's just "name", and the associated option exists and is a flag,
-    if (eq > 0) {   // eq == 0  ignored
+    // don't accept it as positional argument.
-        struct bstr p = bstr_cut(str, eq + 1);
+    if (val.start || (opt && m_option_required_params(opt) == 0)) {
-        str = bstr_splice(str, 0, eq);
-        opt = m_option_list_findb(desc->fields, str);
         if (!opt) {
             mp_msg(MSGT_CFGPARSER, MSGL_ERR,
                    "Option %.*s: %.*s doesn't have a %.*s parameter.\n",
-                   BSTR_P(opt_name), BSTR_P(obj_name), BSTR_P(str));
+                   BSTR_P(opt_name), BSTR_P(obj_name), BSTR_P(name));
             return M_OPT_UNKNOWN;
         }
-        r = m_option_parse(opt, str, p, NULL);
+        r = m_option_parse(opt, name, val, NULL);
         if (r < 0) {
             if (r > M_OPT_EXIT)
                 mp_msg(MSGT_CFGPARSER, MSGL_ERR, "Option %.*s: "
                        "Error while parsing %.*s parameter %.*s (%.*s)\n",
-                       BSTR_P(opt_name), BSTR_P(obj_name), BSTR_P(str),
+                       BSTR_P(opt_name), BSTR_P(obj_name), BSTR_P(name),
-                       BSTR_P(p));
+                       BSTR_P(val));
             return r;
         }
-        if (dst) {
+        *out_name = name;
-            dst[0] = bstrdup0(NULL, str);
+        *out_val = val;
-            dst[1] = bstrdup0(NULL, p);
+        return 1;
-        }
     } else {
+        val = name;
+        // positional fields
+        if (val.len == 0) { // Empty field, count it and go on
+            (*nold)++;
+            return 1;
+        }
         if ((*nold) >= oldmax) {
             mp_msg(MSGT_CFGPARSER, MSGL_ERR, "Option %.*s: %.*s has only %d params, so you can't give more than %d unnamed params.\n",
                    BSTR_P(opt_name), BSTR_P(obj_name), oldmax, oldmax);
             return M_OPT_OUT_OF_RANGE;
         }
         opt = &desc->fields[(*nold)];
-        r = m_option_parse(opt, bstr0(opt->name), str, NULL);
+        r = m_option_parse(opt, bstr0(opt->name), val, NULL);
         if (r < 0) {
             if (r > M_OPT_EXIT)
                 mp_msg(MSGT_CFGPARSER, MSGL_ERR, "Option %.*s: "
                        "Error while parsing %.*s parameter %s (%.*s)\n",
                        BSTR_P(opt_name), BSTR_P(obj_name), opt->name,
-                       BSTR_P(str));
+                       BSTR_P(val));
             return r;
         }
-        if (dst) {
+        *out_name = bstr0(opt->name);
-            dst[0] = talloc_strdup(NULL, opt->name);
+        *out_val = val;
-            dst[1] = bstrdup0(NULL, str);
-        }
         (*nold)++;
+        return 1;
     }
-    return 1;
 }
 
+// Consider -vf a=b:c:d. This parses "b:c:d" into name/value pairs, stored as
+// linear array in *_ret. In particular, desc contains what options a the
+// object takes, and verifies the option values as well.
 static int get_obj_params(struct bstr opt_name, struct bstr name,
-                          struct bstr params, const m_struct_t *desc,
+                          struct bstr *pstr, const m_struct_t *desc,
-                          char separator, char ***_ret)
+                          char ***ret)
 {
     int n = 0, nold = 0, nopts;
-    char **ret;
+    char **args = NULL;
-
+    int num_args = 0;
-    if (!bstrcmp0(params, "help")) { // Help
+    int r = 1;
-        char min[50], max[50];
-        if (!desc->fields) {
-            mp_msg(MSGT_CFGPARSER, MSGL_INFO,
-                   "%.*s doesn't have any options.\n\n", BSTR_P(name));
-            return M_OPT_EXIT - 1;
-        }
-        mp_msg(MSGT_CFGPARSER, MSGL_INFO,
-               "\n Name                 Type            Min        Max\n\n");
-        for (n = 0; desc->fields[n].name; n++) {
-            const m_option_t *opt = &desc->fields[n];
-            if (opt->type->flags & M_OPT_TYPE_HAS_CHILD)
-                continue;
-            if (opt->flags & M_OPT_MIN)
-                sprintf(min, "%-8.0f", opt->min);
-            else
-                strcpy(min, "No");
-            if (opt->flags & M_OPT_MAX)
-                sprintf(max, "%-8.0f", opt->max);
-            else
-                strcpy(max, "No");
-            mp_msg(MSGT_CFGPARSER, MSGL_INFO,
-                   " %-20.20s %-15.15s %-10.10s %-10.10s\n",
-                   opt->name, opt->type->name, min, max);
-        }
-        mp_msg(MSGT_CFGPARSER, MSGL_INFO, "\n");
-        return M_OPT_EXIT - 1;
-    }
 
     for (nopts = 0; desc->fields[nopts].name; nopts++)
         /* NOP */;
 
-    // TODO : Check that each opt can be parsed
+    while (pstr->len > 0) {
-    struct bstr s = params;
+        bstr fname, fval;
-    while (1) {
+        r = split_subconf(pstr, &fname, &fval);
-        bool end = false;
+        if (r < 0)
-        int idx = bstrchr(s, separator);
+            goto exit;
-        if (idx < 0) {
+        if (bstr_equals0(fname, "help"))
-            idx = s.len;
+            goto print_help;
-            end = true;
+        r = get_obj_param(opt_name, name, desc, fname, fval, &nold, nopts,
+                          &fname, &fval);
+        if (r < 0)
+            goto exit;
+
+        if (ret) {
+            MP_TARRAY_APPEND(NULL, args, num_args, bstrto0(NULL, fname));
+            MP_TARRAY_APPEND(NULL, args, num_args, bstrto0(NULL, fval));
         }
-        struct bstr field = bstr_splice(s, 0, idx);
+
-        s = bstr_cut(s, idx + 1);
+        if (!bstr_eatstart0(pstr, ":"))
-        if (field.len == 0) { // Empty field, count it and go on
-            nold++;
-        } else {
-            int r = get_obj_param(opt_name, name, desc, field, &nold, nopts,
-                                  NULL);
-            if (r < 0)
-                return r;
-            n++;
-        }
-        if (end)
             break;
     }
-    if (nold > nopts) {
-        mp_msg(MSGT_CFGPARSER, MSGL_ERR, "Too many options for %.*s\n",
-               BSTR_P(name));
-        return M_OPT_OUT_OF_RANGE;
-    }
-    if (!_ret) // Just test
-        return 1;
-    if (n == 0) // No options or only empty options
-        return 1;
 
-    ret = talloc_array(NULL, char *, (n + 2) * 2);
+    if (ret) {
-    n = nold = 0;
+        if (num_args > 0) {
-    s = params;
+            for (int n = 0; n < 2; n++)
-
+                MP_TARRAY_APPEND(NULL, args, num_args, NULL);
-    while (s.len > 0) {
+            *ret = args;
-        int idx = bstrchr(s, separator);
+            args = NULL;
-        if (idx < 0)
-            idx = s.len;
-        struct bstr field = bstr_splice(s, 0, idx);
-        s = bstr_cut(s, idx + 1);
-        if (field.len == 0) { // Empty field, count it and go on
-            nold++;
         } else {
-            get_obj_param(opt_name, name, desc, field, &nold, nopts,
+            *ret = NULL;
-                          &ret[n * 2]);
-            n++;
         }
     }
-    ret[n * 2] = ret[n * 2 + 1] = NULL;
-    *_ret = ret;
 
-    return 1;
+exit:
+    return r;
+
+print_help: ;
+    char min[50], max[50];
+    if (!desc->fields) {
+        mp_msg(MSGT_CFGPARSER, MSGL_INFO,
+                "%.*s doesn't have any options.\n\n", BSTR_P(name));
+        return M_OPT_EXIT - 1;
+    }
+    mp_msg(MSGT_CFGPARSER, MSGL_INFO,
+            "\n Name                 Type            Min        Max\n\n");
+    for (n = 0; desc->fields[n].name; n++) {
+        const m_option_t *opt = &desc->fields[n];
+        if (opt->type->flags & M_OPT_TYPE_HAS_CHILD)
+            continue;
+        if (opt->flags & M_OPT_MIN)
+            sprintf(min, "%-8.0f", opt->min);
+        else
+            strcpy(min, "No");
+        if (opt->flags & M_OPT_MAX)
+            sprintf(max, "%-8.0f", opt->max);
+        else
+            strcpy(max, "No");
+        mp_msg(MSGT_CFGPARSER, MSGL_INFO,
+                " %-20.20s %-15.15s %-10.10s %-10.10s\n",
+                opt->name, opt->type->name, min, max);
+    }
+    mp_msg(MSGT_CFGPARSER, MSGL_INFO, "\n");
+    return M_OPT_EXIT - 1;
 }
 
-static int parse_obj_settings(struct bstr opt, struct bstr str,
+// Characters which may appear in a filter name
+#define NAMECH "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-"
+
+// Parse one item, e.g. -vf a=b:c:d,e=f:g => parse a=b:c:d into "a" and "b:c:d"
+static int parse_obj_settings(struct bstr opt, struct bstr *pstr,
                               const m_obj_list_t *list,
                               m_obj_settings_t **_ret, int ret_n)
 {
@@ -1880,12 +1871,12 @@ static int parse_obj_settings(struct bstr opt, struct bstr str,
     const m_struct_t *desc;
     m_obj_settings_t *ret = _ret ? *_ret : NULL;
 
-    struct bstr param = bstr0(NULL);
+    bool has_param = false;
-    int idx = bstrchr(str, '=');
+    int idx = bstrspn(*pstr, NAMECH);
-    if (idx >= 0) {
+    bstr str = bstr_splice(*pstr, 0, idx);
-        param = bstr_cut(str, idx + 1);
+    *pstr = bstr_cut(*pstr, idx);
-        str = bstr_splice(str, 0, idx);
+    if (bstr_eatstart0(pstr, "="))
-    }
+        has_param = true;
 
     if (!find_obj_desc(str, list, &desc)) {
         mp_msg(MSGT_CFGPARSER, MSGL_ERR, "Option %.*s: %.*s doesn't exist.\n",
@@ -1893,20 +1884,26 @@ static int parse_obj_settings(struct bstr opt, struct bstr str,
         return M_OPT_INVALID;
     }
 
-    if (param.start) {
+    if (has_param) {
-        if (!desc && _ret) {
+        if (!desc) {
+            // Should perhaps be parsed as escape-able string. But this is a
+            // compatibility path, so it's not worth the trouble.
+            int next = bstrcspn(*pstr, ",");
+            bstr param = bstr_splice(*pstr, 0, next);
+            *pstr = bstr_cut(*pstr, next);
             if (!bstrcmp0(param, "help")) {
                 mp_msg(MSGT_CFGPARSER, MSGL_INFO,
                        "Option %.*s: %.*s have no option description.\n",
                        BSTR_P(opt), BSTR_P(str));
                 return M_OPT_EXIT - 1;
             }
-            plist = talloc_zero_array(NULL, char *, 4);
+            if (_ret) {
-            plist[0] = talloc_strdup(NULL, "_oldargs_");
+                plist = talloc_zero_array(NULL, char *, 4);
-            plist[1] = bstrdup0(NULL, param);
+                plist[0] = talloc_strdup(NULL, "_oldargs_");
+                plist[1] = bstrto0(NULL, param);
+            }
         } else if (desc) {
-            r = get_obj_params(opt, str, param, desc, ':',
+            r = get_obj_params(opt, str, pstr, desc, _ret ? &plist : NULL);
-                               _ret ? &plist : NULL);
             if (r < 0)
                 return r;
         }
@@ -1916,7 +1913,7 @@ static int parse_obj_settings(struct bstr opt, struct bstr str,
 
     ret = talloc_realloc(NULL, ret, struct m_obj_settings, ret_n + 2);
     memset(&ret[ret_n], 0, 2 * sizeof(m_obj_settings_t));
-    ret[ret_n].name = bstrdup0(NULL, str);
+    ret[ret_n].name = bstrto0(NULL, str);
     ret[ret_n].attribs = plist;
 
     *_ret = ret;
@@ -2096,20 +2093,16 @@ static int parse_obj_settings_list(const m_option_t *opt, struct bstr name,
         return M_OPT_EXIT - 1;
     }
 
-    struct bstr s = bstrdup(NULL, param);
-    char *allocptr = s.start;
     int n = 0;
-    while (s.len > 0) {
+    while (param.len > 0) {
-        struct bstr el = get_nextsep(&s, OPTION_LIST_SEPARATOR, 1);
+        int r = parse_obj_settings(name, &param, opt->priv, dst ? &res : NULL, n);
-        int r = parse_obj_settings(name, el, opt->priv, dst ? &res : NULL, n);
+        if (r < 0)
-        if (r < 0) {
-            talloc_free(allocptr);
             return r;
-        }
+        char sep[2] = {OPTION_LIST_SEPARATOR, 0};
-        s = bstr_cut(s, 1);
+        if (param.len > 0 && !bstr_eatstart0(&param, sep))
+            return M_OPT_INVALID;
         n++;
     }
-    talloc_free(allocptr);
     if (n == 0)
         return M_OPT_INVALID;