[pulseaudio-discuss] Edinburgh Murphy meeting notes

[David Henningsson]

On 11/07/2013 10:20 AM, Tanu Kaskinen wrote:
> On Thu, 2013-11-07 at 08:55 +0100, David Henningsson wrote:
>> On 11/06/2013 06:21 PM, Tanu Kaskinen wrote:
>>> On Wed, 2013-11-06 at 15:31 +0100, David Henningsson wrote:
>>>> On 11/06/2013 02:04 PM, Tanu Kaskinen wrote:
>>>>> On Thu, 2013-10-31 at 14:28 +0100, David Henningsson wrote:
>>>>>> On 10/31/2013 11:47 AM, Colin Guthrie wrote:
>>>>>>> At the moment, the routing is very simple. Too simple (obviously!) but
>>>>>>> at least it keeps it manageable and understandable right now. I guess my
>>>>>>> main concern is that it will turn even very simple routing
>>>>>>> configurations into a very complex beast.
>>>>>>
>>>>>> I agree with Colin here.
>>>>>>
>>>>>> But I would like to add something that's even more important: to get
>>>>>> these PA routing patches in digestible chunks. I've tried to say this
>>>>>> ever since Janos and Jaska started working on it more than a year ago,
>>>>>> but the message seems not to get through. It feels like there is a giant
>>>>>> patch set being worked on by Tanu, and having this giant patch set
>>>>>> presented all at once leads to significant risks:
>>>>>
>>>>> The plan was to not present it all at once, but in smaller pieces.
>>>>> However, this would be done only after we have a proof of concept
>>>>> implementation that actually works.
>>>>>
>>>>>>  * The review will be extremely heavy. Nobody will bear to do it.
>>>>>>
>>>>>>  * We might want to rethink things in the beginning of the patch series,
>>>>>> which leads to a lot of things having to be rewritten.
>>>>>
>>>>> Doing the full implementation first and then presenting it piece by
>>>>> piece doesn't help with your second point, so yes, your concern is fully
>>>>> justified. It would be better to actively involve the community in the
>>>>> design discussion already before we have a proposal ready for a full
>>>>> system. That's how I tried to work in the beginning, but Janos wanted to
>>>>> get the full system working ASAP. The reason for that was, in my
>>>>> understanding, that there is some schedule pressure 
>>>>
>>>> Next time you have schedule pressure, please be open about it. I'll do
>>>> my best to be flexible, at least if you try to do the same for me when
>>>> that time comes. I e, if you need a design to be ready by a certain
>>>> deadline; let us know your deadline.
>>>>
>>>> While there isn't a guarantee that I can commit time to your schedule,
>>>> at least having the option would be good for both of us.
>>>>
>>>>> for porting the old
>>>>> Murphy module in Tizen on top of this new routing system. If the new
>>>>> system is not fully functional (or lacks necessary features), the
>>>>> porting can't be done. If the design is done iteratively with the
>>>>> community, it will take too long to reach the fully working state.
>>>>
>>>> Sorry - I don't follow - if the design is not done with the community,
>>>> you'll probably end up having to redo what you have already done, so
>>>> then what's the point of porting anything to it?
>>>
>>> One point would be that with the new routing infrastructure in the core
>>> the module would be easier to maintain and more suitable for upcoming
>>> requirements. But I'm not really qualified to comment whether it makes
>>> more sense to port the module on top of the new stuff even if there's
>>> risk that it will all get rewritten, or to limp on with the old version
>>> (I would much prefer the latter).
>>>
>>>>> So, it's the usual problem of doing stupid things due to time
>>>>> constraints. What's the way forward? I talked with Janos and Jaska, and
>>>>> we agreed that I'd start sending out patches now, even though the
>>>>> backend support for node allocation during planning is incomplete, and
>>>>> the routing plan execution (connection building) is mostly
>>>>> unimplemented. I don't know how that will work out schedule-wise, so
>>>>> perhaps the plan will change again...
>>>>>
>>>>> These are the critical points identified by Janos that I should try to
>>>>> get to upstream ASAP (like within a month) to facilitate the Murphy
>>>>> module porting:
>>>>>     - The node structure (this is already done, although in a separate
>>>>> branch).
>>>>>     - Backend implementation for nodes (this is already almost done,
>>>>> although in a separate branch).
>>>>>     - Support for two-phase routing (i.e. the idea that first we create
>>>>> a routing plan, and only after that's complete, we do the actual routing
>>>>> actions). I need to discuss with Janos again what he meant with this,
>>>>> because this item sounds unrealistic with all the missing code for
>>>>> connection building. It might be that it's sufficient if I manage to
>>>>> convince you that we really need this, so having the implementation
>>>>> ready isn't that critical.
>>>>>     - Either install the core headers for supporting out-of-tree
>>>>> modules, or get the Murphy module to upstream. I need to clarify what
>>>>> the actual requirement is. It might be that what is really needed is an
>>>>> agreement that upstream is not fundamentally opposed to having the
>>>>> Murphy module in upstream as an optional module (implying optional
>>>>> dependencies to Murphy and Lua). Tizen has carried patches for enabling
>>>>> out-of-tree modules so far, so what's the problem with keeping doing
>>>>> that?
>>>>>
>>>>>> To be a bit more constructive, let me give you an example of how to
>>>>>> instead work in iterations. See it just as an example, because it does
>>>>>> not take into account the work already done (and I don't know the
>>>>>> details as well either).
>>>>>> In this example, in every step, there would be a review and merge into
>>>>>> the main branch of PulseAudio.
>>>>>>
>>>>>> Step 1) Introduce node concept. We need only to create nodes for streams
>>>>>> and ports at this point (because we're not sure whether to add a port to
>>>>>> sinks which does not have one).
>>>>>> Make an extremely simple router which just routes to the default
>>>>>> sink/source.
>>>>>>
>>>>>> Step 2) Improve router so that it also replaces the work of
>>>>>> module-device-restore and module-stream-restore.
>>>>>>
>>>>>> Step 3) Improve router so that it also replaces the work of
>>>>>> module-switch-on-port-available.
>>>>>>
>>>>>> Step 4) Add an API which makes it possible to route from a node to
>>>>>> another. Again, just focus on the most common cases first, we can return
>>>>>> -ENOIMPL if the user uses the API for something we don't support at this
>>>>>> point.
>>>>>>
>>>>>> Step 5, 6 etc) Improve the routing engine to handle more and more cases,
>>>>>> including automatically loading loopback modules etc.
>>>>>>
>>>>>> ...and so on. Working in iterations might cause a few detours along the
>>>>>> way, and we might not end up where we think we would when we started,
>>>>>> but probably somewhere better.
>>>>>
>>>>> I'd imagine the steps with incremental development would be more like
>>>>> this:
>>>>>
>>>>> 1) Add a flag in the core, which indicates whether pulseaudio should use
>>>>> the old routing model or the new one. Check the flag wherever someone
>>>>> tries to set pa_sink_input.sink or change a port or profile. If the flag
>>>>> is set, fail the attempted operation. The flag would be controlled by a
>>>>> special module, so if that module is not loaded, everything works as it
>>>>> used to, and if that module is loaded, nothing works.
>>>>>
>>>>> 2, 3, 4) Add the core routing infrastructure piece by piece.
>>>>>
>>>>> 5, 6, 7) Add the backend support.
>>>>>
>>>>> 8) Implement the fallback policy (default sink/source routing).
>>>>>
>>>>> 9, 10, 11) Fix what was broken at step 1. I would hope that
>>>>> module-stream-restore and other old routing modules could become
>>>>> functional at this point, without actually needing to do big changes to
>>>>> those modules (for example, when module-stream-restore sets the sink for
>>>>> a new sink input, that could be automatically mapped to an explicit
>>>>> connection request).
>>>>>
>>>>> 12) Remove the flag from the core and the special module, they have
>>>>> become unnecessary.
>>>>>
>>>>> 13) Add module-murphy.
>>>>>
>>>>> 14) Maybe replace module-stream-restore and friends with some new
>>>>> module(s).
>>>>>
>>>>> The order of the steps can be almost anything, except that step 1 should
>>>>> be the first one and step 12 can't be very near the beginning.
>>>>
>>>> It will be very difficult for me to do reviewing if you insist on such a
>>>> plan; because the final result is so far away. I don't know if I'm
>>>> capable of doing it, at least not with the limited resources I have.
>>>
>>> What's the difficulty with reviewing? 
>>
>> I don't know if I can explain it better. Reviewing becomes more
>> difficult as a patch set becomes bigger and more complex. And your way
>> of doing iterations will not really help, if we don't get any benefit
>> until the last step. Reviewing the big node patch set was quite
>> difficult when you don't see all the steps in the chain.
> 
> By "steps in the chain" do you mean the steps that are going to follow
> the initial node patch set? 

