| Message ID | 8472b869-e00a-171f-05d4-503379702c3b@jkqxz.net |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] fftools: Accept more negative prefixes | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/configure | warning | Failed to apply patch |
On Thu, Aug 20, 2020 at 22:49:16 +0100, Mark Thompson wrote: > With this patch, -disable_auto_conversion_filters does what you want. [...] > + if (!po->name) { > + /* Try to match a boolean option with a negative prefix. */ > + for (int i = 0; i < FF_ARRAY_ELEMS(negative_prefixes); i++) { > + size_t len = strlen(negative_prefixes[i]); > + if (!strncmp(opt, negative_prefixes[i], len)) { > + po = find_option(options, opt + len); > + if (po->name && (po->flags & OPT_BOOL)) { > + arg = "0"; > + break; > + } > + } > + } Without checking in more detail: Wouldn't this break the "-noise_reduction", "-non_deterministic" and "-non_linear_quant" options? Cheers, Moritz
On 21/08/2020 06:49, Moritz Barsnick wrote: > On Thu, Aug 20, 2020 at 22:49:16 +0100, Mark Thompson wrote: > >> With this patch, -disable_auto_conversion_filters does what you want. > [...] >> + if (!po->name) { >> + /* Try to match a boolean option with a negative prefix. */ >> + for (int i = 0; i < FF_ARRAY_ELEMS(negative_prefixes); i++) { >> + size_t len = strlen(negative_prefixes[i]); >> + if (!strncmp(opt, negative_prefixes[i], len)) { >> + po = find_option(options, opt + len); >> + if (po->name && (po->flags & OPT_BOOL)) { >> + arg = "0"; >> + break; >> + } >> + } >> + } > > Without checking in more detail: Wouldn't this break the > "-noise_reduction", "-non_deterministic" and "-non_linear_quant" > options? The intent is to extend what already happens - given an argument like "-nougat", it first looks for the option of any kind called "nougat", then only if that is not found looks for a boolean option called "ugat". In your example, the "noise_reduction" option is found, so "ise_reduction" is never searched for. (Alternatively: if you are referencing a bug which you have spotted in my change but I am missing, could you explain in more detail?) - Mark
On Sat, Aug 22, 2020 at 21:55:21 +0100, Mark Thompson wrote: > > Without checking in more detail: Wouldn't this break the > > "-noise_reduction", "-non_deterministic" and "-non_linear_quant" > > options? > > The intent is to extend what already happens - given an argument like > "-nougat", it first looks for the option of any kind called "nougat", > then only if that is not found looks for a boolean option called > "ugat". In your example, the "noise_reduction" option is found, so > "ise_reduction" is never searched for. Okay, I failed to check that. Thanks for the explanation, perfect. Moritz
diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c index 88fdbeaf1e..1a1740b46e 100644 --- a/fftools/cmdutils.c +++ b/fftools/cmdutils.c @@ -345,6 +345,13 @@ static int write_option(void *optctx, const OptionDef *po, const char *opt, return 0; } +static const char *negative_prefixes[] = { + "no_", + "no", + "disable_", + "disable", +}; + int parse_option(void *optctx, const char *opt, const char *arg, const OptionDef *options) { @@ -352,11 +359,18 @@ int parse_option(void *optctx, const char *opt, const char *arg, int ret; po = find_option(options, opt); - if (!po->name && opt[0] == 'n' && opt[1] == 'o') { - /* handle 'no' bool option */ - po = find_option(options, opt + 2); - if ((po->name && (po->flags & OPT_BOOL))) - arg = "0"; + if (!po->name) { + /* Try to match a boolean option with a negative prefix. */ + for (int i = 0; i < FF_ARRAY_ELEMS(negative_prefixes); i++) { + size_t len = strlen(negative_prefixes[i]); + if (!strncmp(opt, negative_prefixes[i], len)) { + po = find_option(options, opt + len); + if (po->name && (po->flags & OPT_BOOL)) { + arg = "0"; + break; + } + } + } } else if (po->flags & OPT_BOOL) arg = "1"; @@ -764,6 +778,7 @@ int split_commandline(OptionParseContext *octx, int argc, char *argv[], while (optindex < argc) { const char *opt = argv[optindex++], *arg; const OptionDef *po; + int negative_match; int ret; av_log(NULL, AV_LOG_DEBUG, "Reading option '%s' ...", opt); @@ -831,10 +846,19 @@ do { \ } } - /* boolean -nofoo options */ - if (opt[0] == 'n' && opt[1] == 'o' && - (po = find_option(options, opt + 2)) && - po->name && po->flags & OPT_BOOL) { + /* Boolean options with a negative prefix. */ + negative_match = 0; + for (int i = 0; i < FF_ARRAY_ELEMS(negative_prefixes); i++) { + size_t len = strlen(negative_prefixes[i]); + if (!strncmp(opt, negative_prefixes[i], len)) { + po = find_option(options, opt + len); + if (po->name && (po->flags & OPT_BOOL)) { + negative_match = 1; + break; + } + } + } + if (negative_match) { add_opt(octx, po, opt, "0"); av_log(NULL, AV_LOG_DEBUG, " matched as option '%s' (%s) with " "argument 0.\n", po->name, po->help);