never reverse rtp_time. leave a comment explaining

[disorder] / server / speaker.c

diff --git a/server/speaker.c b/server/speaker.c
index aa67b4fe92788c1718760f91349e3654506f6f6b..5d203942bd31820902fe12e27977051ed6a1b915 100644 (file)
--- a/server/speaker.c
+++ b/server/speaker.c
@@ -104,7 +104,7 @@
  *
  * Don't make this too big or arithmetic will start to overflow.
  */
-#define NETWORK_BYTES 1024
+#define NETWORK_BYTES (1024+sizeof(struct rtp_header))
 
 /** @brief Maximum RTP playahead (ms) */
 #define RTP_AHEAD_MS 1000
@@ -726,7 +726,7 @@ static void play(size_t frames) {
      * AVT profile (RFC3551). */
 
     if(idled) {
-      /* There's been a gap.  Fix up the RTP time accordingly. */
+      /* There may have been a gap.  Fix up the RTP time accordingly. */
       struct timeval now;
       uint64_t delta;
       uint64_t target_rtp_time;
@@ -739,13 +739,48 @@ static void play(size_t frames) {
       target_rtp_time = (delta * playing->format.rate
                                * playing->format.channels) / 1000000;
       /* Overflows at ~6 years uptime with 44100Hz stereo */
-      if(target_rtp_time > rtp_time)
+
+      /* rtp_time is the number of samples we've played.  NB that we play
+       * RTP_AHEAD_MS ahead of ourselves, so it may legitimately be ahead of
+       * the value we deduce from time comparison.
+       *
+       * Suppose we have 1s track started at t=0, and another track begins to
+       * play at t=2s.  Suppose RTP_AHEAD_MS=1000 and 44100Hz stereo.  In that
+       * case we'll send 1s of audio as fast as we can, giving rtp_time=88200.
+       * rtp_time stops at this point.
+       *
+       * At t=2s we'll have calculated target_rtp_time=176400.  In this case we
+       * set rtp_time=176400 and the player can correctly conclude that it
+       * should leave 1s between the tracks.
+       *
+       * Suppose instead that the second track arrives at t=0.5s, and that
+       * we've managed to transmit the whole of the first track already.  We'll
+       * have target_rtp_time=44100.
+       *
+       * The desired behaviour is to play the second track back to back with
+       * first.  In this case therefore we do not modify rtp_time.
+       *
+       * Is it ever right to reduce rtp_time?  No; for that would imply
+       * transmitting packets with overlapping timestamp ranges, which does not
+       * make sense.
+       */
+      if(target_rtp_time > rtp_time) {
+        /* More time has elapsed than we've transmitted samples.  That implies
+         * we've been 'sending' silence.  */
         info("advancing rtp_time by %"PRIu64" samples",
              target_rtp_time - rtp_time);
-      else if(target_rtp_time < rtp_time)
-        info("reversing rtp_time by %"PRIu64" samples",
-             rtp_time - target_rtp_time);
-      rtp_time = target_rtp_time;
+        rtp_time = target_rtp_time;
+      } else if(target_rtp_time < rtp_time) {
+        const int64_t samples_ahead = ((uint64_t)RTP_AHEAD_MS
+                                           * config->sample_format.rate
+                                           * config->sample_format.channels
+                                           / 1000);
+        
+        if(target_rtp_time + samples_ahead < rtp_time) {
+          info("reversing rtp_time by %"PRIu64" samples",
+               rtp_time - target_rtp_time);
+        }
+      }
     }
     header.vpxcc = 2 << 6;              /* V=2, P=0, X=0, CC=0 */
     header.seq = htons(rtp_seq++);
@@ -869,6 +904,8 @@ int main(int argc, char **argv) {
     0
   };
   static const int one = 1;
+  int sndbuf, target_sndbuf = 131072;
+  socklen_t len;
   char *sockname, *ssockname;
 #if API_ALSA
   int alsa_nslots = -1, err;
@@ -923,7 +960,17 @@ int main(int argc, char **argv) {
                      res->ai_protocol)) < 0)
       fatal(errno, "error creating broadcast socket");
     if(setsockopt(bfd, SOL_SOCKET, SO_BROADCAST, &one, sizeof one) < 0)
-      fatal(errno, "error settting SO_BROADCAST on broadcast socket");
+      fatal(errno, "error setting SO_BROADCAST on broadcast socket");
+    len = sizeof sndbuf;
+    if(getsockopt(bfd, SOL_SOCKET, SO_SNDBUF,
+                  &sndbuf, &len) < 0)
+      fatal(errno, "error getting SO_SNDBUF");
+    if(setsockopt(bfd, SOL_SOCKET, SO_SNDBUF,
+                  &target_sndbuf, sizeof target_sndbuf) < 0)
+      error(errno, "error setting SO_SNDBUF to %d", target_sndbuf);
+    else
+      info("changed socket send buffer size from %d to %d",
+           sndbuf, target_sndbuf);
     /* We might well want to set additional broadcast- or multicast-related
      * options here */
     if(sres && bind(bfd, sres->ai_addr, sres->ai_addrlen) < 0)
@@ -975,7 +1022,9 @@ int main(int argc, char **argv) {
                                            * config->sample_format.rate
                                            * config->sample_format.channels
                                            / 1000);
+#if 0
         static unsigned logit;
+#endif
 
         /* If we're starting then initialize the base time */
         if(!rtp_time)
@@ -989,7 +1038,7 @@ int main(int argc, char **argv) {
                                      * config->sample_format.channels)
 
                           / 1000000;
-#if 1
+#if 0
         /* TODO remove logging guff */
         if(!(logit++ & 1023))
           info("rtp_time %llu target %llu difference %lld [%lld]",