From ca325d0459c657b8e4d0847f5575c28defd8973a Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Sun, 24 Feb 2019 13:16:55 +0000 Subject: [PATCH 01/17] Fix an error in non_blocking_write, expose a more general version of it, use it when writing an RTSP response with a three-second timeout. --- common.c | 13 +++++++++---- common.h | 2 ++ rtsp.c | 4 ++-- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/common.c b/common.c index cf5c0730..08b5bed1 100644 --- a/common.c +++ b/common.c @@ -955,7 +955,8 @@ uint64_t get_absolute_time_in_fp() { return time_now_fp; } -ssize_t non_blocking_write(int fd, const void *buf, size_t count) { +ssize_t non_blocking_write_with_timeout(int fd, const void *buf, size_t count, int timeout) { + // timeout is in milliseconds void *ibuf = (void *)buf; size_t bytes_remaining = count; int rc = 1; @@ -964,7 +965,7 @@ ssize_t non_blocking_write(int fd, const void *buf, size_t count) { // check that we can do some writing ufds[0].fd = fd; ufds[0].events = POLLOUT; - rc = poll(ufds, 1, 5000); + rc = poll(ufds, 1, timeout); if (rc < 0) { // debug(1, "non-blocking write error waiting for pipe to become ready for writing..."); } else if (rc == 0) { @@ -975,20 +976,24 @@ ssize_t non_blocking_write(int fd, const void *buf, size_t count) { ssize_t bytes_written = write(fd, ibuf, bytes_remaining); if (bytes_written == -1) { // debug(1,"Error %d in non_blocking_write: \"%s\".",errno,strerror(errno)); - rc = -1; + rc = bytes_written; // to imitate the return from write() } else { ibuf += bytes_written; bytes_remaining -= bytes_written; } } } - if (rc == 0) + if (rc > 0) return count - bytes_remaining; // this is just to mimic a normal write/3. else return rc; // return write(fd,buf,count); } +ssize_t non_blocking_write(int fd, const void *buf, size_t count) { + return non_blocking_write_with_timeout(fd,buf,count,5000); // default is 5 seconds. +} + /* from * http://coding.debuntu.org/c-implementing-str_replace-replace-all-occurrences-substring#comment-722 */ diff --git a/common.h b/common.h index e3e02635..9bc849e1 100644 --- a/common.h +++ b/common.h @@ -251,6 +251,8 @@ int get_requested_connection_state_to_output(); void set_requested_connection_state_to_output(int v); +ssize_t non_blocking_write_with_timeout(int fd, const void *buf, size_t count, int timeout); // timeout in milliseconds + ssize_t non_blocking_write(int fd, const void *buf, size_t count); // used in a few places /* from diff --git a/rtsp.c b/rtsp.c index fb4ed4f3..6279d7a1 100644 --- a/rtsp.c +++ b/rtsp.c @@ -779,7 +779,7 @@ int msg_write_response(int fd, rtsp_message *resp) { debug(1, "Attempted to write overlong RTSP packet 3"); return -3; } - ssize_t reply = write(fd, pkt, p - pkt); + ssize_t reply = non_blocking_write_with_timeout(fd, pkt, p - pkt, 3000); // wait three seconds (3,000 milliseconds) for it to become available if (reply == -1) { char errorstring[1024]; strerror_r(errno, (char *)errorstring, sizeof(errorstring)); @@ -787,7 +787,7 @@ int msg_write_response(int fd, rtsp_message *resp) { return -4; } if (reply != p - pkt) { - debug(1, "msg_write_response error -- requested bytes not fully written."); + debug(1, "msg_write_response error -- requested bytes: %d not fully written: %d.",p - pkt, reply); return -5; } return 0; From b26d8155292057e4acf400dc8e40f321e6137a7b Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Mon, 25 Feb 2019 11:38:08 +0000 Subject: [PATCH 02/17] Add indication of dacp-id, source and self IP numbers to some diagnostics. --- rtsp.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/rtsp.c b/rtsp.c index 6279d7a1..923f4ee7 100644 --- a/rtsp.c +++ b/rtsp.c @@ -986,8 +986,10 @@ void handle_setup(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { msg_add_header(resp, "Session", "1"); resp->respcode = 200; // it all worked out okay - debug(1, "Connection %d: SETUP with UDP ports Control: %d, Timing: %d and Audio: %d.", - conn->connection_number, conn->local_control_port, conn->local_timing_port, + debug(1, "Connection %d: SETUP DACP-ID \"%s\" from %s to %s with UDP ports Control: %d, Timing: %d and Audio: %d.", + conn->connection_number, + conn->dacp_id, &conn->client_ip_string, &conn->self_ip_string, + conn->local_control_port, conn->local_timing_port, conn->local_audio_port); } else { @@ -1882,8 +1884,10 @@ static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_messag resp->respcode = 200; } else { resp->respcode = 453; - debug(1, "Connection %d: failed because a connection is already playing.", - conn->connection_number); + debug(1, "Connection %d: DACP-ID \"%s\" on %s failed because a connection is already playing.", + conn->connection_number, + conn->dacp_id, + conn->client_ip_string); } out: From 20967812ad76e39dec183fd66420212dd23b6ee1 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Tue, 26 Feb 2019 15:23:11 +0000 Subject: [PATCH 03/17] Fix a bug in the activity_monitor software that could lock up a player thread so that it couln't be cancelled. If a player was cancelled while performing an activity script, the activity_monitor_mutex would remain locked. --- activity_monitor.c | 8 +-- audio_alsa.c | 124 +++++++++++++++++++++++++++++---------------- audio_sndio.c | 3 +- common.c | 3 ++ common.h | 1 + player.c | 12 +++++ rtsp.c | 10 ++-- shairport.c | 2 +- 8 files changed, 109 insertions(+), 54 deletions(-) diff --git a/activity_monitor.c b/activity_monitor.c index 51eeb0a2..3c525c19 100644 --- a/activity_monitor.c +++ b/activity_monitor.c @@ -111,7 +111,9 @@ void going_inactive(int block) { } void activity_monitor_signify_activity(int active) { - pthread_mutex_lock(&activity_monitor_mutex); + // this could be pthread_cancelled and they is likely to be cancellation points in the + // hooked-on procedures + pthread_cleanup_debug_mutex_lock(&activity_monitor_mutex, 10000, 1); player_state = active == 0 ? ps_inactive : ps_active; // Now, although we could simply let the state machine in the activity monitor thread // look after eveything, we will change state here in two situations: @@ -123,7 +125,7 @@ void activity_monitor_signify_activity(int active) { // // The reason for all this is that we might want to perform the attached scripts // and wait for them to complete before continuing. If they were perfomed in the - // activity monitor thread, then we coldn't wait for them to complete. + // activity monitor thread, then we couldn't wait for them to complete. // Thus, the only time the thread will execute a going_... function is when a non-zero // timeout actually matures. @@ -138,7 +140,7 @@ void activity_monitor_signify_activity(int active) { } pthread_cond_signal(&activity_monitor_cv); - pthread_mutex_unlock(&activity_monitor_mutex); + pthread_cleanup_pop(1); // release the mutex } void activity_thread_cleanup_handler(__attribute__((unused)) void *arg) { diff --git a/audio_alsa.c b/audio_alsa.c index ee134a54..5896b431 100644 --- a/audio_alsa.c +++ b/audio_alsa.c @@ -323,48 +323,7 @@ int actual_open_alsa_device(void) { snd_strerror(ret)); return ret; } - snd_pcm_format_t sf; - switch (sample_format) { - case SPS_FORMAT_S8: - sf = SND_PCM_FORMAT_S8; - frame_size = 2; - break; - case SPS_FORMAT_U8: - sf = SND_PCM_FORMAT_U8; - frame_size = 2; - break; - case SPS_FORMAT_S16: - sf = SND_PCM_FORMAT_S16; - frame_size = 4; - break; - case SPS_FORMAT_S24: - sf = SND_PCM_FORMAT_S24; - frame_size = 8; - break; - case SPS_FORMAT_S24_3LE: - sf = SND_PCM_FORMAT_S24_3LE; - frame_size = 6; - break; - case SPS_FORMAT_S24_3BE: - sf = SND_PCM_FORMAT_S24_3BE; - frame_size = 6; - break; - case SPS_FORMAT_S32: - sf = SND_PCM_FORMAT_S32; - frame_size = 8; - break; - default: - sf = SND_PCM_FORMAT_S16; // this is just to quieten a compiler warning - frame_size = 4; - debug(1, "Unsupported output format at audio_alsa.c"); - return -EINVAL; - } - ret = snd_pcm_hw_params_set_format(alsa_handle, alsa_params, sf); - if (ret < 0) { - warn("audio_alsa: Sample format %d not available for device \"%s\": %s", sample_format, - alsa_out_dev, snd_strerror(ret)); - return ret; - } + ret = snd_pcm_hw_params_set_channels(alsa_handle, alsa_params, 2); if (ret < 0) { @@ -379,6 +338,83 @@ int actual_open_alsa_device(void) { snd_strerror(ret)); return ret; } + + + if (sample_format == SPS_FORMAT_AUTO) { + + struct alsa_formats { + snd_pcm_format_t alsa_sound_format; + enum sps_format_t sps_sound_format; + int frame_size; + char * description; + }; + + struct alsa_formats formats[] = {{SND_PCM_FORMAT_S32,SPS_FORMAT_S32,8,"S32"}, + {SND_PCM_FORMAT_S24,SPS_FORMAT_S24,8,"S24"}, + {SND_PCM_FORMAT_S24_3LE,SPS_FORMAT_S24_3LE,6,"S24_LE"}, + {SND_PCM_FORMAT_S24_3BE,SPS_FORMAT_S24_3BE,6,"S24_BE"}, + {SND_PCM_FORMAT_S16,SPS_FORMAT_S16,4,"S16"}}; + + int acceptable_format_found = 0; + unsigned int i = 0; + while((!acceptable_format_found) && (i < sizeof(formats) / sizeof(formats[0]))) { + if (snd_pcm_hw_params_set_format(alsa_handle, alsa_params, formats[i].alsa_sound_format) == 0) { + acceptable_format_found = 1; + frame_size = formats[i].frame_size; + config.output_format = formats[i].sps_sound_format; + debug(1,"alsa: output format automatically chosen to be \"%s\".",formats[i].description); + } else { + i++; + } + } + if (!acceptable_format_found) { + die("Can't find a suitable automatic output format setting."); + } + } else { + snd_pcm_format_t sf; + switch (sample_format) { + case SPS_FORMAT_S8: + sf = SND_PCM_FORMAT_S8; + frame_size = 2; + break; + case SPS_FORMAT_U8: + sf = SND_PCM_FORMAT_U8; + frame_size = 2; + break; + case SPS_FORMAT_S16: + sf = SND_PCM_FORMAT_S16; + frame_size = 4; + break; + case SPS_FORMAT_S24: + sf = SND_PCM_FORMAT_S24; + frame_size = 8; + break; + case SPS_FORMAT_S24_3LE: + sf = SND_PCM_FORMAT_S24_3LE; + frame_size = 6; + break; + case SPS_FORMAT_S24_3BE: + sf = SND_PCM_FORMAT_S24_3BE; + frame_size = 6; + break; + case SPS_FORMAT_S32: + sf = SND_PCM_FORMAT_S32; + frame_size = 8; + break; + default: + sf = SND_PCM_FORMAT_S16; // this is just to quieten a compiler warning + frame_size = 4; + debug(1, "Unsupported output format at audio_alsa.c"); + return -EINVAL; + } + + ret = snd_pcm_hw_params_set_format(alsa_handle, alsa_params, sf); + if (ret < 0) { + warn("audio_alsa: Sample format %d not available for device \"%s\": %s", sample_format, + alsa_out_dev, snd_strerror(ret)); + return ret; + } + } if (set_period_size_request != 0) { debug(1, "Attempting to set the period size"); @@ -898,8 +934,10 @@ static int init(int argc, char **argv) { config.output_format = SPS_FORMAT_U8; else if (strcasecmp(str, "S8") == 0) config.output_format = SPS_FORMAT_S8; + else if (strcasecmp(str, "AUTOMATIC") == 0) + config.output_format = SPS_FORMAT_AUTO; else { - warn("Invalid output format \"%s\". It should be \"U8\", \"S8\", " + warn("Invalid output format \"%s\". It should be \"Automatic\", \"U8\", \"S8\", " "\"S16\", \"S24\", " "\"S24_3LE\", \"S24_3BE\" or " "\"S32\". It is set to \"S16\".", diff --git a/audio_sndio.c b/audio_sndio.c index f22ed5af..3a544b9a 100644 --- a/audio_sndio.c +++ b/audio_sndio.c @@ -75,6 +75,7 @@ struct sndio_formats { static struct sndio_formats formats[] = {{"S8", SPS_FORMAT_S8, 8, 1, 1, SIO_LE_NATIVE}, {"U8", SPS_FORMAT_U8, 8, 1, 0, SIO_LE_NATIVE}, {"S16", SPS_FORMAT_S16, 16, 2, 1, SIO_LE_NATIVE}, + {"AUTOMATIC", SPS_FORMAT_S16, 16, 2, 1, SIO_LE_NATIVE}, // TODO: make this really automatic? {"S24", SPS_FORMAT_S24, 24, 4, 1, SIO_LE_NATIVE}, {"S24_3LE", SPS_FORMAT_S24_3LE, 24, 3, 1, 1}, {"S24_3BE", SPS_FORMAT_S24_3BE, 24, 3, 1, 0}, @@ -146,7 +147,7 @@ static int init(int argc, char **argv) { } if (!found) die("Invalid output format \"%s\". Should be one of: S8, U8, S16, S24, " - "S24_3LE, S24_3BE, S32", + "S24_3LE, S24_3BE, S32, Automatic", tmp); } } diff --git a/common.c b/common.c index 08b5bed1..ec70e2fe 100644 --- a/common.c +++ b/common.c @@ -1390,6 +1390,9 @@ int64_t generate_zero_frames(char *outp, size_t number_of_frames, enum sps_forma break; case SPS_FORMAT_UNKNOWN: die("Unexpected SPS_FORMAT_UNKNOWN while calculating dither mask."); + break; + case SPS_FORMAT_AUTO: + die("Unexpected SPS_FORMAT_AUTO while calculating dither mask."); } dither_mask -= 1; // int64_t r = r64i(); diff --git a/common.h b/common.h index 9bc849e1..6f92abbe 100644 --- a/common.h +++ b/common.h @@ -80,6 +80,7 @@ enum sps_format_t { SPS_FORMAT_S24_3LE, SPS_FORMAT_S24_3BE, SPS_FORMAT_S32, + SPS_FORMAT_AUTO, } sps_format_t; typedef struct { diff --git a/player.c b/player.c index 72a79ef8..881a7359 100644 --- a/player.c +++ b/player.c @@ -682,6 +682,10 @@ static inline void process_sample(int32_t sample, char **outp, enum sps_format_t break; case SPS_FORMAT_UNKNOWN: die("Unexpected SPS_FORMAT_UNKNOWN while calculating dither mask."); + break; + case SPS_FORMAT_AUTO: + die("Unexpected SPS_FORMAT_AUTO while calculating dither mask."); + break; } dither_mask -= 1; // int64_t r = r64i(); @@ -758,6 +762,10 @@ static inline void process_sample(int32_t sample, char **outp, enum sps_format_t break; case SPS_FORMAT_UNKNOWN: die("Unexpected SPS_FORMAT_UNKNOWN while outputting samples"); + break; + case SPS_FORMAT_AUTO: + die("Unexpected SPS_FORMAT_AUTO while outputting samples"); + break; } *outp += result; @@ -1644,6 +1652,10 @@ void *player_thread_func(void *arg) { break; case SPS_FORMAT_UNKNOWN: die("Unknown format choosing output bit depth"); + break; + case SPS_FORMAT_AUTO: + die("Unexpected SPS_FORMAT_AUTO while outputting samples"); + break; } debug(3, "Output bit depth is %d.", output_bit_depth); diff --git a/rtsp.c b/rtsp.c index 923f4ee7..3f27a982 100644 --- a/rtsp.c +++ b/rtsp.c @@ -187,7 +187,7 @@ int pc_queue_add_item(pc_queue *the_queue, const void *the_stuff, int block) { int rc; if (the_queue) { if (block == 0) { - rc = pthread_mutex_trylock(&the_queue->pc_queue_lock); + rc = debug_mutex_lock(&the_queue->pc_queue_lock, 10000, 2); if (rc == EBUSY) return EBUSY; } else @@ -1718,7 +1718,7 @@ static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_messag } if (have_the_player) { - debug(3, "RTSP conversation thread %d has acquired play lock.", conn->connection_number); + debug(3, "Connection %d: ANNOUNCE has acquired play lock.", conn->connection_number); // now, if this new session did not break in, then it's okay to reset the next UDP ports // to the start of the range @@ -1884,10 +1884,8 @@ static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_messag resp->respcode = 200; } else { resp->respcode = 453; - debug(1, "Connection %d: DACP-ID \"%s\" on %s failed because a connection is already playing.", - conn->connection_number, - conn->dacp_id, - conn->client_ip_string); + debug(1, "Connection %d: ANNOUNCE failed because another connection is already playing.", + conn->connection_number); } out: diff --git a/shairport.c b/shairport.c index 21f4ff52..02aa362b 100644 --- a/shairport.c +++ b/shairport.c @@ -1577,7 +1577,7 @@ int main(int argc, char **argv) { debug(1, "use_mmap_if_available is %d.", config.no_mmap ? 0 : 1); debug(1, "output_rate is %d.", config.output_rate); debug(1, - "output_format is %d (0-unknown, 1-S8, 2-U8, 3-S16, 4-S24, 5-S24_3LE, 6-S24_3BE, 7-S32).", + "output_format is %d (0-unknown, 1-S8, 2-U8, 3-S16, 4-S24, 5-S24_3LE, 6-S24_3BE, 7-S32, 8-Automatic).", config.output_format); debug(1, "audio backend desired buffer length is %f seconds.", config.audio_backend_buffer_desired_length); From 4f1608fcbd184dc098e5daeb1a1971ad70b78c04 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Tue, 26 Feb 2019 15:24:49 +0000 Subject: [PATCH 04/17] Fix a bug in the activity_monitor and add automatic bit-depth selection. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 24c85b90..fc133aa7 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ # Process this file with autoconf to produce a configure script. AC_PREREQ([2.50]) -AC_INIT([shairport-sync], [3.3d54], [mikebrady@eircom.net]) +AC_INIT([shairport-sync], [3.3d55], [mikebrady@eircom.net]) AM_INIT_AUTOMAKE AC_CONFIG_SRCDIR([shairport.c]) AC_CONFIG_HEADERS([config.h]) From 621f70a4df72b7b07fb1e595d4d34c04373bc57a Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Wed, 27 Feb 2019 11:58:03 +0000 Subject: [PATCH 05/17] Remove a change that proved unnecessary to replay a write() with a non-blocking write(). --- rtsp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rtsp.c b/rtsp.c index 3f27a982..5b6748f1 100644 --- a/rtsp.c +++ b/rtsp.c @@ -779,7 +779,7 @@ int msg_write_response(int fd, rtsp_message *resp) { debug(1, "Attempted to write overlong RTSP packet 3"); return -3; } - ssize_t reply = non_blocking_write_with_timeout(fd, pkt, p - pkt, 3000); // wait three seconds (3,000 milliseconds) for it to become available + ssize_t reply = write(fd, pkt, p - pkt); if (reply == -1) { char errorstring[1024]; strerror_r(errno, (char *)errorstring, sizeof(errorstring)); From f74e2c2c9651f8c4ce304d36cce3266dda1cdaf7 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Wed, 27 Feb 2019 12:53:09 +0000 Subject: [PATCH 06/17] Revert some experimental changes made to explore problems with Cygwin D-Bus access. --- dbus-service.c | 126 ++++++++++++++++--------------------------------- 1 file changed, 41 insertions(+), 85 deletions(-) diff --git a/dbus-service.c b/dbus-service.c index a51df7f6..a1cde3fd 100644 --- a/dbus-service.c +++ b/dbus-service.c @@ -20,7 +20,6 @@ int service_is_running = 0; ShairportSyncDiagnostics *shairportSyncDiagnosticsSkeleton = NULL; ShairportSyncRemoteControl *shairportSyncRemoteControlSkeleton = NULL; ShairportSyncAdvancedRemoteControl *shairportSyncAdvancedRemoteControlSkeleton = NULL; -GDBusConnection *ourBusConnection = NULL; guint ownerID = 0; @@ -434,14 +433,12 @@ gboolean notify_drift_tolerance_callback(ShairportSync *skeleton, } gboolean notify_disable_standby_mode_callback(ShairportSync *skeleton, - __attribute__((unused)) gpointer user_data) { + __attribute__((unused)) gpointer user_data) { char *th = (char *)shairport_sync_get_disable_standby_mode(skeleton); - if ((strcasecmp(th, "no") == 0) || (strcasecmp(th, "off") == 0) || - (strcasecmp(th, "never") == 0)) { + if ((strcasecmp(th, "no") == 0) || (strcasecmp(th, "off") == 0) || (strcasecmp(th, "never") == 0)) { config.disable_standby_mode = disable_standby_off; config.keep_dac_busy = 0; - } else if ((strcasecmp(th, "yes") == 0) || (strcasecmp(th, "on") == 0) || - (strcasecmp(th, "always") == 0)) { + } else if ((strcasecmp(th, "yes") == 0) || (strcasecmp(th, "on") == 0) || (strcasecmp(th, "always") == 0)) { config.disable_standby_mode = disable_standby_always; config.keep_dac_busy = 1; } else if (strcasecmp(th, "while_active") == 0) @@ -449,17 +446,17 @@ gboolean notify_disable_standby_mode_callback(ShairportSync *skeleton, else { warn("An unrecognised disable_standby_mode: \"%s\" was requested via D-Bus interface.", th); switch (config.disable_standby_mode) { - case disable_standby_off: - shairport_sync_set_disable_standby_mode(skeleton, "off"); - break; - case disable_standby_always: - shairport_sync_set_disable_standby_mode(skeleton, "always"); - break; - case disable_standby_while_active: - shairport_sync_set_disable_standby_mode(skeleton, "while_active"); - break; - default: - break; + case disable_standby_off: + shairport_sync_set_disable_standby_mode(skeleton, "off"); + break; + case disable_standby_always: + shairport_sync_set_disable_standby_mode(skeleton, "always"); + break; + case disable_standby_while_active: + shairport_sync_set_disable_standby_mode(skeleton, "while_active"); + break; + default: + break; } } return TRUE; @@ -659,7 +656,7 @@ static void on_dbus_name_acquired(GDBusConnection *connection, const gchar *name G_CALLBACK(notify_alacdecoder_callback), NULL); g_signal_connect(shairportSyncSkeleton, "notify::disable-standby-mode", G_CALLBACK(notify_disable_standby_mode_callback), NULL); - g_signal_connect(shairportSyncSkeleton, "notify::volume-control-profile", + g_signal_connect(shairportSyncSkeleton, "notify::volume-control-profile", G_CALLBACK(notify_volume_control_profile_callback), NULL); g_signal_connect(shairportSyncSkeleton, "notify::disable-standby", G_CALLBACK(notify_disable_standby_callback), NULL); @@ -747,21 +744,21 @@ static void on_dbus_name_acquired(GDBusConnection *connection, const gchar *name debug(1, ">> Active set to \"false\""); switch (config.disable_standby_mode) { - case disable_standby_off: - shairport_sync_set_disable_standby_mode(SHAIRPORT_SYNC(shairportSyncSkeleton), "off"); - debug(1, ">> disable standby mode set to \"off\""); - break; - case disable_standby_always: - shairport_sync_set_disable_standby_mode(SHAIRPORT_SYNC(shairportSyncSkeleton), "always"); - debug(1, ">> disable standby mode set to \"always\""); - break; - case disable_standby_while_active: - shairport_sync_set_disable_standby_mode(SHAIRPORT_SYNC(shairportSyncSkeleton), "while_active"); - debug(1, ">> disable standby mode set to \"while_active\""); - break; - default: - debug(1, "invalid disable_standby mode!"); - break; + case disable_standby_off: + shairport_sync_set_disable_standby_mode(SHAIRPORT_SYNC(shairportSyncSkeleton), "off"); + debug(1, ">> disable standby mode set to \"off\""); + break; + case disable_standby_always: + shairport_sync_set_disable_standby_mode(SHAIRPORT_SYNC(shairportSyncSkeleton), "always"); + debug(1, ">> disable standby mode set to \"always\""); + break; + case disable_standby_while_active: + shairport_sync_set_disable_standby_mode(SHAIRPORT_SYNC(shairportSyncSkeleton), "while_active"); + debug(1, ">> disable standby mode set to \"while_active\""); + break; + default: + debug(1,"invalid disable_standby mode!"); + break; } #ifdef CONFIG_SOXR @@ -849,31 +846,6 @@ static void on_dbus_name_acquired(GDBusConnection *connection, const gchar *name service_is_running = 1; } -// from http://www.ccp4.ac.uk/dist/checkout/glib-2.34.3/gio/tests/gdbus-tests.c -/* ---------------------------------------------------------------------------------------------------- - */ - -GDBusConnection *_g_bus_get_priv(GBusType bus_type, GCancellable *cancellable, GError **error) { - gchar *address; - GDBusConnection *ret; - - ret = NULL; - - address = g_dbus_address_get_for_bus_sync(bus_type, cancellable, error); - if (address == NULL) - goto out; - - ret = g_dbus_connection_new_for_address_sync(address, - G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT | - G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION, - NULL, /* GDBusAuthObserver */ - cancellable, error); - g_free(address); - -out: - return ret; -} - static void on_dbus_name_lost_again(__attribute__((unused)) GDBusConnection *connection, __attribute__((unused)) const gchar *name, __attribute__((unused)) gpointer user_data) { @@ -891,17 +863,13 @@ static void on_dbus_name_lost(__attribute__((unused)) GDBusConnection *connectio pid_t pid = getpid(); char interface_name[256] = ""; snprintf(interface_name, sizeof(interface_name), "org.gnome.ShairportSync.i%d", pid); - // GBusType dbus_bus_type = G_BUS_TYPE_SYSTEM; - // if (config.dbus_service_bus_type == DBT_session) - // dbus_bus_type = G_BUS_TYPE_SESSION; + GBusType dbus_bus_type = G_BUS_TYPE_SYSTEM; + if (config.dbus_service_bus_type == DBT_session) + dbus_bus_type = G_BUS_TYPE_SESSION; // debug(1, "Looking for a Shairport Sync native D-Bus interface \"%s\" on the %s bus.", // interface_name,(config.dbus_service_bus_type == DBT_session) ? "session" : "system"); - ownerID = - g_bus_own_name_on_connection(ourBusConnection, interface_name, G_BUS_NAME_OWNER_FLAGS_REPLACE, - on_dbus_name_acquired, on_dbus_name_lost_again, NULL, NULL); - - // g_bus_own_name(dbus_bus_type, interface_name, G_BUS_NAME_OWNER_FLAGS_NONE, NULL, - // on_dbus_name_acquired, on_dbus_name_lost_again, NULL, NULL); + g_bus_own_name(dbus_bus_type, interface_name, G_BUS_NAME_OWNER_FLAGS_NONE, NULL, + on_dbus_name_acquired, on_dbus_name_lost_again, NULL, NULL); } int start_dbus_service() { @@ -911,18 +879,8 @@ int start_dbus_service() { dbus_bus_type = G_BUS_TYPE_SESSION; // debug(1, "Looking for a Shairport Sync native D-Bus interface \"org.gnome.ShairportSync\" on // the %s bus.",(config.dbus_service_bus_type == DBT_session) ? "session" : "system"); - - ourBusConnection = _g_bus_get_priv(dbus_bus_type, NULL, NULL); - if (ourBusConnection) { - debug(1, "Got a connection to the bus."); - g_dbus_connection_set_exit_on_close(ourBusConnection, FALSE); - ownerID = g_bus_own_name_on_connection(ourBusConnection, "org.gnome.ShairportSync", - G_BUS_NAME_OWNER_FLAGS_REPLACE, on_dbus_name_acquired, - on_dbus_name_lost, NULL, NULL); - } - - // ownerID = g_bus_own_name(dbus_bus_type, "org.gnome.ShairportSync", G_BUS_NAME_OWNER_FLAGS_NONE, - // NULL, on_dbus_name_acquired, on_dbus_name_lost, NULL, NULL); + ownerID = g_bus_own_name(dbus_bus_type, "org.gnome.ShairportSync", G_BUS_NAME_OWNER_FLAGS_NONE, + NULL, on_dbus_name_acquired, on_dbus_name_lost, NULL, NULL); return 0; // this is just to quieten a compiler warning } @@ -932,11 +890,9 @@ void stop_dbus_service() { g_bus_unown_name(ownerID); else debug(1, "Zero OwnerID for \"org.gnome.ShairportSync\"."); - if (ourBusConnection) - g_object_unref(ourBusConnection); - else - debug(1, "NULL BusConnection for \"org.gnome.ShairportSync\"."); - service_is_running = 0; + service_is_running = 0; } -int dbus_service_is_running() { return service_is_running; } +int dbus_service_is_running() { + return service_is_running; +} From 03eb21a9465b13ad92d66fd6bdde9b9a4dae8bb7 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Wed, 27 Feb 2019 18:27:53 +0000 Subject: [PATCH 07/17] Check rate when gwtting negotiated sndio parameters back. --- audio_sndio.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/audio_sndio.c b/audio_sndio.c index 3a544b9a..16367662 100644 --- a/audio_sndio.c +++ b/audio_sndio.c @@ -65,21 +65,21 @@ struct sio_par par; struct sndio_formats { const char *name; enum sps_format_t fmt; - + int rate; unsigned int bits; unsigned int bps; unsigned int sig; unsigned int le; }; -static struct sndio_formats formats[] = {{"S8", SPS_FORMAT_S8, 8, 1, 1, SIO_LE_NATIVE}, - {"U8", SPS_FORMAT_U8, 8, 1, 0, SIO_LE_NATIVE}, - {"S16", SPS_FORMAT_S16, 16, 2, 1, SIO_LE_NATIVE}, - {"AUTOMATIC", SPS_FORMAT_S16, 16, 2, 1, SIO_LE_NATIVE}, // TODO: make this really automatic? - {"S24", SPS_FORMAT_S24, 24, 4, 1, SIO_LE_NATIVE}, - {"S24_3LE", SPS_FORMAT_S24_3LE, 24, 3, 1, 1}, - {"S24_3BE", SPS_FORMAT_S24_3BE, 24, 3, 1, 0}, - {"S32", SPS_FORMAT_S32, 24, 4, 1, SIO_LE_NATIVE}}; +static struct sndio_formats formats[] = {{"S8", SPS_FORMAT_S8, 44100, 8, 1, 1, SIO_LE_NATIVE}, + {"U8", SPS_FORMAT_U8, 44100, 8, 1, 0, SIO_LE_NATIVE}, + {"S16", SPS_FORMAT_S16, 44100, 16, 2, 1, SIO_LE_NATIVE}, + {"AUTOMATIC", SPS_FORMAT_S16, 44100, 16, 2, 1, SIO_LE_NATIVE}, // TODO: make this really automatic? + {"S24", SPS_FORMAT_S24, 44100, 24, 4, 1, SIO_LE_NATIVE}, + {"S24_3LE", SPS_FORMAT_S24_3LE, 44100, 24, 3, 1, 1}, + {"S24_3BE", SPS_FORMAT_S24_3BE, 44100, 24, 3, 1, 0}, + {"S32", SPS_FORMAT_S32, 44100, 24, 4, 1, SIO_LE_NATIVE}}; static void help() { printf(" -d output-device set the output device [default*|...]\n"); } @@ -175,6 +175,7 @@ static int init(int argc, char **argv) { written = played = 0; time_of_last_onmove_cb = 0; at_least_one_onmove_cb_seen = 0; + for (i = 0; i < sizeof(formats) / sizeof(formats[0]); i++) { if (formats[i].fmt == config.output_format) { @@ -185,12 +186,12 @@ static int init(int argc, char **argv) { break; } } - + if (!sio_setpar(hdl, &par) || !sio_getpar(hdl, &par)) die("sndio: failed to set audio parameters"); for (i = 0, found = 0; i < sizeof(formats) / sizeof(formats[0]); i++) { if (formats[i].bits == par.bits && formats[i].bps == par.bps && formats[i].sig == par.sig && - formats[i].le == par.le) { + formats[i].le == par.le && formats[i].rate == par.rate) { config.output_format = formats[i].fmt; found = 1; break; @@ -198,6 +199,7 @@ static int init(int argc, char **argv) { } if (!found) die("sndio: failed to negotiate audio parameters"); + framesize = par.bps * par.pchan; config.output_rate = par.rate; From 1b47023c7857249d676761daa3affac086badfc1 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Wed, 27 Feb 2019 18:45:45 +0000 Subject: [PATCH 08/17] Make a dying error message a bit more meaningful. --- audio_sndio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/audio_sndio.c b/audio_sndio.c index 16367662..dd403894 100644 --- a/audio_sndio.c +++ b/audio_sndio.c @@ -198,7 +198,7 @@ static int init(int argc, char **argv) { } } if (!found) - die("sndio: failed to negotiate audio parameters"); + die("sndio: could not set output device to the required format and rate."); framesize = par.bps * par.pchan; From 1a0cd5096ed246566fbbe8342f22862ceb4ed793 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Wed, 27 Feb 2019 18:46:15 +0000 Subject: [PATCH 09/17] Quieten a warning when there is no cnfiguration file. --- shairport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shairport.c b/shairport.c index 02aa362b..8f87e1c9 100644 --- a/shairport.c +++ b/shairport.c @@ -867,7 +867,7 @@ int parse_options(int argc, char **argv) { } else { if (config_error_type(&config_file_stuff) == CONFIG_ERR_FILE_IO) - debug(1, "Error reading configuration file \"%s\": \"%s\".", + debug(2, "Error reading configuration file \"%s\": \"%s\".", config_error_file(&config_file_stuff), config_error_text(&config_file_stuff)); else { die("Line %d of the configuration file \"%s\":\n%s", config_error_line(&config_file_stuff), From dc393b98596a4d306c386f0610b280539b1126f0 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Wed, 27 Feb 2019 19:30:32 +0000 Subject: [PATCH 10/17] make the formats rate unsigned to avoid a warning. --- audio_sndio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/audio_sndio.c b/audio_sndio.c index dd403894..fcfcc4f6 100644 --- a/audio_sndio.c +++ b/audio_sndio.c @@ -65,7 +65,7 @@ struct sio_par par; struct sndio_formats { const char *name; enum sps_format_t fmt; - int rate; + unsigned int rate; unsigned int bits; unsigned int bps; unsigned int sig; From 042d940415ecd7d87388e5f8810e255282c0128e Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Thu, 28 Feb 2019 14:21:05 +0000 Subject: [PATCH 11/17] Remove the AUTO bit-depth setting. It's a hack and needs more consideration. --- audio_alsa.c | 122 +++++++++++++++++++-------------------------------- common.c | 2 - common.h | 1 - player.c | 9 ---- shairport.c | 2 +- 5 files changed, 45 insertions(+), 91 deletions(-) diff --git a/audio_alsa.c b/audio_alsa.c index 5896b431..2312d839 100644 --- a/audio_alsa.c +++ b/audio_alsa.c @@ -339,83 +339,51 @@ int actual_open_alsa_device(void) { return ret; } - - if (sample_format == SPS_FORMAT_AUTO) { - - struct alsa_formats { - snd_pcm_format_t alsa_sound_format; - enum sps_format_t sps_sound_format; - int frame_size; - char * description; - }; - - struct alsa_formats formats[] = {{SND_PCM_FORMAT_S32,SPS_FORMAT_S32,8,"S32"}, - {SND_PCM_FORMAT_S24,SPS_FORMAT_S24,8,"S24"}, - {SND_PCM_FORMAT_S24_3LE,SPS_FORMAT_S24_3LE,6,"S24_LE"}, - {SND_PCM_FORMAT_S24_3BE,SPS_FORMAT_S24_3BE,6,"S24_BE"}, - {SND_PCM_FORMAT_S16,SPS_FORMAT_S16,4,"S16"}}; - - int acceptable_format_found = 0; - unsigned int i = 0; - while((!acceptable_format_found) && (i < sizeof(formats) / sizeof(formats[0]))) { - if (snd_pcm_hw_params_set_format(alsa_handle, alsa_params, formats[i].alsa_sound_format) == 0) { - acceptable_format_found = 1; - frame_size = formats[i].frame_size; - config.output_format = formats[i].sps_sound_format; - debug(1,"alsa: output format automatically chosen to be \"%s\".",formats[i].description); - } else { - i++; - } - } - if (!acceptable_format_found) { - die("Can't find a suitable automatic output format setting."); - } - } else { - snd_pcm_format_t sf; - switch (sample_format) { - case SPS_FORMAT_S8: - sf = SND_PCM_FORMAT_S8; - frame_size = 2; - break; - case SPS_FORMAT_U8: - sf = SND_PCM_FORMAT_U8; - frame_size = 2; - break; - case SPS_FORMAT_S16: - sf = SND_PCM_FORMAT_S16; - frame_size = 4; - break; - case SPS_FORMAT_S24: - sf = SND_PCM_FORMAT_S24; - frame_size = 8; - break; - case SPS_FORMAT_S24_3LE: - sf = SND_PCM_FORMAT_S24_3LE; - frame_size = 6; - break; - case SPS_FORMAT_S24_3BE: - sf = SND_PCM_FORMAT_S24_3BE; - frame_size = 6; - break; - case SPS_FORMAT_S32: - sf = SND_PCM_FORMAT_S32; - frame_size = 8; - break; - default: - sf = SND_PCM_FORMAT_S16; // this is just to quieten a compiler warning - frame_size = 4; - debug(1, "Unsupported output format at audio_alsa.c"); - return -EINVAL; - } - - ret = snd_pcm_hw_params_set_format(alsa_handle, alsa_params, sf); - if (ret < 0) { - warn("audio_alsa: Sample format %d not available for device \"%s\": %s", sample_format, - alsa_out_dev, snd_strerror(ret)); - return ret; - } + snd_pcm_format_t sf; + switch (sample_format) { + case SPS_FORMAT_S8: + sf = SND_PCM_FORMAT_S8; + frame_size = 2; + break; + case SPS_FORMAT_U8: + sf = SND_PCM_FORMAT_U8; + frame_size = 2; + break; + case SPS_FORMAT_S16: + sf = SND_PCM_FORMAT_S16; + frame_size = 4; + break; + case SPS_FORMAT_S24: + sf = SND_PCM_FORMAT_S24; + frame_size = 8; + break; + case SPS_FORMAT_S24_3LE: + sf = SND_PCM_FORMAT_S24_3LE; + frame_size = 6; + break; + case SPS_FORMAT_S24_3BE: + sf = SND_PCM_FORMAT_S24_3BE; + frame_size = 6; + break; + case SPS_FORMAT_S32: + sf = SND_PCM_FORMAT_S32; + frame_size = 8; + break; + default: + sf = SND_PCM_FORMAT_S16; // this is just to quieten a compiler warning + frame_size = 4; + debug(1, "Unsupported output format at audio_alsa.c"); + return -EINVAL; } + ret = snd_pcm_hw_params_set_format(alsa_handle, alsa_params, sf); + if (ret < 0) { + warn("audio_alsa: Sample format %d not available for device \"%s\": %s", sample_format, + alsa_out_dev, snd_strerror(ret)); + return ret; + } + + if (set_period_size_request != 0) { debug(1, "Attempting to set the period size"); ret = snd_pcm_hw_params_set_period_size_near(alsa_handle, alsa_params, &period_size_requested, @@ -934,10 +902,8 @@ static int init(int argc, char **argv) { config.output_format = SPS_FORMAT_U8; else if (strcasecmp(str, "S8") == 0) config.output_format = SPS_FORMAT_S8; - else if (strcasecmp(str, "AUTOMATIC") == 0) - config.output_format = SPS_FORMAT_AUTO; else { - warn("Invalid output format \"%s\". It should be \"Automatic\", \"U8\", \"S8\", " + warn("Invalid output format \"%s\". It should be \"U8\", \"S8\", " "\"S16\", \"S24\", " "\"S24_3LE\", \"S24_3BE\" or " "\"S32\". It is set to \"S16\".", diff --git a/common.c b/common.c index ec70e2fe..4db41149 100644 --- a/common.c +++ b/common.c @@ -1391,8 +1391,6 @@ int64_t generate_zero_frames(char *outp, size_t number_of_frames, enum sps_forma case SPS_FORMAT_UNKNOWN: die("Unexpected SPS_FORMAT_UNKNOWN while calculating dither mask."); break; - case SPS_FORMAT_AUTO: - die("Unexpected SPS_FORMAT_AUTO while calculating dither mask."); } dither_mask -= 1; // int64_t r = r64i(); diff --git a/common.h b/common.h index 6f92abbe..9bc849e1 100644 --- a/common.h +++ b/common.h @@ -80,7 +80,6 @@ enum sps_format_t { SPS_FORMAT_S24_3LE, SPS_FORMAT_S24_3BE, SPS_FORMAT_S32, - SPS_FORMAT_AUTO, } sps_format_t; typedef struct { diff --git a/player.c b/player.c index 881a7359..485166ee 100644 --- a/player.c +++ b/player.c @@ -683,9 +683,6 @@ static inline void process_sample(int32_t sample, char **outp, enum sps_format_t case SPS_FORMAT_UNKNOWN: die("Unexpected SPS_FORMAT_UNKNOWN while calculating dither mask."); break; - case SPS_FORMAT_AUTO: - die("Unexpected SPS_FORMAT_AUTO while calculating dither mask."); - break; } dither_mask -= 1; // int64_t r = r64i(); @@ -763,9 +760,6 @@ static inline void process_sample(int32_t sample, char **outp, enum sps_format_t case SPS_FORMAT_UNKNOWN: die("Unexpected SPS_FORMAT_UNKNOWN while outputting samples"); break; - case SPS_FORMAT_AUTO: - die("Unexpected SPS_FORMAT_AUTO while outputting samples"); - break; } *outp += result; @@ -1653,9 +1647,6 @@ void *player_thread_func(void *arg) { case SPS_FORMAT_UNKNOWN: die("Unknown format choosing output bit depth"); break; - case SPS_FORMAT_AUTO: - die("Unexpected SPS_FORMAT_AUTO while outputting samples"); - break; } debug(3, "Output bit depth is %d.", output_bit_depth); diff --git a/shairport.c b/shairport.c index 8f87e1c9..3a0502ca 100644 --- a/shairport.c +++ b/shairport.c @@ -1577,7 +1577,7 @@ int main(int argc, char **argv) { debug(1, "use_mmap_if_available is %d.", config.no_mmap ? 0 : 1); debug(1, "output_rate is %d.", config.output_rate); debug(1, - "output_format is %d (0-unknown, 1-S8, 2-U8, 3-S16, 4-S24, 5-S24_3LE, 6-S24_3BE, 7-S32, 8-Automatic).", + "output_format is %d (0-unknown, 1-S8, 2-U8, 3-S16, 4-S24, 5-S24_3LE, 6-S24_3BE, 7-S32).", config.output_format); debug(1, "audio backend desired buffer length is %f seconds.", config.audio_backend_buffer_desired_length); From 41b584bd4314a119ac8cb9b8b8fe7eb9a8f0088e Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Thu, 28 Feb 2019 14:48:41 +0000 Subject: [PATCH 12/17] bring the active state timeout to 1 second and tidy up a few debug messages. --- audio_alsa.c | 4 ++-- scripts/shairport-sync.conf | 2 +- shairport.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/audio_alsa.c b/audio_alsa.c index 2312d839..a8449af6 100644 --- a/audio_alsa.c +++ b/audio_alsa.c @@ -1002,7 +1002,7 @@ static int init(int argc, char **argv) { } } - debug(1, "alsa: disable_standby_mode is %d.", config.disable_standby_mode); + debug(1, "alsa: disable_standby_mode is \"%s\".", config.disable_standby_mode == disable_standby_off ? "never" : config.disable_standby_mode == disable_standby_always ? "always" : "while_active"); } optind = 1; // optind=0 is equivalent to optind=1 plus special behaviour @@ -1552,7 +1552,7 @@ void *alsa_buffer_monitor_thread_code(__attribute__((unused)) void *arg) { int okb = -1; while (1) { if (okb != config.keep_dac_busy) { - debug(1,"keep_dac_busy is now %d",config.keep_dac_busy); + debug(1,"keep_dac_busy is now \"%s\"",config.keep_dac_busy == 0 ? "no" : "yes"); okb = config.keep_dac_busy; } if ((config.keep_dac_busy != 0) && (alsa_device_initialised == 0)) { diff --git a/scripts/shairport-sync.conf b/scripts/shairport-sync.conf index c5d83eb5..2108e2d1 100644 --- a/scripts/shairport-sync.conf +++ b/scripts/shairport-sync.conf @@ -59,7 +59,7 @@ sessioncontrol = // "active" state starts when play begins and ends when the active_state_timeout has elapsed after play ends, unless another play session starts before the timeout has fully elapsed. // run_this_before_entering_active_state = "/full/path/to/application and args"; // make sure the application has executable permission. If it's a script, include the shebang (#!/bin/...) on the first line // run_this_after_exiting_active_state = "/full/path/to/application and args"; // make sure the application has executable permission. If it's a script, include the shebang (#!/bin/...) on the first line -// active_state_timeout = 10; // wait for this number of seconds after play ends before leaving the active state, unless another play session begins. +// active_state_timeout = 1; // wait for this number of seconds after play ends before leaving the active state, unless another play session begins. // run_this_if_an_unfixable_error_is_detected = "/full/path/to/application and args"; // if a problem occurs that can't be cleared by Shairport Sync itself, hook a program on here to deal with it. An error code-string is passed as the last argument. // Many of these "unfixable" problems are caused by malfunctioning output devices, and sometimes it is necessary to restart the whole device to clear the problem. diff --git a/shairport.c b/shairport.c index 3a0502ca..47f82702 100644 --- a/shairport.c +++ b/shairport.c @@ -342,7 +342,7 @@ int parse_options(int argc, char **argv) { // nothing else comes in first. config.fixedLatencyOffset = 11025; // this sounds like it works properly. config.diagnostic_drop_packet_fraction = 0.0; - config.active_state_timeout = 10.0; + config.active_state_timeout = 1.0; config.volume_range_hw_priority = 0; // if combining software and hardware volume control, give the software priority // i.e. when reducing volume, reduce the sw first before reducing the software. From a98a7906de9b7019a72f977a35efba8d69cfffb1 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Thu, 28 Feb 2019 15:05:32 +0000 Subject: [PATCH 13/17] set active state timeout to 3 seconds. Enough for a YouTube movie switch. --- scripts/shairport-sync.conf | 2 +- shairport.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/shairport-sync.conf b/scripts/shairport-sync.conf index 2108e2d1..213fd2b0 100644 --- a/scripts/shairport-sync.conf +++ b/scripts/shairport-sync.conf @@ -59,7 +59,7 @@ sessioncontrol = // "active" state starts when play begins and ends when the active_state_timeout has elapsed after play ends, unless another play session starts before the timeout has fully elapsed. // run_this_before_entering_active_state = "/full/path/to/application and args"; // make sure the application has executable permission. If it's a script, include the shebang (#!/bin/...) on the first line // run_this_after_exiting_active_state = "/full/path/to/application and args"; // make sure the application has executable permission. If it's a script, include the shebang (#!/bin/...) on the first line -// active_state_timeout = 1; // wait for this number of seconds after play ends before leaving the active state, unless another play session begins. +// active_state_timeout = 3; // wait for this number of seconds after play ends before leaving the active state, unless another play session begins. // run_this_if_an_unfixable_error_is_detected = "/full/path/to/application and args"; // if a problem occurs that can't be cleared by Shairport Sync itself, hook a program on here to deal with it. An error code-string is passed as the last argument. // Many of these "unfixable" problems are caused by malfunctioning output devices, and sometimes it is necessary to restart the whole device to clear the problem. diff --git a/shairport.c b/shairport.c index 47f82702..b65db5ef 100644 --- a/shairport.c +++ b/shairport.c @@ -342,7 +342,7 @@ int parse_options(int argc, char **argv) { // nothing else comes in first. config.fixedLatencyOffset = 11025; // this sounds like it works properly. config.diagnostic_drop_packet_fraction = 0.0; - config.active_state_timeout = 1.0; + config.active_state_timeout = 3.0; config.volume_range_hw_priority = 0; // if combining software and hardware volume control, give the software priority // i.e. when reducing volume, reduce the sw first before reducing the software. From d2e1910fbaad060d27d6cd4b192ef158af68a3db Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Thu, 28 Feb 2019 17:33:51 +0000 Subject: [PATCH 14/17] set active state timeout to 10 seconds. Enough for a YouTube movie switch -- 3 or 5 seconds doesn't seem to be enough. --- scripts/shairport-sync.conf | 2 +- shairport.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/shairport-sync.conf b/scripts/shairport-sync.conf index 213fd2b0..e2ffeba4 100644 --- a/scripts/shairport-sync.conf +++ b/scripts/shairport-sync.conf @@ -59,7 +59,7 @@ sessioncontrol = // "active" state starts when play begins and ends when the active_state_timeout has elapsed after play ends, unless another play session starts before the timeout has fully elapsed. // run_this_before_entering_active_state = "/full/path/to/application and args"; // make sure the application has executable permission. If it's a script, include the shebang (#!/bin/...) on the first line // run_this_after_exiting_active_state = "/full/path/to/application and args"; // make sure the application has executable permission. If it's a script, include the shebang (#!/bin/...) on the first line -// active_state_timeout = 3; // wait for this number of seconds after play ends before leaving the active state, unless another play session begins. +// active_state_timeout = 10.0; // wait for this number of seconds after play ends before leaving the active state, unless another play session begins. // run_this_if_an_unfixable_error_is_detected = "/full/path/to/application and args"; // if a problem occurs that can't be cleared by Shairport Sync itself, hook a program on here to deal with it. An error code-string is passed as the last argument. // Many of these "unfixable" problems are caused by malfunctioning output devices, and sometimes it is necessary to restart the whole device to clear the problem. diff --git a/shairport.c b/shairport.c index b65db5ef..3a0502ca 100644 --- a/shairport.c +++ b/shairport.c @@ -342,7 +342,7 @@ int parse_options(int argc, char **argv) { // nothing else comes in first. config.fixedLatencyOffset = 11025; // this sounds like it works properly. config.diagnostic_drop_packet_fraction = 0.0; - config.active_state_timeout = 3.0; + config.active_state_timeout = 10.0; config.volume_range_hw_priority = 0; // if combining software and hardware volume control, give the software priority // i.e. when reducing volume, reduce the sw first before reducing the software. From a3736837d359277b12fb511d25a45143c0d1a8b7 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Thu, 28 Feb 2019 21:51:11 +0000 Subject: [PATCH 15/17] set default for disable standby to 'never' --- audio_alsa.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/audio_alsa.c b/audio_alsa.c index a8449af6..4010629d 100644 --- a/audio_alsa.c +++ b/audio_alsa.c @@ -985,7 +985,7 @@ static int init(int argc, char **argv) { /* Get the optional disable_standby_mode setting. */ - config.disable_standby_mode = disable_standby_while_active; + config.disable_standby_mode = disable_standby_off; config.keep_dac_busy = 0; if (config_lookup_string(config.cfg, "alsa.disable_standby_mode", &str)) { if ((strcasecmp(str, "no") == 0) || (strcasecmp(str, "off") == 0) || (strcasecmp(str, "never") == 0)) @@ -998,7 +998,7 @@ static int init(int argc, char **argv) { else { warn("Invalid disable_standby_mode option choice \"%s\". It should be " "\"always\", \"while_active\" or \"never\". " - "It is set to \"while_active\"."); + "It is set to \"never\"."); } } From 182c0a31a4454636f97a152cf68a707368fa0670 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Thu, 28 Feb 2019 21:53:07 +0000 Subject: [PATCH 16/17] set default for disable standby to 'never' --- scripts/shairport-sync.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/shairport-sync.conf b/scripts/shairport-sync.conf index e2ffeba4..9d4edcda 100644 --- a/scripts/shairport-sync.conf +++ b/scripts/shairport-sync.conf @@ -86,7 +86,7 @@ alsa = // use_mmap_if_available = "yes"; // Use this optional advanced setting to control whether MMAP-based output is used to communicate with the DAC. Default is "yes" // use_hardware_mute_if_available = "no"; // Use this optional advanced setting to control whether the hardware in the DAC is used for muting. Default is "no", for compatibility with other audio players. // maximum_stall_time = 0.200; // Use this optional advanced setting to control how long to wait for data to be consumed by the output device before considering it an error. It should never approach 200 ms. -// disable_standby_mode = "while_active"; // Some DACs make small "popping" noises when they go in and out of standby mode. Settings can be: "always", "while_active" or "never". Default is "while_active". You can use "yes" instead of "always" and "no instead of "never". +// disable_standby_mode = "never"; // Some DACs make small "popping" noises when they go in and out of standby mode. Settings can be: "always", "while_active" or "never". Default is "never", but only for backwards compatibility. You can use "yes" instead of "always" and "no instead of "never". }; // Parameters for the "sndio" audio back end. All are optional. From 0ef617924f1d5cd67abda05caacddfc08c38d637 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Thu, 28 Feb 2019 22:13:21 +0000 Subject: [PATCH 17/17] fix active mode lockup problem, set defaults for active mode and disable standby. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index fc133aa7..7126a1b0 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ # Process this file with autoconf to produce a configure script. AC_PREREQ([2.50]) -AC_INIT([shairport-sync], [3.3d55], [mikebrady@eircom.net]) +AC_INIT([shairport-sync], [3.3d56], [mikebrady@eircom.net]) AM_INIT_AUTOMAKE AC_CONFIG_SRCDIR([shairport.c]) AC_CONFIG_HEADERS([config.h])