Revise resend interval calculation to spread it out over a long latency.

diff --git a/player.c b/player.c
index 046fded6a534989914e9341f26e08b8ebf7c61c9..61cb25635826f961f9b517a36dba015d8a8ace94 100644 (file)
--- a/player.c
+++ b/player.c
@@ -512,25 +512,30 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, int64_t timestamp
         }
 
         // here, we should check for missing frames
+        int resend_interval = (((250 * 44100) / 352) / 1000); // approximately 250 ms intervals
         const int number_of_resend_attempts = 8;
+        int latency_based_resend_interval =
+            (conn->latency) / (number_of_resend_attempts * conn->max_frames_per_packet);
+        if (latency_based_resend_interval > resend_interval)
+          resend_interval = latency_based_resend_interval;
+
+        if (conn->resend_interval != resend_interval) {
+          debug(1, "Resend interval for latency of %" PRId64 " frames is %d frames.", conn->latency,
+                resend_interval);
+          conn->resend_interval = resend_interval;
+        }
         if (!conn->ab_buffering) {
           int j;
           for (j = 1; j <= number_of_resend_attempts; j++) {
             // check j times, after a short period of has elapsed, assuming 352 frames per packet
-            // int back_step = (((250 * 44100) / 352) / 1000) * (j); // approx 250 ms intervals
-            // int back_step = ((BUFFER_FRAMES-(22050/352)) / number_of_resend_attempts) * j;
-            int back_step = ((seq_diff(conn->ab_read, conn->ab_write,
-                           conn->ab_read)-(22050/352)) / number_of_resend_attempts) * j;
-            if (back_step<(((190 * 44100) / 352) / 1000) * j) {
-              // debug(1,"resend request back_step %d is too small. Reset to %d.",back_step,(((190 * 44100) / 352) / 1000) * j);
-              back_step = (((190 * 44100) / 352) / 1000) * j;
-            }
+
+            int back_step = resend_interval * j;
+
+            int32_t sd = seq_diff(conn->ab_read, conn->ab_write, conn->ab_read);
             int k;
             for (k = -2; k <= 2; k++) {
-              if (back_step <
-                  seq_diff(conn->ab_read, conn->ab_write,
-                           conn->ab_read)) { // if it's within the range of frames in use...
-                int item_to_check = (conn->ab_write - back_step) & 0xffff;
+              if ((back_step + k) < sd) { // if it's within the range of frames in use...
+                int item_to_check = (conn->ab_write - back_step + k) & 0xffff;
                 seq_t next = item_to_check;
                 abuf_t *check_buf = conn->audio_buffer + BUFIDX(next);
                 if ((!check_buf->ready) &&
@@ -539,7 +544,7 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, int64_t timestamp
                   check_buf->resend_level = j;
                   if (config.disable_resend_requests == 0) {
                     rtp_request_resend(next, 1, conn);
-                    if (j >= 6)
+                    if (j >= number_of_resend_attempts - 2)
                       debug(2, "Resend request level #%d for packet %u in range %u to %u.", j, next,
                             conn->ab_read, conn->ab_write);
                     conn->resend_requests++;

diff --git a/player.h b/player.h
index e6339477eb8fd43f9b1c2c53b0de227f5e4a945c..d941c65841b613aa794195935f19bd332f87b7a0 100644 (file)
--- a/player.h
+++ b/player.h
@@ -46,7 +46,17 @@ typedef struct audio_buffer_entry { // decoded audio packets
 } abuf_t;
 
 // default buffer size
-// needs to be a power of 2 because of the way BUFIDX(seqno) works
+// This eeds to be a power of 2 because of the way BUFIDX(seqno) works.
+// 512 is the minimum for normal operation -- it gives 512*352/44100 or just over 4 seconds of
+// buffers.
+// For at least 10 seconds, you need to go to 2048.
+// Resend requests will be spaced out evenly in the latency period, subject to a minimum interval of
+// about 0.25 seconds.
+// Each buffer occupies 352*4 bytes plus about, say, 64 bytes of overhead in various places, say
+// rougly 1,500 bytes per buffer.
+// Thus, 2048 buffers will occupy about 3 megabytes -- no big deal in a normal machine but maybe a
+// problem in an embedded device.
+
 #define BUFFER_FRAMES 1024
 
 typedef struct {
@@ -57,6 +67,7 @@ typedef struct {
 
 typedef struct {
   int connection_number;   // for debug ID purposes, nothing else...
+  int resend_interval;     // this is really just for debugging
   int AirPlayVersion;      // zero if not an AirPlay session. Used to help calculate latency
   int64_t latency;         // the actual latency used for this play session
   int64_t minimum_latency; // set if an a=min-latency: line appears in the ANNOUNCE message; zero

diff --git a/shairport.c b/shairport.c
index 30dc9d6523c3cea4d08376eaff65ff12a85086fe..9ce8b7deee81532d48a33d9cbc8298a57f21ea3d 100644 (file)
--- a/shairport.c
+++ b/shairport.c
@@ -274,7 +274,7 @@ int parse_options(int argc, char **argv) {
       optind = j;
 
   optCon = poptGetContext(NULL, optind, (const char **)argv, optionsTable, 0);
-  if (optCon==NULL)
+  if (optCon == NULL)
     die("Can not get a secondary popt context.");
   poptSetOtherOptionHelp(optCon, "[OPTIONS]* ");
 
@@ -321,7 +321,7 @@ int parse_options(int argc, char **argv) {
   }
 
   poptFreeContext(optCon);
-  
+
   if ((daemonisewith) && (daemonisewithout))
     die("Select either daemonize_with_pid_file or daemonize_without_pid_file -- you have selected "
         "both!");
@@ -892,7 +892,7 @@ int parse_options(int argc, char **argv) {
       optind = j;
 
   optCon = poptGetContext(NULL, optind, (const char **)argv, optionsTable, 0);
-  if (optCon==NULL)
+  if (optCon == NULL)
     die("Can not get a popt context.");
   poptSetOtherOptionHelp(optCon, "[OPTIONS]* ");
 
@@ -1052,13 +1052,13 @@ void shairport_startup_complete(void) {
   }
 }
 
-const char *pid_file_proc(char* fn,size_t max_length) {
+const char *pid_file_proc(char *fn, size_t max_length) {
   if (fn) {
     snprintf(fn, max_length, "%s/%s.pid", config.computed_piddir,
              daemon_pid_file_ident ? daemon_pid_file_ident : "unknown");
     // debug(1,"fn \"%s\".",fn);
   } else {
-    debug(1,"the sise of the buffer for the PID file path is zero.");
+    debug(1, "the sise of the buffer for the PID file path is zero.");
   }
   return fn;
 }
@@ -1358,12 +1358,16 @@ int main(int argc, char **argv) {
   // audio_backend_latency_offset instead.
 
   if (config.userSuppliedLatency) {
-    inform("The fixed latency setting is deprecated, as Shairport Sync can now get the correct "
-           "latency from the source.");
-    inform("Please remove this setting and use the relevant audio_backend_latency_offset setting, "
-           "if necessary, to compensate for delays elsewhere.");
-    if ((config.userSuppliedLatency!=0) && ((config.userSuppliedLatency<4410) || (config.userSuppliedLatency>BUFFER_FRAMES*352-22050)))
-      die("An out-of-range fixed latency has been specified. It must be between 4410 and %d (at 44100 frames per second).",BUFFER_FRAMES*352-22050);
+    inform("The fixed latency setting is deprecated, as Shairport Sync gets the correct "
+           "latency automatically from the source.");
+    inform("Use the audio_backend_latency_offset_in_seconds setting "
+           "instead to compensate for timing issues.");
+    if ((config.userSuppliedLatency != 0) &&
+        ((config.userSuppliedLatency < 4410) ||
+         (config.userSuppliedLatency > BUFFER_FRAMES * 352 - 22050)))
+      die("An out-of-range fixed latency has been specified. It must be between 4410 and %d (at "
+          "44100 frames per second).",
+          BUFFER_FRAMES * 352 - 22050);
   }
 
   /* print out version */
@@ -1380,7 +1384,7 @@ int main(int argc, char **argv) {
   /* Print out options */
   debug(1, "statistics_requester status is %d.", config.statistics_requested);
   debug(1, "daemon status is %d.", config.daemonise);
-  debug(1, "deamon pid file is \"%s\".", pid_file_proc(pid_file_path_string,4096));
+  debug(1, "deamon pid file is \"%s\".", pid_file_proc(pid_file_path_string, 4096));
   debug(1, "rtsp listening port is %d.", config.port);
   debug(1, "udp base port is %d.", config.udp_port_base);
   debug(1, "udp port range is %d.", config.udp_port_range);