From 63091bd18a90bb8a5db4599232dbf7384a3abc23 Mon Sep 17 00:00:00 2001 From: Sebastien Vincent Date: Fri, 10 Sep 2010 16:17:18 +0000 Subject: [PATCH] Fix hold/unhold media issue in XMPP. Send UDP hole punch packet after SDP. --- .../protocol/jabber/CallPeerJabberImpl.java | 3 + .../CallPeerMediaHandlerJabberImpl.java | 29 ++++++++- .../sip/CallPeerMediaHandlerSipImpl.java | 62 ------------------- .../impl/protocol/sip/CallPeerSipImpl.java | 2 + 4 files changed, 33 insertions(+), 63 deletions(-) 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 d5920b837..d42e1e603 100644 --- a/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerJabberImpl.java +++ b/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerJabberImpl.java @@ -411,7 +411,10 @@ public void putOnHold(boolean onHold) if(onHold) type = SessionInfoType.hold; else + { type = SessionInfoType.unhold; + getMediaHandler().reinitAllContents(); + } //we are now on hold and need to realize this before potentially //spoling it all with an exception while sending the packet :). diff --git a/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerJabberImpl.java b/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerJabberImpl.java index e194e3b5e..8f27338c1 100644 --- a/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerJabberImpl.java +++ b/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerJabberImpl.java @@ -412,7 +412,7 @@ public List createContentList() content.addChildExtension(hash); } } - + mediaDescs.add(content); } } @@ -468,6 +468,33 @@ private ContentPacketExtension createContentForOffer( return content; } + /** + * Reinitialize all media contents. + * + * @throws OperationFailedException if we fail to handle content + * for reasons like failing to initialize media devices or streams. + * @throws IllegalArgumentException if there's a problem with the syntax or + * the semantics of content. Method is synchronized in order to + * avoid closing mediaHandler when we are currently in process of + * initializing, configuring and starting streams and anybody interested + * in this operation can synchronize to the mediaHandler instance to wait + * processing to stop (method setState in CallPeer). + */ + public void reinitAllContents() + throws OperationFailedException, + IllegalArgumentException + { + for(String key : remoteContentMap.keySet()) + { + ContentPacketExtension ext = remoteContentMap.get(key); + + if(ext != null) + { + processContent(ext); + } + } + } + /** * Reinitialize a media content such as video. * diff --git a/src/net/java/sip/communicator/impl/protocol/sip/CallPeerMediaHandlerSipImpl.java b/src/net/java/sip/communicator/impl/protocol/sip/CallPeerMediaHandlerSipImpl.java index 86115b8b4..12ee47b91 100644 --- a/src/net/java/sip/communicator/impl/protocol/sip/CallPeerMediaHandlerSipImpl.java +++ b/src/net/java/sip/communicator/impl/protocol/sip/CallPeerMediaHandlerSipImpl.java @@ -711,68 +711,6 @@ protected void throwOperationFailedException( String message, message, errorCode, cause, logger); } - /** - * Our parent method configures stream to use the specified - * format, target, target, and - * direction. It also used to start it but we changed that because - * in jabber stream configuration and starting are decoupled. We'll - * eventually decouple them here too but until we get there we temporarily - * override the parent method and make it behave like it previously used to. - * This way we'll always start a stream when we configure it, until we - * implement ICE for SIP and change that. At that point we should also make - * this method private (or rather the one in the parent), as well as the - * super{@link #sendHolePunchPacket(MediaStreamTarget)}. - * - * @param device the MediaDevice to be used by stream - * for capture and playback - * @param format the MediaFormat that we'd like the new stream - * to transmit in. - * @param target the MediaStreamTarget containing the RTP and - * RTCP address:port couples that the new stream would be sending - * packets to. - * @param direction the MediaDirection that we'd like the new - * stream to use (i.e. sendonly, sendrecv, recvonly, or inactive). - * @param rtpExtensions the list of RTPExtensions that should be - * enabled for this stream. - * @param stream the MediaStream that we'd like to configure. - * - * @return the MediaStream that we received as a parameter (for - * convenience reasons). - * - * @throws OperationFailedException if setting the MediaFormat - * or connecting to the specified MediaDevice fails for some - * reason. - */ - @Override - protected MediaStream configureStream( MediaDevice device, - MediaFormat format, - MediaStreamTarget target, - MediaDirection direction, - List rtpExtensions, - MediaStream stream) - throws OperationFailedException - { - super.configureStream(device, format, target, - direction, rtpExtensions, stream); - - //the reason we have this temporary method is so that we could start - //the streams, so here we go. - if ( ! stream.isStarted()) - stream.start(); - - // in case we won't be sending outgoing traffic, send empty packet to - // punch a hole for the incoming packets. In order to work better this - // should actually be done after we send our offer/answer - if(stream instanceof VideoMediaStream - && getDirectionUserPreference(MediaType.VIDEO) - == MediaDirection.RECVONLY) - { - sendHolePunchPacket(target); - } - - return stream; - } - /** * Returns the transport manager that is handling our address management. * diff --git a/src/net/java/sip/communicator/impl/protocol/sip/CallPeerSipImpl.java b/src/net/java/sip/communicator/impl/protocol/sip/CallPeerSipImpl.java index d50b62888..74c13f819 100644 --- a/src/net/java/sip/communicator/impl/protocol/sip/CallPeerSipImpl.java +++ b/src/net/java/sip/communicator/impl/protocol/sip/CallPeerSipImpl.java @@ -532,6 +532,7 @@ public void processAck(ServerTransaction serverTransaction, Request ack) if (!CallPeerState.isOnHold(peerState)) { setState(CallPeerState.CONNECTED); + getMediaHandler().start(); // as its connected, set initial mute status, // corresponding call status @@ -656,6 +657,7 @@ public void processInviteOK(ClientTransaction clientTransaction, if (!CallPeerState.isOnHold(getState())) { setState(CallPeerState.CONNECTED); + getMediaHandler().start(); // as its connected, set initial mute status, // corresponding call status