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

* [FFmpeg-devel] [PATCH] lavu/opt: Mention that AVOptions is not reentrant
@ 2024-06-05 13:18 Andrew Sayers
  2024-06-05 13:34 ` Paul B Mahol
  0 siblings, 1 reply; 38+ messages in thread
From: Andrew Sayers @ 2024-06-05 13:18 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andrew Sayers

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.

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".

^ permalink raw reply	[flat|nested] 38+ messages in thread