libs/gst/controller/: Protect against values larger or smaller than the minimum or maximum allowed value for the prop...

Original commit message from CVS:
* libs/gst/controller/gstcontroller.c:
(gst_controlled_property_new):
* libs/gst/controller/gstcontrollerprivate.h:
* libs/gst/controller/gstinterpolation.c:
(gst_controlled_property_find_control_point_node),
(interpolate_none_get), (interpolate_none_get_enum_value_array),
(interpolate_none_get_string_value_array),
(interpolate_trigger_get),
(interpolate_trigger_get_enum_value_array),
(interpolate_trigger_get_string_value_array):
Protect against values larger or smaller than the minimum or maximum
allowed value for the property when using values that can be compared.
Optimize trigger interpolator a bit by taking the last requested value
into account instead of always looping through the complete list.
Fix coding style a bit, everywhere else we use "return foo" instead
of "return (foo)".
* tests/check/libs/controller.c: (GST_START_TEST),
(gst_controller_suite):
Add unit test for the protection against too large or too small
values.

ChangeLog

@@ -1,3 +1,29 @@
+2007-06-09  Sebastian Dröge  <slomo@circular-chaos.org>
+
+	* libs/gst/controller/gstcontroller.c:
+	(gst_controlled_property_new):
+	* libs/gst/controller/gstcontrollerprivate.h:
+	* libs/gst/controller/gstinterpolation.c:
+	(gst_controlled_property_find_control_point_node),
+	(interpolate_none_get), (interpolate_none_get_enum_value_array),
+	(interpolate_none_get_string_value_array),
+	(interpolate_trigger_get),
+	(interpolate_trigger_get_enum_value_array),
+	(interpolate_trigger_get_string_value_array):
+	Protect against values larger or smaller than the minimum or maximum
+	allowed value for the property when using values that can be compared.
+
+	Optimize trigger interpolator a bit by taking the last requested value
+	into account instead of always looping through the complete list.
+
+	Fix coding style a bit, everywhere else we use "return foo" instead
+	of "return (foo)".
+	
+	* tests/check/libs/controller.c: (GST_START_TEST),
+	(gst_controller_suite):
+	Add unit test for the protection against too large or too small
+	values.
+
 2007-06-08  Sebastian Dröge  <slomo@circular-chaos.org>
 
 	* docs/random/slomo/controller.txt:

libs/gst/controller/gstcontroller.c

@@ -340,36 +340,60 @@ gst_controlled_property_new (GObject * object, const gchar * name)
           GParamSpecInt *tpspec = G_PARAM_SPEC_INT (pspec);
 
           g_value_set_int (&prop->default_value, tpspec->default_value);
