[asterisk-commits] kpfleming: branch kpfleming/sdp-parsing-unbreakage r229637 - /team/kpfleming/...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Thu Nov 12 07:25:49 CST 2009


Author: kpfleming
Date: Thu Nov 12 07:25:45 2009
New Revision: 229637

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=229637
Log:
restructure session state change logic so it actually works

convert sendonly magic numbers to enum

more coding guidelines fixes


Modified:
    team/kpfleming/sdp-parsing-unbreakage/channels/chan_sip.c

Modified: team/kpfleming/sdp-parsing-unbreakage/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/team/kpfleming/sdp-parsing-unbreakage/channels/chan_sip.c?view=diff&rev=229637&r1=229636&r2=229637
==============================================================================
--- team/kpfleming/sdp-parsing-unbreakage/channels/chan_sip.c (original)
+++ team/kpfleming/sdp-parsing-unbreakage/channels/chan_sip.c Thu Nov 12 07:25:45 2009
@@ -1803,6 +1803,13 @@
 static int sip_sipredirect(struct sip_pvt *p, const char *dest);
 
 /*--- Codec handling / SDP */
+enum media_session_mode {
+	MEDIA_SENDRECV = 0,
+	MEDIA_SENDONLY,
+	MEDIA_RECVONLY,
+	MEDIA_INACTIVE,
+};
+
 static void try_suggested_sip_codec(struct sip_pvt *p);
 static const char *get_sdp_iterate(int* start, struct sip_request *req, const char *name);
 static char get_sdp_line(int *start, int stop, struct sip_request *req, const char **value);
@@ -1810,7 +1817,7 @@
 static int process_sdp(struct sip_pvt *p, struct sip_request *req, int t38action);
 static int process_sdp_o(const char *o, struct sip_pvt *p);
 static int process_sdp_c(const char *c, struct ast_hostent *hp);
-static int process_sdp_a_sendonly(const char *a, int *sendonly);
+static int process_sdp_a_mode(const char *a, enum media_session_mode *mode);
 static int process_sdp_a_audio(const char *a, struct sip_pvt *p, struct ast_rtp *newaudiortp, int *last_rtpmap_codec);
 static int process_sdp_a_video(const char *a, struct sip_pvt *p, struct ast_rtp *newvideortp, int *last_rtpmap_codec);
 static int process_sdp_a_text(const char *a, struct sip_pvt *p, struct ast_rtp *newtextrtp, int *last_rtpmap_codec);
