Merge branch 'development'

Limit the maximum number of open file handles in Docker images.
Increase the level of optimisation (to -O3) while building the Shairport Sync application itself.
Enabled termination of a disconnected session in AirPlay 2 operation after a timeout, which was disabled by default.
Fixed a bug that prevented Shairport Sync from terminating cleanly when a fatal error that occurred while directly accessing the alsa output device.
Fixed a bug that prevented Shairport Sync from recovering cleanly if a player disconnected without warning. The problem was that the player thread would not respond to cancel request. Fixed by moving a pthreadtestcancel() call to the innermost loop.
Fixed compilation errors on old versions of Mac OS X by, reordering some of the files to be included in shairport.c.
Avoided using TCP_KEEPINTVL and TCP_KEEPCNT if they are not defined (they are not defined in older versions of Mac OS X).
Fixed a race condition with the metadata queues. The problem was that the queues were being initialised by threads launched by the main thread which, having started the threads, proceeded to use the queues. But if the threads were late in starting, the queues might not be initialised by the time the main thread tried to use them. Fixed by initialising the queues in the main thread.
Enable the ALSA backend to access mixers on a devices with a hdmi: prefix.
Update the help text for the ALSA backend to denote HDMI devices using the hdmi: prefix rather than hw:.
Manage thread cancellation state explicitly in audio_ao.c
CAR INSTALL guide: Simplify some of the wording. Change order and allow a few seconds before starting systemd-timesync service.

CAR INSTALL.md

@@ -191,15 +191,16 @@ if test $MODE = RUN ; then
 else
 
   # If script execution gets in here, it starts services needed for normal operation.
-  /bin/systemctl start systemd-timesyncd || :
   /bin/systemctl start dhcpcd || /bin/systemctl start NetworkManager || :
+  /bin/sleep 2 # may be necessary while the network becomes available
+  /bin/systemctl start systemd-timesyncd || :
 
 fi
 
 exit 0 # normal exit here
 ```
 
-#### Disable Unused Services
+#### Disable Unused Services - Optional
 These optional steps have been tested on a Raspberry Pi only -- they have not been tested on other systems.
 Some services are not necessary for this setup and can be disabled as follows:
 ```
@@ -207,11 +208,9 @@ Some services are not necessary for this setup and can be disabled as follows:
 # systemctl disable triggerhappy
 # systemctl disable dphys-swapfile
 ```
-#### Optional: Read-only mode – Raspberry Pi Specific
-This optional step is applicable to a Raspberry Pi only. Run `sudo raspi-config` and then choose `Performance Options` > `Overlay Filesystem` and choose to enable the overlay filesystem, and to set the boot partition to be write-protected. (The idea here is that this offers more protection against files being corrupted by the sudden removal of power.)
 
-### Final Steps
 
+#### Disable Unused Services - Mandatory
 You now need to disable some services; that is, you need to stop them starting automatically on power-up. This is because they either interfere with the system's operation in WiFi Access Point mode, or because they won't work when the system isn't connected to the Internet. Only one of the `NetworkManager` and the `dhcpcd` service will be present in your system, but it's no harm to try to disable both.
 ```
 # systemctl disable dhcpcd
