[pulseaudio-discuss] module-combine-sink interface for dynamic control of slaves

[Steffen Pfendtner]

Thanks for the suggestions!
I will go them through along with the code and think about it.

Am Samstag, den 10.12.2016, 19:39 +0200 schrieb Tanu Kaskinen:
> 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.
>