From 50008adf4af5360f87109d6264aa77ac82f2a80a Mon Sep 17 00:00:00 2001 From: wm4 Date: Mon, 26 Jun 2017 21:07:00 +0200 Subject: options: handle suffixes like -add in a more generic way This affects options like --vf or --display-tags. These used a "*" suffix to match all options starting with a specific name, and handled the rest in the option parser. Change this to remove the "*" special case, and require every option parser to declare a list of allowed suffixes via m_option_type.actions. The new way is conceptually simpler, because we don't have to account for the "*" in a bunch of places anymore, and instead everything is centrally handled in the CLI part of the option parser, where it's actually needed. It automatically enables suffixes like -add for a bunch of other stringlist options. --- common/encode_lavc.c | 6 +-- options/m_config.c | 94 +++++++++++++++++++++++---------- options/m_option.c | 144 +++++++++++++++++++++++---------------------------- options/m_option.h | 29 +++++------ options/options.c | 6 +-- player/command.c | 25 --------- 6 files changed, 151 insertions(+), 153 deletions(-) diff --git a/common/encode_lavc.c b/common/encode_lavc.c index 5fbad39d44..2ae9eef6bd 100644 --- a/common/encode_lavc.c +++ b/common/encode_lavc.c @@ -39,13 +39,13 @@ const struct m_sub_options encode_config = { .opts = (const m_option_t[]) { OPT_STRING("o", file, M_OPT_FIXED | CONF_NOCFG | CONF_PRE_PARSE | M_OPT_FILE), OPT_STRING("of", format, M_OPT_FIXED), - OPT_STRINGLIST("ofopts*", fopts, M_OPT_FIXED), + OPT_STRINGLIST("ofopts", fopts, M_OPT_FIXED), OPT_FLOATRANGE("ofps", fps, M_OPT_FIXED, 0.0, 1000000.0), OPT_FLOATRANGE("omaxfps", maxfps, M_OPT_FIXED, 0.0, 1000000.0), OPT_STRING("ovc", vcodec, M_OPT_FIXED), - OPT_STRINGLIST("ovcopts*", vopts, M_OPT_FIXED), + OPT_STRINGLIST("ovcopts", vopts, M_OPT_FIXED), OPT_STRING("oac", acodec, M_OPT_FIXED), - OPT_STRINGLIST("oacopts*", aopts, M_OPT_FIXED), + OPT_STRINGLIST("oacopts", aopts, M_OPT_FIXED), OPT_FLAG("oharddup", harddup, M_OPT_FIXED), OPT_FLOATRANGE("ovoffset", voffset, M_OPT_FIXED, -1000000.0, 1000000.0), OPT_FLOATRANGE("oaoffset", aoffset, M_OPT_FIXED, -1000000.0, 1000000.0), diff --git a/options/m_config.c b/options/m_config.c index 27bd5b620e..5de3b319d5 100644 --- a/options/m_config.c +++ b/options/m_config.c @@ -536,14 +536,8 @@ struct m_config_option *m_config_get_co_raw(const struct m_config *config, for (int n = 0; n < config->num_opts; n++) { struct m_config_option *co = &config->opts[n]; struct bstr coname = bstr0(co->name); - if ((co->opt->type->flags & M_OPT_TYPE_ALLOW_WILDCARD) - && bstr_endswith0(coname, "*")) { - coname.len--; - if (bstrcmp(bstr_splice(name, 0, coname.len), coname) == 0) - return co; - } else if (bstrcmp(coname, name) == 0) { + if (bstrcmp(coname, name) == 0) return co; - } } return NULL; @@ -782,25 +776,61 @@ int m_config_set_option_raw(struct m_config *config, struct m_config_option *co, } } +// Handle CLI exceptions to option handling. // Used to turn "--no-foo" into "--foo=no". -static struct m_config_option *m_config_find_negation_opt(struct m_config *config, - struct bstr *name) +// It also handles looking up "--vf-add" as "--vf". +static struct m_config_option *m_config_mogrify_cli_opt(struct m_config *config, + struct bstr *name, + bool *out_negate, + int *out_add_flags) { - assert(!m_config_get_co(config, *name)); - - if (!bstr_eatstart0(name, "no-")) - return NULL; + *out_negate = false; + *out_add_flags = 0; struct m_config_option *co = m_config_get_co(config, *name); + if (co) + return co; + + // Turn "--no-foo" into "foo" + set *out_negate. + if (!co && bstr_eatstart0(name, "no-")) { + co = m_config_get_co(config, *name); + + // Not all choice types have this value - if they don't, then parsing + // them will simply result in an error. Good enough. + if (co && co->opt->type != CONF_TYPE_FLAG && + co->opt->type != CONF_TYPE_CHOICE && + co->opt->type != &m_option_type_aspect) + return NULL; + + *out_negate = true; + return co; + } - // Not all choice types have this value - if they don't, then parsing them - // will simply result in an error. Good enough. - if (co && co->opt->type != CONF_TYPE_FLAG && - co->opt->type != CONF_TYPE_CHOICE && - co->opt->type != &m_option_type_aspect) - co = NULL; + // Might be a suffix "action", like "--vf-add". Expensively check for + // matches. (Also, we don't allow you to combine them with "--no-".) + for (int n = 0; n < config->num_opts; n++) { + co = &config->opts[n]; + const struct m_option_type *type = co->opt->type; + struct bstr coname = bstr0(co->name); - return co; + if (!bstr_startswith(*name, coname)) + continue; + + for (int i = 0; type->actions && type->actions[i].name; i++) { + const struct m_option_action *action = &type->actions[i]; + bstr suffix = bstr0(action->name); + + if (bstr_endswith(*name, suffix) && + (name->len == coname.len + 1 + suffix.len) && + name->start[coname.len] == '-') + { + *out_add_flags = action->flags; + return co; + } + } + } + + return NULL; } static int m_config_parse_option(struct m_config *config, struct bstr name, @@ -808,12 +838,14 @@ static int m_config_parse_option(struct m_config *config, struct bstr name, { assert(config != NULL); - struct m_config_option *co = m_config_get_co(config, name); - if (!co) { - co = m_config_find_negation_opt(config, &name); - if (!co) - return M_OPT_UNKNOWN; + bool negate; + struct m_config_option *co = + m_config_mogrify_cli_opt(config, &name, &negate, &(int){0}); + if (!co) + return M_OPT_UNKNOWN; + + if (negate) { if (param.len) return M_OPT_DISALLOW_PARAM; @@ -904,11 +936,17 @@ int m_config_set_option_node(struct m_config *config, bstr name, int m_config_option_requires_param(struct m_config *config, bstr name) { - struct m_config_option *co = m_config_get_co(config, name); + bool negate; + int flags; + struct m_config_option *co = + m_config_mogrify_cli_opt(config, &name, &negate, &flags); + if (!co) - return m_config_find_negation_opt(config, &name) ? 0 : M_OPT_UNKNOWN; - if (bstr_endswith0(name, "-clr")) + return M_OPT_UNKNOWN; + + if (negate || (flags & M_OPT_TYPE_OPTIONAL_PARAM)) return 0; + return m_option_required_params(co->opt); } diff --git a/options/m_option.c b/options/m_option.c index 2899ad01d5..333417e438 100644 --- a/options/m_option.c +++ b/options/m_option.c @@ -89,27 +89,15 @@ int m_option_required_params(const m_option_t *opt) return 1; } -static const struct m_option *m_option_list_findb(const struct m_option *list, - struct bstr name) +const m_option_t *m_option_list_find(const m_option_t *list, const char *name) { for (int i = 0; list[i].name; i++) { - struct bstr lname = bstr0(list[i].name); - if ((list[i].type->flags & M_OPT_TYPE_ALLOW_WILDCARD) - && bstr_endswith0(lname, "*")) { - lname.len--; - if (bstrcmp(bstr_splice(name, 0, lname.len), lname) == 0) - return &list[i]; - } else if (bstrcmp(lname, name) == 0) + if (strcmp(list[i].name, name) == 0) return &list[i]; } return NULL; } -const m_option_t *m_option_list_find(const m_option_t *list, const char *name) -{ - return m_option_list_findb(list, bstr0(name)); -} - int m_option_set_node_or_string(struct mp_log *log, const m_option_t *opt, const char *name, void *dst, struct mpv_node *src) { @@ -1223,20 +1211,17 @@ static int parse_str_list(struct mp_log *log, const m_option_t *opt, { char **res; int op = OP_NONE; - if (bstr_endswith0(bstr0(opt->name), "*")) { - struct bstr suffix = bstr_cut(name, strlen(opt->name) - 1); - if (bstrcmp0(suffix, "-add") == 0) - op = OP_ADD; - else if (bstrcmp0(suffix, "-pre") == 0) - op = OP_PRE; - else if (bstrcmp0(suffix, "-del") == 0) - op = OP_DEL; - else if (bstrcmp0(suffix, "-clr") == 0) - op = OP_CLR; - else if (suffix.len == 0) - op = OP_NONE; - else - return M_OPT_UNKNOWN; + + if (bstr_endswith0(name, "-add")) { + op = OP_ADD; + } else if (bstr_endswith0(name, "-pre")) { + op = OP_PRE; + } else if (bstr_endswith0(name, "-del")) { + op = OP_DEL; + } else if (bstr_endswith0(name, "-clr")) { + op = OP_CLR; + } else if (bstr_endswith0(name, "-set")) { + op = OP_NONE; } // Clear the list ?? @@ -1382,23 +1367,22 @@ static int str_list_get(const m_option_t *opt, void *ta_parent, } const m_option_type_t m_option_type_string_list = { - /* A list of strings separated by ','. - * Option with a name ending in '*' permits using the following suffixes: - * -add: Add the given parameters at the end of the list. - * -pre: Add the given parameters at the beginning of the list. - * -del: Remove the entry at the given indices. - * -clr: Clear the list. - * e.g: -vf-add flip,mirror -vf-del 2,5 - */ .name = "String list", .size = sizeof(char **), - .flags = M_OPT_TYPE_ALLOW_WILDCARD, .parse = parse_str_list, .print = print_str_list, .copy = copy_str_list, .free = free_str_list, .get = str_list_get, .set = str_list_set, + .actions = (const struct m_option_action[]){ + {"set"}, + {"add"}, + {"pre"}, + {"del"}, + {"clr", M_OPT_TYPE_OPTIONAL_PARAM}, + {0} + }, }; static void str_list_append(void *dst, bstr s) @@ -1627,7 +1611,7 @@ static int parse_print(struct mp_log *log, const m_option_t *opt, const m_option_type_t m_option_type_print_fn = { .name = "Print", - .flags = M_OPT_TYPE_ALLOW_WILDCARD | M_OPT_TYPE_OPTIONAL_PARAM, + .flags = M_OPT_TYPE_OPTIONAL_PARAM, .parse = parse_print, }; @@ -2916,46 +2900,39 @@ static int parse_obj_settings_list(struct mp_log *log, const m_option_t *opt, assert(opt->priv); - if (bstr_endswith0(bstr0(opt->name), "*")) { - struct bstr suffix = bstr_cut(name, strlen(opt->name) - 1); - if (bstrcmp0(suffix, "-add") == 0) - op = OP_ADD; - else if (bstrcmp0(suffix, "-set") == 0) - op = OP_NONE; - else if (bstrcmp0(suffix, "-pre") == 0) - op = OP_PRE; - else if (bstrcmp0(suffix, "-del") == 0) - op = OP_DEL; - else if (bstrcmp0(suffix, "-clr") == 0) - op = OP_CLR; - else if (bstrcmp0(suffix, "-toggle") == 0) - op = OP_TOGGLE; - else if (suffix.len == 0) - op = OP_NONE; - else { - char pre[80]; - snprintf(pre, sizeof(pre), "%.*s", (int)(strlen(opt->name) - 1), - opt->name); - mp_err(log, "Option %.*s: unknown postfix %.*s\n" - "Supported postfixes are:\n" - " %s-set\n" - " Overwrite the old list with the given list\n\n" - " %s-add\n" - " Append the given list to the current list\n\n" - " %s-pre\n" - " Prepend the given list to the current list\n\n" - " %s-del x,y,...\n" - " Remove the given elements. Take the list element index (starting from 0).\n" - " Negative index can be used (i.e. -1 is the last element).\n" - " Filter names work as well.\n\n" - " %s-toggle\n" - " Add the filter to the list, or remove it if it's already added.\n\n" - " %s-clr\n" - " Clear the current list.\n\n", - BSTR_P(name), BSTR_P(suffix), pre, pre, pre, pre, pre, pre); - - return M_OPT_UNKNOWN; - } + if (bstr_endswith0(name, "-add")) { + op = OP_ADD; + } else if (bstr_endswith0(name, "-set")) { + op = OP_NONE; + } else if (bstr_endswith0(name, "-pre")) { + op = OP_PRE; + } else if (bstr_endswith0(name, "-del")) { + op = OP_DEL; + } else if (bstr_endswith0(name, "-clr")) { + op = OP_CLR; + } else if (bstr_endswith0(name, "-toggle")) { + op = OP_TOGGLE; + } else if (bstr_endswith0(name, "-help")) { + mp_err(log, "Option %s:\n" + "Supported operations are:\n" + " %s-set\n" + " Overwrite the old list with the given list\n\n" + " %s-add\n" + " Append the given list to the current list\n\n" + " %s-pre\n" + " Prepend the given list to the current list\n\n" + " %s-del x,y,...\n" + " Remove the given elements. Take the list element index (starting from 0).\n" + " Negative index can be used (i.e. -1 is the last element).\n" + " Filter names work as well.\n\n" + " %s-toggle\n" + " Add the filter to the list, or remove it if it's already added.\n\n" + " %s-clr\n" + " Clear the current list.\n\n", + opt->name, opt->name, opt->name, opt->name, opt->name, + opt->name, opt->name); + + return M_OPT_EXIT; } if (!bstrcmp0(param, "help")) { @@ -3243,13 +3220,22 @@ static int get_obj_settings_list(const m_option_t *opt, void *ta_parent, const m_option_type_t m_option_type_obj_settings_list = { .name = "Object settings list", .size = sizeof(m_obj_settings_t *), - .flags = M_OPT_TYPE_ALLOW_WILDCARD, .parse = parse_obj_settings_list, .print = print_obj_settings_list, .copy = copy_obj_settings_list, .free = free_obj_settings_list, .set = set_obj_settings_list, .get = get_obj_settings_list, + .actions = (const struct m_option_action[]){ + {"add"}, + {"set"}, + {"pre"}, + {"del"}, + {"clr", M_OPT_TYPE_OPTIONAL_PARAM}, + {"help", M_OPT_TYPE_OPTIONAL_PARAM}, + {"toggle"}, + {0} + }, }; #undef VAL diff --git a/options/m_option.h b/options/m_option.h index 20021c2122..fc879d44f1 100644 --- a/options/m_option.h +++ b/options/m_option.h @@ -243,6 +243,15 @@ union m_option_value { //////////////////////////////////////////////////////////////////////////// +struct m_option_action { + // The name of the suffix, e.g. "add" for a list. If the option is named + // "foo", this will be available as "--foo-add". Note that no suffix (i.e. + // "--foo" is implicitly always available. + const char *name; + // One of M_OPT_TYPE*. + unsigned int flags; +}; + // Option type description struct m_option_type { const char *name; @@ -318,6 +327,10 @@ struct m_option_type { // static strings (and even mpv_node_list.keys), though. int (*get)(const m_option_t *opt, void *ta_parent, struct mpv_node *dst, void *src); + + // Optional: list of suffixes, terminated with a {0} entry. An empty list + // behaves like the list being NULL. + const struct m_option_action *actions; }; // Option description @@ -412,17 +425,11 @@ struct m_option { // These flags are used to describe special parser capabilities or behavior. -// Wildcard matching flag. -/** If set the option type has a use for option names ending with a * - * (used for -aa*), this only affects the option name matching. - */ -#define M_OPT_TYPE_ALLOW_WILDCARD (1 << 1) - // The parameter is optional and by default no parameter is preferred. If // ambiguous syntax is used ("--opt value"), the command line parser will // assume that the argument takes no parameter. In config files, these // options can be used without "=" and value. -#define M_OPT_TYPE_OPTIONAL_PARAM (1 << 2) +#define M_OPT_TYPE_OPTIONAL_PARAM (1 << 0) ///////////////////////////// Parser flags ///////////////////////////////// @@ -460,14 +467,6 @@ struct m_option { char *m_option_strerror(int code); // Find the option matching the given name in the list. -/** \ingroup Options - * This function takes the possible wildcards into account (see - * \ref M_OPT_TYPE_ALLOW_WILDCARD). - * - * \param list Pointer to an array of \ref m_option. - * \param name Name of the option. - * \return The matching option or NULL. - */ const m_option_t *m_option_list_find(const m_option_t *list, const char *name); // Helper to parse options, see \ref m_option_type::parse. diff --git a/options/options.c b/options/options.c index 1937a7fe65..a803dccf5d 100644 --- a/options/options.c +++ b/options/options.c @@ -367,7 +367,7 @@ const m_option_t mp_opts[] = { OPT_CHOICE_OR_INT("hls-bitrate", hls_bitrate, 0, 0, INT_MAX, ({"no", -1}, {"min", 0}, {"max", INT_MAX})), - OPT_STRINGLIST("display-tags*", display_tags, 0), + OPT_STRINGLIST("display-tags", display_tags, 0), #if HAVE_CDDA OPT_SUBSTRUCT("cdda", stream_cdda_opts, stream_cdda_conf, 0), @@ -414,9 +414,9 @@ const m_option_t mp_opts[] = { // ------------------------- codec/vfilter options -------------------- OPT_SETTINGSLIST("af-defaults", af_defs, 0, &af_obj_list, ), - OPT_SETTINGSLIST("af*", af_settings, 0, &af_obj_list, ), + OPT_SETTINGSLIST("af", af_settings, 0, &af_obj_list, ), OPT_SETTINGSLIST("vf-defaults", vf_defs, 0, &vf_obj_list, ), - OPT_SETTINGSLIST("vf*", vf_settings, 0, &vf_obj_list, ), + OPT_SETTINGSLIST("vf", vf_settings, 0, &vf_obj_list, ), OPT_CHOICE("deinterlace", deinterlace, 0, ({"auto", -1}, diff --git a/player/command.c b/player/command.c index bb94fb0892..bc9ee4ab26 100644 --- a/player/command.c +++ b/player/command.c @@ -351,15 +351,7 @@ int mp_on_set_option(void *ctx, struct m_config_option *co, void *data, int flag { struct MPContext *mpctx = ctx; struct command_ctx *cmd = mpctx->command_ctx; - - // Normalize "vf*" to "vf" const char *name = co->name; - bstr bname = bstr0(name); - char tmp[50]; - if (bstr_eatend0(&bname, "*")) { - snprintf(tmp, sizeof(tmp), "%.*s", BSTR_P(bname)); - name = tmp; - } // Skip going through mp_property_generic_option (typically), because the // property implementation is trivial, and can break some obscure features @@ -430,17 +422,6 @@ static int mp_property_generic_option(void *ctx, struct m_property *prop, return M_PROPERTY_NOT_IMPLEMENTED; } -// Dumb special-case: the option name ends in a "*". -static int mp_property_generic_option_star(void *ctx, struct m_property *prop, - int action, void *arg) -{ - struct m_property prop2 = *prop; - char name[80]; - snprintf(name, sizeof(name), "%s*", prop->name); - prop2.name = name; - return mp_property_generic_option(ctx, &prop2, action, arg); -} - /// Playback speed (RW) static int mp_property_playback_speed(void *ctx, struct m_property *prop, int action, void *arg) @@ -5721,12 +5702,6 @@ void command_init(struct MPContext *mpctx) .call = mp_property_generic_option, .is_option = true, }; - - bstr bname = bstr0(prop.name); - if (bstr_eatend0(&bname, "*")) { - prop.name = bstrto0(ctx, bname); - prop.call = mp_property_generic_option_star; - } } if (prop.name) { -- cgit v1.2.3