On 08/05/2013 01:37 PM, Tanu Kaskinen wrote:
> On Wed, 2013-07-17 at 12:26 +0200, David Henningsson wrote:
>> On 07/17/2013 11:22 AM, Tanu Kaskinen wrote:
>>> On Wed, 2013-07-17 at 09:27 +0200, David Henningsson wrote:
>>>> On 07/16/2013 03:20 PM, Tanu Kaskinen wrote:
>>>>> Yes, there can be both an explicit and a default connection between two
>>>>> nodes, or it can be also thought as being one connection that is both
>>>>> explicit and default. I don't know what is a better way to think about
>>>>> it, but perhaps it doesn't matter anyway.
>>>>
>>>> I would prefer the latter way. I would then see "default" and "explicit"
>>>> as a property of the edge.
>>>
>>> I agree about that, at a conceptual level. Would you perhaps like to
>>> have it this way also at the code level, i.e. have connection objects
>>> with "default" and "explicit" fields instead of storing the information
>>> in the nodes?
>>
>> Good question. Actually one option could be to store the connections as
>> global list-like matrix instead of lists on the individual node objects.
>> After all, you'll need the connection on both sides, and unless you want
>> to duplicate information, you'll have to do a global search on one of
>> the sides anyway. Since the number of nodes < 100, I don't think this
>> would matter from a performance standpoint.
>>
>> You would end up with a global list with entries looking something like:
>>
>> struct pa_node_connection_t {
>> pa_node *from_node;
>> pa_node *to_node;
>> bool default;
>
> A single "default" boolean isn't enough. A connection may be part of
> both endpoints' default routing, or just one. It's necessary to
> differentiate between the different cases, because disabling one node's
> default connections may not remove the connection, if it's also a
> default connection of the other node.
I'm not sure I know the background enough to understand why you need to
store whether the connection is done on the basis on a request from the
from_node or to_node (?), but at this point, sure, we can have a bool
from_default and a bool to_default if that solves your problem.
>
>> bool explicit;
>> bool explicitly_disabled;
>> };
>>
>> Where there is only one entry for every from_node/to_node combination,
>> explicit and explicitly_disabled cannot both be true, and there is a
>> connection if and only if (!explictitly_disabled) && (explicit || default).
>> And no list entry for a node combination, is the same as all three bools
>> being false.
>
> I don't like the idea that there would be a connection object also for
> connections that don't exist. If the "explicitly disabled" feature is
> needed (I will argue later in this mail why it's not needed), it can be
> implemented in some other way, such as a separate blacklist of
> connections.
>
> I do, however, think that having separate connection objects is a good
> idea after all. I think it's entirely possible that we would at some
> point want to have additional properties for connections (I don't have
> any examples in mind, though), or we might want to refer to a connection
> with some more convenient way than with a pair of nodes.
>
>>>>> I don't like the idea that if a client wants to move a stream, it has to
>>>>> break the operation down to "remove + add", and then the server tries to
>>>>> guess what the client really meant. Having a "move" operations keeps the
>>>>> client intention obvious.
>>>>
>>>> Fair point, although the move can be on both sides, whereas one type of
>>>> move would be "I want node A to take data from C instead of B" and the
>>>> other type would be "I want node A to output data to node C instead of B".
>>>
>>> These both are served by the move operation.
>>
>> In the RFC you only have pa_context_move_node_connection, which serves
>> the latter case only. The former case is from "C -> A" to "B -> A", the
>> typical example would be a client wanting to move a recording stream
>> from one source to another.
>
> If you think so, then it's a bug in the documentation. I really meant to
> support both cases with pa_context_move_node_connection().
>
>>>> Or perhaps you want to swap, so that if you're currently on "A -> B and
>>>> C -> D" and you want "A -> D and B -> C" and you want to do all of this
>>>> atomically. The number of operations you want to do might grow out of
>>>> hand unless you have a "batch mode".
>>>
>>> The core doesn't currently have a concept of atomic swapping, so I don't
>>> see the need for a separate swap command, but the core has a concept of
>>> atomic moving.
>>
>> Well, IIRC, we have move_start and move_finish commands? A swap command
>> would just be two move_starts and then two move_finish.
>
> Good point. You got me convinced :) So, let's forget about the separate
> move operation, and perhaps also the separate add and remove operations?
> pa_context_edit_connections() should be good enough for everything.
>
>>>>> What operations do you mean? Moving or removing a default connection is
>>>>> not supported as such, but if the client tries that anyway, we can
>>>>> implicitly convert the connection to an explicit one and disable default
>>>>> connections, or we can require the client to do these operations
>>>>> explicitly, but I think the latter would be too inconvenient for the
>>>>> client.
>>>>
>>>> Ok, I think I didn't read the proposal well enough. Having done that, I
>>>> understand that you're suggesting a global switch "default connections
>>>> on/off" only. Or is it a per-node switch?
>>>
>>> It is a per-node switch.
>>>
>>>> I have another idea that might be worth considering: how about that the
>>>> "explicit" layer can both enable and disable connections? So that there
>>>> could be a default connection between A and B, but there is also some
>>>> sort of explicit override that disables it. This would be more flexible
>>>> than a more global on/off switch.
>>>
>>> I'm not sure what you mean. Do you perhaps mean that the default
>>> connection on/off switch should be per-node (which it already is in my
>>> proposal), or that it should be per-connection (so that if there are
>>> multiple default from node A, it's possible to disable only a subset of
>>> those)?
>>>
>>> I didn't make make it possible to disable individual default
>>> connections, because I had a feeling that it would have very messy
>>> semantics. If default connection from A to B is disabled, what is the
>>> routing code supposed to do when conditions change and the default
>>> routing is re-evaluated? Can it ever reactivate the connection between A
>>> and B again? Is the per-connection disabling handled as a blacklist of
>>> connections that must never be automatically activated?
>>
>> If the A -> B route is explicitly_disabled, that overrides any default
>> connections the routing system tries to make.
>
> What is the use case for explicitly disabled connections? I'll assume
> here that your idea was to allow moving a default connection elsewhere
> (making the connection explicit in the process) without disabling all
> default connections for the node.
>
> When the user moves a default connection, the routing system obviously
> shouldn't immediately create another default connection elsewhere to
> replace the disabled connection.
>
> On the other hand, if the routing system doesn't create replacement
> connections, then that results in weird behaviour. Let's say that
> there's a default connection A -> B, and the user moves the connection
> to A -> C. Then B disappears. The routing changes its opinion of the
> best available routing for A, which might be D. So removing node B
> resulted in audio suddenly appearing in node D, even though nothing was
> playing to B.
>
Assume your example of a default connection A -> B which the user
changes into A -> C, by adding an explicit A -> C connection. Without
some sort of explicitly_disabled blacklist that would then include A ->
B, the routing system would be free to route A to *both* B and C.
Whether this is implemented as a bool flag or as a separate blacklist is
an implementation detail, but a bool flag just seemed simpler and faster
to me, than having to look in several lists to figure out whether a
connection exists or not.
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