Yes, everything from "posted patch set" to "problem solved".

> I don't/didn't see those steps myself
> either, except as vague ideas. The code didn't exist. I thought that
> "nodes are certainly something we need", and coded according to that. My
> job is to provide enough explanation along with the patches so that you
> can think "I can see how this will be useful, even though we don't get
> practical benefits quite yet". If you don't understand why something is
> in the code (which is what I assume you mean by the difficulties in
> review), then I have failed at providing enough explanation (or
> alternatively the code is just bad and should be changed).

The problem is that the explanation, if thorough enough to judge whether
the code is bad or not, will be as complicated as writing the rest of
the code.

I would have tried a top-down approach instead, i e start with one
problem, and provide the code needed for solving that problem. If
solving that one problem needs to more than, say, 10 big complex
patches, then you chose the wrong problem, or tried to solve more than
one problem.

That said; I'm not an work flow expert. I'm just trying to figure out
what would make me able to make a qualified review.

>>> Nobody knows the final result, not
>>> even me, although I have pretty good idea of how it will look like. I
>>> will produce code that I think will make sense for meeting the
>>> requirements, and I will explain to you and other reviewers why you
>>> should think that code makes sense too. I will make sure there are no
>>> regressions on the way (with the flag controlling whether the new system
>>> is used or not), so the code can be merged to master small pieces at a
>>> time. The worst thing that can happen is that the design turns out to be
>>> bad and needs to be reverted. 
>>>
>>>>> My point is that it may not be possible to have a linear progression
>>>>> from a "simple system" to a "complex system", like in your example. 
>>>>
>>>> The idea that you intend to create a "complex system" is in itself
>>>> frightening. Please instead try to make a system that is as simple as
>>>> possible given the things you need to handle.
>>>
>>> That's exactly what I've been doing. It's just that "as simple as
>>> possible" doesn't seem to be very simple, but I may get proven wrong.
>>
>> Then try to split the complexity into parts, where each part tries to
>> solve its own complexity problem. As outlined in the section right below:
>>
>>>> To elaborate: we will have a lot of consumers of PulseAudio that are not
>>>> using Murphy. We don't want all these consumers having to carry the
>>>> additional complexity that comes from Murphy integration. OTOH, if there
>>>> are good features that come out of this project that is likely to be
>>>> useful to anyone, we want that.
>>>> How can we find the right intersection, or compromise, where we can get
>>>> nice features without the additional complexity?
>>>>
>>>> As an example of what is bad with complexity, consider the rewinds
>>>> feature, that has kept us from having a proper effects API, because it
>>>> was too difficult to implement when you had to take rewinds into
>>>> account. In theory the complexity of rewinds is not a blocker for an
>>>> effect API, but in practice, it has been.
>>>>
>>>> What future features that will be blocked from having a complex routing
>>>> system remains to be seen, but it is not unlikely this will happen at
>>>> some point.
>>>>
>>>>> I
>>>>> don't want to design and implement core infrastructure in the middle
>>>>> that I know won't scale to what we will need to be able to handle.
>>>>
>>>> Yes, I understand this position. When working in iterations, you
>>>> sometimes end up taking detours. That's the drawback. But the benefits
>>>> of risk reduction, easier reviews etc is greater. IMO.
>>>
>>> If a middle point contains nothing that can be reused in the final
>>> design, the middle point is all wasted effort. 
>>
>> It wouldn't even count as a middle point if *nothing* can be reused. But
>> it really isn't *that* bad, is it?
>>
>>> But if you insist that
>>> it's very very important to get rid of the "use new system" flag as soon
>>> as possible (i.e. reach a point where the new system can be enabled by
>>> default), I can try to think of the smallest possible design that would
>>> allow that, and yet would move towards what I think should be the end
>>> result.
>>
>> It would be good to have that as an option, but I can't really
>> understand why there has to be a flag at all, instead we can have both
>> routing systems work in parallel during a transition period?
> 
> If the two systems work in parallel, there will be two systems
> controlling the same resources (stream routing, profiles, ports), and
> conflicts will be highly likely (or rather, avoiding conflicts will
> require a lot of extra work at least in terms of thinking about all the
> possible scenarios, but perhaps also in terms of code). It is perhaps
> feasible to move the controlling of routing/profiles/ports piece by
> piece from the old system to the new, but the benefit of the flag
> approach is that it's much easier to be confident that there are no
> regressions during the transition phase.

By taking care of the regressions when they show up rather than saving
all regressions to some distant future, you reduce the risk of going
somewhere where the regressions will be more difficult to solve.

> That said, this discussion has forced me to think how I might go about
> moving the control of routing/profiles/ports piece by piece if I really
> had to, and it feels less impossible than I previously thought.

Ok.



-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic