Hi,

I noticed, that SEMS master currently does not support
Re-INVITEs which contain an SDP offer. The problem is, 
that the old local SDP is given to getSdpAnswer and this
routine then adds another m= line which is, however, 
invalid.

I tried to correctly implement the Re-INVITE functionality
in the attached diff. The implementation works as follows:

- the old local sdp is copied to a backup
- the local_sdp instance variable is cleared
- the getSdpAnswer method is called with the cleared instance variable
- the old local sdp and the new local sdp are compared; if they differ
  the version in the o= line of the new sdp is incremented by on as
  the offer-answer rfc mandates

I also fixed a minor problem, where the session id and session version
in the o= line were always set to 0 (or 1, don't remember).

As for the media handling in case of Re-INVITE, I adapted the
onSdpCompleted method of AmSession, to not call RtpStream::init again,
if the session is already attached to the media processor thread. In
this case only the "sendonly" attribute of the remote SDP is evaluated
and if it is present, the setOnHold(true) method of the RTPStream()
object is called. Probably there are more ways in which an RTPStream can
be modified, but only the "set on hold" functionality is implemented.

Also, the case of Re-INVITEs without SDP is probably not handled yet.

Regards,
Emil

-- 
Emil Kroymann
VoIP Services Engineer

Email: [email protected]
Tel: +49-30-203899885
Mobile: +49-176-38389303

ISACO GmbH
Kurfürstenstraße 79
10787 Berlin
Germany

Amtsgericht Charlottenburg, HRB 112464B
Geschäftsführer: Daniel Frommherz

diff --git a/core/AmRtpStream.cpp b/core/AmRtpStream.cpp
index da59800..43d113b 100644
--- a/core/AmRtpStream.cpp
+++ b/core/AmRtpStream.cpp
@@ -456,8 +456,8 @@ AmRtpStream::~AmRtpStream()
 
 int AmRtpStream::getLocalPort()
 {
-  if (hold)
-    return 0;
+  //  if (hold)
+  //    return 0;
 
   if(!l_port)
     setLocalPort();
diff --git a/core/AmSdp.cpp b/core/AmSdp.cpp
index 03f57de..f6ad9eb 100644
--- a/core/AmSdp.cpp
+++ b/core/AmSdp.cpp
@@ -95,38 +95,99 @@ inline string transport_p_2_str(int tp)
   }
 }
 
+bool SdpConnection::operator == (const SdpConnection& other) const
+{
+  return network == other.network && addrType == other.addrType 
+    && address == other.address;
+}
+
+bool SdpOrigin::operator == (const SdpOrigin& other) const
+{
+  return user == other.user && sessId == other.sessId 
+    && sessV == other.sessV && conn == other.conn;
+}
+
+bool SdpPayload::operator == (const SdpPayload& other) const
+{
+  return payload_type == other.payload_type && encoding_name == other.encoding_name 
+    && clock_rate == other.clock_rate && encoding_param == other.encoding_param;
+}
+
 bool SdpPayload::operator == (int r)
 {
   DBG("pl == r: payload_type = %i; r = %i\n", payload_type, r);
   return payload_type == r;
 }
 
