]> git.ipfire.org Git - thirdparty/asterisk.git/commitdiff
Merged revisions 264997 via svnmerge from
authorMark Michelson <mmichelson@digium.com>
Fri, 21 May 2010 16:53:30 +0000 (16:53 +0000)
committerMark Michelson <mmichelson@digium.com>
Fri, 21 May 2010 16:53:30 +0000 (16:53 +0000)
https://origsvn.digium.com/svn/asterisk/trunk

................
  r264997 | mmichelson | 2010-05-21 11:44:27 -0500 (Fri, 21 May 2010) | 38 lines

  Merged revisions 264996 via svnmerge from
  https://origsvn.digium.com/svn/asterisk/branches/1.4

  ........
    r264996 | mmichelson | 2010-05-21 11:28:34 -0500 (Fri, 21 May 2010) | 32 lines

    Allow ast_safe_sleep to defer specific frames until after the sleep has concluded.

    From reviewboard

    Background:
    A Digium customer discovered a somewhat odd bug. The setup is that parties A
    and B are bridged, and party A places party B on hold. While party B is
    listening to hold music, he mashes a bunch of DTMF. Party A takes party
    B off hold while this is happening, but party B continues to hear hold
    music. I could reproduce this about 1 in 5 times.

    The issue:
    When DTMF features are enabled and a user presses keys, the channel that
    the DTMF is streamed to is placed in an ast_safe_sleep for 100 ms, the
    duration of the emulated tone. If an AST_CONTROL_UNHOLD frame is read
    from the channel during the sleep, the frame is dropped. Thus the
    unhold indication is never made to the channel that was originally placed
    on hold.

    The fix:
    Originally, I discussed with Kevin possible ways of fixing the specific
    problem reported. However, we determined that the same type of problem
    could happen in other situations where ast_safe_sleep() is used. Using
    autoservice as a model, I modified ast_safe_sleep_conditional() to
    defer specific frame types so they can be re-queued once the sleep has
    finished. I made a common function for determining if a frame should
    be deferred so that there are not two identical switch blocks to
    maintain.

    Review: https://reviewboard.asterisk.org/r/674/
  ........
................

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.6.2@264998 65c4cc65-6c06-0410-ace0-fbb531ad65f3

include/asterisk/channel.h
main/autoservice.c
main/channel.c

