From dd82f13fa1241266576b508180fcf90b8d9bda2c Mon Sep 17 00:00:00 2001 From: Amaury Pouly Date: Mon, 19 Feb 2018 16:28:11 +1100 Subject: nwz/alsa: various improvements Also audiohw driver to specific device name, rewrite alsa controls code to cache more data, thus making the code easier and use less stack. Avoid using short/long in pcm alsa code since it's the wrong size on 64-bit (simulator for example) Change-Id: Ibc1ec44396e37b6cbdedbcf37300878638e5d2d3 --- firmware/target/hosted/alsa-controls.c | 167 ++++++++++++++++++----------- firmware/target/hosted/alsa-controls.h | 5 +- firmware/target/hosted/pcm-alsa.c | 18 ++-- firmware/target/hosted/pcm-alsa.h | 2 +- firmware/target/hosted/sonynwz/debug-nwz.c | 2 +- 5 files changed, 112 insertions(+), 82 deletions(-) (limited to 'firmware/target/hosted') diff --git a/firmware/target/hosted/alsa-controls.c b/firmware/target/hosted/alsa-controls.c index f4914aa216..8d8c46decd 100644 --- a/firmware/target/hosted/alsa-controls.c +++ b/firmware/target/hosted/alsa-controls.c @@ -19,33 +19,108 @@ ****************************************************************************/ #include "alsa-controls.h" #include "panic.h" +#include "debug.h" #include /* alsa control handle, we keep it open at all times */ static snd_ctl_t *alsa_ctl; /* list of all controls, we allocate and fill it once, so we can easily lookup */ static snd_ctl_elem_list_t *alsa_ctl_list; +static unsigned alsa_ctl_count; +/* for each control, we store some info to avoid constantly using ALSA's horrible lookup functions */ +static struct ctl_t +{ + snd_ctl_elem_id_t *id; /* ID associated with the control */ + const char *name; /* name of the control */ + snd_ctl_elem_type_t type; /* control type (boolean, enum, ...) */ + unsigned count; /* dimension of the control */ + unsigned items; /* number of items (for enums) */ + const char **enum_name; /* names of the enum, indexed by ALSA index */ +} *alsa_ctl_info; -void alsa_controls_init(void) +#if defined(DEBUG) +static const char *alsa_ctl_type_name(snd_ctl_elem_type_t type) +{ + switch(type) + { + case SND_CTL_ELEM_TYPE_BOOLEAN: return "BOOLEAN"; + case SND_CTL_ELEM_TYPE_INTEGER: return "INTEGER"; + case SND_CTL_ELEM_TYPE_ENUMERATED: return "ENUMERATED"; + default: return "???"; + } +} +#endif + +void alsa_controls_init(const char *name) { - snd_ctl_elem_info_t *info; /* allocate list on heap because it is used it is used in other functions */ snd_ctl_elem_list_malloc(&alsa_ctl_list); - /* allocate info on stack so there is no need to free them */ - snd_ctl_elem_info_alloca(&info); /* there is only one card and "default" always works */ - if(snd_ctl_open(&alsa_ctl, "default", 0) < 0) + if(snd_ctl_open(&alsa_ctl, name, 0) < 0) panicf("Cannot open ctl\n"); /* ALSA is braindead: first "get" the list -> only retrieve count */ if(snd_ctl_elem_list(alsa_ctl, alsa_ctl_list) < 0) panicf("Cannot get element list\n"); /* now we can allocate the list since the know the size */ - int count = snd_ctl_elem_list_get_count(alsa_ctl_list); - if(snd_ctl_elem_list_alloc_space(alsa_ctl_list, count) < 0) + alsa_ctl_count = snd_ctl_elem_list_get_count(alsa_ctl_list); + if(snd_ctl_elem_list_alloc_space(alsa_ctl_list, alsa_ctl_count) < 0) panicf("Cannot allocate space for element list\n"); /* ... and get the list again! */ if(snd_ctl_elem_list(alsa_ctl, alsa_ctl_list) < 0) panicf("Cannot get element list\n"); + /* now cache the info */ + alsa_ctl_info = malloc(sizeof(struct ctl_t) * alsa_ctl_count); + snd_ctl_elem_info_t *info; + snd_ctl_elem_info_malloc(&info); + for(unsigned i = 0; i < alsa_ctl_count; i++) + { + /* allocate the ID and retrieve it */ + snd_ctl_elem_id_malloc(&alsa_ctl_info[i].id); + snd_ctl_elem_list_get_id(alsa_ctl_list, i, alsa_ctl_info[i].id); + /* NOTE: name is valid as long as the ID lives; since we keep both, no need to copy */ + alsa_ctl_info[i].name = snd_ctl_elem_id_get_name(alsa_ctl_info[i].id); + /* get info */ + snd_ctl_elem_info_set_id(info, alsa_ctl_info[i].id); + if(snd_ctl_elem_info(alsa_ctl, info) < 0) + panicf("Cannot get control '%s' info", alsa_ctl_info[i].name); + alsa_ctl_info[i].type = snd_ctl_elem_info_get_type(info); + alsa_ctl_info[i].count = snd_ctl_elem_info_get_count(info); + if(alsa_ctl_info[i].type == SND_CTL_ELEM_TYPE_ENUMERATED) + { + alsa_ctl_info[i].items = snd_ctl_elem_info_get_items(info); + alsa_ctl_info[i].enum_name = malloc(sizeof(char *) * alsa_ctl_info[i].items); + for(unsigned j = 0; j < alsa_ctl_info[i].items; j++) + { + snd_ctl_elem_info_set_item(info, j); + if(snd_ctl_elem_info(alsa_ctl, info) < 0) + panicf("Cannot get control '%s' info for item %u", alsa_ctl_info[i].name, j); + /* NOTE: copy string because it becomes invalid after info is freed */ + alsa_ctl_info[i].enum_name[j] = strdup(snd_ctl_elem_info_get_item_name(info)); + } + } + else + alsa_ctl_info[i].items = 0; + } + snd_ctl_elem_info_free(info); + + DEBUGF("ALSA controls:\n"); + for(unsigned i = 0; i < alsa_ctl_count; i++) + { + DEBUGF("- name='%s', type=%s, count=%u\n", alsa_ctl_info[i].name, + alsa_ctl_type_name(alsa_ctl_info[i].type), alsa_ctl_info[i].count); + if(alsa_ctl_info[i].type == SND_CTL_ELEM_TYPE_ENUMERATED) + { + DEBUGF(" items:"); + for(unsigned j = 0; j < alsa_ctl_info[i].items; j++) + { + if(j != 0) + DEBUGF(","); + DEBUGF(" '%s'", alsa_ctl_info[i].enum_name[j]); + } + DEBUGF("\n"); + } + } + } void alsa_controls_close(void) @@ -53,56 +128,33 @@ void alsa_controls_close(void) snd_ctl_close(alsa_ctl); } -/* find a control element ID by name, return false of not found, the id needs - * to be allocated */ -bool alsa_controls_find(snd_ctl_elem_id_t *id, const char *name) +/* find a control element ID by name, return -1 of not found or index into array */ +int alsa_controls_find(const char *name) { - /* ALSA identifies controls by "id"s, it almost makes sense, except ids - * are a horrible opaque structure */ - int count = snd_ctl_elem_list_get_count(alsa_ctl_list); /* enumerate controls */ - for(int i = 0; i < count; i++) - { - snd_ctl_elem_list_get_id(alsa_ctl_list, i, id); - - if(strcmp(snd_ctl_elem_id_get_name(id), name) == 0) - return true; - } + for(unsigned i = 0; i < alsa_ctl_count; i++) + if(strcmp(alsa_ctl_info[i].name, name) == 0) + return i; /* not found */ - return false; + return -1; } bool alsa_has_control(const char *name) { - snd_ctl_elem_id_t *id; - /* allocate things on stack */ - snd_ctl_elem_id_alloca(&id); - /* find control */ - return alsa_controls_find(id, name); + return alsa_controls_find(name) >= 0; } /* find a control element enum index by name, return -1 if not found */ int alsa_controls_find_enum(const char *name, const char *enum_name) { - snd_ctl_elem_id_t *id; - snd_ctl_elem_info_t *info; - /* allocate things on stack to speedup */ - snd_ctl_elem_id_alloca(&id); - snd_ctl_elem_info_alloca(&info); /* find control */ - if(!alsa_controls_find(id, name)) + int idx = alsa_controls_find(name); + if(idx < 0) panicf("Cannot find control '%s'", name); - snd_ctl_elem_info_set_id(info, id); - if(snd_ctl_elem_info(alsa_ctl, info) < 0) - panicf("Cannot get control '%s' info", name); /* list items */ - unsigned count = snd_ctl_elem_info_get_items(info); - for(unsigned i = 0; i < count; i++) + for(unsigned i = 0; i < alsa_ctl_info[idx].items; i++) { - snd_ctl_elem_info_set_item(info, i); - if(snd_ctl_elem_info(alsa_ctl, info) < 0) - panicf("Cannot get control '%s' info for item %u", name, i); - if(strcmp(snd_ctl_elem_info_get_item_name(info), enum_name) == 0) + if(strcmp(alsa_ctl_info[idx].enum_name[i], enum_name) == 0) return i; } return -1; @@ -112,27 +164,21 @@ int alsa_controls_find_enum(const char *name, const char *enum_name) void alsa_controls_get_set(bool get, const char *name, snd_ctl_elem_type_t type, unsigned nr_values, long *val) { - snd_ctl_elem_id_t *id; - snd_ctl_elem_info_t *info; snd_ctl_elem_value_t *value; /* allocate things on stack to speedup */ - snd_ctl_elem_id_alloca(&id); - snd_ctl_elem_info_alloca(&info); snd_ctl_elem_value_alloca(&value); /* find control */ - if(!alsa_controls_find(id, name)) + int idx = alsa_controls_find(name); + if(idx < 0) panicf("Cannot find control '%s'", name); /* check the type of the control */ - snd_ctl_elem_info_set_id(info, id); - if(snd_ctl_elem_info(alsa_ctl, info) < 0) - panicf("Cannot get control '%s' info", name); - if(snd_ctl_elem_info_get_type(info) != type) + if(alsa_ctl_info[idx].type != type) panicf("Control '%s' has wrong type (got %d, expected %d", name, - snd_ctl_elem_info_get_type(info), type); - if(snd_ctl_elem_info_get_count(info) != nr_values) + alsa_ctl_info[idx].type, type); + if(alsa_ctl_info[idx].count != nr_values) panicf("Control '%s' has wrong count (got %u, expected %u)", - name, snd_ctl_elem_info_get_count(info), nr_values); - snd_ctl_elem_value_set_id(value, id); + name, alsa_ctl_info[idx].count, nr_values); + snd_ctl_elem_value_set_id(value, alsa_ctl_info[idx].id); /* set value */ if(get) { @@ -185,19 +231,12 @@ void alsa_controls_get(const char *name, snd_ctl_elem_type_t type, /* get control information */ void alsa_controls_get_info(const char *name, int *out_count) { - snd_ctl_elem_id_t *id; - snd_ctl_elem_info_t *info; - /* allocate things on stack to speedup */ - snd_ctl_elem_id_alloca(&id); - snd_ctl_elem_info_alloca(&info); /* find control */ - if(!alsa_controls_find(id, name)) + int idx = alsa_controls_find(name); + if(idx < 0) panicf("Cannot find control '%s'", name); /* get info */ - snd_ctl_elem_info_set_id(info, id); - if(snd_ctl_elem_info(alsa_ctl, info) < 0) - panicf("Cannot get control '%s' info", name); - *out_count = snd_ctl_elem_info_get_count(info); + *out_count = alsa_ctl_info[idx].count; } /* helper function: set a control with a single boolean value */ diff --git a/firmware/target/hosted/alsa-controls.h b/firmware/target/hosted/alsa-controls.h index 0505ee3dd1..af3e584cd9 100644 --- a/firmware/target/hosted/alsa-controls.h +++ b/firmware/target/hosted/alsa-controls.h @@ -27,16 +27,13 @@ #include /* open alsa control interface and list all controls, keep control open */ -void alsa_controls_init(void); +void alsa_controls_init(const char *name); /* close alsa controls */ void alsa_controls_close(void); /* NOTE: all the following functions panic on error. This behaviour could be changed with the * functions returning proper values but that would make errors happen silently */ -/* find a control element ID by name, return false of not found, the id needs - * to be allocated */ -bool alsa_controls_find(snd_ctl_elem_id_t *id, const char *name); /* check wether a control exists */ bool alsa_has_control(const char *name); /* find a control element enum index by name, return -1 if not found */ diff --git a/firmware/target/hosted/pcm-alsa.c b/firmware/target/hosted/pcm-alsa.c index 939a0cabb5..970abc78cf 100644 --- a/firmware/target/hosted/pcm-alsa.c +++ b/firmware/target/hosted/pcm-alsa.c @@ -66,10 +66,10 @@ static const snd_pcm_access_t access_ = SND_PCM_ACCESS_RW_INTERLEAVED; /* access #if defined(SONY_NWZ_LINUX) || defined(HAVE_FIIO_LINUX_CODEC) /* Sony NWZ must use 32-bit per sample */ static const snd_pcm_format_t format = SND_PCM_FORMAT_S32_LE; /* sample format */ -typedef long sample_t; +typedef int32_t sample_t; #else static const snd_pcm_format_t format = SND_PCM_FORMAT_S16; /* sample format */ -typedef short sample_t; +typedef int16_t sample_t; #endif static const int channels = 2; /* count of channels */ static unsigned int real_sample_rate; @@ -260,10 +260,9 @@ error: * and add 48dB to the input volume. We cannot go lower -43dB because several * values between -48dB and -43dB would require a fractional multiplier, which is * stupid to implement for such very low volume. */ -static int dig_vol_mult_l = 2 ^ 16; /* multiplicative factor to apply to each sample */ -static int dig_vol_mult_r = 2 ^ 16; /* multiplicative factor to apply to each sample */ - -void pcm_alsa_set_digital_volume(int vol_db_l, int vol_db_r) +static int dig_vol_mult_l = 2 << 16; /* multiplicative factor to apply to each sample */ +static int dig_vol_mult_r = 2 << 16; /* multiplicative factor to apply to each sample */ +void pcm_set_mixer_volume(int vol_db_l, int vol_db_r) { if(vol_db_l > 0 || vol_db_r > 0 || vol_db_l < -43 || vol_db_r < -43) panicf("invalid pcm alsa volume"); @@ -336,7 +335,7 @@ static bool copy_frames(bool first) { /* We have to convert 16-bit to 32-bit, the need to multiply the * sample by some value so the sound is not too low */ - const short *pcm_ptr = pcm_data; + const int16_t *pcm_ptr = pcm_data; sample_t *sample_ptr = &frames[2*(period_size-frames_left)]; for (int i = 0; i < nframes; i++) { @@ -757,11 +756,6 @@ void pcm_play_dma_postinit(void) #endif } -void pcm_set_mixer_volume(int volume) -{ - (void)volume; -} - int pcm_alsa_get_rate(void) { return real_sample_rate; diff --git a/firmware/target/hosted/pcm-alsa.h b/firmware/target/hosted/pcm-alsa.h index f10574a6da..1392593c0c 100644 --- a/firmware/target/hosted/pcm-alsa.h +++ b/firmware/target/hosted/pcm-alsa.h @@ -25,7 +25,7 @@ #if defined(SONY_NWZ_LINUX) || defined(HAVE_FIIO_LINUX_CODEC) /* Set the PCM volume in dB: each sample with have this volume applied digitally * before being sent to ALSA. Volume must satisfy -43 <= dB <= 0 */ -void pcm_alsa_set_digital_volume(int vol_db_l, int vol_db_r); +void pcm_set_mixer_volume(int vol_db_l, int vol_db_r); #endif /* These two should be invoked in your audiohw_preinit() call! */ diff --git a/firmware/target/hosted/sonynwz/debug-nwz.c b/firmware/target/hosted/sonynwz/debug-nwz.c index c9a430e7c2..859eddb47b 100644 --- a/firmware/target/hosted/sonynwz/debug-nwz.c +++ b/firmware/target/hosted/sonynwz/debug-nwz.c @@ -369,7 +369,7 @@ bool dbg_hw_info_audio(void) { case VOL: vol += inc ? 1 : -1; - pcm_alsa_set_digital_volume(vol, vol); + pcm_set_mixer_volume(vol, vol); break; case ACOUSTIC: audiohw_enable_acoustic(!audiohw_acoustic_enabled()); -- cgit