[asterisk-commits] mnicholson: branch group/newcdr r198181 - in /team/group/newcdr: ./ main/ res/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri May 29 16:34:14 CDT 2009


Author: mnicholson
Date: Fri May 29 16:34:10 2009
New Revision: 198181

URL: http://svn.asterisk.org/svn-view/asterisk?view=rev&rev=198181
Log:
Merged revisions 198139,198146 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/trunk

........
  r198139 | eliel | 2009-05-29 14:46:07 -0500 (Fri, 29 May 2009) | 15 lines
  
  Simplify the Makefile and avoid needing to specify each object file.
  
  Instead of specifying every object file, use make's magic to generate
  it.
  This will generate less conflicts in team branches when a new file is
  added in trunk.
  
  (closes issue #15226)
  Reported by: eliel
  Patches:
        makefile uploaded by eliel (license 64)
  
        Review: http://reviewboard.asterisk.org/r/269/ 
........
  r198146 | russell | 2009-05-29 15:06:59 -0500 (Fri, 29 May 2009) | 38 lines
  
  Resolve issues with choppy sound when using res_timing_pthread.
  
  The situation that caused this problem was when continuous mode was being
  turned on and off while a rate was set for a timing interface.  A very easy
  way to replicate this bug was to do a Playback() from behind a Local channel.
  In this scenario, a rate gets set on the channel for doing file playback.
  At the same time, continuous mode gets turned on and off about every 20 ms
  as frames get queued on to the PBX side channel from the other side of the
  Local channel.
  
  Essentially, this module treated continuous mode and a set rate as mutually
  exclusive states for the timer to be in.  When I dug deep enough, I observed
  the following pattern:
  
     1) Set timer to tick every 20 ms.
     2) Wait almost 20 ms ...
     3) Continuous mode gets turned on for a queued up frame
     4) Continuous mode gets turned off
     5) The timer goes back to its tick per 20 ms. state but starts counting
        at 0 ms.
     6) Goto step 2.
  
  Sometimes, res_timing_pthread would make it 20 ms and produce a timer tick,
  but not most of the time.  This is what produced the choppy sound (or sometimes
  no sound at all).
  
  Now, the module treats continuous mode and a set rate as completely independent
  timer modes.  They can be enabled and disabled independently of each other and
  things work as expected.
  
  
  (closes issue #14412)
  Reported by: dome
  Patches:
        issue14412.diff.txt uploaded by russell (license 2)
        issue14412-1.6.1.0.diff.txt uploaded by russell (license 2)
  Tested by: DennisD, russell
........

Modified:
    team/group/newcdr/   (props changed)
    team/group/newcdr/main/Makefile
    team/group/newcdr/res/res_timing_pthread.c

Propchange: team/group/newcdr/
------------------------------------------------------------------------------
    automerge = on

Propchange: team/group/newcdr/
------------------------------------------------------------------------------
--- svnmerge-integrated (original)
+++ svnmerge-integrated Fri May 29 16:34:10 2009
@@ -1,1 +1,1 @@
-/trunk:1-198104
+/trunk:1-198180

Modified: team/group/newcdr/main/Makefile
URL: http://svn.asterisk.org/svn-view/asterisk/team/group/newcdr/main/Makefile?view=diff&rev=198181&r1=198180&r2=198181
==============================================================================
--- team/group/newcdr/main/Makefile (original)
+++ team/group/newcdr/main/Makefile Fri May 29 16:34:10 2009
@@ -17,19 +17,9 @@
 
 include $(ASTTOPDIR)/Makefile.moddir_rules
 
-OBJS= tcptls.o io.o sched.o logger.o frame.o loader.o config.o channel.o \
-	translate.o file.o pbx.o cli.o md5.o term.o heap.o \
-	ulaw.o alaw.o callerid.o fskmodem.o image.o app.o \
-	cdr.o cel.o tdd.o acl.o udptl.o manager.o asterisk.o \
-	dsp.o chanvars.o indications.o autoservice.o db.o privacy.o \
-	astmm.o astfd.o enum.o srv.o dns.o aescrypt.o aestab.o aeskey.o \
-	utils.o plc.o jitterbuf.o dnsmgr.o devicestate.o \
-	netsock.o slinfactory.o ast_expr2.o ast_expr2f.o \
-	cryptostub.o sha1.o http.o fixedjitterbuf.o abstract_jb.o \
-	strcompat.o threadstorage.o dial.o event.o adsistub.o audiohook.o \
-	astobj2.o hashtab.o global_datastores.o version.o \
-	features.o taskprocessor.o timing.o datastore.o xml.o xmldoc.o \
-	strings.o bridging.o poll.o rtp_engine.o stun.o autochan.o
+SRC=$(wildcard *.c)
+OBJSFILTER=fskmodem_int.o fskmodem_float.o cygload.o buildinfo.o
+OBJS=$(filter-out $(OBJSFILTER),$(SRC:.c=.o))
 
 # we need to link in the objects statically, not as a library, because
 # otherwise modules will not have them available if none of the static

Modified: team/group/newcdr/res/res_timing_pthread.c
URL: http://svn.asterisk.org/svn-view/asterisk/team/group/newcdr/res/res_timing_pthread.c?view=diff&rev=198181&r1=198180&r2=198181
==============================================================================
--- team/group/newcdr/res/res_timing_pthread.c (original)
+++ team/group/newcdr/res/res_timing_pthread.c Fri May 29 16:34:10 2009
@@ -75,7 +75,6 @@
 enum pthread_timer_state {
 	TIMER_STATE_IDLE,
 	TIMER_STATE_TICKING,
-	TIMER_STATE_CONTINUOUS,
 };
 
 struct pthread_timer {
@@ -85,13 +84,15 @@
 	/*! Interval in ms for current rate */
 	unsigned int interval;
 	unsigned int tick_count;
+	unsigned int pending_ticks;
 	struct timeval start;
+	unsigned int continuous:1;
 };
 
 static void pthread_timer_destructor(void *obj);
 static struct pthread_timer *find_timer(int handle, int unlinkobj);
-static void write_byte(int wr_fd);
-static void read_pipe(int rd_fd, unsigned int num, int clear);
+static void write_byte(struct pthread_timer *timer);
+static void read_pipe(struct pthread_timer *timer, unsigned int num);
 
 /*!
  * \brief Data for the timing thread
@@ -148,24 +149,60 @@
 	ao2_ref(timer, -1);
 }
 
-static void set_state(struct pthread_timer *timer)
-{
-	unsigned int rate = timer->rate;
-
-	if (rate) {
-		timer->state = TIMER_STATE_TICKING;
+static int pthread_timer_set_rate(int handle, unsigned int rate)
+{
+	struct pthread_timer *timer;
+
+	if (!(timer = find_timer(handle, 0))) {
+		errno = EINVAL;
+		return -1;
+	}
+
+	if (rate > MAX_RATE) {
+		ast_log(LOG_ERROR, "res_timing_pthread only supports timers at a "
+				"max rate of %d / sec\n", MAX_RATE);
+		errno = EINVAL;
+		return -1;
+	}
+
+	ao2_lock(timer);
+
+	if ((timer->rate = rate)) {
 		timer->interval = roundf(1000.0 / ((float) rate));
 		timer->start = ast_tvnow();
+		timer->state = TIMER_STATE_TICKING;
 	} else {
-		timer->state = TIMER_STATE_IDLE;
 		timer->interval = 0;
 		timer->start = ast_tv(0, 0);
-	}
-
+		timer->state = TIMER_STATE_IDLE;
+	}
 	timer->tick_count = 0;
-}
-
-static int pthread_timer_set_rate(int handle, unsigned int rate)
+
+	ao2_unlock(timer);
+
+	ao2_ref(timer, -1);
+
+	return 0;
+}
+
+static void pthread_timer_ack(int handle, unsigned int quantity)
+{
+	struct pthread_timer *timer;
+
+	ast_assert(quantity > 0);
+
+	if (!(timer = find_timer(handle, 0))) {
+		return;
+	}
+
+	ao2_lock(timer);
+	read_pipe(timer, quantity);
+	ao2_unlock(timer);
+
+	ao2_ref(timer, -1);
+}
+
+static int pthread_timer_enable_continuous(int handle)
 {
 	struct pthread_timer *timer;
 
@@ -174,19 +211,11 @@
 		return -1;
 	}
 
-	if (rate > MAX_RATE) {
-		ast_log(LOG_ERROR, "res_timing_pthread only supports timers at a max rate of %d / sec\n",
-			MAX_RATE);
-		errno = EINVAL;
-		return -1;
-	}
-
 	ao2_lock(timer);
-	timer->rate = rate;
-	if (timer->state != TIMER_STATE_CONTINUOUS) {
-		set_state(timer);
-	}
-
+	if (!timer->continuous) {
+		timer->continuous = 1;
+		write_byte(timer);
+	}
 	ao2_unlock(timer);
 
 	ao2_ref(timer, -1);
@@ -194,27 +223,7 @@
 	return 0;
 }
 
-static void pthread_timer_ack(int handle, unsigned int quantity)
-{
-	struct pthread_timer *timer;
-
-	ast_assert(quantity > 0);
-
-	if (!(timer = find_timer(handle, 0))) {
-		return;
-	}
-
-	if (timer->state == TIMER_STATE_CONTINUOUS) {
-		/* Leave the pipe alone, please! */
-		return;
-	}
-
-	read_pipe(timer->pipe[PIPE_READ], quantity, 0);
-
-	ao2_ref(timer, -1);
-}
-
-static int pthread_timer_enable_continuous(int handle)
+static int pthread_timer_disable_continuous(int handle)
 {
 	struct pthread_timer *timer;
 
@@ -224,27 +233,10 @@
 	}
 
 	ao2_lock(timer);