@@ -6836,9 +6843,9 @@
 	/* We only want the m and c lines for audio */
 	for (m = get_sdp_iterate(&miterator, req, "m"); !ast_strlen_zero(m); m = get_sdp_iterate(&miterator, req, "m")) {
 		if ((media == SDP_AUDIO && ((sscanf(m, "audio %30d/%30d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
-		    (sscanf(m, "audio %30d RTP/AVP %n", &x, &len) == 1 && len > 0))) ||
-			(media == SDP_VIDEO && ((sscanf(m, "video %30d/%30d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
-		    (sscanf(m, "video %30d RTP/AVP %n", &x, &len) == 1 && len > 0)))) {
+					    (sscanf(m, "audio %30d RTP/AVP %n", &x, &len) == 1 && len > 0))) ||
+		    (media == SDP_VIDEO && ((sscanf(m, "video %30d/%30d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
+					    (sscanf(m, "video %30d RTP/AVP %n", &x, &len) == 1 && len > 0)))) {
 			/* See if there's a c= line for this media stream.
 			 * XXX There is no guarantee that we'll be grabbing the c= line for this
 			 * particular media stream here. However, this is the same logic used in process_sdp.
@@ -6915,8 +6922,8 @@
 	int codec;
 
 	/* Others */
-	int sendonly = -1;
-	int vsendonly = -1;
+	enum media_session_mode amode = MEDIA_SENDRECV;
+	enum media_session_mode vmode = MEDIA_SENDRECV;
 	int numberofports;
 	int numberofmediastreams = 0;
 	int last_rtpmap_codec = 0;
@@ -6992,9 +6999,9 @@
 			}
 			break;
 		case 'a':
-			if (process_sdp_a_sendonly(value, &sendonly)) {
+			if (process_sdp_a_mode(value, &amode)) {
 				processed = TRUE;
-				vsendonly = sendonly;
+				vmode = amode;
 			} else if (process_sdp_a_audio(value, p, newaudiortp, &last_rtpmap_codec)) {
 				processed = TRUE;
 			} else if (process_sdp_a_video(value, p, newvideortp, &last_rtpmap_codec)) {
@@ -7101,18 +7108,6 @@
 
 			if (p->t38.state != T38_ENABLED) {
 				memset(&p->t38.their_parms, 0, sizeof(p->t38.their_parms));
-
-				/* KPF: these state changes should not occur until we decide whether
-				   we are actually going to accept this offer or not
-				*/
-				/* Remote party offers T38, we need to update state */
-				if ((t38action == SDP_T38_ACCEPT) &&
-				    (p->t38.state == T38_LOCAL_REINVITE)) {
-					change_t38_state(p, T38_ENABLED);
-				} else if ((t38action == SDP_T38_INITIATE) &&
-					   p->owner && p->lastinvite) {
-					change_t38_state(p, T38_PEER_REINVITE); /* T38 Offered in re-invite from remote party */
-				}
 			}
 		} else {
 			ast_log(LOG_WARNING, "Unsupported SDP media type in offer: %s\n", m);
@@ -7154,13 +7149,13 @@
 				break;
 			case 'a':
 				if (audio) {
-					if (process_sdp_a_sendonly(value, &sendonly)) {
+					if (process_sdp_a_mode(value, &amode)) {
 						processed = TRUE;
 					} else if (process_sdp_a_audio(value, p, newaudiortp, &last_rtpmap_codec)) {
 						processed = TRUE;
 					}
 				} else if (video) {
-					if (process_sdp_a_sendonly(value, &vsendonly)) {
+					if (process_sdp_a_mode(value, &vmode)) {
 						processed = TRUE;
 					} else if (process_sdp_a_video(value, p, newvideortp, &last_rtpmap_codec)) {
 						processed = TRUE;
@@ -7187,7 +7182,7 @@
 	}
 
 	/* Sanity checks */
-	if (aportno == -1 && vportno == -1 && iportno == -1  && tportno == -1) {
+	if (aportno == -1 && vportno == -1 && iportno == -1 && tportno == -1) {
 		/* No acceptable offer found in SDP  - we have no ports */
 		/* Do not change RTP or VRTP if this is a re-invite */
 		return -2;
@@ -7196,10 +7191,6 @@
 	if (numberofmediastreams > 3) {
 		/* We have too many fax, audio and/or video and/or text media streams, fail this offer */
 		return -3;
-	}
-
-	if (iportno == -1) {
-		change_t38_state(p, T38_DISABLED);
 	}
 
 	/* Now gather all of the codecs that we are asked for: */
@@ -7227,47 +7218,17 @@
 			    ast_rtp_lookup_mime_multiple(s2, SIPBUFSIZE, apeernoncodeccapability, 0, 0),
 			    ast_rtp_lookup_mime_multiple(s3, SIPBUFSIZE, newnoncodeccapability, 0, 0));
 	}
-	if (!newjointcapability) {
-		/* If T.38 was not negotiated either, totally bail out... */
-		if ((p->t38.state == T38_DISABLED) || !iportno) {
-			ast_log(LOG_NOTICE, "No compatible codecs, not accepting this offer!\n");
-			/* Do NOT Change current setting */
-			return -1;
-		} else {
-			ast_debug(3, "Have T.38 but no audio codecs, accepting offer anyway\n");
-			/* KPF: except we don't actually finish processing the offer, so the call is broken */
-			return 0;
-		}
-	}
-
-	/* We are now ready to change the sip session and p->rtp and p->vrtp with the offered codecs, since
-		they are acceptable */
-	p->jointcapability = newjointcapability;	        /* Our joint codec profile for this call */
-	p->peercapability = newpeercapability;		        /* The other sides capability in latest offer */
-	p->jointnoncodeccapability = newnoncodeccapability;	/* DTMF capabilities */
-
-	ast_rtp_pt_copy(p->rtp, newaudiortp);
-	if (p->vrtp)
-		ast_rtp_pt_copy(p->vrtp, newvideortp);
-	if (p->trtp)
-		ast_rtp_pt_copy(p->trtp, newtextrtp);
-
-	if (ast_test_flag(&p->flags[0], SIP_DTMF) == SIP_DTMF_AUTO) {
-		ast_clear_flag(&p->flags[0], SIP_DTMF);
-		if (newnoncodeccapability & AST_RTP_DTMF) {
-			/* XXX Would it be reasonable to drop the DSP at this point? XXX */
-			ast_set_flag(&p->flags[0], SIP_DTMF_RFC2833);
-			/* Since RFC2833 is now negotiated we need to change some properties of the RTP stream */
-			ast_rtp_setdtmf(p->rtp, 1);
-			ast_rtp_setdtmfcompensate(p->rtp, ast_test_flag(&p->flags[1], SIP_PAGE2_RFC2833_COMPENSATE));
-		} else {
-			ast_set_flag(&p->flags[0], SIP_DTMF_INBAND);
-		}
-	}
-
+
+	if (!newjointcapability && (aportno != -1)) {
+		ast_log(LOG_NOTICE, "No compatible codecs, not accepting this offer!\n");
+		return -1;
+	}
+
+	/* We are now ready to change the sip session with the offered codecs, since
+	   they are acceptable */
 	/* Setup audio address and port */
 	if (p->rtp) {
-		if (aportno > 0) {
+		if (aportno != -1) {
 			struct sockaddr_in sin;
 
 			sin.sin_family = AF_INET;
@@ -7277,6 +7238,23 @@
 			if (debug) {
 				ast_verbose("Peer audio RTP is at port %s:%d\n", ast_inet_ntoa(sin.sin_addr), ntohs(sin.sin_port));
 			}
+			p->jointcapability = newjointcapability;	        /* Our joint codec profile for this call */
+			p->peercapability = newpeercapability;		        /* The other sides capability in latest offer */
+			p->jointnoncodeccapability = newnoncodeccapability;	/* DTMF capabilities */
+
+			ast_rtp_pt_copy(p->rtp, newaudiortp);
+			if (ast_test_flag(&p->flags[0], SIP_DTMF) == SIP_DTMF_AUTO) {
+				ast_clear_flag(&p->flags[0], SIP_DTMF);
+				if (newnoncodeccapability & AST_RTP_DTMF) {
+					/* XXX Would it be reasonable to drop the DSP at this point? XXX */
+					ast_set_flag(&p->flags[0], SIP_DTMF_RFC2833);
+					/* Since RFC2833 is now negotiated we need to change some properties of the RTP stream */
+					ast_rtp_setdtmf(p->rtp, 1);
+					ast_rtp_setdtmfcompensate(p->rtp, ast_test_flag(&p->flags[1], SIP_PAGE2_RFC2833_COMPENSATE));
+				} else {
+					ast_set_flag(&p->flags[0], SIP_DTMF_INBAND);
+				}
+			}
 		} else {
 			ast_rtp_stop(p->rtp);
 			if (debug) {
@@ -7284,10 +7262,9 @@
 			}
 		}
 	}
-
 	/* Setup video address and port */
 	if (p->vrtp) {
-		if (vportno > 0) {
+		if (vportno != -1) {
 			struct sockaddr_in sin;
 
 			sin.sin_family = AF_INET;
@@ -7297,6 +7274,7 @@
 			if (debug) {
 				ast_verbose("Peer video RTP is at port %s:%d\n", ast_inet_ntoa(sin.sin_addr), ntohs(sin.sin_port));
 			}
+			ast_rtp_pt_copy(p->vrtp, newvideortp);
 		} else {
 			ast_rtp_stop(p->vrtp);
 			if (debug) {
@@ -7304,10 +7282,9 @@
 			}
 		}
 	}
-
 	/* Setup text address and port */
 	if (p->trtp) {
-		if (tportno > 0) {
+		if (tportno != -1) {
 			struct sockaddr_in sin;
 
 			sin.sin_family = AF_INET;
@@ -7317,6 +7294,7 @@
 			if (debug) {
 				ast_verbose("Peer T.140 RTP is at port %s:%d\n", ast_inet_ntoa(sin.sin_addr), ntohs(sin.sin_port));
 			}
+			ast_rtp_pt_copy(p->trtp, newtextrtp);
 		} else {
 			ast_rtp_stop(p->trtp);
 			if (debug) {
@@ -7326,13 +7304,14 @@
 	}
 	/* Setup image address and port */
 	if (p->udptl) {
-		if (iportno > 0) {
+		if (iportno != -1) {
 			struct sockaddr_in sin;
 
 			sin.sin_family = AF_INET;
 			sin.sin_port = htons(iportno);
 			if (ast_test_flag(&p->flags[0], SIP_NAT) && ast_test_flag(&p->flags[1], SIP_PAGE2_UDPTL_DESTINATION)) {
 				struct sockaddr_in remote_address = { 0, };
+
 				ast_rtp_get_peer(p->rtp, &remote_address);
 				if (remote_address.sin_addr.s_addr) {
 					memcpy(&sin, &remote_address, sizeof(sin));
@@ -7345,12 +7324,26 @@
 			if (debug) {
 				ast_debug(1,"Peer T.38 UDPTL is at port %s:%d\n", ast_inet_ntoa(sin.sin_addr), ntohs(sin.sin_port));
 			}
+			/* Remote party offers T38, we need to update state */
+			if ((t38action == SDP_T38_ACCEPT) &&
+			    (p->t38.state == T38_LOCAL_REINVITE)) {
+				change_t38_state(p, T38_ENABLED);
+			} else if ((t38action == SDP_T38_INITIATE) &&
+				   p->owner && p->lastinvite) {
+				change_t38_state(p, T38_PEER_REINVITE); /* T38 Offered in re-invite from remote party */
+			}
 		} else {
+			change_t38_state(p, T38_DISABLED);
 			ast_udptl_stop(p->udptl);
 			if (debug) {
 				ast_debug(1, "Peer doesn't want T.38 UDPTL\n");
 			}
 		}
+	}
+
+	if ((aportno == -1) && (p->t38.state != T38_DISABLED)) {
+		ast_debug(3, "Have T.38 but no audio, accepting offer anyway\n");
+		return 0;
 	}
 
 	/* Ok, we're going with this offer */
@@ -7375,7 +7368,7 @@
 		ast_set_write_format(p->owner, p->owner->writeformat);
 	}
 	
-	if (ast_test_flag(&p->flags[1], SIP_PAGE2_CALL_ONHOLD) && ahp && ahp->h_addr && (!sendonly || sendonly == -1)) {
+	if (ast_test_flag(&p->flags[1], SIP_PAGE2_CALL_ONHOLD) && ahp && ahp->h_addr && (amode == MEDIA_SENDRECV)) {
 		ast_queue_control(p->owner, AST_CONTROL_UNHOLD);
 		/* Activate a re-invite */
 		ast_queue_frame(p->owner, &ast_null_frame);
@@ -7393,13 +7386,13 @@
 			sip_peer_hold(p, FALSE);
 		}
 		ast_clear_flag(&p->flags[1], SIP_PAGE2_CALL_ONHOLD); /* Clear both flags */
-	} else if ((ahp && !ahp->h_addr) || (sendonly && sendonly != -1)) {
+	} else if ((ahp && !ahp->h_addr) || (amode != MEDIA_SENDRECV)) {
 		int already_on_hold = ast_test_flag(&p->flags[1], SIP_PAGE2_CALL_ONHOLD);
 
 		ast_queue_control_data(p->owner, AST_CONTROL_HOLD, 
 				       S_OR(p->mohsuggest, NULL),
 				       !ast_strlen_zero(p->mohsuggest) ? strlen(p->mohsuggest) + 1 : 0);
-		if (sendonly) {
+		if (amode != MEDIA_SENDRECV) {
 			ast_rtp_stop(p->rtp);
 		}
 		/* RTCP needs to go ahead, even if we're on hold!!! */
@@ -7415,10 +7408,10 @@
 				      p->owner->name, 
 				      p->owner->uniqueid);
 		}
-		if (sendonly == 1) {
+		if (amode == MEDIA_SENDONLY) {
 			/* One directional hold (sendonly/recvonly) */
 			ast_set_flag(&p->flags[1], SIP_PAGE2_CALL_ONHOLD_ONEDIR);
-		} else if (sendonly == 2) {
+		} else if (amode == MEDIA_INACTIVE) {
 			/* Inactive stream */
 			ast_set_flag(&p->flags[1], SIP_PAGE2_CALL_ONHOLD_INACTIVE);
 		} else {
@@ -7530,24 +7523,19 @@
 	return FALSE;
 }
 
-static int process_sdp_a_sendonly(const char *a, int *sendonly)
-{
-	int found = FALSE;
-
+static int process_sdp_a_mode(const char *a, enum media_session_mode *mode)
+{
 	if (!strcasecmp(a, "sendonly")) {
-		if (*sendonly == -1)
-			*sendonly = 1;
-		found = TRUE;
+		*mode = MEDIA_SENDONLY;
+		return TRUE;
 	} else if (!strcasecmp(a, "inactive")) {
-		if (*sendonly == -1)
-			*sendonly = 2;
-		found = TRUE;
+		*mode = MEDIA_INACTIVE;
+		return TRUE;
 	}  else if (!strcasecmp(a, "sendrecv")) {
-		if (*sendonly == -1)
-			*sendonly = 0;
-		found = TRUE;
-	}
-	return found;
+		*mode = MEDIA_SENDRECV;
+		return TRUE;
+	}
+	return FALSE;
 }
 
 static int process_sdp_a_audio(const char *a, struct sip_pvt *p, struct ast_rtp *newaudiortp, int *last_rtpmap_codec)
@@ -7574,10 +7562,13 @@
 			int format = 0;
 			for (codec_n = 0; codec_n < MAX_RTP_PT; codec_n++) {
 				format = ast_rtp_codec_getformat(codec_n);
-				if (!format)	/* non-codec or not found */
+				if (!format) {
+					/* non-codec or not found */
 					continue;
-				if (option_debug)
+				}
+				if (option_debug) {
 					ast_log(LOG_DEBUG, "Setting framing for %d to %ld\n", format, framing);
+				}
 				ast_codec_pref_setsize(pref, format, framing);
 			}
 			ast_rtp_codec_setpref(p->rtp, pref);
@@ -7588,19 +7579,20 @@
 		if (*last_rtpmap_codec < SDP_MAX_RTPMAP_CODECS) {
 			if (ast_rtp_set_rtpmap_type(newaudiortp, codec, "audio", mimeSubtype,
 			    ast_test_flag(&p->flags[0], SIP_G726_NONSTANDARD) ? AST_RTP_OPT_G726_NONSTANDARD : 0) != -1) {
-				if (debug)
+				if (debug) {
 					ast_verbose("Found audio description format %s for ID %d\n", mimeSubtype, codec);
+				}
 				//found_rtpmap_codecs[last_rtpmap_codec] = codec;
 				(*last_rtpmap_codec)++;
 				found = TRUE;
 			} else {
 				ast_rtp_unset_m_type(newaudiortp, codec);
-				if (debug)
+				if (debug) {
 					ast_verbose("Found unknown media description format %s for ID %d\n", mimeSubtype, codec);
+				}
 			}
-		} else {
-			if (debug)
-				ast_verbose("Discarded description format %s for ID %d\n", mimeSubtype, codec);
+		} else if (debug) {
+			ast_verbose("Discarded description format %s for ID %d\n", mimeSubtype, codec);
 		}
 	}
 
@@ -7620,20 +7612,21 @@
 			/* Note: should really look at the '#chans' params too */
 			if (!strncasecmp(mimeSubtype, "H26", 3) || !strncasecmp(mimeSubtype, "MP4", 3)) {
 				if (ast_rtp_set_rtpmap_type(newvideortp, codec, "video", mimeSubtype, 0) != -1) {
-					if (debug)
+					if (debug) {
 						ast_verbose("Found video description format %s for ID %d\n", mimeSubtype, codec);
+					}
 					//found_rtpmap_codecs[last_rtpmap_codec] = codec;
 					(*last_rtpmap_codec)++;
 					found = TRUE;
 				} else {
 					ast_rtp_unset_m_type(newvideortp, codec);
-					if (debug) 
+					if (debug) {
 						ast_verbose("Found unknown media description format %s for ID %d\n", mimeSubtype, codec);
+					}
 				}
 			}
-		} else {
-			if (debug)
-				ast_verbose("Discarded description format %s for ID %d\n", mimeSubtype, codec);
+		} else if (debug) {
+			ast_verbose("Discarded description format %s for ID %d\n", mimeSubtype, codec);
 		}
 	}
 
@@ -7657,9 +7650,8 @@
 					found = TRUE;
 				}
 			}
-		} else {
-			if (debug)
-				ast_verbose("Discarded description format %s for ID %d\n", mimeSubtype, codec);
+		} else if (debug) {
+			ast_verbose("Discarded description format %s for ID %d\n", mimeSubtype, codec);
 		}
 	}
 
@@ -7673,7 +7665,7 @@
 	int x;
 
 	if ((sscanf(a, "T38FaxMaxBuffer:%30d", &x) == 1)) {
-		ast_debug(3, "MaxBufferSize:%d\n", x);
+		ast_debug(3, "MaxBufferSize: %d\n", x);
 		found = TRUE;
 	} else if ((sscanf(a, "T38MaxBitRate:%30d", &x) == 1) || (sscanf(a, "T38FaxMaxRate:%30d", &x) == 1)) {
 		ast_debug(3, "T38MaxBitRate: %d\n", x);




More information about the asterisk-commits mailing list