Review Board 1.7.16


Add ability to set an alternate source for RTP media. Helps in SIP glare situations.

Review Request #252 - Created May 19, 2009 and submitted

Mark Michelson
branches/1.4/
Reviewers
asterisk-dev
Asterisk
For a description of the problem, see this: http://lists.digium.com/pipermail/asterisk-dev/2009-May/038348.html

This patch implements a way to let the RTP stack know that there may be a potential alternate source for media. The alterations to RTP are:

* new function ast_rtp_set_altpeer to set what the alternate media source may be.
* new sockaddr_in structures inside the ast_rtp and ast_rtcp structures to hold these alternate sources.
* ast_rtp_read and ast_rtcp_read have been updated to expect media from these alternate sources.

The alterations to chan_sip are:

* new function get_ip_and_port_from_sdp to get the remote IP address and port for audio/video streams from the SDP
* When we are going to respond to a REINVITE with a 491, we call get_ip_and_port_from_sdp, followed by ast_rtp_set_altpeer so that the
  RTP stack will not react incorrectly when it receives media from this alternate source.

Please see the "Testing Done" section for some code review details.
I tested by setting up phones and Asterisk boxes as shown in the first diagram in the link I printed in the "Description."

I found that in REINVITE glare situations, I always successfully had two-way audio. There was a slight catch, though. Usually there would be about a 0.5-1 second gap between when the callee answered the phone and when the caller was able to hear the callee's audio. I have not yet been able to track this odd behavior down. So, in addition to making sure that what I have presented here looks reasonable, if any reviewers might be able to point out what potentially is causing the short delay upon answering the calls, please speak up.

Diff revision 2 (Latest)

1 2
1 2

  1. /branches/1.4/channels/chan_sip.c: Loading...
  2. /branches/1.4/include/asterisk/rtp.h: Loading...
  3. /branches/1.4/main/rtp.c: Loading...
/branches/1.4/channels/chan_sip.c
Revision 194872 New Change
[20] 5107 lines
[+20] [+] static void change_hold_state(struct sip_pvt *dialog, struct sip_request *req, int holdstate, int sendonly)
5108
	else
5108
	else
5109
		ast_set_flag(&dialog->flags[1], SIP_PAGE2_CALL_ONHOLD_ACTIVE);
5109
		ast_set_flag(&dialog->flags[1], SIP_PAGE2_CALL_ONHOLD_ACTIVE);
5110
	return;
5110
	return;
5111
}
5111
}
5112

    
   
5112

   

    
   
5113
enum media_type {

    
   
5114
	SDP_AUDIO,

    
   
5115
	SDP_VIDEO,

    
   
5116
};

    
   
5117

   

    
   
5118
static int get_ip_and_port_from_sdp(struct sip_request *req, const enum media_type media, struct sockaddr_in *sin)

    
   
