Fix a bug in the activity_monitor software that could lock up a player thread so...

diff --git a/activity_monitor.c b/activity_monitor.c
index 51eeb0a285a674c1e614955f806231d0bc7016f1..3c525c196db431401e0a490ed850d17dc611a910 100644 (file)
--- 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 ee134a542bcf1860cbeef2e5de4db4b224171aa5..5896b4317fc6c3781ea4208c201184bb65faac03 100644 (file)
--- 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 f22ed5af1430867d0a87826f40157adf3e526922..3a544b9a80d59071f7fdd25f819cfe8fade08207 100644 (file)
--- 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 08b5bed1e0afde1b1d96b47bc96d0c0c5c1bc55b..ec70e2fed31db6e19eb34b95b0147cea9759b787 100644 (file)
--- 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 9bc849e1e85be53b8fee2244386ae1afd9d4b314..6f92abbe37311d14efd2abc4c44a9211dadaf729 100644 (file)
--- 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 72a79ef8e0515cf34ef2886dbd3b34640f6bd489..881a735955bb7b2e34660810ead04af013d47a37 100644 (file)
--- 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 923f4ee70c7134a205cf7d0f2d9c4c261b8c08e4..3f27a98240fd92c2ee79404ddf0e312ef88e4f43 100644 (file)
--- 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 21f4ff52705178a500143f62e36a665ed3f0e618..02aa362b4138ab0040c3b8bbc4ff6854463e4658 100644 (file)
--- 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);