-	timer->state = TIMER_STATE_CONTINUOUS;
-	write_byte(timer->pipe[PIPE_WRITE]);
-	ao2_unlock(timer);
-
-	ao2_ref(timer, -1);
-
-	return 0;
-}
-
-static int pthread_timer_disable_continuous(int handle)
-{
-	struct pthread_timer *timer;
-
-	if (!(timer = find_timer(handle, 0))) {
-		errno = EINVAL;
-		return -1;
-	}
-
-	ao2_lock(timer);
-	set_state(timer);
-	read_pipe(timer->pipe[PIPE_READ], 0, 1);
+	if (timer->continuous) {
+		timer->continuous = 0;
+		read_pipe(timer, 1);
+	}
 	ao2_unlock(timer);
 
 	ao2_ref(timer, -1);
@@ -261,9 +253,11 @@
 		return res;
 	}
 
-	if (timer->state == TIMER_STATE_CONTINUOUS) {
+	ao2_lock(timer);
+	if (timer->continuous && timer->pending_ticks == 1) {
 		res = AST_TIMING_EVENT_CONTINUOUS;
 	}
+	ao2_unlock(timer);
 
 	ao2_ref(timer, -1);
 
@@ -338,7 +332,7 @@
 {
 	struct timeval now;
 
-	if (timer->state == TIMER_STATE_IDLE || timer->state == TIMER_STATE_CONTINUOUS) {
+	if (timer->state == TIMER_STATE_IDLE) {
 		return 0;
 	}
 
@@ -347,6 +341,7 @@
 	if (timer->tick_count < (ast_tvdiff_ms(now, timer->start) / timer->interval)) {
 		timer->tick_count++;
 		if (!timer->tick_count) {
+			/* Handle overflow. */
 			timer->start = now;
 		}
 		return 1;
@@ -355,13 +350,16 @@
 	return 0;
 }
 
-static void read_pipe(int rd_fd, unsigned int quantity, int clear)
-{
-	ast_assert(quantity || clear);
-
-	if (!quantity && clear) {
-		quantity = 1;
-	}
+/*!
+ * \internal
+ * \pre timer is locked
+ */
+static void read_pipe(struct pthread_timer *timer, unsigned int quantity)
+{
+	int rd_fd = timer->pipe[PIPE_READ];
+
+	ast_assert(quantity);
+	ast_assert(quantity >= timer->pending_ticks);
 
 	do {
 		unsigned char buf[1024];
@@ -376,6 +374,8 @@
 		FD_SET(rd_fd, &rfds);
 
 		if (select(rd_fd + 1, &rfds, NULL, NULL, &timeout) != 1) {
+			ast_debug(1, "Reading not available on timing pipe, "
+					"quantity: %u\n", quantity);
 			break;
 		}
 
@@ -386,33 +386,35 @@
 			if (errno == EAGAIN) {
 				continue;
 			}
-			ast_log(LOG_ERROR, "read failed on timing pipe: %s\n", strerror(errno));
+			ast_log(LOG_ERROR, "read failed on timing pipe: %s\n",
+					strerror(errno));
 			break;
 		}
 
-		if (clear) {
-			continue;
-		}
-
 		quantity -= res;
+		timer->pending_ticks -= res;
 	} while (quantity);
 }
 
-static void write_byte(int wr_fd)
-{
+/*!
+ * \internal
+ * \pre timer is locked
+ */
+static void write_byte(struct pthread_timer *timer)
+{
+	ssize_t res;
+	unsigned char x = 42;
+
 	do {
-		ssize_t res;
-		unsigned char x = 42;
-
-		res = write(wr_fd, &x, 1);
-
-		if (res == -1) {
-			if (errno == EAGAIN) {
-				continue;
-			}
-			ast_log(LOG_ERROR, "Error writing to timing pipe: %s\n", strerror(errno));
-		}
-	} while (0);
+		res = write(timer->pipe[PIPE_WRITE], &x, 1);
+	} while (res == -1 && errno == EAGAIN);
+
+	if (res == -1) {
+		ast_log(LOG_ERROR, "Error writing to timing pipe: %s\n",
+				strerror(errno));
+	} else {
+		timer->pending_ticks++;
+	}
 }
 
 static int run_timer(void *obj, void *arg, int flags)
@@ -424,11 +426,9 @@
 	}
 
 	ao2_lock(timer);
-
 	if (check_timer(timer)) {
-		write_byte(timer->pipe[PIPE_WRITE]);
-	}
-
+		write_byte(timer);
+	}
 	ao2_unlock(timer);
 
 	return 0;




More information about the asterisk-commits mailing list