index 3d58c83f607cd7fd30b83d77649915b68e54e536..821ed09b0c9b846e0ef206454572da4172db95e1 100644 (file)
@@ -1135,7 +1135,24 @@ int ast_indicate_data(struct ast_channel *chan, int condition, const void *data,
   \return Returns < 0 on  failure, 0 if nothing ever arrived, and the # of ms remaining otherwise */
 int ast_waitfor(struct ast_channel *chan, int ms);
 
-/*! \brief Wait for a specified amount of time, looking for hangups
+/*!
+ * \brief Should we keep this frame for later?
+ *
+ * There are functions such as ast_safe_sleep which will
+ * service a channel to ensure that it does not have a
+ * have a large backlog of queued frames. When this happens,
+ * we want to hold on to specific frame types and just drop
+ * others. This function will tell if the frame we just
+ * read should be held onto.
+ *
+ * \param frame The frame we just read
+ * \retval 1 frame should be kept
+ * \retval 0 frame should be dropped
+ */
+int ast_is_deferrable_frame(const struct ast_frame *frame);
+
+/*!
+ * \brief Wait for a specified amount of time, looking for hangups
  * \param chan channel to wait for
  * \param ms length of time in milliseconds to sleep
  * Waits for a specified amount of time, servicing the channel as required.
index 4e1cd5203358bb2f62af0d7f47e20d858df995c9..0a963097d406df716ffebcb9ed6f695ece4d6bdd 100644 (file)
@@ -128,32 +128,8 @@ static void *autoservice_run(void *ign)
                         * thread in charge of this channel will know. */
 
                        defer_frame = &hangup_frame;
-               } else {
-
-                       /* Do not add a default entry in this switch statement.  Each new
-                        * frame type should be addressed directly as to whether it should
-                        * be queued up or not. */
-
-                       switch (f->frametype) {
-                       /* Save these frames */
-                       case AST_FRAME_DTMF_END:
-                       case AST_FRAME_CONTROL:
-                       case AST_FRAME_TEXT:
-                       case AST_FRAME_IMAGE:
-                       case AST_FRAME_HTML:
-                               defer_frame = f;
-                               break;
-
-                       /* Throw these frames away */
-                       case AST_FRAME_DTMF_BEGIN:
-                       case AST_FRAME_VOICE:
-                       case AST_FRAME_VIDEO:
-                       case AST_FRAME_NULL:
-                       case AST_FRAME_IAX:
-                       case AST_FRAME_CNG:
-                       case AST_FRAME_MODEM:
-                               break;
-                       }
+               } else if (ast_is_deferrable_frame(f)) {
+                       defer_frame = f;
                }
 
                if (defer_frame) {
index d8a83034d0b2bd26a606c1783807d1808373783a..426e6a7d943b26cbbc5bebbfb17bef2aa2ea1fa1 100644 (file)
@@ -1330,12 +1330,41 @@ struct ast_channel *ast_channel_search_locked(int (*is_match)(struct ast_channel
        return c;
 }
 
+int ast_is_deferrable_frame(const struct ast_frame *frame)
+{
+       /* Do not add a default entry in this switch statement.  Each new
+        * frame type should be addressed directly as to whether it should
+        * be queued up or not.
+        */
+       switch (frame->frametype) {
+       case AST_FRAME_DTMF_END:
+       case AST_FRAME_CONTROL:
+       case AST_FRAME_TEXT:
+       case AST_FRAME_IMAGE:
+       case AST_FRAME_HTML:
+               return 1;
+
+       case AST_FRAME_DTMF_BEGIN:
+       case AST_FRAME_VOICE:
+       case AST_FRAME_VIDEO:
+       case AST_FRAME_NULL:
+       case AST_FRAME_IAX:
+       case AST_FRAME_CNG:
+       case AST_FRAME_MODEM:
+               return 0;
+       }
+       return 0;
+}
+
 /*! \brief Wait, look for hangups and condition arg */
 int ast_safe_sleep_conditional(struct ast_channel *chan, int ms, int (*cond)(void*), void *data)
 {
        struct ast_frame *f;
        struct ast_silence_generator *silgen = NULL;
        int res = 0;
+       AST_LIST_HEAD_NOLOCK(, ast_frame) deferred_frames;
+
+       AST_LIST_HEAD_INIT_NOLOCK(&deferred_frames);
 
        /* If no other generator is present, start silencegen while waiting */
        if (ast_opt_transmit_silence && !chan->generatordata) {
@@ -1343,6 +1372,7 @@ int ast_safe_sleep_conditional(struct ast_channel *chan, int ms, int (*cond)(voi
        }
 
        while (ms > 0) {
+               struct ast_frame *dup_f = NULL;
                if (cond && ((*cond)(data) == 0)) {
                        break;
                }
@@ -1357,7 +1387,18 @@ int ast_safe_sleep_conditional(struct ast_channel *chan, int ms, int (*cond)(voi
                                res = -1;
                                break;
                        }
-                       ast_frfree(f);
+
+                       if (!ast_is_deferrable_frame(f)) {
+                               ast_frfree(f);
+                               continue;
+                       }
+                       
+                       if ((dup_f = ast_frisolate(f))) {
+                               if (dup_f != f) {
+                                       ast_frfree(f);
+                               }
+                               AST_LIST_INSERT_HEAD(&deferred_frames, dup_f, frame_list);
+                       }
                }
        }
 
@@ -1366,6 +1407,19 @@ int ast_safe_sleep_conditional(struct ast_channel *chan, int ms, int (*cond)(voi
                ast_channel_stop_silence_generator(chan, silgen);
        }
 
+       /* We need to free all the deferred frames, but we only need to
+        * queue the deferred frames if there was no error and no
+        * hangup was received
+        */
+       ast_channel_lock(chan);
+       while ((f = AST_LIST_REMOVE_HEAD(&deferred_frames, frame_list))) {
+               if (!res) {
+                       ast_queue_frame_head(chan, f);
+               }
+               ast_frfree(f);
+       }
+       ast_channel_unlock(chan);
+
        return res;
 }