[alsa-devel] [PATCH] ascenario: Add scenario support to alsa-lib

[Takashi Iwai]

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