From 460f8448247447e851acacdddaffedf101422187 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Wed, 6 May 2015 13:55:58 +0000 Subject: [PATCH] Add new option to set the desired DAC buffer length. Quieten a few debug messages. --- RELEASENOTES.md | 3 ++- common.h | 1 + player.c | 11 ++++------- rtsp.c | 2 +- scripts/shairport-sync.conf | 1 + shairport.c | 19 +++++++++++++++++-- 6 files changed, 26 insertions(+), 11 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 84b25e47..7e7d404a 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -17,7 +17,8 @@ If you enable metadata, please note that the option has changed somewhat. The op * New command-line option `-c ` or `--configfile=` allows you to specify a configuration file other than `/etc/shairport-sync.conf`. * Session Timeout and Allow Session Interruption can now be set independently. This is really some "housekeeping" as referred to above -- it's a kind of a bug fix, where the bug in question is an inappropriate connection of the setting of two parameters. To explain: (1) By default, when a source such as iTunes starts playing to the Shairport Sync device, any other source attempting to start a play session receives a "busy" signal. If a source disappears without warning, Shairport Sync will wait for 120 seconds before dropping the session and allowing another source to start a play session. (2) The command-line option `-t` or `--timeout` allows you to set the wait time before dropping the session. If you set this parameter to `0`, Shairport Sync will not send a "busy" signal, thus allowing another source to interrupt an existing one. (3) The problem is that if you set the parameter to `0`, a session will never be dropped if the source disappears without warning. The (obvious) fix for this is to separate the setting of the two parameters, and this is now done in the configuration file `/etc/shairport-sync.conf` -- please see the settings `allow_session_interruption` and `session_timeout`. The behaviour of the `-t` and `--timeout` command-line options is unchanged but deprecated. - * New Option -- "Ignore Volume Control". If you set this to "yes", the output from Shairport Sync is always set at 100%. This is useful when you want to set the volume locally. Available via the settings file only. + * New Option -- "Ignore Volume Control" ('ignore_volume_control'). If you set this to "yes", the output from Shairport Sync is always set at 100%. This is useful when you want to set the volume locally. Available via the settings file only. + * New Option -- "Set DAC Buffer Desired Length" ('dac_buffer_desired_length'). Default is 6615 frames, or 0.15 seconds. On some slower machines, particularly with metadata processing going on, the DAC buffer can underflow on this setting,. so it might be worth making the buffer larger. A problem on software mixers only is that changes to volume control settings have to propagate through the buffer to be heard, so the larger the buffer, the longer the response time. This is only a problem if you're using a software mixer; hardware mixers are unaffected. Available via the settings file only. * Statistics option correctly reports when no frames are received in a sampling interval. Version 2.3 diff --git a/common.h b/common.h index f5f145ed..fdbe32b0 100644 --- a/common.h +++ b/common.h @@ -76,6 +76,7 @@ typedef struct { char *logfile; char *errfile; char *configfile; + uint dac_buffer_queue_desired_length; } shairport_cfg; //true if Shairport Sync is supposed to be sending output to the output device, false otherwise diff --git a/player.c b/player.c index 21bec3bd..8c168128 100644 --- a/player.c +++ b/player.c @@ -92,8 +92,6 @@ static int late_packet_message_sent; static uint64_t packet_count = 0; static int32_t last_seqno_read; - - // interthread variables static double software_mixer_volume = 1.0; static int fix_volume = 0x10000; @@ -105,7 +103,6 @@ static pthread_mutex_t vol_mutex = PTHREAD_MUTEX_INITIALIZER; #define MAX_PACKET 2048 // DAC buffer occupancy stuff -#define DAC_BUFFER_QUEUE_DESIRED_LENGTH 6615 #define DAC_BUFFER_QUEUE_MINIMUM_LENGTH 5000 typedef struct audio_buffer_entry { // decoded audio packets @@ -517,7 +514,7 @@ static abuf_t *buffer_get_frame(void) { first_packet_time_to_play = reference_timestamp_time+((delta+(int64_t)config.latency)<<32)/44100; // using the latency requested... if (local_time_now>=first_packet_time_to_play) { - debug(2,"First packet is late! It should have played before now. Flushing 0.1 seconds"); + debug(1,"First packet is late! It should have played before now. Flushing 0.1 seconds"); player_flush(first_packet_timestamp+4410); } } @@ -526,7 +523,7 @@ static abuf_t *buffer_get_frame(void) { if (first_packet_time_to_play!=0) { uint32_t filler_size = frame_size; - uint32_t max_dac_delay = DAC_BUFFER_QUEUE_DESIRED_LENGTH; + uint32_t max_dac_delay = config.dac_buffer_queue_desired_length; // if (dac_delay==0) // i.e. if this is the first fill filler_size = 4410; // 0.1 second -- the maximum we'll add to the DAC @@ -576,7 +573,7 @@ static abuf_t *buffer_get_frame(void) { } } } - wait = (ab_buffering || (dac_delay>=DAC_BUFFER_QUEUE_DESIRED_LENGTH) || (!ab_synced)) && (!please_stop); + wait = (ab_buffering || (dac_delay>=config.dac_buffer_queue_desired_length) || (!ab_synced)) && (!please_stop); // wait = (ab_buffering || (seq_diff(ab_read, ab_write) < (config.latency-22000)/(352)) || (!ab_synced)) && (!please_stop); if (wait) { uint64_t time_to_wait_for_wakeup_fp = ((uint64_t)1<<32)/44100; // this is time period of one frame @@ -901,7 +898,7 @@ static void *player_thread_func(void *arg) { // only allow stuffing if there is enough time to do it -- check DAC buffer... if (current_delayrespcode = 200; } diff --git a/scripts/shairport-sync.conf b/scripts/shairport-sync.conf index 03c44d54..781e2b90 100644 --- a/scripts/shairport-sync.conf +++ b/scripts/shairport-sync.conf @@ -16,6 +16,7 @@ general = // resync_threshold = 2205; // a synchronisation error greater than this will cause resynchronisation; 0 disables it // log_verbosity = 0; // "0" means no verbosity, "3" is highest. // ignore_volume_control = "no"; // set this to "yes" if you want the volume to be at 100% no matter what the source's volume control is set to. +// dac_buffer_desired_length = 6615; // If set too small, buffer underflow occurs on low-powered machines. Too long and the response times with software mixer become annoying. }; latencies = diff --git a/shairport.c b/shairport.c index 45cf7b1f..7948eb06 100644 --- a/shairport.c +++ b/shairport.c @@ -341,8 +341,12 @@ int parse_options(int argc, char **argv) { config.output_name=(char *)str; /* Get the port setting. */ - if(config_lookup_int(&config.cfg, "general.port", &value)) - config.port=value; + if(config_lookup_int(&config.cfg, "general.port", &value)) { + if ((value<0) || (value>65535)) + die("Invalid port number \"%sd\". It should be between 0 and 65535, default is 5000",value); + else + config.port=value; + } /* Get the password setting. */ if(config_lookup_string(&config.cfg, "general.password", &str)) @@ -391,6 +395,14 @@ int parse_options(int argc, char **argv) { else die("Invalid ignore_volume_control option choice \"%s\". It should be \"yes\" or \"no\""); } + + /* Get the dac buffer size setting. */ + if(config_lookup_int(&config.cfg, "general.dac_buffer_desired_length", &value)) { + if ((value<0) || (value>66150)) + die("Invalid DAC buffer length \"%sd\". It should be between 0 and 66150, default is 6615",value); + else + config.dac_buffer_queue_desired_length=value; + } /* Get the default latency. */ if(config_lookup_int(&config.cfg, "latencies.default", &value)) @@ -493,6 +505,8 @@ int parse_options(int argc, char **argv) { debug(2,"busy timeout time is %d.",config.timeout); debug(2,"tolerance is %d frames.",config.tolerance); debug(2,"password is \"%s\".",config.password); + debug(2,"ignore_volume_contorl is %d.",config.ignore_volume_control); + debug(2,"dac desired buffer length is %d.",config.dac_buffer_queue_desired_length); #ifdef CONFIG_METADATA debug(2,"metdata enabled is %d.",config.metadata_enabled); debug(2,"metadata pipename is \"%s\".",config.metadata_pipename); @@ -592,6 +606,7 @@ int main(int argc, char **argv) { config.apname = malloc(20 + 100); snprintf(config.apname, 20 + 100, "Shairport Sync on %s", hostname); set_requested_connection_state_to_output(1); // we expect to be able to connect to the output device + config.dac_buffer_queue_desired_length = 6615; // 0.15 seconds. // this is a bit weird, but apparently necessary char* basec = strdup(argv[0]);