| Message ID | 72be1962-95c2-e216-ecb1-5414508e47f9@gyani.pro |
|---|---|
| State | Superseded |
| Headers | show |
On Sun, Nov 03, 2019 at 11:14:25AM +0530, Gyan wrote: > > Helps better identification of expr eval failures. > > Gyan > eval.c | 21 +++++++++++++++++++++ > eval.h | 10 ++++++++++ > version.h | 4 ++-- > 3 files changed, 33 insertions(+), 2 deletions(-) > 3dd142baa0144fd324eb9da8a9932cfd7ab2cd98 0001-avutil-eval-add-function-to-track-variable-use.patch > From 19bce329464676f071707b99575f80e5abe1cd4c Mon Sep 17 00:00:00 2001 > From: Gyan Doshi <ffmpeg@gyani.pro> > Date: Sat, 2 Nov 2019 20:16:42 +0530 > Subject: [PATCH] avutil/eval: add function to track variable use > > Helps avoid multiple evals of cross-referenced expressions > and catch the use of non-applicable variables with respect > to eval or special mode in filters Maybe you should provide more details of the use case of this, maybe with an example. Because it seems not completely obvious thx [...]
On 05-11-2019 04:35 am, Michael Niedermayer wrote: > On Sun, Nov 03, 2019 at 11:14:25AM +0530, Gyan wrote: >> Helps better identification of expr eval failures. >> >> Gyan >> eval.c | 21 +++++++++++++++++++++ >> eval.h | 10 ++++++++++ >> version.h | 4 ++-- >> 3 files changed, 33 insertions(+), 2 deletions(-) >> 3dd142baa0144fd324eb9da8a9932cfd7ab2cd98 0001-avutil-eval-add-function-to-track-variable-use.patch >> From 19bce329464676f071707b99575f80e5abe1cd4c Mon Sep 17 00:00:00 2001 >> From: Gyan Doshi <ffmpeg@gyani.pro> >> Date: Sat, 2 Nov 2019 20:16:42 +0530 >> Subject: [PATCH] avutil/eval: add function to track variable use >> >> Helps avoid multiple evals of cross-referenced expressions >> and catch the use of non-applicable variables with respect >> to eval or special mode in filters > Maybe you should provide more details of the use case of this, maybe > with an example. Because it seems not completely obvious An example of a cross-referenced expression would be 'y=x+rand(10)'. Normally in filters, X would be evaluated first, then Y, then X again. The 2nd eval of X will overwrite the first and can generate a different value. With this func, we can avoid the 2nd eval. I'm in the process of modifying the scale filter to allow 'n', 't', 'pos' variables in frame eval mode (it already supports dynamic output based on incoming frame changes) and I would like to catch their presence in init mode, since av_expr_eval will fail. Now, it can also fail due to circularly referenced expressions e.g. 'x=y+10' and 'y=x+10' and there's no way to detect whether it's this case or if inapplicable variables have been used. Regards, Gyan
On Tue, Nov 05, 2019 at 10:13:52AM +0530, Gyan wrote: > > > On 05-11-2019 04:35 am, Michael Niedermayer wrote: > >On Sun, Nov 03, 2019 at 11:14:25AM +0530, Gyan wrote: > >>Helps better identification of expr eval failures. > >> > >>Gyan > >> eval.c | 21 +++++++++++++++++++++ > >> eval.h | 10 ++++++++++ > >> version.h | 4 ++-- > >> 3 files changed, 33 insertions(+), 2 deletions(-) > >>3dd142baa0144fd324eb9da8a9932cfd7ab2cd98 0001-avutil-eval-add-function-to-track-variable-use.patch > >> From 19bce329464676f071707b99575f80e5abe1cd4c Mon Sep 17 00:00:00 2001 > >>From: Gyan Doshi <ffmpeg@gyani.pro> > >>Date: Sat, 2 Nov 2019 20:16:42 +0530 > >>Subject: [PATCH] avutil/eval: add function to track variable use > >> > >>Helps avoid multiple evals of cross-referenced expressions > >>and catch the use of non-applicable variables with respect > >>to eval or special mode in filters > >Maybe you should provide more details of the use case of this, maybe > >with an example. Because it seems not completely obvious > > An example of a cross-referenced expression would be 'y=x+rand(10)'. > Normally in filters, X would be evaluated first, then Y, then X again. The > 2nd eval of X will overwrite the first and can generate a different value. > With this func, we can avoid the 2nd eval. > > I'm in the process of modifying the scale filter to allow 'n', 't', 'pos' > variables in frame eval mode (it already supports dynamic output based on > incoming frame changes) and I would like to catch their presence in init > mode, since av_expr_eval will fail. Now, it can also fail due to circularly > referenced expressions e.g. 'x=y+10' and 'y=x+10' and there's no way to > detect whether it's this case or if inapplicable variables have been used. please add this (or similar information) to the commit message thx [...]
Gyan (12019-11-05): > An example of a cross-referenced expression would be 'y=x+rand(10)'. > Normally in filters, X would be evaluated first, then Y, then X again. The > 2nd eval of X will overwrite the first and can generate a different value. > With this func, we can avoid the 2nd eval. > > I'm in the process of modifying the scale filter to allow 'n', 't', 'pos' > variables in frame eval mode (it already supports dynamic output based on > incoming frame changes) and I would like to catch their presence in init > mode, since av_expr_eval will fail. Now, it can also fail due to circularly > referenced expressions e.g. 'x=y+10' and 'y=x+10' and there's no way to > detect whether it's this case or if inapplicable variables have been used. Then I would prefer that we wait for that to be done and working to commit this patch. After all, it is entirely possible that you realize this is not exactly what you need to achieve your goal. Also, you may consider if it would be worthwhile to globally improve the expression system: maybe have a context with dedicated structure to define the variables and custom functions instead of just arrays that needs to have the same size. The scale filter is not the only one with multiple expression evaluation, it would be a waste if the simplification was not applicable to all cases easily. (Also, the possibility to define user variables would be nice. But that would require re-writing most of the parser. It needs doing anyway, but it is a huge work.) Regards,
On 05-11-2019 07:40 pm, Nicolas George wrote: > Gyan (12019-11-05): >> An example of a cross-referenced expression would be 'y=x+rand(10)'. >> Normally in filters, X would be evaluated first, then Y, then X again. The >> 2nd eval of X will overwrite the first and can generate a different value. >> With this func, we can avoid the 2nd eval. >> >> I'm in the process of modifying the scale filter to allow 'n', 't', 'pos' >> variables in frame eval mode (it already supports dynamic output based on >> incoming frame changes) and I would like to catch their presence in init >> mode, since av_expr_eval will fail. Now, it can also fail due to circularly >> referenced expressions e.g. 'x=y+10' and 'y=x+10' and there's no way to >> detect whether it's this case or if inapplicable variables have been used. > Then I would prefer that we wait for that to be done and working to > commit this patch. After all, it is entirely possible that you realize > this is not exactly what you need to achieve your goal. I've looked for workarounds using existing API and I haven't found one. In my scale patch, I've implemented this function locally and it fulfills the role. Thought I should insert it generically. > Also, you may consider if it would be worthwhile to globally improve the > expression system: maybe have a context with dedicated structure to > define the variables and custom functions instead of just arrays that > needs to have the same size > > The scale filter is not the only one with multiple expression > evaluation, it would be a waste if the simplification was not applicable > to all cases easily. I have come across the need for this helper in other filters like the various draw* filters but it was never a priority. But the changes to scale/scale2ref filter need it or its equivalent. This one does the trick and I can see how it would be helpful in animation support for drawbox where all vars can refer to each other, so each expression is evaluated 5 times. That work is half-way done and is my next focus. > (Also, the possibility to define user variables would be nice. But that > would require re-writing most of the parser. It needs doing anyway, but > it is a huge work.) That sounds desirable but that needs careful design in terms of sanitation of user input. That looks to be a job for another day. Possibly, early next year. Thanks, Gyan
diff --git a/libavutil/eval.c b/libavutil/eval.c index 48832979e2..ed0fe636f7 100644 --- a/libavutil/eval.c +++ b/libavutil/eval.c @@ -735,6 +735,27 @@ end: return ret; } +int av_expr_count_var(AVExpr *e, int var_start, int var_end) +{ + int i, count = 0; + + if (var_start >= var_end) + return AVERROR(EINVAL); + + if (!e) + return AVERROR(EINVAL); + + for (i = 0; e->type != e_const && i < 3 && e->param[i]; i++) + count += av_expr_count_var(e->param[i], var_start, var_end); + + if (e->type == e_const && + e->a.const_index >= var_start && + e->a.const_index < var_end) + count++; + + return count; +} + 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 dacd22b96e..030e5617cc 100644 --- a/libavutil/eval.h +++ b/libavutil/eval.h @@ -86,6 +86,16 @@ int av_expr_parse(AVExpr **expr, const char *s, */ double av_expr_eval(AVExpr *e, const double *const_values, void *opaque); +/** + * Return the number of occurrences of variables within a range in a parsed expression + * + * @param var_start index of the start of the range of variables being seached for in the const_names identifiers + * @param var_end index of the first variable after the range of variables being seached for in the const_names identifiers + * @return the number of occurrences of the variables in the range, a negative value indicates that no expression was passed + * or the range wasn't strictly monotonic + */ +int av_expr_count_var(AVExpr *e, int var_start, int var_end); + /** * Free a parsed expression previously created with av_expr_parse(). */ diff --git a/libavutil/version.h b/libavutil/version.h index 27d663baf1..af3abf7265 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -79,8 +79,8 @@ */ #define LIBAVUTIL_VERSION_MAJOR 56 -#define LIBAVUTIL_VERSION_MINOR 35 -#define LIBAVUTIL_VERSION_MICRO 101 +#define LIBAVUTIL_VERSION_MINOR 36 +#define LIBAVUTIL_VERSION_MICRO 100 #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \ LIBAVUTIL_VERSION_MINOR, \
Helps better identification of expr eval failures. Gyan From 19bce329464676f071707b99575f80e5abe1cd4c Mon Sep 17 00:00:00 2001 From: Gyan Doshi <ffmpeg@gyani.pro> Date: Sat, 2 Nov 2019 20:16:42 +0530 Subject: [PATCH] avutil/eval: add function to track variable use Helps avoid multiple evals of cross-referenced expressions and catch the use of non-applicable variables with respect to eval or special mode in filters --- libavutil/eval.c | 21 +++++++++++++++++++++ libavutil/eval.h | 10 ++++++++++ libavutil/version.h | 4 ++-- 3 files changed, 33 insertions(+), 2 deletions(-)