[pulseaudio-discuss] [PATCH 00/11] Routing: Move routing policy code out of module-alsa-card

[Tanu Kaskinen]

On Tue, 2013-11-26 at 17:58 +0100, David Henningsson wrote:
> On 11/26/2013 04:46 PM, Tanu Kaskinen wrote:
> > On Tue, 2013-11-26 at 12:55 +0100, David Henningsson wrote:
> >> Another way to do it could be to do the interfaces first and
> >> implementations later; i e, first, present the data structures you want
> >> to add and how they fit together, and get an ack on that before writing
> >> the entire implementation. That could help us agree on the structure
> >> before you spend time on writing all the code.
> > 
> > Here's the list of new core concepts that I proposed in the Edinburgh
> > presentation:
> > 
> >  * Router
> >  * Node
> >  * Routing group
> >  * Connection
> >  * Domain
> >  * Routing plan
> >  * Explicit connection request
> >  * Node features (more likely called Connection requirements)
> >  * Planned connection
> > 
> > You have seen pa_router and pa_node, and it looks like pa_router will be
> > dropped. A new addition is pa_device_prototype. The current plan is to
> > put pa_routing_group, pa_routing_plan, pa_explicit_connection_request
> > and pa_planned_connection into the Murphy module until there's need for
> > those outside the Murphy module, so what's still missing from the core
> > is pa_connection, pa_domain and pa_connection_requirements. 
> 
> So then there is,
>  * Nodes / pa_node - ok
>  * Edges / pa_edge - ok
> 
>  * Device prototype - Not sure if needed
>  * Router - Not sure if needed
>  * Connection requirements - "pretty much open"
>  * Domains - let's forget about them for now
> 
> So next step would be to see how these are connected together with each
> other and with other objects in PulseAudio.

Hmm, by "next step" do you mean something that I should provide, or did
you only refer to the overview that you wrote below:

> Nodes are created for (correct me if I'm wrong)
>  - all sink inputs
>  - all source outputs
>  - all ports
>  - all sinks that do not have ports
>  - all sources that do not have ports
> 
> However, in the future there might be sink inputs / source outputs that
> do not have nodes, because they are loopback connections etc.
> Ports will always have nodes though.
> 
> Nodes are memory managed (owned) by their respective
> sink/source/port/etc and are either present or not - they cannot
> suddenly appear or disappear.
> 
> > struct pa_connection {
> >     pa_core *core;
> >     pa_node *input;
> >     pa_node *output;
> >     char *string;
> > };
> 
> Ok, and out of those I understand input and output. core is not needed
> because neither input nor output can be NULL and nodes have a core
> pointer already.

I prefer to write edge->core instead of edge->input->core, but OK, I'll
remove the core field.

I forgot (at least) one field: index. The edge objects are visible to
clients, so they should have an index.

> I don't know what "string" is for.

Then you forgot what I wrote earlier :) The string field is there
because

    pa_log("Something about edge %s.", edge->string);

is easier to write than

    pa_log("Something about edge %s -> %s.", edge->input->name, edge->output->name);

> > At this point (when the implementation is missing) I don't think
> > stream-to-sink and stream-to-port connections will need any further
> > data, loopback and combine connections will need something when those
> > connection types become supported.
> 
> Ok.
> 
> Is this true:
> 
> Between two nodes there can be only one directed edge. Cycles are not
> allowed (?).

In the current code there are no bidirectional nodes, so there can't be
cycles in the graph that is exposed by nodes. There shouldn't be cycles
at the lower level either, but that's only enforced by whatever logic we
currently have for preventing cycles, and I don't think it's perfect.

There can't be two parallel edges from node A to node B.

If bidirectional nodes are implemented (which is probably a good idea),
then there can be two paths from A to B: for example, one direct edge
from A to B, and one path A -> C -> B. Cycles should be prevented.

> An edge being present is the same as an audio stream being active and
> running from the input node to the output node (?).

If a stream is corked or a device is suspended, then the stream won't be
running, but I would still have an edge from the stream to the device if
they are connected together at a lower level.

> Edges are memory managed (owned) by the pa_core object (?).

Yes.

