On Sat, 2016-12-10 at 16:37 +0200, Tanu Kaskinen wrote: > On Thu, 2016-12-08 at 21:48 +0100, Steffen Pfendtner wrote: > > I would like to extend the module-combine-sink to dynamically add and > > remove the slaves. Especially with pulseaudio-dlna this would be a great > > benefit. > > This would be a very good feature to have. > > > My first intention was to abuse the sink_properties where the > > combine-sink module announces its slaves to the outer world. I could > > hack this to be used in the other direction as well. > > > > From overall architecture what would be the best way to go? > > Public APIs are forever, so I don't think quick and dirty solutions are > appropriate. > > The "standard" approach for adding module-specific APIs is to add > "extension APIs" to libpulse. We currently have three of them: > > https://freedesktop.org/software/pulseaudio/doxygen/ext-device-manager_8h.html > https://freedesktop.org/software/pulseaudio/doxygen/ext-device-restore_8h.html > https://freedesktop.org/software/pulseaudio/doxygen/ext-stream-restore_8h.html > > (I don't think those APIs are very good, so it's probably best to not > copy the design from those.) > > I'm not sure that making an extension API is the best choice in this > case, however. I think the defining property of extension APIs is that > their availability depends on whether the corresponding module is > loaded or not. I can't think of any good reason why it should be > possible to disable the combine sink management API, so implementing it > as an extension seems like adding unnecessary complexity. > > I would implement the API and corresponding protocol changes "directly" > in libpulse and the native protocol, i.e. without designating them as > "extensions". > > Here's some sketching of how the API could look like: > > Global combine sink state could be handled by a "combine sink manager". > It could have functions like this: > > pa_combine_sink_manager_get_info() - This is needed if there's some > global information other than the list of sinks. If there's no such > information, then this function is not needed. > > pa_combine_sink_manager_list_sinks() > pa_combine_sink_manager_add_sink() > pa_combine_sink_manager_remove_sink() > > The individual combine sinks could have functions like this: > > pa_combine_sink_get_info() - The info consists of at least the sink > name and index. I'd be otherwise in favour of only using names as > identifiers, but the index is needed for the subscription API. > > pa_combine_sink_list_outputs() > pa_combine_sink_add_output() > pa_combine_sink_remove_output() > > The outputs (a.k.a. slaves) could have just one function: > > pa_combine_sink_output_get_info() - The info consists of at least the > output name and index, the output sink name, and the index of the sink > input associated with the output. The sink input index can't be used as > the output index, because the sink inputs get removed when the combine > sink suspends. It might be possible to use the output sink name as the > output name, so they perhaps don't need to be separate. If the output > sink name is used as the output name, then also the output sink index > could be reused as the output index. > > Change notifications could be handled with the existing subscription > API. One concern with that is that we need 2 or 3 (depending on whether > the manager object has any state that needs change notifications) new > object types, and the subscription API has reserved only 4 bits for > identifying the type, and out of 16 possible type identifiers 10 are > already used. I think we will need a new event API sooner or later. It can be argued that these functions should be added to introspect.h like all other server control functions, and use the pa_context prefix. The "combine sink manager" object wouldn't be needed; any global information could be stored in pa_server_info. I'm not sure which approach I prefer. Putting it all in introspect.h would be more consistent with the existing API. My only concern is that it would kind of promote the combine sink management API to equal status with the core API, while I'd be more comfortable with treating the new API as something that is layered on top of the core API. There's not much concrete technical difference, though. -- Tanu https://www.patreon.com/tanuk
