[svn-commits] russell: trunk r198146 - /trunk/res/res_timing_pthread.c
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Fri May 29 15:07:06 CDT 2009
Author: russell
Date: Fri May 29 15:06:59 2009
New Revision: 198146
URL: http://svn.asterisk.org/svn-view/asterisk?view=rev&rev=198146
Log:
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:
trunk/res/res_timing_pthread.c
Modified: trunk/res/res_timing_pthread.c
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/res/res_timing_pthread.c?view=diff&rev=198146&r1=198145&r2=198146
==============================================================================
--- trunk/res/res_timing_pthread.c (original)
+++ trunk/res/res_timing_pthread.c Fri May 29 15:06:59 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 svn-commits
mailing list