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

[Takashi Iwai <tiwai@suse.de>]

[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