Re: [FFmpeg-devel] [PATCH] lavu/opt: Mention that AVOptions is not reentrant

[Paul B Mahol <onemda@gmail.com>]

On Wed, Jun 5, 2024 at 3:18 PM Andrew Sayers <ffmpeg-devel@pileofstuff.org>
wrote:

> An external API developer might think they can use AVOptions to modify
> values
> during playback (e.g. putting a playback quality slider next to the volume
> slider).  Make it clear that behaviour is not recommended.
>

There are options that can be changed at runtime,
And it works just fine.


>
> Include the warning in the group description and the text for every
> function
> that sets options, but leave it implicit in functions that get options.
> This reflects the fact that *modifying* options is likely to cause weird
> bugs,
> while *reading* options isn't guaranteed but is likely to be safe.
> ---
>  libavutil/opt.h | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/libavutil/opt.h b/libavutil/opt.h
> index 07e27a9208..13292c6473 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -53,11 +53,16 @@
>   * question is allowed to access the field. This allows us to extend the
>   * semantics of those fields without breaking API compatibility.
>   *
> + * Note that AVOptions functions are not reentrant, and options may be
> accessed
> + * from internal FFmpeg threads.  Unless otherwise noted, it is best to
> avoid
> + * modifying options once a struct has been initialized.
> + *
>   * @section avoptions_scope Scope of AVOptions
>   *
>   * AVOptions is designed to support any set of multimedia configuration
> options
> - * that can be defined at compile-time.  Although it is mainly used to
> expose
> - * FFmpeg options, you are welcome to adapt it to your own use case.
> + * that can be defined at compile-time and set at object creation time.
> Although
> + * it is mainly used to expose FFmpeg options, you are welcome to adapt it
> + * to your own use case.
>   *
>   * No single approach can ever fully solve the problem of configuration,
>   * but please submit a patch if you believe you have found a problem
> @@ -483,6 +488,9 @@ typedef struct AVOptionRanges {
>  /**
>   * Set the values of all AVOption fields to their default values.
>   *
> + * Note: like all AVOptions functions, this is not reentrant.  Unless
> + * otherwise noted, it should only be used before initializing the struct.
> + *
>   * @param s an AVOption-enabled struct (its first member must be a
> pointer to AVClass)
>   */
>  void av_opt_set_defaults(void *s);
> @@ -492,6 +500,9 @@ void av_opt_set_defaults(void *s);
>   * AVOption fields for which (opt->flags & mask) == flags will have their
>   * default applied to s.
>   *
> + * Note: like all AVOptions functions, this is not reentrant.  Unless
> + * otherwise noted, it should only be used before initializing the struct.
> + *
>   * @param s an AVOption-enabled struct (its first member must be a
> pointer to AVClass)
>   * @param mask combination of AV_OPT_FLAG_*
>   * @param flags combination of AV_OPT_FLAG_*
> @@ -661,6 +672,9 @@ enum {
>   * key. ctx must be an AVClass context, storing is done using
>   * AVOptions.
>   *
> + * Note: like all AVOptions functions, this is not reentrant.  Unless
> + * otherwise noted, it should only be used before initializing the struct.
> + *
>   * @param opts options string to parse, may be NULL
>   * @param key_val_sep a 0-terminated list of characters used to
>   * separate key from value
> @@ -679,6 +693,9 @@ int av_set_options_string(void *ctx, const char *opts,
>   * Parse the key-value pairs list in opts. For each key=value pair found,
>   * set the value of the corresponding option in ctx.
>   *
> + * Note: like all AVOptions functions, this is not reentrant.  Unless
> + * otherwise noted, it should only be used before initializing the struct.
> + *
>   * @param ctx          the AVClass object to set options on
>   * @param opts         the options string, key-value pairs separated by a
>   *                     delimiter
> @@ -709,6 +726,9 @@ int av_opt_set_from_string(void *ctx, const char *opts,
>  /**
>   * Set all the options from a given dictionary on an object.
>   *
> + * Note: like all AVOptions functions, this is not reentrant.  Unless
> + * otherwise noted, it should only be used before initializing the struct.
> + *
>   * @param obj a struct whose first element is a pointer to AVClass
>   * @param options options to process. This dictionary will be freed and
> replaced
>   *                by a new one containing all options not found in obj.
> @@ -726,6 +746,9 @@ int av_opt_set_dict(void *obj, struct AVDictionary
> **options);
>  /**
>   * Set all the options from a given dictionary on an object.
>   *
> + * Note: like all AVOptions functions, this is not reentrant.  Unless
> + * otherwise noted, it should only be used before initializing the struct.
> + *
>   * @param obj a struct whose first element is a pointer to AVClass
>   * @param options options to process. This dictionary will be freed and
> replaced
>   *                by a new one containing all options not found in obj.
> @@ -764,6 +787,9 @@ int av_opt_copy(void *dest, const void *src);
>   * @{
>   * Those functions set the field of obj with the given name to value.
>   *
> + * Note: like all AVOptions functions, these are not reentrant.  Unless
> + * otherwise noted, they should only be used before initializing the
> struct.
> + *
>   * @param[in] obj A struct whose first element is a pointer to an AVClass.
>   * @param[in] name the name of the field to set
>   * @param[in] val The value to set. In case of av_opt_set() if the field
> is not
> --
> 2.45.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".