[alsa-devel] [RFC] Add scenario management
Hello.
This patchset add support for scenario management, aka use cases, in alsa-lib and example programs in alsa-utils. It allows to adjust the current audio kcontrol settings from a prespective of a changed use case like the switch from listening to music on your phone to an incoming phone call and back.
To achieve this it offers control aliasing for playback, capture master and switch as well as the option to post- and prefix a sequence of control changes avoiding pops and other unwanted noise.
This is of course not meant to replace gstreamer, PulseAudio or a sound server, but is meant to work in tandem with such audio software. We think alsa-lib is the best place for this as it will be used on small embedded system which only use alsa from the full stack of audio libs in linux. We see potential for desktop usage as well though.
There is also the support of QoS and the submission to salsa on the TODO list, but we think it is best to gather soem feedback and get the core merged before drifting away with to much other features.
Please let me know what can be improved.
Project page is here :-
http://www.slimlogic.co.uk/?p=40
regards Stefan Schmidt
From: Stefan Schmidt stefan@slimlogic.co.uk
It allows switching audio settings between scenarios or uses-cases like listening to music and answering an incoming phone call. Made of control aliasing for playback, capture master and switch as well as the option to post- and prefix a sequence of control changes avoiding pops and other unwanted noise. Some example programs will be available in alsa-utils.
CC: Ian Molton ian@mnementh.co.uk CC: Graeme Gregory gg@slimlogic.co.uk Signed-off-by: Liam Girdwood lrg@slimlogic.co.uk Signed-off-by: Stefan Schmidt stefan@slimlogic.co.uk --- include/Makefile.am | 2 +- include/ascenario.h | 167 +++++++ src/Makefile.am | 2 +- src/ascenario.c | 1357 +++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 1526 insertions(+), 2 deletions(-) create mode 100644 include/ascenario.h create mode 100644 src/ascenario.c
diff --git a/include/Makefile.am b/include/Makefile.am index a291503..572fbc9 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -3,7 +3,7 @@ SUBDIRS = sound sysincludedir = ${includedir}/sys alsaincludedir = ${includedir}/alsa
-alsainclude_HEADERS = asoundlib.h asoundef.h \ +alsainclude_HEADERS = asoundlib.h asoundef.h ascenario.h \ version.h global.h input.h output.h error.h \ conf.h control.h iatomic.h
diff --git a/include/ascenario.h b/include/ascenario.h new file mode 100644 index 0000000..49f1c48 --- /dev/null +++ b/include/ascenario.h @@ -0,0 +1,167 @@ +/* +* ALSA Scenario header file +* +* This library is free software; you can redistribute it and/or modify +* it under the terms of the GNU Lesser General Public License as +* published by the Free Software Foundation; either version 2.1 of +* the License, or (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU Lesser General Public License for more details. +* +* You should have received a copy of the GNU Lesser General Public +* License along with this library; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +* +* Copyright (C) 2008-2009 SlimLogic Ltd +* Authors: Liam Girdwood lrg@slimlogic.co.uk +* Stefan Schmidt stefan@slimlogic.co.uk +*/ + +/** Scenario ID's + * + * Standard Scenario ID's - Add new scenarios at the end. + */ + +#define SND_SCN_PLAYBACK_SPEAKER "playback speaker" +#define SND_SCN_PLAYBACK_HEADPHONES "playback headphone" +#define SND_SCN_PLAYBACK_HEADSET "playback headset" +#define SND_SCN_PLAYBACK_BLUETOOTH "playback bluetooth" +#define SND_SCN_PLAYBACK_HANDSET "playback handset" +#define SND_SCN_PLAYBACK_GSM "playback gsm" +#define SND_SCN_PLAYBACK_LINE "playback line" + +#define SND_SCN_CAPTURE_MIC "capture mic" +#define SND_SCN_CAPTURE_LINE "capture line" +#define SND_SCN_CAPTURE_HEADSET "capture headset" +#define SND_SCN_CAPTURE_HANDSET "capture handset" +#define SND_SCN_CAPTURE_BLUETOOTH "capture bluetooth" +#define SND_SCN_CAPTURE_GSM "capture gsm" + +#define SND_SCN_PHONECALL_HANDSET "phonecall handset" +#define SND_SCN_PHONECALL_HEADSET "phonecall headset" +#define SND_SCN_PHONECALL_BLUETOOTH "phonecall bluetooth" +#define SND_SCN_PHONECALL_IP "phonecall ip" + +/** QOS + * + * Defines Audio Quality of Service + */ +#define SND_QOS_HIFI 0 +#define SND_QOS_VOICE 1 +#define SND_QOS_SYSTEM 2 + +struct snd_scenario; + + +/* TODO: add notification */ + + +/** + * snd_scenario_open - open scenario core + * @card_name: sound card name. + * + * Open scenario manager core for sound card. + */ +struct snd_scenario *snd_scenario_open(const char *card_name); + +/** + * snd_scenario_reload - reload and reparse scenario configuration + * @scn: scenario + * + * Reloads and reparses sound card scenario configuration. + */ +int snd_scenario_reload(struct snd_scenario *scn); + +/** + * snd_scenario_close - close scenario core + * @scn: scenario + * + * Free scenario manager core for sound card. + */ +void snd_scenario_close(struct snd_scenario *scn); + +/** + * snd_scenario_set_scn - set scenario + * @scn: scenario + * @scenario: scenario name + * + * Set new scenario for sound card. + */ +int snd_scenario_set_scn(struct snd_scenario *scn, const char *scenario); + +/** + * snd_scenario_get_scn - get scenario + * @scn: scenario + * + * Get current sound card scenario. + */ +const char *snd_scenario_get_scn(struct snd_scenario *scn); + +/** + * snd_scenario_list - list supported scenarios + * @scn: scenario + * @list: list of supported scenario names. + * + * List supported scenarios for this sound card. + * Returns number of scenarios. + */ +int snd_scenario_list(struct snd_scenario *scn, const char **list[]); + +/** + * snd_scenario_set_qos - set qos + * @qos: qos + * + * Set Quality of Service for this scenario. + */ +int snd_scenario_set_qos(struct snd_scenario *scn, int qos); + +/** + * snd_scenario_get_qos - get qos + * @scn: scenario + * + * Get Quality of Service for this scenario. + */ +int snd_scenario_get_qos(struct snd_scenario *scn); + +/** + * snd_scenario_get_master_playback_volume - get playback volume + * @scn: scenario + * + * Get the master playback volume control name for the current scenario. + */ +int snd_scenario_get_master_playback_volume(struct snd_scenario *scn); + +/** + * snd_scenario_get_master_playback_switch - get playback switch + * @scn: scenario + * + * Get the master playback switch control name for the current scenario. + */ + int snd_scenario_get_master_playback_switch(struct snd_scenario *scn); + +/** + * snd_scenario_get_master_capture_volume - get capture volume + * @scn: scenario + * + * Get the master capture volume control name for the current scenario. + */ +int snd_scenario_get_master_capture_volume(struct snd_scenario *scn); + +/** + * snd_scenario_get_master_capture_switch - get capture switch + * @scn: scenario + * + * Get the master capture switch control name for the current scenario. + */ +int snd_scenario_get_master_capture_switch(struct snd_scenario *scn); + +/** + * snd_scenario_dump - dump + * @card_name: sound card name. + * + * Dump current sound card settings to stdout in scn format. + */ +int snd_scenario_dump(const char *card_name); diff --git a/src/Makefile.am b/src/Makefile.am index 3204fe4..be46cb3 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -14,7 +14,7 @@ SYMFUNCS = endif
lib_LTLIBRARIES = libasound.la -libasound_la_SOURCES = conf.c confmisc.c input.c output.c async.c error.c dlmisc.c socket.c shmarea.c userfile.c names.c +libasound_la_SOURCES = conf.c confmisc.c input.c output.c async.c error.c dlmisc.c socket.c shmarea.c userfile.c names.c ascenario.c
SUBDIRS=control libasound_la_LIBADD = control/libcontrol.la diff --git a/src/ascenario.c b/src/ascenario.c new file mode 100644 index 0000000..d2b116f --- /dev/null +++ b/src/ascenario.c @@ -0,0 +1,1357 @@ +/* + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. + * + * Copyright (C) 2008-2009 SlimLogic Ltd + * Authors: Liam Girdwood lrg@slimlogic.co.uk + * Stefan Schmidt stefan@slimlogic.co.uk + */ + +#define _GNU_SOURCE /* needed of O_NOATIME */ + +#include <stdio.h> +#include <unistd.h> +#include <errno.h> +#include <string.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <ctype.h> +#include <alsa/asoundlib.h> + +#include "../include/ascenario.h" + +#define PRE_SEQ 0 +#define POST_SEQ 1 +#define MAX_SCN 32 +#define MAX_NAME 64 +#define MAX_FILE 256 +#define MAX_BUF 256 +#define ALSA_SCN_DIR "/etc/alsa/scenario" + +/* + * Stores all scenario settings for 1 kcontrol. Hence we have a + * control_settings for each kcontrol in card. + */ +struct control_settings { + char name[MAX_NAME]; + int id; + snd_ctl_elem_type_t type; + int count; /* 1 = mono, 2 = stereo, etc */ + unsigned short *value; /* kcontrol value 2D array */ +}; + +/* + * If sleep is 0 the element contains the settings in control. Else sleep + * contains the sleep time in micro seconds. + */ +struct sequence_element { + unsigned int sleep; /* Sleep time in msecs if sleep element, else 0 */ + struct control_settings *control; + struct sequence_element *next; /* Pointer to next list element */ +}; + +/* + * Describes default mixers and qos for scenario. + * We have a scenario_info for each scenario loaded. + */ +struct scenario_info { + char *name; + char *file; + char *pre_sequence_file; + char *post_sequence_file; + short playback_volume_id; + short playback_switch_id; + short capture_volume_id; + short capture_switch_id; + int qos; +}; + +/* Describe a snd card and all it's scenarios. + */ +struct snd_scenario { + char *card_name; + int current_scenario; + int num_scenarios; /* number of supported scenarios */ + int num_kcontrols; /* number of kcontrols */ + struct sequence_element *pre_seq_list; /* Linked list for pre sequence */ + struct sequence_element *post_seq_list; /* Linked list for post sequence */ + const char **list; + struct scenario_info *scenario; /* var len array of scenario info */ + struct control_settings *control; /* var len array of controls */ +}; + +static void scn_error(const char *fmt,...) +{ + va_list va; + va_start(va, fmt); + fprintf(stderr, "scenario: "); + vfprintf(stderr, fmt, va); + va_end(va); +} + +static void scn_stdout(const char *fmt,...) +{ + va_list va; + va_start(va, fmt); + vfprintf(stdout, fmt, va); + va_end(va); +} + +static inline void set_value(struct snd_scenario *scn, + struct control_settings *control, int count, unsigned short val) +{ + int offset = scn->current_scenario * control->count; + control->value[offset + count] = val; +} + +static inline unsigned short get_value(struct snd_scenario *scn, + struct control_settings *control, int count) +{ + int offset = scn->current_scenario * control->count; + return control->value[offset + count]; +} + +static int dump_control(snd_ctl_t *handle, snd_ctl_elem_id_t *id) +{ + int err, count, i; + snd_ctl_elem_info_t *info; + snd_ctl_elem_type_t type; + snd_ctl_elem_value_t *control; + + snd_ctl_elem_info_alloca(&info); + snd_ctl_elem_value_alloca(&control); + + snd_ctl_elem_info_set_id(info, id); + err = snd_ctl_elem_info(handle, info); + if (err < 0) { + scn_stdout("%s: failed to get ctl info\n"); + return err; + } + + snd_ctl_elem_value_set_id(control, id); + snd_ctl_elem_read(handle, control); + + type = snd_ctl_elem_info_get_type(info); + count = snd_ctl_elem_info_get_count(info); + if (count == 0) + return 0; + + scn_stdout("%u:'%s':%d:", + snd_ctl_elem_id_get_numid(id), + snd_ctl_elem_id_get_name(id), count); + + switch (type) { + case SND_CTL_ELEM_TYPE_BOOLEAN: + for (i = 0; i < count - 1; i++) + scn_stdout("%d,", + snd_ctl_elem_value_get_boolean(control, i)); + scn_stdout("%d", snd_ctl_elem_value_get_boolean(control, i)); + break; + case SND_CTL_ELEM_TYPE_INTEGER: + for (i = 0; i < count - 1; i++) + scn_stdout("%d,", + snd_ctl_elem_value_get_integer(control, i)); + scn_stdout("%d", snd_ctl_elem_value_get_integer(control, i)); + break; + case SND_CTL_ELEM_TYPE_INTEGER64: + for (i = 0; i < count - 1; i++) + scn_stdout("%ld,", + snd_ctl_elem_value_get_integer64(control, i)); + scn_stdout("%ld", + snd_ctl_elem_value_get_integer64(control, i)); + break; + case SND_CTL_ELEM_TYPE_ENUMERATED: + for (i = 0; i < count - 1; i++) + scn_stdout("%d,", + snd_ctl_elem_value_get_enumerated(control, i)); + scn_stdout("%d", + snd_ctl_elem_value_get_enumerated(control, i)); + break; + case SND_CTL_ELEM_TYPE_BYTES: + for (i = 0; i < count - 1; i++) + scn_stdout("%2.2x,", + snd_ctl_elem_value_get_byte(control, i)); + scn_stdout("%2.2x", snd_ctl_elem_value_get_byte(control, i)); + break; + default: + break; + } + scn_stdout("\n"); + return 0; +} + +/* + * Add new kcontrol from sound card into memory database. + */ +static int add_control(snd_ctl_t *handle, snd_ctl_elem_id_t *id, + struct control_settings *control_settings) +{ + int err; + snd_ctl_elem_info_t *info; + snd_ctl_elem_value_t *control; + + snd_ctl_elem_info_alloca(&info); + snd_ctl_elem_value_alloca(&control); + + snd_ctl_elem_info_set_id(info, id); + err = snd_ctl_elem_info(handle, info); + if (err < 0) { + scn_stdout("%s: failed to get ctl info\n"); + return err; + } + + snd_ctl_elem_value_set_id(control, id); + snd_ctl_elem_read(handle, control); + + strncpy(control_settings->name, snd_ctl_elem_id_get_name(id), + MAX_NAME); + control_settings->count = snd_ctl_elem_info_get_count(info); + control_settings->type = snd_ctl_elem_info_get_type(info); + control_settings->id = snd_ctl_elem_id_get_numid(id); + return 0; +} + +static int parse_controls(struct snd_scenario *scn, FILE *f) +{ + struct control_settings *control; + char buf[MAX_BUF], name[MAX_NAME]; + int id, count, line = 1, i; + char *name_start, *name_end, *tbuf; + + while (fgets(buf, MAX_BUF, f) != NULL) { + + /* get id */ + tbuf = buf; + id = atoi(tbuf); + if (id == 0) { + scn_error("%s:id == 0 on line %d\n", __func__, line); + return -EINVAL; + } + for (i = 0; i < scn->num_kcontrols; i++) { + if (id == scn->control[i].id) { + control = &scn->control[i]; + goto get_name; + } + } + scn_error("%s:id not found at line %d\n", __func__, line); + return -EINVAL; +get_name: + /* get name start */ + while (*tbuf != 0 && *tbuf != ''') + tbuf++; + if (*tbuf == 0) + return -EINVAL; + name_start = ++tbuf; + + /* get name end */ + while (*tbuf != 0 && *tbuf != ''') + tbuf++; + if (*tbuf == 0) + return -EINVAL; + name_end = tbuf++; + + /* copy name */ + if ((name_end - name_start) > MAX_NAME) { + scn_error("%s:name too big at %d chars line %d\n", + __func__, name_end - name_start, line); + return -EINVAL; + } + strncpy(name, name_start, name_end - name_start); + name[name_end - name_start] = 0; + if (strcmp(name, control->name)) { + scn_error("%s: name %s and %s don't match at line %d\n", + __func__, name, control->name, line); + return -EINVAL; + } + + /* get count */ + tbuf++; + count = atoi(tbuf); + if (count == 0) { + scn_error("%s:count == 0 on line %d\n", __func__, + line); + return -EINVAL; + } + if (count != control->count) { + scn_error("%s:count does not match at line %d\n", + __func__, line); + return -EINVAL; + } + + /* get vals */ + control->value = malloc(control->count * scn->num_scenarios * + sizeof(unsigned short)); + if (control->value == NULL) + return -ENOMEM; + + while (*tbuf != 0 && *tbuf != ':') + tbuf++; + if (*tbuf == 0) + return -EINVAL; + tbuf++; + + for (i = 0; i < count; i++) { + set_value(scn, control, i, atoi(tbuf)); + while (*tbuf != 0 && *tbuf != ',') + tbuf++; + + if (*tbuf++ == 0 && i < (count - 1)) + return -EINVAL; + } + line++; + } + + return 0; +} + +static char *get_string (char *buf) +{ + char *str, *end; + + /* find '=' */ + while (isblank(*buf)) + buf++; + if (*buf == 0 || *buf != '=') { + scn_error("%s: missing '='\n", __func__); + return NULL; + } + + /* find leading '"' */ + buf++; + while (isblank(*buf)) + buf++; + if (*buf == 0 || *buf != '"') { + scn_error("%s: missing start '"'\n", __func__); + return NULL; + } + str = ++buf; + + /* get value */ + while (*buf != 0 && *buf != '"') + buf++; + end = buf; + + /* find '"' terminator */ + if (*buf == 0 || *buf != '"') { + scn_error("%s: missing terminator '"' %s\n", __func__, buf); + return NULL; + } + + *end = 0; + return strdup(str); +} + +static char *get_control_name (char *buf) +{ + char *str, *end; + + /* find leading '"' */ + buf++; + while (isblank(*buf)) + buf++; + if (*buf == 0 || *buf != '"') { + scn_error("%s: missing start '"'\n", __func__); + return NULL; + } + str = ++buf; + + /* get value */ + while (*buf != 0 && *buf != '"') + buf++; + end = buf; + + /* find '"' terminator */ + if (*buf == 0 || *buf != '"') { + scn_error("%s: missing terminator '"' %s\n", __func__, buf); + return NULL; + } + + *end = 0; + return strdup(str); +} + +static int get_int (char *buf) +{ + /* find '=' */ + while (isblank(*buf)) + buf++; + if (*buf == 0 || *buf != '=') { + scn_error("%s: missing '='\n", __func__); + return -EINVAL; + } + buf++; + return atoi(buf); +} + +static int get_enum (char *buf) +{ + /* find '=' */ + while (isblank(*buf)) + buf++; + if (*buf == 0 || *buf != '=') { + scn_error("%s: missing '='\n", __func__); + return -EINVAL; + } + buf++; + return 0; /* TODO */ +} + +static void seq_list_append(struct snd_scenario *scn, + struct sequence_element *curr, int position) +{ + struct sequence_element *last, *tmp; + + if (position) { + if (!scn->post_seq_list) + scn->post_seq_list = curr; + + else { + tmp = scn->post_seq_list; + while (tmp) { + last = tmp; + tmp = tmp->next; + } + last->next = curr; + } + } + else { + if (!scn->pre_seq_list) { + scn->pre_seq_list = curr; + } + else { + tmp = scn->pre_seq_list; + while (tmp) { + last = tmp; + tmp = tmp->next; + } + last->next = curr; + } + } +} + +static int parse_sequences(struct snd_scenario *scn, FILE *f, int position) +{ + char buf[MAX_BUF], *tbuf, *control_value; + int control_len, i; + struct sequence_element *curr; + + while (fgets(buf, MAX_BUF, f) != NULL) { + + /* Check for lines with comments and ignore */ + if (buf[0] == '#') + continue; + + /* Parse current line and skip blanks */ + tbuf = buf; + while (isblank(*tbuf)) + tbuf++; + + curr = malloc(sizeof(struct sequence_element)); + if (curr == NULL) + return -ENOMEM; + bzero(curr, sizeof(struct sequence_element)); + + curr->control = malloc(sizeof(struct control_settings)); + if (curr->control == NULL) + return -ENOMEM; + bzero(curr->control, sizeof(struct control_settings)); + + curr->control->value = malloc(curr->control->count * scn->num_scenarios + * sizeof(unsigned short)); + if (curr->control->value == NULL) + return -ENOMEM; + bzero(curr->control->value, curr->control->count * scn->num_scenarios + * sizeof(unsigned short)); + + if (strncmp(tbuf, "kcontrol", 8) == 0) { + strncpy(curr->control->name, get_control_name(tbuf + 8), MAX_NAME); + control_len = strlen(curr->control->name); + /* 11 = 8 from kcontrol + 2 quotes + 1 blank */ + control_value = get_string(tbuf + 11 + control_len); + + for (i = 0; i < scn->num_kcontrols; i++) { + if (strncmp(curr->control->name, scn->control[i].name, + control_len) == 0) { + curr->sleep = 0; + curr->control->id = scn->control[i].id; + curr->control->type = scn->control[i].type; + curr->control->count = scn->control[i].count; + set_value(scn, curr->control, curr->control->count, + atoi(control_value)); + seq_list_append(scn, curr, position); + } + } + + continue; + } + + if (strncmp(tbuf, "msleep", 6) == 0) { + curr->sleep = get_int(tbuf + 6); + + /* Free control elements as we only have a sleep element + * here */ + if (curr->control) { + if (curr->control->value) + free(curr->control->value); + free(curr->control); + } + + seq_list_append(scn, curr, position); + continue; + } + } + + return 0; +} + +/* load scenario i */ +static int read_scenario_file(struct snd_scenario *scn) +{ + int fd, ret; + FILE *f; + char filename[MAX_FILE]; + struct scenario_info *info = &scn->scenario[scn->current_scenario]; + + sprintf(filename, "%s/%s/%s", ALSA_SCN_DIR, scn->card_name, + info->file); + + fd = open(filename, O_RDONLY | O_NOATIME); + if (fd < 0) { + scn_error("%s: couldn't open %s\n", __func__, filename); + return fd; + } + + f = fdopen(fd, "r"); + if (f == NULL) { + ret = errno; + goto close; + } + + ret = parse_controls(scn, f); + fclose(f); +close: + close(fd); + return ret; +} + +static int read_sequence_file(struct snd_scenario *scn, int position) +{ + int fd, ret; + FILE *f; + char filename[MAX_FILE]; + struct scenario_info *info = &scn->scenario[scn->current_scenario]; + + if (position == PRE_SEQ) { + sprintf(filename, "%s/%s/%s", ALSA_SCN_DIR, scn->card_name, + info->pre_sequence_file); + } + else { + sprintf(filename, "%s/%s/%s", ALSA_SCN_DIR, scn->card_name, + info->post_sequence_file); + } + + fd = open(filename, O_RDONLY | O_NOATIME); + if (fd < 0) { + return fd; + } + + f = fdopen(fd, "r"); + if (f == NULL) { + ret = errno; + goto close; + } + + ret = parse_sequences(scn, f, position); + fclose(f); +close: + close(fd); + return ret; +} + +static int parse_scenario(struct snd_scenario *scn, FILE *f, int line_) +{ + struct scenario_info *info; + int line = line_ - 1, id = 0, file = 0; + char buf[MAX_BUF], *tbuf; + + scn->scenario = realloc(scn->scenario, + (scn->num_scenarios + 1) * sizeof(struct scenario_info)); + if (scn->scenario == NULL) + return -ENOMEM; + info = scn->scenario + scn->num_scenarios; + + /* Set sequence filename to NULL as it is optional and we want to check + * for NULL to avoid segfaults */ + info->pre_sequence_file = NULL; + info->post_sequence_file = NULL; + + while(fgets(buf, MAX_BUF, f) != NULL) { + + line++; + if (buf[0] == '#') + continue; + + tbuf = buf; + while (isblank(*tbuf)) + tbuf++; + + if (strncmp(tbuf, "Identifier", 10) == 0) { + info->name = get_string(tbuf + 10); + if (info->name == NULL) { + scn_error("%s: failed to get Identifer\n", + __func__); + goto err; + } + id = 1; + continue; + } + + if (strncmp(tbuf, "File", 4) == 0) { + info->file = get_string(tbuf + 4); + if (info->file == NULL) { + scn_error("%s: failed to get File\n", + __func__); + goto err; + } + file = 1; + continue; + } + + if (strncmp(tbuf, "QoS", 3) == 0) { + info->qos = get_enum(tbuf + 3); + if (info->qos < 0) { + scn_error("%s: failed to get QoS\n", + __func__); + goto err; + } + continue; + } + + if (strncmp(tbuf, "MasterPlaybackVolume", 20) == 0) { + info->playback_volume_id = get_int(tbuf + 20); + if (info->playback_volume_id < 0) { + scn_error("%s: failed to get MasterPlaybackVolume\n", + __func__); + goto err; + } + continue; + } + + if (strncmp(tbuf, "MasterPlaybackSwitch", 20) == 0) { + info->playback_switch_id = get_int(tbuf + 20); + if (info->playback_switch_id < 0) { + scn_error("%s: failed to get MasterPlaybackSwitch\n", + __func__); + goto err; + } + continue; + } + + if (strncmp(tbuf, "MasterCaptureVolume", 19) == 0) { + info->capture_volume_id = get_int(tbuf + 19); + if (info->capture_volume_id < 0) { + scn_error("%s: failed to get MasterCaptureVolume\n", + __func__); + goto err; + } + continue; + } + + if (strncmp(tbuf, "MasterCaptureSwitch", 19) == 0) { + info->capture_switch_id = get_int(tbuf + 19); + if (info->capture_switch_id < 0) { + scn_error("%s: failed to get MasterCaptureSwitch\n", + __func__); + goto err; + } + continue; + } + + if (strncmp(tbuf, "PreSequenceFile", 15) == 0) { + info->pre_sequence_file = get_string(tbuf + 15); + if (info->pre_sequence_file == NULL) { + scn_error("%s: failed to get PreSequenceFile\n", + __func__); + goto err; + } + continue; + } + + if (strncmp(tbuf, "PostSequenceFile", 16) == 0) { + info->post_sequence_file = get_string(tbuf + 16); + if (info->post_sequence_file == NULL) { + scn_error("%s: failed to get PostSequenceFile\n", + __func__); + goto err; + } + continue; + } + + if (strncmp(tbuf, "EndSection", 10) == 0) { + break; + } + } + + if (file & id) { + scn->num_scenarios++; + return 0; + } +err: + if (file) { + free(info->file); + info->file = NULL; + } + if (id) { + free(info->name); + info->name = NULL; + } + return -EINVAL; +} + +static int read_master_file(struct snd_scenario *scn, FILE *f) +{ + int line = 0, ret = 0, i; + char buf[MAX_BUF], *tbuf; + + /* parse master config sections */ + while(fgets(buf, MAX_BUF, f) != NULL) { + + if (buf[0] == '#') { + line++; + continue; + } + + if (strncmp(buf, "Section", 7) == 0) { + + tbuf = buf + 7; + while (isblank(*tbuf)) + tbuf++; + + if (strncmp(tbuf, ""Scenario"", 10) == 0) { + line = parse_scenario(scn, f, line); + if (line < 0) { + scn_error("%s: failed to parse " + "scenario\n", __func__); + goto err; + } + continue; + } + } + line++; + } + + /* copy ptrs to scenario names */ + scn->list = malloc(scn->num_scenarios * sizeof(char *)); + if (scn->list == NULL) + ret = -ENOMEM; + for (i = 0; i < scn->num_scenarios; i++) + scn->list[i] = scn->scenario[i].name; + +err: + if (ferror(f)) { + scn_error("%s: failed to read master\n", __func__); + return ferror(f); + } + return ret; +} + +/* load scenario i */ +static int import_master_config(struct snd_scenario *scn) +{ + int fd, ret; + FILE *f; + char filename[MAX_FILE]; + + sprintf(filename, "%s/%s.conf", ALSA_SCN_DIR, scn->card_name); + + fd = open(filename, O_RDONLY | O_NOATIME); + if (fd < 0) { + scn_error("%s: couldn't open %s\n", __func__, filename); + return fd; + } + + f = fdopen(fd, "r"); + if (f == NULL) { + ret = errno; + goto close; + } + + ret = read_master_file(scn, f); + fclose(f); +close: + close(fd); + return ret; +} + +/* parse_card_controls + * @scn: scenario + * + * Parse sound card and store control data in memory db. + */ +static int parse_card_controls(struct snd_scenario *scn) +{ + struct control_settings *control; + snd_ctl_t *handle; + snd_ctl_card_info_t *info; + snd_ctl_elem_list_t *list; + int ret, i; + + snd_ctl_card_info_alloca(&info); + snd_ctl_elem_list_alloca(&list); + + /* open and load snd card */ + ret = snd_ctl_open(&handle, scn->card_name, SND_CTL_READONLY); + if (ret < 0) { + scn_error("%s: control %s open retor: %s\n", __func__, + scn->card_name, snd_strerror(ret)); + return ret; + } + + ret = snd_ctl_card_info(handle, info); + if (ret < 0) { + scn_error("%s :control %s local retor: %s\n", __func__, + scn->card_name, snd_strerror(ret)); + goto close; + } + + ret = snd_ctl_elem_list(handle, list); + if (ret < 0) { + scn_error("%s: cannot determine controls: %s\n", __func__, + snd_strerror(ret)); + goto close; + } + + scn->num_kcontrols = snd_ctl_elem_list_get_count(list); + if (scn->num_kcontrols < 0) { + ret = 0; + goto close; + } + + snd_ctl_elem_list_set_offset(list, 0); + if (snd_ctl_elem_list_alloc_space(list, scn->num_kcontrols) < 0) { + scn_error("%s: not enough memory...\n", __func__); + ret = -ENOMEM; + goto close; + } + if ((ret = snd_ctl_elem_list(handle, list)) < 0) { + scn_error("%s: cannot determine controls: %s\n", __func__, + snd_strerror(ret)); + goto free; + } + + /* allocate db memory for controls */ + scn->control = calloc(scn->num_kcontrols, + sizeof(struct control_settings)); + if (scn->control == NULL) { + ret = -ENOMEM; + goto free; + } + control = scn->control; + + /* iterate through each kcontrol and add to db */ + for (i = 0; i < scn->num_kcontrols; ++i) { + snd_ctl_elem_id_t *id; + snd_ctl_elem_id_alloca(&id); + snd_ctl_elem_list_get_id(list, i, id); + + ret = add_control(handle, id, control++); + if (ret < 0) { + scn_error("%s: failed to add control error %s\n", + __func__, snd_strerror(ret)); + goto close; + } + } +free: + snd_ctl_elem_list_free_space(list); +close: + snd_ctl_close(handle); + return ret; +} + +/* import_scenario_files - + * @scn: scenario + * + * Read and parse scenario_info files the store in memory. + */ +static int import_scenario_files(struct snd_scenario *scn) +{ + int ret; + + ret = import_master_config(scn); + if (ret < 0) { + scn_error("%s: failed to parse master scenario config\n", + __func__); + return ret; + } + + for (scn->current_scenario = 0; + scn->current_scenario < scn->num_scenarios; + scn->current_scenario++) { + + ret = read_scenario_file(scn); + if (ret < 0) { + scn_error("%s: failed to parse scenario %s\n", + __func__, + scn->scenario[scn->current_scenario].name); + scn->current_scenario = -1; + return ret; + } + + if (scn->scenario[scn->current_scenario].pre_sequence_file != NULL) { + ret = read_sequence_file(scn, PRE_SEQ); + if (ret < 0) { + scn_stdout("Warning: PreSequence file defined but" + " missing in scenario "%s"\n", + scn->scenario[scn->current_scenario].name); + } + } + + if (scn->scenario[scn->current_scenario].post_sequence_file != NULL) { + ret = read_sequence_file(scn, POST_SEQ); + if (ret < 0) { + scn_stdout("Warning: PostSequence file defined but" + " missing in scenario "%s"\n", + scn->scenario[scn->current_scenario].name); + } + } + + } + return 0; +} + +/* free all resorces */ +static void free_scn(struct snd_scenario *scn) +{ + //TODO: valgrind to make sure. + int i; + + if (scn == NULL) + return; + + if (scn->control) { + if (scn->control->value) + free(scn->control->value); + free(scn->control); + } + + if (scn->list) + free(scn->list); + if (scn->card_name) + free(scn->card_name); + if (scn->pre_seq_list) + free(scn->pre_seq_list); + if (scn->post_seq_list) + free(scn->post_seq_list); + + if (scn->scenario) { + for (i = 0; i < scn->num_scenarios; i++) { + struct scenario_info *info = &scn->scenario[i]; + + if (info->name) + free(info->name); + if (info->file) + free(info->file); + if (info->pre_sequence_file) + free(info->pre_sequence_file); + if (info->post_sequence_file) + free(info->post_sequence_file); + } + free(scn->scenario); + } + free(scn); +} + +/* + * Init sound card scenario db. + */ +struct snd_scenario *snd_scenario_open(const char *card_name) +{ + struct snd_scenario *scn; + int err; + + // TODO: locking and + // check if card_name scn is already loaded, + // if so reuse to conserve ram. + + scn = malloc(sizeof(struct snd_scenario)); + if (scn == NULL) + return NULL; + bzero(scn, sizeof(struct snd_scenario)); + scn->card_name = strdup(card_name); + if (scn->card_name == NULL) { + free(scn); + return NULL; + } + + /* get info about sound card */ + err = parse_card_controls(scn); + if (err < 0) { + free_scn(scn); + return NULL; + } + + /* get info on scenarios and verify against card */ + err = import_scenario_files(scn); + if (err < 0) { + free_scn(scn); + return NULL; + } + + return scn; +} + +/* + * Reload and reparse scenario db. + */ +int snd_scenario_reload(struct snd_scenario *scn) +{ + free_scn(scn); + + scn->num_kcontrols = parse_card_controls(scn); + if (scn->num_kcontrols <= 0) { + free_scn(scn); + return -EINVAL; + } + + scn->num_scenarios = import_scenario_files(scn); + if (scn->num_scenarios <= 0) { + return -EINVAL; + } + + return 0; +} + +void snd_scenario_close(struct snd_scenario *scn) +{ + free_scn(scn); +} + +static int set_control(snd_ctl_t *handle, snd_ctl_elem_id_t *id, + struct snd_scenario *scn) +{ + struct control_settings *setting; + int ret, count, i, idnum; + snd_ctl_elem_info_t *info; + snd_ctl_elem_type_t type; + snd_ctl_elem_value_t *control; + + snd_ctl_elem_info_alloca(&info); + snd_ctl_elem_value_alloca(&control); + + snd_ctl_elem_info_set_id(info, id); + ret = snd_ctl_elem_info(handle, info); + if (ret < 0) { + scn_error("%s: failed to get ctl info\n", __func__); + return ret; + } + + snd_ctl_elem_value_set_id(control, id); + snd_ctl_elem_read(handle, control); + + idnum = snd_ctl_elem_id_get_numid(id); + for (i = 0; i < scn->num_kcontrols; i++) { + setting = &scn->control[i]; + if (setting->id == idnum) + goto set_val; + } + scn_error("%s: failed to find control %d\n", __func__, idnum); + return 0; + +set_val: + type = snd_ctl_elem_info_get_type(info); + count = snd_ctl_elem_info_get_count(info); + if (count == 0) + return 0; + + switch (type) { + case SND_CTL_ELEM_TYPE_BOOLEAN: + for (i = 0; i < count; i++) + snd_ctl_elem_value_set_boolean(control, i, + get_value(scn, setting, i)); + break; + case SND_CTL_ELEM_TYPE_INTEGER: + for (i = 0; i < count; i++) + snd_ctl_elem_value_set_integer(control, i, + get_value(scn, setting, i)); + + break; + case SND_CTL_ELEM_TYPE_INTEGER64: + for (i = 0; i < count; i++) + snd_ctl_elem_value_set_integer64(control, i, + get_value(scn, setting, i)); + + break; + case SND_CTL_ELEM_TYPE_ENUMERATED: + for (i = 0; i < count; i++) + snd_ctl_elem_value_set_enumerated(control, i, + get_value(scn, setting, i)); + + break; + case SND_CTL_ELEM_TYPE_BYTES: + for (i = 0; i < count; i++) + snd_ctl_elem_value_set_byte(control, i, + get_value(scn, setting, i)); + break; + default: + break; + } + + ret = snd_ctl_elem_write(handle, control); + if (ret < 0) { + scn_error("%s: control %s failed: %s\n", __func__, + setting->name, snd_strerror(ret)); + scn_error("%s: count %d type: %d\n", __func__, + count, type); + for (i = 0; i < count; i++) + fprintf(stderr, "%d ", get_value(scn, setting, i)); + return ret; + } + return 0; +} + +static void exec_sequence(struct sequence_element *seq, struct snd_scenario + *scn, snd_ctl_elem_list_t *list, snd_ctl_t *handle) +{ + int count = snd_ctl_elem_list_get_count(list); + while (seq) { + if (seq->sleep) + usleep(seq->sleep); + else { + snd_ctl_elem_id_t *id; + snd_ctl_elem_id_alloca(&id); + int ret, i, numid; + /* Where is id lookup from numid if you need it? */ + for (i = 0; i < count; ++i) { + snd_ctl_elem_list_get_id(list, i, id); + numid = snd_ctl_elem_id_get_numid(id); + if (numid == seq->control->id) { + ret = set_control(handle, id, scn); + if (ret < 0) { + scn_error("%s: failed to set control %s\n", + __func__, scn->card_name); + } + break; + } + } + } + seq = seq->next; + } +} + +int snd_scenario_set_scn(struct snd_scenario *scn, const char *name) +{ + snd_ctl_card_info_t *info; + snd_ctl_elem_list_t *list; + snd_ctl_t *handle; + int ret, count, i; + + snd_ctl_card_info_alloca(&info); + snd_ctl_elem_list_alloca(&list); + + /* find scenario name */ + for (i = 0; i < scn->num_scenarios; i++) { + if (!strcmp(scn->scenario[i].name, name)) + goto found; + } + scn_error("%s: scenario %s not found\n", __func__, name); + return -EINVAL; + +found: + /* scenario found - now open card */ + scn->current_scenario = i; + ret = snd_ctl_open(&handle, scn->card_name, 0); + if (ret) { + scn_error("%s: control %s open error: %s\n", __func__, + scn->card_name, snd_strerror(ret)); + return ret; + } + + ret = snd_ctl_card_info(handle, info); + if (ret < 0) { + scn_error("%s :control %s local retor: %s\n", __func__, + scn->card_name, snd_strerror(ret)); + goto close; + } + + ret = snd_ctl_elem_list(handle, list); + if (ret < 0) { + scn_error("%s: cannot determine controls: %s\n", __func__, + snd_strerror(ret)); + goto close; + } + + count = snd_ctl_elem_list_get_count(list); + if (count < 0) { + ret = 0; + goto close; + } + + snd_ctl_elem_list_set_offset(list, 0); + if (snd_ctl_elem_list_alloc_space(list, count) < 0) { + scn_error("%s: not enough memory...\n", __func__); + ret = -ENOMEM; + goto close; + } + if ((ret = snd_ctl_elem_list(handle, list)) < 0) { + scn_error("%s: cannot determine controls: %s\n", __func__, + snd_strerror(ret)); + goto free; + } + + /* If we have a sequence list that should be executed before the new + * scenario is set do it now */ + if (scn->pre_seq_list) + exec_sequence(scn->pre_seq_list, scn, list, handle); + + /* iterate through each kcontrol and add to db */ + for (i = 0; i < count; ++i) { + snd_ctl_elem_id_t *id; + snd_ctl_elem_id_alloca(&id); + snd_ctl_elem_list_get_id(list, i, id); + + ret = set_control(handle, id, scn); + if (ret < 0) { + scn_error("%s: failed to set control %s\n", __func__, + scn->card_name); + } + } + + /* If we have a sequence list that should be executed after the new + * scenario is set do it now */ + if (scn->post_seq_list) + exec_sequence(scn->post_seq_list, scn, list, handle); + +free: + snd_ctl_elem_list_free_space(list); +close: + snd_ctl_close(handle); + return ret; +} + +int snd_scenario_dump(const char *card_name) +{ + snd_ctl_t *handle; + snd_ctl_card_info_t *info; + snd_ctl_elem_list_t *list; + int ret, i, count; + + snd_ctl_card_info_alloca(&info); + snd_ctl_elem_list_alloca(&list); + + /* open and load snd card */ + ret = snd_ctl_open(&handle, card_name, SND_CTL_READONLY); + if (ret < 0) { + scn_error("%s: control %s open retor: %s\n", __func__, card_name, + snd_strerror(ret)); + return ret; + } + + ret = snd_ctl_card_info(handle, info); + if (ret < 0) { + scn_error("%s :control %s local retor: %s\n", __func__, + card_name, snd_strerror(ret)); + goto close; + } + + ret = snd_ctl_elem_list(handle, list); + if (ret < 0) { + scn_error("%s: cannot determine controls: %s\n", __func__, + snd_strerror(ret)); + goto close; + } + + count = snd_ctl_elem_list_get_count(list); + if (count < 0) { + ret = 0; + goto close; + } + + snd_ctl_elem_list_set_offset(list, 0); + if (snd_ctl_elem_list_alloc_space(list, count) < 0) { + scn_error("%s: not enough memory...\n", __func__); + ret = -ENOMEM; + goto close; + } + if ((ret = snd_ctl_elem_list(handle, list)) < 0) { + scn_error("%s: cannot determine controls: %s\n", __func__, + snd_strerror(ret)); + goto free; + } + + /* iterate through each kcontrol and add to db */ + for (i = 0; i < count; ++i) { + snd_ctl_elem_id_t *id; + snd_ctl_elem_id_alloca(&id); + snd_ctl_elem_list_get_id(list, i, id); + + ret = dump_control(handle, id); + if (ret < 0) { + scn_error("%s: cannot determine controls: %s\n", + __func__, snd_strerror(ret)); + goto free; + } + } +free: + snd_ctl_elem_list_free_space(list); +close: + snd_ctl_close(handle); + return ret; +} + +const char *snd_scenario_get_scn(struct snd_scenario *scn) +{ + if (scn->current_scenario > 0 && scn->current_scenario < MAX_SCN) + return scn->scenario[scn->current_scenario].name; + else + return NULL; +} + +int snd_scenario_set_qos(struct snd_scenario *scn, int qos) +{ + /* TODO: change QoS kcontrols */ + scn->scenario[scn->current_scenario].qos = qos; + return 0; +} + +int snd_scenario_get_qos(struct snd_scenario *scn) +{ + return scn->scenario[scn->current_scenario].qos; +} + +int snd_scenario_get_master_playback_volume(struct snd_scenario *scn) +{ + return scn->scenario[scn->current_scenario].playback_volume_id; +} + +int snd_scenario_get_master_playback_switch(struct snd_scenario *scn) +{ + return scn->scenario[scn->current_scenario].playback_switch_id; +} + +int snd_scenario_get_master_capture_volume(struct snd_scenario *scn) +{ + return scn->scenario[scn->current_scenario].capture_volume_id; +} + +int snd_scenario_get_master_capture_switch(struct snd_scenario *scn) +{ + return scn->scenario[scn->current_scenario].capture_switch_id; +} + +int snd_scenario_list(struct snd_scenario *scn, const char **list[]) +{ + *list = scn->list; + return scn->num_scenarios; +}
On Thu, Oct 01, 2009 at 11:47:15AM +0200, Stefan Schmidt wrote:
It allows switching audio settings between scenarios or uses-cases like listening to music and answering an incoming phone call. Made of control aliasing for playback, capture master and switch as well as the option to post- and prefix a sequence of control changes avoiding pops and other unwanted noise. Some example programs will be available in alsa-utils.
Overall this looks good - it's certainly dealing with the issues we currently have with separating routing control and 'end user' controls.
+/** Scenario ID's
- Standard Scenario ID's - Add new scenarios at the end.
- */
Extra 's here.
+/**
- snd_scenario_reload - reload and reparse scenario configuration
- @scn: scenario
- Reloads and reparses sound card scenario configuration.
- */
+int snd_scenario_reload(struct snd_scenario *scn);
I guess the idea is that in the future this will be removed and the scenario API will use inotify or similar to pick up changes.
One thing I think I'm missing with the API documentation here is a separation between the API used for setting up scenarios and the API used by random client applications - it's there, but it could be underlined a bit more. Probably putting the client application stuff at the top of the header file would help here since the functions that more people will use will be visible first.
+/* load scenario i */ +static int read_scenario_file(struct snd_scenario *scn) +{
- int fd, ret;
- FILE *f;
- char filename[MAX_FILE];
- struct scenario_info *info = &scn->scenario[scn->current_scenario];
- sprintf(filename, "%s/%s/%s", ALSA_SCN_DIR, scn->card_name,
info->file);
snprintf().
if (strncmp(tbuf, "MasterPlaybackVolume", 20) == 0) {info->playback_volume_id = get_int(tbuf + 20);if (info->playback_volume_id < 0) {scn_error("%s: failed to get MasterPlaybackVolume\n",__func__);goto err;}continue;}
I don't see anywhere which supplies a default value for all these control IDs. A memset() of the allocated block should deal with that I think since IIRC zero isn't a valid control ID. It'd also be nice to be able to use control names instead but that's something that could be added later.
Thinking out loud here but I'm wondering if it might make sense to replace the fixed list of controls that is there currently with something string based which can remap the control names if required. This would allow for (probably in the future) having the scenario pass back a list of controls without the API having to cater for each explicitly, which would allow for other things like hardware EQ controls to be passed on to the scenario users if desired - this is useful when you get things like systems with multiple EQs.
This would complicate the API, though, and so I think it would be better left as-is until we get to the point of having something like a plugin for rewriting the controls for applications so they don't need explicit knowledge of scenarios. At that point it would only impact the scenario manager implementation which should deal with the complexity, at least externally.
I'm also thinking that it could be good to add functions to identify which PCMs on a card to use in the scenario, perhaps differentiated by quality or something. The CPU may have PCMs to multiple devices or with differing capabilities and may want to switch between them depending on scenario (and possibly stream).
On Thu, 2009-10-01 at 13:28 +0100, Mark Brown wrote:
Thinking out loud here but I'm wondering if it might make sense to replace the fixed list of controls that is there currently with something string based which can remap the control names if required. This would allow for (probably in the future) having the scenario pass back a list of controls without the API having to cater for each explicitly, which would allow for other things like hardware EQ controls to be passed on to the scenario users if desired - this is useful when you get things like systems with multiple EQs.
This would complicate the API, though, and so I think it would be better left as-is until we get to the point of having something like a plugin for rewriting the controls for applications so they don't need explicit knowledge of scenarios. At that point it would only impact the scenario manager implementation which should deal with the complexity, at least externally.
I'm also thinking that it could be good to add functions to identify which PCMs on a card to use in the scenario, perhaps differentiated by quality or something. The CPU may have PCMs to multiple devices or with differing capabilities and may want to switch between them depending on scenario (and possibly stream).
Agreed, these all sound like good features for the future and will probably require some sponsorship to complete.
However, what we have atm is enough for most folks to get some working audio kcontrol scenarios configured for their devices.
Liam
On Thu, Oct 01, 2009 at 01:48:24PM +0100, Liam Girdwood wrote:
However, what we have atm is enough for most folks to get some working audio kcontrol scenarios configured for their devices.
Indeed, like I said I think most of what I suggested only makes sense with a transparent remapping of control names for application usability reasons if nothing else. PCM specification is the only thing I can see being useful right now but that's definitely something that could just be bolted on the side of the existing stuff, it shouldn't impact it directly.
What would be nice now would be to tweak the configuration file format to support at least the control remapping, though (so it'd only support a fixed list of target controls but it'd use the standard names, for example). That would mean that when someone does implement more advanced functionality there's more chance that the configuration file format would be static. Configuration file format changes are a pain to manage over upgrades, especially for distros, so if they can be headed off in advance that'd be a win.
On Thu, 2009-10-01 at 14:02 +0100, Mark Brown wrote:
On Thu, Oct 01, 2009 at 01:48:24PM +0100, Liam Girdwood wrote:
However, what we have atm is enough for most folks to get some working audio kcontrol scenarios configured for their devices.
Indeed, like I said I think most of what I suggested only makes sense with a transparent remapping of control names for application usability reasons if nothing else. PCM specification is the only thing I can see being useful right now but that's definitely something that could just be bolted on the side of the existing stuff, it shouldn't impact it directly.
What would be nice now would be to tweak the configuration file format to support at least the control remapping, though (so it'd only support a fixed list of target controls but it'd use the standard names, for example). That would mean that when someone does implement more advanced functionality there's more chance that the configuration file format would be static. Configuration file format changes are a pain to manage over upgrades, especially for distros, so if they can be headed off in advance that'd be a win.
I'm hoping this file format and remapping will be deprecated by the audio graphing interface (as discussed at LPC). We can still keep the same front end API to the apps though.
Liam
On Thu, Oct 01, 2009 at 03:06:32PM +0100, Liam Girdwood wrote:
I'm hoping this file format and remapping will be deprecated by the audio graphing interface (as discussed at LPC). We can still keep the same front end API to the apps though.
Hrm, I'll have to have a think about that. I'd always thought of the two as being complimentary - the graph saying what the connections are and where all the controls are while the scenario says what settings to use and which controls are for applications. I think I can see where you're coming from, though. We should discuss this tonight in person.
Hello.
On Thu, 2009-10-01 at 13:28, Mark Brown wrote:
On Thu, Oct 01, 2009 at 11:47:15AM +0200, Stefan Schmidt wrote:
It allows switching audio settings between scenarios or uses-cases like listening to music and answering an incoming phone call. Made of control aliasing for playback, capture master and switch as well as the option to post- and prefix a sequence of control changes avoiding pops and other unwanted noise. Some example programs will be available in alsa-utils.
Overall this looks good - it's certainly dealing with the issues we currently have with separating routing control and 'end user' controls.
Thanks. I'm sure there a are a lot things to improve, but I had the feeling that it is mature enough for the basic things.
+/** Scenario ID's
- Standard Scenario ID's - Add new scenarios at the end.
- */
Extra 's here.
That way? /** * Scenario ID's * * Standard Scenario ID's - Add new scenarios at the end. */
+/**
- snd_scenario_reload - reload and reparse scenario configuration
- @scn: scenario
- Reloads and reparses sound card scenario configuration.
- */
+int snd_scenario_reload(struct snd_scenario *scn);
I guess the idea is that in the future this will be removed and the scenario API will use inotify or similar to pick up changes.
Indeed. Certainly a Todo item for later enhancements. Do you guys prefer such todo items in the code or noted somewhere else?
One thing I think I'm missing with the API documentation here is a separation between the API used for setting up scenarios and the API used by random client applications - it's there, but it could be underlined a bit more. Probably putting the client application stuff at the top of the header file would help here since the functions that more people will use will be visible first.
Sure can do this.
+/* load scenario i */ +static int read_scenario_file(struct snd_scenario *scn) +{
- int fd, ret;
- FILE *f;
- char filename[MAX_FILE];
- struct scenario_info *info = &scn->scenario[scn->current_scenario];
- sprintf(filename, "%s/%s/%s", ALSA_SCN_DIR, scn->card_name,
info->file);snprintf().
Oops. Good point. Fixed in the next version.
if (strncmp(tbuf, "MasterPlaybackVolume", 20) == 0) {info->playback_volume_id = get_int(tbuf + 20);if (info->playback_volume_id < 0) {scn_error("%s: failed to get MasterPlaybackVolume\n",__func__);goto err;}continue;}I don't see anywhere which supplies a default value for all these control IDs. A memset() of the allocated block should deal with that I think since IIRC zero isn't a valid control ID. It'd also be nice to be able to use control names instead but that's something that could be added later.
ID's start with 1 so the memset approach should work here. Will be fixed in the next patch. Using the control name is also a good suggestion. I did this for the sequencer already. Will add it to the todo list.
Thinking out loud here but I'm wondering if it might make sense to replace the fixed list of controls that is there currently with something string based which can remap the control names if required. This would allow for (probably in the future) having the scenario pass back a list of controls without the API having to cater for each explicitly, which would allow for other things like hardware EQ controls to be passed on to the scenario users if desired - this is useful when you get things like systems with multiple EQs.
This would complicate the API, though, and so I think it would be better left as-is until we get to the point of having something like a plugin for rewriting the controls for applications so they don't need explicit knowledge of scenarios. At that point it would only impact the scenario manager implementation which should deal with the complexity, at least externally.
I'm also thinking that it could be good to add functions to identify which PCMs on a card to use in the scenario, perhaps differentiated by quality or something. The CPU may have PCMs to multiple devices or with differing capabilities and may want to switch between them depending on scenario (and possibly stream).
I aggree with you and Liam here. Wondering again where to put the todo list. Within the code, as separate text file, in the wiki, ...
regards Stefan Schmidt
On Thu, Oct 01, 2009 at 03:19:48PM +0200, Stefan Schmidt wrote:
On Thu, 2009-10-01 at 13:28, Mark Brown wrote:
+/** Scenario ID's
- Standard Scenario ID's - Add new scenarios at the end.
- */
Extra 's here.
That way? /**
- Scenario ID's
- Standard Scenario ID's - Add new scenarios at the end.
*/
There shouldn't be any 's in the above at all ("Scenarios IDs" instead), those are just plain plurals - just a trivial grammar nit.
I guess the idea is that in the future this will be removed and the scenario API will use inotify or similar to pick up changes.
Indeed. Certainly a Todo item for later enhancements. Do you guys prefer such todo items in the code or noted somewhere else?
I have no great preference either way.
Hello.
On Thu, 2009-10-01 at 14:24, Mark Brown wrote:
On Thu, Oct 01, 2009 at 03:19:48PM +0200, Stefan Schmidt wrote:
On Thu, 2009-10-01 at 13:28, Mark Brown wrote:
+/** Scenario ID's
- Standard Scenario ID's - Add new scenarios at the end.
- */
Extra 's here.
That way? /**
- Scenario ID's
- Standard Scenario ID's - Add new scenarios at the end.
*/
There shouldn't be any 's in the above at all ("Scenarios IDs" instead), those are just plain plurals - just a trivial grammar nit.
Ah, non-native speaker problem. :) Fixed.
I guess the idea is that in the future this will be removed and the scenario API will use inotify or similar to pick up changes.
Indeed. Certainly a Todo item for later enhancements. Do you guys prefer such todo items in the code or noted somewhere else?
I have no great preference either way.
Hmm, will think about it.
regards Stefan Schmidt
+/** QOS + * + * Defines Audio Quality of Service + */ +#define SND_QOS_HIFI 0 +#define SND_QOS_VOICE 1 +#define SND_QOS_SYSTEM 2
My $0.02 comment is that I don't know what QOS entails. I have no issue with all the other defines, but this one isn't quite clear. What exactly is supposed to happen if I set the QOS to voice? A lower SNR for voice? A lower latency for voice with a lower group delay? A lower oversampling ratio? An optimized routing for voice? All of the above?
+#define SND_SCN_PHONECALL_HANDSET "phonecall handset" +#define SND_SCN_PHONECALL_HEADSET "phonecall headset" +#define SND_SCN_PHONECALL_BLUETOOTH "phonecall bluetooth" +#define SND_SCN_PHONECALL_IP "phonecall ip"
Same here, this is ambiguous, you can have an IP call over bluetooth. Which scenario would you use then? I guess if you want the scenarios to be used by apps in a portable way, a clearer definition of each scenario is probably needed? Cheers, - Pierre
On Thu, 2009-10-01 at 08:39 -0500, pl bossart wrote:
+/** QOS
- Defines Audio Quality of Service
- */
+#define SND_QOS_HIFI 0 +#define SND_QOS_VOICE 1 +#define SND_QOS_SYSTEM 2
My $0.02 comment is that I don't know what QOS entails. I have no issue with all the other defines, but this one isn't quite clear. What exactly is supposed to happen if I set the QOS to voice? A lower SNR for voice? A lower latency for voice with a lower group delay? A lower oversampling ratio? An optimized routing for voice? All of the above?
It's only about power usage here. i.e. run CODEC with slower clocks internally, may lower SNR.
So maybe
define SND_POWER_QOS_HIFI 0
+#define SND_SCN_PHONECALL_HANDSET "phonecall handset" +#define SND_SCN_PHONECALL_HEADSET "phonecall headset" +#define SND_SCN_PHONECALL_BLUETOOTH "phonecall bluetooth" +#define SND_SCN_PHONECALL_IP "phonecall ip"
Same here, this is ambiguous, you can have an IP call over bluetooth. Which scenario would you use then? I guess if you want the scenarios to be used by apps in a portable way, a clearer definition of each scenario is probably needed?
These are ambiguous, I guess we need to have
#define SND_SCN_PHONECALL_GSM_HANDSET "phonecall gsm handset" #define SND_SCN_PHONECALL_BT_HANDSET "phonecall bt handset" #define SND_SCN_PHONECALL_IP_HANDSET "phonecall ip handset"
For gsm, bluetooth and VOIP handset phone calls respectively.
Thanks
Liam
Hello.
On Thu, 2009-10-01 at 08:39, pl bossart wrote:
+/** QOS
- Defines Audio Quality of Service
- */
+#define SND_QOS_HIFI 0 +#define SND_QOS_VOICE 1 +#define SND_QOS_SYSTEM 2
My $0.02 comment is that I don't know what QOS entails. I have no issue with all the other defines, but this one isn't quite clear. What exactly is supposed to happen if I set the QOS to voice? A lower SNR for voice? A lower latency for voice with a lower group delay? A lower oversampling ratio? An optimized routing for voice? All of the above?
Good point. The key is here that some systems offer lower quality audio which also has a lower power consumption. That would save power when being on a voice call but give you the best quality when listening to your music.
+#define SND_SCN_PHONECALL_HANDSET "phonecall handset" +#define SND_SCN_PHONECALL_HEADSET "phonecall headset" +#define SND_SCN_PHONECALL_BLUETOOTH "phonecall bluetooth" +#define SND_SCN_PHONECALL_IP "phonecall ip"
Same here, this is ambiguous, you can have an IP call over bluetooth. Which scenario would you use then? I guess if you want the scenarios to be used by apps in a portable way, a clearer definition of each scenario is probably needed?
I removed the defines here. We don't use them and should re-think and define them when we actually implement its users.
Thanks for your comments.
regards Stefan Schmidt
Hello.
On Thu, 2009-10-01 at 15:19, Stefan Schmidt wrote:
On Thu, 2009-10-01 at 13:28, Mark Brown wrote:
One thing I think I'm missing with the API documentation here is a separation between the API used for setting up scenarios and the API used by random client applications - it's there, but it could be underlined a bit more. Probably putting the client application stuff at the top of the header file would help here since the functions that more people will use will be visible first.
Sure can do this.
It now has the following order: snd_scenario_list() snd_scenario_set_scn() snd_scenario_get_scn() snd_scenario_get_master_playback_volume() snd_scenario_get_master_playback_switch() snd_scenario_get_master_capture_volume() snd_scenario_get_master_capture_switch() snd_scenario_set_qos() snd_scenario_get_qos() snd_scenario_open() snd_scenario_reload() snd_scenario_close() snd_scenario_dump()
That should match what people are looking for: 1) what scenarios are available 2) get and set them 3) what else can I do? 4) boring setup stuff.
For an updated patch with all your comments inlcuded see below.
Hello.
On Thu, 2009-10-01 at 17:08, Stefan Schmidt wrote:
For an updated patch with all your comments inlcuded see below.
Here is another update on this patch. Fixed a problem I introduced in the last iteration and changed the config tokens from MasterPlaybackVolume to Master Playback Volume, etc. That's based on a siggestion from Mark which we had offlist. Let me quote it here to explain the good reasoning behind it.
--- snip --- If we reuse the standard ALSA control names as the tokens then someone could come along in the future and extend the library to allow any control to be included in the scenarios and existing scenario setups would just work transparently without having the configuration have multiple ways of specifying the same thing. An extension like that would move any questions about what can be included in the scenario out of the library which should make it easier to maintian and less likely to restrict the odd things that some users will inevitably wish to do with it. --- snap ---
On Mon, 5 Oct 2009, Stefan Schmidt wrote:
Hello.
On Thu, 2009-10-01 at 17:08, Stefan Schmidt wrote:
For an updated patch with all your comments inlcuded see below.
Here is another update on this patch. Fixed a problem I introduced in the last iteration and changed the config tokens from MasterPlaybackVolume to Master Playback Volume, etc. That's based on a siggestion from Mark which we had offlist. Let me quote it here to explain the good reasoning behind it.
Question: What's the IDs (integers) returned with snd_scenario_get_master_playback_volume() etc. functions?
Please, include also some test configuration files to review the configuration syntax.
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
Hello.
On Mon, 2009-10-05 at 11:14, Jaroslav Kysela wrote:
On Mon, 5 Oct 2009, Stefan Schmidt wrote:
On Thu, 2009-10-01 at 17:08, Stefan Schmidt wrote:
For an updated patch with all your comments inlcuded see below.
Here is another update on this patch. Fixed a problem I introduced in the last iteration and changed the config tokens from MasterPlaybackVolume to Master Playback Volume, etc. That's based on a siggestion from Mark which we had offlist. Let me quote it here to explain the good reasoning behind it.
Question: What's the IDs (integers) returned with snd_scenario_get_master_playback_volume() etc. functions?
That is part of the code Liam wrote, but let me answer it (and maybe Liam chime in if it is wrong or needs more explanation).
It's just the numid of an alsa control which should be used as master playback, etc in this scenario. It aliases the function we can use with ascenario to the real ones in alsa which can bedifferent for different scenarios.
Please, include also some test configuration files to review the configuration syntax.
Attached. default.conf is the main configuration file located at /etc/alsa/scenario/default.conf
playback_hs is an example of scenario syntax and playback_hs_sequence is an example of the sequence syntax. Both file names are referenced from default.conf and located under /etc/alsa/scenario/default/
regards Stefan Schmidt
Hi,
Some ideas, proposal for further discussion...
On Mon, 5 Oct 2009, Stefan Schmidt wrote:
Hello.
On Mon, 2009-10-05 at 11:14, Jaroslav Kysela wrote:
On Mon, 5 Oct 2009, Stefan Schmidt wrote:
On Thu, 2009-10-01 at 17:08, Stefan Schmidt wrote:
For an updated patch with all your comments inlcuded see below.
Here is another update on this patch. Fixed a problem I introduced in the last iteration and changed the config tokens from MasterPlaybackVolume to Master Playback Volume, etc. That's based on a siggestion from Mark which we had offlist. Let me quote it here to explain the good reasoning behind it.
Question: What's the IDs (integers) returned with snd_scenario_get_master_playback_volume() etc. functions?
That is part of the code Liam wrote, but let me answer it (and maybe Liam chime in if it is wrong or needs more explanation).
It's just the numid of an alsa control which should be used as master playback, etc in this scenario. It aliases the function we can use with ascenario to the real ones in alsa which can bedifferent for different scenarios.
It's not very clear. The exported functions should be documented at least with doxygen.
Also, I would move all _get_ functions to one with this interface:
#define SND_SCENARIO_KEY_KCTL_MASTER_PLAYBACK_VOLUME 1 .....
int snd_scenario_get(scenario, int key, void *result);
It will allow us to extend this interface without adding many functions in future, something like:
#define SND_SCENARIO_KEY_PCM_PLAYBACK 11 .....
Also, using numid's is not much ideal if something changes in the driver. I would prefer to use standard ctl IDs (string, index, interface - result should be snd_ctl_elem_id_t).
Please, include also some test configuration files to review the configurationsyntax.
Attached. default.conf is the main configuration file located at /etc/alsa/scenario/default.conf
Again, I would make configuration syntax compatible with other alsa-lib configuration files. The changes in the configuration files will be small and it will allow us to rewrite the code using parsers in alsa-lib.
Something like:
default.conf:
scenario."playback speaker" { file playback_spk qos HIFI "Master Playback Volume" 8 # should be rather something like: kcontrol.'Master Playback Volume' "'Master Playback Volume':33" # or a short form might be implemented (:33 is ctl_id index, ctl_id.name # would be copied from the variable key name): kcontrol.'Master Playback Volume' ":33" ... }
scenario."playback headphones" { ... }
About "playback_hs_*" files. It seems that the purpose of files is quite similar. I don't understand the reason to have 2 different descriptions.
Also, I would like to consider moving the parser from alsactl initialization code (see alsactl_init.c in alsa-utils package) to alsa-lib and use this parser also for these files. It gives much flexibility, although udev-like syntax is not ideal, I know. But we have full documentation in alsactl_init.xml, at least.
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
At Tue, 6 Oct 2009 10:23:56 +0200 (CEST), Jaroslav Kysela wrote:
Hi,
Some ideas, proposal for further discussion...
On Mon, 5 Oct 2009, Stefan Schmidt wrote:
Hello.
On Mon, 2009-10-05 at 11:14, Jaroslav Kysela wrote:
On Mon, 5 Oct 2009, Stefan Schmidt wrote:
On Thu, 2009-10-01 at 17:08, Stefan Schmidt wrote:
For an updated patch with all your comments inlcuded see below.
Here is another update on this patch. Fixed a problem I introduced in the last iteration and changed the config tokens from MasterPlaybackVolume to Master Playback Volume, etc. That's based on a siggestion from Mark which we had offlist. Let me quote it here to explain the good reasoning behind it.
Question: What's the IDs (integers) returned with snd_scenario_get_master_playback_volume() etc. functions?
That is part of the code Liam wrote, but let me answer it (and maybe Liam chime in if it is wrong or needs more explanation).
It's just the numid of an alsa control which should be used as master playback, etc in this scenario. It aliases the function we can use with ascenario to the real ones in alsa which can bedifferent for different scenarios.
It's not very clear. The exported functions should be documented at least with doxygen.
Also, I would move all _get_ functions to one with this interface:
#define SND_SCENARIO_KEY_KCTL_MASTER_PLAYBACK_VOLUME 1 .....
int snd_scenario_get(scenario, int key, void *result);
It will allow us to extend this interface without adding many functions in future, something like:
#define SND_SCENARIO_KEY_PCM_PLAYBACK 11 .....
I find it's good to have a generic interface, too, but a void pointer is discouraging. It's too ambiguous and inconvenient when you use it ("What type do I have to pass for SND_SCENARIO_XXX on earth???"). OTOH, union is also no good about type-binding and even less extensibility...
Also, using numid's is not much ideal if something changes in the driver. I would prefer to use standard ctl IDs (string, index, interface - result should be snd_ctl_elem_id_t).
Agreed.
Please, include also some test configuration files to review the configurationsyntax.
Attached. default.conf is the main configuration file located at /etc/alsa/scenario/default.conf
Again, I would make configuration syntax compatible with other alsa-lib configuration files. The changes in the configuration files will be small and it will allow us to rewrite the code using parsers in alsa-lib.
Something like:
default.conf:
scenario."playback speaker" { file playback_spk qos HIFI "Master Playback Volume" 8 # should be rather something like: kcontrol.'Master Playback Volume' "'Master Playback Volume':33" # or a short form might be implemented (:33 is ctl_id index, ctl_id.name # would be copied from the variable key name): kcontrol.'Master Playback Volume' ":33" ... }
scenario."playback headphones" { ... }
About "playback_hs_*" files. It seems that the purpose of files is quite similar. I don't understand the reason to have 2 different descriptions.
I personally have no particular preference regarding the configuration syntax between these two, but...
Also, I would like to consider moving the parser from alsactl initialization code (see alsactl_init.c in alsa-utils package) to alsa-lib and use this parser also for these files. It gives much flexibility, although udev-like syntax is not ideal, I know.
Not ideal, it's awful :) So please don't spread it over...
Takashi
On Tue, 6 Oct 2009, Takashi Iwai wrote:
At Tue, 6 Oct 2009 10:23:56 +0200 (CEST), Jaroslav Kysela wrote:
Hi,
Some ideas, proposal for further discussion...
On Mon, 5 Oct 2009, Stefan Schmidt wrote:
Hello.
On Mon, 2009-10-05 at 11:14, Jaroslav Kysela wrote:
On Mon, 5 Oct 2009, Stefan Schmidt wrote:
On Thu, 2009-10-01 at 17:08, Stefan Schmidt wrote:
For an updated patch with all your comments inlcuded see below.
Here is another update on this patch. Fixed a problem I introduced in the last iteration and changed the config tokens from MasterPlaybackVolume to Master Playback Volume, etc. That's based on a siggestion from Mark which we had offlist. Let me quote it here to explain the good reasoning behind it.
Question: What's the IDs (integers) returned with snd_scenario_get_master_playback_volume() etc. functions?
That is part of the code Liam wrote, but let me answer it (and maybe Liam chime in if it is wrong or needs more explanation).
It's just the numid of an alsa control which should be used as master playback, etc in this scenario. It aliases the function we can use with ascenario to the real ones in alsa which can bedifferent for different scenarios.
It's not very clear. The exported functions should be documented at least with doxygen.
Also, I would move all _get_ functions to one with this interface:
#define SND_SCENARIO_KEY_KCTL_MASTER_PLAYBACK_VOLUME 1 .....
int snd_scenario_get(scenario, int key, void *result);
It will allow us to extend this interface without adding many functions in future, something like:
#define SND_SCENARIO_KEY_PCM_PLAYBACK 11 .....
I find it's good to have a generic interface, too, but a void pointer is discouraging. It's too ambiguous and inconvenient when you use it ("What type do I have to pass for SND_SCENARIO_XXX on earth???").
It might be solved with inline functions in header files to cover all types. My opinion is to not use so much function in alsa-lib for each trivial interfaces.
I personally have no particular preference regarding the configuration syntax between these two, but...
Also, I would like to consider moving the parser from alsactl initialization code (see alsactl_init.c in alsa-utils package) to alsa-lib and use this parser also for these files. It gives much flexibility, although udev-like syntax is not ideal, I know.
Not ideal, it's awful :) So please don't spread it over...
It's not so awful. The base syntax is very clear in form key->value binding. What's awful on this?
CTL{reset}="mixer" CTL{name}="Master Playback Volume", CTL{value}="-21dB" CTL{name}="Master Playback Switch", CTL{value}="on" CTL{name}="Headphone Playback Switch", CTL{value}="on,on" CTL{name}="Front Playback Volume", CTL{value}="-29dB,-29dB" CTL{name}="Front Playback Switch", CTL{value}="on,on" CTL{name}="PCM Playback Volume", CTL{value}="0dB,0dB"
Solving complex tasks requires complex description. I'm open to discuss things to make this udev-like syntax more nice. The most irritating thing in this syntax are build-in commands for me, but having another special key for these actions are not a big win, because configuration writers must learn them.
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
At Tue, 6 Oct 2009 11:00:18 +0200 (CEST), Jaroslav Kysela wrote:
On Tue, 6 Oct 2009, Takashi Iwai wrote:
At Tue, 6 Oct 2009 10:23:56 +0200 (CEST), Jaroslav Kysela wrote:
Hi,
Some ideas, proposal for further discussion...
On Mon, 5 Oct 2009, Stefan Schmidt wrote:
Hello.
On Mon, 2009-10-05 at 11:14, Jaroslav Kysela wrote:
On Mon, 5 Oct 2009, Stefan Schmidt wrote:
On Thu, 2009-10-01 at 17:08, Stefan Schmidt wrote: > > For an updated patch with all your comments inlcuded see below.
Here is another update on this patch. Fixed a problem I introduced in the last iteration and changed the config tokens from MasterPlaybackVolume to Master Playback Volume, etc. That's based on a siggestion from Mark which we had offlist. Let me quote it here to explain the good reasoning behind it.
Question: What's the IDs (integers) returned with snd_scenario_get_master_playback_volume() etc. functions?
That is part of the code Liam wrote, but let me answer it (and maybe Liam chime in if it is wrong or needs more explanation).
It's just the numid of an alsa control which should be used as master playback, etc in this scenario. It aliases the function we can use with ascenario to the real ones in alsa which can bedifferent for different scenarios.
It's not very clear. The exported functions should be documented at least with doxygen.
Also, I would move all _get_ functions to one with this interface:
#define SND_SCENARIO_KEY_KCTL_MASTER_PLAYBACK_VOLUME 1 .....
int snd_scenario_get(scenario, int key, void *result);
It will allow us to extend this interface without adding many functions in future, something like:
#define SND_SCENARIO_KEY_PCM_PLAYBACK 11 .....
I find it's good to have a generic interface, too, but a void pointer is discouraging. It's too ambiguous and inconvenient when you use it ("What type do I have to pass for SND_SCENARIO_XXX on earth???").
It might be solved with inline functions in header files to cover all types. My opinion is to not use so much function in alsa-lib for each trivial interfaces.
I personally have no particular preference regarding the configuration syntax between these two, but...
Also, I would like to consider moving the parser from alsactl initialization code (see alsactl_init.c in alsa-utils package) to alsa-lib and use this parser also for these files. It gives much flexibility, although udev-like syntax is not ideal, I know.
Not ideal, it's awful :) So please don't spread it over...
It's not so awful. The base syntax is very clear in form key->value binding. What's awful on this?
CTL{reset}="mixer" CTL{name}="Master Playback Volume", CTL{value}="-21dB" CTL{name}="Master Playback Switch", CTL{value}="on" CTL{name}="Headphone Playback Switch", CTL{value}="on,on" CTL{name}="Front Playback Volume", CTL{value}="-29dB,-29dB" CTL{name}="Front Playback Switch", CTL{value}="on,on" CTL{name}="PCM Playback Volume", CTL{value}="0dB,0dB"
This is all fine, but as you wrote below:
Solving complex tasks requires complex description.
This is the reality. The alsactl's default config shows its limit.
I'm open to discuss things to make this udev-like syntax more nice. The most irritating thing in this syntax are build-in commands for me, but having another special key for these actions are not a big win, because configuration writers must learn them.
The crazy goto is also painful. It reminds me the day of MS BASIC.
thanks,
Takashi
On Tue, 6 Oct 2009, Takashi Iwai wrote:
binding. What's awful on this?
CTL{reset}="mixer" CTL{name}="Master Playback Volume", CTL{value}="-21dB" CTL{name}="Master Playback Switch", CTL{value}="on" CTL{name}="Headphone Playback Switch", CTL{value}="on,on" CTL{name}="Front Playback Volume", CTL{value}="-29dB,-29dB" CTL{name}="Front Playback Switch", CTL{value}="on,on" CTL{name}="PCM Playback Volume", CTL{value}="0dB,0dB"
This is all fine, but as you wrote below:
Solving complex tasks requires complex description.
This is the reality. The alsactl's default config shows its limit.
I'm open to discuss things to make this udev-like syntax more nice. The most irritating thing in this syntax are build-in commands for me, but having another special key for these actions are not a big win, because configuration writers must learn them.
The crazy goto is also painful. It reminds me the day of MS BASIC.
I'm going to change the CTL build-in command to something more readable like:
Old syntax:
CTL{reset}="mixer" CTL{name}="Playback Volume",PROGRAM=="__ctl_search", \ CTL{values}="$env{pvolume}",RESULT!="0",CTL{values}="$env{ppercent}"
New syntax:
CTL{reset}="mixer" CTL{name}="Playback Volume",CTL{do_search}=="1", \ CTL{values}="$env{pvolume}",RESULT!="0",CTL{values}="$env{ppercent}"
But regarding goto, I'm really stuck. We can add some block control:
BEGIN="" .. END="" (looks like a goto anyway)
Or skip some lines:
SKIPLINES="3"
But these solutions do not bring much readability. Another possibility (compromise) is to use a goto command to skip all lines to next label to reduce usage of named labels (which is not a big fun at least in init/default file):
GOTO="" .... LABEL=""
It works without any code change in current parser.
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
At Wed, 7 Oct 2009 10:10:06 +0200 (CEST), Jaroslav Kysela wrote:
On Tue, 6 Oct 2009, Takashi Iwai wrote:
binding. What's awful on this?
CTL{reset}="mixer" CTL{name}="Master Playback Volume", CTL{value}="-21dB" CTL{name}="Master Playback Switch", CTL{value}="on" CTL{name}="Headphone Playback Switch", CTL{value}="on,on" CTL{name}="Front Playback Volume", CTL{value}="-29dB,-29dB" CTL{name}="Front Playback Switch", CTL{value}="on,on" CTL{name}="PCM Playback Volume", CTL{value}="0dB,0dB"
This is all fine, but as you wrote below:
Solving complex tasks requires complex description.
This is the reality. The alsactl's default config shows its limit.
I'm open to discuss things to make this udev-like syntax more nice. The most irritating thing in this syntax are build-in commands for me, but having another special key for these actions are not a big win, because configuration writers must learn them.
The crazy goto is also painful. It reminds me the day of MS BASIC.
I'm going to change the CTL build-in command to something more readable like:
Old syntax:
CTL{reset}="mixer" CTL{name}="Playback Volume",PROGRAM=="__ctl_search", \ CTL{values}="$env{pvolume}",RESULT!="0",CTL{values}="$env{ppercent}"
New syntax:
CTL{reset}="mixer" CTL{name}="Playback Volume",CTL{do_search}=="1", \ CTL{values}="$env{pvolume}",RESULT!="0",CTL{values}="$env{ppercent}"
But regarding goto, I'm really stuck. We can add some block control:
BEGIN="" .. END="" (looks like a goto anyway)
Hm, still not much improvement, indeed.
Or skip some lines:
SKIPLINES="3"
Oh, no! I don't want to debug a DSP assembly :)
But these solutions do not bring much readability. Another possibility (compromise) is to use a goto command to skip all lines to next label to reduce usage of named labels (which is not a big fun at least in init/default file):
GOTO="" .... LABEL=""
It works without any code change in current parser.
I'm afraid that udev config is way too simple for the conditionals. And we do need conditionals to handle many cases. XML can do better for such a purpose, but moving to XML now is another question...
thanks,
Takashi
On Wed, 7 Oct 2009, Takashi Iwai wrote:
But regarding goto, I'm really stuck. We can add some block control:
BEGIN="" .. END="" (looks like a goto anyway)
Hm, still not much improvement, indeed.
Or skip some lines:
SKIPLINES="3"
Oh, no! I don't want to debug a DSP assembly :)
But these solutions do not bring much readability. Another possibility (compromise) is to use a goto command to skip all lines to next label to reduce usage of named labels (which is not a big fun at least in init/default file):
GOTO="" .... LABEL=""
It works without any code change in current parser.
I'm afraid that udev config is way too simple for the conditionals.
There are plenty conditionals in udev configuration files. That was main reason to use this syntax for alsactl.
And we do need conditionals to handle many cases. XML can do better for such a purpose, but moving to XML now is another question...
Really better? XML is good for static data but not for non-linear execution. Take look to HTML/Javascript. Javascript is doing the non-linear stuff and eventually modifies XML at runtime. We have similar situation in alsa-lib. The syntax structure is changed at runtime depending on external data. But it requires some system resources (memory).
Things why I chose udev-like syntax:
- well known "key operator value" syntax - very easy to parse - parser reads and executes code directly without storing the source or portions to memory (really small memory usage) - normal conditions are really small in syntax files
If only the goto command is major drawback, I can live with it.
We need to take account the useability and the implementation complexity. I think a compromise is essential.
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
At Wed, 7 Oct 2009 11:47:19 +0200 (CEST), Jaroslav Kysela wrote:
On Wed, 7 Oct 2009, Takashi Iwai wrote:
But regarding goto, I'm really stuck. We can add some block control:
BEGIN="" .. END="" (looks like a goto anyway)
Hm, still not much improvement, indeed.
Or skip some lines:
SKIPLINES="3"
Oh, no! I don't want to debug a DSP assembly :)
But these solutions do not bring much readability. Another possibility (compromise) is to use a goto command to skip all lines to next label to reduce usage of named labels (which is not a big fun at least in init/default file):
GOTO="" .... LABEL=""
It works without any code change in current parser.
I'm afraid that udev config is way too simple for the conditionals.
There are plenty conditionals in udev configuration files. That was main reason to use this syntax for alsactl.
And we do need conditionals to handle many cases. XML can do better for such a purpose, but moving to XML now is another question...
Really better?
I don't know exactly, honestly. I don't like XML much either :)
XML is good for static data but not for non-linear execution. Take look to HTML/Javascript. Javascript is doing the non-linear stuff and eventually modifies XML at runtime. We have similar situation in alsa-lib. The syntax structure is changed at runtime depending on external data. But it requires some system resources (memory).
Things why I chose udev-like syntax:
- well known "key operator value" syntax
- very easy to parse
- parser reads and executes code directly without storing the source or portions to memory (really small memory usage)
- normal conditions are really small in syntax files
But fairly hard to read. Do you remember the very trivial syntax errors we hadn't noticed over months? It was just because of the bad readability of this config file.
If only the goto command is major drawback, I can live with it.
We need to take account the useability and the implementation complexity. I think a compromise is essential.
I don't care much right now about alsactl init stuff, supposing that we won't add likely any bigger stuff. But I'm against to deploy it over other areas, especially for new ones.
thanks,
Takashi
[Forgot to follow up the first part]
At Tue, 6 Oct 2009 11:00:18 +0200 (CEST), Jaroslav Kysela wrote:
On Tue, 6 Oct 2009, Takashi Iwai wrote:
At Tue, 6 Oct 2009 10:23:56 +0200 (CEST), Jaroslav Kysela wrote:
Hi,
Some ideas, proposal for further discussion...
On Mon, 5 Oct 2009, Stefan Schmidt wrote:
Hello.
On Mon, 2009-10-05 at 11:14, Jaroslav Kysela wrote:
On Mon, 5 Oct 2009, Stefan Schmidt wrote:
On Thu, 2009-10-01 at 17:08, Stefan Schmidt wrote: > > For an updated patch with all your comments inlcuded see below.
Here is another update on this patch. Fixed a problem I introduced in the last iteration and changed the config tokens from MasterPlaybackVolume to Master Playback Volume, etc. That's based on a siggestion from Mark which we had offlist. Let me quote it here to explain the good reasoning behind it.
Question: What's the IDs (integers) returned with snd_scenario_get_master_playback_volume() etc. functions?
That is part of the code Liam wrote, but let me answer it (and maybe Liam chime in if it is wrong or needs more explanation).
It's just the numid of an alsa control which should be used as master playback, etc in this scenario. It aliases the function we can use with ascenario to the real ones in alsa which can bedifferent for different scenarios.
It's not very clear. The exported functions should be documented at least with doxygen.
Also, I would move all _get_ functions to one with this interface:
#define SND_SCENARIO_KEY_KCTL_MASTER_PLAYBACK_VOLUME 1 .....
int snd_scenario_get(scenario, int key, void *result);
It will allow us to extend this interface without adding many functions in future, something like:
#define SND_SCENARIO_KEY_PCM_PLAYBACK 11 .....
I find it's good to have a generic interface, too, but a void pointer is discouraging. It's too ambiguous and inconvenient when you use it ("What type do I have to pass for SND_SCENARIO_XXX on earth???").
It might be solved with inline functions in header files to cover all types.
Yes, but then why not providing functions for all types from the beginning? There shouldn't be so many different types.
A void pointer is nice for closures or an API like X protocol, but I find no big merit in a small API set like scenario.
Well, it's just an implementation detail, though.
My opinion is to not use so much function in alsa-lib for each trivial interfaces.
We do have it per design, no?
The question is rather how often a "trivial" interface would be added in future. If the expected extension is limited, we don't have to worry about this too much. Just adding a few new functions would be much easier to maintain.
thanks,
Takashi
On Tue, 6 Oct 2009, Takashi Iwai wrote:
[Forgot to follow up the first part]
At Tue, 6 Oct 2009 11:00:18 +0200 (CEST), Jaroslav Kysela wrote:
On Tue, 6 Oct 2009, Takashi Iwai wrote:
At Tue, 6 Oct 2009 10:23:56 +0200 (CEST), Jaroslav Kysela wrote:
Hi,
Some ideas, proposal for further discussion...
On Mon, 5 Oct 2009, Stefan Schmidt wrote:
Hello.
On Mon, 2009-10-05 at 11:14, Jaroslav Kysela wrote:
On Mon, 5 Oct 2009, Stefan Schmidt wrote:
> On Thu, 2009-10-01 at 17:08, Stefan Schmidt wrote: >> >> For an updated patch with all your comments inlcuded see below. > > Here is another update on this patch. Fixed a problem I introduced in the last > iteration and changed the config tokens from MasterPlaybackVolume to Master > Playback Volume, etc. That's based on a siggestion from Mark which we had > offlist. Let me quote it here to explain the good reasoning behind it.
Question: What's the IDs (integers) returned with snd_scenario_get_master_playback_volume() etc. functions?
That is part of the code Liam wrote, but let me answer it (and maybe Liam chime in if it is wrong or needs more explanation).
It's just the numid of an alsa control which should be used as master playback, etc in this scenario. It aliases the function we can use with ascenario to the real ones in alsa which can bedifferent for different scenarios.
It's not very clear. The exported functions should be documented at least with doxygen.
Also, I would move all _get_ functions to one with this interface:
#define SND_SCENARIO_KEY_KCTL_MASTER_PLAYBACK_VOLUME 1 .....
int snd_scenario_get(scenario, int key, void *result);
It will allow us to extend this interface without adding many functions in future, something like:
#define SND_SCENARIO_KEY_PCM_PLAYBACK 11 .....
I find it's good to have a generic interface, too, but a void pointer is discouraging. It's too ambiguous and inconvenient when you use it ("What type do I have to pass for SND_SCENARIO_XXX on earth???").
It might be solved with inline functions in header files to cover all types.
Yes, but then why not providing functions for all types from the beginning? There shouldn't be so many different types.
Merging functions with same types might be a good compromise between one function and separate functions for each values.
So at least, the API basics is now clear. I'll prepare a branch in alsa-lib GIT tree to have a start point for first accepted implementation.
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
On Tue, 6 Oct 2009, Jaroslav Kysela wrote:
So at least, the API basics is now clear. I'll prepare a branch in alsa-lib GIT tree to have a start point for first accepted implementation.
The updated header file is now at:
http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=include/ascenario.h;h=0...
My other proposals:
- remove all SND_SCN_ defines - they are quite use-case specific - the names should be just documented somewhere - represent QoS with string (name) rather than integer - QoS is also quite use-case specific - SND_SCN_INT_QOS may be SND_SCN_INT_QOS_POWER ? - what about to remove QOS and add universal parameter interface? something like:
int snd_scenario_set_parameter(snd_scenario_t *scn, char *type, char *value);
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
At Tue, 6 Oct 2009 15:33:39 +0200 (CEST), Jaroslav Kysela wrote:
On Tue, 6 Oct 2009, Jaroslav Kysela wrote:
So at least, the API basics is now clear. I'll prepare a branch in alsa-lib GIT tree to have a start point for first accepted implementation.
The updated header file is now at:
http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=include/ascenario.h;h=0...
My other proposals:
- remove all SND_SCN_ defines - they are quite use-case specific
- the names should be just documented somewhere
Keeping the basic and common strings makes sense, I think. After all, the apps need to use the correct strings.
- represent QoS with string (name) rather than integer - QoS is also quite use-case specific
- SND_SCN_INT_QOS may be SND_SCN_INT_QOS_POWER ?
- what about to remove QOS and add universal parameter interface? something like:
int snd_scenario_set_parameter(snd_scenario_t *scn, char *type, char *value);
This sounds not bad, but let's argue first what parameters could be added in future.
thanks,
Takashi
Hello.
On Tue, 2009-10-06 at 15:46, Takashi Iwai wrote:
At Tue, 6 Oct 2009 15:33:39 +0200 (CEST), Jaroslav Kysela wrote:
On Tue, 6 Oct 2009, Jaroslav Kysela wrote:
My other proposals:
- remove all SND_SCN_ defines - they are quite use-case specific
- the names should be just documented somewhere
Keeping the basic and common strings makes sense, I think. After all, the apps need to use the correct strings.
What would you put under basic and common strings? Remove all that are gsm, bt or ip realted for now?
- represent QoS with string (name) rather than integer - QoS is also quite use-case specific
Hmm, what would that buy us?
- SND_SCN_INT_QOS may be SND_SCN_INT_QOS_POWER ?
Makes sense, done.
- what about to remove QOS and add universal parameter interface? something like:
int snd_scenario_set_parameter(snd_scenario_t *scn, char *type, char *value);
This sounds not bad, but let's argue first what parameters could be added in future.
Any concrete examples here? I'm not against a more generic interface, but I have problems to see good use cases for it.
I implemented the snd_scenario_get_control_id() function you favoured over the explicit functions. Also two more small patches posted in a new thread.
regards Stefan Schmidt
Hello.
On Tue, 2009-10-06 at 15:33, Jaroslav Kysela wrote:
On Tue, 6 Oct 2009, Jaroslav Kysela wrote:
So at least, the API basics is now clear. I'll prepare a branch in alsa-lib GIT tree to have a start point for first accepted implementation.
The updated header file is now at:
http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=include/ascenario.h;h=0...
My other proposals:
- remove all SND_SCN_ defines - they are quite use-case specific
- the names should be just documented somewhere
- represent QoS with string (name) rather than integer - QoS is also quite use-case specific
- SND_SCN_INT_QOS may be SND_SCN_INT_QOS_POWER ?
- what about to remove QOS and add universal parameter interface? something like:
int snd_scenario_set_parameter(snd_scenario_t *scn, char *type, char *value);
I will work on these items over the next days. Patches will be based on the ascenrio branch and get posted here.
regards Stefan Schmidt
Hello.
Just three small fixes to let the current ascenario branch at least compile. Real stuff will come over the next days. Time to aall it a day now.
[PATCH 1/3] ascenario: Fix typedef for snd_scenario_t [PATCH 2/3] ascenario: Fix usage of new typedefed snd_scenario_t [PATCH 3/3] ascenario: Catchup with function declaration changes.
regards Stefan Schmidt
From: Stefan Schmidt stefan@slimlogic.co.uk
Signed-off-by: Stefan Schmidt stefan@slimlogic.co.uk --- include/ascenario.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/ascenario.h b/include/ascenario.h index 003eabe..0c5acb9 100644 --- a/include/ascenario.h +++ b/include/ascenario.h @@ -128,7 +128,7 @@ extern "C" { #define SND_SCN_INT_QOS 1
/** Scenario container */ -typedef snd_scenario_t snd_scenario_t; +typedef struct _snd_scenario_t snd_scenario_t;
/* TODO: add notification */
At Tue, 6 Oct 2009 17:54:29 +0200, Stefan Schmidt wrote:
From: Stefan Schmidt stefan@slimlogic.co.uk
Signed-off-by: Stefan Schmidt stefan@slimlogic.co.uk
include/ascenario.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/ascenario.h b/include/ascenario.h index 003eabe..0c5acb9 100644 --- a/include/ascenario.h +++ b/include/ascenario.h @@ -128,7 +128,7 @@ extern "C" { #define SND_SCN_INT_QOS 1
/** Scenario container */ -typedef snd_scenario_t snd_scenario_t; +typedef struct _snd_scenario_t snd_scenario_t;
I would define like
typedef struct snd_scenario snd_scenario_t;
Takashi
Hello.
On Tue, 2009-10-06 at 18:04, Takashi Iwai wrote:
At Tue, 6 Oct 2009 17:54:29 +0200, Stefan Schmidt wrote:
From: Stefan Schmidt stefan@slimlogic.co.uk
Signed-off-by: Stefan Schmidt stefan@slimlogic.co.uk
include/ascenario.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/ascenario.h b/include/ascenario.h index 003eabe..0c5acb9 100644 --- a/include/ascenario.h +++ b/include/ascenario.h @@ -128,7 +128,7 @@ extern "C" { #define SND_SCN_INT_QOS 1
/** Scenario container */ -typedef snd_scenario_t snd_scenario_t; +typedef struct _snd_scenario_t snd_scenario_t;
I would define like
typedef struct snd_scenario snd_scenario_t;
Fine with me. I just followed what I have seen in control.h here. Will send an updated set tomorrow which will have this fixed.
regards Stefan Schmidt
On Tue, 6 Oct 2009, Stefan Schmidt wrote:
Fine with me. I just followed what I have seen in control.h here. Will send an updated set tomorrow which will have this fixed.
Please, sync with me, again. I forgot to commit some changes to GIT repo.
Thanks, Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
Hello.
On Tue, 2009-10-06 at 19:16, Jaroslav Kysela wrote:
On Tue, 6 Oct 2009, Stefan Schmidt wrote:
Fine with me. I just followed what I have seen in control.h here. Will send an updated set tomorrow which will have this fixed.
Please, sync with me, again. I forgot to commit some changes to GIT repo.
Sure. Appreciated that you are also working on it. Will rebase before I re-submit them. Any special items youare targetting?
regards Stefan Schmidt
From: Stefan Schmidt stefan@slimlogic.co.uk
Signed-off-by: Stefan Schmidt stefan@slimlogic.co.uk --- src/ascenario.c | 56 +++++++++++++++++++++++++++--------------------------- 1 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/src/ascenario.c b/src/ascenario.c index cbd0630..157207a 100644 --- a/src/ascenario.c +++ b/src/ascenario.c @@ -80,7 +80,7 @@ struct scenario_info {
/* Describe a snd card and all its scenarios. */ -struct snd_scenario { +struct _snd_scenario_t { char *card_name; int current_scenario; int num_scenarios; /* number of supported scenarios */ @@ -109,14 +109,14 @@ static void scn_stdout(const char *fmt,...) va_end(va); }
-static inline void set_value(struct snd_scenario *scn, +static inline void set_value(snd_scenario_t *scn, struct control_settings *control, int count, unsigned short val) { int offset = scn->current_scenario * control->count; control->value[offset + count] = val; }
-static inline unsigned short get_value(struct snd_scenario *scn, +static inline unsigned short get_value(snd_scenario_t *scn, struct control_settings *control, int count) { int offset = scn->current_scenario * control->count; @@ -223,7 +223,7 @@ static int add_control(snd_ctl_t *handle, snd_ctl_elem_id_t *id, return 0; }
-static int parse_controls(struct snd_scenario *scn, FILE *f) +static int parse_controls(snd_scenario_t *scn, FILE *f) { struct control_settings *control; char buf[MAX_BUF], name[MAX_NAME]; @@ -408,7 +408,7 @@ static int get_enum (char *buf) return 0; /* TODO */ }
-static void seq_list_append(struct snd_scenario *scn, +static void seq_list_append(snd_scenario_t *scn, struct sequence_element *curr, int position) { struct sequence_element *last, *tmp; @@ -441,7 +441,7 @@ static void seq_list_append(struct snd_scenario *scn, } }
-static int parse_sequences(struct snd_scenario *scn, FILE *f, int position) +static int parse_sequences(snd_scenario_t *scn, FILE *f, int position) { char buf[MAX_BUF], *tbuf, *control_value; int control_len, i; @@ -517,7 +517,7 @@ static int parse_sequences(struct snd_scenario *scn, FILE *f, int position) }
/* load scenario i */ -static int read_scenario_file(struct snd_scenario *scn) +static int read_scenario_file(snd_scenario_t *scn) { int fd, ret; FILE *f; @@ -546,7 +546,7 @@ close: return ret; }
-static int read_sequence_file(struct snd_scenario *scn, int position) +static int read_sequence_file(snd_scenario_t *scn, int position) { int fd, ret; FILE *f; @@ -580,7 +580,7 @@ close: return ret; }
-static int parse_scenario(struct snd_scenario *scn, FILE *f, int line_) +static int parse_scenario(snd_scenario_t *scn, FILE *f, int line_) { struct scenario_info *info; int line = line_ - 1, id = 0, file = 0; @@ -721,7 +721,7 @@ err: return -EINVAL; }
-static int read_master_file(struct snd_scenario *scn, FILE *f) +static int read_master_file(snd_scenario_t *scn, FILE *f) { int line = 0, ret = 0, i; char buf[MAX_BUF], *tbuf; @@ -769,7 +769,7 @@ err: }
/* load scenario i */ -static int import_master_config(struct snd_scenario *scn) +static int import_master_config(snd_scenario_t *scn) { int fd, ret; FILE *f; @@ -801,7 +801,7 @@ close: * * Parse sound card and store control data in memory db. */ -static int parse_card_controls(struct snd_scenario *scn) +static int parse_card_controls(snd_scenario_t *scn) { struct control_settings *control; snd_ctl_t *handle; @@ -886,7 +886,7 @@ close: * * Read and parse scenario_info files the store in memory. */ -static int import_scenario_files(struct snd_scenario *scn) +static int import_scenario_files(snd_scenario_t *scn) { int ret;
@@ -933,7 +933,7 @@ static int import_scenario_files(struct snd_scenario *scn) }
/* free all resorces */ -static void free_scn(struct snd_scenario *scn) +static void free_scn(snd_scenario_t *scn) { /* TODO: valgrind to make sure. */ int i; @@ -977,19 +977,19 @@ static void free_scn(struct snd_scenario *scn) /* * Init sound card scenario db. */ -struct snd_scenario *snd_scenario_open(const char *card_name) +snd_scenario_t *snd_scenario_open(const char *card_name) { - struct snd_scenario *scn; + snd_scenario_t *scn; int err;
/* TODO: locking and * check if card_name scn is already loaded, * if so reuse to conserve ram. */
- scn = malloc(sizeof(struct snd_scenario)); + scn = malloc(sizeof(snd_scenario_t)); if (scn == NULL) return NULL; - bzero(scn, sizeof(struct snd_scenario)); + bzero(scn, sizeof(snd_scenario_t)); scn->card_name = strdup(card_name); if (scn->card_name == NULL) { free(scn); @@ -1016,7 +1016,7 @@ struct snd_scenario *snd_scenario_open(const char *card_name) /* * Reload and reparse scenario db. */ -int snd_scenario_reload(struct snd_scenario *scn) +int snd_scenario_reload(snd_scenario_t *scn) { free_scn(scn);
@@ -1034,13 +1034,13 @@ int snd_scenario_reload(struct snd_scenario *scn) return 0; }
-void snd_scenario_close(struct snd_scenario *scn) +void snd_scenario_close(snd_scenario_t *scn) { free_scn(scn); }
static int set_control(snd_ctl_t *handle, snd_ctl_elem_id_t *id, - struct snd_scenario *scn) + snd_scenario_t *scn) { struct control_settings *setting; int ret, count, i, idnum; @@ -1122,7 +1122,7 @@ set_val: return 0; }
-static void exec_sequence(struct sequence_element *seq, struct snd_scenario +static void exec_sequence(struct sequence_element *seq, snd_scenario_t *scn, snd_ctl_elem_list_t *list, snd_ctl_t *handle) { int count = snd_ctl_elem_list_get_count(list); @@ -1151,7 +1151,7 @@ static void exec_sequence(struct sequence_element *seq, struct snd_scenario } }
-int snd_scenario_set_scn(struct snd_scenario *scn, const char *name) +int snd_scenario_set_scn(snd_scenario_t *scn, const char *name) { snd_ctl_card_info_t *info; snd_ctl_elem_list_t *list; @@ -1311,7 +1311,7 @@ close: return ret; }
-const char *snd_scenario_get_scn(struct snd_scenario *scn) +const char *snd_scenario_get_scn(snd_scenario_t *scn) { if (scn->current_scenario > 0 && scn->current_scenario < MAX_SCN) return scn->scenario[scn->current_scenario].name; @@ -1319,7 +1319,7 @@ const char *snd_scenario_get_scn(struct snd_scenario *scn) return NULL; }
-int snd_scenario_set_integer(struct snd_scenario *scn, int type, int value) +int snd_scenario_set_integer(snd_scenario_t *scn, int type, int value) { switch (type) { case SND_SCN_INT_QOS: @@ -1330,7 +1330,7 @@ int snd_scenario_set_integer(struct snd_scenario *scn, int type, int value) } }
-int snd_scenario_get_integer(struct snd_scenario *scn, int type, int *value) +int snd_scenario_get_integer(snd_scenario_t *scn, int type, int *value) { if (value == NULL) return -EINVAL; @@ -1343,14 +1343,14 @@ int snd_scenario_get_integer(struct snd_scenario *scn, int type, int *value) } }
-int snd_scenario_get_control_id(struct snd_scenario *scn, int type, +int snd_scenario_get_control_id(snd_scenario_t *scn, int type, snd_ctl_elem_id_t *id) { /* not yet implemented */ return -EINVAL; }
-int snd_scenario_list(struct snd_scenario *scn, const char **list[]) +int snd_scenario_list(snd_scenario_t *scn, const char **list[]) { if (scn == NULL || list == NULL) return -EINVAL;
From: Stefan Schmidt stefan@slimlogic.co.uk
This is just enough to compile ascenario.c with last changes to the declarations.
Signed-off-by: Stefan Schmidt stefan@slimlogic.co.uk --- src/ascenario.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/ascenario.c b/src/ascenario.c index 157207a..ce0a795 100644 --- a/src/ascenario.c +++ b/src/ascenario.c @@ -1034,9 +1034,10 @@ int snd_scenario_reload(snd_scenario_t *scn) return 0; }
-void snd_scenario_close(snd_scenario_t *scn) +int snd_scenario_close(snd_scenario_t *scn) { free_scn(scn); + return 0; }
static int set_control(snd_ctl_t *handle, snd_ctl_elem_id_t *id, @@ -1241,7 +1242,7 @@ close: return ret; }
-int snd_scenario_dump(const char *card_name) +int snd_scenario_dump(snd_output_t *output, const char *card_name) { snd_ctl_t *handle; snd_ctl_card_info_t *info; @@ -1321,6 +1322,8 @@ const char *snd_scenario_get_scn(snd_scenario_t *scn)
int snd_scenario_set_integer(snd_scenario_t *scn, int type, int value) { + int qos; + switch (type) { case SND_SCN_INT_QOS: scn->scenario[scn->current_scenario].qos = qos;
On Tue, Oct 06, 2009 at 03:33:39PM +0200, Jaroslav Kysela wrote:
- represent QoS with string (name) rather than integer - QoS is also quite use-case specific
I believe the idea is that QoS should be relatively generic - the long term idea is that the application should be able to give some idea of the sort of output it is trying to provide to the stack to enable the stack to make decisions about configuration.
Hello.
On Tue, 2009-10-06 at 10:23, Jaroslav Kysela wrote:
Some ideas, proposal for further discussion...
Thanks for that.
On Mon, 5 Oct 2009, Stefan Schmidt wrote:
On Mon, 2009-10-05 at 11:14, Jaroslav Kysela wrote:
On Mon, 5 Oct 2009, Stefan Schmidt wrote:
On Thu, 2009-10-01 at 17:08, Stefan Schmidt wrote:
Question: What's the IDs (integers) returned with snd_scenario_get_master_playback_volume() etc. functions?
That is part of the code Liam wrote, but let me answer it (and maybe Liam chime in if it is wrong or needs more explanation).
It's just the numid of an alsa control which should be used as master playback, etc in this scenario. It aliases the function we can use with ascenario to the real ones in alsa which can bedifferent for different scenarios.
It's not very clear. The exported functions should be documented at least with doxygen.
They are documented with doxygen in ascenario.h. But I just see that not all return values are described.
Also, I would move all _get_ functions to one with this interface:
#define SND_SCENARIO_KEY_KCTL_MASTER_PLAYBACK_VOLUME 1 .....
int snd_scenario_get(scenario, int key, void *result);
It will allow us to extend this interface without adding many functions in future, something like:
#define SND_SCENARIO_KEY_PCM_PLAYBACK 11 .....
I understand your point about having a more generic function for this. The design of ascenario was however fixed to only these four functions. They were not added to copy the funtionality already in alsa-lib, but to offer a control alias for the four most used functions under the same name over all scenarios. (Liam, as always, jump in if I explain it wrong here as you did the design way before I started to work on it.)
Also, using numid's is not much ideal if something changes in the driver. I would prefer to use standard ctl IDs (string, index, interface - result should be snd_ctl_elem_id_t).
That's something Mark and Takashi alrerady brought up and we agreed on. Is this something you want to have before a merge is considered or is this something you would accept as follow-up patch?
Please, include also some test configuration files to review the configurationsyntax.
Attached. default.conf is the main configuration file located at /etc/alsa/scenario/default.conf
Again, I would make configuration syntax compatible with other alsa-lib configuration files. The changes in the configuration files will be small and it will allow us to rewrite the code using parsers in alsa-lib.
Something like:
default.conf:
scenario."playback speaker" { file playback_spk qos HIFI "Master Playback Volume" 8 # should be rather something like: kcontrol.'Master Playback Volume' "'Master Playback Volume':33" # or a short form might be implemented (:33 is ctl_id index, ctl_id.name # would be copied from the variable key name): kcontrol.'Master Playback Volume' ":33" ... }
scenario."playback headphones" { ... }
About "playback_hs_*" files. It seems that the purpose of files is quite similar. I don't understand the reason to have 2 different descriptions.
Hmm, I'm not sure I get what you mean here. Do you mean that the post and pre sequence files are similar? That could of course be different on different hardware. It's not a must that we would use the same sequence before and after the scenario to reduce the pops and other noise.
Also, I would like to consider moving the parser from alsactl initialization code (see alsactl_init.c in alsa-utils package) to alsa-lib and use this parser also for these files. It gives much flexibility, although udev-like syntax is not ideal, I know. But we have full documentation in alsactl_init.xml, at least.
The configuration format may indeed be critical as changing it after a merge would be painful. Besides the actual code changes it would cost a fiar amount of time to test and debug these. Liam, what do you say about it?
regards Stefan Schmidt
From: Stefan Schmidt stefan@slimlogic.co.uk
ascndump is used to dump the kcontrols in a syntax that can be used with ascnctl later. To list and set scenarios you can use ascnctl. This patch depends on the ascenario infrstructure to be merged in alsa-lib.
CC: Ian Molton ian@mnementh.co.uk CC: Graeme Gregory gg@slimlogic.co.uk Signed-off-by: Liam Girdwood lrg@slimlogic.co.uk Signed-off-by: Stefan Schmidt stefan@slimlogic.co.uk --- Makefile.am | 2 +- ascenario/Makefile.am | 14 ++++++++ ascenario/alsa-scn.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++ ascenario/dump.c | 33 ++++++++++++++++++ configure.in | 1 + 5 files changed, 137 insertions(+), 1 deletions(-) create mode 100644 ascenario/Makefile.am create mode 100644 ascenario/alsa-scn.c create mode 100644 ascenario/dump.c
diff --git a/Makefile.am b/Makefile.am index 5296977..34c542f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,6 +1,6 @@ INCLUDES=-I$(top_srcdir)/include
-SUBDIRS = include alsactl utils m4 po +SUBDIRS = include alsactl ascenario utils m4 po if ALSAMIXER SUBDIRS += alsamixer endif diff --git a/ascenario/Makefile.am b/ascenario/Makefile.am new file mode 100644 index 0000000..88f5a67 --- /dev/null +++ b/ascenario/Makefile.am @@ -0,0 +1,14 @@ +noinst_PROGRAMS = \ + ascnctl \ + ascndump + +ascndump_SOURCES = dump.c + +ascnctl_SOURCES = alsa-scn.c + +INCLUDES = \ + -Wall -I$(top_srcdir)/include + +ascndump_LDADD = -lasound + +ascnctl_LDADD = -lasound diff --git a/ascenario/alsa-scn.c b/ascenario/alsa-scn.c new file mode 100644 index 0000000..162d988 --- /dev/null +++ b/ascenario/alsa-scn.c @@ -0,0 +1,88 @@ +/* + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. + * + * Copyright (C) 2008-2009 SlimLogic Ltd + * Authors: Liam Girdwood lrg@slimlogic.co.uk + * Stefan Schmidt stefan@slimlogic.co.uk + */ + +#include <stdio.h> +#include <string.h> +#include <stdlib.h> +#include <unistd.h> +#include <alsa/ascenario.h> + +#define OP_LIST 1 +#define OP_SET 2 + +void print_usage(char *name) { + printf("Usage: %s <cmd> [<scenario>]\n" + " list - list available scenarios\n" + " set <name> - apply scenario <name>\n\n", name); +} + +int main(int argc, char *argv[]) +{ + struct snd_scenario *scn; + int num, i, op = 0, err, ret = 0; + const char **list; + + if(argc > 2) { + if(!strcmp(argv[1], "set")) + op = OP_SET; + } else if(argc > 1) { + if(!strcmp(argv[1], "list")) + op = OP_LIST; + } + + if(!op) { + print_usage(argv[0]); + exit(1); + } + + /* open library */ + scn = snd_scenario_open("default"); + if (scn == NULL) { + printf("%s: failed to init\n", argv[0]); + return 0; + } + + switch (op) { + case OP_LIST: + /* get list of scenarios */ + num = snd_scenario_list(scn, &list); + if (num) { + for(i = 0 ; i < num ; i++) + printf("%s\n", list[i]); + } else { + printf("%s: no scenarios\n", argv[0]); + ret = 2; + } + break; + case OP_SET: + err = snd_scenario_set_scn(scn, argv[2]); + if (err < 0) { + printf("failed to set scenario: %s\n", argv[2]); + ret = 3; + } + break; + default: + print_usage(argv[0]); + break; + } + + snd_scenario_close(scn); + return ret; +} diff --git a/ascenario/dump.c b/ascenario/dump.c new file mode 100644 index 0000000..b433ee6 --- /dev/null +++ b/ascenario/dump.c @@ -0,0 +1,33 @@ +/* + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. + * + * Copyright (C) 2008-2009 SlimLogic Ltd + * Authors: Liam Girdwood lrg@slimlogic.co.uk + * Stefan Schmidt stefan@slimlogic.co.uk + */ + +#include <stdio.h> +#include <alsa/ascenario.h> + +int main(int argc, char *argv[]) +{ + if (argc != 2) { + printf("usage: %s card (hw: prefix needed" + " e.g. hw:CARD=0 or hw:Intel)\n", argv[0]); + return 0; + } + + return snd_scenario_dump(argv[1]); +} diff --git a/configure.in b/configure.in index 931d034..8fcf1d3 100644 --- a/configure.in +++ b/configure.in @@ -270,6 +270,7 @@ AC_OUTPUT(Makefile alsactl/Makefile alsactl/init/Makefile \ m4/Makefile po/Makefile.in \ alsaconf/alsaconf alsaconf/Makefile \ alsaconf/po/Makefile \ + ascenario/Makefile \ aplay/Makefile include/Makefile iecset/Makefile utils/Makefile \ utils/alsa-utils.spec seq/Makefile seq/aconnect/Makefile \ seq/aplaymidi/Makefile seq/aseqdump/Makefile seq/aseqnet/Makefile \
At Thu, 1 Oct 2009 11:47:14 +0200, Stefan Schmidt wrote:
Hello.
This patchset add support for scenario management, aka use cases, in alsa-lib and example programs in alsa-utils. It allows to adjust the current audio kcontrol settings from a prespective of a changed use case like the switch from listening to music on your phone to an incoming phone call and back.
Great, I've been waiting for this :)
To achieve this it offers control aliasing for playback, capture master and switch as well as the option to post- and prefix a sequence of control changes avoiding pops and other unwanted noise.
This is of course not meant to replace gstreamer, PulseAudio or a sound server, but is meant to work in tandem with such audio software. We think alsa-lib is the best place for this as it will be used on small embedded system which only use alsa from the full stack of audio libs in linux. We see potential for desktop usage as well though.
Agreed. If any need, I can merge it to salsa-lib, too.
thanks,
Takashi
Hello.
On Fri, 2009-10-02 at 11:12, Takashi Iwai wrote:
At Thu, 1 Oct 2009 11:47:14 +0200, Stefan Schmidt wrote:
This patchset add support for scenario management, aka use cases, in alsa-lib and example programs in alsa-utils. It allows to adjust the current audio kcontrol settings from a prespective of a changed use case like the switch from listening to music on your phone to an incoming phone call and back.
Great, I've been waiting for this :)
:)
To achieve this it offers control aliasing for playback, capture master and switch as well as the option to post- and prefix a sequence of control changes avoiding pops and other unwanted noise.
This is of course not meant to replace gstreamer, PulseAudio or a sound server, but is meant to work in tandem with such audio software. We think alsa-lib is the best place for this as it will be used on small embedded system which only use alsa from the full stack of audio libs in linux. We see potential for desktop usage as well though.
Agreed. If any need, I can merge it to salsa-lib, too.
Do you prefer doing that yourself or should I sent you another patch for salsa? We definetly want it there, too. I already downloaded the last salsa release but was waiting if everyone is happy with the updated patch I sent yesterday.
regards Stefan Schmidt
At Fri, 2 Oct 2009 11:55:16 +0200, Stefan Schmidt wrote:
Hello.
On Fri, 2009-10-02 at 11:12, Takashi Iwai wrote:
At Thu, 1 Oct 2009 11:47:14 +0200, Stefan Schmidt wrote:
To achieve this it offers control aliasing for playback, capture master and switch as well as the option to post- and prefix a sequence of control changes avoiding pops and other unwanted noise.
This is of course not meant to replace gstreamer, PulseAudio or a sound server, but is meant to work in tandem with such audio software. We think alsa-lib is the best place for this as it will be used on small embedded system which only use alsa from the full stack of audio libs in linux. We see potential for desktop usage as well though.
Agreed. If any need, I can merge it to salsa-lib, too.
Do you prefer doing that yourself or should I sent you another patch for salsa? We definetly want it there, too. I already downloaded the last salsa release but was waiting if everyone is happy with the updated patch I sent yesterday.
I think we should concentrate on to fix and merge to the main alsa-lib at first. The backport to salsa would be fairly easy for scenario.
thanks,
Takashi
On Fri, 2 Oct 2009, Takashi Iwai wrote:
At Thu, 1 Oct 2009 11:47:14 +0200, Stefan Schmidt wrote:
Hello.
This patchset add support for scenario management, aka use cases, in alsa-lib and example programs in alsa-utils. It allows to adjust the current audio kcontrol settings from a prespective of a changed use case like the switch from listening to music on your phone to an incoming phone call and back.
Great, I've been waiting for this :)
To achieve this it offers control aliasing for playback, capture master and switch as well as the option to post- and prefix a sequence of control changes avoiding pops and other unwanted noise.
This is of course not meant to replace gstreamer, PulseAudio or a sound server, but is meant to work in tandem with such audio software. We think alsa-lib is the best place for this as it will be used on small embedded system which only use alsa from the full stack of audio libs in linux. We see potential for desktop usage as well though.
Agreed. If any need, I can merge it to salsa-lib, too.
I'm not very persuaded to merge such thing to alsa-lib (at least not to alsa-lib.so file - I may accept another library in the alsa-lib package). My comments:
1) configuration syntax
- I know, simple text files are fine, but there are some structured things - we have already configuration parsers in alsa-lib for structured texts which can be migrated to XML some day
2) mixing high-level and low-level things
- the idea is to "hide" some hardware details introducing some abstraction, but the scenario API ended with "ctl interface" instead using a mixer API for apps? - I would like to see handling all "virtual card" stuff in the scenario API, so user chooses one scenario and gets mixer controls, preconfigured pcm devices for playback and capture, rawmidi devices, timer devices etc.; it means that user can divide - for example - one physical card to more virtual ones with a custom configuration
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
At Fri, 2 Oct 2009 11:58:47 +0200 (CEST), Jaroslav Kysela wrote:
On Fri, 2 Oct 2009, Takashi Iwai wrote:
At Thu, 1 Oct 2009 11:47:14 +0200, Stefan Schmidt wrote:
Hello.
This patchset add support for scenario management, aka use cases, in alsa-lib and example programs in alsa-utils. It allows to adjust the current audio kcontrol settings from a prespective of a changed use case like the switch from listening to music on your phone to an incoming phone call and back.
Great, I've been waiting for this :)
To achieve this it offers control aliasing for playback, capture master and switch as well as the option to post- and prefix a sequence of control changes avoiding pops and other unwanted noise.
This is of course not meant to replace gstreamer, PulseAudio or a sound server, but is meant to work in tandem with such audio software. We think alsa-lib is the best place for this as it will be used on small embedded system which only use alsa from the full stack of audio libs in linux. We see potential for desktop usage as well though.
Agreed. If any need, I can merge it to salsa-lib, too.
I'm not very persuaded to merge such thing to alsa-lib (at least not to alsa-lib.so file - I may accept another library in the alsa-lib package).
Hm, I don't find it so problematic to merge it since the scenario stuff is pretty clean and doesn't drag any other dependency into it. I understand that merging to alsa-lib would bring less hustle for developers. It's simply there.
OTOH, if scenario is supposed to be expanded fast, it'd be safer to stay as a separate library. And, another bonus is that it can be re-used for salsa-lib without changes.
So, the biggest question is how stable scenario is, especially from API viewpoint. Merging into alsa-lib means that the stuff could be considered as a part of LSB in future, too.
My comments:
configuration syntax
- I know, simple text files are fine, but there are some structured things - we have already configuration parsers in alsa-lib for structured texts which can be migrated to XML some day
This is always a question. IMO, a simple text file parser is OK as long as it's really simple. Reusing alsa-lib parser wouldn't save much code.
If more structural configuration is required, we should reconsider what to use. But, this means anyway a major API change, and this shouldn't happen so much.
mixing high-level and low-level things
- the idea is to "hide" some hardware details introducing some abstraction, but the scenario API ended with "ctl interface" instead using a mixer API for apps?
Take a look at ASoC drivers. Many (most?) of them are not suited well for mixer abstraction at all. Accessing directly ctl interface is far easier for them.
- I would like to see handling all "virtual card" stuff in the scenario API, so user chooses one scenario and gets mixer controls, preconfigured pcm devices for playback and capture, rawmidi devices, timer devices etc.; it means that user can divide - for example - one physical card to more virtual ones with a custom configuration
This would be a nice extension.
thanks,
Takashi
On Fri, 2009-10-02 at 11:58 +0200, Jaroslav Kysela wrote:
I'm not very persuaded to merge such thing to alsa-lib (at least not to alsa-lib.so file - I may accept another library in the alsa-lib package). My comments:
configuration syntax
- I know, simple text files are fine, but there are some structured things - we have already configuration parsers in alsa-lib for structured texts which can be migrated to XML some day
I know XML is great for parsing but the main audience for this is embedded users so we need to keep it simple (and small) with few external dependencies. Not all embedded devices will ship with XML.
Liam
On Fri, 2 Oct 2009, Liam Girdwood wrote:
On Fri, 2009-10-02 at 11:58 +0200, Jaroslav Kysela wrote:
I'm not very persuaded to merge such thing to alsa-lib (at least not to alsa-lib.so file - I may accept another library in the alsa-lib package). My comments:
configuration syntax
- I know, simple text files are fine, but there are some structured things - we have already configuration parsers in alsa-lib for structured texts which can be migrated to XML some day
I know XML is great for parsing but the main audience for this is embedded users so we need to keep it simple (and small) with few external dependencies. Not all embedded devices will ship with XML.
I don't mean to use a complex XML parsing library with DOM handling. I wrote very simple XML parser in C on few hundred lines for another project so it's no issue also for the embedded platform. I just wanted to point - the structure of alsa-lib configuration files is very similar to XML.
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
participants (6)
-
Jaroslav Kysela -
Liam Girdwood -
Mark Brown -
pl bossart -
Stefan Schmidt -
Takashi Iwai