From be854d4be890c95922cbe4e5cbbaa7fc408b9f85 Mon Sep 17 00:00:00 2001
From: Thibault Saunier <tsaunier@igalia.com>
Date: Sat, 2 Feb 2019 13:43:35 -0300
Subject: [PATCH] validate:scenario: Allow using set-vars from numeric
 expressions

And require them to follow the `$varname` (can't be $(varname) as
parenthesis have another meaning in those expressions).

Still accept "duration" and "position" as varname for backward compat
but update our scenarios anyway.
---
 validate/data/scenarios/fast_forward.scenario | 10 +++---
 .../data/scenarios/reverse_playback.scenario  |  2 +-
 .../scenarios/scrub_backward_seeking.scenario |  4 +--
 .../scrub_backward_seeking_full.scenario      |  4 +--
 .../scenarios/scrub_forward_seeking.scenario  |  2 +-
 .../scrub_forward_seeking_full.scenario       |  2 +-
 .../data/scenarios/seek_backward.scenario     |  8 ++---
 validate/data/scenarios/seek_forward.scenario |  7 ++--
 validate/gst/validate/gst-validate-scenario.c | 35 +++++++++++++++----
 validate/gst/validate/gst-validate-utils.c    |  9 +++--
 10 files changed, 55 insertions(+), 28 deletions(-)

diff --git a/validate/data/scenarios/fast_forward.scenario b/validate/data/scenarios/fast_forward.scenario
index ee228441e2..1475647c02 100644
--- a/validate/data/scenarios/fast_forward.scenario
+++ b/validate/data/scenarios/fast_forward.scenario
@@ -1,8 +1,8 @@
 description, duration=25.0, seek=true, need-clock-sync=true, min-media-duration=5.0
 include,location=includes/default-seek-flags.scenario
 seek, name=Fast-forward-seek, playback-time=0.0, rate=2.0,  start=0.0, flags="$(default-flags)"
-seek, name=Fast-forward-seek, playback-time="min(10.0, duration*0.0625)", rate=4.0,  start=0.0, flags="$(default-flags)"
-seek, name=Fast-forward-seek, playback-time="min(20.0, duration*0.125)", rate=8.0,  start=0.0, flags="$(default-flags)"
-seek, name=Fast-forward-seek, playback-time="min(40.0, duration*0.25)", rate=16.0, start=0.0, flags="$(default-flags)"
-seek, name=Fast-forward-seek, playback-time="min(80.0, duration*0.50)", rate=32.0, start=0.0, flags="$(default-flags)"
-stop, playback-time="min(duration - 0.3, 160.0)"
+seek, name=Fast-forward-seek, playback-time="min(10.0, $duration * 0.0625)", rate=4.0,  start=0.0, flags="$(default-flags)"
+seek, name=Fast-forward-seek, playback-time="min(20.0, $duration * 0.125)", rate=8.0,  start=0.0, flags="$(default-flags)"
+seek, name=Fast-forward-seek, playback-time="min(40.0, $duration * 0.25)", rate=16.0, start=0.0, flags="$(default-flags)"
+seek, name=Fast-forward-seek, playback-time="min(80.0, $duration * 0.50)", rate=32.0, start=0.0, flags="$(default-flags)"
+stop, playback-time="min($duration - 0.3, 160.0)"
diff --git a/validate/data/scenarios/reverse_playback.scenario b/validate/data/scenarios/reverse_playback.scenario
index 53ac7d1ffc..612dddf08e 100644
--- a/validate/data/scenarios/reverse_playback.scenario
+++ b/validate/data/scenarios/reverse_playback.scenario
@@ -1,3 +1,3 @@
 description, seek=true, reverse-playback=true
 include,location=includes/default-seek-flags.scenario
-seek, name=Reverse-seek, playback-time=0.0, rate=-1.0, start="max(duration - 15.0, 0.0)", stop=duration, flags="$(default-flags)"
+seek, name=Reverse-seek, playback-time=0.0, rate=-1.0, start="max($duration - 15.0, 0.0)", stop=$(duration), flags="$(default-flags)"
diff --git a/validate/data/scenarios/scrub_backward_seeking.scenario b/validate/data/scenarios/scrub_backward_seeking.scenario
index f54890c276..d2fd34f581 100644
--- a/validate/data/scenarios/scrub_backward_seeking.scenario
+++ b/validate/data/scenarios/scrub_backward_seeking.scenario
@@ -1,8 +1,8 @@
 description, seek=true, handles-states=true, needs_preroll=true
 include,location=includes/default-seek-flags.scenario
 pause, playback-time=0.0
-seek, playback-time=0.0, start="duration - 0.5", flags="$(default-flags)"
-seek, playback-time=0.0, start=position-0.1, repeat="min(10, (duration - 0.6))/0.1", flags="$(default-flags)"
+seek, playback-time=0.0, start="$duration - 0.5", flags="$(default-flags)"
+seek, playback-time=0.0, start=position-0.1, repeat="min(10, ($duration - 0.6))/0.1", flags="$(default-flags)"
 play, playback-time=0.0
 stop, playback-time=1.0
 
