[svn-commits] russell: branch russell/iax2_media r116977 - in /team/russell/iax2_media: cha...

SVN commits to the Digium repositories svn-commits at lists.digium.com
Sun May 18 22:40:32 CDT 2008


Author: russell
Date: Sun May 18 22:40:31 2008
New Revision: 116977

URL: http://svn.digium.com/view/asterisk?view=rev&rev=116977
Log:
Check in a work in progress.  While half awake one night I got to thinking
about media handling in chan_iax2.  The way frames get sent up to the core
from the IAX2 processing threads is by queueing frames onto the ast_channel.
However, it's very expensive as it requires malloc'ing a new frame and
expensive locking with deadlock avoidance (accessing an ast_channel from a
channel driver is usually pretty ugly, as Asterisk locking rules require
that you get the ast_channel lock _before_ the channel's pvt lock, and coming
from the channel driver, you usually have the pvt before the channel.)

So, anyway, I started playing around with ways to pass audio to the core by
queueing the audio on to the pvt struct, and allowing the core to use the
iax2_read callback to get it.  So far, I have avoided the nasty deadlock
avoidance that had to be done when queueing frames, and it appears to have
some performance improvements already.  However, I still have more I want to
do to reduce how many times frames are malloc'd and copied around within
chan_iax2 ...

Modified:
    team/russell/iax2_media/channels/chan_iax2.c
    team/russell/iax2_media/channels/iax2-parser.c
    team/russell/iax2_media/channels/iax2-parser.h
    team/russell/iax2_media/formats/format_pcm.c
    team/russell/iax2_media/include/asterisk/channel.h
    team/russell/iax2_media/include/asterisk/linkedlists.h
    team/russell/iax2_media/main/channel.c
    team/russell/iax2_media/main/sched.c
    team/russell/iax2_media/main/slinfactory.c

Modified: team/russell/iax2_media/channels/chan_iax2.c
URL: http://svn.digium.com/view/asterisk/team/russell/iax2_media/channels/chan_iax2.c?view=diff&rev=116977&r1=116976&r2=116977
==============================================================================
--- team/russell/iax2_media/channels/chan_iax2.c (original)
+++ team/russell/iax2_media/channels/chan_iax2.c Sun May 18 22:40:31 2008
@@ -464,8 +464,6 @@
 	int dropped;
 	int ooo;
 };
