From 440b592f640bdc09577e88fba4f28a4e0571b56a Mon Sep 17 00:00:00 2001 From: Mke Brady Date: Thu, 1 Oct 2015 08:21:21 +0100 Subject: [PATCH] Make automatic latency selection default. Warn of deprecation of static latencies --- rtsp.c | 13 +++++-- scripts/shairport-sync.conf | 25 +++++++------ shairport.c | 71 +++++++++++++++++++++++++++---------- 3 files changed, 77 insertions(+), 32 deletions(-) diff --git a/rtsp.c b/rtsp.c index 7e4c077c..4066a2db 100644 --- a/rtsp.c +++ b/rtsp.c @@ -674,13 +674,16 @@ static void handle_setup(rtsp_conn_info *conn, rtsp_message *req, rtsp_message * } #endif - // select latency + // This latency-setting mechanism is deprecated and will be removed. + // If no non-standard latency is chosen, automatic negotiated latency setting is permitted. + + // Select a static latency // if iTunes V10 or later is detected, use the iTunes latency setting // if AirPlay is detected, use the AirPlay latency setting // for everything else, use the general latency setting, if given, or // else use the default latency setting - config.latency = 88200; + config.latency = -1; if (config.userSuppliedLatency) config.latency = config.userSuppliedLatency; @@ -716,6 +719,12 @@ static void handle_setup(rtsp_conn_info *conn, rtsp_message *req, rtsp_message * debug(2, "Unrecognised User-Agent. Using latency of %d frames.", config.latency); } } + + if (config.latency==-1) { + // this means that no static latency was set, so we'll allow it to be set dynamically + config.latency=88200; // to be sure, to be sure + config.use_negotiated_latencies = 1; + } char *hdr = msg_get_header(req, "Transport"); if (!hdr) goto error; diff --git a/scripts/shairport-sync.conf b/scripts/shairport-sync.conf index a2975789..8a4dbf72 100644 --- a/scripts/shairport-sync.conf +++ b/scripts/shairport-sync.conf @@ -19,17 +19,6 @@ general = // 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. }; -// Latencies for different sources. These have been estimated from listening tests. -// It's probably better to compensate for a delay in the output device using the alsa "audio_backend_latency_offset" setting -- see below. -latencies = -{ -// use_negotiated_latencies = "no"; // set this to "yes" to negotiate latencies with the client, and ignore the static settings listed below -// default = 88200; // used for unrecognised sources and for iTunes up to and including iTunes 9.X. -// itunes = 99400; // used for iTunes 10 or later -// airplay = 88200; -// forkedDaapd = 99400; -}; - // How to deal with metadata, including artwork metadata = { @@ -76,3 +65,17 @@ stdout = // audio_backend_latency_offset = 0; // Set this offset to compensate for a fixed delay in the audio back end. E.g. if the output device delays by 100 ms, set this to -4410. // audio_backend_buffer_desired_length = 44100; // Having started to send audio at the right time, send all subsequent audio this many frames ahead of time, creating a buffer this size. }; + +// The following static latency settings are deprecated -- do not use them for new installations. Shairport Sync now sets latencies automatically. +// To compensate for a delay in the output device using the alsa/stdio/pipe "audio_backend_latency_offset" setting. + +// Static latencies for different sources. These have been estimated from listening tests. +latencies = +{ +// default = 88200; // used for unrecognised sources and for iTunes up to and including iTunes 9.X. +// itunes = 99400; // used for iTunes 10 or later +// airplay = 88200; +// forkedDaapd = 99400; +}; + + diff --git a/shairport.c b/shairport.c index cfc76622..beb8d21b 100644 --- a/shairport.c +++ b/shairport.c @@ -399,29 +399,19 @@ int parse_options(int argc, char **argv) { die("Invalid ignore_volume_control option choice \"%s\". It should be \"yes\" or \"no\""); } - /* Get the dynamic latencies setting. */ - if (config_lookup_string(config.cfg, "latencies.use_negotiated_latencies", &str)) { - if (strcasecmp(str, "no") == 0) - config.use_negotiated_latencies = 0; - else if (strcasecmp(str, "yes") == 0) - config.use_negotiated_latencies = 1; - else - die("Invalid use_negotiated_latencies option choice \"%s\". It should be \"yes\" or \"no\""); - } - - /* Get the default latency. */ + /* Get the default latency. Deprecated! */ if (config_lookup_int(config.cfg, "latencies.default", &value)) - config.latency = value; + config.userSuppliedLatency = value; - /* Get the itunes latency. */ + /* Get the itunes latency. Deprecated! */ if (config_lookup_int(config.cfg, "latencies.itunes", &value)) config.iTunesLatency = value; - /* Get the AirPlay latency. */ + /* Get the AirPlay latency. Deprecated! */ if (config_lookup_int(config.cfg, "latencies.airplay", &value)) config.AirPlayLatency = value; - /* Get the forkedDaapd latency. */ + /* Get the forkedDaapd latency. Deprecated! */ if (config_lookup_int(config.cfg, "latencies.forkedDaapd", &value)) config.ForkedDaapdLatency = value; @@ -648,14 +638,14 @@ int main(int argc, char **argv) { config.configfile = configuration_file_path; config.statistics_requested = 0; // don't print stats in the log - config.latency = 88200; // AirPlay. Is also reset in rtsp.c when play is about to start + config.latency = -1; // -1 means not set. 88200 works well. This is also reset in rtsp.c when play is about to start config.userSuppliedLatency = 0; // zero means none supplied - config.iTunesLatency = 99400; // this seems to work pretty well for iTunes from Version 10 (?) + config.iTunesLatency = -1; // -1 means not supplied. 99400 seems to work pretty well for iTunes from Version 10 (?) // upwards-- two left-ear headphones, one from the iMac jack, one // from an NSLU2 running a cheap "3D Sound" USB Soundcard - config.AirPlayLatency = 88200; // this seems to work pretty well for AirPlay -- Syncs sound and + config.AirPlayLatency = -1; // -1 means not set. 88200 seems to work well for AirPlay -- Syncs sound and // vision on AppleTV, but also used for iPhone/iPod/iPad sources - config.ForkedDaapdLatency = 99400; // Seems to be right + config.ForkedDaapdLatency = -1; // -1 means not set. 99400 seems to be right config.resyncthreshold = 441 * 5; // this number of frames is 50 ms config.timeout = 120; // this number of seconds to wait for [more] audio before switching to idle. config.tolerance = 88; // this number of frames of error before attempting to correct it. @@ -833,6 +823,49 @@ int main(int argc, char **argv) { daemon_log(LOG_NOTICE, "startup"); + /* Mess around with the latency options */ + // Basically, we used to rely on static latencies -- 99400 for iTunes 10 or later and forkedDaapd, 88200 for everything else + // Nowadays we allow the source to set the latency, which works out at 99651 for iTunes 10 and forkedDaapd and 88220 for everything else + // What we want to do here is allow the source to set the latency unless the user has specified an non-standard latency. + // If the user has specified a standard latency, we suggest to them to stop doing it. + // If they specify a non-standard latency, we suggest the user to use the audio_backend_latency_offset instead. + + if (config.AirPlayLatency!=-1) { + if (config.AirPlayLatency==88200) { + inform("It is not necessary to set the AirPlay latency to 88200 -- you should remove this setting or configuration option, as it is deprecated."); + config.AirPlayLatency= -1; + } else { + inform("The AirPlay 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.iTunesLatency!=-1) { + if (config.iTunesLatency==99400) { + inform("It is not necessary to set the iTunes latency to 99400 -- you should remove this setting or configuration option, as it is deprecated and ignored."); + config.iTunesLatency= -1; + } else { + inform("The iTunes 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.ForkedDaapdLatency!=-1) { + if (config.ForkedDaapdLatency==99400) { + inform("It is not necessary to set the forkedDaapd latency to 99400 -- you should remove this setting or configuration option, as it is deprecated and ignored."); + config.ForkedDaapdLatency= -1; + } else { + inform("The forkedDaapd 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) { + inform("The default 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."); + } + + /* Print out options */ debug(2, "statistics_requester status is %d.", config.statistics_requested);