[alsa-devel] [PATCH v2 0/3] alsa-lib: Improve 2.1 surround support
Thanks Takashi for your encouraging review,
Diff since last version is only the things you remarked about in the first patch series.
Besides that, I find it also OK to just return the channel (>= 0) or a negative error. But it's no big matter in which form is.
This one I didn't change, just because then I would just have had to change it back in the second patch anyway.
David Henningsson (3): route: Allow chmap syntax for slave channels in ttable route: Select slave chmap based on ttable information conf: Allow 2.1 surround to use different number of channels
src/conf/pcm/surround21.conf | 7 +- src/pcm/pcm_route.c | 323 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 290 insertions(+), 40 deletions(-)
Instead of writing e g "0" and "1", one can now write "FL" and "FR" instead.
E g: ttable.0.FL 1 ttable.1.FR 1 ttable.2.LFE 1
Signed-off-by: David Henningsson david.henningsson@canonical.com --- src/pcm/pcm_route.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/pcm/pcm_route.c b/src/pcm/pcm_route.c index 2beedf6..56318d4 100644 --- a/src/pcm/pcm_route.c +++ b/src/pcm/pcm_route.c @@ -789,6 +789,24 @@ static void snd_pcm_route_dump(snd_pcm_t *pcm, snd_output_t *out) snd_pcm_dump(route->plug.gen.slave, out); }
+static int strtochannel(const char *id, long *channel) +{ + int err; + int ch; + err = safe_strtol(id, channel); + if (err >= 0) + return err; + + ch = (int) snd_pcm_chmap_from_string(id); + if (ch == -1) + return -EINVAL; + + /* For now, assume standard channel mapping */ + *channel = ch - SND_CHMAP_FL; + return 0; +} + + static const snd_pcm_ops_t snd_pcm_route_ops = { .close = snd_pcm_route_close, .info = snd_pcm_generic_info, @@ -983,7 +1001,7 @@ int snd_pcm_route_determine_ttable(snd_config_t *tt, const char *id; if (snd_config_get_id(jnode, &id) < 0) continue; - err = safe_strtol(id, &schannel); + err = strtochannel(id, &schannel); if (err < 0) { SNDERR("Invalid slave channel: %s", id); return -EINVAL; @@ -1046,7 +1064,7 @@ int snd_pcm_route_load_ttable(snd_config_t *tt, snd_pcm_route_ttable_entry_t *tt const char *id; if (snd_config_get_id(jnode, &id) < 0) continue; - err = safe_strtol(id, &schannel); + err = strtochannel(id, &schannel); if (err < 0 || schannel < 0 || (unsigned int) schannel > tt_ssize || (schannels > 0 && schannel >= schannels)) {
It means we need to initialize this order:
1) Read the ttable to figure out which channels are present 2) Open slave pcm and find a matching chmap 3) Determine size of ttable (this can now depend on the chmap) 4) Read ttable coefficients 5) At prepare time, select the matching chmap
Signed-off-by: David Henningsson david.henningsson@canonical.com --- src/pcm/pcm_route.c | 319 ++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 276 insertions(+), 43 deletions(-)
diff --git a/src/pcm/pcm_route.c b/src/pcm/pcm_route.c index 56318d4..ab17fa7 100644 --- a/src/pcm/pcm_route.c +++ b/src/pcm/pcm_route.c @@ -103,6 +103,7 @@ typedef struct { snd_pcm_format_t sformat; int schannels; snd_pcm_route_params_t params; + snd_pcm_chmap_t *chmap; } snd_pcm_route_t;
#endif /* DOC_HIDDEN */ @@ -518,6 +519,7 @@ static int snd_pcm_route_close(snd_pcm_t *pcm) } free(params->dsts); } + free(route->chmap); return snd_pcm_generic_close(pcm); }
@@ -789,21 +791,187 @@ static void snd_pcm_route_dump(snd_pcm_t *pcm, snd_output_t *out) snd_pcm_dump(route->plug.gen.slave, out); }
-static int strtochannel(const char *id, long *channel) +/* + * Converts a string to an array of channel indices: + * - Given a number, the result is an array with one element, + * containing that number + * - Given a channel name (e g "FL") and a chmap, + * it will look this up in the chmap and return all matches + * - Given a channel name and no chmap, the result is an array with one element, + containing alsa standard channel map. Note that this might be a negative + number in case of "UNKNOWN", "NA" or "MONO". + * Return value is number of matches written. + */ +static int strtochannel(const char *id, snd_pcm_chmap_t *chmap, + long *channel, int channel_size) { - int err; int ch; - err = safe_strtol(id, channel); - if (err >= 0) - return err; + if (safe_strtol(id, channel) >= 0) + return 1;
ch = (int) snd_pcm_chmap_from_string(id); if (ch == -1) return -EINVAL;
- /* For now, assume standard channel mapping */ - *channel = ch - SND_CHMAP_FL; + if (chmap) { + int i, r = 0; + /* Start with highest channel to simplify implementation of + determine ttable size */ + for (i = chmap->channels - 1; i >= 0; i--) { + if ((int) chmap->pos[i] != ch) + continue; + if (r >= channel_size) + continue; + channel[r++] = i; + } + return r; + } + else { + /* Assume ALSA standard channel mapping */ + *channel = ch - SND_CHMAP_FL; + return 1; + } +} + +#define MAX_CHMAP_CHANNELS 256 + +static int determine_chmap(snd_config_t *tt, snd_pcm_chmap_t **tt_chmap) +{ + snd_config_iterator_t i, inext; + snd_pcm_chmap_t *chmap; + + assert(tt && tt_chmap); + chmap = malloc(sizeof(snd_pcm_chmap_t) + + MAX_CHMAP_CHANNELS * sizeof(unsigned int)); + + chmap->channels = 0; + snd_config_for_each(i, inext, tt) { + const char *id; + snd_config_iterator_t j, jnext; + snd_config_t *in = snd_config_iterator_entry(i); + + if (!snd_config_get_id(in, &id) < 0) + continue; + if (snd_config_get_type(in) != SND_CONFIG_TYPE_COMPOUND) + goto err; + snd_config_for_each(j, jnext, in) { + int ch, k, found; + long schannel; + snd_config_t *jnode = snd_config_iterator_entry(j); + if (snd_config_get_id(jnode, &id) < 0) + continue; + if (safe_strtol(id, &schannel) >= 0) + continue; + ch = (int) snd_pcm_chmap_from_string(id); + if (ch == -1) + goto err; + + found = 0; + for (k = 0; k < (int) chmap->channels; k++) + if (ch == (int) chmap->pos[k]) { + found = 1; + break; + } + if (found) + continue; + + if (chmap->channels >= MAX_CHMAP_CHANNELS) { + SNDERR("Too many channels in ttable chmap"); + goto err; + } + chmap->pos[chmap->channels++] = ch; + } + } + + + *tt_chmap = chmap; return 0; + +err: + *tt_chmap = NULL; + free(chmap); + return -EINVAL; +} + +static int find_matching_chmap(snd_pcm_t *spcm, snd_pcm_chmap_t *tt_chmap, + snd_pcm_chmap_t **found_chmap, int *schannels) +{ + snd_pcm_chmap_query_t** chmaps = snd_pcm_query_chmaps(spcm); + int i; + + *found_chmap = NULL; + + if (chmaps == NULL) + return 0; /* chmap API not supported for this slave */ + + for (i = 0; chmaps[i]; i++) { + unsigned int j, k; + int match = 1; + snd_pcm_chmap_t *c = &chmaps[i]->map; + if (*schannels >= 0 && (int) c->channels != *schannels) + continue; + + for (j = 0; j < tt_chmap->channels; j++) { + int found = 0; + unsigned int ch = tt_chmap->pos[j]; + for (k = 0; k < c->channels; k++) + if (c->pos[k] == ch) { + found = 1; + break; + } + if (!found) { + match = 0; + break; + } + } + + if (match) { + int size = sizeof(snd_pcm_chmap_t) + c->channels * sizeof(unsigned int); + *found_chmap = malloc(size); + if (!*found_chmap) { + snd_pcm_free_chmaps(chmaps); + return -ENOMEM; + } + memcpy(*found_chmap, c, size); + *schannels = c->channels; + break; + } + } + + snd_pcm_free_chmaps(chmaps); + + if (*found_chmap == NULL) { + SNDERR("Found no matching channel map"); + return -EINVAL; + } + return 0; +} + +static int route_chmap_init(snd_pcm_t *pcm) +{ + int set_map = 0; + snd_pcm_chmap_t *current; + snd_pcm_route_t *route = pcm->private_data; + if (!route->chmap) + return 0; + if (snd_pcm_state(pcm) != SND_PCM_STATE_PREPARED) + return 0; + + /* Check if we really need to set the chmap or not. + This is important in case set_chmap is not implemented. */ + current = snd_pcm_get_chmap(route->plug.gen.slave); + if (!current) + return -ENOSYS; + if (current->channels != route->chmap->channels) + set_map = 1; + else + set_map = memcmp(current->pos, route->chmap->pos, + current->channels); + free(current); + if (!set_map) + return 0; + + return snd_pcm_set_chmap(route->plug.gen.slave, route->chmap); }
@@ -939,6 +1107,7 @@ int snd_pcm_route_open(snd_pcm_t **pcmp, const char *name, route->plug.undo_write = snd_pcm_plugin_undo_write_generic; route->plug.gen.slave = slave; route->plug.gen.close_slave = close_slave; + route->plug.init = route_chmap_init;
err = snd_pcm_new(&pcm, SND_PCM_TYPE_ROUTE, name, slave->stream, slave->mode); if (err < 0) { @@ -963,16 +1132,10 @@ int snd_pcm_route_open(snd_pcm_t **pcmp, const char *name, return 0; }
-/** - * \brief Determine route matrix sizes - * \param tt Configuration root describing route matrix - * \param tt_csize Returned client size in elements - * \param tt_ssize Returned slave size in elements - * \retval zero on success otherwise a negative error code - */ -int snd_pcm_route_determine_ttable(snd_config_t *tt, - unsigned int *tt_csize, - unsigned int *tt_ssize) +static int _snd_pcm_route_determine_ttable(snd_config_t *tt, + unsigned int *tt_csize, + unsigned int *tt_ssize, + snd_pcm_chmap_t *chmap) { snd_config_iterator_t i, inext; long csize = 0, ssize = 0; @@ -1001,7 +1164,7 @@ int snd_pcm_route_determine_ttable(snd_config_t *tt, const char *id; if (snd_config_get_id(jnode, &id) < 0) continue; - err = strtochannel(id, &schannel); + err = strtochannel(id, chmap, &schannel, 1); if (err < 0) { SNDERR("Invalid slave channel: %s", id); return -EINVAL; @@ -1020,6 +1183,20 @@ int snd_pcm_route_determine_ttable(snd_config_t *tt, }
/** + * \brief Determine route matrix sizes + * \param tt Configuration root describing route matrix + * \param tt_csize Returned client size in elements + * \param tt_ssize Returned slave size in elements + * \retval zero on success otherwise a negative error code + */ +int snd_pcm_route_determine_ttable(snd_config_t *tt, + unsigned int *tt_csize, + unsigned int *tt_ssize) +{ + return _snd_pcm_route_determine_ttable(tt, tt_csize, tt_ssize, NULL); +} + +/** * \brief Load route matrix * \param tt Configuration root describing route matrix * \param ttable Returned route matrix @@ -1030,10 +1207,10 @@ int snd_pcm_route_determine_ttable(snd_config_t *tt, * \param schannels Slave channels * \retval zero on success otherwise a negative error code */ -int snd_pcm_route_load_ttable(snd_config_t *tt, snd_pcm_route_ttable_entry_t *ttable, - unsigned int tt_csize, unsigned int tt_ssize, - unsigned int *tt_cused, unsigned int *tt_sused, - int schannels) +static int _snd_pcm_route_load_ttable(snd_config_t *tt, snd_pcm_route_ttable_entry_t *ttable, + unsigned int tt_csize, unsigned int tt_ssize, + unsigned int *tt_cused, unsigned int *tt_sused, + int schannels, snd_pcm_chmap_t *chmap) { int cused = -1; int sused = -1; @@ -1060,17 +1237,18 @@ int snd_pcm_route_load_ttable(snd_config_t *tt, snd_pcm_route_ttable_entry_t *tt snd_config_for_each(j, jnext, in) { snd_config_t *jnode = snd_config_iterator_entry(j); double value; - long schannel; + int ss; + long *scha = alloca(tt_ssize * sizeof(long)); const char *id; if (snd_config_get_id(jnode, &id) < 0) continue; - err = strtochannel(id, &schannel); - if (err < 0 || - schannel < 0 || (unsigned int) schannel > tt_ssize || - (schannels > 0 && schannel >= schannels)) { + + ss = strtochannel(id, chmap, scha, tt_ssize); + if (ss < 0) { SNDERR("Invalid slave channel: %s", id); return -EINVAL; } + err = snd_config_get_real(jnode, &value); if (err < 0) { long v; @@ -1081,9 +1259,18 @@ int snd_pcm_route_load_ttable(snd_config_t *tt, snd_pcm_route_ttable_entry_t *tt } value = v; } - ttable[cchannel * tt_ssize + schannel] = value; - if (schannel > sused) - sused = schannel; + + for (k = 0; (int) k < ss; k++) { + long schannel = scha[k]; + if (schannel < 0 || (unsigned int) schannel > tt_ssize || + (schannels > 0 && schannel >= schannels)) { + SNDERR("Invalid slave channel: %s", id); + return -EINVAL; + } + ttable[cchannel * tt_ssize + schannel] = value; + if (schannel > sused) + sused = schannel; + } } if (cchannel > cused) cused = cchannel; @@ -1093,6 +1280,26 @@ int snd_pcm_route_load_ttable(snd_config_t *tt, snd_pcm_route_ttable_entry_t *tt return 0; }
+/** + * \brief Load route matrix + * \param tt Configuration root describing route matrix + * \param ttable Returned route matrix + * \param tt_csize Client size in elements + * \param tt_ssize Slave size in elements + * \param tt_cused Used client elements + * \param tt_sused Used slave elements + * \param schannels Slave channels + * \retval zero on success otherwise a negative error code + */ +int snd_pcm_route_load_ttable(snd_config_t *tt, snd_pcm_route_ttable_entry_t *ttable, + unsigned int tt_csize, unsigned int tt_ssize, + unsigned int *tt_cused, unsigned int *tt_sused, + int schannels) +{ + return _snd_pcm_route_load_ttable(tt, ttable, tt_csize, tt_ssize, + tt_cused, tt_sused, schannels, NULL); +} + /*! \page pcm_plugins
\section pcm_plugins_route Plugin: Route & Volume @@ -1100,6 +1307,9 @@ int snd_pcm_route_load_ttable(snd_config_t *tt, snd_pcm_route_ttable_entry_t *tt This plugin converts channels and applies volume during the conversion. The format and rate must match for both of them.
+SCHANNEL can be a channel name instead of a number (e g FL, LFE). +If so, a matching channel map will be selected for the slave. + \code pcm.name { type route # Route & Volume conversion PCM @@ -1150,6 +1360,7 @@ int _snd_pcm_route_open(snd_pcm_t **pcmp, const char *name, int err; snd_pcm_t *spcm; snd_config_t *slave = NULL, *sconf; + snd_pcm_chmap_t *tt_chmap, *chmap; snd_pcm_format_t sformat = SND_PCM_FORMAT_UNKNOWN; int schannels = -1; snd_config_t *tt = NULL; @@ -1198,37 +1409,59 @@ int _snd_pcm_route_open(snd_pcm_t **pcmp, const char *name, return -EINVAL; }
- err = snd_pcm_route_determine_ttable(tt, &csize, &ssize); + err = determine_chmap(tt, &tt_chmap); if (err < 0) { - snd_config_delete(sconf); + free(ttable); return err; } - ttable = malloc(csize * ssize * sizeof(snd_pcm_route_ttable_entry_t)); - if (ttable == NULL) { - snd_config_delete(sconf); - return -ENOMEM; - } - err = snd_pcm_route_load_ttable(tt, ttable, csize, ssize, - &cused, &sused, schannels); + + err = snd_pcm_open_slave(&spcm, root, sconf, stream, mode, conf); + snd_config_delete(sconf); if (err < 0) { + free(tt_chmap); free(ttable); - snd_config_delete(sconf); return err; }
- err = snd_pcm_open_slave(&spcm, root, sconf, stream, mode, conf); - snd_config_delete(sconf); + if (tt_chmap) { + err = find_matching_chmap(spcm, tt_chmap, &chmap, &schannels); + free(tt_chmap); + if (err < 0) + return err; + } + + err = _snd_pcm_route_determine_ttable(tt, &csize, &ssize, chmap); + if (err < 0) { + free(chmap); + snd_pcm_close(spcm); + return err; + } + ttable = malloc(csize * ssize * sizeof(snd_pcm_route_ttable_entry_t)); + if (ttable == NULL) { + free(chmap); + snd_pcm_close(spcm); + return -ENOMEM; + } + err = _snd_pcm_route_load_ttable(tt, ttable, csize, ssize, + &cused, &sused, schannels, chmap); if (err < 0) { + free(chmap); free(ttable); + snd_pcm_close(spcm); return err; } + err = snd_pcm_route_open(pcmp, name, sformat, schannels, ttable, ssize, cused, sused, spcm, 1); free(ttable); - if (err < 0) + if (err < 0) { + free(chmap); snd_pcm_close(spcm); + } + ((snd_pcm_route_t*) (*pcmp)->private_data)->chmap = chmap; + return err; } #ifndef DOC_HIDDEN
2014-02-28 15:57 GMT+08:00 David Henningsson < david.henningsson@canonical.com>:
It means we need to initialize this order:
- Read the ttable to figure out which channels are present
- Open slave pcm and find a matching chmap
- Determine size of ttable (this can now depend on the chmap)
- Read ttable coefficients
- At prepare time, select the matching chmap
Do this patch assume the slave.pcm of the route plugin must have a channel map ?
are there any side effect when some of sound cards using route but does not have channel map ?
e.g. ice1712,
playback.pcm { type hooks slave.pcm { type route ttable.0.8 1 ttable.1.9 1 slave.pcm { type hw card $CARD } slave.format S32_LE slave.channels 10 }
rme9652, ....
slave.pcm { type route slave { pcm { type hw card $CARD } channels 26 } ttable.0.24 1 ttable.1.25 1
On 02/28/2014 02:49 PM, Raymond Yau wrote:
2014-02-28 15:57 GMT+08:00 David Henningsson <david.henningsson@canonical.com mailto:david.henningsson@canonical.com>:
It means we need to initialize this order: 1) Read the ttable to figure out which channels are present 2) Open slave pcm and find a matching chmap 3) Determine size of ttable (this can now depend on the chmap) 4) Read ttable coefficients 5) At prepare time, select the matching chmap
Do this patch assume the slave.pcm of the route plugin must have a channel map ?
No. In that case, it will fall back to ALSA's standard channel map ( FL -> 0, FR -> 1, etc).
are there any side effect when some of sound cards using route but does not have channel map ?
If there is no channel map, things should remain unchanged.
e.g. ice1712,
playback.pcm { type hooks slave.pcm { type route ttable.0.8 1 ttable.1.9 1 slave.pcm { type hw card $CARD } slave.format S32_LE slave.channels 10 }
rme9652, ....
slave.pcm { type route slave { pcm { type hw card $CARD } channels 26 } ttable.0.24 1 ttable.1.25 1
This way, cards that support LFE on four channels (e g laptop with internal subwoofer) can do that, and other cards on a six channel setup can use that as well.
Well, note that there is still a reference to "pcm.surround51" left here. In practice, for HDA Intel sound cards this does not matter as both surround51 and surround40 reference the same definition. (And that's the only card I currently know of that actually does surround2.1 over four channels.)
Signed-off-by: David Henningsson david.henningsson@canonical.com --- src/conf/pcm/surround21.conf | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/conf/pcm/surround21.conf b/src/conf/pcm/surround21.conf index be29020..7f4676b 100644 --- a/src/conf/pcm/surround21.conf +++ b/src/conf/pcm/surround21.conf @@ -51,10 +51,9 @@ pcm.!surround21 { ] } } - slave.channels 6 - ttable.0.0 1 - ttable.1.1 1 - ttable.2.5 1 + ttable.0.FL 1 + ttable.1.FR 1 + ttable.2.LFE 1 hint { description "2.1 Surround output to Front and Subwoofer speakers" device $DEV
At Fri, 28 Feb 2014 08:57:04 +0100, David Henningsson wrote:
Thanks Takashi for your encouraging review,
Diff since last version is only the things you remarked about in the first patch series.
Besides that, I find it also OK to just return the channel (>= 0) or a negative error. But it's no big matter in which form is.
This one I didn't change, just because then I would just have had to change it back in the second patch anyway.
OK, applied all patches now. Thanks!
Takashi
David Henningsson (3): route: Allow chmap syntax for slave channels in ttable route: Select slave chmap based on ttable information conf: Allow 2.1 surround to use different number of channels
src/conf/pcm/surround21.conf | 7 +- src/pcm/pcm_route.c | 323 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 290 insertions(+), 40 deletions(-)
-- 1.7.9.5
participants (3)
-
David Henningsson
-
Raymond Yau
-
Takashi Iwai