> > I can do useful stuff without domains, and adding domains will be pretty
> > big refactoring, because connections will be subclassed by domains and
> > nodes will have to deal with being part of multiple domains (so a single
> > node type like "port" or "sink input" won't be sufficient, because a
> > node can be e.g. both a port and a module-defined type). I'd rather not
> > go too much into thinking about the domain details before the
> > stream-to-port routing is working.
> > 
> > pa_connection_requirements is pretty much open how it will look like. I
> > think the first thing we want to do with that is to make
> > stream-to-bluetooth-node routing work so that an appropriate profile is
> > selected automatically based on the stream type. On what basis should we
> > do that selection? Phone streams should use HSP and music streams should
> > use A2DP, but should the HSP profile describe itself as "suitable for
> > phone audio", or should the HSP profile describe the attributes that
> > make it somehow more suitable for phone audio than A2DP? I like the
> > latter more, but in this specific case I think that approach would be
> > too complicated. The main reason why HSP should be preferred for phone
> > streams is that it's full duplex, but streams appear one at a time, and
> > if the phone downlink stream appears first, the policy is going to have
> > hard time somehow taking into account the uplink stream that doesn't
> > exist yet, and if it doesn't take it into account, then being a
> > full-duplex profile has no value. Lower latency would be another reason
> > to prefer HSP, but I'm not actually convinced that HSP has lower latency
> > than A2DP, a clarification from someone that knows for sure would be
> > welcome. The latency constants in module-bluez4-device.c actually
> > suggest that HSP has higher playback latency than A2DP. So, if we go
> > with the simpler stream role based policy instead,
> > pa_connection_requirements would look like this:
> > 
> > struct pa_connection_requirements {
> >     pa_media_role_t suitable_for_media_role;
> > };
> > 
> > (Yes, I'd like to have an enumeration for media roles, but we can go
> > with strings too.)
> > 
> > Setting suitable_for_media_role in pa_connection_requirements would
> > require the routing target to be explicitly marked as suitable for
> > routees with that role.
> > 
> > So, now you know what structs I propose for the stage where we can start
> > to have some benefits for users.
> 
> What benefits are you anticipating?

Hmm, I was probably thinking of something like

    pactl set-node-routing some-stream-node some-inactive-port-node

which would magically activate the target port. Furthermore, if we
assume that the connection requirements thing is also available, along
with some suitable policy module implementation, then also automatic
selection of the most suitable Bluetooth profile would work.

But that's perhaps thinking too far already. If the next thing to do is
to implement edge creation and deletion and nothing else, that doesn't
buy us much functionality. I think activating nodes for use, such as
activating a port, should be done separately from creating an edge. That
is, pa_core_create_edge() shouldn't as a side effect change any ports or
profiles. There are a couple of reasons why I think that way:

1) The default policy (i.e. when there are no policy modules) should be
as simple as possible. If creating a new edge conflicts with an existing
edge, the edge creation should fail by default. It might be possible to
convince me to allow port switching if it doesn't conflict with existing
edges, but it's simpler to never switch ports or profiles by default.

2) I believe a good router module will use routing planning, and
executing the plan has these steps: delete/detach edges, activate nodes,
create/attach edges. Here edge creation will happen after the nodes have
already been activated, so if there's code in pa_core_create_edge() for
activating nodes, that code will be redundant.

So, I'd add node activation (pa_node_activate() function?) separately to
the infrastructure. To make use of that functionality, I could implement
a module that always honors the requested connection of a new node,
regardless of any existing edges, and blindly activates the requested
nodes. By that I mean this:

    pacat --route=speakers /dev/zero &
    pacat --route=headphones /dev/zero

Assuming that speakers and headphones are mutually exclusive, the second
pacat would cause the first pacat to get killed, because speakers became
unavailable when the router module activated the headphones. (In case
the router module is not loaded and we rely on the default behaviour,
the second pacat should fail.) This example of course requires a
protocol and client API extensions too.

I propose that I set that pacat example as the target. There are several
things to implement:

1) pa_core_create_edge()/pa_core_delete_edge()
2) pa_node_activate()
3) Protocol extension & client API for setting the requested routing for
new streams
4) --route option for pacat
5) Infrastructure support for passing the requested routing of new nodes
to routing modules
6) Policy in module-experimental-router

Assuming that you agree that this is a good direction to go, I suppose
you'd like to get all of these changes in a single patch set?

-- 
Tanu