abstract/fixed/adpative jitter buffer: disallow frame re-inserts

It was possible for a frame to be re-inserted into a jitter buffer after it
had been removed from it. A case when this happened was if a frame was read
out of the jitterbuffer, passed to the translation core, and then multiple
frames were returned from said translation core. Upon multiple frames being
returned the first is passed on, but sebsequently "chained" frames are put
back into the read queue. Thus it was possible for a frame to go back into
the jitter buffer where this would cause problems.

This patch adds a flag to frames that are inserted into the channel's read
queue after translation. The abstract jitter buffer code then checks for this
flag and ignores any frames marked as such.

Change-Id: I276c44edc9dcff61e606242f71274265c7779587

diff --git a/include/asterisk/abstract_jb.h b/include/asterisk/abstract_jb.h
index 8a5e3d27ffdb30e2ac562fa47b54713f5b5288c8..173b22a5c5c9d79c313cc353c9e0c90030b48e57 100644 (file)
--- a/include/asterisk/abstract_jb.h
+++ b/include/asterisk/abstract_jb.h
@@ -109,6 +109,8 @@ typedef int (*jb_remove_impl)(void *jb, struct ast_frame **fout);
 typedef void (*jb_force_resynch_impl)(void *jb);
 /*! \brief Empty and reset jb */
 typedef void (*jb_empty_and_reset_impl)(void *jb);
+/*! \brief Check if late */
+typedef int (*jb_is_late_impl)(void *jb, long ts);
 
 
 /*!
@@ -127,6 +129,7 @@ struct ast_jb_impl
        jb_remove_impl remove;
        jb_force_resynch_impl force_resync;
        jb_empty_and_reset_impl empty_and_reset;
+       jb_is_late_impl is_late;
 };
 
 /*!

diff --git a/include/asterisk/frame.h b/include/asterisk/frame.h
index 20f40f8634ca12e77f0b5f28d1e49de93ea1639b..108dcafe0bb62f4d821f49b74848d5c308f67b74 100644 (file)
--- a/include/asterisk/frame.h
+++ b/include/asterisk/frame.h
@@ -133,6 +133,8 @@ enum ast_frame_type {
 enum {
        /*! This frame contains valid timing information */
        AST_FRFLAG_HAS_TIMING_INFO = (1 << 0),
+       /*! This frame has been requeued */
+       AST_FRFLAG_REQUEUED = (1 << 1),
 };
 
 struct ast_frame_subclass {

diff --git a/include/jitterbuf.h b/include/jitterbuf.h
index 6da11a65bd418e7a127a68d85da5008db9667a62..32579fc6c97cdd8e0f42311ab17674ad2c95e2e7 100644 (file)
--- a/include/jitterbuf.h
+++ b/include/jitterbuf.h
@@ -166,6 +166,9 @@ enum jb_return_code jb_setconf(jitterbuf *jb, jb_conf *conf);
 typedef void __attribute__((format(printf, 1, 2))) (*jb_output_function_t)(const char *fmt, ...);
 void jb_setoutput(jb_output_function_t err, jb_output_function_t warn, jb_output_function_t dbg);
 
+/*! \brief Checks if the given time stamp is late */
+int jb_is_late(jitterbuf *jb, long ts);
+
 #ifdef __cplusplus
 }
 #endif

diff --git a/main/abstract_jb.c b/main/abstract_jb.c
index b629fe826d2838c66e49ed8681f4da34c6cf473d..1c9eb5172fefa85d69cf9fde1c8ec17bc969b0c7 100644 (file)
--- a/main/abstract_jb.c
+++ b/main/abstract_jb.c
@@ -67,6 +67,7 @@ static long jb_next_fixed(void *jb);
 static int jb_remove_fixed(void *jb, struct ast_frame **fout);
 static void jb_force_resynch_fixed(void *jb);
 static void jb_empty_and_reset_fixed(void *jb);
+static int jb_is_late_fixed(void *jb, long ts);
 /* adaptive */
 static void * jb_create_adaptive(struct ast_jb_conf *general_config);
 static void jb_destroy_adaptive(void *jb);
@@ -77,6 +78,7 @@ static long jb_next_adaptive(void *jb);
 static int jb_remove_adaptive(void *jb, struct ast_frame **fout);
 static void jb_force_resynch_adaptive(void *jb);
 static void jb_empty_and_reset_adaptive(void *jb);
+static int jb_is_late_adaptive(void *jb, long ts);
 
 /* Available jb implementations */
 static const struct ast_jb_impl avail_impl[] = {
@@ -92,6 +94,7 @@ static const struct ast_jb_impl avail_impl[] = {
                .remove = jb_remove_fixed,
                .force_resync = jb_force_resynch_fixed,
                .empty_and_reset = jb_empty_and_reset_fixed,
+               .is_late = jb_is_late_fixed,
        },
        {
                .name = "adaptive",
@@ -105,6 +108,7 @@ static const struct ast_jb_impl avail_impl[] = {
                .remove = jb_remove_adaptive,
                .force_resync = jb_force_resynch_adaptive,
                .empty_and_reset = jb_empty_and_reset_adaptive,
+               .is_late = jb_is_late_adaptive,
        }
 };
 
@@ -706,6 +710,11 @@ static void jb_empty_and_reset_fixed(void *jb)
        }
 }
 
+static int jb_is_late_fixed(void *jb, long ts)
+{
+       return fixed_jb_is_late(jb, ts);
+}
+
 /* adaptive */
 
 static void *jb_create_adaptive(struct ast_jb_conf *general_config)
