On Fri, 2014-09-19 at 14:42 +0200, David Henningsson wrote: > > On 2014-09-19 10:38, Tanu Kaskinen wrote: > > Hi, > > > > I was thinking how ladspa controls could be implemented and exposed in > > the client API with the upcoming control framework. Not that I have > > plans to implement a client API for ladspa controls any time soon, but I > > want to have this some day, and the current control API doesn't seem > > optimal for supporting this functionality. > > > > The current plan for the definition of pa_control_info is the following: > > > > typedef struct pa_control_info { > > uint32_t index; /**< Index of this control. */ > > const char *name; /**< Name of this control. */ > > const char *description; /**< Description of this control. */ > > pa_proplist *proplist; /**< Property list for this control. */ > > > > pa_control_type_t type; /**< Type of this control. */ > > void *data; > > /**< Type-dependent data. See pa_control_type_t for information > > * about what data each type has. */ > > } pa_control_info; > > > > It seems to me that the type field should be changed from an enumeration > > to a string, so that modules would be free to introduce new control > > types without modifications to the core. > > > > It would also be nice to be able to associate arbitrary controls > > directly with e.g. a sink. For that I propose to add an array of > > controls to pa_sink_info and friends: > > > > struct pa_sink_info { > > ... > > struct { > > const char *key; /* e.g. "volume" or "ladspa" */ > > uint32_t control; /* index of the control */ > > } *controls; > > unsigned n_controls; > > }; > > Why do we need a *key field here, if each control's name is unique, we > can just use that as a key instead? No, the control name identifies the object, this key here identifies the semantics of the control. But you're half-right, I think the key field was a bad idea. If the goal is to support arbitrary ladspa filters, there will be no defined semantics for the controls. I gave "ladspa" as an example for the key value, but that only identifies the type of the control, not its purpose. The type is already available in the control object, so adding it here would be just redundant. I think we should keep the "volume_control" field in pa_sink_info. To allow applications to get the ladspa controls (and all other controls) associated with a sink, I propose that we add simple array of control indexes to pa_sink_info: struct pa_sink_info { ... uint32_t *controls; /* all controls, including the volume control */ unsigned n_controls; uint32_t volume_control; }; Some modules might still want to define specific semantics for controls. Using the sink proplist would probably be good enough for that. For example, module-equalizer-sink could implement its client interface by creating a control object and associating it with its sink by setting property "module-equalizer-sink.eq-control" = "6". -- Tanu