Review Board 1.7.16


1.4 implementation of fix for issue 12434

Review Request #311 - Created July 15, 2009 and submitted

Mark Michelson
/branches/1.4/
12434
Reviewers
asterisk-dev
Asterisk
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. Also, since T.38 negotiation is different in those branches, further testing of T.38-specific code will be needed there. I will post a trunk version of these changes to reviewboard after I have written the code.
I set up 2 sipp scenarios. One offered audio and video, and the other offered audio and T.38. I verified that Asterisk gave always gave an appropriate answer to the video and T.38 offers.

Diff revision 2 (Latest)

1 2
1 2

  1. /branches/1.4/channels/chan_sip.c: Loading...
/branches/1.4/channels/chan_sip.c
Revision 199724 New Change
[20] 908 lines
[+20] [+] struct sip_refer {
909
	int attendedtransfer;				/*!< Attended or blind transfer? */
909
	int attendedtransfer;				/*!< Attended or blind transfer? */
910
	int localtransfer;				/*!< Transfer to local domain? */
910
	int localtransfer;				/*!< Transfer to local domain? */
911
	enum referstatus status;			/*!< REFER status */
911
	enum referstatus status;			/*!< REFER status */
912
};
912
};
913

    
   
913

   

    
   
914
struct offered_media {

    
   
915
	int offered;

    
   
916
	char text[128];

    
   
917
};
914
/*! \brief sip_pvt: PVT structures are used for each SIP dialog, ie. a call, a registration, a subscribe  */
918
/*! \brief sip_pvt: PVT structures are used for each SIP dialog, ie. a call, a registration, a subscribe  */
915
static struct sip_pvt {
919
static struct sip_pvt {
916
	ast_mutex_t lock;			/*!< Dialog private lock */
920
	ast_mutex_t lock;			/*!< Dialog private lock */
917
	int method;				/*!< SIP method that opened this dialog */
921
	int method;				/*!< SIP method that opened this dialog */
918
	enum invitestates invitestate;		/*!< The state of the INVITE transaction only */
922
	enum invitestates invitestate;		/*!< The state of the INVITE transaction only */
[+20] [20] 113 lines
[+20] static struct sip_pvt {
1032
	int request_queue_sched_id;		/*!< Scheduler ID of any scheduled action to process queued requests */
1036
	int request_queue_sched_id;		/*!< Scheduler ID of any scheduled action to process queued requests */
1033
	struct sip_pvt *next;			/*!< Next dialog in chain */
1037
	struct sip_pvt *next;			/*!< Next dialog in chain */
1034
	struct sip_invite_param *options;	/*!< Options for INVITE */
1038
	struct sip_invite_param *options;	/*!< Options for INVITE */
1035
	int autoframing;
1039
	int autoframing;
1036
	int hangupcause;			/*!< Storage of hangupcause copied from our owner before we disconnect from the AST channel (only used at hangup) */
1040
	int hangupcause;			/*!< Storage of hangupcause copied from our owner before we disconnect from the AST channel (only used at hangup) */

    
   
1041
	/*! When receiving an SDP offer, it is important to take note of what media types were offered.

    
   
1042
	 * By doing this, even if we don't want to answer a particular media stream with something meaningful, we can

    
   
1043
	 * still put an m= line in our answer with the port set to 0.

    
   
1044
	 *

    
   
1045
	 * The reason for the length being 3 is that in this branch of Asterisk, the only media types supported are 

    
   
1046
	 * image, audio, and video. Therefore we need to keep track of which types of media were offered.

    
   
1047
	 *

    
   
1048
	 * Note that if we wanted to be 100% correct, we would keep a list of all media streams offered. That way we could respond

    
   
1049
	 * even to unknown media types, and we could respond to multiple streams of the same type. Such large-scale changes

    
   
1050
	 * are not a good idea for released branches, though, so we're compromising by just making sure that for the common cases:

    
   
1051
	 * audio and video, and audio and T.38, we give the appropriate response to both media streams.

    
   
1052
	 *

    
   
1053
	 * The large-scale changes would be a good idea for implementing during an SDP rewrite.

    
   
1054
	 */

    
   
1055
	struct offered_media offered_media[3];
1037
} *iflist = NULL;
1056
} *iflist = NULL;
1038

    
   
1057

   
1039
/*! Max entires in the history list for a sip_pvt */
1058
/*! Max entires in the history list for a sip_pvt */
1040
#define MAX_HISTORY_ENTRIES 50
1059
#define MAX_HISTORY_ENTRIES 50
1041

    
   
1060

   
[+20] [20] 4070 lines
[+20] [+] static void change_hold_state(struct sip_pvt *dialog, struct sip_request *req, int holdstate, int sendonly)
5112
}
5131
}
5113

    
   
