[FFmpeg-devel] [PATCH 3/5] avutil/opt: Add search flag to search children after the current object

[Nicolas George]

Andreas Rheinhardt (12021-09-15):
> The current way searches children first which is odd and not always
> intended.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> ---
> Missing APIchanges entry and version bump.
> 
>  libavutil/opt.c | 12 ++++++++++++
>  libavutil/opt.h |  7 +++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/libavutil/opt.c b/libavutil/opt.c
> index 41284d4ecd..de06728cd1 100644
> --- a/libavutil/opt.c
> +++ b/libavutil/opt.c
> @@ -1679,6 +1679,11 @@ const AVOption *av_opt_find2(void *obj, const char *name, const char *unit,
>          return NULL;
>  
>      if (search_flags & AV_OPT_SEARCH_CHILDREN) {
> +        /* Searching children both last and first makes no sense. */
> +        if (search_flags & AV_OPT_SEARCH_CHILDREN_AFTER_ME)
> +            return NULL;
> +

> +search_children:

No gotos for anything less obvious than cleanup after error, please. Yes
it makes this patch simpler, but it will make the next ones much harder.
I can see a few reasonable ways of avoiding it, I do not know which one
you would prefer. The cleanest would probably be to split searching in
the children in a separate function, and call it conditionally once at
the beginning and once at the end.

Also, would there be a drawback in making it the default? If I
understand correctly, you said there are currently no meaningful
collisions, so we can choose now without breaking anything. If we
consider that children are usually more generic code, it makes sense to
search them later by default.

>          if (search_flags & AV_OPT_SEARCH_FAKE_OBJ) {
>              void *iter = NULL;
>              const AVClass *child;
> @@ -1691,6 +1696,11 @@ const AVOption *av_opt_find2(void *obj, const char *name, const char *unit,
>                  if (o = av_opt_find2(child, name, unit, opt_flags, search_flags, target_obj))
>                      return o;
>          }
> +        /* If the AV_OPT_SEARCH_CHILDREN_AFTER_ME flag is set,
> +         * then we have already unsuccesfully checked our own options
> +         * and it is certain that this option is unrecognized. */
> +        if (search_flags & AV_OPT_SEARCH_CHILDREN_AFTER_ME)
> +            return NULL;
>      }
>  
>      while (o = av_opt_next(obj, o)) {
> @@ -1706,6 +1716,8 @@ const AVOption *av_opt_find2(void *obj, const char *name, const char *unit,
>              return o;
>          }
>      }
> +    if (search_flags & AV_OPT_SEARCH_CHILDREN_AFTER_ME)
> +        goto search_children;
>      return NULL;
>  }
>  
> diff --git a/libavutil/opt.h b/libavutil/opt.h
> index 2820435eec..5ce4a8d03d 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -558,6 +558,13 @@ int av_opt_eval_q     (void *obj, const AVOption *o, const char *val, AVRational
>  
>  #define AV_OPT_SEARCH_CHILDREN   (1 << 0) /**< Search in possible children of the
>                                                 given object first. */
> +/**
> + * Search in possible children of the given object after having
> + * searched the options of the object itself.
> + * This flag must not be combined with AV_OPT_SEARCH_CHILDREN.
> + */
> +#define AV_OPT_SEARCH_CHILDREN_AFTER_ME   (1 << 3)
> +
>  /**
>   *  The obj passed to av_opt_find() is fake -- only a double pointer to AVClass
>   *  instead of a required pointer to a struct containing AVClass. This is

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/20210915/a6758c8d/attachment.sig>