Olivier asked me to review David's branch which adds codegen for Call. <http://cgit.collabora.com/git/user/alsuren/telepathy-glib.git/log/?h=call1> I've ended up rebasing it and generally doing some polish; I haven't actually reviewed it yet.
Obvious things missing include: gtk-doc. I haven't checked whether there are TpProxy subclasses for the non-Channel client-side bits of Call.
Added the gtkdoc bits: http://cgit.collabora.com/git/user/tester/telepathy-glib.git/log/?h=call1 There are proxies for the client-facing bits, but not for the tp-fs facing bits. That said, the next step is probably to import the relevant hand-written code from telepathy-yell.
Latest call1 code is available here: http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/log/?h=call1 It contains generated code for call1 spec, complete client-side high-level API (TpCallChannel, TpCallContent and TpCallStream) and WIP service-side high-level API (TpBaseCallChannel, TpBaseCallContent and TpBaseCallStream). TODO/open questions: * missing gtkdoc for TpBaseCall* * tp_base_call_channel_set_state() should set flags and details? Some flags like RINGING can be handled internally, but not all afaik. * tp_base_call_channel_update_member_flags() should emit StateChanged * TpBaseCallStreamRequestReceivingFunc should return gboolean for success * tp_base_call_*: add g_return_if guards * add padding to public class structs for future vtable * SetQueued no implemented * Why tp_base_call_channel_set_ringing_dbus() does not call a vmethod to let protocol inform the other side? * why is TpBaseCallContent the only object to have a deinit? * why does TpBaseCallContent and TpBaseCallStream register themself on the bus? other TpBase let subclass decide * Shouldn't tp_base_call_channel_hangup() remove all contents for us? or assert they are all removed after vmethod? * When removing a content, do we have to remove its streams before? should we assert that the content has no stream when removing it? * When hangup the call, should we remove members? * Should tp_base_call_channel_add_content_dbus() assert that vmethod called tp_base_call_channel_add_content()? * Why is initial_audio in public TpBaseCallChannel struct but not initial_audio_name? * TpyBaseCallChannel implements the DTMF iface, but does all protocal wants it? How are CMs supposed to implement DTMF, I see no way for TpyBaseCallChannel subclasses to access priv->dtmf_player... * Does mutable-contents==FALSE means we can't add and can't remove contents? is it possible to have protocols that can add but not remove or the other way around? Is it possible that a protocol can add or remove audio content but not video content? * Content::remove(): spec says Google Talk video call can't remove content and should raise Not Implemented (shouldn't it be NotCapable?) but TpyBaseCallContent does not let subclass define if it is supported. Should it just check the mutable-contents property?
You should really look at my WIP branch, I think you're duplicating some of the stuff I've already done..... the call1-serviceside branch on my git.collabora.co.uk tree. * missing gtkdoc for TpBaseCall* * tp_base_call_*: add g_return_if guards * SetQueued no implemented * add padding to public class structs for future vtable There are all in the TODO category. * Shouldn't tp_base_call_channel_hangup() remove all contents for us? or assert they are all removed after vmethod? * When hangup the call, should we remove members? I think we should leave the channel as is until it closed. I don't really see the point of removing members or contents. Note that channel closing may take some time after hangup because of the clearing state in cell phone calls. * When removing a content, do we have to remove its streams before? should we assert that the content has no stream when removing it? I think the user should be able to remove the Content and that should destroy all the streams/endpoints/mediadescriptions associated with it. * TpyBaseCallChannel implements the DTMF iface, but does all protocal wants it? How are CMs supposed to implement DTMF, I see no way for TpyBaseCallChannel subclasses to access priv->dtmf_player... I have a branch here that removes that dtmf player crap. It should go away entirely as all the timing is now delegated to the streaming implementation. My branch also implements the Media side of DTMF in TpMediaBaseContent. I think TpBaseCallChannel should implemement the interface but let the subclass decide to show it in the "Interfaces" property or not. * Does mutable-contents==FALSE means we can't add and can't remove contents? is it possible to have protocols that can add but not remove or the other way around? Is it possible that a protocol can add or remove audio content but not video content? Afaik, it means they can't change at all. This is mostly for Google Talk and some crap SIP servers. Protocols that support partial changes are actually negotiated, so you would add it and the other side would remove it. * Content::remove(): spec says Google Talk video call can't remove content and should raise Not Implemented (shouldn't it be NotCapable?) but TpyBaseCallContent does not let subclass define if it is supported. Should it just check the mutable-contents property? I think it should. * tp_base_call_channel_set_state() should set flags and details? Some flags like RINGING can be handled internally, but not all afaik. * tp_base_call_channel_update_member_flags() should emit StateChanged * TpBaseCallStreamRequestReceivingFunc should return gboolean for success * Why tp_base_call_channel_set_ringing_dbus() does not call a vmethod to let protocol inform the other side? * why is TpBaseCallContent the only object to have a deinit? * why does TpBaseCallContent and TpBaseCallStream register themself on the bus? other TpBase let subclass decide * Should tp_base_call_channel_add_content_dbus() assert that vmethod called tp_base_call_channel_add_content()? * Why is initial_audio in public TpBaseCallChannel struct but not initial_audio_name? I'll let Sjoerd answer these as I have no clue.
(In reply to comment #4) > You should really look at my WIP branch, I think you're duplicating some of > the stuff I've already done..... the call1-serviceside branch on my > git.collabora.co.uk tree. > > * missing gtkdoc for TpBaseCall* > * tp_base_call_*: add g_return_if guards > * SetQueued no implemented > * add padding to public class structs for future vtable > > There are all in the TODO category. > > * Shouldn't tp_base_call_channel_hangup() remove all contents for us? or > assert they are all removed after vmethod? > * When hangup the call, should we remove members? > > I think we should leave the channel as is until it closed. I don't really see > the point of removing members or contents. Note that channel closing may take > some time after hangup because of the clearing state in cell phone calls. Note that the spec says: "Request that the call is ended. All contents will be removed from the Call so that the Contents property will be the empty list." So we should change spec? > * TpyBaseCallChannel implements the DTMF iface, but does all protocal wants > it? How are CMs supposed to implement DTMF, I see no way for TpyBaseCallChannel > subclasses to access priv->dtmf_player... > > I have a branch here that removes that dtmf player crap. It should go away > entirely as all the timing is now delegated to the streaming implementation. My > branch also implements the Media side of DTMF in TpMediaBaseContent. I think > TpBaseCallChannel should implemement the interface but let the subclass decide > to show it in the "Interfaces" property or not. Ok, I'll pick that from your branch then. > * Does mutable-contents==FALSE means we can't add and can't remove contents? > is it possible to have protocols that can add but not remove or the other way > around? Is it possible that a protocol can add or remove audio content but not > video content? > > Afaik, it means they can't change at all. This is mostly for Google Talk and > some crap SIP servers. Protocols that support partial changes are actually > negotiated, so you would add it and the other side would remove it. Ok, so I added a mutable-contents construct-only property on TpBaseCallChannel and if it is FALSE then AddContent and Content::Remove fails. > * Content::remove(): spec says Google Talk video call can't remove content and > should raise Not Implemented (shouldn't it be NotCapable?) but > TpyBaseCallContent does not let subclass define if it is supported. Should it > just check the mutable-contents property? > > I think it should. ok, it's done like that in my branch now.
I updated my branch with all the TODO now done. Remaining only a few questions: * tp_base_call_channel_set_state() should set flags and details? Some flags like RINGING can be handled internally, but not all afaik. * tp_base_call_channel_update_member_flags() should emit StateChanged? * why is TpBaseCallContent the only object to have a deinit? * why does TpBaseCallContent and TpBaseCallStream register themself on the bus? other TpBase let subclass decide
My branch now also contains TpBaseMediaCallContent and TpCallContentMediaDescription objects, based on tp-yell and Olivier's code. Finished porting them to latest spec and made it build.
Branch now also contains TpBaseMediaCallStream and TpCallStreamEndpoint. A few DBus methods are left unimplemented, but most of the tp-glib code is now there. Now porting CMs should be possible!
Started porting gabble: http://cgit.collabora.com/git/user/xclaesse/telepathy-gabble.git/log/?h=call1 and empathy: http://cgit.collabora.com/git/user/xclaesse/empathy.git/log/?h=call1 Also need my tp-farstream branch: http://cgit.collabora.com/git/user/xclaesse/telepathy-farstream.git/log/?h=call1 atm making a call just crash \o/
(In reply to comment #9) > atm making a call just crash \o/ for the record, crashes and warnings are all fixed, but other side does not ring :)
For the record, here are Olivier's comments: <ocrete> your tp-fs branch looks good (except that we should fix the controlling/endpoint thing) <ocrete> please split/rename the "misc fixes" commit <ocrete> in gabble.. call_member_flags_changed_cb() is always because of progres_made ? <ocrete> 22:39:55> did you just put progress_made everywhere ? <ocrete> we have to propagate the correct reasons <ocrete> updated is always false when coming to the other side I think.. its meaningless anyway its only for client->cm communications <ocrete> also, gabble needs to know if each new MD was accepted or not.. <ocrete> so the callback shouldnt be NULL <ocrete> in codec_array_to_list() the updated codec is useful because gabble should only send <description-info> for those codecs where it is set to TRUE instead of trying to do diff (so we can delete from code in the jingle-media-rtp.c <ocrete> for the remote contact == 0.. it means for all contacts <ocrete> with xmpp, it's never ICE-lite <ocrete> TP_IFACE_CHANNEL_TYPE_CALL ".InitialAudio" -> isnt there a define for these already ? <ocrete> like TP_PROP_CHANNEL_TYPE_CALL_INITIAL_AUDIO <ocrete> I also wonder if sending state succesful completions shouldnt also be reported to the CM <ocrete> 02:49:08> xclaesse: you empathy branch just looks good. I can'T find anythign to say about it !
0.17.5
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.