On Sat, 11 Jan 2020, Michael Niedermayer wrote:

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

To me the second option seems more future proof, but I have no specific use case in mind.

Regards,
Marton
_______________________________________________
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".

Reply via email to