[FFmpeg-devel] [PATCHv2 1/3] avutil/eval: separate AVExpr state to a new AVExprState struct
Marton Balint
cus at passwd.hu
Thu Jan 9 01:41:24 EET 2020
On Wed, 1 Jan 2020, Marton Balint wrote:
>
>
> 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?
Ping for this.
Thanks,
Marton
More information about the ffmpeg-devel
mailing list