-string SdpAttribute::print() const {
+string SdpAttribute::print() const
+{
   if (value.empty())
     return "a="+attribute+CRLF;
   else
     return "a="+attribute+":"+value+CRLF;
 }
 
+bool SdpAttribute::operator==(const SdpAttribute& other) const
+{
+  return attribute==other.attribute && (value.empty() || (value==other.value));
+}
+
+bool SdpMedia::operator == (const SdpMedia& other) const
+{
+  if (payloads.empty()) {
+    if (!other.payloads.empty())
+      return false;
+
+  } else if (other.payloads.empty()) {
+    return false;
+
+  } else {    
+    std::pair<vector<SdpPayload>::const_iterator, vector<SdpPayload>::const_iterator> pl_mismatch 
+      = std::mismatch(payloads.begin(), payloads.end(), other.payloads.begin());
+
+    if (pl_mismatch.first != payloads.end() || pl_mismatch.second != other.payloads.end())
+      return false;
+  }
+
+  if (attributes.empty()) {
+    if (!other.attributes.empty()) {
+      return false;
+    }
+  } else {
+    std::pair<vector<SdpAttribute>::const_iterator, vector<SdpAttribute>::const_iterator> a_mismatch 
+      = std::mismatch(attributes.begin(), attributes.end(), other.attributes.begin());
+
+    if (a_mismatch.first != attributes.end() || a_mismatch.second != other.attributes.end())
+      return false;
+  }
+
+  return type == other.type && port == other.port && nports == other.nports 
+    && transport == other.transport && conn == other.conn && dir == other.dir;
+}
+
 //
 // class AmSdp: Methods
 //
 AmSdp::AmSdp()
   : remote_active(false),
-    accepted_media(0)
+    accepted_media(0),
+    origin(),
+    sessionName(),
+    conn(),
+    media()
 {
-  l_origin.user = "sems";
-  l_origin.sessId = get_random();
-  l_origin.sessV = get_random();
+  origin.user = "sems";
+  origin.sessId = get_random();
+  origin.sessV = get_random();
 }
 
 AmSdp::AmSdp(const AmSdp& p_sdp_msg)
   : version(p_sdp_msg.version),
     origin(p_sdp_msg.origin),
-    l_origin(p_sdp_msg.l_origin),
     sessionName(p_sdp_msg.sessionName),
     conn(p_sdp_msg.conn),
     media(p_sdp_msg.media),
+    attributes(p_sdp_msg.attributes),
     remote_active(false),
     accepted_media(0)
 {
@@ -194,6 +255,22 @@ void AmSdp::print(string& body) const
 
       out_buf += "\r\n" + options;
 
+      options = "";
+
+      for (std::vector<SdpAttribute>::const_iterator a_it = media_it->attributes.begin();
+	   a_it != media_it->attributes.end(); a_it++) {
+
+	if (a_it->value.empty()) {
+	  options += "a=" + a_it->attribute;
+	} else {
+	  options += "a=" + a_it->attribute + ":" + a_it->value;
+	}
+	options +=" \r\n";
+      }
+
+      out_buf += options;
+
+
       switch(media_it->dir){
       case SdpMedia::DirActive:
 	  out_buf += "a=direction:active\r\n";
@@ -238,6 +315,42 @@ const SdpPayload *AmSdp::findPayload(const string& name) const
   return NULL;
 }
 
+bool AmSdp::operator == (const AmSdp& other) const
+{
+  if (attributes.empty()) {
+    if (!other.attributes.empty())
+      return false;
+
+  } else if (other.attributes.empty()) {
+    return false;
+
+  } else {
+    std::pair<vector<SdpAttribute>::const_iterator, vector<SdpAttribute>::const_iterator> a_mismatch
+      = std::mismatch(attributes.begin(), attributes.end(), other.attributes.begin());
+
+    if (a_mismatch.first != attributes.end() || a_mismatch.second != other.attributes.end())
+      return false;
+  }
+   
+  if (media.empty()) {
+    if (!other.media.empty())
+      return false;
+
+  } else if (other.media.empty()) {
+    return false;
+
+  } else {
+    std::pair<vector<SdpMedia>::const_iterator, vector<SdpMedia>::const_iterator> m_mismatch
+      = std::mismatch(media.begin(), media.end(), other.media.begin());
+
+    if (m_mismatch.first != media.end() || m_mismatch.second != other.media.end())
+      return false;
+  }
+
+  return version == other.version && origin == other.origin 
+    && sessionName == other.sessionName && uri == other.uri && conn == other.conn;
+}
+
 //parser
 static bool parse_sdp_line_ex(AmSdp* sdp_msg, char*& s)
 {
diff --git a/core/AmSdp.h b/core/AmSdp.h
index 441eefb..063f7b2 100644
--- a/core/AmSdp.h
+++ b/core/AmSdp.h
@@ -71,6 +71,14 @@ struct SdpConnection
   string address;
 
   SdpConnection() : address() {}
+
+  SdpConnection(const SdpConnection& other)
+    : network(other.network), addrType(other.addrType),
+      ipv4(other.ipv4), ipv6(other.ipv6),
+      address(other.address)
+  {}
+
+  bool operator == (const SdpConnection& other) const;
 };
 
 /** \brief o=... line in SDP */
@@ -80,6 +88,15 @@ struct SdpOrigin
   unsigned int sessId;
   unsigned int sessV;
   SdpConnection conn;
+
+  SdpOrigin() : user(), conn() {}
+
+  SdpOrigin(const SdpOrigin& other)
+    : user(other.user), sessId(other.sessId), sessV(other.sessV),
+      conn(other.conn)
+  {}
+
+  bool operator == (const SdpOrigin& other) const;
 };
 /** 
  * \brief sdp payload
@@ -104,7 +121,16 @@ struct SdpPayload
   SdpPayload(int pt, const string& name, int rate, int param) 
     : int_pt(-1), payload_type(pt), encoding_name(name), clock_rate(rate), encoding_param(param) {}
 
+  SdpPayload(const SdpPayload& other)
+    : type(other.type), int_pt(other.int_pt), payload_type(other.payload_type),
+      encoding_name(other.encoding_name), clock_rate(other.clock_rate),
+      format(other.format), sdp_format_parameters(other.sdp_format_parameters),
+      encoding_param(other.encoding_param)
+  {}
+
   bool operator == (int r);
+
+  bool operator == (const SdpPayload& other) const;
 };
 
 /** \brief a=... line in SDP */
@@ -122,7 +148,12 @@ struct SdpAttribute
   SdpAttribute(const string& attribute)
     : attribute(attribute) { }
 
+  SdpAttribute(const SdpAttribute& other)
+    : attribute(other.attribute), value(other.value) {}
+
   string print() const;
+
+  bool operator == (const SdpAttribute& other) const;
 };
 
 /** \brief m=... line in SDP */
