On Mon, 26 Feb 2024, Anton Khirnov wrote:
Quoting Marton Balint (2024-02-23 20:05:06)
On Fri, 23 Feb 2024, Anton Khirnov wrote:
AVOption.array_max_size is added before AVOption.unit to avoid
increasing sizeof(AVOption).
---
doc/APIchanges | 3 +
libavutil/opt.c | 344 ++++++++++++++++++++++++++++++++++++------
libavutil/opt.h | 26 ++++
libavutil/tests/opt.c | 34 +++++
tests/ref/fate/opt | 23 ++-
5 files changed, 385 insertions(+), 45 deletions(-)
[...]
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -288,6 +288,16 @@ enum AVOptionType{
*/
#define AV_OPT_FLAG_CHILD_CONSTS (1 << 18)
+/**
+ * The option is an array.
+ *
+ * When adding array options to an object, @ref AVOption.offset should refer to
+ * a pointer corresponding to the option type. The pointer should be
immediately
+ * followed by an unsigned int that will store the number of elements in the
+ * array.
+ */
+#define AV_OPT_FLAG_ARRAY (1 << 19)
+
/**
* AVOption
*/
@@ -313,6 +323,16 @@ typedef struct AVOption {
union {
int64_t i64;
double dbl;
+
+ /**
+ * This member is always used for AV_OPT_FLAG_ARRAY options. When
+ * non-NULL, the first character of the string must be the separator to
+ * be used for (de)serializing lists to/from strings with av_opt_get(),
This is quite ugly. Also it breaks the assumption that if the user sets an
option value to the default value of the option, than it will work.
I don't follow, what assumption are you talking about?
The more I think about it, it is actually a broader problem.
You are changing the semantics of existing AV_OPT_TYPE_xxx types. So
previously an option with AV_OPT_TYPE_STRING used to have default value in
default_val.str. After your patch, it will be either default_val.str, or
default_val.str[1], based on if it is an array or not.
I think the API user safely assumed that if the option type is known to
him, he will always find the default value in the relevant default_val
field. It is actually a bigger issue for an array of AV_OPT_TYPE_INT,
because previously to get the default value AVOption->default_val.i64 was
used, and now .str[1] should be instead...
The way I see it, the ARRAY modifier should either be a flag/mask of the
option type (not AVOption->flags), or it should be a entierly new type,
AV_OPT_TYPE_ARRAY, and its base type should be stored elsewhere.
For new opt types, we can define the semantics of default_val as we want,
so Andreas's suggestion is good to make default_val point to an
AVArrayOptionSettings struct or something like that.
Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
