[FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg: rewrite checking whether codec AVOptions have been used

[Marton Balint]

On Thu, 23 May 2024, Anton Khirnov wrote:

> Share the code between encoding and decoding. Instead of checking every
> stream's options dictionary (which is also used for other purposes),
> track all used options in a dedicated dictionary.
> ---
> fftools/cmdutils.c        | 17 ++++++++----
> fftools/cmdutils.h        |  4 ++-
> fftools/ffmpeg.c          | 49 ++++++++++++++++++++++++++++++++++
> fftools/ffmpeg.h          |  3 ++-
> fftools/ffmpeg_demux.c    | 50 ++++++++---------------------------
> fftools/ffmpeg_mux.c      |  1 +
> fftools/ffmpeg_mux.h      |  3 +++
> fftools/ffmpeg_mux_init.c | 55 +++++----------------------------------
> fftools/ffmpeg_opt.c      | 18 -------------
> fftools/ffplay.c          |  2 +-
> fftools/ffprobe.c         |  2 +-
> 11 files changed, 89 insertions(+), 115 deletions(-)
>

[...]

> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -493,6 +493,55 @@ int check_avoptions(AVDictionary *m)
>     return 0;
> }
>
> +int check_avoptions_used(const AVDictionary *opts, const AVDictionary *opts_used,
> +                         void *logctx, int decode)
> +{
> +    const AVClass  *class = avcodec_get_class();
> +    const AVClass *fclass = avformat_get_class();
> +
> +    const int flag = decode ? AV_OPT_FLAG_DECODING_PARAM :
> +                              AV_OPT_FLAG_ENCODING_PARAM;
> +    const AVDictionaryEntry *e = NULL;
> +
> +    while ((e = av_dict_iterate(opts, e))) {
> +        const AVOption *option, *foption;
> +        char *optname, *p;
> +
> +        optname = av_strdup(e->key);
> +        if (!optname)
> +            return AVERROR(ENOMEM);
> +
> +        p = strchr(optname, ':');
> +        if (p)
> +            *p = 0;
> +
> +        option = av_opt_find(&class, optname, NULL, 0,
> +                             AV_OPT_SEARCH_CHILDREN | AV_OPT_SEARCH_FAKE_OBJ);
> +        foption = av_opt_find(&fclass, optname, NULL, 0,
> +                              AV_OPT_SEARCH_CHILDREN | AV_OPT_SEARCH_FAKE_OBJ);
> +        av_freep(&optname);
> +        if (!option || foption)
> +            continue;
> +
> +        if (!(option->flags & flag)) {
> +            av_log(logctx, AV_LOG_ERROR, "Codec AVOption %s (%s) is not a %s "
> +                   "option.\n", option->name, option->help ? option->help : "",

Why the change of e->key to option->name? The full user specified option 
should be printed with specifier, so the user will know exactly which 
specifier matched the wrong stream type.

> +                   decode ? "decoding" : "encoding");
> +            return AVERROR(EINVAL);
> +        }
> +
> +        if (!av_dict_get(opts_used, e->key, NULL, 0)) {
> +            av_log(logctx, AV_LOG_WARNING, "Codec AVOption %s (%s) has not been used "
> +                   "for any stream. The most likely reason is either wrong type "
> +                   "(e.g. a video option with no video streams) or that it is a "
> +                   "private option of some decoder which was not actually used "
> +                   "for any stream.\n", option->name, option->help ? option->help : "");

Same here. The non-matching specifer should also be printed, not only the 
option name, so please keep using e->key in the warning message.

Thanks,
Marton