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. -- Tanu https://www.patreon.com/tanuk
