Fix hold/unhold media issue in XMPP. Send UDP hole punch packet after SDP.

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 d5920b8..d42e1e6 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 class CallPeerJabberImpl
         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 e194e3b..8f27338 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 class CallPeerMediaHandlerJabberImpl
                             content.addChildExtension(hash);
                         }
                     }
-                    
+
                     mediaDescs.add(content);
                 }
             }
@@ -468,6 +468,33 @@ public class CallPeerMediaHandlerJabberImpl
         return content;
     }
 
+     /**
+     * Reinitialize all media contents.
+     *
+     * @throws OperationFailedException if we fail to handle <tt>content</tt>
+     * for reasons like failing to initialize media devices or streams.
+     * @throws IllegalArgumentException if there's a problem with the syntax or
+     * the semantics of <tt>content</tt>. 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 86115b8..12ee47b 100644
--- a/src/net/java/sip/communicator/impl/protocol/sip/CallPeerMediaHandlerSipImpl.java
+++ b/src/net/java/sip/communicator/impl/protocol/sip/CallPeerMediaHandlerSipImpl.java
@@ -712,68 +712,6 @@ public class CallPeerMediaHandlerSipImpl
     }
 
     /**
-     * Our parent method configures <tt>stream</tt> to use the specified
-     * <tt>format</tt>, <tt>target</tt>, <tt>target</tt>, and
-     * <tt>direction</tt>. 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 <tt>MediaDevice</tt> to be used by <tt>stream</tt>
-     * for capture and playback
-     * @param format the <tt>MediaFormat</tt> that we'd like the new stream
-     * to transmit in.
-     * @param target the <tt>MediaStreamTarget</tt> containing the RTP and
-     * RTCP address:port couples that the new stream would be sending
-     * packets to.
-     * @param direction the <tt>MediaDirection</tt> that we'd like the new
-     * stream to use (i.e. sendonly, sendrecv, recvonly, or inactive).
-     * @param rtpExtensions the list of <tt>RTPExtension</tt>s that should be
-     * enabled for this stream.
-     * @param stream the <tt>MediaStream</tt> that we'd like to configure.
-     *
-     * @return the <tt>MediaStream</tt> that we received as a parameter (for
-     * convenience reasons).
-     *
-     * @throws OperationFailedException if setting the <tt>MediaFormat</tt>
-     * or connecting to the specified <tt>MediaDevice</tt> fails for some
-     * reason.
-     */
-    @Override
-    protected MediaStream configureStream( MediaDevice          device,
-                                           MediaFormat          format,
-                                           MediaStreamTarget    target,
-                                           MediaDirection       direction,
-                                           List<RTPExtension>   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.
      *
      * @return 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 d50b628..74c13f8 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 class CallPeerSipImpl
         if (!CallPeerState.isOnHold(peerState))
         {
             setState(CallPeerState.CONNECTED);
+            getMediaHandler().start();
 
             // as its connected, set initial mute status,
             // corresponding call status
@@ -656,6 +657,7 @@ public class CallPeerSipImpl
         if (!CallPeerState.isOnHold(getState()))
         {
             setState(CallPeerState.CONNECTED);
+            getMediaHandler().start();
 
             // as its connected, set initial mute status,
             // corresponding call status