diff --git a/ChangeLog b/ChangeLog index 40b05e6319..3ca55b8273 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,22 @@ +2005-11-28 Wim Taymans + + * check/gst/gstutils.c: (GST_START_TEST), (gst_utils_suite): + More checks. + + * gst/gstclock.c: (gst_clock_finalize), (gst_clock_set_master), + (do_linear_regression), (gst_clock_add_observation): + Cleanups. + Release lock when the clock cannot be slaved. + Catch the case where the regression returned an invalid denominator. + + * gst/gstutils.c: (gst_util_div128_64_iterate), + (gst_util_div128_64), (gst_util_uint64_scale_int64), + (gst_util_uint64_scale), (gst_util_uint64_scale_int): + Add protentially more performant non-iterative 128/64 divide function + that unfortunatly does not work yet. + Shortcut the trivial 0/X = 0 case. + Remove the warnings on overflow. + 2005-11-28 Thomas Vander Stichele * gst/gstplugin.c: (gst_plugin_register_func): diff --git a/check/gst/gstutils.c b/check/gst/gstutils.c index 465d278a59..00b48b017f 100644 --- a/check/gst/gstutils.c +++ b/check/gst/gstutils.c @@ -192,9 +192,8 @@ GST_START_TEST (test_math_scale) G_MAXINT32) != G_MAXUINT64 - 100); /* overflow */ - ASSERT_WARNING (gst_util_uint64_scale_int (G_MAXUINT64 - 1, 10, - 1) != G_MAXUINT64); - ASSERT_WARNING (gst_util_uint64_scale_int (G_MAXUINT64 - 1, G_MAXINT32, + fail_if (gst_util_uint64_scale_int (G_MAXUINT64 - 1, 10, 1) != G_MAXUINT64); + fail_if (gst_util_uint64_scale_int (G_MAXUINT64 - 1, G_MAXINT32, 1) != G_MAXUINT64); } GST_END_TEST; @@ -229,12 +228,34 @@ GST_START_TEST (test_math_scale_uint64) G_MAXUINT64) != G_MAXUINT64 - 100); /* overflow */ - ASSERT_WARNING (gst_util_uint64_scale (G_MAXUINT64 - 1, 10, - 1) != G_MAXUINT64); - ASSERT_WARNING (gst_util_uint64_scale (G_MAXUINT64 - 1, G_MAXUINT64, + fail_if (gst_util_uint64_scale (G_MAXUINT64 - 1, 10, 1) != G_MAXUINT64); + fail_if (gst_util_uint64_scale (G_MAXUINT64 - 1, G_MAXUINT64, 1) != G_MAXUINT64); } GST_END_TEST; + +GST_START_TEST (test_math_scale_random) +{ + guint64 val, num, denom, res;; + GRand *rand; + gint i; + + rand = g_rand_new (); + + i = 1000; + while (i--) { + val = ((guint64) g_rand_int (rand)) << 32 | g_rand_int (rand); + num = ((guint64) g_rand_int (rand)) << 32 | g_rand_int (rand); + denom = ((guint64) g_rand_int (rand)) << 32 | g_rand_int (rand); + + res = gst_util_uint64_scale (val, num, denom); + } + g_rand_free (rand); + +} + +GST_END_TEST; + Suite * gst_utils_suite (void) { @@ -246,6 +267,7 @@ gst_utils_suite (void) tcase_add_test (tc_chain, test_buffer_probe_once); tcase_add_test (tc_chain, test_math_scale); tcase_add_test (tc_chain, test_math_scale_uint64); + tcase_add_test (tc_chain, test_math_scale_random); return s; } diff --git a/gst/gstclock.c b/gst/gstclock.c index 48a1561dc2..204538264d 100644 --- a/gst/gstclock.c +++ b/gst/gstclock.c @@ -589,19 +589,17 @@ gst_clock_finalize (GObject * object) { GstClock *clock = GST_CLOCK (object); - GST_OBJECT_LOCK (clock); + GST_CLOCK_SLAVE_LOCK (clock); if (clock->clockid) { gst_clock_id_unschedule (clock->clockid); gst_clock_id_unref (clock->clockid); clock->clockid = NULL; } - - g_cond_free (clock->entries_changed); - g_free (clock->times); clock->times = NULL; - GST_OBJECT_UNLOCK (clock); + GST_CLOCK_SLAVE_UNLOCK (clock); + g_cond_free (clock->entries_changed); g_mutex_free (clock->slave_lock); G_OBJECT_CLASS (parent_class)->finalize (object); @@ -883,18 +881,15 @@ gst_clock_set_master (GstClock * clock, GstClock * master) g_return_val_if_fail (GST_IS_CLOCK (clock), FALSE); GST_OBJECT_LOCK (clock); - /* we always allow setting the master to NULL */ if (master && !GST_OBJECT_FLAG_IS_SET (clock, GST_CLOCK_FLAG_CAN_SET_MASTER)) goto not_supported; GST_DEBUG_OBJECT (clock, "slaving to master clock %p", master); gst_object_replace ((GstObject **) & clock->master, (GstObject *) master); - GST_OBJECT_UNLOCK (clock); GST_CLOCK_SLAVE_LOCK (clock); - if (clock->clockid) { gst_clock_id_unschedule (clock->clockid); gst_clock_id_unref (clock->clockid); @@ -910,7 +905,6 @@ gst_clock_set_master (GstClock * clock, GstClock * master) gst_clock_id_wait_async (clock->clockid, (GstClockCallback) gst_clock_slave_callback, clock); } - GST_CLOCK_SLAVE_UNLOCK (clock); return TRUE; @@ -918,6 +912,7 @@ gst_clock_set_master (GstClock * clock, GstClock * master) not_supported: { GST_DEBUG_OBJECT (clock, "cannot be slaved to a master clock"); + GST_OBJECT_UNLOCK (clock); return FALSE; } } @@ -1034,6 +1029,9 @@ do_linear_regression (GstClock * clock, GstClockTime * m_num, sxy += newx4 * newy4 - xbar4 * ybar4; } + if (sxx == 0) + goto invalid; + *m_num = sxy; *m_denom = sxx; *xbase = xmin; @@ -1046,6 +1044,11 @@ do_linear_regression (GstClock * clock, GstClockTime * m_num, DEBUG (" r2 = %g", *r_squared); return TRUE; + +invalid: + { + return FALSE; + } } /** @@ -1088,7 +1091,8 @@ gst_clock_add_observation (GstClock * clock, GstClockTime slave, if (clock->filling && clock->time_index < clock->window_threshold) goto filling; - do_linear_regression (clock, &m_num, &m_denom, &b, &xbase, r_squared); + if (!do_linear_regression (clock, &m_num, &m_denom, &b, &xbase, r_squared)) + goto invalid; GST_CLOCK_SLAVE_UNLOCK (clock); @@ -1096,6 +1100,7 @@ gst_clock_add_observation (GstClock * clock, GstClockTime slave, "adjusting clock to m=%" G_GUINT64_FORMAT "/%" G_GUINT64_FORMAT ", b=%" G_GUINT64_FORMAT " (rsquared=%g)", m_num, m_denom, b, *r_squared); + /* if we have a valid regression, adjust the clock */ gst_clock_set_calibration (clock, xbase, b, m_num, m_denom); return TRUE; @@ -1103,9 +1108,14 @@ gst_clock_add_observation (GstClock * clock, GstClockTime slave, filling: { GST_CLOCK_SLAVE_UNLOCK (clock); - return FALSE; } +invalid: + { + /* no valid regression has been done, ignore the result then */ + GST_CLOCK_SLAVE_UNLOCK (clock); + return TRUE; + } } static void diff --git a/gst/gstutils.c b/gst/gstutils.c index 361684ab41..264587387f 100644 --- a/gst/gstutils.c +++ b/gst/gstutils.c @@ -356,6 +356,102 @@ typedef union } l; } GstUInt64; +static guint64 +gst_util_div128_64_iterate (GstUInt64 c1, GstUInt64 c0, guint64 denom) +{ + gint i; + gint64 mask; + GstUInt64 a0; + + /* full 128/64 case, very slow... */ + /* quotient is c1, c0 */ + a0.ll = 0; /* remainder a0 */ + + /* This can be done faster, inspiration in Hacker's Delight p152 */ + for (i = 0; i < 128; i++) { + /* shift 192 bits remainder:quotient, we only need to + * check the top bit since denom is only 64 bits. */ + /* sign extend top bit into mask */ + mask = ((gint32) a0.l.high) >> 31; + mask |= (a0.ll = (a0.ll << 1) | (c1.l.high >> 31)); + c1.ll = (c1.ll << 1) | (c0.l.high >> 31); + c0.ll <<= 1; + + /* if remainder >= denom or top bit was set */ + if (mask >= denom) { + a0.ll -= denom; + c0.ll += 1; + } + } + return c0.ll; +} + +/* based on Hacker's Delight p152 */ +static guint64 +gst_util_div128_64 (GstUInt64 c1, GstUInt64 c0, guint64 denom) +{ + GstUInt64 q1, q0, rhat; + GstUInt64 v, cmp1, cmp2; + guint s; + + v.ll = denom; + + /* count number of leading zeroes, we know they must be in the high + * part of denom since denom > G_MAXUINT32. */ + s = v.l.high | (v.l.high >> 1); + s |= (s >> 2); + s |= (s >> 4); + s |= (s >> 8); + s = ~(s | (s >> 16)); + s = s - ((s >> 1) & 0x55555555); + s = (s & 0x33333333) + ((s >> 2) & 0x33333333); + s = (s + (s >> 4)) & 0x0f0f0f0f; + s += (s >> 8); + s = (s + (s >> 16)) & 0x3f; + + /* normalize divisor and dividend */ + v.ll <<= s; + c1.ll = (c1.ll << s) | ((c0.l.high >> (32 - s)) & (-s >> 31)); + c0.ll <<= s; + + q1.ll = c1.ll / v.l.high; + rhat.ll = c1.ll - q1.ll * v.l.high; + + cmp1.l.high = rhat.l.low; + cmp1.l.low = c0.l.high; + cmp2.ll = q1.ll * v.l.low; + + while (q1.l.high || cmp2.ll > cmp1.ll) { + q1.ll--; + rhat.ll += v.l.high; + if (rhat.l.high) + break; + cmp1.l.high = rhat.l.low; + cmp2.ll -= v.l.low; + } + c1.l.high = c1.l.low; + c1.l.low = c0.l.high; + c1.ll -= q1.ll * v.ll; + q0.ll = c1.ll / v.l.high; + rhat.ll = c1.ll - q0.ll * v.l.high; + + cmp1.l.high = rhat.l.low; + cmp1.l.low = c0.l.low; + cmp2.ll = q0.ll * v.l.low; + + while (q0.l.high || cmp2.ll > cmp1.ll) { + q0.ll--; + rhat.ll += v.l.high; + if (rhat.l.high) + break; + cmp1.l.high = rhat.l.low; + cmp2.ll -= v.l.low; + } + q0.l.high += q1.l.low; + + return q0.ll; +} + static guint64 gst_util_uint64_scale_int64 (guint64 val, guint64 num, guint64 denom) { @@ -405,36 +501,15 @@ gst_util_uint64_scale_int64 (guint64 val, guint64 num, guint64 denom) result.l.high = c1.ll / den; result.l.low = c0.ll / den; } else { - gint i; - gint64 mask; - - /* full 128/64 case, very slow... */ - /* quotient is c1, c0 */ - a0.ll = 0; /* remainder a0 */ - - /* This can be done faster, inspiration in Hacker's Delight p152 */ - for (i = 0; i < 128; i++) { - /* shift 192 bits remainder:quotient, we only need to - * check the top bit since denom is only 64 bits. */ - /* sign extend top bit into mask */ - mask = ((gint32) a0.l.high) >> 31; - mask |= (a0.ll = (a0.ll << 1) | (c1.l.high >> 31)); - c1.ll = (c1.ll << 1) | (c0.l.high >> 31); - c0.ll <<= 1; - - /* if remainder >= denom or top bit was set */ - if (mask >= denom) { - a0.ll -= denom; - c0.ll += 1; - } - } - result.ll = c0.ll; + if (TRUE) + result.ll = gst_util_div128_64_iterate (c1, c0, denom); + else + result.ll = gst_util_div128_64 (c1, c0, denom); } return result.ll; overflow: { - g_warning ("int64 scaling overflow"); return G_MAXUINT64; } } @@ -456,6 +531,9 @@ gst_util_uint64_scale (guint64 val, guint64 num, guint64 denom) { g_return_val_if_fail (denom != 0, G_MAXUINT64); + if (num == 0) + return 0; + /* if the denom is high, we need to do a 64 muldiv */ if (denom > G_MAXINT32) goto do_int64; @@ -500,6 +578,9 @@ gst_util_uint64_scale_int (guint64 val, gint num, gint denom) g_return_val_if_fail (denom > 0, G_MAXUINT64); g_return_val_if_fail (num >= 0, G_MAXUINT64); + if (num == 0) + return 0; + if (val <= G_MAXUINT32) { /* simple case */ result.ll = val * num / denom; @@ -525,7 +606,6 @@ gst_util_uint64_scale_int (guint64 val, gint num, gint denom) overflow: { - g_warning ("int scaling overflow"); return G_MAXUINT64; } } diff --git a/tests/check/gst/gstutils.c b/tests/check/gst/gstutils.c index 465d278a59..00b48b017f 100644 --- a/tests/check/gst/gstutils.c +++ b/tests/check/gst/gstutils.c @@ -192,9 +192,8 @@ GST_START_TEST (test_math_scale) G_MAXINT32) != G_MAXUINT64 - 100); /* overflow */ - ASSERT_WARNING (gst_util_uint64_scale_int (G_MAXUINT64 - 1, 10, - 1) != G_MAXUINT64); - ASSERT_WARNING (gst_util_uint64_scale_int (G_MAXUINT64 - 1, G_MAXINT32, + fail_if (gst_util_uint64_scale_int (G_MAXUINT64 - 1, 10, 1) != G_MAXUINT64); + fail_if (gst_util_uint64_scale_int (G_MAXUINT64 - 1, G_MAXINT32, 1) != G_MAXUINT64); } GST_END_TEST; @@ -229,12 +228,34 @@ GST_START_TEST (test_math_scale_uint64) G_MAXUINT64) != G_MAXUINT64 - 100); /* overflow */ - ASSERT_WARNING (gst_util_uint64_scale (G_MAXUINT64 - 1, 10, - 1) != G_MAXUINT64); - ASSERT_WARNING (gst_util_uint64_scale (G_MAXUINT64 - 1, G_MAXUINT64, + fail_if (gst_util_uint64_scale (G_MAXUINT64 - 1, 10, 1) != G_MAXUINT64); + fail_if (gst_util_uint64_scale (G_MAXUINT64 - 1, G_MAXUINT64, 1) != G_MAXUINT64); } GST_END_TEST; + +GST_START_TEST (test_math_scale_random) +{ + guint64 val, num, denom, res;; + GRand *rand; + gint i; + + rand = g_rand_new (); + + i = 1000; + while (i--) { + val = ((guint64) g_rand_int (rand)) << 32 | g_rand_int (rand); + num = ((guint64) g_rand_int (rand)) << 32 | g_rand_int (rand); + denom = ((guint64) g_rand_int (rand)) << 32 | g_rand_int (rand); + + res = gst_util_uint64_scale (val, num, denom); + } + g_rand_free (rand); + +} + +GST_END_TEST; + Suite * gst_utils_suite (void) { @@ -246,6 +267,7 @@ gst_utils_suite (void) tcase_add_test (tc_chain, test_buffer_probe_once); tcase_add_test (tc_chain, test_math_scale); tcase_add_test (tc_chain, test_math_scale_uint64); + tcase_add_test (tc_chain, test_math_scale_random); return s; }