5119
{

    
   
5120
	const char *m;

    
   
5121
	const char *c;

    
   
5122
	int miterator = req->sdp_start;

    
   
5123
	int citerator = req->sdp_start;

    
   
5124
	int x = 0;

    
   
5125
	int numberofports;

    
   
5126
	int len;

    
   
5127
	char host[258] = ""; /*Initialize to empty so we will know if we have any input */

    
   
5128
	struct ast_hostent audiohp;

    
   
5129
	struct hostent *hp;

    
   
5130

   

    
   
5131
	c = get_sdp_iterate(&citerator, req, "c");

    
   
5132
	if (sscanf(c, "IN IP4 %256s", host) != 1) {

    
   
5133
		ast_log(LOG_WARNING, "Invalid host in c= line, '%s'\n", c);

    
   
5134
		/* Continue since there may be a valid host in a c= line specific to the audio stream */

    
   
5135
	}

    
   
5136
	/* We only want the m and c lines for audio */

    
   
5137
	while ((m = get_sdp_iterate(&miterator, req, "m"))) {

    
   
5138
		if ((media == SDP_AUDIO && ((sscanf(m, "audio %d/%d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||

    
   
5139
		    (sscanf(m, "audio %d RTP/AVP %n", &x, &len) == 1 && len > 0))) ||

    
   
5140
			(media == SDP_VIDEO && ((sscanf(m, "video %d/%d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||

    
   
5141
		    (sscanf(m, "video %d RTP/AVP %n", &x, &len) == 1 && len > 0)))) {

    
   
5142
			/* See if there's a c= line for this media stream.

    
   
5143
			 * XXX There is no guarantee that we'll be grabbing the c= line for this

    
   
5144
			 * particular media stream here. However, this is the same logic used in process_sdp.

    
   
5145
			 */

    
   
5146
			c = get_sdp_iterate(&citerator, req, "c");

    
   
5147
			if (!ast_strlen_zero(c)) {

    
   
5148
				sscanf(c, "IN IP4 %256s", host);

    
   
5149
			}

    
   
5150
			break;

    
   
5151
		}

    
   
5152
	}

    
   
5153

   

    
   
5154
	if (ast_strlen_zero(host) || x == 0) {

    
   
5155
		ast_log(LOG_WARNING, "Failed to read an alternate host or port in SDP. Expect %s problems\n", media == SDP_AUDIO ? "audio" : "video");

    
   
5156
		return -1;

    
   
5157
	}

    
   
5158

   

    
   
5159
	hp = ast_gethostbyname(host, &audiohp);

    
   
5160
	if (!hp) {

    
   
5161
		ast_log(LOG_WARNING, "Could not look up IP address of alternate hostname. Expect %s problems\n", media == SDP_AUDIO? "audio" : "video");

    
   
5162
		return -1;

    
   
5163
	}

    
   
5164

   

    
   
5165
	memcpy(&sin->sin_addr, hp->h_addr, sizeof(sin->sin_addr));

    
   
5166
	sin->sin_port = htons(x);

    
   
5167
	return -0;

    
   
5168
}

    
   
5169

   
5113
/*! \brief Process SIP SDP offer, select formats and activate RTP channels
5170
/*! \brief Process SIP SDP offer, select formats and activate RTP channels
5114
	If offer is rejected, we will not change any properties of the call
5171
	If offer is rejected, we will not change any properties of the call
5115
 	Return 0 on success, a negative value on errors.
5172
 	Return 0 on success, a negative value on errors.
5116
	Must be called after find_sdp().
5173
	Must be called after find_sdp().
5117
*/
5174
*/
[+20] [20] 9333 lines
[+20] [+] static int handle_request_invite(struct sip_pvt *p, struct sip_request *req, int debug, int seqno, struct sockaddr_in *sin, int *recount, char *e, int *nounlock)
14451
			 */
14508
			 */
14452
			__sip_ack(p, p->lastinvite, FLAG_RESPONSE, 0);
14509
			__sip_ack(p, p->lastinvite, FLAG_RESPONSE, 0);
14453
		} else {
14510
		} else {
14454
			/* We already have a pending invite. Sorry. You are on hold. */
14511
			/* We already have a pending invite. Sorry. You are on hold. */
14455
			p->glareinvite = seqno;
14512
			p->glareinvite = seqno;

    
   
14513
			if (p->rtp && find_sdp(req)) {

    
   
14514
				struct sockaddr_in sin;

    
   
14515
				if (get_ip_and_port_from_sdp(req, SDP_AUDIO, &sin)) {

    
   
14516
					ast_log(LOG_WARNING, "Failed to set an alternate media source on glared reinvite. Audio may not work properly on this call.\n");

    
   
14517
				} else {

    
   
14518
					ast_rtp_set_alt_peer(p->rtp, &sin);

    
   
14519
				}

    
   
14520
				if (p->vrtp) {

    
   
14521
					if (get_ip_and_port_from_sdp(req, SDP_VIDEO, &sin)) {

    
   
14522
						ast_log(LOG_WARNING, "Failed to set an alternate media source on glared reinvite. Video may not work properly on this call.\n");

    
   
14523
					} else {

    
   
14524
						ast_rtp_set_alt_peer(p->vrtp, &sin);

    
   
14525
					}

    
   
14526
				}

    
   
14527
			}
14456
			transmit_response_reliable(p, "491 Request Pending", req);
14528
			transmit_response_reliable(p, "491 Request Pending", req);
14457
			if (option_debug)
14529
			if (option_debug)
14458
				ast_log(LOG_DEBUG, "Got INVITE on call where we already have pending INVITE, deferring that - %s\n", p->callid);
14530
				ast_log(LOG_DEBUG, "Got INVITE on call where we already have pending INVITE, deferring that - %s\n", p->callid);
14459
			/* Don't destroy dialog here */
14531
			/* Don't destroy dialog here */
14460
			return 0;
14532
			return 0;
[+20] [20] 4662 lines
/branches/1.4/include/asterisk/rtp.h
Revision 194872 New Change
 
/branches/1.4/main/rtp.c
Revision 194872 New Change
 
  1. /branches/1.4/channels/chan_sip.c: Loading...
  2. /branches/1.4/include/asterisk/rtp.h: Loading...
  3. /branches/1.4/main/rtp.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.