diff --git a/validate/data/scenarios/scrub_backward_seeking_full.scenario b/validate/data/scenarios/scrub_backward_seeking_full.scenario
index ebbcba63a9..19f802b97c 100644
--- a/validate/data/scenarios/scrub_backward_seeking_full.scenario
+++ b/validate/data/scenarios/scrub_backward_seeking_full.scenario
@@ -1,8 +1,8 @@
 description, seek=true, handles-states=true, needs_preroll=true
 include,location=includes/default-seek-flags.scenario
 pause, playback-time=0.0
-seek, playback-time=0.0, start="duration - 0.5", flags="$(default-flags)"
-seek, playback-time=0.0, start=position-0.1, repeat="(duration - 0.6)/0.1", flags="$(default-flags)"
+seek, playback-time=0.0, start="$duration - 0.5", flags="$(default-flags)"
+seek, playback-time=0.0, start=position-0.1, repeat="($duration - 0.6)/0.1", flags="$(default-flags)"
 play, playback-time=0.0
 stop, playback-time=1.0
 
diff --git a/validate/data/scenarios/scrub_forward_seeking.scenario b/validate/data/scenarios/scrub_forward_seeking.scenario
index f602a1eb53..58e3ab19c1 100644
--- a/validate/data/scenarios/scrub_forward_seeking.scenario
+++ b/validate/data/scenarios/scrub_forward_seeking.scenario
@@ -1,6 +1,6 @@
 description, seek=true, handles-states=true, needs_preroll=true
 include,location=includes/default-seek-flags.scenario
 pause, playback-time=0.0
-seek, playback-time=0.0, start=position+0.1, repeat="min(10, (duration - 0.5))/0.1", flags="$(default-flags)"
+seek, playback-time=0.0, start=position+0.1, repeat="min(10, ($duration - 0.5))/0.1", flags="$(default-flags)"
 play, playback-time=0.0
 stop, playback-time=1.0
diff --git a/validate/data/scenarios/scrub_forward_seeking_full.scenario b/validate/data/scenarios/scrub_forward_seeking_full.scenario
index 3ba96ce340..e50821f7a3 100644
--- a/validate/data/scenarios/scrub_forward_seeking_full.scenario
+++ b/validate/data/scenarios/scrub_forward_seeking_full.scenario
@@ -1,6 +1,6 @@
 description, seek=true, handles-states=true, needs_preroll=true
 include,location=includes/default-seek-flags.scenario
 pause, playback-time=0.0
-seek, playback-time=0.0, start=position+0.1, repeat="(duration - 0.5)/0.1", flags="$(default-flags)"
+seek, playback-time=0.0, start=position+0.1, repeat="($duration - 0.5)/0.1", flags="$(default-flags)"
 play, playback-time=0.0
 stop, playback-time=1.0
diff --git a/validate/data/scenarios/seek_backward.scenario b/validate/data/scenarios/seek_backward.scenario
index 11deceaaa3..31da4a3588 100644
--- a/validate/data/scenarios/seek_backward.scenario
+++ b/validate/data/scenarios/seek_backward.scenario
@@ -1,6 +1,6 @@
 description, seek=true, duration=30, need-clock-sync=true
 include,location=includes/default-seek-flags.scenario
-seek, name=Backward-seek, playback-time="min(5.0, (duration/4))", rate=1.0, start=0.0, flags="$(default-flags)"
-seek, name=Backward-seek, playback-time="min(10.0, 2*(duration/4))", rate=1.0, start="min(5.0, duration/4)", flags="$(default-flags)"
-seek, name=Backward-seek, playback-time="min(15.0, 3*(duration/4))", rate=1.0, start="min(10.0, 2*(duration/4))", flags="$(default-flags)"
-stop, playback-time="min(15.0, 3*(duration/4))"
+seek, name=Backward-seek, playback-time="min(5.0, ($duration / 4))", rate=1.0, start=0.0, flags="$(default-flags)"
+seek, name=Backward-seek, playback-time="min(10.0, 2*($duration / 4))", rate=1.0, start="min(5.0, $duration / 4)", flags="$(default-flags)"
+seek, name=Backward-seek, playback-time="min(15.0, 3*($duration / 4))", rate=1.0, start="min(10.0, 2*($duration / 4))", flags="$(default-flags)"
+stop, playback-time="min(15.0, 3*($duration / 4))"
diff --git a/validate/data/scenarios/seek_forward.scenario b/validate/data/scenarios/seek_forward.scenario
index 081b7a90d8..dc3af11b38 100644
--- a/validate/data/scenarios/seek_forward.scenario
+++ b/validate/data/scenarios/seek_forward.scenario
@@ -1,6 +1,7 @@
 description, seek=true, duration=20, need-clock-sync=true
 include,location=includes/default-seek-flags.scenario
