rtpjitterbuffer: improved rtx-rtt averaging
authorHavard Graff <havard.graff@gmail.com>
Thu, 11 Aug 2016 21:07:44 +0000 (23:07 +0200)
committerOlivier CrĂȘte <olivier.crete@collabora.com>
Wed, 14 Sep 2016 23:37:50 +0000 (19:37 -0400)
The basic idea is this:
1. For *larger* rtx-rtt, weigh a new measurement as before
2. For *smaller* rtx-rtt, be a bit more conservative and weigh a bit less
3. For very large measurements, consider them "outliers"
   and count them a lot less

The idea being that reducing the rtx-rtt is much more harmful then
increasing it, since we don't want to be underestimating the rtt of the
network, and when using this number to estimate the latency you need for
you jitterbuffer, you would rather want it to be a bit larger then a bit
smaller, potentially losing rtx-packets. The "outlier-detector" is there
to prevent a single skewed measurement to affect the outcome too much.
On wireless networks, these are surprisingly common.

https://bugzilla.gnome.org/show_bug.cgi?id=769768

gst/rtpmanager/gstrtpjitterbuffer.c
tests/check/elements/rtpjitterbuffer.c

index 5b372ea..1a71947 100644 (file)
@@ -3543,6 +3543,32 @@ get_rtx_retry_period (GstRtpJitterBufferPrivate * priv,
   return rtx_retry_period;
 }
 
+/*
+  1. For *larger* rtx-rtt, weigh a new measurement as before (1/8th)
+  2. For *smaller* rtx-rtt, be a bit more conservative and weigh a bit less (1/16th)
+  3. For very large measurements (> avg * 2), consider them "outliers"
+     and count them a lot less (1/48th)
+*/
+static void
+update_avg_rtx_rtt (GstRtpJitterBufferPrivate * priv, GstClockTime rtt)
+{
+  gint weight;
+
+  if (priv->avg_rtx_rtt == 0) {
+    priv->avg_rtx_rtt = rtt;
+    return;
+  }
+
+  if (rtt > 2 * priv->avg_rtx_rtt)
+    weight = 48;
+  else if (rtt > priv->avg_rtx_rtt)
+    weight = 8;
+  else
+    weight = 16;
+
+  priv->avg_rtx_rtt = (rtt + (weight - 1) * priv->avg_rtx_rtt) / weight;
+}
+
 static void
 update_rtx_stats (GstRtpJitterBuffer * jitterbuffer, TimerData * timer,
     GstClockTime dts, gboolean success)
@@ -3574,10 +3600,7 @@ update_rtx_stats (GstRtpJitterBuffer * jitterbuffer, TimerData * timer,
   if (timer->num_rtx_retry == timer->num_rtx_received &&
       dts != GST_CLOCK_TIME_NONE && dts > timer->rtx_last) {
     delay = dts - timer->rtx_last;
-    if (priv->avg_rtx_rtt == 0)
-      priv->avg_rtx_rtt = delay;
-    else
-      priv->avg_rtx_rtt = (delay + 7 * priv->avg_rtx_rtt) / 8;
+    update_avg_rtx_rtt (priv, delay);
   } else {
     delay = 0;
   }
index 5e41def..a2d50d9 100644 (file)
@@ -1796,7 +1796,7 @@ GST_START_TEST (test_rtx_duplicate_packet_updates_rtx_stats)
               "rtx-per-packet", G_TYPE_DOUBLE, 1.0,
               "rtx-rtt", G_TYPE_UINT64, (guint64)
               /* Use the rtx-rtt formula. Can be subject to change though. */
-              ((now - rtx_request_6) + 7 * (now - rtx_request_7)) / 8, NULL)));
+              ((now - rtx_request_6) + 47 * (now - rtx_request_7)) / 48, NULL)));
 
   gst_object_unref (testclock);
   gst_harness_teardown (h);