5132

   
5114
enum media_type {
5133
enum media_type {
5115
	SDP_AUDIO,
5134
	SDP_AUDIO,
5116
	SDP_VIDEO,
5135
	SDP_VIDEO,

    
   
5136
	SDP_IMAGE,
5117
};
5137
};
5118

    
   
5138

   
5119
static int get_ip_and_port_from_sdp(struct sip_request *req, const enum media_type media, struct sockaddr_in *sin)
5139
static int get_ip_and_port_from_sdp(struct sip_request *req, const enum media_type media, struct sockaddr_in *sin)
5120
{
5140
{
5121
	const char *m;
5141
	const char *m;
[+20] [20] 116 lines
[+20] [+] static int process_sdp(struct sip_pvt *p, struct sip_request *req)
5238
	ast_rtp_pt_clear(newvideortp);
5258
	ast_rtp_pt_clear(newvideortp);
5239

    
   
5259

   
5240
	/* Update our last rtprx when we receive an SDP, too */
5260
	/* Update our last rtprx when we receive an SDP, too */
5241
	p->lastrtprx = p->lastrtptx = time(NULL); /* XXX why both ? */
5261
	p->lastrtprx = p->lastrtptx = time(NULL); /* XXX why both ? */
5242

    
   
5262

   

    
   
5263
	memset(p->offered_media, 0, sizeof(p->offered_media));
5243

    
   
5264

   
5244
	/* Try to find first media stream */
5265
	/* Try to find first media stream */
5245
	m = get_sdp(req, "m");
5266
	m = get_sdp(req, "m");
5246
	destiterator = req->sdp_start;
5267
	destiterator = req->sdp_start;
5247
	c = get_sdp_iterate(&destiterator, req, "c");
5268
	c = get_sdp_iterate(&destiterator, req, "c");
[+20] [20] 28 lines
[+20] static int process_sdp(struct sip_pvt *p, struct sip_request *req)
5276
		numberofports = 1;
5297
		numberofports = 1;
5277
		len = -1;
5298
		len = -1;
5278
		if ((sscanf(m, "audio %d/%d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
5299
		if ((sscanf(m, "audio %d/%d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
5279
		    (sscanf(m, "audio %d RTP/AVP %n", &x, &len) == 1 && len > 0)) {
5300
		    (sscanf(m, "audio %d RTP/AVP %n", &x, &len) == 1 && len > 0)) {
5280
			audio = TRUE;
5301
			audio = TRUE;

    
   
5302
			p->offered_media[SDP_AUDIO].offered = TRUE;
5281
			numberofmediastreams++;
5303
			numberofmediastreams++;
5282
			/* Found audio stream in this media definition */
5304
			/* Found audio stream in this media definition */
5283
			portno = x;
5305
			portno = x;
5284
			/* Scan through the RTP payload types specified in a "m=" line: */
5306
			/* Scan through the RTP payload types specified in a "m=" line: */
5285
			for (codecs = m + len; !ast_strlen_zero(codecs); codecs = ast_skip_blanks(codecs + len)) {
5307
			codecs = m + len;

    
   
5308
			ast_copy_string(p->offered_media[SDP_AUDIO].text, codecs, sizeof(p->offered_media[SDP_AUDIO].text));

    
   
5309
			for (; !ast_strlen_zero(codecs); codecs = ast_skip_blanks(codecs + len)) {
5286
				if (sscanf(codecs, "%d%n", &codec, &len) != 1) {
5310
				if (sscanf(codecs, "%d%n", &codec, &len) != 1) {
5287
					ast_log(LOG_WARNING, "Error in codec string '%s'\n", codecs);
5311
					ast_log(LOG_WARNING, "Error in codec string '%s'\n", codecs);
5288
					return -1;
5312
					return -1;
5289
				}
5313
				}
5290
				if (debug)
5314
				if (debug)
5291
					ast_verbose("Found RTP audio format %d\n", codec);
5315
					ast_verbose("Found RTP audio format %d\n", codec);
5292
				ast_rtp_set_m_type(newaudiortp, codec);
5316
				ast_rtp_set_m_type(newaudiortp, codec);
5293
			}
5317
			}
5294
		} else if ((sscanf(m, "video %d/%d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
5318
		} else if ((sscanf(m, "video %d/%d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
5295
		    (sscanf(m, "video %d RTP/AVP %n", &x, &len) == 1 && len >= 0)) {
5319
		    (sscanf(m, "video %d RTP/AVP %n", &x, &len) == 1 && len >= 0)) {
5296
			/* If it is not audio - is it video ? */
5320
			/* If it is not audio - is it video ? */
5297
			ast_clear_flag(&p->flags[0], SIP_NOVIDEO);	
5321
			ast_clear_flag(&p->flags[0], SIP_NOVIDEO);

    
   
5322
			p->offered_media[SDP_VIDEO].offered = TRUE;
5298
			numberofmediastreams++;
5323
			numberofmediastreams++;
5299
			vportno = x;
5324
			vportno = x;
5300
			/* Scan through the RTP payload types specified in a "m=" line: */
5325
			/* Scan through the RTP payload types specified in a "m=" line: */
5301
			for (codecs = m + len; !ast_strlen_zero(codecs); codecs = ast_skip_blanks(codecs + len)) {
5326
			codecs = m + len;

    
   
5327
			ast_copy_string(p->offered_media[SDP_VIDEO].text, codecs, sizeof(p->offered_media[SDP_VIDEO].text));

    
   
5328
			for (; !ast_strlen_zero(codecs); codecs = ast_skip_blanks(codecs + len)) {
5302
				if (sscanf(codecs, "%d%n", &codec, &len) != 1) {
5329
				if (sscanf(codecs, "%d%n", &codec, &len) != 1) {
5303
					ast_log(LOG_WARNING, "Error in codec string '%s'\n", codecs);
5330
					ast_log(LOG_WARNING, "Error in codec string '%s'\n", codecs);
5304
					return -1;
5331
					return -1;
5305
				}
5332
				}
5306
				if (debug)
5333
				if (debug)
5307
					ast_verbose("Found RTP video format %d\n", codec);
5334
					ast_verbose("Found RTP video format %d\n", codec);
5308
				ast_rtp_set_m_type(newvideortp, codec);
5335
				ast_rtp_set_m_type(newvideortp, codec);
5309
			}
5336
			}
5310
		} else if (p->udptl && ( (sscanf(m, "image %d udptl t38%n", &x, &len) == 1 && len > 0) || 
5337
		} else if (p->udptl && ( (sscanf(m, "image %d udptl t38%n", &x, &len) == 1 && len > 0) || 
5311
		 (sscanf(m, "image %d UDPTL t38%n", &x, &len) == 1 && len >= 0) )) {
5338
		 (sscanf(m, "image %d UDPTL t38%n", &x, &len) == 1 && len >= 0) )) {
5312
			if (debug)
5339
			if (debug)
5313
				ast_verbose("Got T.38 offer in SDP in dialog %s\n", p->callid);
5340
				ast_verbose("Got T.38 offer in SDP in dialog %s\n", p->callid);

    
   
5341
			p->offered_media[SDP_IMAGE].offered = TRUE;
5314
			udptlportno = x;
5342
			udptlportno = x;
5315
			numberofmediastreams++;
5343
			numberofmediastreams++;
5316
			
5344
			
5317
			if (p->owner && p->lastinvite) {
5345
			if (p->owner && p->lastinvite) {
5318
				p->t38.state = T38_PEER_REINVITE; /* T38 Offered in re-invite from remote party */
5346
				p->t38.state = T38_PEER_REINVITE; /* T38 Offered in re-invite from remote party */
[+20] [20] 1315 lines
[+20] [+] static enum sip_result add_sdp(struct sip_request *resp, struct sip_pvt *p, int add_audio, int add_t38)
6634
	char *a_video_next = a_video;
6662
	char *a_video_next = a_video;
6635
	char *a_modem_next = a_modem;
6663
	char *a_modem_next = a_modem;
6636
	size_t a_audio_left = sizeof(a_audio);
6664
	size_t a_audio_left = sizeof(a_audio);
6637
	size_t a_video_left = sizeof(a_video);
6665
	size_t a_video_left = sizeof(a_video);
6638
	size_t a_modem_left = sizeof(a_modem);
6666
	size_t a_modem_left = sizeof(a_modem);

    
   
6667
	char dummy_answer[256];
6639

    
   
6668

   
6640
	int x;
6669
	int x;
6641
	int capability = 0;
6670
	int capability = 0;
6642
	int needvideo = FALSE;
6671
	int needvideo = FALSE;
6643
	int debug = sip_debug_test_pvt(p);
6672
	int debug = sip_debug_test_pvt(p);
[+20] [20] 246 lines
[+20] static enum sip_result add_sdp(struct sip_request *resp, struct sip_pvt *p, int add_audio, int add_t38)
6890
	add_line(resp, stime);
6919
	add_line(resp, stime);
6891
	if (add_audio) {
6920
	if (add_audio) {
6892
		add_line(resp, m_audio);
6921
		add_line(resp, m_audio);
6893
		add_line(resp, a_audio);
6922
		add_line(resp, a_audio);
6894
		add_line(resp, hold);
6923
		add_line(resp, hold);

    
   
6924
	} else if (p->offered_media[SDP_AUDIO].offered) {

    
   
6925
		snprintf(dummy_answer, sizeof(dummy_answer), "m=audio 0 RTP/AVP %s\r\n", p->offered_media[SDP_AUDIO].text);

    
   
6926
		add_line(resp, dummy_answer);
6895
	}
6927
	}
6896
	if (needvideo) { /* only if video response is appropriate */
6928
	if (needvideo) { /* only if video response is appropriate */
6897
		add_line(resp, m_video);
6929
		add_line(resp, m_video);
6898
		add_line(resp, a_video);
6930
		add_line(resp, a_video);
6899
		add_line(resp, hold);	/* Repeat hold for the video stream */
6931
		add_line(resp, hold);	/* Repeat hold for the video stream */

    
   
6932
	} else if (p->offered_media[SDP_VIDEO].offered) {

    
   
6933
		snprintf(dummy_answer, sizeof(dummy_answer), "m=video 0 RTP/AVP %s\r\n", p->offered_media[SDP_VIDEO].text);

    
   
6934
		add_line(resp, dummy_answer);
6900
	}
6935
	}
6901
	if (add_t38) {
6936
	if (add_t38) {
6902
		add_line(resp, m_modem);
6937
		add_line(resp, m_modem);
6903
		add_line(resp, a_modem);
6938
		add_line(resp, a_modem);

    
   
6939
	} else if (p->offered_media[SDP_IMAGE].offered) {

    
   
6940
		add_line(resp, "m=image 0 udptl t38\r\n");
6904
	}
6941
	}
6905

    
   
6942

   
6906
	/* Update lastrtprx when we send our SDP */
6943
	/* Update lastrtprx when we send our SDP */
6907
	p->lastrtprx = p->lastrtptx = time(NULL); /* XXX why both ? */
6944
	p->lastrtprx = p->lastrtptx = time(NULL); /* XXX why both ? */
6908

    
   
6945

   
[+20] [20] 131 lines
[+20] [+] static int transmit_reinvite_with_sdp(struct sip_pvt *p)
7040
	add_header(&req, "Supported", SUPPORTED_EXTENSIONS);
7077
	add_header(&req, "Supported", SUPPORTED_EXTENSIONS);
7041
	if (sipdebug)
7078
	if (sipdebug)
7042
		add_header(&req, "X-asterisk-Info", "SIP re-invite (External RTP bridge)");
7079
		add_header(&req, "X-asterisk-Info", "SIP re-invite (External RTP bridge)");
7043
	if (!ast_test_flag(&p->flags[0], SIP_NO_HISTORY))
7080
	if (!ast_test_flag(&p->flags[0], SIP_NO_HISTORY))
7044
		append_history(p, "ReInv", "Re-invite sent");
7081
		append_history(p, "ReInv", "Re-invite sent");

    
   
7082
	memset(p->offered_media, 0, sizeof(p->offered_media));
7045
	add_sdp(&req, p, 1, 0);
7083
	add_sdp(&req, p, 1, 0);
7046
	/* Use this as the basis */
7084
	/* Use this as the basis */
7047
	initialize_initreq(p, &req);
7085
	initialize_initreq(p, &req);
7048
	p->lastinvite = p->ocseq;
7086
	p->lastinvite = p->ocseq;
7049
	ast_set_flag(&p->flags[0], SIP_OUTGOING);		/* Change direction of this dialog */
7087
	ast_set_flag(&p->flags[0], SIP_OUTGOING);		/* Change direction of this dialog */
[+20] [20] 12 lines
[+20] [+] static int transmit_reinvite_with_t38_sdp(struct sip_pvt *p)
7062
	
7100
	
7063
	add_header(&req, "Allow", ALLOWED_METHODS);
7101
	add_header(&req, "Allow", ALLOWED_METHODS);
7064
	add_header(&req, "Supported", SUPPORTED_EXTENSIONS);
7102
	add_header(&req, "Supported", SUPPORTED_EXTENSIONS);
7065
	if (sipdebug)
7103
	if (sipdebug)
7066
		add_header(&req, "X-asterisk-info", "SIP re-invite (T38 switchover)");
7104
		add_header(&req, "X-asterisk-info", "SIP re-invite (T38 switchover)");

    
   
7105
	memset(p->offered_media, 0, sizeof(p->offered_media));
7067
	add_sdp(&req, p, 0, 1);
7106
	add_sdp(&req, p, 0, 1);
7068

    
   
7107

   
7069
	/* Use this as the basis */
7108
	/* Use this as the basis */
7070
	initialize_initreq(p, &req);
7109
	initialize_initreq(p, &req);
7071
	ast_set_flag(&p->flags[0], SIP_OUTGOING);		/* Change direction of this dialog */
7110
	ast_set_flag(&p->flags[0], SIP_OUTGOING);		/* Change direction of this dialog */
[+20] [20] 317 lines
[+20] [+] static int transmit_invite(struct sip_pvt *p, int sipmethod, int sdp, int init)
7389
		}
7428
		}
7390

    
   
7429

   
7391
		ast_channel_unlock(chan);
7430
		ast_channel_unlock(chan);
7392
	}
7431
	}
7393
	if (sdp) {
7432
	if (sdp) {

    
   
7433
		memset(p->offered_media, 0, sizeof(p->offered_media));
7394
		if (p->udptl && p->t38.state == T38_LOCAL_REINVITE) {
7434
		if (p->udptl && p->t38.state == T38_LOCAL_REINVITE) {
7395
			ast_udptl_offered_from_local(p->udptl, 1);
7435
			ast_udptl_offered_from_local(p->udptl, 1);
7396
			if (option_debug)
7436
			if (option_debug)
7397
				ast_log(LOG_DEBUG, "T38 is in state %d on channel %s\n", p->t38.state, p->owner ? p->owner->name : "<none>");
7437
				ast_log(LOG_DEBUG, "T38 is in state %d on channel %s\n", p->t38.state, p->owner ? p->owner->name : "<none>");
7398
			add_sdp(&req, p, 0, 1);
7438
			add_sdp(&req, p, 0, 1);
[+20] [20] 11802 lines
  1. /branches/1.4/channels/chan_sip.c: Loading...

https://reviewboard.asterisk.org/ runs on a server provided by Digium, Inc. and uses bandwidth donated to the open source Asterisk community by API Digital Communications in Huntsville, AL USA.
Please report problems with this site to asteriskteam@digium.com.