+          g_value_init (&prop->min_value, prop->type);
+          g_value_set_int (&prop->min_value, tpspec->minimum);
+          g_value_init (&prop->max_value, prop->type);
+          g_value_set_int (&prop->max_value, tpspec->maximum);
         }
           break;
         case G_TYPE_UINT:{
           GParamSpecUInt *tpspec = G_PARAM_SPEC_UINT (pspec);
 
           g_value_set_uint (&prop->default_value, tpspec->default_value);
+          g_value_init (&prop->min_value, prop->type);
+          g_value_set_uint (&prop->min_value, tpspec->minimum);
+          g_value_init (&prop->max_value, prop->type);
+          g_value_set_uint (&prop->max_value, tpspec->maximum);
         }
           break;
         case G_TYPE_LONG:{
           GParamSpecLong *tpspec = G_PARAM_SPEC_LONG (pspec);
 
           g_value_set_long (&prop->default_value, tpspec->default_value);
+          g_value_init (&prop->min_value, prop->type);
+          g_value_set_long (&prop->min_value, tpspec->minimum);
+          g_value_init (&prop->max_value, prop->type);
+          g_value_set_long (&prop->max_value, tpspec->maximum);
         }
           break;
         case G_TYPE_ULONG:{
           GParamSpecULong *tpspec = G_PARAM_SPEC_ULONG (pspec);
 
           g_value_set_ulong (&prop->default_value, tpspec->default_value);
+          g_value_init (&prop->min_value, prop->type);
+          g_value_set_ulong (&prop->min_value, tpspec->minimum);
+          g_value_init (&prop->max_value, prop->type);
+          g_value_set_ulong (&prop->max_value, tpspec->maximum);
         }
           break;
         case G_TYPE_FLOAT:{
           GParamSpecFloat *tpspec = G_PARAM_SPEC_FLOAT (pspec);
 
           g_value_set_float (&prop->default_value, tpspec->default_value);
+          g_value_init (&prop->min_value, prop->type);
+          g_value_set_float (&prop->min_value, tpspec->minimum);
+          g_value_init (&prop->max_value, prop->type);
+          g_value_set_float (&prop->max_value, tpspec->maximum);
         }
           break;
         case G_TYPE_DOUBLE:{
           GParamSpecDouble *tpspec = G_PARAM_SPEC_DOUBLE (pspec);
 
           g_value_set_double (&prop->default_value, tpspec->default_value);
+          g_value_init (&prop->min_value, prop->type);
+          g_value_set_double (&prop->min_value, tpspec->minimum);
+          g_value_init (&prop->max_value, prop->type);
+          g_value_set_double (&prop->max_value, tpspec->maximum);
         }
           break;
         case G_TYPE_BOOLEAN:{

libs/gst/controller/gstcontrollerprivate.h

@@ -100,6 +100,8 @@ typedef struct _GstControlledProperty
   GType type;                   /* type of the handled property */
   GType base;                   /* base-type of the handled property */
   GValue default_value;         /* default value for the handled property */
+  GValue min_value;             /* min value for the handled property */
+  GValue max_value;             /* max value for the handled property */
   GValue result_value;          /* result value location for the interpolation method */
   GstControlPoint last_value;     /* the last value a _sink call wrote */
   GstControlPoint live_value;     /* temporary value override for live input */

libs/gst/controller/gstinterpolation.c

@@ -71,7 +71,7 @@ gst_controlled_property_find_control_point_node (GstControlledProperty * prop,
   if (prev_node)
     prop->last_requested_value = prev_node;
 
-  return (prev_node);
+  return prev_node;
 }
 
 /*  steps-like (no-)interpolation, default */
@@ -86,26 +86,30 @@ interpolate_none_get (GstControlledProperty * prop, GstClockTime timestamp)
           gst_controlled_property_find_control_point_node (prop, timestamp))) {
     GstControlPoint *cp = node->data;
 
-    return (&cp->value);
+    return &cp->value;
   }
-  return (&prop->default_value);
+  return &prop->default_value;
 }
 
+#define interpolate_none_get_boolean interpolate_none_get
+
 #define DEFINE_NONE_GET(type) \