-
-struct iax2_pvt_ref;
 
 struct chan_iax2_pvt {
 	/*! Socket to send/receive on for this call */
@@ -642,6 +640,9 @@
 	int frames_dropped;
 	/*! received frame count: (just for stats) */
 	int frames_received;
+
+	AST_LIST_HEAD_NOLOCK(, iax_frame) audio_q;
+	AST_LIST_HEAD_NOLOCK(, iax_frame) used_audio_q;
 };
 
 /*!
@@ -1381,6 +1382,7 @@
 {
 	struct chan_iax2_pvt *pvt = obj;
 	struct iax_frame *cur = NULL;
+	jb_frame frame;
 
 	iax2_destroy_helper(pvt);
 
@@ -1400,19 +1402,23 @@
 		pvt->reg->callno = 0;
 	}
 
-	if (!pvt->owner) {
-		jb_frame frame;
-		if (pvt->vars) {
-		    ast_variables_destroy(pvt->vars);
-		    pvt->vars = NULL;
-		}
-
-		while (jb_getall(pvt->jb, &frame) == JB_OK) {
-			iax2_frame_free(frame.data);
-		}
-
-		jb_destroy(pvt->jb);
-		ast_string_field_free_memory(pvt);
+	if (pvt->vars) {
+	    ast_variables_destroy(pvt->vars);
+	    pvt->vars = NULL;
+	}
+
+	while (jb_getall(pvt->jb, &frame) == JB_OK) {
+		iax2_frame_free(frame.data);
+	}
+
+	jb_destroy(pvt->jb);
+	ast_string_field_free_memory(pvt);
+	
+	while ((cur = AST_LIST_REMOVE_HEAD(&pvt->used_audio_q, pvt_q_entry))) {
+		iax2_frame_free(cur);
+	}
+	while ((cur = AST_LIST_REMOVE_HEAD(&pvt->audio_q, pvt_q_entry))) {
+		iax2_frame_free(cur);
 	}
 }
 
@@ -1430,7 +1436,7 @@
 		tmp = NULL;
 		return NULL;
 	}
-		
+
 	tmp->prefs = prefs;
 	tmp->pingid = -1;
 	tmp->lagid = -1;
@@ -1450,6 +1456,8 @@
 	jb_setconf(tmp->jb,&jbconf);
 
 	AST_LIST_HEAD_INIT_NOLOCK(&tmp->dpentries);
+	AST_LIST_HEAD_INIT_NOLOCK(&tmp->audio_q);
+	AST_LIST_HEAD_INIT_NOLOCK(&tmp->used_audio_q);
 
 	return tmp;
 }
@@ -2078,25 +2086,42 @@
 }
 
 /*!
+ * \note called with call_num locked
+ */
+static void iax2_queue_audio(uint16_t call_num, struct iax_frame *fr)
+{
+	struct chan_iax2_pvt *pvt = iaxs[call_num];
+
+	ast_assert(pvt->owner != NULL);
+
+	AST_LIST_INSERT_TAIL(&pvt->audio_q, fr, pvt_q_entry);
+
+	while (AST_LIST_FIRST(&pvt->used_audio_q) && 
+		AST_LIST_NEXT(AST_LIST_FIRST(&pvt->used_audio_q), pvt_q_entry) && 
+		(fr = AST_LIST_REMOVE_HEAD(&pvt->used_audio_q, pvt_q_entry))) {
+		iax2_frame_free(fr);
+	}
+
+	ast_channel_alert(pvt->owner);
+}
+
+/*!
+ * Just deliver the packet by using queueing.
+ *
  * \note This function assumes that iaxsl[callno] is locked when called.
- *
- * \note IMPORTANT NOTE!!! Any time this function is used, even if iaxs[callno]
- * was valid before calling it, it may no longer be valid after calling it.
- * This function calls iax2_queue_frame(), which may unlock and lock the mutex 
- * associated with this callno, meaning that another thread may grab it and destroy the call.
  */