-seek, name=First-forward-seek, playback-time="min(5.0, (duration/8))", start="min(10, 2*(duration/8))", flags="$(default-flags)"
-seek, name=Second-forward-seek, playback-time="min(15.0, 3*(duration/8))", start="min(20, 4*(duration/8))", flags="$(default-flags)"
-seek, name=Third-forward-seek, playback-time="min(25, 5*(duration/8))", start="min(30.0, 6*(duration/8))", flags="$(default-flags)"
+set-vars, test=(string)5.0
+seek, name=First-forward-seek, playback-time="min($test, ($duration/8))", start="min(10, 2*($duration/8))", flags="$(default-flags)"
+seek, name=Second-forward-seek, playback-time="min(15.0, 3*($duration/8))", start="min(20, 4*($duration/8))", flags="$(default-flags)"
+seek, name=Third-forward-seek, playback-time="min(25, 5*($duration/8))", start="min(30.0, 6*($duration/8))", flags="$(default-flags)"
 stop, playback-time=35.0
diff --git a/validate/gst/validate/gst-validate-scenario.c b/validate/gst/validate/gst-validate-scenario.c
index 9ddd9ef182..b1e668b46d 100644
--- a/validate/gst/validate/gst-validate-scenario.c
+++ b/validate/gst/validate/gst-validate-scenario.c
@@ -441,6 +441,8 @@ static gboolean
 _set_variable_func (const gchar * name, double *value, gpointer user_data)
 {
   gboolean res;
+  const gchar *value_str;
+  gchar *tmp;
   GstValidateScenario *scenario = GST_VALIDATE_SCENARIO (user_data);
   GstElement *pipeline = gst_validate_scenario_get_pipeline (scenario);
 
@@ -450,7 +452,7 @@ _set_variable_func (const gchar * name, double *value, gpointer user_data)
     return FALSE;
   }
 
-  if (!g_strcmp0 (name, "duration")) {
+  if (!g_strcmp0 (name, "$duration") || !g_strcmp0 (name, "duration")) {
     gint64 duration;
 
     if (!(res =
@@ -478,7 +480,7 @@ _set_variable_func (const gchar * name, double *value, gpointer user_data)
       *value = ((double) duration / GST_SECOND);
 
     goto done;
-  } else if (!g_strcmp0 (name, "position")) {
+  } else if (!g_strcmp0 (name, "$position") || !g_strcmp0 (name, "position")) {
     gint64 position;
 
     if (!gst_element_query_position (pipeline, GST_FORMAT_TIME, &position)) {
@@ -494,13 +496,34 @@ _set_variable_func (const gchar * name, double *value, gpointer user_data)
     goto done;
   }
 
-fail:
-  gst_object_unref (pipeline);
-  return FALSE;
+  if (name[0] != '$') {
+    g_error ("Variable name %s is invalid as it doesn't start with $", name);
+
+    goto fail;
+  }
+
+  if (gst_structure_get_double (scenario->priv->vars, &name[1], value))
+    goto done;
+
+  value_str = gst_structure_get_string (scenario->priv->vars, &name[1]);
+  *value = g_strtod (value_str, &tmp);
+  if (tmp[0] != '\0') {
+    gchar *vars = gst_structure_to_string (scenario->priv->vars);
+    g_error ("Variable name: %s=%s is not a double (%s)", name, value_str,
+        vars);
+    g_free (vars);
+
+    goto fail;
+  }
+
 
 done:
   gst_object_unref (pipeline);
   return TRUE;
+
+fail:
+  gst_object_unref (pipeline);
+  return FALSE;
 }
 
 /* Check that @list doesn't contain any non-optional actions */
@@ -1757,8 +1780,8 @@ gst_validate_execute_action (GstValidateActionType * action_type,
   gst_object_unref (scenario);
 
   if (!gst_structure_has_field (action->structure, "sub-action")) {
-    gst_structure_free (action->structure);
 
+    gst_structure_free (action->structure);
     action->priv->printed = FALSE;
     action->structure = gst_structure_copy (action->priv->main_structure);
 
diff --git a/validate/gst/validate/gst-validate-utils.c b/validate/gst/validate/gst-validate-utils.c
index 15de10201b..8b3943b7e6 100644
--- a/validate/gst/validate/gst-validate-utils.c
+++ b/validate/gst/validate/gst-validate-utils.c
@@ -361,8 +361,8 @@ _read_builtin (MathParser * parser)
   gint pos = 0;
 
   c = _peek (parser);
-  if (isalpha (c) || c == '_') {
-    while (isalpha (c) || isdigit (c) || c == '_') {
+  if (isalpha (c) || c == '_' || c == '$') {
+    while (isalpha (c) || isdigit (c) || c == '_' || c == '$') {
       token[pos++] = _next (parser);
       c = _peek (parser);
     }
@@ -389,7 +389,10 @@ _read_builtin (MathParser * parser)
           && parser->variable_func (token, &v1, parser->user_data)) {
         v0 = v1;
       } else {
-        _error (parser, "Could not look up value for variable %s!");
+        gchar *err =
+            g_strdup_printf ("Could not look up value for variable %s!", token);
+        _error (parser, err);
+        g_free (err);
       }
     }
   } else {