From b9d3a036b11758a503b92eb56de865a8afb8772d Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Tue, 15 May 2018 12:08:01 +0100 Subject: [PATCH] Revise resend interval calculation to spread it out over a long latency. --- player.c | 31 ++++++++++++++++++------------- player.h | 13 ++++++++++++- shairport.c | 28 ++++++++++++++++------------ 3 files changed, 46 insertions(+), 26 deletions(-) diff --git a/player.c b/player.c index 046fded6..61cb2563 100644 --- 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 e6339477..d941c658 100644 --- 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 30dc9d65..9ce8b7de 100644 --- 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);