Add the server (videobridge) region to jingle #316
Conversation
The Jingle operation set does not need to know about all of the extensions we support (and take their configuration as parameters).
| /** | ||
| * Adds a group packet extension to a {@link JingleIQ}, and a | ||
| * {@link BundlePacketExtension} to each of its contents. I.e. adds | ||
| * everything that we deem necessary to turn enable {@code bundle} in an |
There was a problem hiding this comment.
extra word turn? or maybe i'm misreading this (as I'm not sure I quite follow what it's saying)
| public static void addStartMutedExtension( | ||
| JingleIQ jingleIQ, boolean audioMute, boolean videoMute) | ||
| { | ||
| // FIXME Move this to a place where offer's contents are created or |
There was a problem hiding this comment.
what's stopping us from doing this now?
There was a problem hiding this comment.
That's just a stale comment, I removed it.
| @@ -363,7 +397,8 @@ else if (reInvite) | |||
| } | |||
| // Existing peers SSRCs | |||
| RtpDescriptionPacketExtension rtpDescPe | |||
| = JingleUtils.getRtpDescription(cpe); | |||
| = net.java.sip.communicator.impl.protocol.jabber.jinglesdp | |||
| .JingleUtils.getRtpDescription(cpe); | |||
There was a problem hiding this comment.
i wonder if it's worth renaming this to JicofoJingleUtils to 1) avoid the long qualified naming and 2) further denote that these are jicofo-specific jingle util functions
| * @throws OperationFailedException if we are unable to send a packet | ||
| * because the XMPP connection is broken. | ||
| */ | ||
| private boolean sendJingleIq( |
There was a problem hiding this comment.
it seems a little odd to me that we have this generic sendJingleIq function which has to then look at fields to figure out which JingleIq actually needs to be created and sent. are there not other places (which presumably trigger/call this) which already know what is happening and can create the necessary more directly?
There was a problem hiding this comment.
are there not other places (which presumably trigger/call this) which already know what is happening and can create the necessary more directly?
Yes. There's only one place where this is used, and the only reason I extracted it is to avoid having the code inline (it's 50 lines of code in a method that is already over 50 lines)
There was a problem hiding this comment.
ok i see now. is the comment inaccurate, though? it says "creates either session-accept or transport-replace" but the code creates either session-initiate or transport-replace. and, while fixing that (unless I'm misunderstanding) could you add to the comment how it decides to make one vs the other? (it looks like it's based on the reInvite member variable or the participant having a jingle session?)
No description provided.