If you try to add video to an existing audio call, but I don't support any of the codecs you propose, TpFs calls Error(Codec_Negotiation_Failed, ...) on the stream, and my Gabble correctly emits StreamError(..., Codec_Negotiation_Failed, ...) for that stream. However, my Gabble just sends you a content-remove with no reason. Firstly, it should send a content-reject. Secondly, as per <http://mail.jabber.org/pipermail/jingle/2009-November/001112.html> it should include <reason><failed-application/></reason> in the content-reject. Thirdly, it should also include <failed-application/> when terminating a session because none of the codecs in the initiate were supported. Fourthly, it should react to session-terminate with <failed-application/> by emitting StreamError(..., Codec_Negotiation_Failed, ...) for each stream before ending the call. This would allow the UI to tell the user what happened in more cases than it currently can.
This branch has all the aforementioned behavior implemented. Empathy is happy. I will make sure all tests pass and write new tests tomorrow.
OK! Passes all tests. Still need to actually write tests for the added features, but the actual code is ready for review.
OK! Tests added. They might be a tad excessive. I started adding assertions to existing tests, that might be the way to go.
trivia (don't have to be fixed for a merge): http://git.collabora.co.uk/?p=user/eitan/telepathy-gabble.git;a=commitdiff;h=ace3ba397ca79e4b78a2cf211fd89079f2142a3a misc indentation change in first chunk -- http://git.collabora.co.uk/?p=user/eitan/telepathy-gabble.git;a=commitdiff;h=ce3d6bec79e2db2f8e225e5af3881e74cc33a405 can use g_str_equal rather then !tp_strdiff since txt is guaranteed not-NULL ---- trivia: switch (…) { not sure these switch()es are in house style, but check up on that, I think the { should be on the next line ----
(In reply to comment #4) > trivia (don't have to be fixed for a merge): > > http://git.collabora.co.uk/?p=user/eitan/telepathy-gabble.git;a=commitdiff;h=ace3ba397ca79e4b78a2cf211fd89079f2142a3a > > misc indentation change in first chunk Fixed styling, e5c82a9 > > -- > > http://git.collabora.co.uk/?p=user/eitan/telepathy-gabble.git;a=commitdiff;h=ce3d6bec79e2db2f8e225e5af3881e74cc33a405 > > can use g_str_equal rather then !tp_strdiff since txt is guaranteed not-NULL > Using a GEnum mapping instead, as peeps said on #telepathy, 0dea816 > ---- > > trivia: > > switch (…) { > > not sure these switch()es are in house style, but check up on that, > I think the { should be on the next line Yeah, you are right about it. e5c82a9
Vivek gives his thumbs up. Merged.
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.