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