[FFmpeg-devel] [PATCH v2 2/4] ffmpeg: Add display_matrix option

[Marton Balint]

On Mon, 15 Aug 2022, Thilo Borgmann wrote:

> diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
> index 18e768b386..22ba654bb0 100644
> --- a/fftools/cmdutils.c
> +++ b/fftools/cmdutils.c
> @@ -131,6 +131,22 @@ int64_t parse_time_or_die(const char *context, const char *timestr,
>      return us;
>  }
> 
> +static AVDictionary *parse_dict_or_die(const char *context,
> +                                       const char *dict_str)
> +{
> +    AVDictionary *dict = NULL;
> +    int ret = av_dict_parse_string(&dict, dict_str, "=", ",", 0);

Please use the av_opt syntax for the dictionary for better consistency:
av_dict_parse_string(&options, val, "=", ":", 0)

> +    if (ret < 0) {
> +        av_log(NULL, AV_LOG_FATAL,
> +               "Failed to create a dictionary from '%s': %s!\n",
> +               dict_str, av_err2str(ret));
> +        exit_program(1);
> +    }
> +
> +
> +    return dict;
> +}
> +
>  void show_help_options(const OptionDef *options, const char *msg, int req_flags,
>                         int rej_flags, int alt_flags)
>  {
> @@ -288,6 +304,8 @@ static int write_option(void *optctx, const OptionDef *po, const char *opt,
>          *(float *)dst = parse_number_or_die(opt, arg, OPT_FLOAT, -INFINITY, INFINITY);
>      } else if (po->flags & OPT_DOUBLE) {
>          *(double *)dst = parse_number_or_die(opt, arg, OPT_DOUBLE, -INFINITY, INFINITY);
> +    } else if (po->flags & OPT_DICT) {
> +        *(AVDictionary **)dst = parse_dict_or_die(opt, arg);
>      } else if (po->u.func_arg) {
>          int ret = po->u.func_arg(optctx, opt, arg);
>          if (ret < 0) {
> diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
> index d87e162ccd..6a519c6546 100644
> --- a/fftools/cmdutils.h
> +++ b/fftools/cmdutils.h
> @@ -129,6 +129,7 @@ typedef struct SpecifierOpt {
>          uint64_t ui64;
>          float      f;
>          double   dbl;
> +        AVDictionary *dict;
>      } u;
>  } SpecifierOpt;
> 
> @@ -157,6 +158,7 @@ typedef struct OptionDef {
>  #define OPT_DOUBLE 0x20000
>  #define OPT_INPUT  0x40000
>  #define OPT_OUTPUT 0x80000
> +#define OPT_DICT  0x100000
>       union {
>          void *dst_ptr;
>          int (*func_arg)(void *, const char *, const char *);
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 97f14b2a5b..cc038aae6b 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -62,6 +62,7 @@
>  #define SPECIFIER_OPT_FMT_ui64 "%"PRIu64
>  #define SPECIFIER_OPT_FMT_f    "%f"
>  #define SPECIFIER_OPT_FMT_dbl  "%lf"
> +#define SPECIFIER_OPT_FMT_dict "%p"

The error message which uses this will make no sense. You should modify 
WARN_MULTIPLE_OPT_USAGE to make something sensible out of a dict.

E.g. make SPECIFIER_OPT_FMT_dict "%s" and create 
#define SPECIFIER_OPT_FUNC_dict as a dumper function. (the dumper function 
of other types can be #defined as identities)

Regards,
Marton