Bug 47296

Summary: Call: Can not be used with Approvers
Product: Telepathy Reporter: Olivier Crête <olivier.crete>
Component: tp-specAssignee: Telepathy bugs list <telepathy-bugs>
Status: RESOLVED MOVED QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: normal    
Priority: medium    
Version: git master   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: proposed spec patch

Description Olivier Crête 2012-03-13 17:57:40 UTC
Created attachment 58410 [details] [review]
proposed spec patch

The way the Call spec works clashes with the way approvers where designed. I believe that the Call bit is the best way of doing things.

The problem is that the Call spec says that the user should not be notified of an incoming call before the Call reaches the INITIALISED state, but it will only reach that state when network connectivity has been been established, which requires a Handler to be present. So the handler need to runs before the approver. Which isn't how the Approver/handler interaction currently works.

In the short term, I'm proposing using BypassApproval for all Call channels so they go straight to the Handler.. And instead implement other Approvers as Observers that call Accept or Hangup on the channel... If an Observer (not the Handler) calls Hangup, then it would be the responsability of the Handler to call Close (so we can do CLEARING states correctly).


For the NEXT branch, I suggest we re-work the approver/handler process around the same principles for Call/FT/Tubes:

- Have a single Handler per type and call it right after observers
- add an optional Approvable interface that woudl contain: Accept(), Reject() and a property that would be "ReadyToAnnounce"
- have the approvers be called only if the Approvable interface exists and ReadyToAnnounce has turned to True
- Drop the DispatchOperation (do we really have any case where you dispatch more than one channel at the same time).
Comment 1 Xavier Claessens 2012-03-14 02:52:37 UTC
It is not so easy to fix actually. Take for example the case where you have both KDE and GNOME installed. Both desktop environments will provide a different call handler.

With current design, the idea is that if gnome-shell is running, that will be the approver UI showing the channel first, and then it can decide to HandleWith(empathy-call). Note that I'm not sure it actually does, but it could.

