diff mbox

[FFmpeg-devel] avutil/eval: add function to track variable use

Message ID 007f0a15-884b-7010-a1d1-c466972d170f@gyani.pro
State Superseded
Headers show

Commit Message

Gyan Doshi Nov. 5, 2019, 1:05 p.m. UTC 
On 05-11-2019 03:55 pm, Michael Niedermayer wrote:
> 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

Done.

Thanks,
Gyan
From 2931fa96b8e7f80581ded280907655753e54def0 Mon Sep 17 00:00:00 2001
From: Gyan Doshi <ffmpeg@gyani.pro>
Date: Sat, 2 Nov 2019 20:16:42 +0530
Subject: [PATCH v2 1/2] avutil/eval: add function to track variable use

1)Some filters allow cross-referenced expressions e.g. x=y+10. In
such cases, filters evaluate expressions multiple times for
successful evaluation of all expressions. If the expression for one or
more variables contains a RNG, the result may vary across evaluation
leading to inconsistent values across the cross-referenced expressions.

2)A related case is circular expressions e.g. x=y+10 and y=x+10 which
cannot be succesfully resolved.

3)Certain filter variables may only be applicable in specific eval modes
and lead to a failure of evaluation in other modes e.g. pts is only relevant
for frame eval mode.

At present, there is no reliable means to identify these occurrences and
thus the error messages provided are broad or inaccurate. The helper
function introduced - av_expr_count_var - allows developers to identify
the use of one or more variables in expressions and thus tailor the error
message or allow for a graceful fallback.
---
 libavutil/eval.c    | 21 +++++++++++++++++++++
 libavutil/eval.h    | 10 ++++++++++
 libavutil/version.h |  4 ++--
 3 files changed, 33 insertions(+), 2 deletions(-)

Comments

Gyan Doshi Nov. 9, 2019, 2:51 p.m. UTC | #1 
On 05-11-2019 06:35 pm, Gyan wrote:
>
>
> On 05-11-2019 03:55 pm, Michael Niedermayer wrote:
>> 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
>
> Done.

Ping.

Gyan
Michael Niedermayer Nov. 9, 2019, 4:09 p.m. UTC | #2 
On Tue, Nov 05, 2019 at 06:35:54PM +0530, Gyan wrote:
> 
> 
> On 05-11-2019 03:55 pm, Michael Niedermayer wrote:
> >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
> 
> Done.
> 
> Thanks,
> Gyan

>  eval.c    |   21 +++++++++++++++++++++
>  eval.h    |   10 ++++++++++
>  version.h |    4 ++--
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 740f6166ddb4b1a581aee7c34edd05d328265df1  v2-0001-avutil-eval-add-function-to-track-variable-use.patch
> From 2931fa96b8e7f80581ded280907655753e54def0 Mon Sep 17 00:00:00 2001
> From: Gyan Doshi <ffmpeg@gyani.pro>
> Date: Sat, 2 Nov 2019 20:16:42 +0530
> Subject: [PATCH v2 1/2] avutil/eval: add function to track variable use

i think the patch is ok but i think nicolas wants to wait
with pushing this until there is a 2nd patch which uses the new
function

Thanks

[...]
Gyan Doshi Nov. 9, 2019, 4:13 p.m. UTC | #3 
On 09-11-2019 09:39 pm, Michael Niedermayer wrote:
> On Tue, Nov 05, 2019 at 06:35:54PM +0530, Gyan wrote:
>>
>> On 05-11-2019 03:55 pm, Michael Niedermayer wrote:
>>> 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
>> Done.
>>
>> Thanks,
>> Gyan
>>   eval.c    |   21 +++++++++++++++++++++
>>   eval.h    |   10 ++++++++++
>>   version.h |    4 ++--
>>   3 files changed, 33 insertions(+), 2 deletions(-)
>> 740f6166ddb4b1a581aee7c34edd05d328265df1  v2-0001-avutil-eval-add-function-to-track-variable-use.patch
>>  From 2931fa96b8e7f80581ded280907655753e54def0 Mon Sep 17 00:00:00 2001
>> From: Gyan Doshi <ffmpeg@gyani.pro>
>> Date: Sat, 2 Nov 2019 20:16:42 +0530
>> Subject: [PATCH v2 1/2] avutil/eval: add function to track variable use
> i think the patch is ok but i think nicolas wants to wait
> with pushing this until there is a 2nd patch which uses the new
> function

OK, my scale patch is ready; will submit that in a couple of days, after 
some more testing.

Thanks,
Gyan
Gyan Doshi Nov. 12, 2019, 1:52 p.m. UTC | #4 
On 09-11-2019 09:39 pm, Michael Niedermayer wrote:
> On Tue, Nov 05, 2019 at 06:35:54PM +0530, Gyan wrote:
>>
>> On 05-11-2019 03:55 pm, Michael Niedermayer wrote:
>>> 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
>> Done.
>>
>> Thanks,
>> Gyan
>>   eval.c    |   21 +++++++++++++++++++++
>>   eval.h    |   10 ++++++++++
>>   version.h |    4 ++--
>>   3 files changed, 33 insertions(+), 2 deletions(-)
>> 740f6166ddb4b1a581aee7c34edd05d328265df1  v2-0001-avutil-eval-add-function-to-track-variable-use.patch
>>  From 2931fa96b8e7f80581ded280907655753e54def0 Mon Sep 17 00:00:00 2001
>> From: Gyan Doshi <ffmpeg@gyani.pro>
>> Date: Sat, 2 Nov 2019 20:16:42 +0530
>> Subject: [PATCH v2 1/2] avutil/eval: add function to track variable use
> i think the patch is ok but i think nicolas wants to wait
> with pushing this until there is a 2nd patch which uses the new
> function

Scale patch posted -  https://patchwork.ffmpeg.org/patch/16222/

Gyan
diff mbox

Patch

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, \