[FFmpeg-devel] [PATCHv2 1/3] avutil/eval: separate AVExpr state to a new AVExprState struct
Michael Niedermayer
michael at niedermayer.cc
Thu Jan 9 23:56:52 EET 2020
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 <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?
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
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
Thanks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200109/19deb6b1/attachment.sig>
More information about the ffmpeg-devel
mailing list