@@ -146,6 +177,12 @@ struct SdpMedia
   std::vector<SdpAttribute> attributes; // unknown attributes
 
   SdpMedia() : conn() {}
+
+  SdpMedia(const SdpMedia& other)
+    : type(other.type), port(other.port), nports(other.nports), transport(other.transport),
+      conn(other.conn), dir(other.dir), payloads(other.payloads), attributes(other.attributes) {}
+
+  bool operator == (const SdpMedia& other) const;
 };
 
 /**
@@ -177,8 +214,7 @@ public:
 
   unsigned int accepted_media;   // index of the media which we accept (todo: multi stream)
 
-  SdpOrigin        l_origin;      // local origin (o= )
-    
+
   AmSdp();
   AmSdp(const AmSdp& p_sdp_msg);
 
@@ -201,6 +237,9 @@ public:
    * Test if remote UA supports 'telefone_event'.
    */
   //bool hasTelephoneEvent();
+
+  bool operator == (const AmSdp& other) const;
+
 };
 
 #endif
diff --git a/core/AmSession.cpp b/core/AmSession.cpp
index dbf4cce..aeca12a 100644
--- a/core/AmSession.cpp
+++ b/core/AmSession.cpp
@@ -923,8 +923,8 @@ bool AmSession::getSdpOffer(AmSdp& offer)
 
   offer.version = 0;
   offer.origin.user = "sems";
-  offer.origin.sessId = 1;
-  offer.origin.sessV = 1;
+  //offer.origin.sessId = 1;
+  //offer.origin.sessV = 1;
   offer.sessionName = "sems";
   offer.conn.network = NT_IN;
   offer.conn.addrType = AT_V4;
@@ -949,12 +949,8 @@ bool AmSession::getSdpOffer(AmSdp& offer)
 
 struct codec_priority_cmp
 {
-private:
-  const vector<SdpPayload>& original_order;
-
 public:
-  codec_priority_cmp(const vector<SdpPayload>& original_order)
-    : original_order(original_order) {}
+  codec_priority_cmp() {}
 
   bool operator()(const SdpPayload& left, const SdpPayload& right)
   {
@@ -965,13 +961,6 @@ public:
 	return false;
     }
 
-    for (vector<SdpPayload>::const_iterator it = original_order.begin(); it != original_order.end(); it++) {
-      if ((strcasecmp(left.encoding_name.c_str(),it->encoding_name.c_str())==0 && left.clock_rate == it->clock_rate)
-	  && (strcasecmp(right.encoding_name.c_str(),it->encoding_name.c_str())!=0 || right.clock_rate != it->clock_rate))
-	return true;
-      if (strcasecmp(right.encoding_name.c_str(),it->encoding_name.c_str())==0 && right.clock_rate == it->clock_rate)
-	return false;
-    }
     return false;
   }
 };
@@ -983,8 +972,8 @@ bool AmSession::getSdpAnswer(const AmSdp& offer, AmSdp& answer)
 
   answer.version = 0;
   answer.origin.user = "sems";
-  answer.origin.sessId = 1;
-  answer.origin.sessV = 1;
+  //answer.origin.sessId = 1;
+  //answer.origin.sessV = 1;
   answer.sessionName = "sems";
   answer.conn.network = NT_IN;
   answer.conn.addrType = AT_V4;
@@ -1004,6 +993,15 @@ bool AmSession::getSdpAnswer(const AmSdp& offer, AmSdp& answer)
   answer_media.transport = TP_RTPAVP;
   answer_media.dir = SdpMedia::DirBoth;
 