@@ -812,6 +821,11 @@ const struct ast_jb_impl *ast_jb_get_impl(enum ast_jb_type type)
        return NULL;
 }
 
+static int jb_is_late_adaptive(void *jb, long ts)
+{
+       return jb_is_late(jb, ts);
+}
+
 #define DEFAULT_TIMER_INTERVAL 20
 #define DEFAULT_SIZE  200
 #define DEFAULT_TARGET_EXTRA  40
@@ -895,7 +909,22 @@ static struct ast_frame *hook_event_cb(struct ast_channel *chan, struct ast_fram
                }
        }
 
-       if (!frame) {
+       /*
+        * If the frame has been requeued (for instance when the translate core returns
+        * more than one frame) then if the frame is late we want to immediately return
+        * it. Otherwise attempt to insert it into the jitterbuffer.
+        *
+        * If the frame is requeued and late then in all likely hood it's a frame that
+        * that was previously retrieved from the jitterbuffer, passed to the translate
+        * core, and then put back into the channel read queue. Even if it had not been
+        * in the jitterbuffer prior to now it needs to be the next frame "out".
+        *
+        * However late arriving frames that have not been requeued (i.e. regular frames)
+        * need to be passed to the jitterbuffer so they can be appropriately dropped. As
+        * well any requeued frames that are not late should be put into the jitterbuffer.
+        */
+       if (!frame || (ast_test_flag(frame, AST_FRFLAG_REQUEUED) &&
+                      framedata->jb_impl->is_late(framedata->jb_obj, frame->ts))) {
                return frame;
        }
 

diff --git a/main/channel.c b/main/channel.c
index 6083dcc4a1a93279f3df17b81147704f27007ad5..637488a9c51178c73c27df78ff82c020bc414e26 100644 (file)
--- a/main/channel.c
+++ b/main/channel.c
@@ -4335,12 +4335,19 @@ static struct ast_frame *__ast_read(struct ast_channel *chan, int dropaudio)
                         * at the end of the queue.
                         */
                        if (AST_LIST_NEXT(f, frame_list)) {
+                               struct ast_frame *cur, *multi_frame = AST_LIST_NEXT(f, frame_list);
+
+                               /* Mark these frames as being re-queued */
+                               for (cur = multi_frame; cur; cur = AST_LIST_NEXT(cur, frame_list)) {
+                                       ast_set_flag(cur, AST_FRFLAG_REQUEUED);
+                               }
+
                                if (!readq_tail) {
-                                       ast_queue_frame_head(chan, AST_LIST_NEXT(f, frame_list));
+                                       ast_queue_frame_head(chan, multi_frame);
                                } else {
-                                       __ast_queue_frame(chan, AST_LIST_NEXT(f, frame_list), 0, readq_tail);
+                                       __ast_queue_frame(chan, multi_frame, 0, readq_tail);
                                }
-                               ast_frfree(AST_LIST_NEXT(f, frame_list));
+                               ast_frfree(multi_frame);
                                AST_LIST_NEXT(f, frame_list) = NULL;
                        }
 

diff --git a/main/fixedjitterbuf.c b/main/fixedjitterbuf.c
index 17cad494ae8cbfb450f07a70a059de6ba64eaa45..ebc5259e9e50492b99ea203c9a042e2b1cc70405 100644 (file)
--- a/main/fixedjitterbuf.c
+++ b/main/fixedjitterbuf.c
@@ -196,7 +196,6 @@ int fixed_jb_put_first(struct fixed_jb *jb, void *data, long ms, long ts, long n
        return fixed_jb_put(jb, data, ms, ts, now);
 }
 
-
 int fixed_jb_put(struct fixed_jb *jb, void *data, long ms, long ts, long now)
 {
        struct fixed_jb_frame *frame, *next, *newframe;
@@ -349,3 +348,8 @@ int fixed_jb_remove(struct fixed_jb *jb, struct fixed_jb_frame *frameout)
 
        return FIXED_JB_OK;
 }
+
+int fixed_jb_is_late(struct fixed_jb *jb, long ts)
+{
+       return jb->rxcore + jb->delay + ts < jb->next_delivery;
+}

diff --git a/main/fixedjitterbuf.h b/main/fixedjitterbuf.h
index df9bbac5529d8c9964f98ef484f516ba836c1b36..ab8e5e2f886c9714076b5c094b112bc3da3d9a72 100644 (file)
--- a/main/fixedjitterbuf.h
+++ b/main/fixedjitterbuf.h
@@ -85,6 +85,9 @@ int fixed_jb_remove(struct fixed_jb *jb, struct fixed_jb_frame *frameout);
 
 void fixed_jb_set_force_resynch(struct fixed_jb *jb);
 
+/*! \brief Checks if the given time stamp is late */
+int fixed_jb_is_late(struct fixed_jb *jb, long ts);
+
 #if defined(__cplusplus) || defined(c_plusplus)
 }
 #endif

diff --git a/main/jitterbuf.c b/main/jitterbuf.c
index 1bfe508dac80fc5ddf772055d4ea1a2147b4afe9..df3164f39315c76c1f896be2722c55f9eb8c1ff5 100644 (file)
--- a/main/jitterbuf.c
+++ b/main/jitterbuf.c
@@ -845,4 +845,7 @@ enum jb_return_code jb_setconf(jitterbuf *jb, jb_conf *conf)
        return JB_OK;
 }
 
-
+int jb_is_late(jitterbuf *jb, long ts)
+{
+       return ts + jb->info.current < jb->info.next_voice_ts;
+}