From 53bbd5127f8bb9132aa9e252928d748f103d7e4e Mon Sep 17 00:00:00 2001 From: Andre Noll Date: Sun, 5 Sep 2021 20:16:59 +0200 Subject: [PATCH] Teach writers to abort gracefully on early EOF. For very short streams it may happen that the receiver and decoder unregister themselves from the buffer tree before the writer had a chance to query the information from the decoder which it needs to open the audio device. This leads to errors such as Aug 25 14:24:51 schubert (5) get_btr_value: cmd sample_rate: Operation not supported Aug 25 14:24:51 schubert (5) get_btr_value: cmd channels: Operation not supported Aug 25 14:24:51 schubert (5) get_btr_value: cmd sample_format: Operation not supported Aug 25 14:24:51 schubert (4) alsa_init: channels count not available: Invalid argument This may happen with all receivers, audio formats and writers, although it is most common with ogg streams. This commit changes get_btr_sample_rate() and friends to return a standard error code rather than assuming success. The alsa, ao and oss writers are patched to check the return value and fail gracefully if one of these functions fails. --- alsa_write.c | 13 +++++++++---- ao_write.c | 12 +++++++++--- oss_write.c | 12 +++++++++--- write.h | 6 +++--- write_common.c | 38 +++++++++++++++++++++----------------- 5 files changed, 51 insertions(+), 30 deletions(-) diff --git a/alsa_write.c b/alsa_write.c index 9822da67..2d834022 100644 --- a/alsa_write.c +++ b/alsa_write.c @@ -297,13 +297,18 @@ again: if (bytes == 0) /* no data available */ return 0; pad = wn->private_data = para_calloc(sizeof(*pad)); - get_btr_sample_rate(btrn, &val); + ret = get_btr_sample_rate(btrn, &val); + if (ret < 0) + goto err; pad->sample_rate = val; - get_btr_channels(btrn, &val); + ret = get_btr_channels(btrn, &val); + if (ret < 0) + goto err; pad->channels = val; - get_btr_sample_format(btrn, &val); + ret = get_btr_sample_format(btrn, &val); + if (ret < 0) + goto err; pad->sample_format = get_alsa_pcm_format(val); - PARA_INFO_LOG("%u channel(s), %uHz\n", pad->channels, pad->sample_rate); ret = alsa_init(wn); diff --git a/ao_write.c b/ao_write.c index 447dea84..037b9299 100644 --- a/ao_write.c +++ b/ao_write.c @@ -357,9 +357,15 @@ static int aow_post_select(__a_unused struct sched *s, void *context) goto remove_btrn; if (ret == 0) return 0; - get_btr_sample_rate(wn->btrn, &rate); - get_btr_channels(wn->btrn, &ch); - get_btr_sample_format(wn->btrn, &format); + ret = get_btr_sample_rate(wn->btrn, &rate); + if (ret < 0) + goto remove_btrn; + ret = get_btr_channels(wn->btrn, &ch); + if (ret < 0) + goto remove_btrn; + ret = get_btr_sample_format(wn->btrn, &format); + if (ret < 0) + goto remove_btrn; ret = aow_init(wn, rate, ch, format); if (ret < 0) goto remove_btrn; diff --git a/oss_write.c b/oss_write.c index 311a514d..0565167c 100644 --- a/oss_write.c +++ b/oss_write.c @@ -199,9 +199,15 @@ static int oss_post_select(__a_unused struct sched *s, void *context) if (sound_device_is_busy()) return 0; - get_btr_sample_rate(btrn, &rate); - get_btr_channels(btrn, &ch); - get_btr_sample_format(btrn, &format); + ret = get_btr_sample_rate(btrn, &rate); + if (ret < 0) + goto out; + ret = get_btr_channels(btrn, &ch); + if (ret < 0) + goto out; + ret = get_btr_sample_format(btrn, &format); + if (ret < 0) + goto out; ret = oss_init(wn, rate, ch, format); if (ret < 0) goto out; diff --git a/write.h b/write.h index cb0beff8..833cb69a 100644 --- a/write.h +++ b/write.h @@ -66,7 +66,7 @@ const struct writer *writer_get(int wid); const char *writer_name(int wid); void register_writer_node(struct writer_node *wn, struct btr_node *parent, struct sched *s); -void get_btr_sample_rate(struct btr_node *btrn, int32_t *result); -void get_btr_channels(struct btr_node *btrn, int32_t *result); -void get_btr_sample_format(struct btr_node *btrn, int32_t *result); +int get_btr_sample_rate(struct btr_node *btrn, int32_t *result); +int get_btr_channels(struct btr_node *btrn, int32_t *result); +int get_btr_sample_format(struct btr_node *btrn, int32_t *result); void print_writer_helps(bool detailed); diff --git a/write_common.c b/write_common.c index 14cc98a4..41c3eb23 100644 --- a/write_common.c +++ b/write_common.c @@ -174,24 +174,24 @@ void print_writer_helps(bool detailed) } } -static void get_btr_value(struct btr_node *btrn, const char *cmd, +static int get_btr_value(struct btr_node *btrn, const char *cmd, int32_t *result) { char *buf = NULL; int ret = btr_exec_up(btrn, cmd, &buf); - if (ret < 0) { - /* - * This really should not happen. It means one of our parent - * nodes died unexpectedly. Proceed with fingers crossed. - */ - PARA_CRIT_LOG("cmd %s: %s\n", cmd, para_strerror(-ret)); - *result = 0; - return; - } + *result = 0; + /* + * Errors may happen when the decoder returns EOF before the writer had + * a chance to query the buffer tree for the channel count, sample rate + * etc. + */ + if (ret < 0) + return ret; ret = para_atoi32(buf, result); assert(ret >= 0); free(buf); + return ret; } /** @@ -200,11 +200,11 @@ static void get_btr_value(struct btr_node *btrn, const char *cmd, * \param btrn Where to start the search. * \param result Filled in by this function. * - * This function is assumed to succeed and terminates on errors. + * \return Standard. */ -void get_btr_sample_rate(struct btr_node *btrn, int32_t *result) +int get_btr_sample_rate(struct btr_node *btrn, int32_t *result) { - get_btr_value(btrn, "sample_rate", result); + return get_btr_value(btrn, "sample_rate", result); } /** @@ -212,10 +212,12 @@ void get_btr_sample_rate(struct btr_node *btrn, int32_t *result) * * \param btrn See \ref get_btr_sample_rate. * \param result See \ref get_btr_sample_rate. + * + * \return Standard. */ -void get_btr_channels(struct btr_node *btrn, int32_t *result) +int get_btr_channels(struct btr_node *btrn, int32_t *result) { - get_btr_value(btrn, "channels", result); + return get_btr_value(btrn, "channels", result); } /** @@ -223,8 +225,10 @@ void get_btr_channels(struct btr_node *btrn, int32_t *result) * * \param btrn See \ref get_btr_sample_rate. * \param result Contains the sample format as an enum sample_format type. + * + * \return Standard. */ -void get_btr_sample_format(struct btr_node *btrn, int32_t *result) +int get_btr_sample_format(struct btr_node *btrn, int32_t *result) { - get_btr_value(btrn, "sample_format", result); + return get_btr_value(btrn, "sample_format", result); } -- 2.39.2