[asterisk-commits] mmichelson: trunk r207424 - in /trunk: ./ channels/chan_sip.c

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Mon Jul 20 14:48:18 CDT 2009


Author: mmichelson
Date: Mon Jul 20 14:48:12 2009
New Revision: 207424

URL: http://svn.asterisk.org/svn-view/asterisk?view=rev&rev=207424
Log:
Merged revisions 207423 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
  r207423 | mmichelson | 2009-07-20 14:39:59 -0500 (Mon, 20 Jul 2009) | 33 lines
  
  Answer video SDP offers properly when videosupport is not enabled.
  
  Copied from Review board:
  
  In issue 12434, the reporter describes a situation in which audio and video 
  is offered on the call, but because videosupport is disabled in sip.conf, 
  Asterisk gives no response at all to the video offer. According to RFC 3264, 
  all media offers should have a corresponding answer. For offers we do not 
  intend to actually reply to with meaningful values, we should still reply 
  with the port for the media stream set to 0.
  
  In this patch, we take note of what types of media have been offered and 
  save the information on the sip_pvt. The SDP in the response will take into 
  account whether media was offered. If we are not otherwise going to answer 
  a media offer, we will insert an appropriate m= line with the port set to 0.
  
  It is important to note that this patch is pretty much a bandage being 
  applied to a broken bone. The patch *only* helps for situations where video 
  is offered but videosupport is disabled and when udptl_pt is disabled but 
  T.38 is offered. Asterisk is not guaranteed to respond to every media offer. 
  Notable cases are when multiple streams of the same type are offered. 
  The 2 media stream limit is still present with this patch, too.
  
  In trunk and the 1.6.X branches, things will be a bit different since Asterisk 
  also supports text in SDPs as well.
  
  (closes issue #12434)
  Reported by: mnnojd
  
  Review: https://reviewboard.asterisk.org/r/311
  Review: https://reviewboard.asterisk.org/r/313
........

Modified:
    trunk/   (props changed)
    trunk/channels/chan_sip.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-1.4-merged' - no diff available.

Modified: trunk/channels/chan_sip.c
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/channels/chan_sip.c?view=diff&rev=207424&r1=207423&r2=207424
==============================================================================
--- trunk/channels/chan_sip.c (original)
+++ trunk/channels/chan_sip.c Mon Jul 20 14:48:12 2009
@@ -1691,8 +1691,10 @@
 	int st_max_se;                  /*!< Highest threshold for session refresh interval */
 };
 
-
-
+struct offered_media {
+	int offered;
+	char text[128];
+};
 
 /*! \brief Structure used for each SIP dialog, ie. a call, a registration, a subscribe.
  * Created and initialized by sip_alloc(), the descriptor goes into the list of
@@ -1856,6 +1858,21 @@
 	 * have with this peer to determine its allowed methods.
 	 */
 	unsigned int allowed_methods;
+	/*! When receiving an SDP offer, it is important to take note of what media types were offered.
+	 * By doing this, even if we don't want to answer a particular media stream with something meaningful, we can
+	 * still put an m= line in our answer with the port set to 0.
+	 *
+	 * The reason for the length being 4 is that in this branch of Asterisk, the only media types supported are 
+	 * image, audio, text, and video. Therefore we need to keep track of which types of media were offered.
+	 *
+	 * Note that if we wanted to be 100% correct, we would keep a list of all media streams offered. That way we could respond
+	 * even to unknown media types, and we could respond to multiple streams of the same type. Such large-scale changes
+	 * are not a good idea for released branches, though, so we're compromising by just making sure that for the common cases:
+	 * audio and video, audio and T.38, and audio and text, we give the appropriate response to both media streams.
+	 *
+	 * The large-scale changes would be a good idea for implementing during an SDP rewrite.
+	 */
+	struct offered_media offered_media[4];
 }; 
 
 
@@ -7947,6 +7964,8 @@
 enum media_type {
 	SDP_AUDIO,
 	SDP_VIDEO,
+	SDP_IMAGE,
+	SDP_TEXT,
 };
 
 static int get_ip_and_port_from_sdp(struct sip_request *req, const enum media_type media, struct sockaddr_in *sin)
@@ -8077,6 +8096,8 @@
 	/* Update our last rtprx when we receive an SDP, too */
 	p->lastrtprx = p->lastrtptx = time(NULL); /* XXX why both ? */
 
+	memset(p->offered_media, 0, sizeof(p->offered_media));
+
 	/* Store the SDP version number of remote UA. This will allow us to 
 	distinguish between session modifications and session refreshes. If 
 	the remote UA does not send an incremented SDP version number in a 
@@ -8197,11 +8218,14 @@
 		if ((sscanf(m, "audio %d/%d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
 		    (sscanf(m, "audio %d RTP/AVP %n", &x, &len) == 1 && len > 0)) {
 			audio = TRUE;
+			p->offered_media[SDP_AUDIO].offered = TRUE;
 			numberofmediastreams++;
 			/* Found audio stream in this media definition */
 			portno = x;
 			/* Scan through the RTP payload types specified in a "m=" line: */
