[asterisk-commits] rizzo: trunk r74813 - /trunk/main/rtp.c

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Thu Jul 12 10:42:57 CDT 2007


Author: rizzo
Date: Thu Jul 12 10:42:56 2007
New Revision: 74813

URL: http://svn.digium.com/view/asterisk?view=rev&rev=74813
Log:
a little bit of code cleanup to rtp.c, mostly to function 
ast_rtp_new_with_bindaddr(): 

1. add comments to the logic of the main loop;
2. use a common exit point on failure so the cleanup is done only in one place;
3. handle failures in rtp_socket() in the main loop of the function;

No functional changes except for #3 above, so it is not yet
worthwhile merging this and other changes to 1.4

Once the cleanup work on this file will be complete (which among
other things should include some extensions to the stun support)
it might be a good thing to push all the changes to 1.4


Modified:
    trunk/main/rtp.c

Modified: trunk/main/rtp.c
URL: http://svn.digium.com/view/asterisk/trunk/main/rtp.c?view=diff&rev=74813&r1=74812&r2=74813
==============================================================================
--- trunk/main/rtp.c (original)
+++ trunk/main/rtp.c Thu Jul 12 10:42:56 2007
@@ -2003,14 +2003,18 @@
 	return buf;
 }
 
-/*! \brief Open RTP or RTCP socket for a session */
-static int rtp_socket(void)
-{
-	int s;
-	long flags;
-	s = socket(AF_INET, SOCK_DGRAM, 0);
-	if (s > -1) {
-		flags = fcntl(s, F_GETFL);
+/*! \brief Open RTP or RTCP socket for a session.
+ * Print a message on failure. 
+ */
+static int rtp_socket(const char *type)
+{
+	int s = socket(AF_INET, SOCK_DGRAM, 0);
+	if (s < 0) {
+		if (type == NULL)
+			type = "RTP/RTCP";
+		ast_log(LOG_WARNING, "Unable to allocate %s socket: %s\n", type, strerror(errno));
+	} else {
+		long flags = fcntl(s, F_GETFL);
 		fcntl(s, F_SETFL, flags | O_NONBLOCK);
 #ifdef SO_NO_CHECK
 		if (nochecksums)
@@ -2031,13 +2035,12 @@
 
 	if (!(rtcp = ast_calloc(1, sizeof(*rtcp))))
 		return NULL;
-	rtcp->s = rtp_socket();
+	rtcp->s = rtp_socket("RTCP");
 	rtcp->us.sin_family = AF_INET;
 	rtcp->them.sin_family = AF_INET;
 
 	if (rtcp->s < 0) {
 		ast_free(rtcp);
-		ast_log(LOG_WARNING, "Unable to allocate RTCP socket: %s\n", strerror(errno));
 		return NULL;
 	}
 
@@ -2067,7 +2070,6 @@
 {
 	struct ast_rtp *rtp;
 	int x;
-	int first;
 	int startplace;
 	
 	if (!(rtp = ast_calloc(1, sizeof(*rtp))))
@@ -2075,70 +2077,68 @@
 
 	ast_rtp_new_init(rtp);
 
-	rtp->s = rtp_socket();
-	if (rtp->s < 0) {
-		ast_free(rtp);
-		ast_log(LOG_ERROR, "Unable to allocate socket: %s\n", strerror(errno));
-		return NULL;
-	}
+	rtp->s = rtp_socket("RTP");
+	if (rtp->s < 0)
+		goto fail;
 	if (sched && rtcpenable) {
 		rtp->sched = sched;
 		rtp->rtcp = ast_rtcp_new();
 	}
 	
-	/* Select a random port number in the range of possible RTP */
+	/*
+	 * Try to bind the RTP port, x, and possibly the RTCP port, x+1 as well.
+	 * Start from a random (even, by RTP spec) port number, and
+	 * iterate until success or no ports are available.
+	 * Note that the requirement of RTP port being even, or RTCP being the
+	 * next one, cannot be enforced in presence of a NAT box because the
+	 * mapping is not under our control.
+	 */
 	x = (ast_random() % (rtpend-rtpstart)) + rtpstart;
-	x = x & ~1;
-	/* Save it for future references. */
-	startplace = x;
-	/* Iterate tring to bind that port and incrementing it otherwise untill a port was found or no ports are available. */
+	x = x & ~1;		/* make it an even number */
+	startplace = x;		/* remember the starting point */
+	/* this is constant across the loop */
+	rtp->us.sin_addr = addr;
+	if (rtp->rtcp)
+		rtp->rtcp->us.sin_addr = addr;
 	for (;;) {
-		/* Must be an even port number by RTP spec */
 		rtp->us.sin_port = htons(x);
-		rtp->us.sin_addr = addr;
-
-		/* If there's rtcp, initialize it as well. */
-		if (rtp->rtcp) {
+		if (!bind(rtp->s, (struct sockaddr *)&rtp->us, sizeof(rtp->us))) {
+			/* bind succeeded, if no rtcp then we are done */
+			if (!rtp->rtcp)
+				break;
+			/* have rtcp, try to bind it */
 			rtp->rtcp->us.sin_port = htons(x + 1);
-			rtp->rtcp->us.sin_addr = addr;
-		}
-		/* Try to bind it/them. */
-		if (!(first = bind(rtp->s, (struct sockaddr *)&rtp->us, sizeof(rtp->us))) &&
-			(!rtp->rtcp || !bind(rtp->rtcp->s, (struct sockaddr *)&rtp->rtcp->us, sizeof(rtp->rtcp->us))))
-			break;
-		if (!first) {
-			/* Primary bind succeeded! Gotta recreate it */
+			if (!bind(rtp->rtcp->s, (struct sockaddr *)&rtp->rtcp->us, sizeof(rtp->rtcp->us)))
+				break;	/* success again, we are really done */
+			/*
+			 * RTCP bind failed, so close and recreate the
+			 * already bound RTP socket for the next round.
+			 */
 			close(rtp->s);
-			rtp->s = rtp_socket();
-		}
+			rtp->s = rtp_socket("RTP");
+			if (rtp->s < 0)
+				goto fail;
+		}
+		/*
+		 * If we get here, there was an error in one of the bind()
+		 * calls, so make sure it is nothing unexpected.
+		 */
 		if (errno != EADDRINUSE) {
 			/* We got an error that wasn't expected, abort! */
 			ast_log(LOG_ERROR, "Unexpected bind error: %s\n", strerror(errno));
-			close(rtp->s);
-			if (rtp->rtcp) {
-				close(rtp->rtcp->s);
-				ast_free(rtp->rtcp);
-			}
-			ast_free(rtp);
-			return NULL;
-		}
-		/* The port was used, increment it (by two). */
+			goto fail;
+		}
+		/*
+		 * One of the ports is in use. For the next iteration,
+		 * increment by two and handle wraparound.
+		 * If we reach the starting point, then declare failure.
+		 */
 		x += 2;
-		/* Did we go over the limit ? */
 		if (x > rtpend)
-			/* then, start from the begingig. */
 			x = (rtpstart + 1) & ~1;
-		/* Check if we reached the place were we started. */
 		if (x == startplace) {
-			/* If so, there's no ports available. */
 			ast_log(LOG_ERROR, "No RTP ports remaining. Can't setup media stream for this call.\n");
-			close(rtp->s);
-			if (rtp->rtcp) {
-				close(rtp->rtcp->s);
-				ast_free(rtp->rtcp);
-			}
-			ast_free(rtp);
-			return NULL;
+			goto fail;
 		}
 	}
 	rtp->sched = sched;
@@ -2149,6 +2149,16 @@
 	}
 	ast_rtp_pt_default(rtp);
 	return rtp;
+
+fail:
+	if (rtp->s >= 0)
+		close(rtp->s);
+	if (rtp->rtcp) {
+		close(rtp->rtcp->s);
+		ast_free(rtp->rtcp);
+	}
+	ast_free(rtp);
+	return NULL;
 }
 
 struct ast_rtp *ast_rtp_new(struct sched_context *sched, struct io_context *io, int rtcpenable, int callbackmode)




More information about the asterisk-commits mailing list