diff mbox series

[FFmpeg-devel,25/35] avutil/opt: add av_opt_to_string

Message ID 20210607230414.612-26-dcnieho@gmail.com
State Superseded, archived
Headers show
Series avdevice (mostly dshow) enhancements
Related show

Checks

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

Commit Message

Diederick C. Niehorster June 7, 2021, 11:04 p.m. UTC
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(-)

Comments

Nicolas George June 8, 2021, 2:32 p.m. UTC | #1
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,
Diederick C. Niehorster June 8, 2021, 9:19 p.m. UTC | #2
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
Nicolas George June 9, 2021, 11:54 a.m. UTC | #3
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,
Diederick C. Niehorster June 10, 2021, 9:39 a.m. UTC | #4
(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
Diederick C. Niehorster June 10, 2021, 10:08 a.m. UTC | #5
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
Diederick C. Niehorster June 11, 2021, 8:47 p.m. UTC | #6
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 mbox series

Patch

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, \