Re: [FFmpeg-devel] [PATCH 1/3] avutil/eval: Add av_expr_count_func() similar to av_expr_count_vars()

[Marton Balint]

On Tue, 17 Dec 2019, Michael Niedermayer wrote:

On Sun, Dec 15, 2019 at 01:59:23PM +0100, Marton Balint wrote:


On Fri, 6 Dec 2019, Michael Niedermayer wrote:

Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
---
libavutil/eval.c | 28 ++++++++++++++++++++--------
libavutil/eval.h | 11 +++++++++++
2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/libavutil/eval.c b/libavutil/eval.c
index 62d2ae938b..d527f6a9d0 100644
--- a/libavutil/eval.c
+++ b/libavutil/eval.c
@@ -166,8 +166,8 @@ struct AVExpr {
       e_sgn,
   } type;
   double value; // is sign in other types
+    int const_index;
   union {
-        int const_index;
       double (*func0)(double);
       double (*func1)(void *, double);
       double (*func2)(void *, double, double);
@@ -185,7 +185,7 @@ static double eval_expr(Parser *p, AVExpr *e)
{
   switch (e->type) {
       case e_value:  return e->value;
-        case e_const:  return e->value * p->const_values[e->a.const_index];
+        case e_const:  return e->value * p->const_values[e->const_index];
       case e_func0:  return e->value * e->a.func0(eval_expr(p, e->param[0]));
       case e_func1:  return e->value * e->a.func1(p->opaque, eval_expr(p, 
e->param[0]));
       case e_func2:  return e->value * e->a.func2(p->opaque, eval_expr(p, 
e->param[0]), eval_expr(p, e->param[1]));
@@ -367,7 +367,7 @@ static int parse_primary(AVExpr **e, Parser *p)
       if (strmatch(p->s, p->const_names[i])) {
           p->s+= strlen(p->const_names[i]);
           d->type = e_const;
-            d->a.const_index = i;
+            d->const_index = i;
           *e = d;
           return 0;
       }
@@ -478,6 +478,7 @@ static int parse_primary(AVExpr **e, Parser *p)
           if (strmatch(next, p->func1_names[i])) {
               d->a.func1 = p->funcs1[i];
               d->type = e_func1;
+                d->const_index = i;
               *e = d;
               return 0;
           }
@@ -487,6 +488,7 @@ static int parse_primary(AVExpr **e, Parser *p)
           if (strmatch(next, p->func2_names[i])) {
               d->a.func2 = p->funcs2[i];
               d->type = e_func2;
+                d->const_index = i;
               *e = d;
               return 0;
           }
@@ -735,22 +737,32 @@ end:
   return ret;
}

-int av_expr_count_vars(AVExpr *e, unsigned *counter, int size)
+static int expr_count(AVExpr *e, unsigned *counter, int size, int type)
{
   int i;

   if (!e || !counter || !size)
       return AVERROR(EINVAL);

-    for (i = 0; e->type != e_const && i < 3 && e->param[i]; i++)
-        av_expr_count_vars(e->param[i], counter, size);
+    for (i = 0; e->type != type && i < 3 && e->param[i]; i++)
+        expr_count(e->param[i], counter, size, type);

-    if (e->type == e_const && e->a.const_index < size)
-        counter[e->a.const_index]++;
+    if (e->type == type && e->const_index < size)
+        counter[e->const_index]++;

   return 0;
}

+int av_expr_count_vars(AVExpr *e, unsigned *counter, int size)
+{
+    return expr_count(e, counter, size, e_const);
+}
+
+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)
{
   Parser p = { 0 };
diff --git a/libavutil/eval.h b/libavutil/eval.h
index 9bdb10cca2..688c523fbe 100644
--- a/libavutil/eval.h
+++ b/libavutil/eval.h
@@ -96,6 +96,17 @@ double av_expr_eval(AVExpr *e, const double *const_values, 
void *opaque);
*/
int av_expr_count_vars(AVExpr *e, unsigned *counter, int size);

+/**
+ * Track the presence of functions and their number of occurrences in a parsed 
expression
+ *
+ * @param counter a zero-initialized array where the count of each function 
will be stored
+ * @param size size of array
+ * @param arg number of arguments the counted functions have
+ * @return 0 on success, a negative value indicates that no expression or 
array was passed
+ * or size was zero
+ */
+int av_expr_count_func(AVExpr *e, unsigned *counter, int size, int arg);
+

In order to define a function like this in public API you should change the
functions list enum in eval.c to become public API as well. Otherwise the
user would not know which function has which identifier. Also the number of
functions should also become public API.

The user would know which function is the i-th function in the array which
the user passed to the expression evaluator.
It indeed would not work with built in functions, thats the same though with
the constants counted by av_expr_count_vars()

I missed that only the user functions are counted. I guess it is fine then 
from an API perspective, but please emphasize this in the docs.

Thanks,
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".