On Fri, Jan 10, 2020 at 12:49:08AM +0100, Marton Balint wrote: > > > On Thu, 9 Jan 2020, Michael Niedermayer wrote: > > >On Wed, Jan 01, 2020 at 05:40:33PM +0100, 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 <c...@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? > > > >Well, AV_EXPR_STATE_NB_VARS should not be a value in a public header > >which does not contain a comment saying that it must not be used outside > >libavutil > > My intention was that it _is_ public, because it lets the user know the > length (or rather the minimum length) of the variable array. The user could > just as easily use FF_ARRAY_ELEMS(AVExprState->vars). > > > > >also the question is, can this struct be made opaque for the user ? > >if so we can change it in any way in the future, including making it > >non opaque if needed > > Well, in v1 the struct was opaque but you wanted to be able to initialize > the state and I preferred a non-opaque struct to do that instead of a > parameter list in _alloc(). > > I can revert to v1, but I still think using _alloc() is not the way to > initialize state. How do you want me to proceed?
Well First question is what is the state? * always an array of doubles ? then id say let the user decide the length for each instance of the state * something arbitrary extendible (int, byte, stacks, fifos, string, filehandles, sockets, arrays, UI elements, ...) that needs something better designed, an opaque struct and alloc/free/set/get function could be a minimlistic solution [...] Thanks -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB In a rich man's house there is no place to spit but his face. -- Diogenes of Sinope
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".