| Message ID | 20210607230414.612-26-dcnieho@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | avdevice (mostly dshow) enhancements | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
Diederick Niehorster (12021-06-08): > This function allows formatting an option value stored in a double (such as the min and max fields of an AVOption, or min_value and max_value of an AVOptionRange) properly, e.g. 1 for a AV_OPT_TYPE_PIXEL_FMT -> yuyv422. Useful when printing more info about an option than just its value. Usage will be shown in upcoming device_get_capabilities example. av_opt_get (body changed) still passes FATE. Please wrap this. > > Signed-off-by: Diederick Niehorster <dcnieho@gmail.com> > --- > libavutil/opt.c | 93 +++++++++++++++++++++++++++++++++++++-------- > libavutil/opt.h | 12 +++++- > libavutil/version.h | 2 +- > 3 files changed, 89 insertions(+), 18 deletions(-) > > diff --git a/libavutil/opt.c b/libavutil/opt.c > index ab127b30fa..3e385852eb 100644 > --- a/libavutil/opt.c > +++ b/libavutil/opt.c > @@ -776,24 +776,14 @@ static void format_duration(char *buf, size_t size, int64_t d) > *(--e) = 0; > } > > -int av_opt_get(void *obj, const char *name, int search_flags, uint8_t **out_val) > +static int print_option(void* dst, enum AVOptionType type, int search_flags, uint8_t** out_val) I really do not like the fact that it always makes a dynamic allocation, especially since in most cases we know the buffer has a small bounded size. I'd rather have AVWriter here, but in the meantime, please consider using AVBPrint. > { > - void *dst, *target_obj; > - const AVOption *o = av_opt_find2(obj, name, NULL, 0, search_flags, &target_obj); > - uint8_t *bin, buf[128]; > + uint8_t* bin, buf[128]; Here and everywhere, the pointer mark belongs with the variable name. This is an obvious example of why: written like this, it seems that bin and buf[128] are both uint8_t*. Correctly written, it's obvious that only bin is a pointer. > int len, i, ret; > int64_t i64; > > - if (!o || !target_obj || (o->offset<=0 && o->type != AV_OPT_TYPE_CONST)) > - return AVERROR_OPTION_NOT_FOUND; > - > - if (o->flags & AV_OPT_FLAG_DEPRECATED) > - av_log(obj, AV_LOG_WARNING, "The \"%s\" option is deprecated: %s\n", name, o->help); > - > - dst = (uint8_t *)target_obj + o->offset; > - > buf[0] = 0; > - switch (o->type) { > + switch (type) { > case AV_OPT_TYPE_BOOL: > ret = snprintf(buf, sizeof(buf), "%s", (char *)av_x_if_null(get_bool_name(*(int *)dst), "invalid")); > break; > @@ -819,9 +809,6 @@ int av_opt_get(void *obj, const char *name, int search_flags, uint8_t **out_val) > case AV_OPT_TYPE_RATIONAL: > ret = snprintf(buf, sizeof(buf), "%d/%d", ((AVRational *)dst)->num, ((AVRational *)dst)->den); > break; > - case AV_OPT_TYPE_CONST: > - ret = snprintf(buf, sizeof(buf), "%f", o->default_val.dbl); > - break; I think you should have left the case here. > case AV_OPT_TYPE_STRING: > if (*(uint8_t **)dst) { > *out_val = av_strdup(*(uint8_t **)dst); > @@ -889,6 +876,80 @@ int av_opt_get(void *obj, const char *name, int search_flags, uint8_t **out_val) > return *out_val ? 0 : AVERROR(ENOMEM); > } > > +int av_opt_get(void *obj, const char *name, int search_flags, uint8_t **out_val) > +{ > + void *dst, *target_obj; > + const AVOption *o = av_opt_find2(obj, name, NULL, 0, search_flags, &target_obj); > + uint8_t buf[128]; > + int ret; > + > + if (!o || !target_obj || (o->offset<=0 && o->type != AV_OPT_TYPE_CONST)) > + return AVERROR_OPTION_NOT_FOUND; > + > + if (o->flags & AV_OPT_FLAG_DEPRECATED) > + av_log(obj, AV_LOG_WARNING, "The \"%s\" option is deprecated: %s\n", name, o->help); > + > + dst = (uint8_t *)target_obj + o->offset; > + > + > + if (o->type != AV_OPT_TYPE_CONST) > + return print_option(dst, o->type, search_flags, out_val); > + > + // special case for a const > + ret = snprintf(buf, sizeof(buf), "%f", o->default_val.dbl); > + if (ret >= sizeof(buf)) > + return AVERROR(EINVAL); > + *out_val = av_strdup(buf); > + return *out_val ? 0 : AVERROR(ENOMEM); > +} > +int av_opt_to_string(double val, enum AVOptionType type, uint8_t** out_val) > +{ > + *out_val = NULL; > + > + switch (type) { > + case AV_OPT_TYPE_FLAGS: > + case AV_OPT_TYPE_BOOL: > + case AV_OPT_TYPE_INT: > + case AV_OPT_TYPE_PIXEL_FMT: > + case AV_OPT_TYPE_SAMPLE_FMT: > + { > + int temp = lrint(val); > + return print_option(&temp, type, 0, out_val); > + } > + case AV_OPT_TYPE_INT64: > + case AV_OPT_TYPE_DURATION: > + case AV_OPT_TYPE_CHANNEL_LAYOUT: > + { > + int64_t temp = llrint(val); > + return print_option(&temp, type, 0, out_val); > + } > + case AV_OPT_TYPE_UINT64: > + { > + uint64_t temp = llrint(val); > + return print_option(&temp, type, 0, out_val); > + } > + case AV_OPT_TYPE_FLOAT: > + { > + float temp = (float)val; > + return print_option(&temp, type, 0, out_val); > + } > + case AV_OPT_TYPE_DOUBLE: > + return print_option(&val, type, 0, out_val); > + > + default: > + // AV_OPT_TYPE_DICT, > + // AV_OPT_TYPE_COLOR, > + // AV_OPT_TYPE_BINARY, > + // AV_OPT_TYPE_STRING, > + // AV_OPT_TYPE_RATIONAL, > + // AV_OPT_TYPE_IMAGE_SIZE, > + // AV_OPT_TYPE_VIDEO_RATE, > + // AV_OPT_TYPE_CONST > + // cannot be stored in a single double, and are thus not a valid input > + return AVERROR(EINVAL); > + } > +} > + > static int get_number(void *obj, const char *name, const AVOption **o_out, double *num, int *den, int64_t *intnum, > int search_flags) > { > diff --git a/libavutil/opt.h b/libavutil/opt.h > index ac0b4567a6..bd386c411b 100644 > --- a/libavutil/opt.h > +++ b/libavutil/opt.h > @@ -361,7 +361,7 @@ typedef struct AVOptionRanges { > * for (range_index = 0; range_index < ranges->nb_ranges; range_index++) { > * for (component_index = 0; component_index < ranges->nb_components; component_index++) > * range[component_index] = ranges->range[ranges->nb_ranges * component_index + range_index]; > - * //do something with range here. > + * //do something with range here. For printing the value, av_opt_to_string() may be of use. > * } > * av_opt_freep_ranges(&ranges); > * @endcode > @@ -760,6 +760,16 @@ int av_opt_get_channel_layout(void *obj, const char *name, int search_flags, int > * be freed with av_dict_free() by the caller > */ > int av_opt_get_dict_val(void *obj, const char *name, int search_flags, AVDictionary **out_val); > +/** Empty line please. > + * Format option value and output to string. > + * @param[in] val an option value that can be represented as a double. > + * @param[in] type of the option. > + * @param[out] out_val value of the option will be written here > + * @return >=0 on success, a negative error code otherwise > + * > + * @note the returned string will be av_malloc()ed and must be av_free()ed by the caller > + */ > +int av_opt_to_string(double val, enum AVOptionType type, uint8_t **out_val); If it only works for number-like options, then the name should reflect it, and the documentation should state it. Also, I have reservations about using double here: large integers cannot be accurately coded with double. > /** > * @} > */ > diff --git a/libavutil/version.h b/libavutil/version.h > index 34b83112de..77ca4becd6 100644 > --- a/libavutil/version.h > +++ b/libavutil/version.h > @@ -79,7 +79,7 @@ > */ > > #define LIBAVUTIL_VERSION_MAJOR 57 > -#define LIBAVUTIL_VERSION_MINOR 1 > +#define LIBAVUTIL_VERSION_MINOR 2 > #define LIBAVUTIL_VERSION_MICRO 100 > > #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \ Regards,
On Tue, Jun 8, 2021 at 4:32 PM Nicolas George <george@nsup.org> wrote: > > Diederick Niehorster (12021-06-08): > Please wrap this. Will do. > > -int av_opt_get(void *obj, const char *name, int search_flags, uint8_t **out_val) > > +static int print_option(void* dst, enum AVOptionType type, int search_flags, uint8_t** out_val) > > I really do not like the fact that it always makes a dynamic allocation, > especially since in most cases we know the buffer has a small bounded > size. > > I'd rather have AVWriter here, but in the meantime, please consider > using AVBPrint. The guts of this function is unchanged from av_opt_get, just moved here, should i address this in this patch, or is it a separate issue? Happy to do either. > > { > > - void *dst, *target_obj; > > - const AVOption *o = av_opt_find2(obj, name, NULL, 0, search_flags, &target_obj); > > - uint8_t *bin, buf[128]; > > > + uint8_t* bin, buf[128]; > > Here and everywhere, the pointer mark belongs with the variable name. > This is an obvious example of why: written like this, it seems that bin > and buf[128] are both uint8_t*. Correctly written, it's obvious that > only bin is a pointer. Absolutely, auto-formatter got me. > > - case AV_OPT_TYPE_CONST: > > - ret = snprintf(buf, sizeof(buf), "%f", o->default_val.dbl); > > - break; > > I think you should have left the case here. ok > > int av_opt_get_dict_val(void *obj, const char *name, int search_flags, AVDictionary **out_val); > > +/** > > Empty line please. done. > > + * Format option value and output to string. > > + * @param[in] val an option value that can be represented as a double. > > + * @param[in] type of the option. > > + * @param[out] out_val value of the option will be written here > > + * @return >=0 on success, a negative error code otherwise > > + * > > + * @note the returned string will be av_malloc()ed and must be av_free()ed by the caller > > + */ > > +int av_opt_to_string(double val, enum AVOptionType type, uint8_t **out_val); > > If it only works for number-like options, then the name should reflect > it, and the documentation should state it. > > Also, I have reservations about using double here: large integers cannot > be accurately coded with double. I understand your concerns. Perhaps its good to think about the use case (which is more narrow than the current name suggests): av_opt_query_ranges and the device capabilities API return a bunch of AVOptionRange, which contains the fields: double value_min, value_max; I need a function to format these properly (e.g. "yuyv422" instead of 1), and that does not take a AVOption as input, since these min and max values are not stored in an AVOption (and this new function could be used for formatting the values stored in the fields double min and double max in an AVOption as well). Hence the input to the function is double. I agree thats not ideal, nor is it great that values are stored as doubles in a AVOptionRange, since that limits its use to things representable as double (which arguably anything with a range is, logically, but as you said, double can't represent everything). My ideal solution would be to change the following def in AVOption: union { int64_t i64; double dbl; const char *str; /* TODO those are unused now */ AVRational q; } default_val; into a named union, and use that for the default_val of an AVOption, and for AVOptionRange's value_min and value_max. (and possibly also for AVOption's min and max fields, but that may be too big a change). Thats a significant API change, but AVOptionRange is hardly used in the ffmpeg code base (I have no idea about user code, but since they're hardly exposed, i'd expect the same?), but would allow expressing ranges precisely, regardless of type. That would make a more general to_string function possible as well. Anyway, I'd be pretty happy with the solution of just adding a function with a restricted use case and better name. What do you think? Thanks and all the best, Dee
Diederick C. Niehorster (12021-06-08): > The guts of this function is unchanged from av_opt_get, just moved > here, should i address this in this patch, or is it a separate issue? > Happy to do either. The critical part is the API of the new public function, because this we cannot fix later. Unfortunately, to get a good API, you will not be able to reuse the implementation of av_opt_get() as it is. Therefore, you will probably need two changes: (1) change the implementation of av_opt_get() and (2) add the new function. Whether this should be done in one or two patches depends on the specifics, on the amount of code in each change and how much they cover each other. > > > +int av_opt_to_string(double val, enum AVOptionType type, uint8_t **out_val); > > > > If it only works for number-like options, then the name should reflect > > it, and the documentation should state it. > > > > Also, I have reservations about using double here: large integers cannot > > be accurately coded with double. > > I understand your concerns. Perhaps its good to think about the use > case (which is more narrow than the current name suggests): > av_opt_query_ranges and the device capabilities API return a bunch of > AVOptionRange, which contains the fields: > double value_min, value_max; > I need a function to format these properly (e.g. "yuyv422" instead of > 1), and that does not take a AVOption as input, since these min and > max values are not stored in an AVOption (and this new function could > be used for formatting the values stored in the fields double min and > double max in an AVOption as well). Hence the input to the function is > double. > > I agree thats not ideal, nor is it great that values are stored as > doubles in a AVOptionRange, since that limits its use to things > representable as double (which arguably anything with a range is, > logically, but as you said, double can't represent everything). My > ideal solution would be to change the following def in AVOption: I see the issue. Since we have a double, we must do with a double. > union { > int64_t i64; > double dbl; > const char *str; > /* TODO those are unused now */ > AVRational q; > } default_val; > > into a named union, and use that for the default_val of an AVOption, > and for AVOptionRange's value_min and value_max. (and possibly also > for AVOption's min and max fields, but that may be too big a change). > Thats a significant API change, but AVOptionRange is hardly used in > the ffmpeg code base (I have no idea about user code, but since > they're hardly exposed, i'd expect the same?), but would allow > expressing ranges precisely, regardless of type. That would make a > more general to_string function possible as well. I have long-term projects of rewriting the options system, with each type (channel layouts, pixel formats, color ranges, etc.) coming with a descriptor for printing and parsing it, and a de-centralized way of adding new types. Unfortunately, first I need to convince people here that we need a better strings API. In the meantime: > Anyway, I'd be pretty happy with the solution of just adding a > function with a restricted use case and better name. What do you > think? In the meantime, as I suggested, I think using AVBPrint is the best option: void av_num_opt_bprint(AVBPrint *bp, AVOptionType type, double val); Regards,
(NB: I have reorganized your reply a bit to make it for me to respond to) On Wed, Jun 9, 2021 at 1:54 PM Nicolas George <george@nsup.org> wrote: > > The critical part is the API of the new public function, because this we > cannot fix later. > > Unfortunately, to get a good API, you will not be able to reuse the > implementation of av_opt_get() as it is. Therefore, you will probably > need two changes: (1) change the implementation of av_opt_get() and (2) > add the new function. > > Whether this should be done in one or two patches depends on the > specifics, on the amount of code in each change and how much they cover > each other. > > In the meantime, as I suggested, I think using AVBPrint is the best > option: > > void av_num_opt_bprint(AVBPrint *bp, AVOptionType type, double val); Ok, see if i understand this correctly. To use AVBPrint in this new API (which when reading up on it, seems like a good idea since it does something similar to C++'s std::string's small string optimization, and many string here will be very short, so no dynamic allocations), i would need to change the guts of av_opt_get() that i would like to reuse to take an AVBPrint. That would be pretty similar to the current uint8_t buf[128] currently used in the function, but remove the arbitrary (if rarely hit) size limit. What is the size of the internal buffer of AVBPrint? So in av_opt_get(), I'd do something like this: AVBPrint buf; av_bprint_init(&buf, 0, AV_BPRINT_SIZE_AUTOMATIC); // 1. call internal print function, with &buf // ... // 2. provide output to caller if (!av_bprint_is_complete(&buf)) { av_bprint_finalize(&buf,NULL); return AVERROR(ENOMEM); } ret = av_bprint_finalize(&buf,out_val); return ret<0 ? ret : 0; I'll experiment with that. > > I have long-term projects of rewriting the options system, with each > type (channel layouts, pixel formats, color ranges, etc.) coming with a > descriptor for printing and parsing it, and a de-centralized way of > adding new types. Unfortunately, first I need to convince people here > that we need a better strings API That sounds nice. Lets keep things here as they are then. Have you laid out your plans somewhere i can read about them? Cheers, Dee
On Thu, Jun 10, 2021 at 11:39 AM Diederick C. Niehorster <dcnieho@gmail.com> wrote: > So in av_opt_get(), I'd do something like this: > AVBPrint buf; > av_bprint_init(&buf, 0, AV_BPRINT_SIZE_AUTOMATIC); > // 1. call internal print function, with &buf > // ... > // 2. provide output to caller > if (!av_bprint_is_complete(&buf)) > { > av_bprint_finalize(&buf,NULL); > return AVERROR(ENOMEM); > } > ret = av_bprint_finalize(&buf,out_val); > return ret<0 ? ret : 0; > > I'll experiment with that. Ok, i have experimented with this, and hit some problems quickly. There are at least the following issues: 1. how to deal with AV_OPT_ALLOW_NULL in av_opt_get, if its guts use a move over to AVBPrint? leaving AVBPrint empty is not the same as a null, and does not signal null. 2. some cases forward to other functions, most importantly AV_OPT_TYPE_DICT, which uses av_dict_get_string. These do not play with AVBPrint, but just plain char** or similar. Should i use a local buffer in that case and then copy/print its contents into the AVBPrint? That seems to not improve the situation. This is assuming we'd want to reuse the guts of av_opt_get for this new function, which i think we would. Not doing so would mean a lot of code duplication. And there is also that this would be the only av_opt_* function returning output through a AVBPrint instead of plain arrays. Is that inconsistency in the API worth usually avoiding a dynamic allocation in code that'll not be used in a critical path? Cheers, Dee
On Thu, Jun 10, 2021 at 12:08 PM Diederick C. Niehorster <dcnieho@gmail.com> wrote: > > On Thu, Jun 10, 2021 at 11:39 AM Diederick C. Niehorster > <dcnieho@gmail.com> wrote: > > So in av_opt_get(), I'd do something like this: > > AVBPrint buf; > > av_bprint_init(&buf, 0, AV_BPRINT_SIZE_AUTOMATIC); > > // 1. call internal print function, with &buf > > // ... > > // 2. provide output to caller > > if (!av_bprint_is_complete(&buf)) > > { > > av_bprint_finalize(&buf,NULL); > > return AVERROR(ENOMEM); > > } > > ret = av_bprint_finalize(&buf,out_val); > > return ret<0 ? ret : 0; > > > > I'll experiment with that. > > Ok, i have experimented with this, and hit some problems quickly. > There are at least the following issues: > 1. how to deal with AV_OPT_ALLOW_NULL in av_opt_get, if its guts use a > move over to AVBPrint? leaving AVBPrint empty is not the same as a > null, and does not signal null. > 2. some cases forward to other functions, most importantly > AV_OPT_TYPE_DICT, which uses av_dict_get_string. These do not play > with AVBPrint, but just plain char** or similar. Should i use a local > buffer in that case and then copy/print its contents into the > AVBPrint? That seems to not improve the situation. > This is assuming we'd want to reuse the guts of av_opt_get for this > new function, which i think we would. Not doing so would mean a lot of > code duplication. > > And there is also that this would be the only av_opt_* function > returning output through a AVBPrint instead of plain arrays. Is that > inconsistency in the API worth usually avoiding a dynamic allocation > in code that'll not be used in a critical path? I've just sent v2 of the patch set. Given what i have written above, i have not moved the function to using AVBPrint, so i can reuse av_opt_get internals and don't introduce inconsistency in the options API. That said, moving it _all_ over to your better strings API proposal[1] sounds like a pretty sweet thing to do! I also hear you on your idea for better options. While the function i add here allows me to format pixel_format and sample_formats correctly because they have their own options type, I still have the issue that another option "codec" is just printed as int, as there is no AV_OPT_TYPE_CODEC, or otherwise information that it should be turned into a codec name... Hope that something like your proposal will make that a thing of the past. Cheers, Dee
diff --git a/libavutil/opt.c b/libavutil/opt.c index ab127b30fa..3e385852eb 100644 --- a/libavutil/opt.c +++ b/libavutil/opt.c @@ -776,24 +776,14 @@ static void format_duration(char *buf, size_t size, int64_t d) *(--e) = 0; } -int av_opt_get(void *obj, const char *name, int search_flags, uint8_t **out_val) +static int print_option(void* dst, enum AVOptionType type, int search_flags, uint8_t** out_val) { - void *dst, *target_obj; - const AVOption *o = av_opt_find2(obj, name, NULL, 0, search_flags, &target_obj); - uint8_t *bin, buf[128]; + uint8_t* bin, buf[128]; int len, i, ret; int64_t i64; - if (!o || !target_obj || (o->offset<=0 && o->type != AV_OPT_TYPE_CONST)) - return AVERROR_OPTION_NOT_FOUND; - - if (o->flags & AV_OPT_FLAG_DEPRECATED) - av_log(obj, AV_LOG_WARNING, "The \"%s\" option is deprecated: %s\n", name, o->help); - - dst = (uint8_t *)target_obj + o->offset; - buf[0] = 0; - switch (o->type) { + switch (type) { case AV_OPT_TYPE_BOOL: ret = snprintf(buf, sizeof(buf), "%s", (char *)av_x_if_null(get_bool_name(*(int *)dst), "invalid")); break; @@ -819,9 +809,6 @@ int av_opt_get(void *obj, const char *name, int search_flags, uint8_t **out_val) case AV_OPT_TYPE_RATIONAL: ret = snprintf(buf, sizeof(buf), "%d/%d", ((AVRational *)dst)->num, ((AVRational *)dst)->den); break; - case AV_OPT_TYPE_CONST: - ret = snprintf(buf, sizeof(buf), "%f", o->default_val.dbl); - break; case AV_OPT_TYPE_STRING: if (*(uint8_t **)dst) { *out_val = av_strdup(*(uint8_t **)dst); @@ -889,6 +876,80 @@ int av_opt_get(void *obj, const char *name, int search_flags, uint8_t **out_val) return *out_val ? 0 : AVERROR(ENOMEM); } +int av_opt_get(void *obj, const char *name, int search_flags, uint8_t **out_val) +{ + void *dst, *target_obj; + const AVOption *o = av_opt_find2(obj, name, NULL, 0, search_flags, &target_obj); + uint8_t buf[128]; + int ret; + + if (!o || !target_obj || (o->offset<=0 && o->type != AV_OPT_TYPE_CONST)) + return AVERROR_OPTION_NOT_FOUND; + + if (o->flags & AV_OPT_FLAG_DEPRECATED) + av_log(obj, AV_LOG_WARNING, "The \"%s\" option is deprecated: %s\n", name, o->help); + + dst = (uint8_t *)target_obj + o->offset; + + + if (o->type != AV_OPT_TYPE_CONST) + return print_option(dst, o->type, search_flags, out_val); + + // special case for a const + ret = snprintf(buf, sizeof(buf), "%f", o->default_val.dbl); + if (ret >= sizeof(buf)) + return AVERROR(EINVAL); + *out_val = av_strdup(buf); + return *out_val ? 0 : AVERROR(ENOMEM); +} +int av_opt_to_string(double val, enum AVOptionType type, uint8_t** out_val) +{ + *out_val = NULL; + + switch (type) { + case AV_OPT_TYPE_FLAGS: + case AV_OPT_TYPE_BOOL: + case AV_OPT_TYPE_INT: + case AV_OPT_TYPE_PIXEL_FMT: + case AV_OPT_TYPE_SAMPLE_FMT: + { + int temp = lrint(val); + return print_option(&temp, type, 0, out_val); + } + case AV_OPT_TYPE_INT64: + case AV_OPT_TYPE_DURATION: + case AV_OPT_TYPE_CHANNEL_LAYOUT: + { + int64_t temp = llrint(val); + return print_option(&temp, type, 0, out_val); + } + case AV_OPT_TYPE_UINT64: + { + uint64_t temp = llrint(val); + return print_option(&temp, type, 0, out_val); + } + case AV_OPT_TYPE_FLOAT: + { + float temp = (float)val; + return print_option(&temp, type, 0, out_val); + } + case AV_OPT_TYPE_DOUBLE: + return print_option(&val, type, 0, out_val); + + default: + // AV_OPT_TYPE_DICT, + // AV_OPT_TYPE_COLOR, + // AV_OPT_TYPE_BINARY, + // AV_OPT_TYPE_STRING, + // AV_OPT_TYPE_RATIONAL, + // AV_OPT_TYPE_IMAGE_SIZE, + // AV_OPT_TYPE_VIDEO_RATE, + // AV_OPT_TYPE_CONST + // cannot be stored in a single double, and are thus not a valid input + return AVERROR(EINVAL); + } +} + static int get_number(void *obj, const char *name, const AVOption **o_out, double *num, int *den, int64_t *intnum, int search_flags) { diff --git a/libavutil/opt.h b/libavutil/opt.h index ac0b4567a6..bd386c411b 100644 --- a/libavutil/opt.h +++ b/libavutil/opt.h @@ -361,7 +361,7 @@ typedef struct AVOptionRanges { * for (range_index = 0; range_index < ranges->nb_ranges; range_index++) { * for (component_index = 0; component_index < ranges->nb_components; component_index++) * range[component_index] = ranges->range[ranges->nb_ranges * component_index + range_index]; - * //do something with range here. + * //do something with range here. For printing the value, av_opt_to_string() may be of use. * } * av_opt_freep_ranges(&ranges); * @endcode @@ -760,6 +760,16 @@ int av_opt_get_channel_layout(void *obj, const char *name, int search_flags, int * be freed with av_dict_free() by the caller */ int av_opt_get_dict_val(void *obj, const char *name, int search_flags, AVDictionary **out_val); +/** + * Format option value and output to string. + * @param[in] val an option value that can be represented as a double. + * @param[in] type of the option. + * @param[out] out_val value of the option will be written here + * @return >=0 on success, a negative error code otherwise + * + * @note the returned string will be av_malloc()ed and must be av_free()ed by the caller + */ +int av_opt_to_string(double val, enum AVOptionType type, uint8_t **out_val); /** * @} */ diff --git a/libavutil/version.h b/libavutil/version.h index 34b83112de..77ca4becd6 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -79,7 +79,7 @@ */ #define LIBAVUTIL_VERSION_MAJOR 57 -#define LIBAVUTIL_VERSION_MINOR 1 +#define LIBAVUTIL_VERSION_MINOR 2 #define LIBAVUTIL_VERSION_MICRO 100 #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
This function allows formatting an option value stored in a double (such as the min and max fields of an AVOption, or min_value and max_value of an AVOptionRange) properly, e.g. 1 for a AV_OPT_TYPE_PIXEL_FMT -> yuyv422. Useful when printing more info about an option than just its value. Usage will be shown in upcoming device_get_capabilities example. av_opt_get (body changed) still passes FATE. Signed-off-by: Diederick Niehorster <dcnieho@gmail.com> --- libavutil/opt.c | 93 +++++++++++++++++++++++++++++++++++++-------- libavutil/opt.h | 12 +++++- libavutil/version.h | 2 +- 3 files changed, 89 insertions(+), 18 deletions(-)