Bug 65657

Summary:	Jingle: wait for session-initiate ack, then send candidates
Product:	Wocky	Reporter:	Will Thompson <will>
Component:	General	Assignee:	Telepathy bugs list <telepathy-bugs>
Status:	NEW ---	QA Contact:	Telepathy bugs list <telepathy-bugs>
Severity:	normal
Priority:	medium	CC:	will
Version:	unspecified	Keywords:	patch
Hardware:	All
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	A patch!

Description Will Thompson 2013-06-11 15:56:37 UTC

There seems to be a race condition in Jitsi: if we send a transport-info
full of candidates immediately after sending a session-info, it
sometimes seems to miss the candidates we send it. Waiting until after
it acks our session-initiate seems to do the trick. This is only visible
in an application which only ever sends a single local candidate,
immediately after initiating; it is probably masked in Telepathy by new
candidates trickling in after the call starts as we get STUN replies.

The previous code would call _transmit_candidates when accepting a call,
too, but I don't think this is necessary: in the case where the call is
incoming, wocky_jingle_content_add_candidate() will tell the transport
to send out new candidates as they are added because the state is >
EMPTY.

Comment 1 Will Thompson 2013-06-11 15:57:30 UTC

Created attachment 80703 [details] [review]
A patch!

Comment 2 Simon McVittie 2013-06-24 12:27:20 UTC

> Waiting until after
> it acks our session-initiate seems to do the trick.

Seems reasonable, but I'd like to test interop with other picky almost-Jingle clients (*coughGooglecough*) before applying.

> The previous code would call _transmit_candidates when accepting a call,
> too, but I don't think this is necessary: in the case where the call is
> incoming, wocky_jingle_content_add_candidate() will tell the transport
> to send out new candidates as they are added because the state is >
> EMPTY.

This seems subtle enough that someone should check this reasoning before saying "yes".

Comment 3 Will Thompson 2013-06-27 14:42:01 UTC

I accidentally merged this patch: http://cgit.freedesktop.org/wocky/commit/?id=d8fcf78

So then I reverted it: http://cgit.freedesktop.org/wocky/commit/?id=c0e8186

Sorry!

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.