Ths "namehint" is a list, and there doesn't seem to have been any history where the separator would be a colon.

Signed-off-by: Mark Hills mark@xwax.org --- src/control/namehint.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/control/namehint.c b/src/control/namehint.c index ecd470f3..d81d3a7e 100644 --- a/src/control/namehint.c +++ b/src/control/namehint.c @@ -543,10 +543,10 @@ static int add_software_devices(snd_config_t *config, snd_config_t *rw_config, * User-defined hints are gathered from namehint.IFACE tree like: * * <code> - * namehint.pcm {<br> + * namehint.pcm [<br> * myfile "file:FILE=/tmp/soundwave.raw|Save sound output to /tmp/soundwave.raw"<br> - * myplug "plug:front:Do all conversions for front speakers"<br> - * } + * myplug "plug:front|Do all conversions for front speakers"<br> + * ] * </code> * * Note: The device description is separated with '|' char.

Looks like the documented behaviour was broken in commit 1ba513f9 in 2006, with the introduction of multiple fields.

I've chosen to match the described behaviour. Prior to this patch, using namehint could be made to work by exploiting the lack of escaping of the "name", populating the other fields:

"plug:front|DESCDo all conversions for front speakers"

rather than that which is documented and presumed to be the intention for asoundrc files:

"plug:front|Do all conversions for front speakers"

Everything seems to strongly suggest nobody is using this feature; I can find no working examples through a web search and probably someone would have hit this bug. It's not documented in configuration, only in the snd_device_name_hint() call. So it would probably clutter things to provide compatibility for the old behaviour.

I have found it to be very useful since working in Chromium, where it is the only way to expose chosen ALSA devices to web applications.

A temporary buffer is required to null-terminate the string. I see no use of alloca() in the code, presumably to avoid unbounded stack size. So memory is allocated on the heap.

Signed-off-by: Mark Hills mark@xwax.org --- src/control/namehint.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/control/namehint.c b/src/control/namehint.c index d81d3a7e..e4f696ad 100644 --- a/src/control/namehint.c +++ b/src/control/namehint.c @@ -78,6 +78,31 @@ static int hint_list_add(struct hint_list *list, return 0; }

+/** + * Add a namehint from string given in a user configuration file + */ +static int hint_list_add_custom(struct hint_list *list, + const char *entry) +{ + int err; + const char *sep; + char *name; + + assert(entry); + + sep = strchr(entry, '|'); + if (sep == NULL) + return hint_list_add(list, entry, NULL); + + name = strndup(entry, sep - entry); + if (name == NULL) + return -ENOMEM; + + err = hint_list_add(list, name, sep + 1); + free(name); + return err; +} + static void zero_handler(const char *file ATTRIBUTE_UNUSED, int line ATTRIBUTE_UNUSED, const char *function ATTRIBUTE_UNUSED, @@ -626,7 +651,7 @@ int snd_device_name_hint(int card, const char *iface, void ***hints) if (snd_config_get_string(snd_config_iterator_entry(i), &str) < 0) continue; - err = hint_list_add(&list, str, NULL); + err = hint_list_add_custom(&list, str); if (err < 0) goto __error; }

On systems with a network mounted home directory this is thoroughly useful to allow for a core set of asoundrc settings, but with different settings on different hosts.

It's not possibly to implement this in our own asoundrc or local customisation, as it's too late. The installation file must be modified.

This is similar to ~/.Xdefaults-* on some systems.

Signed-off-by: Mark Hills mark@xwax.org --- src/conf/alsa.conf | 11 +++++++++++ 1 file changed, 11 insertions(+)

diff --git a/src/conf/alsa.conf b/src/conf/alsa.conf index 18427ec6..4dae0e9c 100644 --- a/src/conf/alsa.conf +++ b/src/conf/alsa.conf @@ -24,6 +24,17 @@ "/alsa/asoundrc" ] } + { + @func concat + strings [ + "~/.asoundrc-" + { + @func getenv + vars [ HOSTNAME ] + default "localhost" + } + ] + } ] errors false }

