When a client calls tp_account_channel_request_create_and_handle_channel_async it results in UpdateCapabilities being called on Gabble with the temporary client, which does not have any caps or filters set. Since Gabble resets the capabilties in UpdateCapabilities, and the temporary client does not have any caps or filters, it sets use_call_channels to FALSE in GabbleMediaFactory. This causes StreamedMedia channels to be created instead of Call1, unless Gabble is restarted.
Created attachment 68801 [details] [review] connection: Don't reset the capabilities in UpdateCapabilities
Created attachment 68802 [details] [review] connection: Don't reset the capabilities in UpdateCapabilities Get rid of gabble_caps_channel_manager_reset_capabilities and its implementation in GabbleMediaFactory too, since they are unused now.
From #telepathy on Freenode: 10:05 <sjoerd> rishi: btw remind me of your gabble bug number ? 10:06 <rishi> sjoerd: https://bugs.freedesktop.org/56181 10:07 <sjoerd> rishi: looks good, can you commit or do you not have access? 10:08 <rishi> sjoerd: I have access. 10:09 <sjoerd> cool, add a reviewed by tag and go go go :) 10:09 <rishi> sjoerd: Ok to push to master as well? 10:09 <sjoerd> yeah
By removing gabble_caps_channel_manager_reset_capabilities aren't you breaking API in the stable branch? Also if you're should remove the reset_caps member from GabbleCapsChannelManagerIface struct.
(In reply to comment #4) > By removing gabble_caps_channel_manager_reset_capabilities aren't you > breaking API in the stable branch? _reset_capabilities isn't public API. Also it doesn't remove StreamedMedia, it just changes the details of the Call1 detection (in a way that doesn't break Call..)
(In reply to comment #5) > (In reply to comment #4) > > By removing gabble_caps_channel_manager_reset_capabilities aren't you > > breaking API in the stable branch? > > _reset_capabilities isn't public API. I don't understand why it's not? Its signature was removed from gabble/caps-channel-manager.h which certainly is public API. For example, the Ytstenut plugin defines a new caps channel manager implementation based on this public API.
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > By removing gabble_caps_channel_manager_reset_capabilities aren't you > > > breaking API in the stable branch? > > > > _reset_capabilities isn't public API. > > I don't understand why it's not? Its signature was removed from > gabble/caps-channel-manager.h which certainly is public API. For example, > the Ytstenut plugin defines a new caps channel manager implementation based > on this public API. It's an experimental API that we don't even bother installing the headers for in distribution. So i guess it's public for some value of public for which i really can't care much (and i'm not sure we gave any abi stability guarantees about it ever given it's marked as experimental). The API removed here is fundamentally broken though and based on me misunderstanding the spec, so better get rid of it.
(In reply to comment #7) > The API removed here is fundamentally broken though and based on me > misunderstanding the spec, so better get rid of it. Another possible resolution would be to keep the API, but warn if the vfunc is actually implemented. However, the plugin API is sufficiently flaky that the SONAME of the plugin changes every version (even in the stable branch!), which I think indicates that Sjoerd is right - we don't trust the plugin API this much. The update needed for ytstenut-plugins will be to delete this line: > plugin-base/channel-manager.c: iface->reset_caps = NULL; which I don't think is particularly onerous.
Created attachment 69827 [details] [review] caps-channel-manager: Drop reset_caps, which is now unused
(In reply to comment #9) > Created attachment 69827 [details] [review] > caps-channel-manager: Drop reset_caps, which is now unused 16:01 < smcv> rishi: I'm inclined to do that on master but not on the stable branch, I think (because while we don't guarantee API even on the stable branch anyway, I happen to know that this change will break Ytstenut.)
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.