[FFmpeg-devel] [PATCH] avfilter: allow floating point formatting in expr_int_format (vf_drawtext.c)

[Stefano Sabatini]

On date Saturday 2023-12-02 15:54:34 +0100, Lennart Klebl wrote:
> Since I was trying for quite some time to get proper floating point
> formatting of positive and negative numbers working with the existing
> drawtext/expr_int_format implementation, I added the f format specifier
> that is only applied when used in conjunction with a width.
> 
> 

> From 14c438a2bcf2ea17a6818ef5cdc6a7ee9ef30184 Mon Sep 17 00:00:00 2001
> From: "lennart.klebl at t-online.de" <lennart.klebl at t-online.de>
> Date: Sat, 2 Dec 2023 15:53:20 +0100
> Subject: [PATCH] avfilter: allow floating point formatting in expr_int_format
>  (vf_drawtext.c)
> 
> ---
>  doc/filters.texi          | 12 ++++++++----
>  libavfilter/vf_drawtext.c | 19 ++++++++++++++-----
>  2 files changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index de19d130cc..1b11124434 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -12807,14 +12807,18 @@ the constants @var{text_w} and @var{text_h} will have an undefined
>  value.
>  
>  @item expr_int_format, eif
> -Evaluate the expression's value and output as formatted integer.
> +Evaluate the expression's value and output as formatted integer or floating
> +point number.
>  
>  The first argument is the expression to be evaluated, just as for the @var{expr} function.
>  The second argument specifies the output format. Allowed values are @samp{x},
> - at samp{X}, @samp{d} and @samp{u}. They are treated exactly as in the
> + at samp{X}, @samp{d}, @samp{u}, and @samp{f}. They are treated exactly as in the
>  @code{printf} function.
> -The third parameter is optional and sets the number of positions taken by the output.
> -It can be used to add padding with zeros from the left.
> +

> +When specifying @samp{f}, the third parameter is required and specifies the
> +number of decimal places for floating point formatting. Otherwise, the third

I think we want to use a sane default when not specified, in this case
you can skip the specification of .N.

> +parameter is optional and sets the number of positions taken by the output. It
> +can be used to add padding with zeros from the left.
>  
>  @item gmtime
>  The time at which the filter is running, expressed in UTC.
> diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
> index c5477cbff1..be0490bbb3 100644
> --- a/libavfilter/vf_drawtext.c
> +++ b/libavfilter/vf_drawtext.c
> @@ -1359,6 +1359,7 @@ static int func_eval_expr_int_format(AVFilterContext *ctx, AVBPrint *bp,
>      int ret;
>      unsigned int positions = 0;
>      char fmt_str[30] = "%";
> +    char fmt_arg = argv[1][0];
>  
>      /*
>       * argv[0] expression to be converted to `int`
> @@ -1376,9 +1377,9 @@ static int func_eval_expr_int_format(AVFilterContext *ctx, AVBPrint *bp,
>          return ret;
>      }
>  
> -    if (!strchr("xXdu", argv[1][0])) {
> +    if (!strchr("xXduf", fmt_arg)) {
>          av_log(ctx, AV_LOG_ERROR, "Invalid format '%c' specified,"
> -                " allowed values: 'x', 'X', 'd', 'u'\n", argv[1][0]);
> +                " allowed values: 'x', 'X', 'd', 'u', 'f'\n", fmt_arg);
>          return AVERROR(EINVAL);
>      }
>  
> @@ -1400,14 +1401,22 @@ static int func_eval_expr_int_format(AVFilterContext *ctx, AVBPrint *bp,
>      }
>  #endif
>  

> +    if (fmt_arg == 'f' && argc < 3) fmt_arg = 'd';
> +

We should avoid this unexpected fallback, just skip the .N
specification in this case.

>      if (argc == 3)
> -        av_strlcatf(fmt_str, sizeof(fmt_str), "0%u", positions);
> -    av_strlcatf(fmt_str, sizeof(fmt_str), "%c", argv[1][0]);
> +        if (fmt_arg == 'f')
> +            av_strlcatf(fmt_str, sizeof(fmt_str), ".%u", positions);
> +        else
> +            av_strlcatf(fmt_str, sizeof(fmt_str), "0%u", positions);
> +    av_strlcatf(fmt_str, sizeof(fmt_str), "%c", fmt_arg);