From 6e7358c76272130e18f818e17c16baccc637a163 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Wed, 27 May 2015 16:36:31 +0000 Subject: [PATCH] Simplify calculation of buffer timing. Rename dac_buffer_queue_desired_length --- RELEASENOTES.md | 2 ++ common.h | 2 +- player.c | 33 +++++++++++++++------------------ scripts/shairport-sync.conf | 8 +++++--- shairport.c | 12 ++++++------ 5 files changed, 29 insertions(+), 28 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index b0a676b8..f2dc05b5 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -12,6 +12,8 @@ When `make install` is executed, a sample configuration is installed or updated If you enable metadata, please note that the option has changed somewhat. The option `-M` has a new long name equivalent: `--metadata-pipename` and the argument you provide must now be the full name of the metadata pipe, e.g. `-M /tmp/shairport-sync-metadata`. +Change the name of the setting "dac_buffer_desired_length" to "backend_buffer_desired_length" + * Enhancements * Shairport Sync now reads settings from the configuration file `/etc/shairport-sync.conf`. This has settings for most command-line options and it's where any new settings will go. A default configuration file will be installed if one doesn't exist, and a sample file configuration file is always installed or updated. Details of settings are provided in the sample file. Shairport Sync relies on the `libconfig` library to read configuration files. For the present, you can disable the new feature (and save the space taken up by `libconfig`) by using the configure option `--without-configfile-support`. * New command-line option `-c ` or `--configfile=` allows you to specify a configuration file other than `/etc/shairport-sync.conf`. diff --git a/common.h b/common.h index fdbe32b0..ed4b8cab 100644 --- a/common.h +++ b/common.h @@ -76,7 +76,7 @@ typedef struct { char *logfile; char *errfile; char *configfile; - uint dac_buffer_queue_desired_length; + uint backend_buffer_desired_length; // this will be the desired number of frames in the audio backend buffer -- the DAC buffer for ALSA } 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 838b0e23..f352938a 100644 --- a/player.c +++ b/player.c @@ -505,21 +505,23 @@ static abuf_t *buffer_get_frame(void) { // debug(1,"First frame seen with timestamp..."); first_packet_timestamp=curframe->timestamp; // we will keep buffering until we are supposed to start playing this - // here, see if we should start playing. We need to know when to allow the packets to be sent to the player - // we will get a fix every second or so, which will be stored as a pair consisting of - // the time when the packet with a particular timestamp should be played. - // it might not be the timestamp of our first packet, however, so we might have to do some calculations. + // Here, see if we should start playing. We need to know when to allow the packets to be sent to the player. + // We will be getting a timing fix every second or so from the source, which will be stored as a pair consisting of - + // i.e. the local time that corresponds to a specific packet timestamp. + // It might not be the timestamp of our first packet, however, so we might have to do some calculations. + // Also, we have to allow for the desired latency. int64_t delta = ((int64_t)first_packet_timestamp-(int64_t)reference_timestamp); - // Now, if the back end can tell us about its latency, we will set the first packet's time to play to be the exact time. - // Otherwise we will set it to the exact time less the config.dac_buffer_queue_desired_length, a proxy for the lead time we want the stream to appear at the output - // We will probably change this in the future to pick up the lead time from the back end itself + // Now, if the back end can tell us about its latency, we will set the first packet's time to play to be the exact time it's supposed to play plus the desired latency. + // Otherwise we will set it to the exact time plus desired latency less the config.backend_buffer_desired_length, a proxy for the lead time we want the stream to appear at the output. + + // We may change this in the future to pick up the lead time from the back end itself if (config.output->delay) first_packet_time_to_play = reference_timestamp_time+((delta+(int64_t)config.latency)<<32)/44100; // using the latency requested... else - first_packet_time_to_play = reference_timestamp_time+((delta+(int64_t)config.latency-(int64_t)config.dac_buffer_queue_desired_length)<<32)/44100; // using the latency requested... + first_packet_time_to_play = reference_timestamp_time+((delta+(int64_t)config.latency-(int64_t)config.backend_buffer_desired_length)<<32)/44100; // using the latency requested... if (local_time_now>=first_packet_time_to_play) { debug(1,"First packet is late! It should have played before now. Flushing 0.1 seconds"); @@ -531,7 +533,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 = config.dac_buffer_queue_desired_length; + uint32_t max_dac_delay = config.backend_buffer_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 @@ -582,13 +584,9 @@ static abuf_t *buffer_get_frame(void) { } } - // here, we work out whether to wait or not - // if we are getting feedback from the backend, we can use that. - // otherwise we have to release a buffer when the time is right. + // Here, we work out whether to release a packet or wait + // We release a buffer when the time is right. - if (config.output->delay) - wait = (ab_buffering || (dac_delay>=config.dac_buffer_queue_desired_length) || (!ab_synced)) && (!please_stop); - else { int do_wait = 1; if ((curframe) && (curframe->ready) && (curframe->timestamp)) { uint32_t reference_timestamp; @@ -597,7 +595,7 @@ static abuf_t *buffer_get_frame(void) { if (reference_timestamp) { // if we have a reference time uint32_t packet_timestamp=curframe->timestamp; int64_t delta = ((int64_t)packet_timestamp-(int64_t)reference_timestamp); - int64_t offset = (int64_t)config.latency-(int64_t)config.dac_buffer_queue_desired_length; + int64_t offset = (int64_t)config.latency-(int64_t)config.backend_buffer_desired_length; int64_t net_offset = delta+offset; int64_t time_to_play = reference_timestamp_time; int64_t net_offset_fp_sec; @@ -617,8 +615,7 @@ static abuf_t *buffer_get_frame(void) { } } wait = (ab_buffering || (do_wait!=0) || (!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 time_to_wait_for_wakeup_fp *= 4*352; // four full 352-frame packets diff --git a/scripts/shairport-sync.conf b/scripts/shairport-sync.conf index e4886b5b..e5c35a9d 100644 --- a/scripts/shairport-sync.conf +++ b/scripts/shairport-sync.conf @@ -14,8 +14,10 @@ general = // drift = 88; // allow this number of frames of drift before correcting it // 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. +// 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. + +// This may well change -- move it to the backend's settings... +// audio_backend_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 = @@ -54,6 +56,6 @@ alsa = // These are parameters for the pipe back end, an experimental back end that directs output to a pipe. pipe = { -// name = "/dev/null"; +// name = "stdout"; }; diff --git a/shairport.c b/shairport.c index 8421f702..472f7d30 100644 --- a/shairport.c +++ b/shairport.c @@ -396,12 +396,12 @@ int parse_options(int argc, char **argv) { 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)) { + /* Get the audio backend's desired buffer size setting. */ + if(config_lookup_int(&config.cfg, "general.audio_backend_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); + die("Invalid audio backend buffer desired length \"%sd\". It should be between 0 and 66150, default is 6615",value); else - config.dac_buffer_queue_desired_length=value; + config.backend_buffer_desired_length=value; } /* Get the default latency. */ @@ -506,7 +506,7 @@ int parse_options(int argc, char **argv) { 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); + debug(2,"dac desired buffer length is %d.",config.backend_buffer_desired_length); #ifdef CONFIG_METADATA debug(2,"metdata enabled is %d.",config.metadata_enabled); debug(2,"metadata pipename is \"%s\".",config.metadata_pipename); @@ -606,7 +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. + config.backend_buffer_desired_length = 6615; // 0.15 seconds. // this is a bit weird, but apparently necessary char* basec = strdup(argv[0]);