ptp clock: Increase tolerance for late follow-up and delay-resp

The follow-up and delay-resp messages carry precise
timestamps for the arrival at the clock master, but
the local return time is unimportant, so we should be very
lenient in accepting them late. Some PTP masters don't
prioritise sending those packets, and we reject all the
responses and never sync - or take forever to do so.

Increase the tolerance to 20x the mean path delay.

Also fix a typo in one debug output that would print
the absolute time of the delay-resp message, not the offset
from the delay-req that it's actually being compared against.

diff --git a/libs/gst/net/gstptpclock.c b/libs/gst/net/gstptpclock.c
index 0be3b90..2035f7f 100644 (file)
--- a/libs/gst/net/gstptpclock.c
+++ b/libs/gst/net/gstptpclock.c
@@ -1101,14 +1101,18 @@ update_ptp_time (PtpDomainData * domain, PtpPendingSync * sync)
 
 #ifdef USE_MEASUREMENT_FILTERING
   /* We check this here and when updating the mean path delay, because
-   * we can get here without a delay response too */
+   * we can get here without a delay response too. The tolerance on
+   * accepting follow-up after a sync is high, because a PTP server
+   * doesn't have to prioritise sending FOLLOW_UP - its purpose is
+   * just to give us the accurate timestamp of the preceding SYNC */
   if (sync->follow_up_recv_time_local != GST_CLOCK_TIME_NONE
       && sync->follow_up_recv_time_local >
-      sync->sync_recv_time_local + 2 * domain->mean_path_delay) {
-    GST_WARNING ("Sync-follow-up delay for domain %u too big: %" GST_TIME_FORMAT
-        " > 2 * %" GST_TIME_FORMAT, domain->domain,
-        GST_TIME_ARGS (sync->follow_up_recv_time_local),
-        GST_TIME_ARGS (domain->mean_path_delay));
+      sync->sync_recv_time_local + 20 * domain->mean_path_delay) {
+    GstClockTimeDiff delay =
+        sync->follow_up_recv_time_local - sync->sync_recv_time_local;
+    GST_WARNING ("Sync-follow-up delay for domain %u too big: %"
+        GST_STIME_FORMAT " > 20 * %" GST_TIME_FORMAT, domain->domain,
+        GST_STIME_ARGS (delay), GST_TIME_ARGS (domain->mean_path_delay));
     synced = FALSE;
     gst_clock_get_calibration (GST_CLOCK_CAST (domain->domain_clock),
         &internal_time, &external_time, &rate_num, &rate_den);
@@ -1366,12 +1370,15 @@ update_mean_path_delay (PtpDomainData * domain, PtpPendingSync * sync)
 #endif
 
 #ifdef USE_MEASUREMENT_FILTERING
+  /* The tolerance on accepting follow-up after a sync is high, because
+   * a PTP server doesn't have to prioritise sending FOLLOW_UP - its purpose is
+   * just to give us the accurate timestamp of the preceding SYNC */
   if (sync->follow_up_recv_time_local != GST_CLOCK_TIME_NONE &&
       domain->mean_path_delay != 0
       && sync->follow_up_recv_time_local >
-      sync->sync_recv_time_local + 2 * domain->mean_path_delay) {
+      sync->sync_recv_time_local + 20 * domain->mean_path_delay) {
     GST_WARNING ("Sync-follow-up delay for domain %u too big: %" GST_TIME_FORMAT
-        " > 2 * %" GST_TIME_FORMAT, domain->domain,
+        " > 20 * %" GST_TIME_FORMAT, domain->domain,
         GST_TIME_ARGS (sync->follow_up_recv_time_local -
             sync->sync_recv_time_local),
         GST_TIME_ARGS (domain->mean_path_delay));
@@ -1393,10 +1400,14 @@ update_mean_path_delay (PtpDomainData * domain, PtpPendingSync * sync)
       sync->delay_resp_recv_time_local - sync->delay_req_send_time_local;
 
 #ifdef USE_MEASUREMENT_FILTERING
-  /* delay_req_delay is a RTT, so 2 times the path delay */
-  if (delay_req_delay > 4 * domain->mean_path_delay) {
+  /* delay_req_delay is a RTT, so 2 times the path delay is what we'd
+   * hope for, but some PTP systems don't prioritise sending DELAY_RESP,
+   * but they must still have placed an accurate reception timestamp.
+   * That means we should be quite tolerant about late DELAY_RESP, and
+   * mostly rely on filtering out jumps in the mean-path-delay elsewhere  */
+  if (delay_req_delay > 20 * domain->mean_path_delay) {
     GST_WARNING ("Delay-request-response delay for domain %u too big: %"
-        GST_TIME_FORMAT " > 4 * %" GST_TIME_FORMAT, domain->domain,
+        GST_TIME_FORMAT " > 20 * %" GST_TIME_FORMAT, domain->domain,
         GST_TIME_ARGS (delay_req_delay),
         GST_TIME_ARGS (domain->mean_path_delay));
     ret = FALSE;