Hiya 'Twas brillig, and Tanu Kaskinen at 23/11/11 19:16 did gyre and gimble: > On Sun, 2011-11-13 at 16:42 +0200, Colin Guthrie wrote: >> Hi, >> >> I've written up my latest proposal to gather feedback before starting >> (hopefully soon now) on the implementation. >> http://www.freedesktop.org/wiki/Software/PulseAudio/RFC/PriorityRouting > > Thanks for your work! That RFC alone looks like it has already taken a > lot of effort. I feel a bit sorry for suggesting such big changes below, > but let's see how the discussion continues... That's why it's there - it's *meant* to get holes picked in it now before I (and hopefully others!) work on the implementation. Better now than after a couple KSLOCs :) > The system seems to be structured quite differently from what Janos was > proposing: that there would be a central routing function that would > have an outer loop that iterates through all streams in priority order, > and an inner loop that iterates through one stream's route list in > priority order, always selecting the first available route, and doing > the required profile and port changes immediately, as well as moving the > streams to the right devices. The primary difference (to my understanding at lease) is the order of the streams when iterating over them to perform the allocation and some form of state tracking to keep a list of devices who must be kept as they have already been used in a previous routing decision (only needed when operating in the mode that allows it to switch ports). I don't think that's necessarily a massive difference, but I do need to accommodate it in the design. I'd like to keep the "role based order" thing generic (i.e. not specifically make it role based order, but make the ordering configurable and modular, such that the module that loads the role-based priority list in some way also registers some other callbacks that contribute to deciding the order over which we iterate the streams). > In your proposal it's unclear how such changes can be done at all - is > that supposed to be done in the route hook callbacks? Not really. The order of processing is totally external to the route hook callbacks. Each route hook deals with one stream only. So this route hook really just represents Janos' inner loop. In the rightup I was just assuming the outer loop would just be: PA_IDXSET_FOREACH(si, u->core->sink_inputs, idx) { ... } But this is clearly inadequate! There needs to be the ordering info incorporated at this level somehow. > If that's the > case, then you're relying on module-rescue-streams to move streams > somewhere when the device disappears that they used previously, > otherwise those streams get killed. No, in the initial patch: http://colin.guthr.ie/git/pulseaudio/commit/?h=route&id=61d73564c1c2b0b832f14bfd4f2b77c98febb735&context=3&ignorews=0&ss=1 in pulsecore/sink.c in the second hunk, I trigger a full reroute after a sink is unlinked. I set an "ignore" sink argument to ensure that everything will be rerouted, but the sink to be unlinked is ignored. The call that does the "full reroute" is pa_route_sink_inputs(), and it's implementation is just the simple PA_IDXSET_FOREACH() mentioned above. It will of course have to have a more deterministic order, but other than this it's fine) This call is before the firing of the PA_CORE_HOOK_SINK_UNLINK hook which is what module-rescue-streams uses to, well, rescue the streams, so a new device should be picked for all streams before it gets the chance to do anything. > Moving the streams to the correct > devices happens only after all profile and port changes are done. I > think that's a serious problem. Streams that are playing to a sink that > changes port to one that is not suitable for the stream will leak their > audio to the new port for a short time. Is this a practical problem? Is it just a matter of tweaking the suggested modifications to pa_sink_set_port() changes to call the pa_route_sink_inputs() call sooner? Or would we need to e.g. pre-reroute, knowing a change is coming, work out a list of streams that *will* be moved, mute them, do the port change, reroute and then unmute the ones we'd silenced? If the latter is needed I don't think it's the end of the world (it certainly does add a lot of complications but not too much so. I'd also suggest ignoring this problem initially and then retro fitting it if it is needed. I think the general implementation would be easier to grok if this kind of thing was added afterwards. > Also the device that > module-rescue-streams chooses can be wrong one, again making sound to go > to a wrong place for a short time. We have had to fight this kind of > issues in the Maemo phones. I think as I said above, m-rescue-streams likely won't ever get a look in. > I think the right approach is to detach a stream whenever the device > that it's initially connected to either disappears or changes port. The > stream should be attached to the correct device when the stream's > routing decision is done. Ahh this is even easier than my mute and pre-route suggestion above. It's much simpler! I don't think this is a problem at all. > I feel that the "multiple lists in one priority list" concept and > priority list weights add unnecessary complexity. I think each stream > should have exactly one priority list attached to them and that's it. > It's up to the routing modules to set the correct list to each stream. This really just boils down to implementation. I've gone through various mechanisms of visualising how this stuff would work, and I very much did consider this approach too and I really don't like it for several reasons. I'll try and outline them below but I've probably forgotten some of the reasons. If only one list is used rather than a group of lists with a key, then each module dealing with priority lists will have to have a lot more boiler plate code to deal with things. Each will have to register hooks for new streams, to check whether the key system they are using has a registered priority list specific to it. So I have a "role-routing" module, it has to look each and every stream and say "do you have a role? Yes, OK your role is "foo", have I registered a list for "foo" (namereg check probably is needed here), No? OK, then I'd better create one now in case someone tries to do the routing, but I need to make sure it actually contains the same contents as the priority list further down the order (i.e. the default) priority list) is actually used, so now I *want* to use the foo priority list if anything changes on this stream (i.e. it's moved to a new sink), but I have to currently use the "default" priority list. If the default priority list changes, I want my stream to be rerouted, but if MY stream is moved, it needs to update the foo priority list. So even why you are trying to attach only one list to a stream, it's still getting more than one in plenty scenarios. It's "preferred" priority list (which might be brand new and empty) and it's "effective" priority list (which is the one it's currently using for routing) The fact that a list can be keyed (and thus actually contain several individual lists of device+ports for each key value) or not just wraps things up so that concepts like "role-based-routing" can be easily implemented without a lot of extra glue code. And the fact that different routing methodologies themselves can be weighted means that we don't have to faff around dealing with the "preferred" and "effective" lists mentioned above. Weightings become even more useful when it gets slightly more complex when you have the three levels like we do now. Role, Application and Default. If the role based list for the "foo" role is empty then our "preferred" is a new, empty list and our "effective" list is the application list. But what if the application list is empty too? Our effective list is the "default". So when some UI tool changes default devices, our stream will happily obey. But then what happens if some data is written for the application list? We only have "preferred" list of foo-role and a "effective" list of "default", but our stream has to be moved as it's now got application data.... I just can't resolve this problem without a weighting system of different priority lists. I'm happy to listen to suggestions (or perhaps explain my concerns more clearly if I'm not being clear) as to how this could be done differently. > I'm also not sure that we should add the priority list introspection and > editing to the core API at this point. It may not be desirable to always > give clients direct access to the priority lists. If some routing module > wants to do that for its lists, then it could do that through an > extension API. The priority list name and description (uiname) wouldn't > be needed in the core. I think that's a really bad idea. Having a role-based routing module and an application-based routing module each providing separate APIs to manipulate effectively the same things is a massive repetition of code. Having a standard set of minimal APIs that can allow for editing of priority lists generically solves this neatly without too much clutter. I'm specifically trying to reduce the kind of clutter I introduced with the module-device-manager extension API which edits priority lists and come up with a standard way that can work for role routing, application routing and default routing as the primary three cases (i.e. what we do now but with added memory), and I really don't want to introduce three new extensions APIs when one core API will do. I'd also very much like to get the priority list info into "pacmd ls" output as it makes it much easier to support people when they come on and ask us why they can't use device X (tho' hopefully that kind of support question will be reduced with better UIs and functionality anyway!!) I do also have a "list editor UI" in my head too that would give power users complete access. Any besides, I don't ever envisage a routing module actively manipulating their own lists much other than where the initially inject new devices. Once the user sets up some preferences of their own, the list should be written to disk and the routing module does nothing to actively manipulate things until the user attaches a brand new device in which case the "initial position" decision kicks in again. Happy to discuss further as always :) Many thanks for the feedback so far. Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/