-			for (codecs = m + len; !ast_strlen_zero(codecs); codecs = ast_skip_blanks(codecs + len)) {
+			codecs = m + len;
+			ast_copy_string(p->offered_media[SDP_AUDIO].text, codecs, sizeof(p->offered_media[SDP_AUDIO].text));
+			for (; !ast_strlen_zero(codecs); codecs = ast_skip_blanks(codecs + len)) {
 				if (sscanf(codecs, "%d%n", &codec, &len) != 1) {
 					ast_log(LOG_WARNING, "Error in codec string '%s'\n", codecs);
 					return -1;
@@ -8215,10 +8239,13 @@
 		    (sscanf(m, "video %d RTP/AVP %n", &x, &len) == 1 && len >= 0)) {
 			video = TRUE;
 			p->novideo = FALSE;
+			p->offered_media[SDP_VIDEO].offered = TRUE;
 			numberofmediastreams++;
 			vportno = x;
 			/* Scan through the RTP payload types specified in a "m=" line: */
-			for (codecs = m + len; !ast_strlen_zero(codecs); codecs = ast_skip_blanks(codecs + len)) {
+			codecs = m + len;
+			ast_copy_string(p->offered_media[SDP_VIDEO].text, codecs, sizeof(p->offered_media[SDP_VIDEO].text));
+			for (; !ast_strlen_zero(codecs); codecs = ast_skip_blanks(codecs + len)) {
 				if (sscanf(codecs, "%d%n", &codec, &len) != 1) {
 					ast_log(LOG_WARNING, "Error in codec string '%s'\n", codecs);
 					return -1;
@@ -8230,11 +8257,14 @@
 		} else if ((sscanf(m, "text %d/%d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
 		    (sscanf(m, "text %d RTP/AVP %n", &x, &len) == 1 && len > 0)) {
 			text = TRUE;
+			p->offered_media[SDP_TEXT].offered = TRUE;
 			p->notext = FALSE;
 			numberofmediastreams++;
 			tportno = x;
 			/* Scan through the RTP payload types specified in a "m=" line: */
-			for (codecs = m + len; !ast_strlen_zero(codecs); codecs = ast_skip_blanks(codecs + len)) {
+			codecs = m + len;
+			ast_copy_string(p->offered_media[SDP_TEXT].text, codecs, sizeof(p->offered_media[SDP_TEXT].text));
+			for (; !ast_strlen_zero(codecs); codecs = ast_skip_blanks(codecs + len)) {
 				if (sscanf(codecs, "%d%n", &codec, &len) != 1) {
 					ast_log(LOG_WARNING, "Error in codec string '%s'\n", codecs);
 					return -1;
@@ -8247,6 +8277,7 @@
 			(sscanf(m, "image %d UDPTL t38%n", &x, &len) == 1 && len > 0) )) {
 			if (debug)
 				ast_verbose("Got T.38 offer in SDP in dialog %s\n", p->callid);
+			p->offered_media[SDP_IMAGE].offered = TRUE;
 			udptlportno = x;
 			numberofmediastreams++;
 		} else 
@@ -9192,7 +9223,6 @@
 	return 0;
 }
 
-
 /*! \brief Prepare SIP response packet */
 static int respprep(struct sip_request *resp, struct sip_pvt *p, const char *msg, const struct sip_request *req)
 {
@@ -9977,6 +10007,7 @@
 
 	char codecbuf[SIPBUFSIZE];
 	char buf[SIPBUFSIZE];
+	char dummy_answer[256];
 
 	/* Set the SDP session name */
 	snprintf(subject, sizeof(subject), "s=%s\r\n", ast_strlen_zero(global_sdpsession) ? "-" : global_sdpsession);
@@ -10246,20 +10277,31 @@
 		add_line(resp, m_audio->str);
 		add_line(resp, a_audio->str);
 		add_line(resp, hold);
+	} else if (p->offered_media[SDP_AUDIO].offered) {
+		snprintf(dummy_answer, sizeof(dummy_answer), "m=audio 0 RTP/AVP %s\r\n", p->offered_media[SDP_AUDIO].text);
+		add_line(resp, dummy_answer);
 	}
 	if (needvideo) { /* only if video response is appropriate */
 		add_line(resp, m_video->str);
 		add_line(resp, a_video->str);
 		add_line(resp, hold);	/* Repeat hold for the video stream */
+	} else if (p->offered_media[SDP_VIDEO].offered) {
+		snprintf(dummy_answer, sizeof(dummy_answer), "m=video 0 RTP/AVP %s\r\n", p->offered_media[SDP_VIDEO].text);
+		add_line(resp, dummy_answer);
 	}
 	if (needtext) { /* only if text response is appropriate */
 		add_line(resp, m_text->str);
 		add_line(resp, a_text->str);
 		add_line(resp, hold);	/* Repeat hold for the text stream */
+	} else if (p->offered_media[SDP_TEXT].offered) {
+		snprintf(dummy_answer, sizeof(dummy_answer), "m=text 0 RTP/AVP %s\r\n", p->offered_media[SDP_TEXT].text);
+		add_line(resp, dummy_answer);
 	}
 	if (add_t38) {
 		add_line(resp, m_modem->str);
 		add_line(resp, a_modem->str);
+	} else if (p->offered_media[SDP_IMAGE].offered) {
+		add_line(resp, "m=image 0 udptl t38\r\n");
 	}
 
 	/* Update lastrtprx when we send our SDP */
@@ -10424,6 +10466,7 @@
 
 	if (p->do_history)
 		append_history(p, "ReInv", "Re-invite sent");
+	memset(p->offered_media, 0, sizeof(p->offered_media));
 
 	try_suggested_sip_codec(p);
 	if (t38version)
@@ -10806,6 +10849,7 @@
 		add_diversion_header(&req, p);
 	}
 	if (sdp) {
+		memset(p->offered_media, 0, sizeof(p->offered_media));
 		if (p->udptl && p->t38.state == T38_LOCAL_REINVITE) {
 			ast_udptl_offered_from_local(p->udptl, 1);
 			ast_debug(1, "T38 is in state %d on channel %s\n", p->t38.state, p->owner ? p->owner->name : "<none>");




More information about the asterisk-commits mailing list