There is no way to know which handler to use before going through approvers, unless we add hacks in MC to check if org.gnome.Shell is on the bus or something like that...
Comment 2 Olivier Crête 2012-03-14 06:39:35 UTC
Maybe we need some kind of config that MC reads to figure out which handler is to be used for which channel type.
Comment 3 Olivier Crête 2012-03-14 08:59:41 UTC
In the short term, they haven't made releases of the KDE handler yet, but I guess something smarter should be done before they do.
Comment 4 Simon McVittie 2012-03-23 06:25:07 UTC
(In reply to comment #0)
> The problem is that the Call spec says that the user should not be notified of
> an incoming call before the Call reaches the INITIALISED state, but it will
> only reach that state when network connectivity has been been established,
> which requires a Handler to be present. So the handler need to runs before the
> approver. Which isn't how the Approver/handler interaction currently works.

Why does the Call spec say this? The only rationale I can find is "to reduce the probability that the user will pick up the call and have it immediately fail". In what situations would this happen? Being behind a particularly nasty firewall?

I feel as though the user should be notified that someone tried and failed to call them (or at least Telepathy shouldn't make it unnecessarily hard to do so), but it should probably look more like an instantly-missed call than an "answer?" prompt, yes.

The Call spec as written defeats part of the purpose of Approvers, which was to avoid taking up memory/CPU time/bandwidth for an operation that the user hadn't agreed to (being in a call). Perhaps that's a necessary sacrifice to make VoIP reliable, though - you want to start the ICE process as quickly as possible, right?

It's possible for a Call Handler to be an Observer, Claim() the CDO non-interactively, and deal with notification and "user-approval" internally. I think the N900/N9 might have done this? This would defeat parts of the purpose of Approvers while preserving others.

The other major purposes that I can remember for Approvers are:

* Be a second phase of dispatching, strictly after Observers. This ensures
  that bits of UI from Approvers don't pop up in the user's face if an
  Observer has already taken action as a non-interactive approver: for
  instance, a call-screening implementation could be an Observer and
  pre-emptively reject calls from undesirables, without the user ever
  seeing a notification bubble or hearing a ringtone. This part wouldn't
  be defeated by having Handlers do approval.

* Avoid having a UI pop up and interrupt the user. Calls have to do this
  a bit to meet normal expectations of how intrusive/urgent a call should
  be treated as, but a Call Handler that does its own notification
  and user-approval can minimize this.

* Avoid divulging that the user is even online at all
  if they don't want to talk to the remote party. If you want to start
  ICE as early as possible, then this isn't really possible, though.

* Choose the Handler explicitly (for "always give the user every
  possible choice" UIs) or implicitly (the user can be nagged by more
  than one Approver, but can only say "yes" to one of them, and that
  Approver's favourite Handler gets priority).

One reason for multiple Approvers is "the full-screen media player use case". While running, a full-screen media player which you can run on your desktop or a dedicated machine (I think the one we discussed was Elisa?) wants to be an Approver so *it* can notify you of incoming calls; Empathy/Shell in the background will still notify you, but not visibly, because Elisa is in the way.

Another reason for multiple Approvers is that you can have your Bluetooth headset and your touchscreen be independent Approvers, which say "yes" when you press the "answer" button on either the headset or the touchscreen. (I think the N900 might even have implemented this?)

> For the NEXT branch, I suggest we re-work the approver/handler process around
> the same principles for Call/FT/Tubes:
> 
> - Have a single Handler per type and call it right after observers

How can this work if you have both Empathy and telepathy-kde installed? Would they both have their Handler not be activatable, and run it as a side-effect of running a main process or entering the desktop session?

In the Elisa use-case, how does Elisa take over from Empathy or telepathy-kde and impose itself as the preferred Handler?

(Not entirely a rhetorical question: one possible answer is to do clever things with D-Bus name-ownership semantics - but that requires a special case for Call channels in the spec, rather than using generic dispatching machinery that doesn't know about channel types, and I think that'd be a backward step.)

The same questions apply to Text channels.

> - Drop the DispatchOperation (do we really have any case where you dispatch
> more than one channel at the same time).

(We do, in MUC Tubes channels, but nobody copes with it.)

The CDO needs to exist in MC in some form - it contains most of the logic for actually dispatching channels (at least since I refactored it - before I did that, McdDispatcher was giant and impossible to understand). Channel requests also have a secret CDO which isn't visible on D-Bus, but exists internally.

It'd be possible to have the dispatcher D-Bus API be entirely in terms of calling a method on the ChannelDispatcher and passing the Channel (or Channels?) you're talking about as an argument,  but I don't think that'd help clarity.

The CDO is also somewhere we can put things that are part of MC's state, but not part of the Channel in the CM. There's not much of that yet, but the list of possible handlers is there, and I think it's somewhat important to have it as an extension point for the future. (In outgoing calls, the ChannelRequest takes on this role, which is partly why the CDO isn't made visible on D-Bus.)
Comment 5 Olivier Crête 2012-03-28 13:45:52 UTC
(In reply to comment #4)
> (In reply to comment #0)
> > The problem is that the Call spec says that the user should not be notified of
> > an incoming call before the Call reaches the INITIALISED state, but it will
> > only reach that state when network connectivity has been been established,
> > which requires a Handler to be present. So the handler need to runs before the
> > approver. Which isn't how the Approver/handler interaction currently works.
> 
> Why does the Call spec say this? The only rationale I can find is "to reduce
> the probability that the user will pick up the call and have it immediately
> fail". In what situations would this happen? Being behind a particularly nasty
> firewall?

That is one thing, also, the other big advantage of doing ICE as early as possible is that the call will be immediately connected after you accept it.

> I feel as though the user should be notified that someone tried and failed to
> call them (or at least Telepathy shouldn't make it unnecessarily hard to do
> so), but it should probably look more like an instantly-missed call than an
> "answer?" prompt, yes.

Yes, one should be notified. But probably only a bubble or something in the log, not a incoming call that you can't answer...

> The Call spec as written defeats part of the purpose of Approvers, which was to
> avoid taking up memory/CPU time/bandwidth for an operation that the user hadn't
> agreed to (being in a call). Perhaps that's a necessary sacrifice to make VoIP
> reliable, though - you want to start the ICE process as quickly as possible,
> right?

The Call spec definitely breaks Approvers, hence me proposing something else. The amount of RAM/CPU/etc that will be used to just connect the call without accepting it is minimal, even for an embedded device.

> It's possible for a Call Handler to be an Observer, Claim() the CDO
> non-interactively, and deal with notification and "user-approval" internally. 
> think the N900/N9 might have done this? This would defeat parts of the purpose
> of Approvers while preserving others.

That doesn't really work with the current spec, what if the approver gives the channel to a different handler? Well we've lost, because we can't change handlers.

> The other major purposes that I can remember for Approvers are:
> 
> [...]
>
> * Avoid divulging that the user is even online at all
>   if they don't want to talk to the remote party. If you want to start
>   ICE as early as possible, then this isn't really possible, though.

This is entirely pointless. In XMPP, you need the presence to initiate a call. And in SIP, the first thing the CM does is to send a "provisional ack" (PRACK) telling the sender that it received the INVITE request.


> * Choose the Handler explicitly (for "always give the user every
>   possible choice" UIs) or implicitly (the user can be nagged by more
>   than one Approver, but can only say "yes" to one of them, and that
>   Approver's favourite Handler gets priority).

You can't change the Handler choice if you start it before the Approver anyway.
 
> One reason for multiple Approvers is "the full-screen media player use case".
> While running, a full-screen media player which you can run on your desktop or
> a dedicated machine (I think the one we discussed was Elisa?) wants to be an
> Approver so *it* can notify you of incoming calls; Empathy/Shell in the
> background will still notify you, but not visibly, because Elisa is in the way.

I think that case could be handled by having a well-known name that the full-screen app can take over as long as it does it before the call comes in. We could use something like Lennart's ReserveDevice spec http://git.0pointer.de/?p=reserve.git;a=blob;f=reserve.txt 

> > For the NEXT branch, I suggest we re-work the approver/handler process around
> > the same principles for Call/FT/Tubes:
> > 
> > - Have a single Handler per type and call it right after observers
> 
> How can this work if you have both Empathy and telepathy-kde installed? Would
> they both have their Handler not be activatable, and run it as a side-effect of
> running a main process or entering the desktop session?
>
> In the Elisa use-case, how does Elisa take over from Empathy or telepathy-kde
> and impose itself as the preferred Handler? 

I think one solution is to have some kind of configuration that is set on mission-control by the desktop-specific session manager maybe ? Or add some keys to the .client file like .desktop files have.

> (Not entirely a rhetorical question: one possible answer is to do clever things
> with D-Bus name-ownership semantics - but that requires a special case for Call
> channels in the spec, rather than using generic dispatching machinery that
> doesn't know about channel types, and I think that'd be a backward step.)

I think we should re-work all channel types. For Jingle file-transfers, it probably makes sense to work like Calls (as a file transfer that doesn't connect is pretty much useless). Same thing can be said of any kind of tube where the IBB mode isn't fast enough (for VNC tubes for example).

> The same questions apply to Text channels.

If you ask me, p2p Text channels should probably only be handled by the logger. And other apps should access the data through the logger. I believe this is how the N9 works (where tracker is their logger)?

> > - Drop the DispatchOperation (do we really have any case where you dispatch
> > more than one channel at the same time).
> 
> (We do, in MUC Tubes channels, but nobody copes with it.)

So it's pretty much pointless..

> The CDO needs to exist in MC in some form - it contains most of the logic for
> actually dispatching channels (at least since I refactored it - before I did
> that, McdDispatcher was giant and impossible to understand). Channel requests
> also have a secret CDO which isn't visible on D-Bus, but exists internally.

How it's implemented inside MC isn't really important imho.

> It'd be possible to have the dispatcher D-Bus API be entirely in terms of
> calling a method on the ChannelDispatcher and passing the Channel (or
> Channels?) you're talking about as an argument,  but I don't think that'd help
> clarity.

I think the Approval API really needs to be on the Channel itself, not on some other object inside MC, that's the basic problem here. I should not that FileTransfer, Tubes and Call all have a Accept() call of some kind.

> The CDO is also somewhere we can put things that are part of MC's state, but
> not part of the Channel in the CM. There's not much of that yet, but the list
> of possible handlers is there, and I think it's somewhat important to have it
> as an extension point for the future. (In outgoing calls, the ChannelRequest
> takes on this role, which is partly why the CDO isn't made visible on D-Bus.)

I think we should avoid designing for things that may or may not exist at some point in the unpredictable future, as unpredictable things tend to not be what we expect them to be. And we should rather concentrate on problems that exist today with the things that we are trying to achieve.
Comment 6 Olivier Crête 2012-05-10 15:17:32 UTC
Another (more hackish) way to do it would be to add a "SelectHandler(s:bus-id)" call to the CDO where the Observer (inside EmpathyCall) could tell MC to reject Claim() and HandleWith() for a different handler, so that Empathy could handle it early.

Also of interest would be a "IgnoreMe()" call on the CDO where a approver can tell MC that it will not be doing anything for a specific CDO, for example Empathy Call only does it's thing if it already has a window for that Contact. Then MC, if all approvers say IgnoreMe() would do as if there was no approver and just dispatch the channel.

And just for safety, we could also add a HandledBy(s: unique-bus-id) signal to the CDO so the observer can know who is going to handle the channel. So if Empathy started handling a channel and MC gives it to someone else, it will know to back off.
Comment 7 GitLab Migration User 2019-12-03 20:26:07 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/telepathy/telepathy-spec/issues/134.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.