-static int __do_deliver(void *data)
-{
-	/* Just deliver the packet by using queueing.  This is called by
-	  the IAX thread with the iaxsl lock held. */
-	struct iax_frame *fr = data;
+static int deliver_audio_frame(struct iax_frame *fr)
+{
 	fr->retrans = -1;
+
 	ast_clear_flag(&fr->af, AST_FRFLAG_HAS_TIMING_INFO);
-	if (iaxs[fr->callno] && !ast_test_flag(iaxs[fr->callno], IAX_ALREADYGONE))
-		iax2_queue_frame(fr->callno, &fr->af);
-	/* Free our iax frame */
-	iax2_frame_free(fr);
-	/* And don't run again */
+
+	if (iaxs[fr->callno] && !ast_test_flag(iaxs[fr->callno], IAX_ALREADYGONE)) {
+		iax2_queue_audio(fr->callno, fr);
+	} else {
+		iax2_frame_free(fr);
+	}
+
 	return 0;
 }
 
@@ -2852,29 +2877,28 @@
 		switch(ret) {
 		case JB_OK:
 			fr = frame.data;
-			__do_deliver(fr);
-			/* __do_deliver() can cause the call to disappear */
-			pvt = iaxs[callno];
+			deliver_audio_frame(fr);
+			fr = NULL;
 			break;
 		case JB_INTERP:
 		{
-			struct ast_frame af = { 0, };
-			
+			struct iax_frame *iaxfr;
+		
+			if (!(iaxfr = iax_frame_new(DIRECTION_INGRESS, 1024, 1))) {
+				break;
+			}
+
 			/* create an interpolation frame */
-			af.frametype = AST_FRAME_VOICE;
-			af.subclass = pvt->voiceformat;
-			af.samples  = frame.ms * 8;
-			af.src  = "IAX2 JB interpolation";
-			af.delivery = ast_tvadd(pvt->rxcore, ast_samp2tv(next, 1000));
-			af.offset = AST_FRIENDLY_OFFSET;
-			
-			/* queue the frame:  For consistency, we would call __do_deliver here, but __do_deliver wants an iax_frame,
-			 * which we'd need to malloc, and then it would free it.  That seems like a drag */
-			if (!ast_test_flag(iaxs[callno], IAX_ALREADYGONE)) {
-				iax2_queue_frame(callno, &af);
-				/* iax2_queue_frame() could cause the call to disappear */
-				pvt = iaxs[callno];
-			}
+			/* XXX Umm ... setting number of samples but with no audio data? O.o */
+			iaxfr->af.frametype = AST_FRAME_VOICE;
+			iaxfr->af.subclass = pvt->voiceformat;
+			iaxfr->af.samples  = frame.ms * 8;
+			iaxfr->af.src  = "IAX2 JB interpolation";
+			iaxfr->af.delivery = ast_tvadd(pvt->rxcore, ast_samp2tv(next, 1000));
+			iaxfr->af.offset = AST_FRIENDLY_OFFSET;
+		
+			deliver_audio_frame(iaxfr);
+			iaxfr = NULL;
 		}
 			break;
 		case JB_DROP:
@@ -2943,7 +2967,8 @@
 	if ( (!ast_test_flag(iaxs[fr->callno], IAX_USEJITTERBUF)) ) {
 		if (tsout)
 			*tsout = fr->ts;
-		__do_deliver(fr);
+		deliver_audio_frame(fr);
+		fr = NULL;
 		return -1;
 	}
 
@@ -2957,10 +2982,7 @@
 
 		/* deliver any frames in the jb */
 		while (jb_getall(iaxs[fr->callno]->jb, &frame) == JB_OK) {
-			__do_deliver(frame.data);
-			/* __do_deliver() can make the call disappear */
-			if (!iaxs[fr->callno])
-				return -1;
+			deliver_audio_frame(frame.data);
 		}
 
 		jb_reset(iaxs[fr->callno]->jb);
@@ -2970,7 +2992,8 @@
 		/* deliver this frame now */
 		if (tsout)
 			*tsout = fr->ts;
-		__do_deliver(fr);
+		deliver_audio_frame(fr);
+		fr = NULL;
 		return -1;
 	}
 
@@ -3723,8 +3746,33 @@
 
 static struct ast_frame *iax2_read(struct ast_channel *c) 
 {
-	ast_log(LOG_NOTICE, "I should never be called!\n");
-	return &ast_null_frame;
+	struct ast_frame *fr;
+	unsigned short callno = PTR_TO_CALLNO(c->tech_pvt);
+	struct iax_frame *iax_fr;
+	struct chan_iax2_pvt *pvt;
+
+	ast_mutex_lock(&iaxsl[callno]);
+
+	pvt = iaxs[callno];
+
+	ast_assert(pvt != NULL);
+	if (!pvt) {
+		ast_mutex_unlock(&iaxsl[callno]);
+		return &ast_null_frame;
+	}
+
+	if (!(iax_fr = AST_LIST_REMOVE_HEAD(&pvt->audio_q, pvt_q_entry))) {
+		ast_mutex_unlock(&iaxsl[callno]);
+		return &ast_null_frame;
+	}
+
+	fr = &iax_fr->af;
+
+	AST_LIST_INSERT_TAIL(&pvt->used_audio_q, iax_fr, pvt_q_entry);
+
+	ast_mutex_unlock(&iaxsl[callno]);
+
+	return fr;
 }
 
 static int iax2_start_transfer(unsigned short callno0, unsigned short callno1, int mediaonly)
@@ -7822,7 +7870,7 @@
 					struct iax_frame *duped_fr;
 
 					/* Common things */
-					f.src = "IAX2";
+					f.src = "IAX2-1";
 					f.mallocd = 0;
 					f.offset = 0;
 					if (f.datalen && (f.frametype == AST_FRAME_VOICE)) 
@@ -7955,7 +8003,7 @@
 
 	/* allocate an iax_frame with 4096 bytes of data buffer */
 	fr = alloca(sizeof(*fr) + 4096);
-	fr->callno = 0;
+	memset(fr, 0, sizeof(*fr));
 	fr->afdatalen = 4096; /* From alloca() above */
 
 	/* Copy frequently used parameters to the stack */
@@ -9465,7 +9513,7 @@
 		return 1;
 	}
 	/* Common things */
-	f.src = "IAX2";
+	f.src = "IAX2-2";
 	f.mallocd = 0;
 	f.offset = 0;
 	f.len = 0;