-static gboolean \
+static GValue * \
-interpolate_none_get_##type##_value_array (GstControlledProperty * prop, \
+interpolate_none_get_##type (GstControlledProperty * prop, GstClockTime timestamp) \
-    GstClockTime timestamp, GstValueArray * value_array) \
 { \
-  gint i; \
+  GList *node; \
-  GstClockTime ts = timestamp; \
-  g##type *values = (g##type *) value_array->values; \
   \
-  for(i = 0; i < value_array->nbsamples; i++) { \
+  if ((node = \
-    *values = g_value_get_##type (interpolate_none_get (prop,ts)); \
+          gst_controlled_property_find_control_point_node (prop, timestamp))) { \
-    ts += value_array->sample_interval; \
+    GstControlPoint *cp = node->data; \
-    values++; \
+    g##type ret = g_value_get_##type (&cp->value); \
+    if (g_value_get_##type (&prop->min_value) > ret) \
+      return &prop->min_value; \
+    else if (g_value_get_##type (&prop->max_value) < ret) \
+      return &prop->max_value; \
+    return &cp->value; \
   } \
-  return (TRUE); \
+  return &prop->default_value; \
 }
 
 DEFINE_NONE_GET (int);
@@ -116,7 +120,33 @@ DEFINE_NONE_GET (ulong);
 DEFINE_NONE_GET (float);
 DEFINE_NONE_GET (double);
 
-DEFINE_NONE_GET (boolean);
+#define DEFINE_NONE_GET_VALUE_ARRAY(type) \
+static gboolean \
+interpolate_none_get_##type##_value_array (GstControlledProperty * prop, \
+    GstClockTime timestamp, GstValueArray * value_array) \
+{ \
+  gint i; \
+  GstClockTime ts = timestamp; \
+  g##type *values = (g##type *) value_array->values; \
+  \
+  for(i = 0; i < value_array->nbsamples; i++) { \
+    *values = g_value_get_##type (interpolate_none_get_##type (prop,ts)); \
+    ts += value_array->sample_interval; \
+    values++; \
+  } \
+  return TRUE; \
+}
+
+DEFINE_NONE_GET_VALUE_ARRAY (int);
+
+DEFINE_NONE_GET_VALUE_ARRAY (uint);
+DEFINE_NONE_GET_VALUE_ARRAY (long);
+
+DEFINE_NONE_GET_VALUE_ARRAY (ulong);
+DEFINE_NONE_GET_VALUE_ARRAY (float);
+DEFINE_NONE_GET_VALUE_ARRAY (double);
+
+DEFINE_NONE_GET_VALUE_ARRAY (boolean);
 
 static gboolean
 interpolate_none_get_enum_value_array (GstControlledProperty * prop,
@@ -131,7 +161,7 @@ interpolate_none_get_enum_value_array (GstControlledProperty * prop,
     ts += value_array->sample_interval;
     values++;
   }
-  return (TRUE);
+  return TRUE;
 }
 
 static gboolean
@@ -147,7 +177,7 @@ interpolate_none_get_string_value_array (GstControlledProperty * prop,
     ts += value_array->sample_interval;
     values++;
   }
-  return (TRUE);
+  return TRUE;
 }
 
 static GstInterpolateMethod interpolate_none = {
@@ -181,31 +211,41 @@ interpolate_trigger_get (GstControlledProperty * prop, GstClockTime timestamp)
   GstControlPoint *cp;
 
   /* check if there is a value at the registered timestamp */
-  for (node = prop->values; node; node = g_list_next (node)) {
+  if ((node =
+          gst_controlled_property_find_control_point_node (prop, timestamp))) {
     cp = node->data;
     if (timestamp == cp->timestamp) {
-      return (&cp->value);
+      return &cp->value;
     }
   }
 
-  return (&prop->default_value);
+  return &prop->default_value;
 }
 
+#define interpolate_trigger_get_boolean interpolate_trigger_get
+
 #define DEFINE_TRIGGER_GET(type) \
-static gboolean \
+static GValue * \
-interpolate_trigger_get_##type##_value_array (GstControlledProperty * prop, \
+interpolate_trigger_get_##type (GstControlledProperty * prop, GstClockTime timestamp) \
-    GstClockTime timestamp, GstValueArray * value_array) \
 { \
-  gint i; \
+  GList *node; \
-  GstClockTime ts = timestamp; \
+  GstControlPoint *cp; \
-  g##type *values = (g##type *) value_array->values; \
   \
-  for(i = 0; i < value_array->nbsamples; i++) { \
+  /* check if there is a value at the registered timestamp */ \
-    *values = g_value_get_##type (interpolate_trigger_get (prop,ts)); \
+  if ((node = \
-    ts += value_array->sample_interval; \
+          gst_controlled_property_find_control_point_node (prop, timestamp))) { \
-    values++; \
+    cp = node->data; \
+    if (timestamp == cp->timestamp) { \
+      g##type ret = g_value_get_##type (&cp->value); \
+      if (g_value_get_##type (&prop->min_value) > ret) \
+        return &prop->min_value; \
+      else if (g_value_get_##type (&prop->max_value) < ret) \
+        return &prop->max_value; \
+      return &cp->value; \
+    } \
   } \
-  return (TRUE); \
+  \
+  return &prop->default_value; \
 }
 
 DEFINE_TRIGGER_GET (int);
@@ -216,7 +256,33 @@ DEFINE_TRIGGER_GET (ulong);
 DEFINE_TRIGGER_GET (float);
 DEFINE_TRIGGER_GET (double);
 
-DEFINE_TRIGGER_GET (boolean);
+#define DEFINE_TRIGGER_GET_VALUE_ARRAY(type) \
+static gboolean \
+interpolate_trigger_get_##type##_value_array (GstControlledProperty * prop, \
+    GstClockTime timestamp, GstValueArray * value_array) \
+{ \
+  gint i; \
+  GstClockTime ts = timestamp; \
+  g##type *values = (g##type *) value_array->values; \
+  \
+  for(i = 0; i < value_array->nbsamples; i++) { \
+    *values = g_value_get_##type (interpolate_trigger_get_##type (prop,ts)); \
+    ts += value_array->sample_interval; \
+    values++; \
+  } \
+  return TRUE; \
+}
+
+DEFINE_TRIGGER_GET_VALUE_ARRAY (int);
+
+DEFINE_TRIGGER_GET_VALUE_ARRAY (uint);
+DEFINE_TRIGGER_GET_VALUE_ARRAY (long);
+
+DEFINE_TRIGGER_GET_VALUE_ARRAY (ulong);
+DEFINE_TRIGGER_GET_VALUE_ARRAY (float);
+DEFINE_TRIGGER_GET_VALUE_ARRAY (double);
+
+DEFINE_TRIGGER_GET_VALUE_ARRAY (boolean);
 
 static gboolean
 interpolate_trigger_get_enum_value_array (GstControlledProperty * prop,
@@ -231,7 +297,7 @@ interpolate_trigger_get_enum_value_array (GstControlledProperty * prop,
     ts += value_array->sample_interval;
     values++;
   }
-  return (TRUE);
+  return TRUE;
 }
 
 static gboolean
@@ -247,7 +313,7 @@ interpolate_trigger_get_string_value_array (GstControlledProperty * prop,
     ts += value_array->sample_interval;
     values++;
   }
-  return (TRUE);
+  return TRUE;
 }
 
 static GstInterpolateMethod interpolate_trigger = {
@@ -279,6 +345,7 @@ static g##type \
 _interpolate_linear_get_##type (GstControlledProperty * prop, GstClockTime timestamp) \
 { \
   GList *node; \
+  g##type ret; \
   \
   if ((node = gst_controlled_property_find_control_point_node (prop, timestamp))) { \
     GstControlPoint *cp1, *cp2; \
@@ -294,20 +361,23 @@ _interpolate_linear_get_##type (GstControlledProperty * prop, GstClockTime times
       value2 = g_value_get_##type (&cp2->value); \
       slope = (gdouble) (value2 - value1) / gst_guint64_to_gdouble (cp2->timestamp - cp1->timestamp); \
       \
-      return ((g##type) (value1 + gst_guint64_to_gdouble (timestamp - cp1->timestamp) * slope)); \
+      ret = (g##type) (value1 + gst_guint64_to_gdouble (timestamp - cp1->timestamp) * slope); \
     } \
     else { \
-      return (g_value_get_##type (&cp1->value)); \
+      ret = g_value_get_##type (&cp1->value); \
     } \
+  } else { \
+    ret = g_value_get_##type (&prop->default_value); \
   } \
-  return (g_value_get_##type (&prop->default_value)); \
+  ret = CLAMP (ret, g_value_get_##type (&prop->min_value), g_value_get_##type (&prop->max_value)); \
+  return ret; \
 } \
 \
 static GValue * \
 interpolate_linear_get_##type (GstControlledProperty * prop, GstClockTime timestamp) \
 { \
   g_value_set_##type (&prop->result_value,_interpolate_linear_get_##type (prop,timestamp)); \
-  return (&prop->result_value); \
+  return &prop->result_value; \
 } \
 \
 static gboolean \
@@ -323,7 +393,7 @@ interpolate_linear_get_##type##_value_array (GstControlledProperty * prop, \
     ts += value_array->sample_interval; \
     values++; \
   } \
-  return (TRUE); \
+  return TRUE; \
 }
 
 DEFINE_LINEAR_GET (int);
@@ -459,6 +529,7 @@ static g##type \
 _interpolate_cubic_get_##type (GstControlledProperty * prop, GstClockTime timestamp) \
 { \
   GList *node; \
+  g##type ret; \
   \
   if (prop->nvalues <= 2) \
     return _interpolate_linear_get_##type (prop, timestamp); \
@@ -475,7 +546,7 @@ _interpolate_cubic_get_##type (GstControlledProperty * prop, GstClockTime timest
     if ((node = g_list_next (node))) { \
       gdouble diff1, diff2; \
       g##type value1,value2; \
-      gdouble ret; \
+      gdouble out; \
       \
       cp2 = node->data; \
       \
@@ -485,24 +556,27 @@ _interpolate_cubic_get_##type (GstControlledProperty * prop, GstClockTime timest
       diff1 = gst_guint64_to_gdouble (timestamp - cp1->timestamp); \
       diff2 = gst_guint64_to_gdouble (cp2->timestamp - timestamp); \
       \
-      ret = (cp2->cache.cubic.z * diff1 * diff1 * diff1 + cp1->cache.cubic.z * diff2 * diff2 * diff2) / cp1->cache.cubic.h; \
+      out = (cp2->cache.cubic.z * diff1 * diff1 * diff1 + cp1->cache.cubic.z * diff2 * diff2 * diff2) / cp1->cache.cubic.h; \
-      ret += (value2 / cp1->cache.cubic.h - cp1->cache.cubic.h * cp2->cache.cubic.z) * diff1; \
+      out += (value2 / cp1->cache.cubic.h - cp1->cache.cubic.h * cp2->cache.cubic.z) * diff1; \
-      ret += (value1 / cp1->cache.cubic.h - cp1->cache.cubic.h * cp1->cache.cubic.z) * diff2; \
+      out += (value1 / cp1->cache.cubic.h - cp1->cache.cubic.h * cp1->cache.cubic.z) * diff2; \
       \
-      return (g##type) ret; \
+      ret = (g##type) out; \
     } \
     else { \
-      return (g_value_get_##type (&cp1->value)); \
+      ret = g_value_get_##type (&cp1->value); \
     } \
+  } else {\
+    ret = g_value_get_##type (&prop->default_value); \
   } \
-  return (g_value_get_##type (&prop->default_value)); \
+  ret = CLAMP (ret, g_value_get_##type (&prop->min_value), g_value_get_##type (&prop->max_value)); \
+  return ret; \
 } \
 \
 static GValue * \
 interpolate_cubic_get_##type (GstControlledProperty * prop, GstClockTime timestamp) \
 { \
   g_value_set_##type (&prop->result_value,_interpolate_cubic_get_##type (prop,timestamp)); \
-  return (&prop->result_value); \
+  return &prop->result_value; \
 } \
 \
 static gboolean \
@@ -518,7 +592,7 @@ interpolate_cubic_get_##type##_value_array (GstControlledProperty * prop, \
     ts += value_array->sample_interval; \
     values++; \
   } \
-  return (TRUE); \
+  return TRUE; \
 }
 
 DEFINE_CUBIC_GET (int);

tests/check/libs/controller.c

@@ -1076,6 +1076,63 @@ GST_START_TEST (controller_interpolate_linear_value_array)
 
 GST_END_TEST;
 
+
+/* test if values below minimum and above maximum are clipped */
+GST_START_TEST (controller_linear_invalid_values)
+{
+  GstController *ctrl;
+  GstElement *elem;
+  gboolean res;
+  GValue val_float = { 0, };
+
+  gst_controller_init (NULL, NULL);
+
+  elem = gst_element_factory_make ("testmonosource", "test_source");
+
+  /* that property should exist and should be controllable */
+  ctrl = gst_controller_new (G_OBJECT (elem), "float", NULL);
+  fail_unless (ctrl != NULL, NULL);
+
+  /* set interpolation mode */
+  fail_unless (gst_controller_set_interpolation_mode (ctrl, "float",
+          GST_INTERPOLATE_LINEAR));
+
+  /* set control values */
+  g_value_init (&val_float, G_TYPE_FLOAT);
+  g_value_set_float (&val_float, 200.0);
+  res = gst_controller_set (ctrl, "float", 0 * GST_SECOND, &val_float);
+  fail_unless (res, NULL);
+  g_value_set_float (&val_float, -200.0);
+  res = gst_controller_set (ctrl, "float", 4 * GST_SECOND, &val_float);
+  fail_unless (res, NULL);
+
+  /* now pull in values for some timestamps and see if clipping works */
+  /* 200.0 */
+  gst_controller_sync_values (ctrl, 0 * GST_SECOND);
+  fail_unless (GST_TEST_MONO_SOURCE (elem)->val_float == 100.0, NULL);
+  /* 100.0 */
+  gst_controller_sync_values (ctrl, 1 * GST_SECOND);
+  fail_unless (GST_TEST_MONO_SOURCE (elem)->val_float == 100.0, NULL);
+  /* 50.0 */
+  gst_controller_sync_values (ctrl, 1 * GST_SECOND + 500 * GST_MSECOND);
+  fail_unless (GST_TEST_MONO_SOURCE (elem)->val_float == 50.0, NULL);
+  /* 0.0 */
+  gst_controller_sync_values (ctrl, 2 * GST_SECOND);
+  fail_unless (GST_TEST_MONO_SOURCE (elem)->val_float == 0.0, NULL);
+  /* -100.0 */
+  gst_controller_sync_values (ctrl, 3 * GST_SECOND);
+  fail_unless (GST_TEST_MONO_SOURCE (elem)->val_float == 0.0, NULL);
+  /* -200.0 */
+  gst_controller_sync_values (ctrl, 4 * GST_SECOND);
+  fail_unless (GST_TEST_MONO_SOURCE (elem)->val_float == 0.0, NULL);
+
+  GST_INFO ("controller->ref_count=%d", G_OBJECT (ctrl)->ref_count);
+  g_object_unref (ctrl);
+  gst_object_unref (elem);
+}
+
+GST_END_TEST;
+
 static Suite *
 gst_controller_suite (void)
 {
@@ -1106,6 +1163,7 @@ gst_controller_suite (void)
   tcase_add_test (tc, controller_helper_any_gobject);
   tcase_add_test (tc, controller_misc);
   tcase_add_test (tc, controller_interpolate_linear_value_array);
+  tcase_add_test (tc, controller_linear_invalid_values);
 
   return s;
 }