]> git.ipfire.org Git - thirdparty/shairport-sync.git/commitdiff
Be more careful checking for ENODEV conditions in audio_alsa.c. Still a few places...
authorMike Brady <4265913+mikebrady@users.noreply.github.com>
Sat, 31 Dec 2022 17:16:02 +0000 (17:16 +0000)
committerMike Brady <4265913+mikebrady@users.noreply.github.com>
Sat, 31 Dec 2022 17:16:02 +0000 (17:16 +0000)
audio_alsa.c
player.c
rtsp.c

index 608696ceada02492a3e23bfebcbf5d59b327a002..4fb4049c4708f569a26e45e05d126ff078504fd6 100644 (file)
@@ -166,6 +166,22 @@ int volume_based_mute_is_active =
 // use this to allow the use of snd_pcm_writei or snd_pcm_mmap_writei
 snd_pcm_sframes_t (*alsa_pcm_write)(snd_pcm_t *, const void *, snd_pcm_uframes_t) = snd_pcm_writei;
 
+void handle_unfixable_error(int errorCode) {
+  if (config.unfixable_error_reported == 0) {
+    config.unfixable_error_reported = 1;
+    char messageString[1024];
+    messageString[0] = '\0';
+    snprintf(messageString, sizeof(messageString), "output_device_error_%d", errorCode);
+    if (config.cmd_unfixable) {
+      command_execute(config.cmd_unfixable, messageString, 1);
+    } else {
+      die("An unrecoverable error, \"output_device_error_%d\", has been "
+          "detected. Doing an emergency exit, as no run_this_if_an_unfixable_error_is_detected program.",
+          errorCode);
+    }
+  }
+}
+
 static int precision_delay_and_status(snd_pcm_state_t *state, snd_pcm_sframes_t *delay,
                                       yndk_type *using_update_timestamps);
 static int standard_delay_and_status(snd_pcm_state_t *state, snd_pcm_sframes_t *delay,
@@ -277,40 +293,33 @@ static void help(void) {
 void set_alsa_out_dev(char *dev) { alsa_out_dev = dev; } // ugh -- not static!
 
 // assuming pthread cancellation is disabled
+// returns zero of all is okay, a Unx error code if there's a problem
 static int open_mixer() {
   int response = 0;
   if (alsa_mix_ctrl != NULL) {
     debug(3, "Open Mixer");
-    int ret = 0;
     snd_mixer_selem_id_alloca(&alsa_mix_sid);
     snd_mixer_selem_id_set_index(alsa_mix_sid, alsa_mix_index);
     snd_mixer_selem_id_set_name(alsa_mix_sid, alsa_mix_ctrl);
 
-    if ((snd_mixer_open(&alsa_mix_handle, 0)) < 0) {
+    if ((response = snd_mixer_open(&alsa_mix_handle, 0)) < 0) {
       debug(1, "Failed to open mixer");
-      response = -1;
     } else {
       debug(3, "Mixer device name is \"%s\".", alsa_mix_dev);
-      if ((snd_mixer_attach(alsa_mix_handle, alsa_mix_dev)) < 0) {
+      if ((response = snd_mixer_attach(alsa_mix_handle, alsa_mix_dev)) < 0) {
         debug(1, "Failed to attach mixer");
-        response = -2;
       } else {
-        if ((snd_mixer_selem_register(alsa_mix_handle, NULL, NULL)) < 0) {
+        if ((response = snd_mixer_selem_register(alsa_mix_handle, NULL, NULL)) < 0) {
           debug(1, "Failed to register mixer element");
-          response = -3;
         } else {
-          ret = snd_mixer_load(alsa_mix_handle);
-          if (ret < 0) {
+          if ((response = snd_mixer_load(alsa_mix_handle)) < 0) {
             debug(1, "Failed to load mixer element");
-            response = -4;
           } else {
             debug(3, "Mixer control is \"%s\",%d.", alsa_mix_ctrl, alsa_mix_index);
             alsa_mix_elem = snd_mixer_find_selem(alsa_mix_handle, alsa_mix_sid);
             if (!alsa_mix_elem) {
               warn("failed to find mixer control \"%s\",%d.", alsa_mix_ctrl, alsa_mix_index);
-              response = -5;
-            } else {
-              response = 1; // we found a hardware mixer and successfully opened it
+              response = -ENXIO; // don't let this be ENODEV!
             }
           }
         }
@@ -321,21 +330,25 @@ static int open_mixer() {
 }
 
 // assuming pthread cancellation is disabled
-static void close_mixer() {
+static int close_mixer() {
+  int ret = 0;
   if (alsa_mix_handle) {
-    snd_mixer_close(alsa_mix_handle);
+    ret = snd_mixer_close(alsa_mix_handle);
     alsa_mix_handle = NULL;
   }
+  return ret;
 }
 
 // assuming pthread cancellation is disabled
-static void do_snd_mixer_selem_set_playback_dB_all(snd_mixer_elem_t *mix_elem, double vol) {
-  if (snd_mixer_selem_set_playback_dB_all(mix_elem, vol, 0) != 0) {
+static int do_snd_mixer_selem_set_playback_dB_all(snd_mixer_elem_t *mix_elem, double vol) {
+  int response = 0;
+  if ((response = snd_mixer_selem_set_playback_dB_all(mix_elem, vol, 0)) != 0) {
     debug(1, "Can't set playback volume accurately to %f dB.", vol);
-    if (snd_mixer_selem_set_playback_dB_all(mix_elem, vol, -1) != 0)
-      if (snd_mixer_selem_set_playback_dB_all(mix_elem, vol, 1) != 0)
+    if ((response = snd_mixer_selem_set_playback_dB_all(mix_elem, vol, -1)) != 0)
+      if ((response = snd_mixer_selem_set_playback_dB_all(mix_elem, vol, 1)) != 0)
         debug(1, "Could not set playback dB volume on the mixer.");
   }
+  return response;
 }
 
 // This array is a sequence of the output rates to be tried if automatic speed selection is
@@ -444,19 +457,6 @@ static int actual_open_alsa_device(int do_auto_setup) {
         warn("The output device \"%s\" is busy and can't be used by Shairport Sync at present.",
              alsa_out_dev);
       debug(2, "the alsa output_device \"%s\" is busy.", alsa_out_dev);
-    } else {
-      if (config.unfixable_error_reported == 0) {
-        config.unfixable_error_reported = 1;        
-        char messageString[1024];
-        messageString[0] = '\0';
-        snprintf(messageString, sizeof(messageString), "output_device_error_%d", -ret);            
-        if (config.cmd_unfixable) {
-          command_execute(config.cmd_unfixable, messageString, 1);
-        } else {
-          die("an unrecoverable error, \"output_device_error_%d\", has been "
-              "detected.", -ret);
-        }
-      }
     }
     alsa_handle_status = ret;
     frames_sent_break_occurred = 1;
@@ -906,11 +906,11 @@ static int prepare_mixer() {
     // Now, start trying to initialise the alsa device with the settings
     // obtained
     pthread_cleanup_debug_mutex_lock(&alsa_mixer_mutex, 1000, 1);
-    if (open_mixer() == 1) {
+    if (open_mixer() == 0) {
       if (snd_mixer_selem_get_playback_volume_range(alsa_mix_elem, &alsa_mix_minv, &alsa_mix_maxv) <
-          0)
+          0) {
         debug(1, "Can't read mixer's [linear] min and max volumes.");
-      else {
+      else {
         if (snd_mixer_selem_get_playback_dB_range(alsa_mix_elem, &alsa_mix_mindb,
                                                   &alsa_mix_maxdb) == 0) {
 
@@ -940,14 +940,12 @@ static int prepare_mixer() {
                "a dB volume scale.",
                alsa_mix_ctrl);
 
-          if (snd_ctl_open(&ctl, alsa_mix_dev, 0) < 0) {
+          if ((response = snd_ctl_open(&ctl, alsa_mix_dev, 0)) < 0) {
             warn("Cannot open control \"%s\"", alsa_mix_dev);
-            response = -1;
           }
-          if (snd_ctl_elem_id_malloc(&elem_id) < 0) {
+          if ((response = snd_ctl_elem_id_malloc(&elem_id)) < 0) {
             debug(1, "Cannot allocate memory for control \"%s\"", alsa_mix_dev);
             elem_id = NULL;
-            response = -2;
           } else {
             snd_ctl_elem_id_set_interface(elem_id, SND_CTL_ELEM_IFACE_MIXER);
             snd_ctl_elem_id_set_name(elem_id, alsa_mix_ctrl);
@@ -965,22 +963,6 @@ static int prepare_mixer() {
               debug(1, "Cannot get the dB range from the volume control \"%s\"", alsa_mix_ctrl);
             }
           }
-          /*
-          debug(1, "Min and max volumes are %d and
-          %d.",alsa_mix_minv,alsa_mix_maxv);
-          alsa_mix_maxdb = 0;
-          if ((alsa_mix_maxv!=0) && (alsa_mix_minv!=0))
-            alsa_mix_mindb =
-          -20*100*(log10(alsa_mix_maxv*1.0)-log10(alsa_mix_minv*1.0));
-          else if (alsa_mix_maxv!=0)
-            alsa_mix_mindb = -20*100*log10(alsa_mix_maxv*1.0);
-          audio_alsa.volume = &linear_volume; // insert the linear volume
-          function
-          audio_alsa.parameters = &parameters; // likewise the parameters
-          stuff
-          debug(1,"Max and min dB calculated are %d and
-          %d.",alsa_mix_maxdb,alsa_mix_mindb);
-          */
         }
       }
       if (((config.alsa_use_hardware_mute == 1) &&
@@ -992,7 +974,8 @@ static int prepare_mixer() {
       } else {
         // debug(1, "Has mixer but not using hardware mute.");
       }
-      close_mixer();
+      if (response == 0)
+        response = close_mixer();
     }
     debug_mutex_unlock(&alsa_mixer_mutex, 3); // release the mutex
     pthread_cleanup_pop(0);
@@ -1399,7 +1382,7 @@ static int set_mute_state() {
   pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); // make this un-cancellable
   pthread_cleanup_debug_mutex_lock(&alsa_mixer_mutex, 10000, 0);
   if ((alsa_backend_state != abm_disconnected) && (config.alsa_use_hardware_mute == 1) &&
-      (open_mixer() == 1)) {
+      (open_mixer() == 0)) {
     response = 0; // okay if actually using the mute facility
     debug(2, "alsa: actually set_mute_state");
     int mute = 0;
@@ -1718,36 +1701,20 @@ static int stats(uint64_t *raw_measurement_time, uint64_t *corrected_measurement
   *the_delay = hd;
   return ret;
 }
-/*
-static int get_rate_information(uint64_t *elapsed_time, uint64_t *frames_played) {
-  // elapsed_time is in nanoseconds
-  int response = 0; // zero means okay
-  if (measurement_data_is_valid) {
-    *elapsed_time = measurement_time - measurement_start_time;
-    *frames_played = frames_played_at_measurement_time - frames_played_at_measurement_start_time;
-  } else {
-    *elapsed_time = 0;
-    *frames_played = 0;
-    response = -1;
-  }
-  return response;
-}
-*/
 
 static int do_play(void *buf, int samples) {
   // assuming the alsa_mutex has been acquired
-  // debug(3,"audio_alsa play called.");
-  int oldState;
-  pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); // make this un-cancellable
-
-  snd_pcm_state_t state;
-  snd_pcm_sframes_t my_delay;
-  int ret = delay_and_status(&state, &my_delay, NULL);
+  int ret = 0;
+  if ((samples != 0) && (buf != NULL)) {
 
-  if (ret == 0) { // will be non-zero if an error or a stall
+    int oldState;
+    pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); // make this un-cancellable
 
-    if ((samples != 0) && (buf != NULL)) {
+    snd_pcm_state_t state;
+    snd_pcm_sframes_t my_delay;
+    ret = delay_and_status(&state, &my_delay, NULL);
 
+    if (ret == 0) { // will be non-zero if an error or a stall
       // just check the state of the DAC
 
       if ((state != SND_PCM_STATE_PREPARED) && (state != SND_PCM_STATE_RUNNING) &&
@@ -1784,51 +1751,27 @@ static int do_play(void *buf, int samples) {
             debug(1, "alsa: recovering from a previous underrun.");
           else
             debug(1, "alsa: underrun while writing %d samples to alsa device.", samples);
-          int tret = snd_pcm_recover(alsa_handle, ret, 1);
-          if (tret < 0) {
-            warn("alsa: can't recover from SND_PCM_STATE_XRUN: %s.", snd_strerror(tret));
-          }
+          ret = snd_pcm_recover(alsa_handle, ret, 1);
         } else if (ret == -ESTRPIPE) { /* suspended */
-          debug(1, "alsa: suspended while writing %d samples to alsa device.", samples);
-          int tret;
-          while ((tret = snd_pcm_resume(alsa_handle)) == -EAGAIN) {
-            sleep(1); /* wait until the suspend flag is released */
-            if (tret < 0) {
-              warn("alsa: can't recover from SND_PCM_STATE_SUSPENDED state, "
-                   "snd_pcm_prepare() "
-                   "failed: %s.",
-                   snd_strerror(tret));
-            }
-          }
-        } else {
-
-          if (ret == -ENODEV) {
-            if (config.unfixable_error_reported == 0) {
-              config.unfixable_error_reported = 1;
-              if (config.cmd_unfixable) {
-                command_execute(config.cmd_unfixable, "output_device_error_19", 1);
-              } else {
-                die("an unrecoverable error, \"output_device_error_19\", has been "
-                    "detected.");
-              }
-            }
-          } else {
-            char errorstring[1024];
-            strerror_r(-ret, (char *)errorstring, sizeof(errorstring));
-            debug(1, "alsa: error %d (\"%s\") writing %d samples to alsa device.", ret,
-                  (char *)errorstring, samples);
-          }
+          if (state != prior_state)
+            debug(1, "alsa: suspended while writing %d samples to alsa device.", samples);
+          if ((ret = snd_pcm_resume(alsa_handle)) == -ENOSYS)
+            ret = snd_pcm_prepare(alsa_handle);
+        } else if (ret >= 0) {
+          debug(1, "alsa: only %d of %d samples output.", ret, samples);
         }
       }
     }
-  } else {
-    debug(1,
-          "alsa: device status returns fault status %d and SND_PCM_STATE_* "
-          "%d  for play.",
-          ret, state);
+    pthread_setcancelstate(oldState, NULL);
+    if (ret < 0) {
+      char errorstring[1024];
+      strerror_r(-ret, (char *)errorstring, sizeof(errorstring));
+      debug(1, "alsa: SND_PCM_STATE_* %d, error %d (\"%s\") writing %d samples to alsa device.",
+            state, ret, (char *)errorstring, samples);
+    }
+    if (ret == -ENODEV) // if the device isn't there...
+      handle_unfixable_error(-ret);
   }
-
-  pthread_setcancelstate(oldState, NULL);
   return ret;
 }
 
@@ -1854,6 +1797,9 @@ static int do_open(int do_auto_setup) {
       // any previously-reported frame count
       frames_sent_for_playing = 0;
       alsa_backend_state = abm_connected; // only do this if it really opened it.
+    } else {
+      if (ret == -ENODEV) // if the device isn't there...
+        handle_unfixable_error(-ret);
     }
   } else {
     debug(1, "alsa: do_open() -- output device already open.");
@@ -2006,7 +1952,7 @@ static void do_volume(double vol) { // caller is assumed to have the alsa_mutex
   pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); // make this un-cancellable
   set_volume = vol;
   pthread_cleanup_debug_mutex_lock(&alsa_mixer_mutex, 1000, 1);
-  if (volume_set_request && (open_mixer() == 1)) {
+  if (volume_set_request && (open_mixer() == 0)) {
     if (has_softvol) {
       if (ctl && elem_id) {
         snd_ctl_elem_value_t *value;
index 4cf4a66e29929bb398eed261f7964da1cfc87de3..e526f66e470487d108ee0068701dfe921f235b74 100644 (file)
--- a/player.c
+++ b/player.c
@@ -531,9 +531,9 @@ void player_put_packet(int original_format, seq_t seqno, uint32_t actual_timesta
           (uint64_t)(config.resend_control_first_check_time * (uint64_t)1000000000);
       uint64_t resend_repeat_interval =
           (uint64_t)(config.resend_control_check_interval_time * (uint64_t)1000000000);
-      uint64_t minimum_remaining_time = (uint64_t)((config.resend_control_last_check_time +
-                                                    config.audio_backend_buffer_desired_length) *
-                                                   (uint64_t)1000000000);
+      uint64_t minimum_remaining_time = (uint64_t)(
+          (config.resend_control_last_check_time + config.audio_backend_buffer_desired_length) *
+          (uint64_t)1000000000);
       uint64_t latency_time = (uint64_t)(conn->latency * (uint64_t)1000000000);
       latency_time = latency_time / (uint64_t)conn->input_rate;
 
@@ -1982,9 +1982,8 @@ void *player_thread_func(void *arg) {
 
   debug(3, "Output frame bytes is %d.", conn->output_bytes_per_frame);
 
-  conn->dac_buffer_queue_minimum_length =
-      (uint64_t)(config.audio_backend_buffer_interpolation_threshold_in_seconds *
-                 config.output_rate);
+  conn->dac_buffer_queue_minimum_length = (uint64_t)(
+      config.audio_backend_buffer_interpolation_threshold_in_seconds * config.output_rate);
   debug(3, "dac_buffer_queue_minimum_length is %" PRIu64 " frames.",
         conn->dac_buffer_queue_minimum_length);
 
diff --git a/rtsp.c b/rtsp.c
index 2104caca01b7afa44498ca50c41795c23b514647..aa95e2d517655490ff011690594ff0cc3821b028 100644 (file)
--- a/rtsp.c
+++ b/rtsp.c
@@ -37,6 +37,7 @@
 #include <net/if.h>
 #include <netdb.h>
 #include <netinet/in.h>
+#include <netinet/tcp.h>
 #include <poll.h>
 #include <pthread.h>
 #include <stdio.h>
@@ -47,7 +48,6 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
-#include <netinet/tcp.h>
 
 #include <sys/ioctl.h>
 
@@ -1223,10 +1223,10 @@ ssize_t timed_read_from_rtsp_connection(rtsp_conn_info *conn, uint64_t wait_time
     uint64_t time_to_wait_to = get_absolute_time_in_ns();
     ;
     time_to_wait_to = time_to_wait_to + wait_time;
-    
+
     int flags = 1;
     if (setsockopt(conn->fd, SOL_SOCKET, SO_KEEPALIVE, (void *)&flags, sizeof(flags))) {
-      debug(1,"can't enable keepalive checking on the RTSP socket");
+      debug(1, "can't enable keepalive checking on the RTSP socket");
     }
 
     // remaining_time will be zero if wait_time is zero
@@ -1355,7 +1355,7 @@ enum rtsp_read_request_response rtsp_read_request(rtsp_conn_info *conn, rtsp_mes
       // Note: the socket will be closed when the thread exits
       goto shutdown;
     }
-    
+
     // An ETIMEDOUT error usually means keepalive has failed.
 
     if (nread < 0) {
@@ -5497,39 +5497,40 @@ void *rtsp_listen_loop(__attribute((unused)) void *arg) {
         if (getsockname(conn->fd, (struct sockaddr *)&conn->local, &size_of_reply) == 0) {
 
           // 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
-          
+          int keepAliveCount = 5;     // check this many times
+          int keepAliveInterval = 5;  // wait this many seconds between checks
 
 #if defined COMPILE_FOR_BSD || defined COMPILE_FOR_OSX
-          #define SOL_OPTION IPPROTO_TCP
+#define SOL_OPTION IPPROTO_TCP
 #else
-          #define SOL_OPTION SOL_TCP
+#define SOL_OPTION SOL_TCP
 #endif
 
 #ifdef COMPILE_FOR_OSX
-          #define KEEP_ALIVE_OR_IDLE_OPTION TCP_KEEPALIVE
+#define KEEP_ALIVE_OR_IDLE_OPTION TCP_KEEPALIVE
 #else
-          #define KEEP_ALIVE_OR_IDLE_OPTION TCP_KEEPIDLE
+#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, 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_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, TCP_KEEPINTVL, (void *)&keepAliveInterval,
+                         sizeof(keepAliveInterval))) {
+            debug(1, "can't set the keepidle missing count interval");
           };
-       
+
           // initialise the connection info
           void *client_addr = NULL, *self_addr = NULL;
           conn->connection_ip_family = conn->local.SAFAMILY;