[FFmpeg-devel] [PATCH 25/38] avfilter/avfilter: Honour the short options documentation

[Nicolas George]

Andreas Rheinhardt (12021-09-12):
> The documentation for filter arguments states that short options must
> precede long options (i.e. those of the form key=value). Yet if
> process_options() encounters arguments not abiding by this, it simply
> treats short options after a long option as if it were parsing short
> options for the first time. In particular, it overwrites options already
> set earlier, possibly via other short options. This is not how it is
> intended (as a comment in the code indicates).
> 
> This commit modifies the code to reject further shorthand options
> after a long option has been encountered. After all, avfilter_init_str()
> errors out upon unrecognized options, so it is intended to be picky.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> ---
> The while loop that is removed below is actually just an elaborate
> "o = NULL", which av_opt_next() interprets as "start the iteration".
> 
>  libavfilter/avfilter.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index cc499fd5ed..165ab1f44a 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -814,6 +814,7 @@ static int process_options(AVFilterContext *ctx, AVDictionary **options,
>      const AVOption *o = NULL;
>      int ret;
>      char *av_uninit(parsed_key), *av_uninit(value);

> +    const AVClass *priv = ctx->filter->priv_class;

I do not understand why you make this a pointer rather than an int,
since it is only used as a boolean. "int shorthand_allowed" would be
clearer IMHO.

>      const char *key;
>      int offset= -1;
>  
> @@ -824,8 +825,7 @@ static int process_options(AVFilterContext *ctx, AVDictionary **options,
>          const char *shorthand = NULL;
>          int flags = AV_DICT_DONT_STRDUP_VAL;
>  
> -        o = av_opt_next(ctx->priv, o);
> -        if (o) {
> +        if (priv && (o = av_opt_next(ctx->priv, o))) {
>              if (o->type == AV_OPT_TYPE_CONST || o->offset == offset)
>                  continue;
>              offset = o->offset;
> @@ -848,7 +848,7 @@ static int process_options(AVFilterContext *ctx, AVDictionary **options,
>          if (parsed_key) {
>              key = parsed_key;
>              flags |= AV_DICT_DONT_STRDUP_KEY;
> -            while ((o = av_opt_next(ctx->priv, o))); /* discard all remaining shorthand */
> +            priv = NULL; /* reject all remaining shorthand */
>          } else {
>              key = shorthand;
>          }

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210913/3512a260/attachment.sig>