Modified: team/russell/iax2_media/channels/iax2-parser.c
URL: http://svn.digium.com/view/asterisk/team/russell/iax2_media/channels/iax2-parser.c?view=diff&rev=116977&r1=116976&r2=116977
==============================================================================
--- team/russell/iax2_media/channels/iax2-parser.c (original)
+++ team/russell/iax2_media/channels/iax2-parser.c Sun May 18 22:40:31 2008
@@ -968,6 +968,7 @@
 	fr->af.delivery.tv_usec = 0;
 	fr->af.data = fr->afdata;
 	fr->af.len = f->len;
+	fr->af.flags = 0;
 	if (fr->af.datalen) {
 		size_t copy_len = fr->af.datalen;
 		if (copy_len > fr->afdatalen) {
@@ -999,7 +1000,7 @@
 			if (fr->afdatalen >= datalen) {
 				size_t afdatalen = fr->afdatalen;
 				AST_LIST_REMOVE_CURRENT(list);
-				memset(fr, 0, sizeof(*fr));
+				memset(fr, 0, sizeof(*fr) + afdatalen);
 				fr->afdatalen = afdatalen;
 				break;
 			}

Modified: team/russell/iax2_media/channels/iax2-parser.h
URL: http://svn.digium.com/view/asterisk/team/russell/iax2_media/channels/iax2-parser.h?view=diff&rev=116977&r1=116976&r2=116977
==============================================================================
--- team/russell/iax2_media/channels/iax2-parser.h (original)
+++ team/russell/iax2_media/channels/iax2-parser.h Sun May 18 22:40:31 2008
@@ -125,6 +125,7 @@
 	int retrans;
 	/* Easy linking */
 	AST_LIST_ENTRY(iax_frame) list;
+	AST_LIST_ENTRY(iax_frame) pvt_q_entry;
 	/* Actual, isolated frame header */
 	struct ast_frame af;
 	/*! Amount of space _allocated_ for data */

Modified: team/russell/iax2_media/formats/format_pcm.c
URL: http://svn.digium.com/view/asterisk/team/russell/iax2_media/formats/format_pcm.c?view=diff&rev=116977&r1=116976&r2=116977
==============================================================================
--- team/russell/iax2_media/formats/format_pcm.c (original)
+++ team/russell/iax2_media/formats/format_pcm.c Sun May 18 22:40:31 2008
@@ -82,6 +82,7 @@
 	s->fr.frametype = AST_FRAME_VOICE;
 	s->fr.subclass = s->fmt->format;
 	s->fr.mallocd = 0;
+	s->fr.flags = 0;
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, BUF_SIZE);
 	if ((res = fread(s->fr.data, 1, s->fr.datalen, s->f)) < 1) {
 		if (res)

Modified: team/russell/iax2_media/include/asterisk/channel.h
URL: http://svn.digium.com/view/asterisk/team/russell/iax2_media/include/asterisk/channel.h?view=diff&rev=116977&r1=116976&r2=116977
==============================================================================
--- team/russell/iax2_media/include/asterisk/channel.h (original)
+++ team/russell/iax2_media/include/asterisk/channel.h Sun May 18 22:40:31 2008
@@ -1636,6 +1636,7 @@
         AST_LIST_ENTRY(ast_group_info) list;   
 };
 
+int ast_channel_alert(struct ast_channel *chan);
 
 #if defined(__cplusplus) || defined(c_plusplus)
 }

Modified: team/russell/iax2_media/include/asterisk/linkedlists.h
URL: http://svn.digium.com/view/asterisk/team/russell/iax2_media/include/asterisk/linkedlists.h?view=diff&rev=116977&r1=116976&r2=116977
==============================================================================
--- team/russell/iax2_media/include/asterisk/linkedlists.h (original)
+++ team/russell/iax2_media/include/asterisk/linkedlists.h Sun May 18 22:40:31 2008
@@ -762,8 +762,9 @@
 		if (cur) {						\
 			(head)->first = cur->field.next;		\
 			cur->field.next = NULL;				\
-			if ((head)->last == cur)			\
+			if ((head)->last == cur) {			\
 				(head)->last = NULL;			\
+			} \
 		}							\
 		cur;							\
 	})