+  vector<SdpAttribute>::const_iterator apos
+    = std::find(m_it->attributes.begin(), m_it->attributes.end(), SdpAttribute("sendonly"));
+
+  if (apos != m_it->attributes.end()) {
+    answer_media.attributes.push_back(SdpAttribute("recvonly"));
+  } else {
+    answer_media.attributes.push_back(SdpAttribute("sendrecv"));
+  }
+
   // Calculate the intersection with the offered set of payloads
 
   vector<SdpPayload>::const_iterator it = m_it->payloads.begin();
@@ -1036,7 +1034,9 @@ bool AmSession::getSdpAnswer(const AmSdp& offer, AmSdp& answer)
     }
   }
 
-  std::sort(answer_media.payloads.begin(),answer_media.payloads.end(),codec_priority_cmp(m_it->payloads));
+  // sort payload type in the answer according to the priority given in the codec_order configuration key
+  std::stable_sort(answer_media.payloads.begin(),answer_media.payloads.end(),codec_priority_cmp());
+
   return true;
 }
 
@@ -1062,16 +1062,37 @@ int AmSession::onSdpCompleted(const AmSdp& local_sdp, const AmSdp& remote_sdp)
     return -1;
   }
 
+  bool set_on_hold = false;
+  if (!remote_sdp.media.empty()) {
+    vector<SdpAttribute>::const_iterator pos =
+      std::find(remote_sdp.media[0].attributes.begin(), remote_sdp.media[0].attributes.end(), SdpAttribute("sendonly"));
+    set_on_hold = pos != remote_sdp.media[0].attributes.end();
+  }
+
   lockAudio();
   // TODO: 
   //   - get the right media ID
   //   - check if the stream coresponding to the media ID 
   //     should be created or updated   
   //
-  int ret = RTPStream()->init(getPayloadProvider(),0,local_sdp,remote_sdp);
+  int ret = -1;
+  if (!processing_media.get()) {
+    ret = RTPStream()->init(getPayloadProvider(),0,local_sdp,remote_sdp);
+  } else {
+    if (set_on_hold) {
+      RTPStream()->setOnHold(true);
+    } else {
+      if (RTPStream()->getOnHold()) {
+	RTPStream()->setOnHold(false);
+      }
+    }
+    ret = 0;
+  }
   unlockAudio();
 
-  setInbandDetector(AmConfig::DefaultDTMFDetector);
+  if (!processing_media.get()) {
+    setInbandDetector(AmConfig::DefaultDTMFDetector);
+  }
 
   if(ret){
     ERROR("while initializing RTP stream\n");
diff --git a/core/AmSipDialog.cpp b/core/AmSipDialog.cpp
index 90535e3..72562de 100644
--- a/core/AmSipDialog.cpp
+++ b/core/AmSipDialog.cpp
@@ -84,7 +84,8 @@ AmSipDialog::AmSipDialog(AmSipDialogEventHandler* h)
     next_hop_port(AmConfig::NextHopPort),
     next_hop_ip(AmConfig::NextHopIP),
     next_hop_for_replies(AmConfig::NextHopForReplies),
-    outbound_interface(-1), out_intf_for_replies(false)
+    outbound_interface(-1), out_intf_for_replies(false),
+    sdp_local(), sdp_remote()
 {
   assert(h);
   if (reliable_1xx)
@@ -320,7 +321,9 @@ int AmSipDialog::onTxSdp(const string& body)
 
 int AmSipDialog::getSdpBody(string& sdp_body)
 {
-    switch(oa_state){
+  AmSdp old_sdp_local(sdp_local);
+
+  switch(oa_state){
     case OA_None:
     case OA_Completed:
       if(hdl->getSdpOffer(sdp_local)){
@@ -332,7 +335,11 @@ int AmSipDialog::getSdpBody(string& sdp_body)
       }
       break;
     case OA_OfferRecved:
+      sdp_local.media.clear();
       if(hdl->getSdpAnswer(sdp_remote,sdp_local)){
+	if (!(sdp_local == old_sdp_local)) {
+	  sdp_local.origin.sessV = old_sdp_local.origin.sessV + 1;
+	}
 	sdp_local.print(sdp_body);
       }
       else {
@@ -347,9 +354,9 @@ int AmSipDialog::getSdpBody(string& sdp_body)
 
     default: 
       break;
-    }
+  }
 
-    return 0;
+  return 0;
 }
 
 int AmSipDialog::rel100OnRequestIn(const AmSipRequest& req)

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Semsdev mailing list
[email protected]
http://lists.iptel.org/mailman/listinfo/semsdev

Reply via email to