'Twas brillig, and David Henningsson at 14/11/11 09:52 did gyre and gimble: > On 11/14/2011 10:31 AM, Colin Guthrie wrote: >> 'Twas brillig, and David Henningsson at 14/11/11 07:32 did gyre and >> gimble: >>> 2) I'm a little confused that you don't send a pointer to the actual >>> sink input, only its proplist. Why? >> >> Well two reasons really. >> The second, and arguably more valid, is that this function is also >> called before the pa_sink_input pointer is ready (i.e. with >> sink_input_new_data. In this case I simply don't have such a pointer to >> pass in, but I do have it's proplist. > Makes sense...I think :-) > > My point is that routing modules is likely to want to use as much > information as possible, and even if "information hiding" is a good > principle in general, I'm not sure that applies to routing modules. > Therefore we might be better off with supplying the sink input pointer > instead of just the proplist (or both, if you want). In an ideal world it would maybe be a good idea, but as it would be not be possible to supply such a pointer in all cases (see my second point quoted above) I think it's best to be consistent and just supply the proplist in all cases rather than sometimes pass the sink input pointer and some times not. >>> 4) Do we really need the ignore field? It is only used in the unlink >>> phase (right?) and in that case the source/sink should already been in a >>> state that the routing modules can use to know that they should not >>> prefer it. >> >> Yeah it's for unlinking, but no, it's not called when the sink is in a >> state that routing modules can know about it. >> >> As you can see here: >> >> http://colin.guthr.ie/git/pulseaudio/diff/src/pulsecore/sink.c?h=route&id=61d73564c1c2b0b832f14bfd4f2b77c98febb735 >> >> >> it's called when the sink is still linked. If delay the call until after >> we unlink, modules like module-rescue-streams will already have done >> their job and selected a new device, which is something we want to >> avoid. Also there is some code that iterates through and kills any >> streams before the sink is unlinked, so we actually can't delay it too >> much anyway. >> >> It's not actually that different to what module-rescue-streams does (it >> also has an "skip" sink) >> >> find_evacuation_sink(pa_core *c, pa_sink_input *i, pa_sink *skip); >> >> So I don't think we can avoid this approach. > > Hmm. In my world module-rescue-streams would either be completely > replaced by the general routing, or changed to listen to the "route" > hook, as this is essentially a routing module. I think in all practical cases for systems that do use priority list routing, module-rescue-streams would never actually be used, but if there are no priority list modules loaded it could still kick in. That said, it could indeed change to hook into the route callback rather than the unlink one as you suggest. We'd just have to make sure it was hooking in there at a lower priority than all the official lists (i.e. it would be called after the 'default-sink' list and thus pretty much always be a NOOP in a typical desktop setup). But as this is pretty much unnecessary overhead for a desktop system (why call a route hook that just returns HOOK_OK every time a stream is routed?), I'm not 100% sure it should be moved to this structure. I'm not against it, just not fully thought through the different scenarios where it would be genuinely useful. From a purely architectural POV it would make sense to move it, but sometimes FCO savings can trump that! > It would just have seemed more natural to read everything out of the > sink state, rather to have one sink state and one pointer essentially > saying "don't trust the sink state". But maybe adding another sink state > "unlinking" would be overkill... Yeah that would be the alternative. It can be retrofitted fairly easy if/when we decide such a state is a good idea without too much churn, so I'm not super concerned about this right now. Will keep it in mind if we do add another state to sinks for e.g. "connecting" as has been loosely mooted recently (by me mainly :D) as messing about with the states all at once is likely the best plan if we do it at all :) 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/