This was the original bug that caused me to start looking at the ring buffer functions.

In the API this is documented as:

"Delay is distance between current application frame position and sound frame position. It's positive and less than buffer size in normal situation, negative on playback underrun and greater than buffer size on capture overrun. "

Because dsnoop was returning the buffer space available to the hardware the return value was always quite large, and moved in the wrong direction.

With this patch, dsnoop now gives results which are comparable to using the "hw" device directly. My test case was with snd-echo3g (Layla3G).

Signed-off-by: Mark Hills mark@xwax.org --- src/pcm/pcm_local.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h index 89d4125b..1fa8e61d 100644 --- a/src/pcm/pcm_local.h +++ b/src/pcm/pcm_local.h @@ -589,7 +589,7 @@ static inline snd_pcm_uframes_t snd_pcm_mmap_playback_delay(snd_pcm_t *pcm)

static inline snd_pcm_uframes_t snd_pcm_mmap_capture_delay(snd_pcm_t *pcm) { - return snd_pcm_mmap_capture_hw_avail(pcm); + return snd_pcm_mmap_capture_avail(pcm); }

static inline snd_pcm_sframes_t snd_pcm_mmap_delay(snd_pcm_t *pcm)

Signed-off-by: Mark Hills mark@xwax.org --- src/pcm/pcm_local.h | 6 ++++++ 1 file changed, 6 insertions(+)

diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h index 1fa8e61d..cf018fc0 100644 --- a/src/pcm/pcm_local.h +++ b/src/pcm/pcm_local.h @@ -582,11 +582,17 @@ static inline snd_pcm_uframes_t snd_pcm_mmap_hw_offset(snd_pcm_t *pcm) return *pcm->hw.ptr % pcm->buffer_size; }

+/* + * \retval number of frames pending from application to hardware + */ static inline snd_pcm_uframes_t snd_pcm_mmap_playback_delay(snd_pcm_t *pcm) { return snd_pcm_mmap_playback_hw_avail(pcm); }

