On Wed, Feb 10, 2016 at 9:47 AM, Anton Khirnov <[email protected]> wrote: > Quoting Hendrik Leppkes (2016-02-10 09:11:17) >> On Wed, Feb 10, 2016 at 9:01 AM, Anton Khirnov <[email protected]> wrote: >> > Quoting Hendrik Leppkes (2016-02-09 23:13:32) >> >> On Tue, Feb 9, 2016 at 6:54 PM, Anton Khirnov <[email protected]> wrote: >> >> > --- >> >> > doc/APIchanges | 5 +++++ >> >> > libavutil/opt.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ >> >> > libavutil/opt.h | 12 ++++++++++++ >> >> > libavutil/version.h | 2 +- >> >> > 4 files changed, 63 insertions(+), 1 deletion(-) >> >> > >> >> > diff --git a/doc/APIchanges b/doc/APIchanges >> >> > index 6f7a141..fdcbdf4 100644 >> >> > --- a/doc/APIchanges >> >> > +++ b/doc/APIchanges >> >> > @@ -13,6 +13,11 @@ libavutil: 2015-08-28 >> >> > >> >> > API changes, most recent first: >> >> > >> >> > +2016-xx-xx - lavu 55.6.0 >> >> > + xxxxxxx opt.h - Add AV_OPT_TYPE_BUFFERREF, av_opt_set_bufferref() and >> >> > + av_opt_get_bufferref() >> >> > + >> >> > + >> >> > 2016-xx-xx - xxxxxxx - lavf 57.3.0 - avformat.h >> >> > Add AVFormatContext.opaque, io_open and io_close, allowing custom IO >> >> > for muxers and demuxers that open additional files. >> >> > diff --git a/libavutil/opt.c b/libavutil/opt.c >> >> > index b3435e0..e5a0e92 100644 >> >> > --- a/libavutil/opt.c >> >> > +++ b/libavutil/opt.c >> >> > @@ -27,6 +27,7 @@ >> >> > >> >> > #include "avutil.h" >> >> > #include "avstring.h" >> >> > +#include "buffer.h" >> >> > #include "common.h" >> >> > #include "opt.h" >> >> > #include "eval.h" >> >> > @@ -327,6 +328,26 @@ int av_opt_set_dict_val(void *obj, const char >> >> > *name, const AVDictionary *val, in >> >> > return 0; >> >> > } >> >> > >> >> > +int av_opt_set_bufferref(void *obj, const char *name, AVBufferRef >> >> > *ref, int search_flags) >> >> > +{ >> >> > + void *target_obj; >> >> > + AVBufferRef **dst; >> >> > + const AVOption *o = av_opt_find2(obj, name, NULL, 0, search_flags, >> >> > &target_obj); >> >> > + >> >> > + if (!o || !target_obj) >> >> > + return AVERROR_OPTION_NOT_FOUND; >> >> > + if (o->type != AV_OPT_TYPE_BUFFERREF || o->flags & >> >> > AV_OPT_FLAG_READONLY) >> >> > + return AVERROR(EINVAL); >> >> > + >> >> > + dst = (AVBufferRef **)(((uint8_t *)target_obj) + o->offset); >> >> > + av_buffer_unref(dst); >> >> > + *dst = av_buffer_ref(ref); >> >> > + if (!*dst) >> >> > + return AVERROR(ENOMEM); >> >> > + >> >> > + return 0; >> >> > +} >> >> > + >> >> > int av_opt_get(void *obj, const char *name, int search_flags, uint8_t >> >> > **out_val) >> >> > { >> >> > void *dst, *target_obj; >> >> > @@ -447,6 +468,25 @@ int av_opt_get_dict_val(void *obj, const char >> >> > *name, int search_flags, AVDiction >> >> > return 0; >> >> > } >> >> > >> >> > +int av_opt_get_bufferref(void *obj, const char *name, int >> >> > search_flags, AVBufferRef **out_val) >> >> > +{ >> >> > + void *target_obj; >> >> > + AVBufferRef *src; >> >> > + const AVOption *o = av_opt_find2(obj, name, NULL, 0, search_flags, >> >> > &target_obj); >> >> > + >> >> > + if (!o || !target_obj) >> >> > + return AVERROR_OPTION_NOT_FOUND; >> >> > + if (o->type != AV_OPT_TYPE_BUFFERREF) >> >> > + return AVERROR(EINVAL); >> >> > + >> >> > + src = *(AVBufferRef **)(((uint8_t *)target_obj) + o->offset); >> >> > + *out_val = av_buffer_ref(src); >> >> > + if (!*out_val) >> >> > + return AVERROR(ENOMEM); >> >> > + >> >> > + return 0; >> >> > +} >> >> > + >> >> > int av_opt_flag_is_set(void *obj, const char *field_name, const char >> >> > *flag_name) >> >> > { >> >> > const AVOption *field = av_opt_find(obj, field_name, NULL, 0, 0); >> >> > @@ -577,6 +617,7 @@ void av_opt_set_defaults(void *s) >> >> > break; >> >> > case AV_OPT_TYPE_BINARY: >> >> > case AV_OPT_TYPE_DICT: >> >> > + case AV_OPT_TYPE_BUFFERREF: >> >> > /* Cannot set defaults for these types */ >> >> > break; >> >> > default: >> >> > @@ -670,6 +711,10 @@ void av_opt_free(void *obj) >> >> > av_dict_free((AVDictionary **)(((uint8_t *)obj) + >> >> > o->offset)); >> >> > break; >> >> > >> >> > + case AV_OPT_TYPE_BUFFERREF: >> >> > + av_buffer_unref((AVBufferRef **)(((uint8_t *)obj) + >> >> > o->offset)); >> >> > + break; >> >> > + >> >> > default: >> >> > break; >> >> > } >> >> > diff --git a/libavutil/opt.h b/libavutil/opt.h >> >> > index 8413206..713b49d 100644 >> >> > --- a/libavutil/opt.h >> >> > +++ b/libavutil/opt.h >> >> > @@ -29,6 +29,7 @@ >> >> > >> >> > #include "rational.h" >> >> > #include "avutil.h" >> >> > +#include "buffer.h" >> >> > #include "dict.h" >> >> > #include "log.h" >> >> > >> >> > @@ -225,6 +226,7 @@ enum AVOptionType{ >> >> > AV_OPT_TYPE_RATIONAL, >> >> > AV_OPT_TYPE_BINARY, ///< offset must point to a pointer >> >> > immediately followed by an int for the length >> >> > AV_OPT_TYPE_DICT, >> >> > + AV_OPT_TYPE_BUFFERREF, >> >> > AV_OPT_TYPE_CONST = 128, >> >> > }; >> >> > >> >> > @@ -503,6 +505,11 @@ int av_opt_set_bin (void *obj, const char >> >> > *name, const uint8_t *val, int siz >> >> > */ >> >> > int av_opt_set_dict_val(void *obj, const char *name, const >> >> > AVDictionary *val, int search_flags); >> >> > /** >> >> > + * @note This function will make a new reference to the underlying >> >> > AVBuffer. The >> >> > + * AVBufferRef passed to this function remains owned by the caller. >> >> > + */ >> >> > +int av_opt_set_bufferref(void *obj, const char *name, AVBufferRef >> >> > *ref, int search_flags); >> >> > +/** >> >> > * @} >> >> > */ >> >> > >> >> > @@ -531,6 +538,11 @@ int av_opt_get_q (void *obj, const char >> >> > *name, int search_flags, AVRationa >> >> > */ >> >> > int av_opt_get_dict_val(void *obj, const char *name, int search_flags, >> >> > AVDictionary **out_val); >> >> > /** >> >> > + * @note The returned reference is owned by the caller and must be >> >> > freed with >> >> > + * av_buffer_unref() when no longer needed. >> >> > + */ >> >> > +int av_opt_get_bufferref(void *obj, const char *name, int >> >> > search_flags, AVBufferRef **out_val); >> >> > +/** >> >> > * @} >> >> >> >> This is extremely specific, and I for one do not like the precedence >> >> this sets. Maybe AVOptions are not the right way to communicate >> >> complex objects? >> > >> > Do you suggest another way? >> > >> >> You mean, like, actual API that directly takes an AVBufferRef? > > That would be specific to the buffersrc filter? I fail to see how it is > better than this. Or what is wrong with this patch in the first place. >
I would take one specific API over starting a trend of adding so-called "generic" API for complex struct types. I already see someone using this as a precedence argument to add even more crazy option API for other structs. The way I see it, the opt.h API should be for actual configuration, ie. options, not a way to avoid creating proper APIs. - Hendrik _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
