From f26b85ff9ed73e950c454fb855e9a2b1f4201333 Mon Sep 17 00:00:00 2001 From: Lyubomir Marinov Date: Tue, 3 Apr 2012 20:10:09 +0000 Subject: [PATCH] Fixes multiple stability issues with the (experimental) support for the cobri Jabber extension. --- .../impl/protocol/jabber/CallJabberImpl.java | 138 ++++++++++++++++-- .../protocol/jabber/CallPeerJabberImpl.java | 36 +++-- .../jabber/CallPeerMediaHandlerGTalkImpl.java | 2 +- .../jabber/RawUdpTransportManager.java | 95 ++++++++++-- .../service/protocol/AbstractCallPeer.java | 2 +- .../protocol/media/MediaAwareCallPeer.java | 4 +- 6 files changed, 229 insertions(+), 48 deletions(-) diff --git a/src/net/java/sip/communicator/impl/protocol/jabber/CallJabberImpl.java b/src/net/java/sip/communicator/impl/protocol/jabber/CallJabberImpl.java index 9f95fa089..6b81c1630 100644 --- a/src/net/java/sip/communicator/impl/protocol/jabber/CallJabberImpl.java +++ b/src/net/java/sip/communicator/impl/protocol/jabber/CallJabberImpl.java @@ -510,20 +510,21 @@ public void closeCobriStreamConnector( } /** - * Allocates cobri (conference) channels for asp ecific MediaType + * Allocates cobri (conference) channels for a specific MediaType * to be used by a specific CallPeer. * * @param peer the CallPeer which is to use the allocated cobri * (conference) channels - * @param mediaTypes the MediaTypes for which cobri (conference) - * channels are to be allocated + * @param rdpes the RtpDescriptionPacketExtensions which specify + * the MediaTypes for which cobri (conference) channels are to be + * allocated * @return a CobriConferenceIQ which describes the allocated cobri * (conference) channels for the specified mediaTypes which are to * be used by the specified peer; otherwise, null */ public CobriConferenceIQ createCobriChannels( CallPeerJabberImpl peer, - Iterable mediaTypes) + Iterable rdpes) { if (!isConferenceFocus()) return null; @@ -558,8 +559,9 @@ public CobriConferenceIQ createCobriChannels( if (cobri != null) conferenceRequest.setID(cobri.getID()); - for (MediaType mediaType : mediaTypes) + for (RtpDescriptionPacketExtension rdpe : rdpes) { + MediaType mediaType = MediaType.parseString(rdpe.getMedia()); String contentName = mediaType.toString(); CobriConferenceIQ.Content contentRequest = new CobriConferenceIQ.Content(contentName); @@ -581,12 +583,16 @@ public CobriConferenceIQ createCobriChannels( CobriConferenceIQ.Channel localChannelRequest = new CobriConferenceIQ.Channel(); + for (PayloadTypePacketExtension ptpe : rdpe.getPayloadTypes()) + localChannelRequest.addPayloadType(ptpe); contentRequest.addChannel(localChannelRequest); } CobriConferenceIQ.Channel remoteChannelRequest = new CobriConferenceIQ.Channel(); + for (PayloadTypePacketExtension ptpe : rdpe.getPayloadTypes()) + remoteChannelRequest.addPayloadType(ptpe); contentRequest.addChannel(remoteChannelRequest); } @@ -614,9 +620,8 @@ public CobriConferenceIQ createCobriChannels( String conferenceResponseID = conferenceResponse.getID(); /* - * Update the complete VideoBridgeConferenceIQ - * representation maintained by this instance with the - * information given by the (current) response. + * Update the complete CobriConferenceIQ representation maintained by + * this instance with the information given by the (current) response. */ if (cobri == null) { @@ -654,8 +659,9 @@ else if (!cobriID.equals(conferenceResponseID)) conferenceResult.setID(conferenceResponseID); - for (MediaType mediaType : mediaTypes) + for (RtpDescriptionPacketExtension rdpe : rdpes) { + MediaType mediaType = MediaType.parseString(rdpe.getMedia()); CobriConferenceIQ.Content contentResponse = conferenceResponse.getContent(mediaType.toString()); @@ -668,11 +674,10 @@ else if (!cobriID.equals(conferenceResponseID)) conferenceResult.addContent(contentResult); /* - * The local channel may have been allocated in a - * previous method call as part of the allocation of - * the first remote channel in the respective - * content. Anyway, the current method caller still - * needs to know about it. + * The local channel may have been allocated in a previous + * method call as part of the allocation of the first remote + * channel in the respective content. Anyway, the current method + * caller still needs to know about it. */ CobriConferenceIQ.Content content = cobri.getContent(contentName); @@ -778,4 +783,109 @@ public CobriStreamConnector createCobriStreamConnector( return cobriStreamConnector; } + + /** + * Expires specific (cobri) conference channels used by a specific + * CallPeer. + * + * @param peer the CallPeer which uses the specified (cobri) + * conference channels to be expired + * @param conference a CobriConferenceIQ which specifies the + * (cobri) conference channels to be expired + */ + public void expireCobriChannels( + CallPeerJabberImpl peer, + CobriConferenceIQ conference) + { + // Formulate the CobriConferenceIQ request which is to be sent. + if (cobri != null) + { + String conferenceID = cobri.getID(); + + if (conferenceID.equals(conference.getID())) + { + CobriConferenceIQ conferenceRequest = new CobriConferenceIQ(); + + conferenceRequest.setID(conferenceID); + + for (CobriConferenceIQ.Content content + : conference.getContents()) + { + CobriConferenceIQ.Content cobriContent + = cobri.getContent(content.getName()); + + if (cobriContent != null) + { + CobriConferenceIQ.Content contentRequest + = conferenceRequest.getOrCreateContent( + cobriContent.getName()); + + for (CobriConferenceIQ.Channel channel + : content.getChannels()) + { + CobriConferenceIQ.Channel cobriChannel + = cobriContent.getChannel(channel.getID()); + + if (cobriChannel != null) + { + CobriConferenceIQ.Channel channelRequest + = new CobriConferenceIQ.Channel(); + + channelRequest.setExpire(0); + channelRequest.setID(cobriChannel.getID()); + contentRequest.addChannel(channelRequest); + } + } + } + } + + /* + * Remove the channels which are to be expired from the internal + * state of the conference managed by this CallJabberImpl. + */ + for (CobriConferenceIQ.Content contentRequest + : conferenceRequest.getContents()) + { + CobriConferenceIQ.Content cobriContent + = cobri.getContent(contentRequest.getName()); + + for (CobriConferenceIQ.Channel channelRequest + : contentRequest.getChannels()) + { + CobriConferenceIQ.Channel cobriChannel + = cobriContent.getChannel(channelRequest.getID()); + + cobriContent.removeChannel(cobriChannel); + + /* + * If the last remote channel is to be expired, expire + * the local channel as well. + */ + if (cobriContent.getChannelCount() == 1) + { + cobriChannel = cobriContent.getChannel(0); + + channelRequest = new CobriConferenceIQ.Channel(); + channelRequest.setExpire(0); + channelRequest.setID(cobriChannel.getID()); + contentRequest.addChannel(channelRequest); + + cobriContent.removeChannel(cobriChannel); + + break; + } + } + } + + /* + * At long last, send the CobriConferenceIQ request to expire + * the channels. + */ + conferenceRequest.setTo(cobri.getFrom()); + conferenceRequest.setType(IQ.Type.SET); + getProtocolProvider().getConnection().sendPacket( + conferenceRequest); + } + } + } } diff --git a/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerJabberImpl.java b/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerJabberImpl.java index 84e244566..1039605eb 100644 --- a/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerJabberImpl.java +++ b/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerJabberImpl.java @@ -400,7 +400,6 @@ public synchronized void answer() Reason.GENERAL_ERROR, reasonText); - getMediaHandler().getTransportManager().close(); setState(CallPeerState.FAILED, reasonText); getProtocolProvider().getConnection().sendPacket(errResp); return; @@ -439,12 +438,10 @@ public void hangup(boolean failed, } CallPeerState prevPeerState = getState(); - getMediaHandler().getTransportManager().close(); - if (failed) - setState(CallPeerState.FAILED, reasonText); - else - setState(CallPeerState.DISCONNECTED, reasonText); + setState( + failed ? CallPeerState.FAILED : CallPeerState.DISCONNECTED, + reasonText); JingleIQ responseIQ = null; @@ -574,7 +571,6 @@ public void processSessionTerminate(JingleIQ jingleIQ) reasonStr += " " + text; } - getMediaHandler().getTransportManager().close(); setState(CallPeerState.DISCONNECTED, reasonStr); } @@ -610,7 +606,6 @@ public void processSessionAccept(JingleIQ sessionInitIQ) sessionInitIQ.getSID(), Reason.INCOMPATIBLE_PARAMETERS, exc.getClass().getName() + ": " + exc.getMessage()); - getMediaHandler().getTransportManager().close(); setState(CallPeerState.FAILED, "Error: " + exc.getMessage()); getProtocolProvider().getConnection().sendPacket(errResp); return; @@ -1113,7 +1108,6 @@ public void processContentAccept(JingleIQ content) sessionInitIQ.getSID(), Reason.INCOMPATIBLE_PARAMETERS, "Error: " + exc.getMessage()); - getMediaHandler().getTransportManager().close(); setState(CallPeerState.FAILED, "Error: " + exc.getMessage()); getProtocolProvider().getConnection().sendPacket(errResp); return; @@ -1152,7 +1146,6 @@ public void processContentModify(JingleIQ content) sessionInitIQ.getSID(), Reason.INCOMPATIBLE_PARAMETERS, "Error: " + exc.getMessage()); - getMediaHandler().getTransportManager().close(); setState(CallPeerState.FAILED, "Error: " + exc.getMessage()); getProtocolProvider().getConnection().sendPacket(errResp); return; @@ -1201,7 +1194,6 @@ public void processContentReject(JingleIQ content) sessionInitIQ.getSID(), Reason.INCOMPATIBLE_PARAMETERS, "Error: content rejected"); - getMediaHandler().getTransportManager().close(); setState(CallPeerState.FAILED, "Error: content rejected"); getProtocolProvider().getConnection().sendPacket(errResp); return; @@ -1255,7 +1247,6 @@ public void processTransportInfo(JingleIQ jingleIQ) Reason.GENERAL_ERROR, reasonText); - getMediaHandler().getTransportManager().close(); setState(CallPeerState.FAILED, reasonText); getProtocolProvider().getConnection().sendPacket(errResp); @@ -1297,15 +1288,30 @@ protected void sendTransportInfo(Iterable contents) transportInfo.setTo(getAddress()); transportInfo.setType(IQ.Type.SET); - PacketCollector collector = protocolProvider.getConnection() - .createPacketCollector( - new PacketIDFilter(transportInfo.getPacketID())); + PacketCollector collector + = protocolProvider.getConnection().createPacketCollector( + new PacketIDFilter(transportInfo.getPacketID())); protocolProvider.getConnection().sendPacket(transportInfo); collector.nextResult(SmackConfiguration.getPacketReplyTimeout()); collector.cancel(); } + @Override + public void setState(CallPeerState newState, String reason, int reasonCode) + { + try + { + if (CallPeerState.DISCONNECTED.equals(newState) + || CallPeerState.FAILED.equals(newState)) + getMediaHandler().getTransportManager().close(); + } + finally + { + super.setState(newState, reason, reasonCode); + } + } + /** * Transfer (in the sense of call transfer) this CallPeer to a * specific callee address which may optionally be participating in an diff --git a/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerGTalkImpl.java b/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerGTalkImpl.java index f6ea3200c..491569ba5 100644 --- a/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerGTalkImpl.java +++ b/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerGTalkImpl.java @@ -20,7 +20,7 @@ import net.java.sip.communicator.util.*; /** - * An Google Talk specific extension of the generic media handler. + * A Google Talk-specific extension of the generic media handler. * * @author Sebastien Vincent */ diff --git a/src/net/java/sip/communicator/impl/protocol/jabber/RawUdpTransportManager.java b/src/net/java/sip/communicator/impl/protocol/jabber/RawUdpTransportManager.java index a21d9e18d..9d723ac6c 100644 --- a/src/net/java/sip/communicator/impl/protocol/jabber/RawUdpTransportManager.java +++ b/src/net/java/sip/communicator/impl/protocol/jabber/RawUdpTransportManager.java @@ -74,18 +74,84 @@ protected void closeStreamConnector( MediaType mediaType, StreamConnector streamConnector) { - if (streamConnector instanceof CobriStreamConnector) + try { - CallPeerJabberImpl peer = getCallPeer(); - CallJabberImpl call = peer.getCall(); + boolean superCloseStreamConnector = true; + + if (streamConnector instanceof CobriStreamConnector) + { + CallPeerJabberImpl peer = getCallPeer(); - call.closeCobriStreamConnector( - peer, - mediaType, - (CobriStreamConnector) streamConnector); + if (peer != null) + { + CallJabberImpl call = peer.getCall(); + + if (call != null) + { + superCloseStreamConnector = false; + call.closeCobriStreamConnector( + peer, + mediaType, + (CobriStreamConnector) streamConnector); + } + } + } + if (superCloseStreamConnector) + super.closeStreamConnector(mediaType, streamConnector); + } + finally + { + /* + * Expire the CobriConferenceIQ.Channel associated with the closed + * StreamConnector. + */ + if (cobri != null) + { + CobriConferenceIQ.Content content + = cobri.getContent(mediaType.toString()); + + if (content != null) + { + List channels + = content.getChannels(); + + if (channels.size() == 2) + { + CobriConferenceIQ requestConferenceIQ + = new CobriConferenceIQ(); + + requestConferenceIQ.setID(cobri.getID()); + + CobriConferenceIQ.Content requestContent + = requestConferenceIQ.getOrCreateContent( + content.getName()); + + requestContent.addChannel(channels.get(1 /* remote */)); + + /* + * Regardless of whether the request to expire the + * Channel associated with mediaType succeeds, consider + * the Channel in question expired. Since + * RawUdpTransportManager allocates a single channel per + * MediaType, consider the whole Content expired. + */ + cobri.removeContent(content); + + CallPeerJabberImpl peer = getCallPeer(); + + if (peer != null) + { + CallJabberImpl call = peer.getCall(); + + if (call != null) + call.expireCobriChannels( + peer, + requestConferenceIQ); + } + } + } + } } - else - super.closeStreamConnector(mediaType, streamConnector); } /** @@ -437,7 +503,8 @@ public void startCandidateHarvest( */ if (call.isConferenceFocus()) { - List mediaTypes = new ArrayList(); + List mediaTypes + = new ArrayList(); for (ContentPacketExtension cpe : cpes) { @@ -454,8 +521,8 @@ public void startCandidateHarvest( if ((cobri == null) || (cobri.getContent(mediaType.toString()) == null)) { - if (!mediaTypes.contains(mediaType)) - mediaTypes.add(mediaType); + if (!mediaTypes.contains(rtpDesc)) + mediaTypes.add(rtpDesc); } } if (mediaTypes.size() != 0) @@ -467,8 +534,8 @@ public void startCandidateHarvest( */ if (cobri == null) cobri = new CobriConferenceIQ(); - for (MediaType mediaType : mediaTypes) - cobri.getOrCreateContent(mediaType.toString()); + for (RtpDescriptionPacketExtension mediaType : mediaTypes) + cobri.getOrCreateContent(mediaType.getMedia()); CobriConferenceIQ conferenceResult = call.createCobriChannels(peer, mediaTypes); diff --git a/src/net/java/sip/communicator/service/protocol/AbstractCallPeer.java b/src/net/java/sip/communicator/service/protocol/AbstractCallPeer.java index 10a2f6c52..6ee25248a 100644 --- a/src/net/java/sip/communicator/service/protocol/AbstractCallPeer.java +++ b/src/net/java/sip/communicator/service/protocol/AbstractCallPeer.java @@ -456,7 +456,7 @@ public CallPeerState getState() */ public void setState(CallPeerState newState, String reason) { - this.setState(newState, reason, -1); + setState(newState, reason, -1); } /** diff --git a/src/net/java/sip/communicator/service/protocol/media/MediaAwareCallPeer.java b/src/net/java/sip/communicator/service/protocol/media/MediaAwareCallPeer.java index be9c8c693..2f8990a09 100644 --- a/src/net/java/sip/communicator/service/protocol/media/MediaAwareCallPeer.java +++ b/src/net/java/sip/communicator/service/protocol/media/MediaAwareCallPeer.java @@ -481,10 +481,8 @@ public void setState(CallPeerState newState, String reason, int reasonCode) { // make sure whatever happens to close the media if (CallPeerState.DISCONNECTED.equals(newState) - || CallPeerState.FAILED.equals(newState)) - { + || CallPeerState.FAILED.equals(newState)) mediaHandler.close(); - } } } }