gst/gststructure.c: Avoid overflows in fixation code when dealing with MAXINT values...

Original commit message from CVS:
Patch by: Olivier Crete <tester at tester dot ca>
* gst/gststructure.c:
(gst_structure_fixate_field_nearest_fraction):
Avoid overflows in fixation code when dealing with MAXINT values, which
v4l2src seems to do.
Fixes #546328.
* tests/check/gst/gststructure.c: (GST_START_TEST):
Make a unit test to check the fix.

diff --git a/ChangeLog b/ChangeLog
index e139d57..2b94298 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,18 @@
 2008-08-05  Wim Taymans  <wim.taymans@collabora.co.uk>
 
+       Patch by: Olivier Crete <tester at tester dot ca>
+
+       * gst/gststructure.c:
+       (gst_structure_fixate_field_nearest_fraction):
+       Avoid overflows in fixation code when dealing with MAXINT values, which
+       v4l2src seems to do.
+       Fixes #546328.
+
+       * tests/check/gst/gststructure.c: (GST_START_TEST):
+       Make a unit test to check the fix. 
+
+2008-08-05  Wim Taymans  <wim.taymans@collabora.co.uk>
+
        * plugins/elements/gstcapsfilter.c: (copy_func),
        (gst_capsfilter_set_property):
        Use new caps suggestion feature of basetransform to request a caps

diff --git a/gst/gststructure.c b/gst/gststructure.c
index 673947c..6939fb4 100644 (file)
--- a/gst/gststructure.c
+++ b/gst/gststructure.c
@@ -2232,16 +2232,13 @@ gst_structure_fixate_field_nearest_fraction (GstStructure * structure,
     const GValue *list_value;
     int i, n;
     const GValue *best = NULL;
-    GValue best_diff = { 0 };
-    GValue cur_diff = { 0 };
-    GValue target = { 0 };
-    gboolean res = FALSE;
+    gdouble target;
+    gdouble cur_diff;
+    gdouble best_diff = G_MAXDOUBLE;
 
-    g_value_init (&best_diff, GST_TYPE_FRACTION);
-    g_value_init (&cur_diff, GST_TYPE_FRACTION);
-    g_value_init (&target, GST_TYPE_FRACTION);
+    target = (gdouble) target_numerator / (gdouble) target_denominator;
 
-    gst_value_set_fraction (&target, target_numerator, target_denominator);
+    GST_DEBUG ("target %g, best %g", target, best_diff);
 
     best = NULL;
 
@@ -2249,28 +2246,32 @@ gst_structure_fixate_field_nearest_fraction (GstStructure * structure,
     for (i = 0; i < n; i++) {
       list_value = gst_value_list_get_value (value, i);
       if (G_VALUE_TYPE (list_value) == GST_TYPE_FRACTION) {
+        gint num, denom;
+        gdouble list_double;
+
+        num = gst_value_get_fraction_numerator (list_value);
+        denom = gst_value_get_fraction_denominator (list_value);
+
+        list_double = ((gdouble) num / (gdouble) denom);
+        cur_diff = target - list_double;
 
-        if (gst_value_compare (list_value, &target) == GST_VALUE_LESS_THAN)
-          gst_value_fraction_subtract (&cur_diff, &target, list_value);
-        else
-          gst_value_fraction_subtract (&cur_diff, list_value, &target);
+        GST_DEBUG ("curr diff %g, list %g", cur_diff, list_double);
 
-        if (!best
-            || gst_value_compare (&cur_diff,
-                &best_diff) == GST_VALUE_LESS_THAN) {
+        if (cur_diff < 0)
+          cur_diff = -cur_diff;
+
+        if (!best || cur_diff < best_diff) {
+          GST_DEBUG ("new best %g", list_double);
           best = list_value;
-          g_value_copy (&cur_diff, &best_diff);
+          best_diff = cur_diff;
         }
       }
     }
     if (best != NULL) {
       gst_structure_set_value (structure, field_name, best);
-      res = TRUE;
+      return TRUE;
     }
-    g_value_unset (&best_diff);
-    g_value_unset (&cur_diff);
-    g_value_unset (&target);
-    return res;
   }
+
   return FALSE;
 }

diff --git a/tests/check/gst/gststructure.c b/tests/check/gst/gststructure.c
index 6658ccb..3c06e46 100644 (file)
--- a/tests/check/gst/gststructure.c
+++ b/tests/check/gst/gststructure.c
@@ -294,7 +294,7 @@ GST_END_TEST;
 
 GST_START_TEST (test_fixate_frac_list)
 {
-  GstStructure *s;
+  GstStructure *s, *s2;
   GValue list = { 0 };
   GValue frac = { 0 };
   gchar *str;
@@ -319,6 +319,9 @@ GST_START_TEST (test_fixate_frac_list)
   GST_DEBUG ("list %s", str);
   g_free (str);
 
+  /* take copy */
+  s2 = gst_structure_copy (s);
+
   /* fixate to the nearest fraction, this should give 15/1 */
   fail_unless (gst_structure_fixate_field_nearest_fraction (s, "frac", 14, 1));
 
@@ -327,6 +330,16 @@ GST_START_TEST (test_fixate_frac_list)
   fail_unless (denom == 1);
 
   gst_structure_free (s);
+  s = s2;
+
+  /* fixate to the nearest fraction, this should give 30/1 */
+  fail_unless (gst_structure_fixate_field_nearest_fraction (s, "frac", G_MAXINT,
+          1));
+
+  fail_unless (gst_structure_get_fraction (s, "frac", &num, &denom));
+  fail_unless (num == 30);
+  fail_unless (denom == 1);
+  gst_structure_free (s);
 }
 
 GST_END_TEST;