On 11 April 2015 at 17:19, Philipp Hancke <fi...@goodadvice.pages.de> wrote:
> hey Ben, > > >> Yes. It is certainly better than doing something like uaCSTA ( > >> http://www.ecma-international.org/publications/files/ECMA-TR/TR-087.pdf) >>> over XMPP. >>> >> >> >> I'll note as a preface to my below comments that CSTA is somewhat of a >> motivation for Rayo being the way it is: stupidly simple. CSTA had the >> ECMA >> treatment and turned into the monster that it is, which no-one with a >> turnover of < $100MM can realistically implement properly. >> > > +1000 > > [snip] > > Example 1 uses node='urn:xmpp:rayo:call:1' -- this seems odd, this should >>> typically contain the client's node and not the urn of the specification. >>> Together with the text in 6.2.2 this looks like an invalid use of caps to >>> me. I'd suggest removing this, this might require changes to the >>> registration process in example 12. >>> >>> >> This was introduced at >> https://github.com/rayo/xmpp/commit/a9f8a448b7fe7cfd7088204e9f2256 >> 202ac94b58. >> I would love to hear alternative suggestions for identifying the type of a >> Rayo entity (call or mixer). >> > > calls and mixers should have different features and hence different caps > strings, right? > > I am assuming it is easy to map caps hashes to caps. This can be tricky to > implement in some situations. In practice, you might hardcore the caps > hashes of rayo servers or calls so you can avoid the disco dance as often > as possible. Noted. I'll propose a patch to deal with this. > The use of directed presence is somewhat odd, I think this should be a >>> <message/>. The rationale for this seems to be having the presence track >>> the session status and allowing the call control server to notice if the >>> client closed its connected to the server unexpectedly. >>> >>> >> I figure you mean for an <offer/>? This is the point at which a new call >> entity comes online on the network, in much the same way as an IM user's >> resource comes online, and such a change in entity presence naturally fits >> with <presence/>. I'm not sure why this would be considered a message. >> > > That's fine. This raises your xmppishness score even :-) > > > If you mean for client registration (6.1), see below. >> >> >> 6.1: the rationale for using presence and <show/> here is that subsequent >>> presence broadcasts which change the <show/> will also be sent to the >>> rayo >>> server? If so then (surprise) it doesn't work due to the way >>> https://tools.ietf.org/html/rfc6121#section-4.4.2 is written (surprise!) >>> >>> It's not clear to me if there is a presence subscription between the >>> client and the rayo server. >>> >>> >> This functionality generally exists to permit a client to quiesce on >> shutdown. Adhearsion's behaviour is to send DND presence on SIGTERM, in >> order that the Rayo server not send it any new calls. It then waits for >> existing calls to clear, before finally shutting down. Additional SIGTERMs >> progress through the shutdown process faster, rejecting any new calls sent >> by a leaky server, and eventually hanging up existing calls before >> shutting >> down. >> > > Did you consider sending unavailable without closing the stream? That > terminate your presence session but I don't think you're relying on the > behaviour of being an "active resource". > > The only reason I can see for not doing this is that it makes it harder to > determine for the rayo server if that is coming from the client or is sent > from the server because the clients connection died. And that's why I didn't do this. > It is intended that a presence subscription be created for the purpose you >> describe above, but I agree this section could do with clarification. I >> would love to hear suggestions for how we might make that clearer. >> > > How about a section showing that the rayo server subscribes to the users > presence (and possibly this creates a bidirectional subscription)? > > I think the protocol should still work without relying on it, but it gives > the rayo server more information (such as a change of the user to an away > status) which a smart rayo server might take into account when routing > calls in multiclient/multidevice scenarios. Noted; patch coming. > 6.2.1 Outbound Call: the link for dial command is broken. >>> >>> >> Fixed at >> https://github.com/rayo/xmpp/commit/b2a7c43c7b5a5559e64db723781b9b >> 9d6bd3be3b >> > > ta. > > > Example 22: I don't think using presence for this is appropriate. Here and >>> e.g. example 44. Active speaker detection is NOT a presence. >>> >>> >> All state changes for an entity are signalled using a change to presence >> like this. Presence is used as a generic event package. Alternative >> > > Right. And this is what I am objecting. While we have done this in the > past, the "modern" thing is to use pubsub for it. > > > proposals welcome that cover all such cases, but example 22 is not >> sufficiently different from the rest of the protocol to warrant specific >> treatment. >> > > The alternative would be: > <message from='pubsub.shakespeare.lit' to='franci...@denmark.lit' > id='foo'> > <event xmlns='http://jabber.org/protocol/pubsub#event'> > <items node='princely_musings'> > <item id='ae890ac52d0df67ed7cfdf51b644e901'> > <ringing xmlns='urn:xmpp:rayo:1'/> > </item> > </items> > </event> > </message> > > Due to the transient nature of ringing, you could use transient > notifications: > <message from='pubsub.shakespeare.lit' to='franci...@denmark.lit'> > <event xmlns='http://jabber.org/protocol/pubsub#event'> > <items node='ringing'/> > </event> > </message> > > Arguably, more overhead. Quite some overhead in stanza size, and quite some overhead in number of spec lines to be read in order to implement a server or client. See my note about a simple protocol. > Again, these are events like any other, indicating a change in state of >> the >> resource. Thinking about it, this case is *somewhat* similar to XEP-0085, >> except that they are aggregated across entities at the mixer level and >> then >> disambiguated via an attribute. Again though, I don't think this is >> sufficiently different from other events to warrant specific treatment. >> >> >> I think the whole presence stuff should work in a different way. The >>> client should have a directed presence exchange with the rayo server. And >>> inside that <iq/> or <message><pubsub/></message> should be used. >>> >>> >> Why do you think it should work differently? I'm happy for it to do so if >> there's a strong motivation, but I've yet to hear a convincing argument >> against the current scheme. >> >> IQ is used for command execution. IQ doesn't seem to jive with the idea of >> an event, though. I could perhaps learn to live with these being PubSub, >> but that's a humongous dependency for what is currently a very simple >> protocol. >> > > basically you'd just use example 2 from xep-0060 instead of <presence/>. > > Possibly, you could model the command part using xep-0060 as well but that > seems like overkill. Or you could use adhoc commands... but might want to > avoid forms. Too many solutions to a simple problem... > Ad-hoc commands don't appear to add anything beyond the simple IQ semantics on which we currently depend. I've implemented it before and am still unsure of the value of that spec. > alot of it depends on the definition of a simple implementation. My definition is the number of lines of specifications one must read in order to implement. XEP-0060 is a huge spec, and raises the barrier to entry for understanding of Rayo. If XEP-0060 had a simple profile which was very small, we might depend on that, but as it is, it's a big price to pay. I think, on evaluating the tradeoff between the elegance of such a dependency and the simplicity and deployed nature of the current protocol, this (and MUC below) might be candidates for a v2 of the protocol. > Lance says this should use MUC. >>> >>> >> A dependency on XEP-0045 would kill this specification. I have no desire >> to >> go down that road. >> > > You are halfway down already with the bidirectional exchange of directed > presence. > > In fact, MUC might be appropriate in multi-agent scenarios even. But > afaics it should just work when used in a MUC (the call joins the MUC by > sending direct presence and someone inside the MUC accepts...). I'm not sure what a multi-agent scenario or the concept of a call joining a MUC are. Are you suggesting replacing Rayo mixers with MUC? > It might be too late to fix that, depending on how widely this is >>> deployed. >>> >> >> >> We would have to bump the protocol version (which I guess a move to Draft >> would to anyway, to :1) and be careful about allowing a smooth upgrade >> > > In general, we don't need to bump the protocol version just for moving to > draft. You just confused me with this. > Section 9 uses rayo:0 for discovery whereas rayo:1 is used everywhere > else. I presume rayo:0 is a glitch? > Correct. Fixed at https://github.com/rayo/xmpp/commit/1e36ba230261e59d29a96d0528f01c0b105ffbab . > Oh... and section 8 (use cases) needs to be removed before this can move > to draft. Unless you volunteeer to write one, but I think that is already > sufficiently covered. Fixed at https://github.com/rayo/xmpp/commit/5486042d1fad8898ea1f9695c5d7d46072bfbc9c > path, but it's not absolutely out of the question. I don't, however, want >> to go down a rabbit hole without a particularly good reason. >> > > Right. I do not think breaking code just to create a more "beautiful" > solution is worth it. > > philipp >