@@ -219,12 +218,18 @@ You now need to disable some services; that is, you need to stop them starting a
 # systemctl disable wpa_supplicant
 # systemctl disable systemd-timesyncd
 ```
-Lastly, note that the WiFi credentials you used initially to connect to your network (e.g. your home network) will have been stored in the system in plain text. This is convenient for when you want to reconnect to update (see later), but if you prefer to delete them, they will be in `/etc/wpa_supplicant/wpa_supplicant.conf`
+Lastly, note that the WiFi credentials you used initially to connect to your network (e.g. your home network) will have been stored in the system in plain text. This is convenient for when you want to reconnect to update (see later), but if you prefer to delete them, they will be in `/etc/wpa_supplicant/wpa_supplicant.conf`.
 
+#### Optional: Read-only mode – Raspberry Pi Specific
+This optional step is applicable to a Raspberry Pi only. Run `sudo raspi-config` and then choose `Performance Options` > `Overlay Filesystem` and choose to enable the overlay filesystem, and to set the boot partition to be write-protected. (The idea here is that this offers more protection against files being corrupted by the sudden removal of power.)
+
+### Final Step
 When you are finished, carefully power down the machine before unplugging it from power:
 ```
 # poweroff
 ```
+Note: doing a `reboot` here doesn't seem to work properly -- it really does seem necessary to power off.
+
 ### Ready
 Install the Raspberry Pi in your car. It should be powered from a source that is switched off when you leave the car, otherwise the slight current drain will eventually flatten the car's battery.
 
@@ -241,7 +246,7 @@ However, if you're *upgrading* the operating system to e.g. from Bullseye to Boo
 If it's a Raspberry Pi and you have optionally enabled the read-only mode, you must take the device out of Read-only mode:  
 Run `sudo raspi-config` and then choose `Performance Options` > `Overlay Filesystem` and choose to disable the overlay filesystem and to set the boot partition not to be write-protected. This is so that changes can be written to the file system; you can make the filesystem read-only again later. Save the changes and reboot the system.
 #### Undo Optimisations
-If you have disabled any of the services listed in the [Disable Unused Services](#disable-unused-services) section, you should re-enable them. (But *do not* re-eneable `NetworkManager`, `dhcpcd`, `wpa_supplicant` or `systemd-timesyncd` -- they are handled specially by the startup script.)
+If you have disabled any of the services listed in the [Disable Unused Services - Optional](#disable-unused-services---optional) section, you should re-enable them. (But *do not* re-eneable `NetworkManager`, `dhcpcd`, `wpa_supplicant` or `systemd-timesyncd` -- they are handled specially by the startup script.)
 #### Perform Legacy Updates
 Over time, the arrangements by which the system is prepared for operation has changed to make it easier to revert to normal operation when necessary for maintenance, updates, etc. A small number of the old settings need to be changed to bring them up to date with the present arrangements. Once the required changes have been made, your system will be ready for the update process detailed below. Here are those legacy changes you need to make, just once:
 

audio_alsa.c

@@ -1,7 +1,7 @@
 /*
  * libalsa output driver. This file is part of Shairport.
  * Copyright (c) Muffinman, Skaman 2013
- * Copyright (c) Mike Brady 2014 -- 2022
+ * Copyright (c) Mike Brady 2014 -- 2024
  * All rights reserved.
  *
  * Permission is hereby granted, free of charge, to any person
@@ -146,6 +146,7 @@ long alsa_mix_minv, alsa_mix_maxv;
 long alsa_mix_mindb, alsa_mix_maxdb;
 
 char *alsa_out_dev = "default";
+char *hw_alsa_out_dev = NULL;
 char *alsa_mix_dev = NULL;
 char *alsa_mix_ctrl = NULL;
 int alsa_mix_index = 0;
@@ -279,19 +280,78 @@ uint64_t frames_sent_for_playing;
 // frames_sent_for_playing (which Shairport Sync might hold) would be invalid.
 int frames_sent_break_occurred;
 
+// if a device name ends in ",DEV=0", drop it. Then if it also begins with "CARD=", drop that too.
+static void simplify_and_printf_mutable_device_name(char *device_name) {
+  if (strstr(device_name, ",DEV=0") == device_name + strlen(device_name) - strlen(",DEV=0")) {
+    char *shortened_device_name = str_replace(device_name, ",DEV=0", "");
+    char *simplified_device_name = str_replace(shortened_device_name, "CARD=", "");
+    printf("      \"%s\"\n", simplified_device_name);
+    free(simplified_device_name);
+    free(shortened_device_name);
+  } else {
+    printf("      \"%s\"\n", device_name);
+  }
+}
+
 static void help(void) {
+
   printf("    -d output-device    set the output device, default is \"default\".\n"
          "    -c mixer-control    set the mixer control name, default is to use no mixer.\n"
          "    -m mixer-device     set the mixer device, default is the output device.\n"
          "    -i mixer-index      set the mixer index, default is 0.\n");
-  int r = system("if [ -d /proc/asound ] ; then echo \"    hardware output devices:\" ; ls -al "
-                 "/proc/asound/ 2>/dev/null | grep '\\->' | tr -s ' ' | cut -d ' ' -f 9 | while "
-                 "read line; do echo \"      \\\"hw:$line\\\"\" ; done ; fi");
-  if (r != 0)
-    debug(2, "error %d executing a script to list alsa hardware device names", r);
+  // look for devices with a name prefix of hw: or hdmi:
+  int card_number = -1;
+  snd_card_next(&card_number);
+
+  if (card_number < 0) {
+    printf("      no hardware output devices found.\n");
+  }
+
+  int at_least_one_device_found = 0;
+  while (card_number >= 0) {
+    void **hints;
+    char *hdmi_str = NULL;
+    char *hw_str = NULL;
+    if (snd_device_name_hint(card_number, "pcm", &hints) == 0) {
+      void **device_on_card_hints = hints;
+      while (*device_on_card_hints != NULL) {
+        char *device_on_card_name = snd_device_name_get_hint(*device_on_card_hints, "NAME");
+        if ((strstr(device_on_card_name, "hw:") == device_on_card_name) && (hw_str == NULL))
+          hw_str = strdup(device_on_card_name);
+        if ((strstr(device_on_card_name, "hdmi:") == device_on_card_name) && (hdmi_str == NULL))
+          hdmi_str = strdup(device_on_card_name);
+        free(device_on_card_name);
+        device_on_card_hints++;
+      }
+      snd_device_name_free_hint(hints);
+      if ((hdmi_str != NULL) || (hw_str != NULL)) {
+        if (at_least_one_device_found == 0) {
+          printf("    hardware output devices:\n");
+          at_least_one_device_found = 1;
+        }
+      }
+      if (hdmi_str != NULL) {
+        simplify_and_printf_mutable_device_name(hdmi_str);
+      } else if (hw_str != NULL) {
+        simplify_and_printf_mutable_device_name(hw_str);
+      }
+      if (hdmi_str != NULL)
+        free(hdmi_str);
+      if (hw_str != NULL)
+        free(hw_str);
+    }
+    snd_card_next(&card_number);
+  }
+  if (at_least_one_device_found == 0)
+    printf("    no hardware output devices found.\n");
 }
 
-void set_alsa_out_dev(char *dev) { alsa_out_dev = dev; } // ugh -- not static!
+void set_alsa_out_dev(char *dev) {
+  alsa_out_dev = dev;
+  if (hw_alsa_out_dev != NULL)
+    free(hw_alsa_out_dev);
+  hw_alsa_out_dev = str_replace(alsa_out_dev, "hdmi:", "hw:");
+} // ugh -- not static!
 
 // assuming pthread cancellation is disabled
 // returns zero of all is okay, a Unx error code if there's a problem
@@ -902,7 +962,7 @@ static int prepare_mixer() {
     pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); // make this un-cancellable
 
     if (alsa_mix_dev == NULL)
-      alsa_mix_dev = alsa_out_dev;
+      alsa_mix_dev = hw_alsa_out_dev;
 
     // Now, start trying to initialise the alsa device with the settings
     // obtained
@@ -1355,11 +1415,19 @@ static int init(int argc, char **argv) {
   }
 
   debug(1, "alsa: output device name is \"%s\".", alsa_out_dev);
-
+  
+  
+  // now, we need a version of the alsa_out_dev that substitutes "hw:" for "hdmi" if it's
+  // there. It seems hw:1 would be a valid devcie name where hdmi:1 would not
+  
+  if (alsa_out_dev != NULL)
+    hw_alsa_out_dev = str_replace(alsa_out_dev, "hdmi:", "hw:");
+  
   // so, now, if the option to keep the DAC running has been selected, start a
   // thread to monitor the
   // length of the queue
   // if the queue gets too short, stuff it with silence
+  
 
   pthread_create(&alsa_buffer_monitor_thread, NULL, &alsa_buffer_monitor_thread_code, NULL);
 
@@ -1376,6 +1444,8 @@ static void deinit(void) {
   debug(3, "Join buffer monitor thread.");
   pthread_join(alsa_buffer_monitor_thread, NULL);
   pthread_setcancelstate(oldState, NULL);
+  if (hw_alsa_out_dev != NULL)
+    free(hw_alsa_out_dev);
 }
 
 static int set_mute_state() {

audio_ao.c

@@ -69,6 +69,8 @@ static void help(void) {
 }
 
 static int init(int argc, char **argv) {
+  int oldState;
+  pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); // make this un-cancellable
   ao_initialize();
   driver = ao_default_driver_id();
   if (driver == -1) {
@@ -133,14 +135,18 @@ static int init(int argc, char **argv) {
     fmt.byte_format = AO_FMT_NATIVE;
     fmt.matrix = strdup("L,R");
   }
+  pthread_setcancelstate(oldState, NULL);
   return 0;
 }
 
 static void deinit(void) {
+  int oldState;
+  pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); // make this un-cancellable
   if (dev != NULL)
     ao_close(dev);
   dev = NULL;
   ao_shutdown();
+  pthread_setcancelstate(oldState, NULL);
 }
 
 static void start(__attribute__((unused)) int sample_rate,
@@ -166,10 +172,13 @@ static int play(void *buf, int samples, __attribute__((unused)) int sample_type,
 
 static void stop(void) {
   // debug(1,"libao stop");
+  int oldState;
+  pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); // make this un-cancellable
   if (dev != NULL) {
     ao_close(dev);
     dev = NULL;
   }
+  pthread_setcancelstate(oldState, NULL);
 }
 
 audio_output audio_ao = {.name = "ao",

configure.ac

@@ -2,7 +2,7 @@
 # Process this file with autoconf to produce a configure script.
 
 AC_PREREQ([2.50])
-AC_INIT([shairport-sync], [4.3.3], [4265913+mikebrady@users.noreply.github.com])
+AC_INIT([shairport-sync], [4.3.4], [4265913+mikebrady@users.noreply.github.com])
 AM_INIT_AUTOMAKE([subdir-objects])
 AC_CONFIG_SRCDIR([shairport.c])
 AC_CONFIG_HEADERS([config.h])

docker/Dockerfile

@@ -55,7 +55,7 @@ COPY . .
 RUN git checkout "$SHAIRPORT_SYNC_BRANCH"
 WORKDIR /shairport-sync/build
 RUN autoreconf -i ../
-RUN ../configure --sysconfdir=/etc --with-alsa --with-pa --with-soxr --with-avahi --with-ssl=openssl \
+RUN CFLAGS="-O3" CXXFLAGS="-O3" ../configure --sysconfdir=/etc --with-alsa --with-pa --with-soxr --with-avahi --with-ssl=openssl \
         --with-airplay-2 --with-metadata --with-dummy --with-pipe --with-dbus-interface \
         --with-stdout --with-mpris-interface --with-mqtt-client \
         --with-apple-alac --with-convolution

docker/classic/Dockerfile

@@ -40,7 +40,7 @@ COPY . .
 RUN git checkout "$SHAIRPORT_SYNC_BRANCH"
 WORKDIR /shairport-sync/build
 RUN autoreconf -i ../
-RUN ../configure --sysconfdir=/etc --with-alsa --with-pa --with-soxr --with-avahi --with-ssl=mbedtls \
+RUN CFLAGS="-O3" CXXFLAGS="-O3" ../configure --sysconfdir=/etc --with-alsa --with-pa --with-soxr --with-avahi --with-ssl=mbedtls \
         --with-metadata --with-dummy --with-pipe --with-dbus-interface \
         --with-stdout --with-mpris-interface --with-mqtt-client \
         --with-apple-alac --with-convolution

docker/classic/etc/s6-overlay/s6-rc.d/02-dbus/run

@@ -1,3 +1,7 @@
 #!/command/with-contenv sh
+
+# Set the limit to the same value Docker has been using in earlier version.
+ulimit -n 1048576
+
 echo "Starting dbus"
 exec s6-notifyoncheck  dbus-daemon --system --nofork --nopidfile

docker/etc/s6-overlay/s6-rc.d/02-dbus/run

@@ -1,3 +1,7 @@
 #!/command/with-contenv sh
+
+# Set the limit to the same value Docker has been using in earlier version.
+ulimit -n 1048576
+
 echo "Starting dbus"
 exec s6-notifyoncheck  dbus-daemon --system --nofork --nopidfile

player.c

@@ -921,7 +921,7 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) {
   pthread_cleanup_push(buffer_get_frame_cleanup_handler,
                        (void *)conn); // undo what's been done so far
   do {
-
+    pthread_testcancel(); // even if no packets are coming in...
     // get the time
     local_time_now = get_absolute_time_in_ns(); // type okay
     // debug(3, "buffer_get_frame is iterating");
@@ -2269,9 +2269,7 @@ void *player_thread_func(void *arg) {
     if (conn->input_bytes_per_frame == 0)
       debug(1, "conn->input_bytes_per_frame is zero!");
 
-    pthread_testcancel();                     // allow a pthread_cancel request to take effect.
-    abuf_t *inframe = buffer_get_frame(conn); // this has cancellation point(s), but it's not
-                                              // guaranteed that they'll always be executed
+    abuf_t *inframe = buffer_get_frame(conn); // this has a (needed!) deliberate cancellation point in it.
     uint64_t local_time_now = get_absolute_time_in_ns(); // types okay
     config.last_access_to_volume_info_time =
         local_time_now; // ensure volume info remains seen as valid
@@ -3233,14 +3231,6 @@ void *player_thread_func(void *arg) {
             frames_seen_in_this_logging_interval = 0;
           }
 
-          // update the watchdog
-          if ((config.dont_check_timeout == 0) && (config.timeout != 0)) {
-            uint64_t time_now = get_absolute_time_in_ns();
-            debug_mutex_lock(&conn->watchdog_mutex, 1000, 0);
-            conn->watchdog_bark_time = time_now;
-            debug_mutex_unlock(&conn->watchdog_mutex, 0);
-          }
-
           // debug(1,"Sync error %lld frames. Amount to stuff %d." ,sync_error,amount_to_stuff);
 
           // new stats calculation. We want a running average of sync error, drift, adjustment,

player.h

@@ -167,14 +167,11 @@ typedef struct {
   SOCKADDR remote, local;
   volatile int stop;
   volatile int running;
-  volatile uint64_t watchdog_bark_time;
-  volatile int watchdog_barks; // number of times the watchdog has timed out and done something
 
   uint64_t playstart;
   uint64_t connection_start_time; // the time the device is selected, which could be a long time
                                   // before a play
-  pthread_t thread, timer_requester, rtp_audio_thread, rtp_control_thread, rtp_timing_thread,
-      player_watchdog_thread;
+  pthread_t thread, timer_requester, rtp_audio_thread, rtp_control_thread, rtp_timing_thread;
 
   // buffers to delete on exit
   int32_t *tbuf;
@@ -380,7 +377,6 @@ typedef struct {
                            // request in flight at the same time
 
   pthread_mutex_t reference_time_mutex;
-  pthread_mutex_t watchdog_mutex;
 
   double local_to_remote_time_gradient; // if no drift, this would be exactly 1.0; likely it's
                                         // slightly above or  below.

rtsp.c

@@ -3,7 +3,7 @@
  * Copyright (c) James Laird 2013
 
  * Modifications associated with audio synchronization, multithreading and
- * metadata handling copyright (c) Mike Brady 2014-2023
+ * metadata handling copyright (c) Mike Brady 2014-2024
  * All rights reserved.
  *
  * Permission is hereby granted, free of charge, to any person
@@ -430,7 +430,8 @@ int pc_queue_add_item(pc_queue *the_queue, const void *the_stuff, int block) {
     } else
       rc = pthread_mutex_lock(&the_queue->pc_queue_lock);
     if (rc)
-      debug(1, "Error locking for pc_queue_add_item");
+      debug(1, "Error %d (\"%s\") locking for pc_queue_add_item. Block is %d.", rc, strerror(rc),
+            block);
     pthread_cleanup_push(pc_queue_cleanup_handler, (void *)the_queue);
     // leave this out if you want this to return if the queue is already full
     // irrespective of the block flag.
@@ -611,57 +612,6 @@ int get_play_lock(rtsp_conn_info *conn, int allow_session_interruption) {
   return response;
 }
 
-void player_watchdog_thread_cleanup_handler(void *arg) {
-  rtsp_conn_info *conn = (rtsp_conn_info *)arg;
-  debug(3, "Connection %d: Watchdog Exit.", conn->connection_number);
-}
-
-void *player_watchdog_thread_code(void *arg) {
-  pthread_cleanup_push(player_watchdog_thread_cleanup_handler, arg);
-  rtsp_conn_info *conn = (rtsp_conn_info *)arg;
-  do {
-    usleep(2000000); // check every two seconds
-    // debug(3, "Connection %d: Check the thread is doing something...", conn->connection_number);
-#ifdef CONFIG_AIRPLAY_2
-    if ((config.dont_check_timeout == 0) && (config.timeout != 0) && (conn->airplay_type == ap_1)) {
-#else
-    if ((config.dont_check_timeout == 0) && (config.timeout != 0)) {
-#endif
-      debug_mutex_lock(&conn->watchdog_mutex, 1000, 0);
-      uint64_t last_watchdog_bark_time = conn->watchdog_bark_time;
-      debug_mutex_unlock(&conn->watchdog_mutex, 0);
-      if (last_watchdog_bark_time != 0) {
-        uint64_t time_since_last_bark =
-            (get_absolute_time_in_ns() - last_watchdog_bark_time) / 1000000000;
-        uint64_t ct = config.timeout; // go from int to 64-bit int
-
-        if (time_since_last_bark >= ct) {
-          conn->watchdog_barks++;
-          if (conn->watchdog_barks == 1) {
-            // debuglev = 3; // tell us everything.
-            debug(1,
-                  "Connection %d: As Yeats almost said, \"Too long a silence / can make a stone "
-                  "of the heart\".",
-                  conn->connection_number);
-            conn->stop = 1;
-            pthread_cancel(conn->thread);
-          } else if (conn->watchdog_barks == 3) {
-            if ((config.cmd_unfixable) && (config.unfixable_error_reported == 0)) {
-              config.unfixable_error_reported = 1;
-              command_execute(config.cmd_unfixable, "unable_to_cancel_play_session", 1);
-            } else {
-              die("an unrecoverable error, \"unable_to_cancel_play_session\", has been detected.",
-                  conn->connection_number);
-            }
-          }
-        }
-      }
-    }
-  } while (1);
-  pthread_cleanup_pop(0); // should never happen
-  pthread_exit(NULL);
-}
-
 static void track_thread(rtsp_conn_info *conn) {
   debug_mutex_lock(&conns_lock, 1000000, 3);
   // look for an empty slot first
@@ -1348,8 +1298,11 @@ enum rtsp_read_request_response rtsp_read_request(rtsp_conn_info *conn, rtsp_mes
         continue;
       }
       if (errno == ETIMEDOUT) {
-        debug(1, "Connection %d: ETIMEDOUT -- keepalive timeout.", conn->connection_number);
-        reply = rtsp_read_request_response_channel_closed;
+        debug(1,
+              "Connection %d: As Yeats almost said, \"Too long a silence / can make a stone "
+              "of the heart\".",
+              conn->connection_number);
+        reply = rtsp_read_request_response_immediate_shutdown_requested;
         // Note: the socket will be closed when the thread exits
         goto shutdown;
       }
@@ -1687,7 +1640,7 @@ void handle_get_info(__attribute((unused)) rtsp_conn_info *conn, rtsp_message *r
               hdr);
       }
     }
-    
+
     // In Stage 1, look for the DACP and Active-Remote
     char *ar = msg_get_header(req, "Active-Remote");
     if (ar) {
@@ -2101,8 +2054,8 @@ void handle_post(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) {
   if (strcmp(req->path, "/feedback") == 0) {
     resp->respcode = 501;
   } else {
-    debug(1, "Connection %d: Airplay 1. Unhandled POST %s Content-Length %d", conn->connection_number,
-          req->path, req->contentlength);
+    debug(1, "Connection %d: Airplay 1. Unhandled POST %s Content-Length %d",
+          conn->connection_number, req->path, req->contentlength);
     debug_log_rtsp_message(2, "POST request", req);
   }
 }
@@ -2115,7 +2068,7 @@ struct pairings {
   uint8_t public_key[32];
 
   struct pairings *next;
-} * pairings;
+} *pairings;
 
 static struct pairings *pairing_find(const char *device_id) {
   for (struct pairings *pairing = pairings; pairing; pairing = pairing->next) {
@@ -3085,7 +3038,7 @@ void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp)
                 // iap->ifa_name);
                 if ((iap->ifa_addr) && (iap->ifa_netmask) && (iap->ifa_flags & IFF_UP) &&
                     ((iap->ifa_flags & IFF_LOOPBACK) == 0) &&
-		    (config.interface == NULL || (strcmp(config.interface, iap->ifa_name) == 0))) {
+                    (config.interface == NULL || (strcmp(config.interface, iap->ifa_name) == 0))) {
                   char buf[INET6_ADDRSTRLEN + 1]; // +1 for a NUL
                   memset(buf, 0, sizeof(buf));
                   if (iap->ifa_addr->sa_family == AF_INET6) {
@@ -4081,13 +4034,9 @@ void metadata_pack_cleanup_function(void *arg) {
 void metadata_thread_cleanup_function(__attribute__((unused)) void *arg) {
   // debug(2, "metadata_thread_cleanup_function called");
   metadata_close();
-  pc_queue_delete(&metadata_queue);
 }
 
 void *metadata_thread_function(__attribute__((unused)) void *ignore) {
-  // create a pc_queue for passing information to a threaded metadata handler
-  pc_queue_init(&metadata_queue, (char *)&metadata_queue_items, sizeof(metadata_package),
-                metadata_queue_size, "pipe");
   metadata_create_multicast_socket();
   metadata_package pack;
   pthread_cleanup_push(metadata_thread_cleanup_function, NULL);
@@ -4113,13 +4062,9 @@ void *metadata_thread_function(__attribute__((unused)) void *ignore) {
 void metadata_multicast_thread_cleanup_function(__attribute__((unused)) void *arg) {
   // debug(2, "metadata_multicast_thread_cleanup_function called");
   metadata_delete_multicast_socket();
-  pc_queue_delete(&metadata_multicast_queue);
 }
 
 void *metadata_multicast_thread_function(__attribute__((unused)) void *ignore) {
-  // create a pc_queue for passing information to a threaded metadata handler
-  pc_queue_init(&metadata_multicast_queue, (char *)&metadata_multicast_queue_items,
-                sizeof(metadata_package), metadata_multicast_queue_size, "multicast");
   metadata_create_multicast_socket();
   metadata_package pack;
   pthread_cleanup_push(metadata_multicast_thread_cleanup_function, NULL);
@@ -4154,13 +4099,9 @@ void metadata_hub_close(void) {}
 void metadata_hub_thread_cleanup_function(__attribute__((unused)) void *arg) {
   // debug(2, "metadata_hub_thread_cleanup_function called");
   metadata_hub_close();
-  pc_queue_delete(&metadata_hub_queue);
 }
 
 void *metadata_hub_thread_function(__attribute__((unused)) void *ignore) {
-  // create a pc_queue for passing information to a threaded metadata handler
-  pc_queue_init(&metadata_hub_queue, (char *)&metadata_hub_queue_items, sizeof(metadata_package),
-                metadata_hub_queue_size, "hub");
   metadata_package pack;
   pthread_cleanup_push(metadata_hub_thread_cleanup_function, NULL);
   while (1) {
@@ -4188,14 +4129,10 @@ void metadata_mqtt_close(void) {}
 void metadata_mqtt_thread_cleanup_function(__attribute__((unused)) void *arg) {
   // debug(2, "metadata_mqtt_thread_cleanup_function called");
   metadata_mqtt_close();
-  pc_queue_delete(&metadata_mqtt_queue);
   // debug(2, "metadata_mqtt_thread_cleanup_function done");
 }
 
 void *metadata_mqtt_thread_function(__attribute__((unused)) void *ignore) {
-  // create a pc_queue for passing information to a threaded metadata handler
-  pc_queue_init(&metadata_mqtt_queue, (char *)&metadata_mqtt_queue_items, sizeof(metadata_package),
-                metadata_mqtt_queue_size, "mqtt");
   metadata_package pack;
   pthread_cleanup_push(metadata_mqtt_thread_cleanup_function, NULL);
   while (1) {
@@ -4242,24 +4179,39 @@ void metadata_init(void) {
     if ((fd == -1) && (errno != ENXIO)) {
       char errorstring[1024];
       strerror_r(errno, (char *)errorstring, sizeof(errorstring));
-      debug(1, "metadata_hub_thread_function -- error %d (\"%s\") opening pipe: \"%s\".", errno,
+      debug(1, "metadata_init -- error %d (\"%s\") opening pipe: \"%s\".", errno,
             (char *)errorstring, path);
       warn("can not open metadata pipe -- error %d (\"%s\") opening pipe: \"%s\".", errno,
            (char *)errorstring, path);
     }
     free(path);
+
+    // initialise the metadata queues first, otherwise the might be a race condition
+    // create a pc_queue for the metadata pipe
+    pc_queue_init(&metadata_queue, (char *)&metadata_queue_items, sizeof(metadata_package),
+                  metadata_queue_size, "pipe");
+
     if (pthread_create(&metadata_thread, NULL, metadata_thread_function, NULL) != 0)
       debug(1, "Failed to create metadata thread!");
 
+    // create a pc_queue for the metadata_multicast_queue
+    pc_queue_init(&metadata_multicast_queue, (char *)&metadata_multicast_queue_items,
+                  sizeof(metadata_package), metadata_multicast_queue_size, "multicast");
     if (pthread_create(&metadata_multicast_thread, NULL, metadata_multicast_thread_function,
                        NULL) != 0)
       debug(1, "Failed to create metadata multicast thread!");
   }
 #ifdef CONFIG_METADATA_HUB
+  // create a pc_queue for the metadata hub
+  pc_queue_init(&metadata_hub_queue, (char *)&metadata_hub_queue_items, sizeof(metadata_package),
+                metadata_hub_queue_size, "hub");
   if (pthread_create(&metadata_hub_thread, NULL, metadata_hub_thread_function, NULL) != 0)
     debug(1, "Failed to create metadata hub thread!");
 #endif
 #ifdef CONFIG_MQTT
+  // create a pc_queue for the MQTT handler
+  pc_queue_init(&metadata_mqtt_queue, (char *)&metadata_mqtt_queue_items, sizeof(metadata_package),
+                metadata_mqtt_queue_size, "mqtt");
   if (pthread_create(&metadata_mqtt_thread, NULL, metadata_mqtt_thread_function, NULL) != 0)
     debug(1, "Failed to create metadata mqtt thread!");
 #endif
@@ -4273,6 +4225,7 @@ void metadata_stop(void) {
     // debug(2, "metadata stop mqtt thread.");
     pthread_cancel(metadata_mqtt_thread);
     pthread_join(metadata_mqtt_thread, NULL);
+    pc_queue_delete(&metadata_mqtt_queue);
     // debug(2, "metadata stop mqtt done.");
 #endif
 #ifdef CONFIG_METADATA_HUB
@@ -4280,6 +4233,7 @@ void metadata_stop(void) {
     pthread_cancel(metadata_hub_thread);
     pthread_join(metadata_hub_thread, NULL);
     // debug(2, "metadata stop hub done.");
+    pc_queue_delete(&metadata_hub_queue);
 #endif
     if (config.metadata_enabled) {
       // debug(2, "metadata stop multicast thread.");
@@ -4287,12 +4241,14 @@ void metadata_stop(void) {
         pthread_cancel(metadata_multicast_thread);
         pthread_join(metadata_multicast_thread, NULL);
         // debug(2, "metadata stop multicast done.");
+        pc_queue_delete(&metadata_multicast_queue);
       }
       if (metadata_thread) {
         // debug(2, "metadata stop metadata_thread thread.");
         pthread_cancel(metadata_thread);
         pthread_join(metadata_thread, NULL);
         // debug(2, "metadata_stop finished successfully.");
+        pc_queue_delete(&metadata_queue);
       }
     }
   }
@@ -4343,6 +4299,8 @@ int send_metadata_to_queue(pc_queue *queue, uint32_t type, uint32_t code, char *
     if (data)
       pack.data = memdup(data, length); // only if it's not a null
   }
+
+  // debug(1, "send_metadata_to_queue %x/%x", type, code);
   int rc = pc_queue_add_item(queue, &pack, block);
   if (rc != 0) {
     if (pack.carrier) {
@@ -5107,7 +5065,7 @@ void rtsp_conversation_thread_cleanup_function(void *arg) {
   if (conn != NULL) {
     int oldState;
     pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState);
-    debug(2, "Connection %d: %s rtsp_conversation_thread_func_cleanup_function called.",
+    debug(3, "Connection %d: %s rtsp_conversation_thread_func_cleanup_function called.",
           conn->connection_number, get_category_string(conn->airplay_stream_category));
 #ifdef CONFIG_AIRPLAY_2
     // AP2
@@ -5199,14 +5157,7 @@ void rtsp_conversation_thread_cleanup_function(void *arg) {
     if (rc)
       debug(1, "Connection %d: error %d destroying flush_mutex.", conn->connection_number, rc);
 
-    debug(3, "Cancel watchdog thread.");
-    pthread_cancel(conn->player_watchdog_thread);
-    debug(3, "Join watchdog thread.");
-    pthread_join(conn->player_watchdog_thread, NULL);
-    debug(3, "Delete watchdog mutex.");
-    pthread_mutex_destroy(&conn->watchdog_mutex);
-
-    debug(2, "Connection %d: Closed.", conn->connection_number);
+    debug(3, "Connection %d: Closed.", conn->connection_number);
     conn->running = 0; // for the garbage collector
     pthread_setcancelstate(oldState, NULL);
   }
@@ -5220,11 +5171,6 @@ void msg_cleanup_function(void *arg) {
 static void *rtsp_conversation_thread_func(void *pconn) {
   rtsp_conn_info *conn = pconn;
 
-  // create the watchdog mutex, initialise the watchdog time and start the watchdog thread;
-  conn->watchdog_bark_time = get_absolute_time_in_ns();
-  pthread_mutex_init(&conn->watchdog_mutex, NULL);
-  pthread_create(&conn->player_watchdog_thread, NULL, &player_watchdog_thread_code, (void *)conn);
-
   int rc = pthread_mutex_init(&conn->flush_mutex, NULL);
   if (rc)
     die("Connection %d: error %d initialising flush_mutex.", conn->connection_number, rc);
@@ -5642,43 +5588,61 @@ void *rtsp_listen_loop(__attribute((unused)) void *arg) {
       } else {
         size_of_reply = sizeof(SOCKADDR);
         if (getsockname(conn->fd, (struct sockaddr *)&conn->local, &size_of_reply) == 0) {
+
+          if ((config.dont_check_timeout == 0) && (config.timeout != 0)) {
+// skip this stuff in OpenBSD
 #ifndef COMPILE_FOR_OPENBSD
-          // Thanks to https://holmeshe.me/network-essentials-setsockopt-SO_KEEPALIVE/ for this.
+            // Thanks to https://holmeshe.me/network-essentials-setsockopt-SO_KEEPALIVE/ for this.
 
-          // turn on keepalive stuff -- wait for keepidle + (keepcnt * keepinttvl time) seconds
-          // before giving up an ETIMEOUT error is returned if the keepalive check fails
+            // turn on keepalive stuff -- wait for keepidle + (keepcnt * keepinttvl time) seconds
+            // before giving up an ETIMEOUT error is returned if the keepalive check fails
 
-          int keepAliveIdleTime = 35; // wait this many seconds before checking for a dropped client
-          int keepAliveCount = 5;     // check this many times
-          int keepAliveInterval = 5;  // wait this many seconds between checks
+            // if TCP_KEEPINTVL is defined, check a few times before declaring the line dead
+            // otherwise just wait a little while longer
 
+#ifdef TCP_KEEPINTVL
+            int keepAliveIdleTime =
+                config.timeout -
+                5 * 5;              // wait this many seconds before checking for a dropped client
+            int keepAliveCount = 5; // check this many times
+            int keepAliveInterval = 5; // wait this many seconds between checks
+#else
+            int keepAliveIdleTime =
+                config.timeout; // wait this many seconds before dropping a client
+#endif
+
+// --- the following is a bit  too complicated
+// decide to use IPPROTO_TCP or SOL_TCP
 #if defined COMPILE_FOR_BSD || defined COMPILE_FOR_OSX
 #define SOL_OPTION IPPROTO_TCP
 #else
 #define SOL_OPTION SOL_TCP
 #endif
-
+// decide to use TCP_KEEPALIVE or TCP_KEEPIDLE
 #ifdef COMPILE_FOR_OSX
 #define KEEP_ALIVE_OR_IDLE_OPTION TCP_KEEPALIVE
 #else
 #define KEEP_ALIVE_OR_IDLE_OPTION TCP_KEEPIDLE
 #endif
 
-          if (setsockopt(conn->fd, SOL_OPTION, KEEP_ALIVE_OR_IDLE_OPTION,
-                         (void *)&keepAliveIdleTime, sizeof(keepAliveIdleTime))) {
-            debug(1, "can't set the keepidle wait time");
-          }
-
-          if (setsockopt(conn->fd, SOL_OPTION, TCP_KEEPCNT, (void *)&keepAliveCount,
-                         sizeof(keepAliveCount))) {
-            debug(1, "can't set the keepidle missing count");
-          }
-          if (setsockopt(conn->fd, SOL_OPTION, TCP_KEEPINTVL, (void *)&keepAliveInterval,
-                         sizeof(keepAliveInterval))) {
-            debug(1, "can't set the keepidle missing count interval");
-          };
+            if (setsockopt(conn->fd, SOL_OPTION, KEEP_ALIVE_OR_IDLE_OPTION,
+                           (void *)&keepAliveIdleTime, sizeof(keepAliveIdleTime))) {
+              debug(1, "can't set the keepAliveIdleTime wait time");
+            }
+// ---
+// if TCP_KEEPINTVL is defined...
+#ifdef TCP_KEEPINTVL
+            if (setsockopt(conn->fd, SOL_OPTION, TCP_KEEPCNT, (void *)&keepAliveCount,
+                           sizeof(keepAliveCount))) {
+              debug(1, "can't set the keepAliveCount count");
+            }
+            if (setsockopt(conn->fd, SOL_OPTION, TCP_KEEPINTVL, (void *)&keepAliveInterval,
+                           sizeof(keepAliveInterval))) {
+              debug(1, "can't set the keepAliveCount count interval");
+            };
 #endif
-
+#endif
+          }
           // initialise the connection info
           void *client_addr = NULL, *self_addr = NULL;
           conn->connection_ip_family = conn->local.SAFAMILY;
@@ -5736,8 +5700,8 @@ void *rtsp_listen_loop(__attribute((unused)) void *arg) {
     pthread_cleanup_pop(1); // should never happen
   } else {
     die("could not establish a service on port %d -- program terminating. Is another instance of "
-         "Shairport Sync running on this device?",
-         config.port);
+        "Shairport Sync running on this device?",
+        config.port);
   }
   debug(1, "Oops -- fell out of the RTSP select loop");
   pthread_exit(NULL);

shairport.c

@@ -25,6 +25,8 @@
  * OTHER DEALINGS IN THE SOFTWARE.
  */
 
+#include <sys/socket.h>
+#include <sys/types.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <getopt.h>
@@ -35,9 +37,7 @@
 #include <popt.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <sys/socket.h>
 #include <sys/stat.h>
-#include <sys/types.h>
 #include <sys/wait.h>
 #include <unistd.h>
 
@@ -1207,8 +1207,8 @@ int parse_options(int argc, char **argv) {
 
     } else {
       if (config_error_type(&config_file_stuff) == CONFIG_ERR_FILE_IO)
-        die("Error reading configuration file \"%s\": \"%s\".",
-              config_file_real_path, config_error_text(&config_file_stuff));
+        die("Error reading configuration file \"%s\": \"%s\".", config_file_real_path,
+            config_error_text(&config_file_stuff));
       else {
         die("Line %d of the configuration file \"%s\":\n%s", config_error_line(&config_file_stuff),
             config_error_file(&config_file_stuff), config_error_text(&config_file_stuff));
@@ -1570,12 +1570,16 @@ const char *pid_file_proc(void) {
 #endif
 
 void exit_rtsp_listener() {
-  pthread_cancel(rtsp_listener_thread);
-  pthread_join(rtsp_listener_thread, NULL); // not sure you need this
+  debug(3, "exit_rtsp_listener begins");
+  if (type_of_exit_cleanup != TOE_emergency) {
+    pthread_cancel(rtsp_listener_thread);
+    pthread_join(rtsp_listener_thread, NULL); // not sure you need this
+  }
+  debug(3, "exit_rtsp_listener ends");
 }
 
 void exit_function() {
-
+  debug(3, "exit_function begins");
   if (type_of_exit_cleanup != TOE_emergency) {
     // the following is to ensure that if libdaemon has been included
     // that most of this code will be skipped when the parent process is exiting
@@ -2039,8 +2043,8 @@ int main(int argc, char **argv) {
   config.debugger_show_file_and_line =
       1; // by default, log the file and line of the originating message
   config.debugger_show_relative_time =
-      1;                // by default, log the  time back to the previous debug message
-  config.timeout = 120; // this number of seconds to wait for [more] audio before switching to idle.
+      1;                // by default, log the time back to the previous debug message
+  config.timeout = 120; // wait this number of seconds to wait for a dropped RTSP connection to come back before declaring it lost.
   config.buffer_start_fill = 220;
 
   config.resync_threshold = 0.050;   // default
@@ -2048,7 +2052,6 @@ int main(int argc, char **argv) {
   config.tolerance = 0.002;
 
 #ifdef CONFIG_AIRPLAY_2
-  config.timeout = 0; // disable watchdog
   config.port = 7000;
 #else
   config.port = 5000;
@@ -2110,12 +2113,9 @@ int main(int argc, char **argv) {
 
 #ifdef COMPILE_FOR_OPENBSD
   /* Any command to be executed at runtime? */
-  int run_cmds =
-    config.cmd_active_start != NULL ||
-    config.cmd_active_stop != NULL ||
-    config.cmd_set_volume != NULL ||
-    config.cmd_start != NULL ||
-    config.cmd_stop != NULL;
+  int run_cmds = config.cmd_active_start != NULL || config.cmd_active_stop != NULL ||
+                 config.cmd_set_volume != NULL || config.cmd_start != NULL ||
+                 config.cmd_stop != NULL;
 #endif
 
   // mDNS supports maximum of 63-character names (we append 13).
@@ -2381,11 +2381,11 @@ int main(int argc, char **argv) {
 #ifdef COMPILE_FOR_OPENBSD
   /* Past first and last sio_open(3), sndio(7) only needs "audio". */
 
-# ifdef CONFIG_METADATA
+#ifdef CONFIG_METADATA
   /* Only coverart cache is created.
    * Only metadata pipe is special. */
   if (!config.metadata_enabled)
-# endif
+#endif
   {
     /* Drop "cpath dpath". */
     if (run_cmds) {