[FFmpeg-devel] [PATCHv2 1/3] avutil/eval: separate AVExpr state to a new AVExprState struct

Marton Balint cus at passwd.hu
Wed Jan 1 18:40:33 EET 2020



On Wed, 1 Jan 2020, Michael Niedermayer wrote:

> On Mon, Dec 30, 2019 at 11:23:40PM +0100, Marton Balint wrote:
>> Also add helper functions to allocate and free such a struct, and make it
>> usable by providing a new av_eval_expr2 function for which you can specify a
>> custom AVExprState.
>>
>> Signed-off-by: Marton Balint <cus at passwd.hu>
>> ---
>>  doc/APIchanges      |  4 ++++
>>  libavutil/eval.c    | 36 +++++++++++++++++++++++++-----------
>>  libavutil/eval.h    | 41 +++++++++++++++++++++++++++++++++++++++++
>>  libavutil/version.h |  2 +-
>>  4 files changed, 71 insertions(+), 12 deletions(-)
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index 3c24dc6fbc..d0b33bda02 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -15,6 +15,10 @@ libavutil:     2017-10-21
>>
>>  API changes, most recent first:
>>
>> +2020-01-xx - xxxxxxxxxx - lavu 56.39.100 - eval.h
>> +  Add AVExprState struct and av_expr_eval2, av_expr_state_alloc,
>> +  av_expr_state_free functions
>> +
>>  2019-12-27 - xxxxxxxxxx - lavu 56.38.100 - eval.h
>>    Add av_expr_count_func().
>>
>> diff --git a/libavutil/eval.c b/libavutil/eval.c
>> index d527f6a9d0..4619d0fba0 100644
>> --- a/libavutil/eval.c
>> +++ b/libavutil/eval.c
>> @@ -53,7 +53,6 @@ typedef struct Parser {
>>      void *opaque;
>>      int log_offset;
>>      void *log_ctx;
>> -#define VARS 10
>>      double *var;
>>  } Parser;
>>
>> @@ -173,7 +172,7 @@ struct AVExpr {
>>          double (*func2)(void *, double, double);
>>      } a;
>>      struct AVExpr *param[3];
>> -    double *var;
>> +    AVExprState *state;
>>  };
>>
>>  static double etime(double v)
>> @@ -191,7 +190,7 @@ static double eval_expr(Parser *p, AVExpr *e)
>>          case e_func2:  return e->value * e->a.func2(p->opaque, eval_expr(p, e->param[0]), eval_expr(p, e->param[1]));
>>          case e_squish: return 1/(1+exp(4*eval_expr(p, e->param[0])));
>>          case e_gauss: { double d = eval_expr(p, e->param[0]); return exp(-d*d/2)/sqrt(2*M_PI); }
>> -        case e_ld:     return e->value * p->var[av_clip(eval_expr(p, e->param[0]), 0, VARS-1)];
>> +        case e_ld:     return e->value * p->var[av_clip(eval_expr(p, e->param[0]), 0, AV_EXPR_STATE_NB_VARS-1)];
>>          case e_isnan:  return e->value * !!isnan(eval_expr(p, e->param[0]));
>>          case e_isinf:  return e->value * !!isinf(eval_expr(p, e->param[0]));
>>          case e_floor:  return e->value * floor(eval_expr(p, e->param[0]));
>> @@ -230,7 +229,7 @@ static double eval_expr(Parser *p, AVExpr *e)
>>              return x;
>>          }
>>          case e_random:{
>> -            int idx= av_clip(eval_expr(p, e->param[0]), 0, VARS-1);
>> +            int idx= av_clip(eval_expr(p, e->param[0]), 0, AV_EXPR_STATE_NB_VARS-1);
>>              uint64_t r= isnan(p->var[idx]) ? 0 : p->var[idx];
>>              r= r*1664525+1013904223;
>>              p->var[idx]= r;
>> @@ -245,7 +244,7 @@ static double eval_expr(Parser *p, AVExpr *e)
>>          case e_taylor: {
>>              double t = 1, d = 0, v;
>>              double x = eval_expr(p, e->param[1]);
>> -            int id = e->param[2] ? av_clip(eval_expr(p, e->param[2]), 0, VARS-1) : 0;
>> +            int id = e->param[2] ? av_clip(eval_expr(p, e->param[2]), 0, AV_EXPR_STATE_NB_VARS-1) : 0;
>>              int i;
>>              double var0 = p->var[id];
>>              for(i=0; i<1000; i++) {
>> @@ -320,7 +319,7 @@ static double eval_expr(Parser *p, AVExpr *e)
>>                  case e_div: return e->value * ((!CONFIG_FTRAPV || d2 ) ? (d / d2) : d * INFINITY);
>>                  case e_add: return e->value * (d + d2);
>>                  case e_last:return e->value * d2;
>> -                case e_st : return e->value * (p->var[av_clip(d, 0, VARS-1)]= d2);
>> +                case e_st : return e->value * (p->var[av_clip(d, 0, AV_EXPR_STATE_NB_VARS-1)]= d2);
>>                  case e_hypot:return e->value * hypot(d, d2);
>>                  case e_atan2:return e->value * atan2(d, d2);
>>                  case e_bitand: return isnan(d) || isnan(d2) ? NAN : e->value * ((long int)d & (long int)d2);
>> @@ -333,13 +332,23 @@ static double eval_expr(Parser *p, AVExpr *e)
>>
>>  static int parse_expr(AVExpr **e, Parser *p);
>>
>> +AVExprState *av_expr_state_alloc(void)
>> +{
>> +    return av_mallocz(sizeof(AVExprState));
>> +}
>> +
>> +void av_expr_state_free(AVExprState **ps)
>> +{
>> +    av_freep(ps);
>> +}
>> +
>>  void av_expr_free(AVExpr *e)
>>  {
>>      if (!e) return;
>>      av_expr_free(e->param[0]);
>>      av_expr_free(e->param[1]);
>>      av_expr_free(e->param[2]);
>> -    av_freep(&e->var);
>> +    av_expr_state_free(&e->state);
>>      av_freep(&e);
>>  }
>>
>> @@ -724,8 +733,8 @@ int av_expr_parse(AVExpr **expr, const char *s,
>>          ret = AVERROR(EINVAL);
>>          goto end;
>>      }
>> -    e->var= av_mallocz(sizeof(double) *VARS);
>> -    if (!e->var) {
>> +    e->state = av_expr_state_alloc();
>> +    if (!e->state) {
>>          ret = AVERROR(ENOMEM);
>>          goto end;
>>      }
>> @@ -763,16 +772,21 @@ int av_expr_count_func(AVExpr *e, unsigned *counter, int size, int arg)
>>      return expr_count(e, counter, size, ((int[]){e_const, e_func1, e_func2})[arg]);
>>  }
>>
>> -double av_expr_eval(AVExpr *e, const double *const_values, void *opaque)
>> +double av_expr_eval2(AVExpr *e, AVExprState *s, const double *const_values, void *opaque)
>>  {
>>      Parser p = { 0 };
>> -    p.var= e->var;
>> +    p.var = s ? s->vars : e->state->vars;
>>
>>      p.const_values = const_values;
>>      p.opaque     = opaque;
>>      return eval_expr(&p, e);
>>  }
>>
>> +double av_expr_eval(AVExpr *e, const double *const_values, void *opaque)
>> +{
>> +    return av_expr_eval2(e, NULL, const_values, opaque);
>> +}
>> +
>>  int av_expr_parse_and_eval(double *d, const char *s,
>>                             const char * const *const_names, const double *const_values,
>>                             const char * const *func1_names, double (* const *funcs1)(void *, double),
>> diff --git a/libavutil/eval.h b/libavutil/eval.h
>> index 068c62cdab..d5b34e54e8 100644
>> --- a/libavutil/eval.h
>> +++ b/libavutil/eval.h
>> @@ -30,6 +30,21 @@
>>
>>  typedef struct AVExpr AVExpr;
>>
>> +/**
>> + * This structure stores a state used by the expression evaluator.
>> + * It must be allocated and freed with its proper allocation functions.
>> + *
>> + * @see av_expr_state_alloc()
>> + * @see av_expr_state_free()
>> + */
>> +typedef struct AVExprState {
>
>> +#define AV_EXPR_STATE_NB_VARS 10
>
> This would make the NB_VARS part of the public API and it would
> not be changeable easily, theres nothing here that says it cannot be used
> by the user

I assumed it is OK as long as you only increase this. If you are adding 
other fields, then you are of course locked to 10 vars. What do you 
propose instead?

Thanks,
Marton


More information about the ffmpeg-devel mailing list