From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 31 Dec 2022 17:16:02 +0000 (+0000) Subject: Be more careful checking for ENODEV conditions in audio_alsa.c. Still a few places... X-Git-Tag: 4.2.1d0~56 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=76387f8291a6f36bdb0feaf1c2aad1ea36c24f1d;p=thirdparty%2Fshairport-sync.git Be more careful checking for ENODEV conditions in audio_alsa.c. Still a few places where it is not done... --- diff --git a/audio_alsa.c b/audio_alsa.c index 608696ce..4fb4049c 100644 --- a/audio_alsa.c +++ b/audio_alsa.c @@ -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 = ¶meters; // 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; diff --git a/player.c b/player.c index 4cf4a66e..e526f66e 100644 --- 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 2104caca..aa95e2d5 100644 --- a/rtsp.c +++ b/rtsp.c @@ -37,6 +37,7 @@ #include #include #include +#include #include #include #include @@ -47,7 +48,6 @@ #include #include #include -#include #include @@ -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;