[FFmpeg-devel] [PATCH] lavu/opt: enhance printing durations.

Clément Bœsch u at pkh.me
Mon Oct 26 21:44:36 CET 2015


On Mon, Oct 26, 2015 at 09:19:58PM +0100, Nicolas George wrote:
> Trim unneeded leading components and trailing zeros.
> Move the formating code in a separate function.
> Use the function also to format the default value, it was currently
> printed as plain integer, inconsistent to the way it is parsed.
> 
> Signed-off-by: Nicolas George <george at nsup.org>
> ---
>  libavutil/opt.c    | 48 +++++++++++++++++++++++++++++++++++++++++++-----
>  tests/ref/fate/opt |  8 ++++----
>  2 files changed, 47 insertions(+), 9 deletions(-)
> 
> 

> Note: While I am touching the options code, I have this crazy idea about
> replacing AV_OPT_TYPE_* that would allow to get out of escaping hell
> completely. I will not have time to act on it soon, but if someone wants to
> hear about it...

What escaping are you talking about?

> 
> 
> diff --git a/libavutil/opt.c b/libavutil/opt.c
> index da1eb16..4fd25ff 100644
> --- a/libavutil/opt.c
> +++ b/libavutil/opt.c
> @@ -26,6 +26,7 @@
>   */
>  
>  #include "avutil.h"
> +#include "avassert.h"
>  #include "avstring.h"
>  #include "channel_layout.h"
>  #include "common.h"
> @@ -639,6 +640,41 @@ int av_opt_set_dict_val(void *obj, const char *name, const AVDictionary *val, in
>      return 0;
>  }
>  
> +static void format_duration(char *buf, size_t size, int64_t d)
> +{
> +    char *e;
> +
> +    av_assert0(size >= 25);
> +    if (d < 0 && d != INT64_MIN) {
> +        *(buf++) = '-';
> +        size--;
> +        d = -d;
> +    }
> +    if (d == INT64_MAX)
> +        snprintf(buf, size, "INT64_MAX");
> +    else if (d == INT64_MIN)
> +        snprintf(buf, size, "INT64_MIN");
> +    else if (d > (int64_t)3600*1000000)
> +        snprintf(buf, size, "%"PRId64":%02d:%02d.%06d", d / 3600000000,
> +                 (int)((d / 60000000) % 60),
> +                 (int)((d / 1000000) % 60),
> +                 (int)(d % 1000000));
> +    else if (d > 60*1000000)
> +        snprintf(buf, size, "%d:%02d.%06d",
> +                 (int)(d / 60000000),
> +                 (int)((d / 1000000) % 60),
> +                 (int)(d % 1000000));
> +    else
> +        snprintf(buf, size, "%d.%06d",
> +                 (int)(d / 1000000),
> +                 (int)(d % 1000000));
> +    e = buf + strlen(buf);
> +    while (e > buf && e[-1] == '0')
> +        *(--e) = 0;
> +    if (e > buf && e[-1] == '.')
> +        *(--e) = 0;
> +}
> +
>  int av_opt_get(void *obj, const char *name, int search_flags, uint8_t **out_val)
>  {
>      void *dst, *target_obj;
> @@ -704,9 +740,8 @@ int av_opt_get(void *obj, const char *name, int search_flags, uint8_t **out_val)
>          break;
>      case AV_OPT_TYPE_DURATION:
>          i64 = *(int64_t *)dst;
> -        ret = snprintf(buf, sizeof(buf), "%"PRIi64":%02d:%02d.%06d",
> -                       i64 / 3600000000, (int)((i64 / 60000000) % 60),
> -                       (int)((i64 / 1000000) % 60), (int)(i64 % 1000000));
> +        format_duration(buf, sizeof(buf), i64);
> +        ret = strlen(buf); // no overflow possible, checked by an assert
>          break;
>      case AV_OPT_TYPE_COLOR:
>          ret = snprintf(buf, sizeof(buf), "0x%02x%02x%02x%02x",
> @@ -1097,9 +1132,12 @@ static void opt_list(void *obj, void *av_log_obj, const char *unit,
>                  }
>                  break;
>              }
> -            case AV_OPT_TYPE_DURATION:
> -                log_value(av_log_obj, AV_LOG_INFO, opt->default_val.i64);
> +            case AV_OPT_TYPE_DURATION: {
> +                char buf[25];
> +                format_duration(buf, sizeof(buf), opt->default_val.i64);
> +                av_log(av_log_obj, AV_LOG_INFO, "%s", buf);
>                  break;
> +            }
>              case AV_OPT_TYPE_INT:
>              case AV_OPT_TYPE_INT64: {
>                  const char *def_const = get_opt_const_name(obj, opt->unit, opt->default_val.i64);
> diff --git a/tests/ref/fate/opt b/tests/ref/fate/opt
> index e9132a5..7b47d42 100644
> --- a/tests/ref/fate/opt
> +++ b/tests/ref/fate/opt
> @@ -31,7 +31,7 @@ TestContext AVOptions:
>    -pix_fmt           <pix_fmt>    E....... set pixfmt (default 0bgr)
>    -sample_fmt        <sample_fmt> E....... set samplefmt (default s16)
>    -video_rate        <video_rate> E....... set videorate (default "25")
> -  -duration          <duration>   E....... set duration (default 1000)
> +  -duration          <duration>   E....... set duration (default 0.001)
>    -color             <color>      E....... set color (default "pink")
>    -cl                <channel_layout> E....... set channel layout (default 0x137)
>    -bin               <binary>     E....... set binary value
> @@ -97,7 +97,7 @@ name:     bool2 default:1 error:
>  name:     bool3 default:1 error:
>  
>  Test av_opt_serialize()
> -num=0,toggle=1,rational=1/1,string=default,escape=\\\=\,,flags=0x00000001,size=200x300,pix_fmt=0bgr,sample_fmt=s16,video_rate=25/1,duration=0:00:00.001000,color=0xffc0cbff,cl=0x137,bin=62696E00,bin1=,bin2=,num64=1,flt=0.333333,dbl=0.333333,bool1=auto,bool2=true,bool3=false
> +num=0,toggle=1,rational=1/1,string=default,escape=\\\=\,,flags=0x00000001,size=200x300,pix_fmt=0bgr,sample_fmt=s16,video_rate=25/1,duration=0.001,color=0xffc0cbff,cl=0x137,bin=62696E00,bin1=,bin2=,num64=1,flt=0.333333,dbl=0.333333,bool1=auto,bool2=true,bool3=false
>  Setting entry with key 'num' to value '0'
>  Setting entry with key 'toggle' to value '1'
>  Setting entry with key 'rational' to value '1/1'
> @@ -108,7 +108,7 @@ Setting entry with key 'size' to value '200x300'
>  Setting entry with key 'pix_fmt' to value '0bgr'
>  Setting entry with key 'sample_fmt' to value 's16'
>  Setting entry with key 'video_rate' to value '25/1'
> -Setting entry with key 'duration' to value '0:00:00.001000'
> +Setting entry with key 'duration' to value '0.001'
>  Setting entry with key 'color' to value '0xffc0cbff'
>  Setting entry with key 'cl' to value '0x137'
>  Setting entry with key 'bin' to value '62696E00'
> @@ -120,7 +120,7 @@ Setting entry with key 'dbl' to value '0.333333'
>  Setting entry with key 'bool1' to value 'auto'
>  Setting entry with key 'bool2' to value 'true'
>  Setting entry with key 'bool3' to value 'false'
> -num=0,toggle=1,rational=1/1,string=default,escape=\\\=\,,flags=0x00000001,size=200x300,pix_fmt=0bgr,sample_fmt=s16,video_rate=25/1,duration=0:00:00.001000,color=0xffc0cbff,cl=0x137,bin=62696E00,bin1=,bin2=,num64=1,flt=0.333333,dbl=0.333333,bool1=auto,bool2=true,bool3=false
> +num=0,toggle=1,rational=1/1,string=default,escape=\\\=\,,flags=0x00000001,size=200x300,pix_fmt=0bgr,sample_fmt=s16,video_rate=25/1,duration=0.001,color=0xffc0cbff,cl=0x137,bin=62696E00,bin1=,bin2=,num64=1,flt=0.333333,dbl=0.333333,bool1=auto,bool2=true,bool3=false
>  
>  Testing av_set_options_string()
>  Setting options string ''

LGTM, thanks

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151026/a770f87b/attachment.sig>


More information about the ffmpeg-devel mailing list