| Message ID | 20191206161818.25008-1-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
On Fri, 6 Dec 2019, Michael Niedermayer wrote: > Signed-off-by: Michael Niedermayer <michael@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. Regards, Marton
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 <michael@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() thats not optimal indeed ... Thanks [...]
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 <michael@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
On Tue, Dec 17, 2019 at 01:50:37AM +0100, Marton Balint wrote: > > > 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 <michael@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. will chnage the docs as in: @@ -97,9 +97,12 @@ 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 + * Track the presence of user provided 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 + * if you passed 5 functions with 2 arguments to av_expr_parse() + * then for arg=2 this will use upto 5 entries. * @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 thx [...]
On Tue, Dec 17, 2019 at 11:18:51AM +0100, Michael Niedermayer wrote: > On Tue, Dec 17, 2019 at 01:50:37AM +0100, Marton Balint wrote: > > > > > > 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 <michael@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. > > will chnage the docs as in: > @@ -97,9 +97,12 @@ 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 > + * Track the presence of user provided 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 > + * if you passed 5 functions with 2 arguments to av_expr_parse() > + * then for arg=2 this will use upto 5 entries. > * @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 will apply with these changes [...]
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); + /** * Free a parsed expression previously created with av_expr_parse(). */
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavutil/eval.c | 28 ++++++++++++++++++++-------- libavutil/eval.h | 11 +++++++++++ 2 files changed, 31 insertions(+), 8 deletions(-)