+/* + * \retval number of frames pending from hardware to application + */ static inline snd_pcm_uframes_t snd_pcm_mmap_capture_delay(snd_pcm_t *pcm) { return snd_pcm_mmap_capture_avail(pcm);

The previous implementation would mean that stop_threshold behaved erratically. The intent is to detect that the buffer is too full, and stop.

In practice, I don't think this was a bug in practice for applications which don't adjust the stop_threshold. The line above catches those cases.

Signed-off-by: Mark Hills mark@xwax.org --- src/pcm/pcm_dsnoop.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/pcm/pcm_dsnoop.c b/src/pcm/pcm_dsnoop.c index c64df381..790d944c 100644 --- a/src/pcm/pcm_dsnoop.c +++ b/src/pcm/pcm_dsnoop.c @@ -165,7 +165,7 @@ static int snd_pcm_dsnoop_sync_ptr(snd_pcm_t *pcm) // printf("sync ptr diff = %li\n", diff); if (pcm->stop_threshold >= pcm->boundary) /* don't care */ return 0; - if ((avail = snd_pcm_mmap_capture_hw_avail(pcm)) >= pcm->stop_threshold) { + if ((avail = snd_pcm_mmap_capture_avail(pcm)) >= pcm->stop_threshold) { gettimestamp(&dsnoop->trigger_tstamp, pcm->tstamp_type); dsnoop->state = SND_PCM_STATE_XRUN; dsnoop->avail_max = avail;

This looks like a simple mistake dating back to 2003 (commit 7470a5b9) where code originated from dmix.

Signed-off-by: Mark Hills mark@xwax.org --- src/pcm/pcm_dsnoop.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/pcm/pcm_dsnoop.c b/src/pcm/pcm_dsnoop.c index 790d944c..3588eb91 100644 --- a/src/pcm/pcm_dsnoop.c +++ b/src/pcm/pcm_dsnoop.c @@ -241,7 +241,7 @@ static int snd_pcm_dsnoop_delay(snd_pcm_t *pcm, snd_pcm_sframes_t *delayp) /* Fall through */ case SNDRV_PCM_STATE_PREPARED: case SNDRV_PCM_STATE_SUSPENDED: - *delayp = snd_pcm_mmap_capture_hw_avail(pcm); + *delayp = snd_pcm_mmap_capture_avail(pcm); return 0; case SNDRV_PCM_STATE_XRUN: return -EPIPE;

Match the equivalent funciton for playback. This is on the assumption that values should be capped at zero, which is what _rewindable() implements.

Signed-off-by: Mark Hills mark@xwax.org --- src/pcm/pcm_dsnoop.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/pcm/pcm_dsnoop.c b/src/pcm/pcm_dsnoop.c index 3588eb91..7904314c 100644 --- a/src/pcm/pcm_dsnoop.c +++ b/src/pcm/pcm_dsnoop.c @@ -352,7 +352,7 @@ static int snd_pcm_dsnoop_pause(snd_pcm_t *pcm ATTRIBUTE_UNUSED, int enable ATTR

static snd_pcm_sframes_t snd_pcm_dsnoop_rewindable(snd_pcm_t *pcm) { - return snd_pcm_mmap_capture_hw_avail(pcm); + return snd_pcm_mmap_capture_hw_rewindable(pcm); }

static snd_pcm_sframes_t snd_pcm_dsnoop_rewind(snd_pcm_t *pcm, snd_pcm_uframes_t frames)

I took time to understand these functions in the context of the rest of the code, which would have been a lot quicker with a comment like this.

Signed-off-by: Mark Hills mark@xwax.org --- src/pcm/pcm_local.h | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)

diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h index cf018fc0..aae58ed3 100644 --- a/src/pcm/pcm_local.h +++ b/src/pcm/pcm_local.h @@ -480,6 +480,13 @@ static inline int snd_pcm_check_error(snd_pcm_t *pcm, int err) return err; }

+/** + * \retval number of frames available to the application for playback + * + * This is how far ahead the hardware position in the ring buffer is, + * compared to the application position. ie. for playback it's the + * number of frames in the empty part of the ring buffer. + */ static inline snd_pcm_uframes_t __snd_pcm_playback_avail(snd_pcm_t *pcm, const snd_pcm_uframes_t hw_ptr, const snd_pcm_uframes_t appl_ptr) @@ -498,6 +505,13 @@ static inline snd_pcm_uframes_t snd_pcm_mmap_playback_avail(snd_pcm_t *pcm) return __snd_pcm_playback_avail(pcm, *pcm->hw.ptr, *pcm->appl.ptr); }

+/* + * \retval number of frames available to the application for capture + * + * This is how far ahead the hardware position in the ring buffer is + * compared to the application position. ie. for capture, it's the + * number of frames in the filled part of the ring buffer. + */ static inline snd_pcm_uframes_t __snd_pcm_capture_avail(snd_pcm_t *pcm, const snd_pcm_uframes_t hw_ptr, const snd_pcm_uframes_t appl_ptr) @@ -529,11 +543,21 @@ static inline snd_pcm_uframes_t snd_pcm_mmap_avail(snd_pcm_t *pcm) return __snd_pcm_avail(pcm, *pcm->hw.ptr, *pcm->appl.ptr); }

+/* + * \retval number of frames available to the hardware for playback + * + * ie. the filled part of the ring buffer + */ static inline snd_pcm_sframes_t snd_pcm_mmap_playback_hw_avail(snd_pcm_t *pcm) { return pcm->buffer_size - snd_pcm_mmap_playback_avail(pcm); }

+/* + * \retval number of frames available to the hardware for capture + * + * ie. the empty part of the ring buffer. + */ static inline snd_pcm_sframes_t snd_pcm_mmap_capture_hw_avail(snd_pcm_t *pcm) { return pcm->buffer_size - snd_pcm_mmap_capture_avail(pcm);