Modified: team/russell/iax2_media/main/channel.c
URL: http://svn.digium.com/view/asterisk/team/russell/iax2_media/main/channel.c?view=diff&rev=116977&r1=116976&r2=116977
==============================================================================
--- team/russell/iax2_media/main/channel.c (original)
+++ team/russell/iax2_media/main/channel.c Sun May 18 22:40:31 2008
@@ -956,7 +956,6 @@
 {
 	struct ast_frame *f;
 	struct ast_frame *cur;
-	int blah = 1;
 	int qlen = 0;
 
 	/* Build us a copy and free the original one */
@@ -991,17 +990,9 @@
 		}
 	}
 	AST_LIST_INSERT_TAIL(&chan->readq, f, frame_list);
-	if (chan->alertpipe[1] > -1) {
-		if (write(chan->alertpipe[1], &blah, sizeof(blah)) != sizeof(blah))
-			ast_log(LOG_WARNING, "Unable to write to alert pipe on %s, frametype/subclass %d/%d (qlen = %d): %s!\n",
-				chan->name, f->frametype, f->subclass, qlen, strerror(errno));
-#ifdef HAVE_ZAPTEL
-	} else if (chan->timingfd > -1) {
-		ioctl(chan->timingfd, ZT_TIMERPING, &blah);
-#endif				
-	} else if (ast_test_flag(chan, AST_FLAG_BLOCKING)) {
-		pthread_kill(chan->blocker, SIGURG);
-	}
+
+	ast_channel_alert(chan);
+
 	ast_channel_unlock(chan);
 	return 0;
 }
@@ -2456,6 +2447,7 @@
 		   the channel driver and f would be only a single frame)
 		*/
 		if (AST_LIST_NEXT(f, frame_list)) {
+			ast_log(LOG_ERROR, "Got a frame with a next set?\n");
 			AST_LIST_HEAD_SET_NOLOCK(&chan->readq, AST_LIST_NEXT(f, frame_list));
 			AST_LIST_NEXT(f, frame_list) = NULL;
 		}
@@ -5115,3 +5107,27 @@
 
 	return ast_say_digit_str_full(chan, buf, ints, lang, audiofd, ctrlfd);
 }
+
+int ast_channel_alert(struct ast_channel *chan)
+{
+	int res = 0;
+	int blah = 0;
+
+	if (chan->alertpipe[1] > -1) {
+		if (write(chan->alertpipe[1], &blah, sizeof(blah)) != sizeof(blah)) {
+			ast_channel_lock(chan);
+			ast_log(LOG_WARNING, "Unable to write to alert pipe on %s: %s!\n",
+				chan->name, strerror(errno));
+			ast_channel_unlock(chan);
+			res = -1;
+		}
+#ifdef HAVE_ZAPTEL
+	} else if (chan->timingfd > -1) {
+		ioctl(chan->timingfd, ZT_TIMERPING, &blah);
+#endif				
+	} else if (ast_test_flag(chan, AST_FLAG_BLOCKING)) {
+		pthread_kill(chan->blocker, SIGURG);
+	}
+
+	return res;
+}

Modified: team/russell/iax2_media/main/sched.c
URL: http://svn.digium.com/view/asterisk/team/russell/iax2_media/main/sched.c?view=diff&rev=116977&r1=116976&r2=116977
==============================================================================
--- team/russell/iax2_media/main/sched.c (original)
+++ team/russell/iax2_media/main/sched.c Sun May 18 22:40:31 2008
@@ -373,7 +373,6 @@
 
 	if (!s) {
 		ast_debug(1, "Attempted to delete nonexistent schedule entry %d!\n", id);
-		ast_assert(0);
 		return -1;
 	}
 	

Modified: team/russell/iax2_media/main/slinfactory.c
URL: http://svn.digium.com/view/asterisk/team/russell/iax2_media/main/slinfactory.c?view=diff&rev=116977&r1=116976&r2=116977
==============================================================================
--- team/russell/iax2_media/main/slinfactory.c (original)
+++ team/russell/iax2_media/main/slinfactory.c Sun May 18 22:40:31 2008
@@ -31,6 +31,7 @@
 #include "asterisk/frame.h"
 #include "asterisk/slinfactory.h"
 #include "asterisk/translate.h"
+#include "asterisk/utils.h"
 
 /*!
  * \brief Initialize an slinfactory




More information about the svn-commits mailing list